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 <nranns@cisco.com>

3 files changed, 151 insertions, 29 deletions

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)