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

4 files changed, 35 insertions, 32 deletions

diff --git a/src/plugins/unittest/fib_test.c b/src/plugins/unittest/fib_test.c
index cc9a572b5ce..57ab29e1f72 100644
--- 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 2d787d41ab6..c1f02dd9073 100644
--- 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)
     {
@@ -160,13 +167,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
              */
             pool_get(abd_pool, abd);
@@ -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 a9dd60e2b20..f77d66c9db8 100644
--- a/src/vnet/bfd/bfd_main.c
+++ b/src/vnet/bfd/bfd_main.c
@@ -466,6 +466,13 @@ bfd_session_start (bfd_main_t * bm, bfd_session_t * 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)
 {
   bfd_main_t *bm = &bfd_main;
@@ -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 3a25830cfb5..074d546c37b 100644
--- 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)