Changing the IP table for an interface is an error if the interface already has an address configured (VPP-601)

Change-Id: I311fc264f73dd3b2b3ce9d7d1c33cd0515b36c4a
Signed-off-by: Neale Ranns <nranns@cisco.com>

13 files changed, 135 insertions, 70 deletions

diff --git a/src/vnet/adj/adj.c b/src/vnet/adj/adj.c
index a99f173f6d0..11f1680d329 100644
--- a/src/vnet/adj/adj.c
+++ b/src/vnet/adj/adj.c
@@ -441,25 +441,3 @@ VLIB_CLI_COMMAND (adj_show_command, static) = {
     .short_help = "show adj [<adj_index>] [interface]",
     .function = adj_show,
 };
-
-/* 
- * DEPRECATED: DO NOT USE
- */
-ip_adjacency_t *
-ip_add_adjacency (ip_lookup_main_t * lm,
-		  ip_adjacency_t * copy_adj,
-		  u32 n_adj,
-		  u32 * adj_index_return)
-{
-  ip_adjacency_t * adj;
-
-  ASSERT(1==n_adj);
-
-  adj = adj_alloc(FIB_PROTOCOL_IP4);
-
-  if (copy_adj)
-      *adj = *copy_adj;
-
-  *adj_index_return = adj_get_index(adj);
-  return adj;
-}
diff --git a/src/vnet/api_errno.h b/src/vnet/api_errno.h
index a5bcb3776bb..5e65ac7bccd 100644
--- a/src/vnet/api_errno.h
+++ b/src/vnet/api_errno.h
@@ -102,7 +102,8 @@ _(URI_FIFO_CREATE_FAILED, -109, "URI FIFO segment create failed")       \
 _(LISP_RLOC_LOCAL, -110, "RLOC address is local")                       \
 _(BFD_EAGAIN, -111, "BFD object cannot be manipulated at this time")	\
 _(INVALID_GPE_MODE, -112, "Invalid GPE mode")                           \
-_(LISP_GPE_ENTRIES_PRESENT, -113, "LISP GPE entries are present")
+_(LISP_GPE_ENTRIES_PRESENT, -113, "LISP GPE entries are present")       \
+_(ADDRESS_FOUND_FOR_INTERFACE, -114, "Address found for interface")
 
 typedef enum
 {
diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c
index 60cd6d40368..f94928b6dad 100644
--- a/src/vnet/interface_api.c
+++ b/src/vnet/interface_api.c
@@ -25,6 +25,7 @@
 #include <vnet/ethernet/ethernet.h>
 #include <vnet/ip/ip.h>
 #include <vnet/fib/fib_table.h>
+#include <vnet/mfib/mfib_table.h>
 #include <vnet/l2/l2_vtr.h>
 #include <vnet/vnet_msg_enum.h>
 #include <vnet/fib/fib_api.h>
@@ -318,6 +319,7 @@ vl_api_sw_interface_set_table_t_handler (vl_api_sw_interface_set_table_t * mp)
   u32 table_id = ntohl (mp->vrf_id);
   u32 sw_if_index = ntohl (mp->sw_if_index);
   vl_api_sw_interface_set_table_reply_t *rmp;
+  CLIB_UNUSED (ip_interface_address_t * ia);
   u32 fib_index;
 
   VALIDATE_SW_IF_INDEX (mp);
@@ -326,21 +328,51 @@ vl_api_sw_interface_set_table_t_handler (vl_api_sw_interface_set_table_t * mp)
 
   if (mp->is_ipv6)
     {
+      /* *INDENT-OFF* */
+      foreach_ip_interface_address (&ip6_main.lookup_main,
+				    ia, sw_if_index,
+				    1 /* honor unnumbered */ ,
+      ({
+        rv = VNET_API_ERROR_ADDRESS_FOUND_FOR_INTERFACE;
+        goto done;
+      }));
+      /* *INDENT-ON* */
+
       fib_index = fib_table_find_or_create_and_lock (FIB_PROTOCOL_IP6,
 						     table_id);
 
       vec_validate (ip6_main.fib_index_by_sw_if_index, sw_if_index);
       ip6_main.fib_index_by_sw_if_index[sw_if_index] = fib_index;
+      fib_index = mfib_table_find_or_create_and_lock (FIB_PROTOCOL_IP6,
+						      table_id);
+      vec_validate (ip6_main.mfib_index_by_sw_if_index, sw_if_index);
+      ip6_main.mfib_index_by_sw_if_index[sw_if_index] = fib_index;
     }
   else
     {
+      /* *INDENT-OFF* */
+      foreach_ip_interface_address (&ip4_main.lookup_main,
+				    ia, sw_if_index,
+				    1 /* honor unnumbered */ ,
+      ({
+        rv = VNET_API_ERROR_ADDRESS_FOUND_FOR_INTERFACE;
+        goto done;
+      }));
+      /* *INDENT-ON* */
 
       fib_index = fib_table_find_or_create_and_lock (FIB_PROTOCOL_IP4,
 						     table_id);
 
       vec_validate (ip4_main.fib_index_by_sw_if_index, sw_if_index);
       ip4_main.fib_index_by_sw_if_index[sw_if_index] = fib_index;
+
+      fib_index = mfib_table_find_or_create_and_lock (FIB_PROTOCOL_IP4,
+						      table_id);
+      vec_validate (ip4_main.mfib_index_by_sw_if_index, sw_if_index);
+      ip4_main.mfib_index_by_sw_if_index[sw_if_index] = fib_index;
     }
+
+done:
   stats_dsunlock ();
 
   BAD_SW_IF_INDEX_LABEL;
diff --git a/src/vnet/ip/ip4_forward.c b/src/vnet/ip/ip4_forward.c
index 66d91ab697d..fe4d6767d0c 100644
--- a/src/vnet/ip/ip4_forward.c
+++ b/src/vnet/ip/ip4_forward.c
@@ -2781,6 +2781,7 @@ add_del_interface_table (vlib_main_t * vm,
 			 unformat_input_t * input, vlib_cli_command_t * cmd)
 {
   vnet_main_t *vnm = vnet_get_main ();
+  ip_interface_address_t *ia;
   clib_error_t *error = 0;
   u32 sw_if_index, table_id;
 
@@ -2802,29 +2803,44 @@ add_del_interface_table (vlib_main_t * vm,
       goto done;
     }
 
-  {
-    ip4_main_t *im = &ip4_main;
-    u32 fib_index;
-
-    fib_index = fib_table_find_or_create_and_lock (FIB_PROTOCOL_IP4,
-						   table_id);
-
-    //
-    // FIXME-LATER
-    //  changing an interface's table has consequences for any connecteds
-    //  and adj-fibs already installed.
-    //
-    vec_validate (im->fib_index_by_sw_if_index, sw_if_index);
-    im->fib_index_by_sw_if_index[sw_if_index] = fib_index;
-
-    fib_index = mfib_table_find_or_create_and_lock (FIB_PROTOCOL_IP4,
-                                                   table_id);
-    vec_validate (im->mfib_index_by_sw_if_index, sw_if_index);
-    im->mfib_index_by_sw_if_index[sw_if_index] = fib_index;
-  }
+  /*
+   * 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 thoses subnets are valid in the destination VRF.
+   */
+  /* *INDENT-OFF* */
+  foreach_ip_interface_address (&ip4_main.lookup_main,
+                                ia, sw_if_index,
+                                1 /* honor unnumbered */,
+  ({
+      ip4_address_t * a;
+
+      a = ip_interface_address_get_address (&ip4_main.lookup_main, ia);
+      error = clib_error_return (0, "interface %U has address %U",
+                                 format_vnet_sw_if_index_name, vnm,
+                                 sw_if_index,
+                                 format_ip4_address, a);
+      goto done;
+   }));
+   /* *INDENT-ON* */
+
+{
+  ip4_main_t *im = &ip4_main;
+  u32 fib_index;
+
+  fib_index = fib_table_find_or_create_and_lock (FIB_PROTOCOL_IP4, table_id);
+
+  vec_validate (im->fib_index_by_sw_if_index, sw_if_index);
+  im->fib_index_by_sw_if_index[sw_if_index] = fib_index;
+
+  fib_index = mfib_table_find_or_create_and_lock (FIB_PROTOCOL_IP4, table_id);
+  vec_validate (im->mfib_index_by_sw_if_index, sw_if_index);
+  im->mfib_index_by_sw_if_index[sw_if_index] = fib_index;
+}
 
 done:
-  return error;
+return error;
 }
 
 /*?
@@ -2835,13 +2851,12 @@ done:
  * an IP Address is assigned to an interface in the table (which adds a route
  * automatically).
  *
- * @note IP addresses added after setting the interface IP table end up in
- * the indicated FIB table. If the IP address is added prior to adding the
- * interface to the FIB table, it will NOT be part of the FIB table. Predictable
- * but potentially counter-intuitive results occur if you provision interface
- * addresses in multiple FIBs. Upon RX, packets will be processed in the last
- * IP table ID provisioned. It might be marginally useful to evade source RPF
- * drops to put an interface address into multiple FIBs.
+ * @note IP addresses added after setting the interface IP table are added to
+ * the indicated FIB table. If an IP address is added prior to changing the
+ * table then this is an error. The control plane must remove these addresses
+ * first and then change the table. VPP will not automatically move the
+ * addresses from the old to the new table as it does not know the validity
+ * of such a change.
  *
  * @cliexpar
  * Example of how to add an interface to an IPv4 FIB table (where 2 is the table-id):
diff --git a/src/vnet/ip/ip6_forward.c b/src/vnet/ip/ip6_forward.c
index 5dd22b99ca2..6f77c6dd69d 100644
--- a/src/vnet/ip/ip6_forward.c
+++ b/src/vnet/ip/ip6_forward.c
@@ -415,9 +415,6 @@ ip6_sw_interface_enable_disable (u32 sw_if_index, u32 is_enable)
 	return;
     }
 
-  if (sw_if_index != 0)
-    ip6_mfib_interface_enable_disable (sw_if_index, is_enable);
-
   vnet_feature_enable_disable ("ip6-unicast", "ip6-lookup", sw_if_index,
 			       is_enable, 0, 0);
 
@@ -2972,6 +2969,7 @@ add_del_ip6_interface_table (vlib_main_t * vm,
 			     vlib_cli_command_t * cmd)
 {
   vnet_main_t *vnm = vnet_get_main ();
+  ip_interface_address_t *ia;
   clib_error_t *error = 0;
   u32 sw_if_index, table_id;
 
@@ -2993,6 +2991,28 @@ add_del_ip6_interface_table (vlib_main_t * vm,
       goto done;
     }
 
+  /*
+   * 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 thoses subnets are valid in the destination VRF.
+   */
+  /* *INDENT-OFF* */
+  foreach_ip_interface_address (&ip6_main.lookup_main,
+                                ia, sw_if_index,
+                                1 /* honor unnumbered */,
+  ({
+      ip4_address_t * a;
+
+      a = ip_interface_address_get_address (&ip6_main.lookup_main, ia);
+      error = clib_error_return (0, "interface %U has address %U",
+                                 format_vnet_sw_if_index_name, vnm,
+                                 sw_if_index,
+                                 format_ip6_address, a);
+      goto done;
+  }));
+  /* *INDENT-ON* */
+
   {
     u32 fib_index = fib_table_find_or_create_and_lock (FIB_PROTOCOL_IP6,
 						       table_id);
@@ -3020,13 +3040,12 @@ done:
  * an IP Address is assigned to an interface in the table (which adds a route
  * automatically).
  *
- * @note IP addresses added after setting the interface IP table end up in
- * the indicated FIB table. If the IP address is added prior to adding the
- * interface to the FIB table, it will NOT be part of the FIB table. Predictable
- * but potentially counter-intuitive results occur if you provision interface
- * addresses in multiple FIBs. Upon RX, packets will be processed in the last
- * IP table ID provisioned. It might be marginally useful to evade source RPF
- * drops to put an interface address into multiple FIBs.
+ * @note IP addresses added after setting the interface IP table are added to
+ * the indicated FIB table. If an IP address is added prior to changing the
+ * table then this is an error. The control plane must remove these addresses
+ * first and then change the table. VPP will not automatically move the
+ * addresses from the old to the new table as it does not know the validity
+ * of such a change.
  *
  * @cliexpar
  * Example of how to add an interface to an IPv6 FIB table (where 2 is the table-id):
diff --git a/src/vnet/ip/ip6_neighbor.c b/src/vnet/ip/ip6_neighbor.c
index 6b53137ffb2..91ff224c68e 100644
--- a/src/vnet/ip/ip6_neighbor.c
+++ b/src/vnet/ip/ip6_neighbor.c
@@ -24,6 +24,7 @@
 #include <vnet/adj/adj_mcast.h>
 #include <vnet/fib/fib_table.h>
 #include <vnet/fib/ip6_fib.h>
+#include <vnet/mfib/ip6_mfib.h>
 
 /**
  * @file
@@ -3283,6 +3284,7 @@ disable_ip6_interface (vlib_main_t * vm, u32 sw_if_index)
 
 	  ip6_neighbor_sw_interface_add_del (vnm, sw_if_index,
 					     0 /* is_add */ );
+	  ip6_mfib_interface_enable_disable (sw_if_index, 0);
 	}
     }
   return error;
@@ -3369,6 +3371,8 @@ enable_ip6_interface (vlib_main_t * vm, u32 sw_if_index)
 		      link_local_address.as_u8[8] &= 0xfd;
 		    }
 
+		  ip6_mfib_interface_enable_disable (sw_if_index, 1);
+
 		  /* essentially "enables" ipv6 on this interface */
 		  error = ip6_add_del_interface_address (vm, sw_if_index,
 							 &link_local_address,
diff --git a/src/vnet/ip/lookup.h b/src/vnet/ip/lookup.h
index 27c70943991..48360b5b41f 100644
--- a/src/vnet/ip/lookup.h
+++ b/src/vnet/ip/lookup.h
@@ -389,11 +389,6 @@ do {								\
   CLIB_PREFETCH (_adj, sizeof (_adj[0]), type);			\
 } while (0)
 
-/* Create new block of given number of contiguous adjacencies. */
-ip_adjacency_t *ip_add_adjacency (ip_lookup_main_t * lm,
-				  ip_adjacency_t * adj,
-				  u32 n_adj, u32 * adj_index_result);
-
 clib_error_t *ip_interface_address_add_del (ip_lookup_main_t * lm,
 					    u32 sw_if_index,
 					    void *address,
diff --git a/test/test_dhcp.py b/test/test_dhcp.py
index 6299975b869..a09c9bdb0b4 100644
--- a/test/test_dhcp.py
+++ b/test/test_dhcp.py
@@ -58,6 +58,13 @@ class TestDHCP(VppTestCase):
             i.set_table_ip6(table_id)
             table_id += 1
 
+    def tearDown(self):
+        super(TestDHCP, self).tearDown()
+        for i in self.pg_interfaces:
+            i.unconfig_ip4()
+            i.unconfig_ip6()
+            i.admin_down()
+
     def send_and_assert_no_replies(self, intf, pkts, remark):
         intf.add_stream(pkts)
         self.pg_enable_capture(self.pg_interfaces)
diff --git a/test/test_gre.py b/test/test_gre.py
index 89f39e4eb11..f2a5e0b0222 100644
--- a/test/test_gre.py
+++ b/test/test_gre.py
@@ -39,6 +39,10 @@ class TestGRE(VppTestCase):
 
     def tearDown(self):
         super(TestGRE, self).tearDown()
+        for i in self.pg_interfaces:
+            i.unconfig_ip4()
+            i.unconfig_ip6()
+            i.admin_down()
 
     def create_stream_ip4(self, src_if, src_ip, dst_ip):
         pkts = []
diff --git a/test/test_ip4_vrf_multi_instance.py b/test/test_ip4_vrf_multi_instance.py
index ddf8f5939d1..b73ac9483c3 100644
--- a/test/test_ip4_vrf_multi_instance.py
+++ b/test/test_ip4_vrf_multi_instance.py
@@ -208,6 +208,7 @@ class TestIp4VrfMultiInst(VppTestCase):
             self.vrf_deleted_list.append(vrf_id)
         for j in range(self.pg_ifs_per_vrf):
             pg_if = self.pg_if_by_vrf_id[vrf_id][j]
+            pg_if.unconfig_ip4()
             if pg_if in self.pg_in_vrf:
                 self.pg_in_vrf.remove(pg_if)
             if pg_if not in self.pg_not_in_vrf:
diff --git a/test/test_ip6_vrf_multi_instance.py b/test/test_ip6_vrf_multi_instance.py
index 7bd4d89c3e7..af80b5ba065 100644
--- a/test/test_ip6_vrf_multi_instance.py
+++ b/test/test_ip6_vrf_multi_instance.py
@@ -224,6 +224,7 @@ class TestIP6VrfMultiInst(VppTestCase):
             self.vrf_reset_list.append(vrf_id)
         for j in range(self.pg_ifs_per_vrf):
             pg_if = self.pg_if_by_vrf_id[vrf_id][j]
+            pg_if.unconfig_ip6()
             if pg_if in self.pg_in_vrf:
                 self.pg_in_vrf.remove(pg_if)
             if pg_if not in self.pg_not_in_vrf:
diff --git a/test/test_mpls.py b/test/test_mpls.py
index 41d9426b508..9082637b0db 100644
--- a/test/test_mpls.py
+++ b/test/test_mpls.py
@@ -17,10 +17,6 @@ from scapy.contrib.mpls import MPLS
 class TestMPLS(VppTestCase):
     """ MPLS Test Case """
 
-    @classmethod
-    def setUpClass(cls):
-        super(TestMPLS, cls).setUpClass()
-
     def setUp(self):
         super(TestMPLS, self).setUp()
 
@@ -44,6 +40,11 @@ class TestMPLS(VppTestCase):
 
     def tearDown(self):
         super(TestMPLS, self).tearDown()
+        for i in self.pg_interfaces:
+            i.unconfig_ip4()
+            i.unconfig_ip6()
+            i.ip6_disable()
+            i.admin_down()
 
     # the default of 64 matches the IP packet TTL default
     def create_stream_labelled_ip4(
diff --git a/test/test_neighbor.py b/test/test_neighbor.py
index 6a60809160b..885bf5a4f4b 100644
--- a/test/test_neighbor.py
+++ b/test/test_neighbor.py
@@ -40,6 +40,13 @@ class ARPTestCase(VppTestCase):
         self.pg3.set_table_ip4(1)
         self.pg3.config_ip4()
 
+    def tearDown(self):
+        super(ARPTestCase, self).tearDown()
+        for i in self.pg_interfaces:
+            i.unconfig_ip4()
+            i.unconfig_ip6()
+            i.admin_down()
+
     def verify_arp_req(self, rx, smac, sip, dip):
         ether = rx[Ether]
         self.assertEqual(ether.dst, "ff:ff:ff:ff:ff:ff")