From 4008ac998f43265451281cb6e759cd6184e50bed Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Mon, 13 Feb 2017 23:20:04 -0800 Subject: [PATCH] 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 --- src/vnet/adj/adj.c | 22 ------------ src/vnet/api_errno.h | 3 +- src/vnet/interface_api.c | 32 +++++++++++++++++ src/vnet/ip/ip4_forward.c | 71 ++++++++++++++++++++++--------------- src/vnet/ip/ip6_forward.c | 39 ++++++++++++++------ src/vnet/ip/ip6_neighbor.c | 4 +++ src/vnet/ip/lookup.h | 5 --- test/test_dhcp.py | 7 ++++ test/test_gre.py | 4 +++ test/test_ip4_vrf_multi_instance.py | 1 + test/test_ip6_vrf_multi_instance.py | 1 + test/test_mpls.py | 9 ++--- test/test_neighbor.py | 7 ++++ 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 [] [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 #include #include +#include #include #include #include @@ -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 #include #include +#include /** * @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") -- 2.16.6