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>
diff --git a/src/vnet/adj/adj.c b/src/vnet/adj/adj.c
index a99f173..11f1680 100644
--- a/src/vnet/adj/adj.c
+++ b/src/vnet/adj/adj.c
@@ -441,25 +441,3 @@
.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 a5bcb37..5e65ac7 100644
--- a/src/vnet/api_errno.h
+++ b/src/vnet/api_errno.h
@@ -102,7 +102,8 @@
_(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 60cd6d4..f94928b 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 @@
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 @@
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 66d91ab..fe4d676 100644
--- a/src/vnet/ip/ip4_forward.c
+++ b/src/vnet/ip/ip4_forward.c
@@ -2781,6 +2781,7 @@
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 @@
goto done;
}
- {
- ip4_main_t *im = &ip4_main;
- u32 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;
- fib_index = fib_table_find_or_create_and_lock (FIB_PROTOCOL_IP4,
- table_id);
+ 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* */
- //
- // 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;
+{
+ ip4_main_t *im = &ip4_main;
+ u32 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;
- }
+ 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 @@
* 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 5dd22b9..6f77c6d 100644
--- a/src/vnet/ip/ip6_forward.c
+++ b/src/vnet/ip/ip6_forward.c
@@ -415,9 +415,6 @@
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 @@
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 @@
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 @@
* 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 6b53137..91ff224 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 @@
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 @@
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 27c7094..48360b5 100644
--- a/src/vnet/ip/lookup.h
+++ b/src/vnet/ip/lookup.h
@@ -389,11 +389,6 @@
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 6299975..a09c9bd 100644
--- a/test/test_dhcp.py
+++ b/test/test_dhcp.py
@@ -58,6 +58,13 @@
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 89f39e4..f2a5e0b 100644
--- a/test/test_gre.py
+++ b/test/test_gre.py
@@ -39,6 +39,10 @@
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 ddf8f59..b73ac94 100644
--- a/test/test_ip4_vrf_multi_instance.py
+++ b/test/test_ip4_vrf_multi_instance.py
@@ -208,6 +208,7 @@
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 7bd4d89..af80b5b 100644
--- a/test/test_ip6_vrf_multi_instance.py
+++ b/test/test_ip6_vrf_multi_instance.py
@@ -224,6 +224,7 @@
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 41d9426..9082637 100644
--- a/test/test_mpls.py
+++ b/test/test_mpls.py
@@ -17,10 +17,6 @@
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 @@
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 6a60809..885bf5a 100644
--- a/test/test_neighbor.py
+++ b/test/test_neighbor.py
@@ -40,6 +40,13 @@
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")