From: Neale Ranns Date: Wed, 2 Aug 2017 12:15:07 +0000 (-0700) Subject: DHCP Client: receive unicast ACKs X-Git-Tag: v17.10-rc1~287 X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=808c5b21c2759564689933d004223052b7895a42;p=vpp.git DHCP Client: receive unicast ACKs despite VPP DHCP client setting neither ciaddr nor giaddr and setting the broadcast bit (see RFC 2131 section 4.1) some DHCP servers will still send a unicast DCHPACK. So as not to drop this VPP must have both 1) a receive FIB entry for the OFFERED IP adress and 2) a 'don't drop me because of uRPF' FIB entry for the DHCP server's address. Change-Id: I167d858deb45629318cbdccf5bf67d971730a42f Signed-off-by: Neale Ranns --- diff --git a/src/vnet/dhcp/client.c b/src/vnet/dhcp/client.c index 25ab3176ae5..cfe62a6f45b 100644 --- a/src/vnet/dhcp/client.c +++ b/src/vnet/dhcp/client.c @@ -22,6 +22,70 @@ static u8 * format_dhcp_client_state (u8 * s, va_list * va); static vlib_node_registration_t dhcp_client_process_node; static void +dhcp_client_add_rx_address (dhcp_client_main_t * dcm, dhcp_client_t * c) +{ + /* Install a local entry for the offered address */ + fib_prefix_t rx = + { + .fp_len = 32, + .fp_addr.ip4 = c->leased_address, + .fp_proto = FIB_PROTOCOL_IP4, + }; + + fib_table_entry_special_add(fib_table_get_index_for_sw_if_index( + FIB_PROTOCOL_IP4, + c->sw_if_index), + &rx, + FIB_SOURCE_DHCP, + (FIB_ENTRY_FLAG_LOCAL)); + + /* And add the server's address as uRPF exempt so we can accept + * local packets from it */ + fib_prefix_t server = + { + .fp_len = 32, + .fp_addr.ip4 = c->dhcp_server, + .fp_proto = FIB_PROTOCOL_IP4, + }; + + fib_table_entry_special_add(fib_table_get_index_for_sw_if_index( + FIB_PROTOCOL_IP4, + c->sw_if_index), + &server, + FIB_SOURCE_URPF_EXEMPT, + (FIB_ENTRY_FLAG_DROP)); +} + +static void +dhcp_client_remove_rx_address (dhcp_client_main_t * dcm, dhcp_client_t * c) +{ + fib_prefix_t rx = + { + .fp_len = 32, + .fp_addr.ip4 = c->leased_address, + .fp_proto = FIB_PROTOCOL_IP4, + }; + + fib_table_entry_special_remove(fib_table_get_index_for_sw_if_index( + FIB_PROTOCOL_IP4, + c->sw_if_index), + &rx, + FIB_SOURCE_DHCP); + fib_prefix_t server = + { + .fp_len = 32, + .fp_addr.ip4 = c->dhcp_server, + .fp_proto = FIB_PROTOCOL_IP4, + }; + + fib_table_entry_special_remove(fib_table_get_index_for_sw_if_index( + FIB_PROTOCOL_IP4, + c->sw_if_index), + &server, + FIB_SOURCE_URPF_EXEMPT); +} + +static void dhcp_client_acquire_address (dhcp_client_main_t * dcm, dhcp_client_t * c) { /* @@ -95,7 +159,9 @@ int dhcp_client_for_us (u32 bi, vlib_buffer_t * b, /* parse through the packet, learn what we can */ if (dhcp->your_ip_address.as_u32) c->leased_address.as_u32 = dhcp->your_ip_address.as_u32; - + + c->dhcp_server.as_u32 = dhcp->server_ip_address.as_u32; + o = (dhcp_option_t *) dhcp->options; while (o->option != 0xFF /* end of options */ && @@ -172,6 +238,14 @@ int dhcp_client_for_us (u32 bi, vlib_buffer_t * b, c->next_transmit = now + 5.0; break; } + /* + * in order to accept unicasted ACKs we need to configure the offered + * address on the interface. However, at this point we may not know the + * subnet-mask (an OFFER may not contain it). So add a temporary receice + * and uRPF excempt entry + */ + dhcp_client_add_rx_address (dcm, c); + /* Received an offer, go send a request */ c->state = DHCP_REQUEST; c->retry_count = 0; @@ -196,6 +270,8 @@ int dhcp_client_for_us (u32 bi, vlib_buffer_t * b, { void (*fp)(u32, u32, u8 *, u8, u8, u8 *, u8 *, u8 *) = c->event_callback; + /* replace the temporary RX address with the correct subnet */ + dhcp_client_remove_rx_address (dcm, c); dhcp_client_acquire_address (dcm, c); /* @@ -831,6 +907,7 @@ int dhcp_client_add_del (dhcp_client_add_del_args_t * a) 1, FIB_ROUTE_PATH_FLAG_NONE); } + dhcp_client_remove_rx_address (dcm, c); dhcp_client_release_address (dcm, c); ip4_sw_interface_enable_disable (c->sw_if_index, 0); diff --git a/src/vnet/fib/fib_entry.h b/src/vnet/fib/fib_entry.h index 5f6ff31297e..93b8016d44f 100644 --- a/src/vnet/fib/fib_entry.h +++ b/src/vnet/fib/fib_entry.h @@ -205,14 +205,9 @@ typedef enum fib_entry_attribute_t_ { /** * Marker. add new entries before this one. */ - FIB_ENTRY_ATTRIBUTE_LAST = FIB_ENTRY_ATTRIBUTE_MULTICAST, + FIB_ENTRY_ATTRIBUTE_LAST = FIB_ENTRY_ATTRIBUTE_URPF_EXEMPT, } fib_entry_attribute_t; -/** - * The maximum number of sources - */ -#define FIB_ENTRY_ATTRIBUTE_MAX (FIB_ENTRY_ATTRIBUTE_LAST+1) - #define FIB_ENTRY_ATTRIBUTES { \ [FIB_ENTRY_ATTRIBUTE_CONNECTED] = "connected", \ [FIB_ENTRY_ATTRIBUTE_ATTACHED] = "attached", \ @@ -226,7 +221,7 @@ typedef enum fib_entry_attribute_t_ { #define FOR_EACH_FIB_ATTRIBUTE(_item) \ for (_item = FIB_ENTRY_ATTRIBUTE_FIRST; \ - _item < FIB_ENTRY_ATTRIBUTE_MAX; \ + _item <= FIB_ENTRY_ATTRIBUTE_LAST; \ _item++) typedef enum fib_entry_flag_t_ { diff --git a/test/test_dhcp.py b/test/test_dhcp.py index 4e8ed4cedb7..6fc291824ec 100644 --- a/test/test_dhcp.py +++ b/test/test_dhcp.py @@ -196,6 +196,10 @@ class TestDHCP(VppTestCase): self.verify_dhcp_has_option(pkt, "hostname", hostname) if client_id: self.verify_dhcp_has_option(pkt, "client_id", client_id) + bootp = pkt[BOOTP] + self.assertEqual(bootp.ciaddr, "0.0.0.0") + self.assertEqual(bootp.giaddr, "0.0.0.0") + self.assertEqual(bootp.flags, 0x8000) def verify_orig_dhcp_request(self, pkt, intf, hostname, ip): self.verify_orig_dhcp_pkt(pkt, intf) @@ -203,6 +207,10 @@ class TestDHCP(VppTestCase): 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") + self.assertEqual(bootp.giaddr, "0.0.0.0") + self.assertEqual(bootp.flags, 0x8000) def verify_relayed_dhcp_discover(self, pkt, intf, src_intf=None, fib_id=0, oui=0, @@ -1057,14 +1065,15 @@ class TestDHCP(VppTestCase): # # Sned back on offer, expect the request # - p = (Ether(dst=self.pg2.local_mac, src=self.pg2.remote_mac) / - IP(src=self.pg2.remote_ip4, dst="255.255.255.255") / - UDP(sport=DHCP4_SERVER_PORT, dport=DHCP4_CLIENT_PORT) / - BOOTP(op=1, - yiaddr=self.pg2.local_ip4) / - DHCP(options=[('message-type', 'offer'), ('end')])) + p_offer = (Ether(dst=self.pg2.local_mac, src=self.pg2.remote_mac) / + IP(src=self.pg2.remote_ip4, dst="255.255.255.255") / + UDP(sport=DHCP4_SERVER_PORT, dport=DHCP4_CLIENT_PORT) / + BOOTP(op=1, yiaddr=self.pg2.local_ip4) / + DHCP(options=[('message-type', 'offer'), + ('server_id', self.pg2.remote_ip4), + ('end')])) - self.pg2.add_stream(p) + self.pg2.add_stream(p_offer) self.pg_enable_capture(self.pg_interfaces) self.pg_start() @@ -1075,19 +1084,18 @@ class TestDHCP(VppTestCase): # # Send an acknowloedgement # - p = (Ether(dst=self.pg2.local_mac, src=self.pg2.remote_mac) / - IP(src=self.pg2.remote_ip4, dst="255.255.255.255") / - UDP(sport=DHCP4_SERVER_PORT, dport=DHCP4_CLIENT_PORT) / - BOOTP(op=1, - yiaddr=self.pg2.local_ip4) / - DHCP(options=[('message-type', 'ack'), - ('subnet_mask', "255.255.255.0"), - ('router', self.pg2.remote_ip4), - ('server_id', self.pg2.remote_ip4), - ('lease_time', 43200), - ('end')])) - - self.pg2.add_stream(p) + p_ack = (Ether(dst=self.pg2.local_mac, src=self.pg2.remote_mac) / + IP(src=self.pg2.remote_ip4, dst="255.255.255.255") / + UDP(sport=DHCP4_SERVER_PORT, dport=DHCP4_CLIENT_PORT) / + BOOTP(op=1, yiaddr=self.pg2.local_ip4) / + DHCP(options=[('message-type', 'ack'), + ('subnet_mask', "255.255.255.0"), + ('router', self.pg2.remote_ip4), + ('server_id', self.pg2.remote_ip4), + ('lease_time', 43200), + ('end')])) + + self.pg2.add_stream(p_ack) self.pg_enable_capture(self.pg_interfaces) self.pg_start() @@ -1103,6 +1111,7 @@ class TestDHCP(VppTestCase): # At the end of this procedure there should be a connected route # in the FIB # + self.assertTrue(find_route(self, self.pg2.local_ip4, 24)) self.assertTrue(find_route(self, self.pg2.local_ip4, 32)) # remove the left over ARP entry @@ -1119,10 +1128,14 @@ class TestDHCP(VppTestCase): # and now the route should be gone # self.assertFalse(find_route(self, self.pg2.local_ip4, 32)) + self.assertFalse(find_route(self, self.pg2.local_ip4, 24)) # - # Start the procedure again. this time have VPP send the clientiid + # Start the procedure again. this time have VPP send the client-ID # + self.pg2.admin_down() + self.sleep(1) + self.pg2.admin_up() self.vapi.dhcp_client(self.pg2.sw_if_index, hostname, client_id=self.pg2.local_mac) @@ -1131,10 +1144,47 @@ class TestDHCP(VppTestCase): self.verify_orig_dhcp_discover(rx[0], self.pg2, hostname, self.pg2.local_mac) + self.pg2.add_stream(p_offer) + self.pg_enable_capture(self.pg_interfaces) + self.pg_start() + + rx = self.pg2.get_capture(1) + self.verify_orig_dhcp_request(rx[0], self.pg2, hostname, + self.pg2.local_ip4) + + # + # unicast the ack to the offered address + # + p_ack = (Ether(dst=self.pg2.local_mac, src=self.pg2.remote_mac) / + IP(src=self.pg2.remote_ip4, dst=self.pg2.local_ip4) / + UDP(sport=DHCP4_SERVER_PORT, dport=DHCP4_CLIENT_PORT) / + BOOTP(op=1, yiaddr=self.pg2.local_ip4) / + DHCP(options=[('message-type', 'ack'), + ('subnet_mask', "255.255.255.0"), + ('router', self.pg2.remote_ip4), + ('server_id', self.pg2.remote_ip4), + ('lease_time', 43200), + ('end')])) + + self.pg2.add_stream(p_ack) + self.pg_enable_capture(self.pg_interfaces) + self.pg_start() + + # + # At the end of this procedure there should be a connected route + # in the FIB + # + self.assertTrue(find_route(self, self.pg2.local_ip4, 32)) + self.assertTrue(find_route(self, self.pg2.local_ip4, 24)) + # # remove the DHCP config # self.vapi.dhcp_client(self.pg2.sw_if_index, hostname, is_add=0) + self.assertFalse(find_route(self, self.pg2.local_ip4, 32)) + self.assertFalse(find_route(self, self.pg2.local_ip4, 24)) + + if __name__ == '__main__': unittest.main(testRunner=VppTestRunner)