fib: Use the same adjacency that BFD is using

Type: improvement

When the adj subsystem is notified of a BFD session, it attempts to find the appropriate adjacency from the session's key.
This could lead to a mismatch between the adj used by BFD and that of FIB. The BFD session stores the adj it is using, so FIB uses that instead.
Since adj is now using the same adj as BFD, it does not need to maintain its own locks.
In BFD it is necessary to initialise the adj index used in INVALID and ensure it is not unlock before listeners are notified of the session delete.

Signed-off-by: Neale Ranns <neale@graphiant.com>
Change-Id: I9630867b10bb18969475299a0c754942a8df0f44

diff --git a/src/plugins/unittest/fib_test.c b/src/plugins/unittest/fib_test.c
index cc9a572..57ab29e 100644 (file)
--- a/src/plugins/unittest/fib_test.c
+++ b/src/plugins/unittest/fib_test.c
@@ -8408,12 +8408,14 @@ fib_test_bfd (void)
     bfd_10_10_10_1.hop_type = BFD_HOP_TYPE_SINGLE;
     bfd_10_10_10_1.udp.key.sw_if_index = tm->hw[0]->sw_if_index;
 
-    adj_bfd_notify(BFD_LISTEN_EVENT_CREATE, &bfd_10_10_10_1);
-
     ai_10_10_10_1 = adj_nbr_add_or_lock(FIB_PROTOCOL_IP4,
                                         VNET_LINK_IP4,
                                         &nh_10_10_10_1,
                                         tm->hw[0]->sw_if_index);
+    bfd_10_10_10_1.udp.adj_index = ai_10_10_10_1;
+
+    adj_bfd_notify(BFD_LISTEN_EVENT_CREATE, &bfd_10_10_10_1);
+
     /*
      * whilst the BFD session is not signalled, the adj is up
      */

diff --git a/src/vnet/adj/adj_bfd.c b/src/vnet/adj/adj_bfd.c
index 2d787d4..c1f02dd 100644 (file)
--- a/src/vnet/adj/adj_bfd.c
+++ b/src/vnet/adj/adj_bfd.c
@@ -114,9 +114,7 @@ void
 adj_bfd_notify (bfd_listen_event_e event,
                 const bfd_session_t *session)
 {
-    const bfd_udp_key_t *key;
     adj_bfd_delegate_t *abd;
-    fib_protocol_t fproto;
     adj_delegate_t *aed;
     adj_index_t ai;
 
@@ -129,19 +127,28 @@ adj_bfd_notify (bfd_listen_event_e event,
         return;
     }
 
-    key = &session->udp.key;
-
-    fproto = (ip46_address_is_ip4 (&key->peer_addr) ?
-              FIB_PROTOCOL_IP4:
-              FIB_PROTOCOL_IP6);
+    switch (session->transport)
+    {
+    case BFD_TRANSPORT_UDP4:
+    case BFD_TRANSPORT_UDP6:
+        /*
+         * pick up the same adjacency that the BFD session is using
+         * to send. The BFD session is holding a lock on this adj.
+         */
+        ai = session->udp.adj_index;
+        break;
+    default:
+        /*
+         * Don't know what adj this session uses
+         */
+        return;
+    }
 
-    /*
-     * find the adj that corresponds to the BFD session.
-     */
-    ai = adj_nbr_add_or_lock(fproto,
-                             fib_proto_to_link(fproto),
-                             &key->peer_addr,
-                             key->sw_if_index);
+    if (INDEX_INVALID == ai)
+    {
+        /* No associated Adjacency with the session */
+        return;
+    }
 
     switch (event)
     {
@@ -159,13 +166,6 @@ adj_bfd_notify (bfd_listen_event_e event,
         }
         else
         {
-            /*
-             * lock the adj. add the delegate.
-             * Locking the adj prevents it being removed and thus maintains
-             * the BFD derived states
-             */
-            adj_lock(ai);
-
             /*
              * allocate and init a new delegate struct
              */
@@ -213,14 +213,12 @@ adj_bfd_notify (bfd_listen_event_e event,
         {
             /*
              * has an associated BFD tracking delegate
-             * remove the BFD tracking delegate, update children, then
-             * unlock the adj
+             * remove the BFD tracking delegate, update children
              */
             adj_delegate_remove(ai, ADJ_DELEGATE_BFD);
             pool_put(abd_pool, abd);
 
             adj_bfd_update_walk(ai);
-            adj_unlock(ai);
         }
         /*
          * else
@@ -228,11 +226,6 @@ adj_bfd_notify (bfd_listen_event_e event,
          */
         break;
     }
-
-    /*
-     * unlock match of the add-or-lock at the start
-     */
-    adj_unlock(ai);
 }
 
 int

diff --git a/src/vnet/bfd/bfd_main.c b/src/vnet/bfd/bfd_main.c
index a9dd60e..f77d66c 100644 (file)
--- a/src/vnet/bfd/bfd_main.c
+++ b/src/vnet/bfd/bfd_main.c
@@ -465,6 +465,13 @@ bfd_session_start (bfd_main_t * bm, bfd_session_t * bs)
   bfd_notify_listeners (bm, BFD_LISTEN_EVENT_CREATE, bs);
 }
 
+void
+bfd_session_stop (bfd_main_t *bm, bfd_session_t *bs)
+{
+  BFD_DBG ("\nStopping session: %U", format_bfd_session, bs);
+  bfd_notify_listeners (bm, BFD_LISTEN_EVENT_DELETE, bs);
+}
+
 void
 bfd_session_set_flags (vlib_main_t * vm, bfd_session_t * bs, u8 admin_up_down)
 {
@@ -1406,7 +1413,6 @@ bfd_put_session (bfd_main_t * bm, bfd_session_t * bs)
 
   vlib_log_info (bm->log_class, "delete session: %U",
                 format_bfd_session_brief, bs);
-  bfd_notify_listeners (bm, BFD_LISTEN_EVENT_DELETE, bs);
   if (bs->auth.curr_key)
     {
       --bs->auth.curr_key->use_count;

diff --git a/src/vnet/bfd/bfd_udp.c b/src/vnet/bfd/bfd_udp.c
index 3a25830..074d546 100644 (file)
--- a/src/vnet/bfd/bfd_udp.c
+++ b/src/vnet/bfd/bfd_udp.c
@@ -527,6 +527,7 @@ bfd_udp_add_session_internal (vlib_main_t * vm, bfd_udp_main_t * bum,
     }
   bfd_udp_session_t *bus = &bs->udp;
   clib_memset (bus, 0, sizeof (*bus));
+  bus->adj_index = ADJ_INDEX_INVALID;
   bfd_udp_key_t *key = &bus->key;
   bfd_udp_key_init (key, sw_if_index, local_addr, peer_addr);
   const bfd_session_t *tmp = bfd_lookup_session (bum, key);
@@ -745,6 +746,7 @@ bfd_udp_del_session_internal (vlib_main_t * vm, bfd_session_t * bs)
 {
   bfd_udp_main_t *bum = &bfd_udp_main;
   BFD_DBG ("free bfd-udp session, bs_idx=%d", bs->bs_idx);
+  bfd_session_stop (bum->bfd_main, bs);
   mhash_unset (&bum->bfd_session_idx_by_bfd_key, &bs->udp.key, NULL);
   adj_unlock (bs->udp.adj_index);
   switch (bs->transport)