ip: fix fib and mfib locks

This patches fixes an issue that could cause
fib locks to underflow: if an API user deletes
a fib and quickly recreates it, the fib may not
have been actually deleted. As a result, the
lock would not be incremented on the create call
leading to the fib potentially disappearing
afterwards - or to the lock to underflow when
the fib is deleted again.

In order to keep the existing API semantics,
we use the locks with API and CLI source as flags.
This means we need to use a different counter
for the interface-related locks.

This also prevents an issue where an interface being
bound to a vrf via API and released via CLI could
mess up the lock counter.

Finally, this will help with cleaning up the
interface-related locks on interface deletion
in a later patch.

Type: fix

Change-Id: I93030a7660646d6dd179ddf27fe4e708aa11b90e
Signed-off-by: Nathan Skrzypczak <nathan.skrzypczak@gmail.com>
Signed-off-by: Aloys Augustin <aloaugus@cisco.com>

3 files changed, 10 insertions, 23 deletions

diff --git a/src/vnet/mpls/interface.c b/src/vnet/mpls/interface.c
index e6c3dfeb801..5e80b9d0532 100644
--- a/src/vnet/mpls/interface.c
+++ b/src/vnet/mpls/interface.c
@@ -35,10 +35,8 @@ mpls_sw_interface_is_enabled (u32 sw_if_index)
 }
 
 int
-mpls_sw_interface_enable_disable (mpls_main_t * mm,
-                                  u32 sw_if_index,
-                                  u8 is_enable,
-                                  u8 is_api)
+mpls_sw_interface_enable_disable (mpls_main_t *mm, u32 sw_if_index,
+				  u8 is_enable)
 {
   fib_node_index_t lfib_index;
   vnet_main_t *vnm = vnet_get_main ();
@@ -60,8 +58,7 @@ mpls_sw_interface_enable_disable (mpls_main_t * mm,
       if (1 != ++mm->mpls_enabled_by_sw_if_index[sw_if_index])
           return (0);
 
-      fib_table_lock(lfib_index, FIB_PROTOCOL_MPLS,
-                     (is_api? FIB_SOURCE_API: FIB_SOURCE_CLI));
+      fib_table_lock (lfib_index, FIB_PROTOCOL_MPLS, FIB_SOURCE_INTERFACE);
 
       vec_validate(mm->fib_index_by_sw_if_index, sw_if_index);
       mm->fib_index_by_sw_if_index[sw_if_index] = lfib_index;
@@ -72,9 +69,8 @@ mpls_sw_interface_enable_disable (mpls_main_t * mm,
       if (0 != --mm->mpls_enabled_by_sw_if_index[sw_if_index])
           return (0);
 
-      fib_table_unlock(mm->fib_index_by_sw_if_index[sw_if_index],
-		       FIB_PROTOCOL_MPLS,
-                       (is_api? FIB_SOURCE_API: FIB_SOURCE_CLI));
+      fib_table_unlock (mm->fib_index_by_sw_if_index[sw_if_index],
+			FIB_PROTOCOL_MPLS, FIB_SOURCE_INTERFACE);
     }
 
   vnet_feature_enable_disable ("mpls-input", "mpls-not-enabled",
@@ -118,7 +114,7 @@ mpls_interface_enable_disable (vlib_main_t * vm,
       goto done;
     }
 
-  rv = mpls_sw_interface_enable_disable(&mpls_main, sw_if_index, enable, 0);
+  rv = mpls_sw_interface_enable_disable (&mpls_main, sw_if_index, enable);
 
   if (VNET_API_ERROR_NO_SUCH_FIB == rv)
       error = clib_error_return (0, "default MPLS table must be created first");
diff --git a/src/vnet/mpls/mpls.h b/src/vnet/mpls/mpls.h
index 00b493f4576..b4f90a13f3c 100644
--- a/src/vnet/mpls/mpls.h
+++ b/src/vnet/mpls/mpls.h
@@ -85,9 +85,8 @@ unformat_function_t unformat_mpls_unicast_label;
 unformat_function_t unformat_mpls_header;
 unformat_function_t unformat_pg_mpls_header;
 
-int mpls_sw_interface_enable_disable (mpls_main_t * mm,
-				      u32 sw_if_index,
-				      u8 is_enable, u8 is_api);
+int mpls_sw_interface_enable_disable (mpls_main_t *mm, u32 sw_if_index,
+				      u8 is_enable);
 
 u8 mpls_sw_interface_is_enabled (u32 sw_if_index);
 
diff --git a/src/vnet/mpls/mpls_api.c b/src/vnet/mpls/mpls_api.c
index e89732f0d10..4efb61786ad 100644
--- a/src/vnet/mpls/mpls_api.c
+++ b/src/vnet/mpls/mpls_api.c
@@ -210,8 +210,6 @@ vl_api_mpls_route_add_del_t_handler (vl_api_mpls_route_add_del_t * mp)
 void
 mpls_table_create (u32 table_id, u8 is_api, const u8 * name)
 {
-  u32 fib_index;
-
   /*
    * The MPLS defult table must also be explicitly created via the API.
    * So in contrast to IP, it gets no special treatment here.
@@ -222,16 +220,11 @@ mpls_table_create (u32 table_id, u8 is_api, const u8 * name)
    * i.e. it can be added many times via the API but needs to be
    * deleted only once.
    */
-  fib_index = fib_table_find (FIB_PROTOCOL_MPLS, table_id);
-
-  if (~0 == fib_index)
-    {
       fib_table_find_or_create_and_lock_w_name (FIB_PROTOCOL_MPLS,
 						table_id,
 						(is_api ?
 						 FIB_SOURCE_API :
 						 FIB_SOURCE_CLI), name);
-    }
 }
 
 static void
@@ -295,9 +288,8 @@ static void
 
   VALIDATE_SW_IF_INDEX (mp);
 
-  rv = mpls_sw_interface_enable_disable (&mpls_main,
-					 ntohl (mp->sw_if_index),
-					 mp->enable, 1);
+  rv = mpls_sw_interface_enable_disable (&mpls_main, ntohl (mp->sw_if_index),
+					 mp->enable);
 
   BAD_SW_IF_INDEX_LABEL;
   REPLY_MACRO (VL_API_SW_INTERFACE_SET_MPLS_ENABLE_REPLY);