From 1985c93bd728db22c946e05691f21a2f9774e458 Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Tue, 16 May 2017 08:46:45 -0700 Subject: ARP learning fixes (VPP-843) learn ARP peers if, 1) it's a reply to a local address, 2) we are sending a response to a request. send proxy ARP responses only in the interface the request was sent. Change-Id: I22b949c65122824233076492b7dd537daca07bc2 Signed-off-by: Neale Ranns (cherry picked from commit d5b6aa139856a1447f7bc5377058202110eaa4cf) --- src/vnet/ethernet/arp.c | 119 ++++++++++++++++++--------------------------- src/vnet/ip/ip6_neighbor.c | 8 ++- src/vnet/ip/lookup.h | 23 ++++++--- test/test_neighbor.py | 38 ++++++++++++--- 4 files changed, 98 insertions(+), 90 deletions(-) diff --git a/src/vnet/ethernet/arp.c b/src/vnet/ethernet/arp.c index 2367014e144..5ad0c3e3c2d 100644 --- a/src/vnet/ethernet/arp.c +++ b/src/vnet/ethernet/arp.c @@ -562,6 +562,7 @@ vnet_arp_set_ip4_over_ethernet_internal (vnet_main_t * vnm, e->sw_if_index = sw_if_index; e->ip4_address = a->ip4; + e->fib_entry_index = FIB_NODE_INDEX_INVALID; clib_memcpy (e->ethernet_address, a->ethernet, sizeof (e->ethernet_address)); @@ -839,66 +840,39 @@ unset_random_arp_entry (void) static int arp_unnumbered (vlib_buffer_t * p0, - u32 pi0, ethernet_header_t * eth0, u32 sw_if_index) + u32 pi0, + ethernet_header_t * eth0, + u32 input_sw_if_index, u32 conn_sw_if_index) { vlib_main_t *vm = vlib_get_main (); vnet_main_t *vnm = vnet_get_main (); vnet_interface_main_t *vim = &vnm->interface_main; vnet_sw_interface_t *si; vnet_hw_interface_t *hi; - u32 unnum_src_sw_if_index; - u32 *broadcast_swifs = 0; u32 *buffers = 0; - u32 n_alloc = 0; vlib_buffer_t *b0; int i; u8 dst_mac_address[6]; i16 header_size; ethernet_arp_header_t *arp0; - /* Save the dst mac address */ - clib_memcpy (dst_mac_address, eth0->dst_address, sizeof (dst_mac_address)); + /* verify that the input interface is unnumbered to the connected. + * the connected interface is the interface on which the subnet is + * configured */ + si = &vim->sw_interfaces[input_sw_if_index]; - /* Figure out which sw_if_index supplied the address */ - unnum_src_sw_if_index = sw_if_index; - - /* Track down all users of the unnumbered source */ - /* *INDENT-OFF* */ - pool_foreach (si, vim->sw_interfaces, - ({ - if (si->flags & VNET_SW_INTERFACE_FLAG_UNNUMBERED && - (si->unnumbered_sw_if_index == unnum_src_sw_if_index)) - { - vec_add1 (broadcast_swifs, si->sw_if_index); - } - })); - /* *INDENT-ON* */ - - /* If there are no interfaces un-unmbered to this interface, - we are done here. */ - if (0 == vec_len (broadcast_swifs)) - return 0; - - /* Allocate buffering if we need it */ - if (vec_len (broadcast_swifs) > 1) + if (!(si->flags & VNET_SW_INTERFACE_FLAG_UNNUMBERED && + (si->unnumbered_sw_if_index == conn_sw_if_index))) { - vec_validate (buffers, vec_len (broadcast_swifs) - 2); - n_alloc = vlib_buffer_alloc (vm, buffers, vec_len (buffers)); - _vec_len (buffers) = n_alloc; - for (i = 0; i < n_alloc; i++) - { - b0 = vlib_get_buffer (vm, buffers[i]); - - /* xerox (partially built) ARP pkt */ - clib_memcpy (b0->data, p0->data, - p0->current_length + p0->current_data); - b0->current_data = p0->current_data; - b0->current_length = p0->current_length; - vnet_buffer (b0)->sw_if_index[VLIB_RX] = - vnet_buffer (p0)->sw_if_index[VLIB_RX]; - } + /* the input interface is not unnumbered to the interface on which + * the sub-net is configured that covers the ARP request. + * So this is not the case for unnumbered.. */ + return 0; } + /* Save the dst mac address */ + clib_memcpy (dst_mac_address, eth0->dst_address, sizeof (dst_mac_address)); + vec_insert (buffers, 1, 0); buffers[0] = pi0; @@ -907,8 +881,8 @@ arp_unnumbered (vlib_buffer_t * p0, b0 = vlib_get_buffer (vm, buffers[i]); arp0 = vlib_buffer_get_current (b0); - hi = vnet_get_sup_hw_interface (vnm, broadcast_swifs[i]); - si = vnet_get_sw_interface (vnm, broadcast_swifs[i]); + hi = vnet_get_sup_hw_interface (vnm, input_sw_if_index); + si = vnet_get_sw_interface (vnm, input_sw_if_index); /* For decoration, most likely */ vnet_buffer (b0)->sw_if_index[VLIB_TX] = hi->sw_if_index; @@ -978,14 +952,25 @@ arp_unnumbered (vlib_buffer_t * p0, } /* The regular path outputs the original pkt.. */ - vnet_buffer (p0)->sw_if_index[VLIB_TX] = broadcast_swifs[0]; + vnet_buffer (p0)->sw_if_index[VLIB_TX] = input_sw_if_index; - vec_free (broadcast_swifs); vec_free (buffers); return !0; } +static u32 +arp_learn (vnet_main_t * vnm, + ethernet_arp_main_t * am, u32 sw_if_index, void *addr) +{ + if (am->limit_arp_cache_size && + pool_elts (am->ip4_entry_pool) >= am->limit_arp_cache_size) + unset_random_arp_entry (); + + vnet_arp_set_ip4_over_ethernet (vnm, sw_if_index, addr, 0, 0); + return (ETHERNET_ARP_ERROR_l3_src_address_learned); +} + static uword arp_input (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_frame_t * frame) { @@ -1133,28 +1118,14 @@ arp_input (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_frame_t * frame) goto drop2; } - /* Learn or update sender's mapping only for requests or unicasts - that don't match local interface address. */ - if (ethernet_address_cast (eth0->dst_address) == - ETHERNET_ADDRESS_UNICAST || is_request0) - { - if (am->limit_arp_cache_size && - pool_elts (am->ip4_entry_pool) >= am->limit_arp_cache_size) - unset_random_arp_entry (); - - vnet_arp_set_ip4_over_ethernet (vnm, sw_if_index0, - &arp0->ip4_over_ethernet[0], - 0 /* is_static */ , 0); - error0 = ETHERNET_ARP_ERROR_l3_src_address_learned; - } - - /* Only send a reply for requests sent which match a local interface. */ - if (!(is_request0 && dst_is_local0)) + /* Learn or update sender's mapping only for replies to addresses + * that are local to the subnet */ + if (arp0->opcode == + clib_host_to_net_u16 (ETHERNET_ARP_OPCODE_reply) && + dst_is_local0) { - error0 = - (arp0->opcode == - clib_host_to_net_u16 (ETHERNET_ARP_OPCODE_reply) ? - ETHERNET_ARP_ERROR_replies_received : error0); + error0 = arp_learn (vnm, am, sw_if_index0, + &arp0->ip4_over_ethernet[0]); goto drop1; } @@ -1188,7 +1159,8 @@ arp_input (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_frame_t * frame) { if (is_unnum0) { - if (!arp_unnumbered (p0, pi0, eth0, conn_sw_if_index0)) + if (!arp_unnumbered (p0, pi0, eth0, + sw_if_index0, conn_sw_if_index0)) goto drop2; } else @@ -1242,6 +1214,10 @@ arp_input (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_frame_t * frame) } } + /* We are going to reply to this request, so learn the sender */ + error0 = arp_learn (vnm, am, sw_if_index0, + &arp0->ip4_over_ethernet[1]); + vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next, n_left_to_next, pi0, next0); @@ -1697,9 +1673,8 @@ arp_entry_free (ethernet_arp_interface_t * eai, ethernet_arp_ip4_entry_t * e) { ethernet_arp_main_t *am = ðernet_arp_main; - /* it's safe to delete the ADJ source on the FIB entry, even if it - * was added */ - fib_table_entry_delete_index (e->fib_entry_index, FIB_SOURCE_ADJ); + if (FIB_NODE_INDEX_INVALID != e->fib_entry_index) + fib_table_entry_delete_index (e->fib_entry_index, FIB_SOURCE_ADJ); hash_unset (eai->arp_entries, e->ip4_address.as_u32); pool_put (am->ip4_entry_pool, e); } diff --git a/src/vnet/ip/ip6_neighbor.c b/src/vnet/ip/ip6_neighbor.c index 92708f14d5b..edc670af3b0 100644 --- a/src/vnet/ip/ip6_neighbor.c +++ b/src/vnet/ip/ip6_neighbor.c @@ -273,7 +273,8 @@ ip6_neighbor_sw_interface_up_down (vnet_main_t * vnm, { n = pool_elt_at_index (nm->neighbor_pool, to_delete[i]); mhash_unset (&nm->neighbor_index_by_key, &n->key, 0); - fib_table_entry_delete_index (n->fib_entry_index, FIB_SOURCE_ADJ); + if (FIB_NODE_INDEX_INVALID != n->fib_entry_index) + fib_table_entry_delete_index (n->fib_entry_index, FIB_SOURCE_ADJ); pool_put (nm->neighbor_pool, n); } @@ -609,6 +610,7 @@ vnet_set_ip6_ethernet_neighbor (vlib_main_t * vm, mhash_set (&nm->neighbor_index_by_key, &k, n - nm->neighbor_pool, /* old value */ 0); n->key = k; + n->fib_entry_index = FIB_NODE_INDEX_INVALID; clib_memcpy (n->link_layer_address, link_layer_address, n_bytes_link_layer_address); @@ -746,7 +748,9 @@ vnet_unset_ip6_ethernet_neighbor (vlib_main_t * vm, adj_nbr_walk_nh6 (sw_if_index, &n->key.ip6_address, ip6_nd_mk_incomplete_walk, NULL); - fib_table_entry_delete_index (n->fib_entry_index, FIB_SOURCE_ADJ); + + if (FIB_NODE_INDEX_INVALID != n->fib_entry_index) + fib_table_entry_delete_index (n->fib_entry_index, FIB_SOURCE_ADJ); pool_put (nm->neighbor_pool, n); out: diff --git a/src/vnet/ip/lookup.h b/src/vnet/ip/lookup.h index 48360b5b41f..0fc2a992a0f 100644 --- a/src/vnet/ip/lookup.h +++ b/src/vnet/ip/lookup.h @@ -416,7 +416,7 @@ ip_interface_address_get_address (ip_lookup_main_t * lm, /* *INDENT-OFF* */ #define foreach_ip_interface_address(lm,a,sw_if_index,loop,body) \ do { \ - vnet_main_t *_vnm = vnet_get_main(); \ + vnet_main_t *_vnm = vnet_get_main(); \ u32 _sw_if_index = sw_if_index; \ vnet_sw_interface_t *_swif; \ _swif = vnet_get_sw_interface (_vnm, _sw_if_index); \ @@ -424,13 +424,20 @@ do { \ /* \ * Loop => honor unnumbered interface addressing. \ */ \ - if (loop && _swif->flags & VNET_SW_INTERFACE_FLAG_UNNUMBERED) \ - _sw_if_index = _swif->unnumbered_sw_if_index; \ - u32 _ia = \ - (vec_len((lm)->if_address_pool_index_by_sw_if_index) \ - > (_sw_if_index)) \ - ? vec_elt ((lm)->if_address_pool_index_by_sw_if_index, \ - (_sw_if_index)) : (u32)~0; \ + if (_swif->flags & VNET_SW_INTERFACE_FLAG_UNNUMBERED) \ + { \ + if (loop) \ + _sw_if_index = _swif->unnumbered_sw_if_index; \ + else \ + /* the interface is unnumbered, by the caller does not want \ + * unnumbered interfaces considered/honoured */ \ + break; \ + } \ + u32 _ia = ((vec_len((lm)->if_address_pool_index_by_sw_if_index) \ + > (_sw_if_index)) ? \ + vec_elt ((lm)->if_address_pool_index_by_sw_if_index, \ + (_sw_if_index)) : \ + (u32)~0); \ ip_interface_address_t * _a; \ while (_ia != ~0) \ { \ diff --git a/test/test_neighbor.py b/test/test_neighbor.py index f2b1cfa47ee..48c6a291248 100644 --- a/test/test_neighbor.py +++ b/test/test_neighbor.py @@ -112,8 +112,11 @@ class ARPTestCase(VppTestCase): intf.add_stream(pkts) self.pg_enable_capture(self.pg_interfaces) self.pg_start() + timeout = 1 for i in self.pg_interfaces: + i.get_capture(0, timeout=timeout) i.assert_nothing_captured(remark=remark) + timeout = 0.1 def test_arp(self): """ ARP """ @@ -438,7 +441,9 @@ class ARPTestCase(VppTestCase): # ERROR Cases # 1 - don't respond to ARP request for address not within the # interface's sub-net - # 1a - nor within the unnumbered subnet + # 1b - nor within the unnumbered subnet + # 1c - nor within the subnet of a different interface + # p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg0.remote_mac) / ARP(op="who-has", hwsrc=self.pg0.remote_mac, @@ -446,6 +451,10 @@ class ARPTestCase(VppTestCase): psrc=self.pg0.remote_ip4)) self.send_and_assert_no_replies(self.pg0, p, "ARP req for non-local destination") + self.assertFalse(find_nbr(self, + self.pg0.sw_if_index, + "10.10.10.3")) + p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg2.remote_mac) / ARP(op="who-has", hwsrc=self.pg2.remote_mac, @@ -455,6 +464,17 @@ class ARPTestCase(VppTestCase): self.pg0, p, "ARP req for non-local destination - unnum") + p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg0.remote_mac) / + ARP(op="who-has", + hwsrc=self.pg0.remote_mac, + pdst=self.pg1.local_ip4, + psrc=self.pg1.remote_ip4)) + self.send_and_assert_no_replies(self.pg0, p, + "ARP req diff sub-net") + self.assertFalse(find_nbr(self, + self.pg0.sw_if_index, + self.pg1.remote_ip4)) + # # 2 - don't respond to ARP request from an address not within the # interface's sub-net @@ -514,15 +534,11 @@ class ARPTestCase(VppTestCase): def test_proxy_arp(self): """ Proxy ARP """ + self.pg1.generate_remote_hosts(2) + # # Proxy ARP rewquest packets for each interface # - arp_req_pg2 = (Ether(src=self.pg2.remote_mac, - dst="ff:ff:ff:ff:ff:ff") / - ARP(op="who-has", - hwsrc=self.pg2.remote_mac, - pdst="10.10.10.3", - psrc=self.pg1.remote_ip4)) arp_req_pg0 = (Ether(src=self.pg0.remote_mac, dst="ff:ff:ff:ff:ff:ff") / ARP(op="who-has", @@ -535,6 +551,12 @@ class ARPTestCase(VppTestCase): hwsrc=self.pg1.remote_mac, pdst="10.10.10.3", psrc=self.pg1.remote_ip4)) + arp_req_pg2 = (Ether(src=self.pg2.remote_mac, + dst="ff:ff:ff:ff:ff:ff") / + ARP(op="who-has", + hwsrc=self.pg2.remote_mac, + pdst="10.10.10.3", + psrc=self.pg1.remote_hosts[1].ip4)) arp_req_pg3 = (Ether(src=self.pg3.remote_mac, dst="ff:ff:ff:ff:ff:ff") / ARP(op="who-has", @@ -607,7 +629,7 @@ class ARPTestCase(VppTestCase): self.pg2.local_mac, self.pg2.remote_mac, "10.10.10.3", - self.pg1.remote_ip4) + self.pg1.remote_hosts[1].ip4) # # A request for an address out of the configured range -- cgit 1.2.3-korg