ip: unlock_fib on if delete

On interface delete we were not removing
the lock taken by a previous ip_table_bind()
call thus preventing the VRFs to be removed.

Type: fix

Change-Id: I11abbb51a09b45cd3390b23d5d601d029c5ea485
Signed-off-by: Nathan Skrzypczak <nathan.skrzypczak@gmail.com>
diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c
index bbb6168..a765c0b 100644
--- a/src/vnet/interface_api.c
+++ b/src/vnet/interface_api.c
@@ -470,6 +470,107 @@
   REPLY_MACRO (VL_API_SW_INTERFACE_SET_TABLE_REPLY);
 }
 
+void
+fib_table_bind (fib_protocol_t fproto, u32 sw_if_index, u32 fib_index)
+{
+  u32 table_id;
+
+  table_id = fib_table_get_table_id (fib_index, fproto);
+  ASSERT (table_id != ~0);
+
+  if (FIB_PROTOCOL_IP6 == fproto)
+    {
+      /*
+       * tell those that are interested that the binding is changing.
+       */
+      ip6_table_bind_callback_t *cb;
+      vec_foreach (cb, ip6_main.table_bind_callbacks)
+	cb->function (&ip6_main, cb->function_opaque,
+		      sw_if_index,
+		      fib_index,
+		      ip6_main.fib_index_by_sw_if_index[sw_if_index]);
+
+      /* unlock currently assigned tables */
+      if (0 != ip6_main.fib_index_by_sw_if_index[sw_if_index])
+	fib_table_unlock (ip6_main.fib_index_by_sw_if_index[sw_if_index],
+			  FIB_PROTOCOL_IP6, FIB_SOURCE_INTERFACE);
+
+      if (0 != table_id)
+	{
+	  /* we need to lock the table now it's inuse */
+	  fib_table_lock (fib_index, FIB_PROTOCOL_IP6, FIB_SOURCE_INTERFACE);
+	}
+
+      ip6_main.fib_index_by_sw_if_index[sw_if_index] = fib_index;
+    }
+  else
+    {
+      /*
+       * tell those that are interested that the binding is changing.
+       */
+      ip4_table_bind_callback_t *cb;
+      vec_foreach (cb, ip4_main.table_bind_callbacks)
+	cb->function (&ip4_main, cb->function_opaque,
+		      sw_if_index,
+		      fib_index,
+		      ip4_main.fib_index_by_sw_if_index[sw_if_index]);
+
+      /* unlock currently assigned tables */
+      if (0 != ip4_main.fib_index_by_sw_if_index[sw_if_index])
+	fib_table_unlock (ip4_main.fib_index_by_sw_if_index[sw_if_index],
+			  FIB_PROTOCOL_IP4, FIB_SOURCE_INTERFACE);
+
+      if (0 != table_id)
+	{
+	  /* we need to lock the table now it's inuse */
+	  fib_index = fib_table_find_or_create_and_lock (
+	    FIB_PROTOCOL_IP4, table_id, FIB_SOURCE_INTERFACE);
+	}
+
+      ip4_main.fib_index_by_sw_if_index[sw_if_index] = fib_index;
+    }
+}
+
+void
+mfib_table_bind (fib_protocol_t fproto, u32 sw_if_index, u32 mfib_index)
+{
+  u32 table_id;
+
+  table_id = mfib_table_get_table_id (mfib_index, fproto);
+  ASSERT (table_id != ~0);
+
+  if (FIB_PROTOCOL_IP6 == fproto)
+    {
+      if (0 != ip6_main.mfib_index_by_sw_if_index[sw_if_index])
+	mfib_table_unlock (ip6_main.mfib_index_by_sw_if_index[sw_if_index],
+			   FIB_PROTOCOL_IP6, MFIB_SOURCE_INTERFACE);
+
+      if (0 != table_id)
+	{
+	  /* we need to lock the table now it's inuse */
+	  mfib_table_lock (mfib_index, FIB_PROTOCOL_IP6,
+			   MFIB_SOURCE_INTERFACE);
+	}
+
+      ip6_main.mfib_index_by_sw_if_index[sw_if_index] = mfib_index;
+    }
+  else
+    {
+      if (0 != ip4_main.mfib_index_by_sw_if_index[sw_if_index])
+	mfib_table_unlock (ip4_main.mfib_index_by_sw_if_index[sw_if_index],
+			   FIB_PROTOCOL_IP4, MFIB_SOURCE_INTERFACE);
+
+      if (0 != table_id)
+	{
+	  /* we need to lock the table now it's inuse */
+	  mfib_index = mfib_table_find_or_create_and_lock (
+	    FIB_PROTOCOL_IP4, table_id, MFIB_SOURCE_INTERFACE);
+	}
+
+      ip4_main.mfib_index_by_sw_if_index[sw_if_index] = mfib_index;
+    }
+}
+
 int
 ip_table_bind (fib_protocol_t fproto, u32 sw_if_index, u32 table_id)
 {
@@ -487,100 +588,24 @@
       return (VNET_API_ERROR_NO_SUCH_FIB);
     }
 
-  if (FIB_PROTOCOL_IP6 == fproto)
-    {
-      /*
-       * If the interface already has in IP address, then a change int
-       * VRF is not allowed. The IP address applied must first be removed.
-       * We do not do that automatically here, since VPP has no knowledge
-       * of whether those subnets are valid in the destination VRF.
-       */
-      /* *INDENT-OFF* */
-      foreach_ip_interface_address (&ip6_main.lookup_main,
-				    ia, sw_if_index,
-				    1 /* honor unnumbered */ ,
-      ({
-        return (VNET_API_ERROR_ADDRESS_FOUND_FOR_INTERFACE);
-      }));
-      /* *INDENT-ON* */
+  /*
+   * If the interface already has in IP address, then a change int
+   * VRF is not allowed. The IP address applied must first be removed.
+   * We do not do that automatically here, since VPP has no knowledge
+   * of whether those subnets are valid in the destination VRF.
+   */
+  /* clang-format off */
+  foreach_ip_interface_address (FIB_PROTOCOL_IP6 == fproto ?
+				&ip6_main.lookup_main : &ip4_main.lookup_main,
+				ia, sw_if_index,
+				1 /* honor unnumbered */ ,
+  ({
+    return (VNET_API_ERROR_ADDRESS_FOUND_FOR_INTERFACE);
+  }));
+  /* clang-format on */
 
-      /*
-       * tell those that are interested that the binding is changing.
-       */
-      ip6_table_bind_callback_t *cb;
-      vec_foreach (cb, ip6_main.table_bind_callbacks)
-	cb->function (&ip6_main, cb->function_opaque,
-		      sw_if_index,
-		      fib_index,
-		      ip6_main.fib_index_by_sw_if_index[sw_if_index]);
-
-      /* unlock currently assigned tables */
-      if (0 != ip6_main.fib_index_by_sw_if_index[sw_if_index])
-	fib_table_unlock (ip6_main.fib_index_by_sw_if_index[sw_if_index],
-			  FIB_PROTOCOL_IP6, FIB_SOURCE_INTERFACE);
-      if (0 != ip6_main.mfib_index_by_sw_if_index[sw_if_index])
-	mfib_table_unlock (ip6_main.mfib_index_by_sw_if_index[sw_if_index],
-			   FIB_PROTOCOL_IP6, MFIB_SOURCE_INTERFACE);
-
-      if (0 != table_id)
-	{
-	  /* we need to lock the table now it's inuse */
-	  fib_table_lock (fib_index, FIB_PROTOCOL_IP6, FIB_SOURCE_INTERFACE);
-	  mfib_table_lock (mfib_index, FIB_PROTOCOL_IP6,
-			   MFIB_SOURCE_INTERFACE);
-	}
-
-      ip6_main.fib_index_by_sw_if_index[sw_if_index] = fib_index;
-      ip6_main.mfib_index_by_sw_if_index[sw_if_index] = mfib_index;
-    }
-  else
-    {
-      /*
-       * If the interface already has in IP address, then a change int
-       * VRF is not allowed. The IP address applied must first be removed.
-       * We do not do that automatically here, since VPP has no knowledge
-       * of whether those subnets are valid in the destination VRF.
-       */
-      /* *INDENT-OFF* */
-      foreach_ip_interface_address (&ip4_main.lookup_main,
-				    ia, sw_if_index,
-				    1 /* honor unnumbered */ ,
-      ({
-        return (VNET_API_ERROR_ADDRESS_FOUND_FOR_INTERFACE);
-      }));
-      /* *INDENT-ON* */
-
-      /*
-       * tell those that are interested that the binding is changing.
-       */
-      ip4_table_bind_callback_t *cb;
-      vec_foreach (cb, ip4_main.table_bind_callbacks)
-	cb->function (&ip4_main, cb->function_opaque,
-		      sw_if_index,
-		      fib_index,
-		      ip4_main.fib_index_by_sw_if_index[sw_if_index]);
-
-      /* unlock currently assigned tables */
-      if (0 != ip4_main.fib_index_by_sw_if_index[sw_if_index])
-	fib_table_unlock (ip4_main.fib_index_by_sw_if_index[sw_if_index],
-			  FIB_PROTOCOL_IP4, FIB_SOURCE_INTERFACE);
-      if (0 != ip4_main.mfib_index_by_sw_if_index[sw_if_index])
-	mfib_table_unlock (ip4_main.mfib_index_by_sw_if_index[sw_if_index],
-			   FIB_PROTOCOL_IP4, MFIB_SOURCE_INTERFACE);
-
-      if (0 != table_id)
-	{
-	  /* we need to lock the table now it's inuse */
-	  fib_index = fib_table_find_or_create_and_lock (
-	    FIB_PROTOCOL_IP4, table_id, FIB_SOURCE_INTERFACE);
-
-	  mfib_index = mfib_table_find_or_create_and_lock (
-	    FIB_PROTOCOL_IP4, table_id, MFIB_SOURCE_INTERFACE);
-	}
-
-      ip4_main.fib_index_by_sw_if_index[sw_if_index] = fib_index;
-      ip4_main.mfib_index_by_sw_if_index[sw_if_index] = mfib_index;
-    }
+  fib_table_bind (fproto, sw_if_index, fib_index);
+  mfib_table_bind (fproto, sw_if_index, mfib_index);
 
   return (0);
 }
diff --git a/src/vnet/ip/ip.h b/src/vnet/ip/ip.h
index 131d687..c2a3801 100644
--- a/src/vnet/ip/ip.h
+++ b/src/vnet/ip/ip.h
@@ -267,6 +267,8 @@
 
 void ip_table_delete (fib_protocol_t fproto, u32 table_id, u8 is_api);
 
+void fib_table_bind (fib_protocol_t fproto, u32 sw_if_index, u32 fib_index);
+void mfib_table_bind (fib_protocol_t fproto, u32 sw_if_index, u32 mfib_index);
 int ip_table_bind (fib_protocol_t fproto, u32 sw_if_index, u32 table_id);
 
 u32 ip_table_get_unused_id (fib_protocol_t fproto);
diff --git a/src/vnet/ip/ip4_forward.c b/src/vnet/ip/ip4_forward.c
index 58af706..de8e8e8 100644
--- a/src/vnet/ip/ip4_forward.c
+++ b/src/vnet/ip/ip4_forward.c
@@ -1090,6 +1090,15 @@
       }));
       /* *INDENT-ON* */
       ip4_mfib_interface_enable_disable (sw_if_index, 0);
+
+      if (0 != im4->fib_index_by_sw_if_index[sw_if_index])
+	fib_table_bind (FIB_PROTOCOL_IP4, sw_if_index, 0);
+      if (0 != im4->mfib_index_by_sw_if_index[sw_if_index])
+	mfib_table_bind (FIB_PROTOCOL_IP4, sw_if_index, 0);
+
+      /* Erase the lookup tables just in case */
+      im4->fib_index_by_sw_if_index[sw_if_index] = ~0;
+      im4->mfib_index_by_sw_if_index[sw_if_index] = ~0;
     }
 
   vnet_feature_enable_disable ("ip4-unicast", "ip4-not-enabled", sw_if_index,
diff --git a/src/vnet/ip/ip6_forward.c b/src/vnet/ip/ip6_forward.c
index 833ce14..9ee3d11 100644
--- a/src/vnet/ip/ip6_forward.c
+++ b/src/vnet/ip/ip6_forward.c
@@ -717,6 +717,15 @@
       }));
       /* *INDENT-ON* */
       ip6_mfib_interface_enable_disable (sw_if_index, 0);
+
+      if (0 != im6->fib_index_by_sw_if_index[sw_if_index])
+	fib_table_bind (FIB_PROTOCOL_IP6, sw_if_index, 0);
+      if (0 != im6->mfib_index_by_sw_if_index[sw_if_index])
+	mfib_table_bind (FIB_PROTOCOL_IP6, sw_if_index, 0);
+
+      /* Erase the lookup tables just in case */
+      im6->fib_index_by_sw_if_index[sw_if_index] = ~0;
+      im6->mfib_index_by_sw_if_index[sw_if_index] = ~0;
     }
 
   vnet_feature_enable_disable ("ip6-unicast", "ip6-not-enabled", sw_if_index,