From: Eyal Bari Date: Wed, 13 Sep 2017 09:29:08 +0000 (+0300) Subject: L2BD,ARP-TERM:fix arp query report mechanism+test X-Git-Tag: v17.10-rc1~61 X-Git-Url: https://gerrit.fd.io/r/gitweb?p=vpp.git;a=commitdiff_plain;h=2019748a0ef815852281aae0a603f0e970fa9d91 L2BD,ARP-TERM:fix arp query report mechanism+test previous mechanism was emitting duplicates of last event, when handling multiple arp queries. tests: * arp events sent for graps * duplicate suppression * verify no events when disabled Change-Id: I84adc23980d43b819261eccf02ec056b5cec61df Signed-off-by: Eyal Bari --- diff --git a/src/vlibapi/api_helper_macros.h b/src/vlibapi/api_helper_macros.h index b8a9978a0a0..13734896298 100644 --- a/src/vlibapi/api_helper_macros.h +++ b/src/vlibapi/api_helper_macros.h @@ -215,7 +215,8 @@ _(from_netconf_server) \ _(to_netconf_client) \ _(from_netconf_client) \ _(oam_events) \ -_(bfd_events) +_(bfd_events) \ +_(wc_ip4_arp_events) typedef struct { diff --git a/src/vnet/ethernet/arp.c b/src/vnet/ethernet/arp.c index 957706c15e2..2f3aa6c73ac 100644 --- a/src/vnet/ethernet/arp.c +++ b/src/vnet/ethernet/arp.c @@ -92,6 +92,8 @@ typedef struct /* Proxy arp vector */ ethernet_proxy_arp_t *proxy_arps; + + uword wc_ip4_arp_publisher_node; } ethernet_arp_main_t; static ethernet_arp_main_t ethernet_arp_main; @@ -106,6 +108,7 @@ typedef struct #define ETHERNET_ARP_ARGS_REMOVE (1<<0) #define ETHERNET_ARP_ARGS_FLUSH (1<<1) #define ETHERNET_ARP_ARGS_POPULATE (1<<2) +#define ETHERNET_ARP_ARGS_WC_PUB (1<<3) } vnet_arp_set_ip4_over_ethernet_rpc_args_t; static const u8 vrrp_prefix[] = { 0x00, 0x00, 0x5E, 0x00, 0x01 }; @@ -540,7 +543,7 @@ arp_adj_fib_add (ethernet_arp_ip4_entry_t * e, u32 fib_index) fib_table_lock (fib_index, FIB_PROTOCOL_IP4, FIB_SOURCE_ADJ); } -int +static int vnet_arp_set_ip4_over_ethernet_internal (vnet_main_t * vnm, vnet_arp_set_ip4_over_ethernet_rpc_args_t * args) @@ -1507,6 +1510,49 @@ vnet_arp_populate_ip4_over_ethernet (vnet_main_t * vnm, return 0; } +/** + * @brief publish wildcard arp event + * @param sw_if_index The interface on which the ARP entires are acted + */ +static int +vnet_arp_wc_publish (u32 sw_if_index, void *a_arg) +{ + ethernet_arp_ip4_over_ethernet_address_t *a = a_arg; + vnet_arp_set_ip4_over_ethernet_rpc_args_t args = { + .flags = ETHERNET_ARP_ARGS_WC_PUB, + .sw_if_index = sw_if_index, + .a = *a + }; + + vl_api_rpc_call_main_thread (set_ip4_over_ethernet_rpc_callback, + (u8 *) & args, sizeof (args)); + return 0; +} + +static void +vnet_arp_wc_publish_internal (vnet_main_t * vnm, + vnet_arp_set_ip4_over_ethernet_rpc_args_t * + args) +{ + vlib_main_t *vm = vlib_get_main (); + ethernet_arp_main_t *am = ðernet_arp_main; + if (am->wc_ip4_arp_publisher_node == (uword) ~ 0) + return; + wc_arp_report_t *r = + vlib_process_signal_event_data (vm, am->wc_ip4_arp_publisher_node, 1, 1, + sizeof *r); + r->ip4 = args->a.ip4.as_u32; + r->sw_if_index = args->sw_if_index; + memcpy (r->mac, args->a.ethernet, sizeof r->mac); +} + +void +wc_arp_set_publisher_node (uword node_index) +{ + ethernet_arp_main_t *am = ðernet_arp_main; + am->wc_ip4_arp_publisher_node = node_index; +} + /* * arp_add_del_interface_address * @@ -1652,6 +1698,7 @@ ethernet_arp_init (vlib_main_t * vm) am->pending_resolutions_by_address = hash_create (0, sizeof (uword)); am->mac_changes_by_address = hash_create (0, sizeof (uword)); + am->wc_ip4_arp_publisher_node = (uword) ~ 0; /* don't trace ARP error packets */ { @@ -1795,6 +1842,8 @@ set_ip4_over_ethernet_rpc_callback (vnet_arp_set_ip4_over_ethernet_rpc_args_t vnet_arp_flush_ip4_over_ethernet_internal (vm, a); else if (a->flags & ETHERNET_ARP_ARGS_POPULATE) vnet_arp_populate_ip4_over_ethernet_internal (vm, a); + else if (a->flags & ETHERNET_ARP_ARGS_WC_PUB) + vnet_arp_wc_publish_internal (vm, a); else vnet_arp_set_ip4_over_ethernet_internal (vm, a); } @@ -2277,31 +2326,9 @@ arp_term_l2bd (vlib_main_t * vm, /* Check if anyone want ARP request events for L2 BDs */ { - pending_resolution_t *mc; ethernet_arp_main_t *am = ðernet_arp_main; - uword *p = hash_get (am->mac_changes_by_address, 0); - if (p) - { - u32 next_index = p[0]; - while (next_index != (u32) ~ 0) - { - int (*fp) (u32, u8 *, u32, u32); - int rv = 1; - mc = pool_elt_at_index (am->mac_changes, next_index); - fp = mc->data_callback; - /* Call the callback, return 1 to suppress dup events */ - if (fp) - rv = (*fp) (mc->data, - arp0->ip4_over_ethernet[0].ethernet, - sw_if_index0, - arp0->ip4_over_ethernet[0].ip4.as_u32); - /* Signal the resolver process */ - if (rv == 0) - vlib_process_signal_event (vm, mc->node_index, - mc->type_opaque, mc->data); - next_index = mc->next_index; - } - } + if (am->wc_ip4_arp_publisher_node != (uword) ~ 0) + vnet_arp_wc_publish (sw_if_index0, &arp0->ip4_over_ethernet[0]); } /* lookup BD mac_by_ip4 hash table for MAC entry */ diff --git a/src/vnet/ethernet/ethernet.h b/src/vnet/ethernet/ethernet.h index 9ca256c94ce..1a9e9790869 100644 --- a/src/vnet/ethernet/ethernet.h +++ b/src/vnet/ethernet/ethernet.h @@ -543,6 +543,8 @@ int vnet_add_del_ip4_arp_change_event (vnet_main_t * vnm, uword type_opaque, uword data, int is_add); +void wc_arp_set_publisher_node (uword inode_index); + void ethernet_arp_change_mac (u32 sw_if_index); void ethernet_ndp_change_mac (u32 sw_if_index); @@ -557,6 +559,13 @@ const u8 *ethernet_ip6_mcast_dst_addr (void); extern vlib_node_registration_t ethernet_input_node; +typedef struct +{ + u32 sw_if_index; + u32 ip4; + u8 mac[6]; +} wc_arp_report_t; + #endif /* included_ethernet_h */ /* diff --git a/src/vpp/api/api.c b/src/vpp/api/api.c index 044ddb5b0c8..e229a5e347f 100644 --- a/src/vpp/api/api.c +++ b/src/vpp/api/api.c @@ -299,17 +299,17 @@ static uword resolver_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f) { - uword event_type; - uword *event_data = 0; - f64 timeout = 100.0; - int i; + volatile f64 timeout = 100.0; + volatile uword *event_data = 0; while (1) { vlib_process_wait_for_event_or_clock (vm, timeout); - event_type = vlib_process_get_events (vm, &event_data); + uword event_type = + vlib_process_get_events (vm, (uword **) & event_data); + int i; switch (event_type) { case RESOLUTION_PENDING_EVENT: @@ -1352,26 +1352,15 @@ arp_change_data_callback (u32 pool_index, u8 * new_mac, return 1; event = pool_elt_at_index (am->arp_events, pool_index); - /* *INDENT-OFF* */ - if (memcmp (&event->new_mac, new_mac, sizeof (event->new_mac))) + if (eth_mac_equal (event->new_mac, new_mac) && + sw_if_index == ntohl (event->sw_if_index)) { - clib_memcpy (event->new_mac, new_mac, sizeof (event->new_mac)); + return 1; } - else - { /* same mac */ - if (sw_if_index == ntohl(event->sw_if_index) && - (!event->mac_ip || - /* for BD case, also check IP address with 10 sec timeout */ - (address == event->address && - (now - arp_event_last_time) < 10.0))) - return 1; - } - /* *INDENT-ON* */ - arp_event_last_time = now; + clib_memcpy (event->new_mac, new_mac, sizeof (event->new_mac)); event->sw_if_index = htonl (sw_if_index); - if (event->mac_ip) - event->address = address; + arp_event_last_time = now; return 0; } @@ -1391,7 +1380,7 @@ nd_change_data_callback (u32 pool_index, u8 * new_mac, event = pool_elt_at_index (am->nd_events, pool_index); /* *INDENT-OFF* */ - if (memcmp (&event->new_mac, new_mac, sizeof (event->new_mac))) + if (memcmp (event->new_mac, new_mac, sizeof (event->new_mac))) { clib_memcpy (event->new_mac, new_mac, sizeof (event->new_mac)); } @@ -1438,13 +1427,123 @@ nd_change_delete_callback (u32 pool_index, u8 * notused) return 0; } +static vlib_node_registration_t wc_ip4_arp_process_node; + +static uword +wc_ip4_arp_process (vlib_main_t * vm, + vlib_node_runtime_t * rt, vlib_frame_t * f) +{ + /* These cross the longjmp boundry (vlib_process_wait_for_event) + * and need to be volatile - to prevent them from being optimized into + * a register - which could change during suspension */ + + volatile wc_arp_report_t prev = { 0 }; + volatile f64 last = vlib_time_now (vm); + + while (1) + { + vlib_process_wait_for_event (vm); + uword event_type; + wc_arp_report_t *event_data = + vlib_process_get_event_data (vm, &event_type); + + f64 now = vlib_time_now (vm); + int i; + for (i = 0; i < vec_len (event_data); i++) + { + /* discard dup event */ + if (prev.ip4 == event_data[i].ip4 && + eth_mac_equal ((u8 *) prev.mac, event_data[i].mac) && + prev.sw_if_index == event_data[i].sw_if_index && + (now - last) < 10.0) + { + continue; + } + prev = event_data[i]; + last = now; + vpe_client_registration_t *reg; + /* *INDENT-OFF* */ + pool_foreach(reg, vpe_api_main.wc_ip4_arp_events_registrations, + ({ + unix_shared_memory_queue_t *q; + q = vl_api_client_index_to_input_queue (reg->client_index); + if (q && q->cursize < q->maxsize) + { + vl_api_ip4_arp_event_t * event = vl_msg_api_alloc (sizeof *event); + memset (event, 0, sizeof *event); + event->_vl_msg_id = htons (VL_API_IP4_ARP_EVENT); + event->client_index = reg->client_index; + event->pid = reg->client_pid; + event->mac_ip = 1; + event->address = event_data[i].ip4; + event->sw_if_index = htonl(event_data[i].sw_if_index); + memcpy(event->new_mac, event_data[i].mac, sizeof event->new_mac); + vl_msg_api_send_shmem (q, (u8 *) &event); + } + })); + /* *INDENT-ON* */ + } + vlib_process_put_event_data (vm, event_data); + } + + return 0; +} + +/* *INDENT-OFF* */ +VLIB_REGISTER_NODE (wc_ip4_arp_process_node,static) = { + .function = wc_ip4_arp_process, + .type = VLIB_NODE_TYPE_PROCESS, + .name = "wildcard-ip4-arp-publisher-process", +}; +/* *INDENT-ON* */ + static void vl_api_want_ip4_arp_events_t_handler (vl_api_want_ip4_arp_events_t * mp) { vpe_api_main_t *am = &vpe_api_main; vnet_main_t *vnm = vnet_get_main (); vl_api_want_ip4_arp_events_reply_t *rmp; - int rv; + int rv = 0; + + if (mp->address == 0) + { + uword *p = + hash_get (am->wc_ip4_arp_events_registration_hash, mp->client_index); + vpe_client_registration_t *rp; + if (p) + { + if (mp->enable_disable) + { + clib_warning ("pid %d: already enabled...", mp->pid); + rv = VNET_API_ERROR_INVALID_REGISTRATION; + goto reply; + } + else + { + rp = + pool_elt_at_index (am->wc_ip4_arp_events_registrations, p[0]); + pool_put (am->wc_ip4_arp_events_registrations, rp); + hash_unset (am->wc_ip4_arp_events_registration_hash, + mp->client_index); + if (pool_elts (am->wc_ip4_arp_events_registrations) == 0) + wc_arp_set_publisher_node (~0); + goto reply; + } + } + if (mp->enable_disable == 0) + { + clib_warning ("pid %d: already disabled...", mp->pid); + rv = VNET_API_ERROR_INVALID_REGISTRATION; + goto reply; + } + pool_get (am->wc_ip4_arp_events_registrations, rp); + rp->client_index = mp->client_index; + rp->client_pid = mp->pid; + hash_set (am->wc_ip4_arp_events_registration_hash, rp->client_index, + rp - am->wc_ip4_arp_events_registrations); + wc_arp_set_publisher_node (wc_ip4_arp_process_node.index); + goto reply; + } if (mp->enable_disable) { @@ -1459,12 +1558,12 @@ vl_api_want_ip4_arp_events_t_handler (vl_api_want_ip4_arp_events_t * mp) if (rv) { pool_put (am->arp_events, event); - goto out; + goto reply; } memset (event, 0, sizeof (*event)); /* Python API expects events to have no context */ - event->_vl_msg_id = ntohs (VL_API_IP4_ARP_EVENT); + event->_vl_msg_id = htons (VL_API_IP4_ARP_EVENT); event->client_index = mp->client_index; event->address = mp->address; event->pid = mp->pid; @@ -1479,7 +1578,7 @@ vl_api_want_ip4_arp_events_t_handler (vl_api_want_ip4_arp_events_t * mp) vpe_resolver_process_node.index, IP4_ARP_EVENT, ~0 /* pool index */ , 0 /* is_add */ ); } -out: +reply: REPLY_MACRO (VL_API_WANT_IP4_ARP_EVENTS_REPLY); } @@ -2072,13 +2171,10 @@ vpe_api_init (vlib_main_t * vm) am->vlib_main = vm; am->vnet_main = vnet_get_main (); - am->interface_events_registration_hash = hash_create (0, sizeof (uword)); - am->to_netconf_server_registration_hash = hash_create (0, sizeof (uword)); - am->from_netconf_server_registration_hash = hash_create (0, sizeof (uword)); - am->to_netconf_client_registration_hash = hash_create (0, sizeof (uword)); - am->from_netconf_client_registration_hash = hash_create (0, sizeof (uword)); - am->oam_events_registration_hash = hash_create (0, sizeof (uword)); - am->bfd_events_registration_hash = hash_create (0, sizeof (uword)); +#define _(a) \ + am->a##_registration_hash = hash_create (0, sizeof (uword)); + foreach_registration_hash; +#undef _ vl_set_memory_region_name ("/vpe-api"); vl_enable_disable_memory_api (vm, 1 /* enable it */ ); @@ -2219,10 +2315,7 @@ format_arp_event (u8 * s, va_list * args) vl_api_ip4_arp_event_t *event = va_arg (*args, vl_api_ip4_arp_event_t *); s = format (s, "pid %d: ", ntohl (event->pid)); - if (event->mac_ip) - s = format (s, "bd mac/ip4 binding events"); - else - s = format (s, "resolution for %U", format_ip4_address, &event->address); + s = format (s, "resolution for %U", format_ip4_address, &event->address); return s; } @@ -2259,6 +2352,13 @@ show_ip_arp_nd_events_fn (vlib_main_t * vm, vlib_cli_output (vm, "%U", format_arp_event, arp_event); })); + vpe_client_registration_t *reg; + pool_foreach(reg, am->wc_ip4_arp_events_registrations, + ({ + vlib_cli_output (vm, "pid %d: bd mac/ip4 binding events", + ntohl (reg->client_pid)); + })); + pool_foreach (nd_event, am->nd_events, ({ vlib_cli_output (vm, "%U", format_nd_event, nd_event); diff --git a/test/test_l2bd_arp_term.py b/test/test_l2bd_arp_term.py index 80a7ff84678..20885f88aed 100644 --- a/test/test_l2bd_arp_term.py +++ b/test/test_l2bd_arp_term.py @@ -5,7 +5,7 @@ import unittest import random import copy -from socket import AF_INET6 +from socket import AF_INET, AF_INET6 from scapy.packet import Raw from scapy.layers.l2 import Ether, ARP @@ -117,7 +117,7 @@ class TestL2bdArpTerm(VppTestCase): self.vapi.bridge_domain_add_del(bd_id=bd_id, is_add=is_add) @classmethod - def arp(cls, src_host, host): + def arp_req(cls, src_host, host): return (Ether(dst="ff:ff:ff:ff:ff:ff", src=src_host.mac) / ARP(op="who-has", hwsrc=src_host.bin_mac, @@ -126,7 +126,15 @@ class TestL2bdArpTerm(VppTestCase): @classmethod def arp_reqs(cls, src_host, entries): - return [cls.arp(src_host, e) for e in entries] + return [cls.arp_req(src_host, e) for e in entries] + + @classmethod + def garp_req(cls, host): + return cls.arp_req(host, host) + + @classmethod + def garp_reqs(cls, entries): + return [cls.garp_req(e) for e in entries] def arp_resp_host(self, src_host, arp_resp): ether = arp_resp[Ether] @@ -146,6 +154,20 @@ class TestL2bdArpTerm(VppTestCase): def arp_resp_hosts(self, src_host, pkts): return {self.arp_resp_host(src_host, p) for p in pkts} + def inttoip4(self, ip): + o1 = int(ip / 16777216) % 256 + o2 = int(ip / 65536) % 256 + o3 = int(ip / 256) % 256 + o4 = int(ip) % 256 + return '%(o1)s.%(o2)s.%(o3)s.%(o4)s' % locals() + + def arp_event_host(self, e): + return Host(mac=':'.join(['%02x' % ord(char) for char in e.new_mac]), + ip4=self.inttoip4(e.address)) + + def arp_event_hosts(self, evs): + return {self.arp_event_host(e) for e in evs} + @classmethod def ns_req(cls, src_host, host): nsma = in6_getnsma(inet_pton(AF_INET6, "fd10::ffff")) @@ -347,6 +369,60 @@ class TestL2bdArpTerm(VppTestCase): self.verify_nd(src_host, hosts, updated) self.bd_add_del(1, is_add=0) + def test_l2bd_arp_term_09(self): + """ L2BD arp term - send garps, verify arp event reports + """ + self.vapi.want_ip4_arp_events() + self.bd_add_del(1, is_add=1) + self.set_bd_flags(1, arp_term=True, flood=False, + uu_flood=False, learn=False) + macs = self.mac_list(range(90, 95)) + hosts = self.ip4_hosts(5, 1, macs) + + garps = self.garp_reqs(hosts) + self.bd_swifs(1)[0].add_stream(garps) + + self.pg_enable_capture(self.pg_interfaces) + self.pg_start() + evs = [self.vapi.wait_for_event(1, "ip4_arp_event") + for i in range(len(hosts))] + ev_hosts = self.arp_event_hosts(evs) + self.assertEqual(len(ev_hosts ^ hosts), 0) + + def test_l2bd_arp_term_10(self): + """ L2BD arp term - send duplicate garps, verify suppression + """ + macs = self.mac_list(range(70, 71)) + hosts = self.ip4_hosts(6, 1, macs) + + """ send the packet 5 times expect one event + """ + garps = self.garp_reqs(hosts) * 5 + self.bd_swifs(1)[0].add_stream(garps) + + self.pg_enable_capture(self.pg_interfaces) + self.pg_start() + evs = [self.vapi.wait_for_event(1, "ip4_arp_event") + for i in range(len(hosts))] + ev_hosts = self.arp_event_hosts(evs) + self.assertEqual(len(ev_hosts ^ hosts), 0) + + def test_l2bd_arp_term_11(self): + """ L2BD arp term - disable ip4 arp events,send garps, verify no events + """ + self.vapi.want_ip4_arp_events(enable_disable=0) + macs = self.mac_list(range(90, 95)) + hosts = self.ip4_hosts(5, 1, macs) + + garps = self.garp_reqs(hosts) + self.bd_swifs(1)[0].add_stream(garps) + + self.pg_enable_capture(self.pg_interfaces) + self.pg_start() + self.sleep(1) + self.assertEqual(len(self.vapi.collect_events()), 0) + self.bd_add_del(1, is_add=0) + if __name__ == '__main__': unittest.main(testRunner=VppTestRunner) diff --git a/test/vpp_papi_provider.py b/test/vpp_papi_provider.py index b63a26583da..fcc678490ba 100644 --- a/test/vpp_papi_provider.py +++ b/test/vpp_papi_provider.py @@ -101,11 +101,11 @@ class VppPapiProvider(object): def wait_for_event(self, timeout, name=None): """ Wait for and return next event. """ if name: - self.test_class.logger.debug("Expecting event within %ss", - timeout) - else: self.test_class.logger.debug("Expecting event '%s' within %ss", name, timeout) + else: + self.test_class.logger.debug("Expecting event within %ss", + timeout) if self._events: self.test_class.logger.debug("Not waiting, event already queued") limit = time.time() + timeout @@ -419,6 +419,12 @@ class VppPapiProvider(object): 'ip_address': ip, 'mac_address': mac}) + def want_ip4_arp_events(self, enable_disable=1, address=0): + return self.api(self.papi.want_ip4_arp_events, + {'enable_disable': enable_disable, + 'address': address, + 'pid': os.getpid(), }) + def l2fib_add_del(self, mac, bd_id, sw_if_index, is_add=1, static_mac=0, filter_mac=0, bvi_mac=0): """Create/delete L2 FIB entry.