From 99536f4b492478bab66ee8c6b0905a940b8db3c9 Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Thu, 25 Jul 2019 06:11:58 -0700 Subject: [PATCH] dhcp: send unicast and broadcast packets via the IP adjacency Type: feature this means DHCP packets are subject to the IP features configured on the interface - the unicast packets already were sent throught the adj - added UT for DHCP client sending a unicast renewal Change-Id: Id50db0b71822f44bf7cb639a524195cdc9873526 Signed-off-by: Neale Ranns --- MAINTAINERS | 6 ++++ src/vlib/trace_funcs.h | 2 +- src/vnet/dhcp/client.c | 82 +++++++++++++++++++++++++++--------------------- src/vnet/dhcp/client.h | 7 +++-- src/vnet/dhcp/dhcp_api.c | 3 +- test/test_dhcp.py | 78 +++++++++++++++++++++++++++++++++++---------- 6 files changed, 122 insertions(+), 56 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 40d1383b2cd..01882c5a3d7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -239,6 +239,12 @@ Y: src/vnet/ipip/FEATURE.yaml M: Ole Troan F: src/vnet/ipip/ +VNET DHCP +I: dhcp +M: Dave Barach +M: Neale Ranns +F: src/vnet/dhcp/ + VNET TLS and TLS engine plugins I: tls M: Florin Coras diff --git a/src/vlib/trace_funcs.h b/src/vlib/trace_funcs.h index 257c3a3a4ff..d0eeb29837b 100644 --- a/src/vlib/trace_funcs.h +++ b/src/vlib/trace_funcs.h @@ -145,7 +145,7 @@ vlib_trace_buffer (vlib_main_t * vm, (struct vlib_trace_main_t *) tm); } - vlib_trace_next_frame (vm, r, next_index); + //vlib_trace_next_frame (vm, r, next_index); pool_get (tm->trace_buffer_pool, h); diff --git a/src/vnet/dhcp/client.c b/src/vnet/dhcp/client.c index b5710684bc1..472de5253ef 100644 --- a/src/vnet/dhcp/client.c +++ b/src/vnet/dhcp/client.c @@ -13,6 +13,7 @@ * limitations under the License. */ #include +#include #include #include #include @@ -50,7 +51,6 @@ static char *dhcp_client_process_stat_strings[] = { "DHCP unknown packets sent", }; - static void dhcp_client_acquire_address (dhcp_client_main_t * dcm, dhcp_client_t * c) { @@ -75,18 +75,6 @@ dhcp_client_release_address (dhcp_client_main_t * dcm, dhcp_client_t * c) c->subnet_mask_width, 1 /*is_del */ ); } -static void -set_l2_rewrite (dhcp_client_main_t * dcm, dhcp_client_t * c) -{ - /* Acquire the L2 rewrite string for the indicated sw_if_index */ - c->l2_rewrite = vnet_build_rewrite_for_sw_interface (dcm->vnet_main, - c->sw_if_index, - VNET_LINK_IP4, - 0 /* broadcast */ ); -} - -void vl_api_rpc_call_main_thread (void *fp, u8 * data, u32 data_length); - static void dhcp_client_proc_callback (uword * client_index) { @@ -142,6 +130,14 @@ dhcp_client_addr_callback (dhcp_client_t * c) FIB_ROUTE_PATH_FLAG_NONE); /* *INDENT-ON* */ } + if (c->dhcp_server.as_u32) + { + ip46_address_t dst = { + .ip4 = c->dhcp_server, + }; + c->ai_ucast = adj_nbr_add_or_lock (FIB_PROTOCOL_IP4, + VNET_LINK_IP4, &dst, c->sw_if_index); + } /* * Call the user's event callback to report DHCP information @@ -378,7 +374,7 @@ send_dhcp_pkt (dhcp_client_main_t * dcm, dhcp_client_t * c, vlib_frame_t *f; dhcp_option_t *o; u16 udp_length, ip_length; - u32 counter_index; + u32 counter_index, node_index; /* Interface(s) down? */ if ((hw->flags & VNET_HW_INTERFACE_FLAG_LINK_UP) == 0) @@ -397,35 +393,37 @@ send_dhcp_pkt (dhcp_client_main_t * dcm, dhcp_client_t * c, /* Build a dhcpv4 pkt from whole cloth */ b = vlib_get_buffer (vm, bi); + VLIB_BUFFER_TRACE_TRAJECTORY_INIT (b); ASSERT (b->current_data == 0); vnet_buffer (b)->sw_if_index[VLIB_RX] = c->sw_if_index; + if (is_broadcast) { - f = vlib_get_frame_to_node (vm, hw->output_node_index); - vnet_buffer (b)->sw_if_index[VLIB_TX] = c->sw_if_index; - clib_memcpy (b->data, c->l2_rewrite, vec_len (c->l2_rewrite)); - ip = (void *) - (((u8 *) vlib_buffer_get_current (b)) + vec_len (c->l2_rewrite)); + node_index = ip4_rewrite_node.index; + vnet_buffer (b)->ip.adj_index[VLIB_TX] = c->ai_bcast; } else { - f = vlib_get_frame_to_node (vm, ip4_lookup_node.index); - vnet_buffer (b)->sw_if_index[VLIB_TX] = ~0; /* use interface VRF */ - ip = vlib_buffer_get_current (b); + ip_adjacency_t *adj = adj_get (c->ai_ucast); + + if (IP_LOOKUP_NEXT_ARP == adj->lookup_next_index) + node_index = ip4_arp_node.index; + else + node_index = ip4_rewrite_node.index; + vnet_buffer (b)->ip.adj_index[VLIB_TX] = c->ai_ucast; } /* Enqueue the packet right now */ + f = vlib_get_frame_to_node (vm, node_index); to_next = vlib_frame_vector_args (f); to_next[0] = bi; f->n_vectors = 1; + vlib_put_frame_to_node (vm, node_index, f); - if (is_broadcast) - vlib_put_frame_to_node (vm, hw->output_node_index, f); - else - vlib_put_frame_to_node (vm, ip4_lookup_node.index, f); - + /* build the headers */ + ip = vlib_buffer_get_current (b); udp = (udp_header_t *) (ip + 1); dhcp = (dhcp_header_t *) (udp + 1); @@ -452,7 +450,8 @@ send_dhcp_pkt (dhcp_client_main_t * dcm, dhcp_client_t * c, udp->dst_port = clib_host_to_net_u16 (UDP_DST_PORT_dhcp_to_server); /* Send the interface MAC address */ - clib_memcpy (dhcp->client_hardware_address, c->l2_rewrite + 6, 6); + clib_memcpy (dhcp->client_hardware_address, + vnet_sw_interface_get_hw_address (vnm, c->sw_if_index), 6); /* And remember it for rx-packet-for-us checking */ clib_memcpy (c->client_hardware_address, dhcp->client_hardware_address, @@ -553,8 +552,6 @@ send_dhcp_pkt (dhcp_client_main_t * dcm, dhcp_client_t * c, /* fix ip length, checksum and udp length */ ip_length = vlib_buffer_length_in_chain (vm, b); - if (is_broadcast) - ip_length -= vec_len (c->l2_rewrite); ip->length = clib_host_to_net_u16 (ip_length); ip->checksum = ip4_header_checksum (ip); @@ -837,7 +834,6 @@ format_dhcp_client (u8 * s, va_list * va) vec_foreach (addr, c->domain_server_address) s = format (s, " dns %U", format_ip4_address, addr); - vec_add1 (s, '\n'); } else { @@ -846,8 +842,17 @@ format_dhcp_client (u8 * s, va_list * va) if (verbose) { - s = format (s, "retry count %d, next xmt %.2f", - c->retry_count, c->next_transmit); + s = + format (s, + "\n lease: lifetime:%d renewal-interval:%d expires:%.2f (now:%.2f)", + c->lease_lifetime, c->lease_renewal_interval, + c->lease_expires, vlib_time_now (dcm->vlib_main)); + s = + format (s, "\n retry-count:%d, next-xmt:%.2f", c->retry_count, + c->next_transmit); + s = + format (s, "\n adjacencies:[unicast:%d broadcast:%d]", c->ai_ucast, + c->ai_bcast); } return s; } @@ -937,12 +942,17 @@ dhcp_client_add_del (dhcp_client_add_del_args_t * a) c->hostname = a->hostname; c->client_identifier = a->client_identifier; c->set_broadcast_flag = a->set_broadcast_flag; + c->ai_ucast = ADJ_INDEX_INVALID; + c->ai_bcast = adj_nbr_add_or_lock (FIB_PROTOCOL_IP4, + VNET_LINK_IP4, + &ADJ_BCAST_ADDR, c->sw_if_index); + do { c->transaction_id = random_u32 (&dcm->seed); } while (c->transaction_id == 0); - set_l2_rewrite (dcm, c); + hash_set (dcm->client_by_sw_if_index, a->sw_if_index, c - dcm->clients); /* @@ -980,11 +990,13 @@ dhcp_client_add_del (dhcp_client_add_del_args_t * a) } dhcp_client_release_address (dcm, c); + adj_unlock (c->ai_ucast); + adj_unlock (c->ai_bcast); + vec_free (c->domain_server_address); vec_free (c->option_55_data); vec_free (c->hostname); vec_free (c->client_identifier); - vec_free (c->l2_rewrite); hash_unset (dcm->client_by_sw_if_index, c->sw_if_index); pool_put (dcm->clients, c); } diff --git a/src/vnet/dhcp/client.h b/src/vnet/dhcp/client.h index 2b5341d76ad..a79d6e59715 100644 --- a/src/vnet/dhcp/client.h +++ b/src/vnet/dhcp/client.h @@ -71,8 +71,6 @@ typedef struct dhcp_client_t_ /* Requested data (option 55) */ u8 *option_55_data; - u8 *l2_rewrite; - /* hostname and software client identifiers */ u8 *hostname; u8 *client_identifier; /* software version, e.g. vpe 1.0 */ @@ -87,6 +85,11 @@ typedef struct dhcp_client_t_ u8 client_hardware_address[6]; u8 client_detect_feature_enabled; + /* the unicast adjacency for the DHCP server */ + adj_index_t ai_ucast; + /* the broadcast adjacency on the link */ + adj_index_t ai_bcast; + dhcp_event_cb_t event_callback; } dhcp_client_t; diff --git a/src/vnet/dhcp/dhcp_api.c b/src/vnet/dhcp/dhcp_api.c index d71ffe93c2d..7eb2bf46a06 100644 --- a/src/vnet/dhcp/dhcp_api.c +++ b/src/vnet/dhcp/dhcp_api.c @@ -239,8 +239,7 @@ dhcp_client_lease_encode (vl_api_dhcp_lease_t * lease, (u8 *) & client->domain_server_address[i], sizeof (ip4_address_t)); - if (NULL != client->l2_rewrite) - clib_memcpy (&lease->host_mac[0], client->l2_rewrite + 6, 6); + clib_memcpy (&lease->host_mac[0], client->client_hardware_address, 6); } static void diff --git a/test/test_dhcp.py b/test/test_dhcp.py index bdc5df7a267..fce41a9b423 100644 --- a/test/test_dhcp.py +++ b/test/test_dhcp.py @@ -210,21 +210,29 @@ class TestDHCP(VppTestCase): data = self.validate_relay_options(pkt, intf, intf.local_ip4, vpn_id, fib_id, oui) - def verify_orig_dhcp_pkt(self, pkt, intf): + def verify_orig_dhcp_pkt(self, pkt, intf, l2_bc=True): ether = pkt[Ether] - self.assertEqual(ether.dst, "ff:ff:ff:ff:ff:ff") + if l2_bc: + self.assertEqual(ether.dst, "ff:ff:ff:ff:ff:ff") + else: + self.assertEqual(ether.dst, intf.remote_mac) self.assertEqual(ether.src, intf.local_mac) ip = pkt[IP] - self.assertEqual(ip.dst, "255.255.255.255") - self.assertEqual(ip.src, "0.0.0.0") + + if (l2_bc): + self.assertEqual(ip.dst, "255.255.255.255") + self.assertEqual(ip.src, "0.0.0.0") + else: + self.assertEqual(ip.dst, intf.remote_ip4) + self.assertEqual(ip.src, intf.local_ip4) udp = pkt[UDP] self.assertEqual(udp.dport, DHCP4_SERVER_PORT) self.assertEqual(udp.sport, DHCP4_CLIENT_PORT) def verify_orig_dhcp_discover(self, pkt, intf, hostname, client_id=None, - broadcast=1): + broadcast=True): self.verify_orig_dhcp_pkt(pkt, intf) self.verify_dhcp_msg_type(pkt, "discover") @@ -240,15 +248,21 @@ class TestDHCP(VppTestCase): self.assertEqual(bootp.flags, 0x0000) def verify_orig_dhcp_request(self, pkt, intf, hostname, ip, - broadcast=1): - self.verify_orig_dhcp_pkt(pkt, intf) + broadcast=True, + l2_bc=True): + self.verify_orig_dhcp_pkt(pkt, intf, l2_bc=l2_bc) self.verify_dhcp_msg_type(pkt, "request") self.verify_dhcp_has_option(pkt, "hostname", hostname) self.verify_dhcp_has_option(pkt, "requested_addr", ip) bootp = pkt[BOOTP] - self.assertEqual(bootp.ciaddr, "0.0.0.0") + + if l2_bc: + self.assertEqual(bootp.ciaddr, "0.0.0.0") + else: + self.assertEqual(bootp.ciaddr, intf.local_ip4) self.assertEqual(bootp.giaddr, "0.0.0.0") + if broadcast: self.assertEqual(bootp.flags, 0x8000) else: @@ -1382,7 +1396,7 @@ class TestDHCP(VppTestCase): rx = self.pg3.get_capture(1) self.verify_orig_dhcp_discover(rx[0], self.pg3, hostname, - broadcast=0) + broadcast=False) # # Send back on offer, unicasted to the offered address. @@ -1404,10 +1418,11 @@ class TestDHCP(VppTestCase): rx = self.pg3.get_capture(1) self.verify_orig_dhcp_request(rx[0], self.pg3, hostname, self.pg3.local_ip4, - broadcast=0) + broadcast=False) # - # Send an acknowledgment + # Send an acknowledgment, the lease renewal time is 2 seconds + # so we should expect the renew straight after # p_ack = (Ether(dst=self.pg3.local_mac, src=self.pg3.remote_mac) / IP(src=self.pg3.remote_ip4, dst=self.pg3.local_ip4) / @@ -1419,6 +1434,7 @@ class TestDHCP(VppTestCase): ('router', self.pg3.remote_ip4), ('server_id', self.pg3.remote_ip4), ('lease_time', 43200), + ('renewal_time', 2), 'end'])) self.pg3.add_stream(p_ack) @@ -1440,11 +1456,33 @@ class TestDHCP(VppTestCase): self.assertTrue(find_route(self, self.pg3.local_ip4, 24)) self.assertTrue(find_route(self, self.pg3.local_ip4, 32)) - # remove the left over ARP entry - self.vapi.ip_neighbor_add_del(self.pg3.sw_if_index, - self.pg3.remote_mac, - self.pg3.remote_ip4, - is_add=0) + # + # wait for the unicasted renewal + # the first attempt will be an ARP packet, since we have not yet + # responded to VPP's request + # + self.logger.info(self.vapi.cli("sh dhcp client intfc pg3 verbose")) + rx = self.pg3.get_capture(1, timeout=10) + + self.assertEqual(rx[0][ARP].pdst, self.pg3.remote_ip4) + + # respond to the arp + p_arp = (Ether(dst=self.pg3.local_mac, src=self.pg3.remote_mac) / + ARP(op="is-at", + hwdst=self.pg3.local_mac, + hwsrc=self.pg3.remote_mac, + pdst=self.pg3.local_ip4, + psrc=self.pg3.remote_ip4)) + self.pg3.add_stream(p_arp) + self.pg_enable_capture(self.pg_interfaces) + self.pg_start() + + # the next packet is the unicasted renewal + rx = self.pg3.get_capture(1, timeout=10) + self.verify_orig_dhcp_request(rx[0], self.pg3, hostname, + self.pg3.local_ip4, + l2_bc=False, + broadcast=False) # # read the DHCP client details from a dump @@ -1468,6 +1506,12 @@ class TestDHCP(VppTestCase): self.assertEqual(clients[0].lease.host_address.rstrip('\0'), self.pg3.local_ip4n) + # remove the left over ARP entry + self.vapi.ip_neighbor_add_del(self.pg3.sw_if_index, + self.pg3.remote_mac, + self.pg3.remote_ip4, + is_add=0) + # # remove the DHCP config # @@ -1482,9 +1526,11 @@ class TestDHCP(VppTestCase): # # Start the procedure again. Use requested lease time option. # + hostname += "-2" self.pg3.admin_down() self.sleep(1) self.pg3.admin_up() + self.pg_enable_capture(self.pg_interfaces) self.vapi.dhcp_client_config(self.pg3.sw_if_index, hostname) rx = self.pg3.get_capture(1) -- 2.16.6