dhcp: fix crash on unicast renewal send

Type: fix

- when the addresses were learnt a copy of the client was sent to the
main thread, this meant the unicast adjacecny was saved on the copy
not on the original.
- Add logging.
- Improve the proxy-node that hands the clint packets so the DHCP
packets are traced.
 - allow a renewal to configure new address data

Change-Id: I6ab0afcccbc4a1cdefdd1b8beeda8fc7ba20ec1f
Signed-off-by: Neale Ranns <nranns@cisco.com>
This commit is contained in:
Neale Ranns
2019-10-15 15:47:55 +00:00
parent d7b306657d
commit 6bcc6a4557
12 changed files with 650 additions and 425 deletions
+1
View File
@@ -17,6 +17,7 @@ add_vpp_plugin(dhcp
dhcp_api.c
dhcp_client_detect.c
dhcp_proxy.c
dhcp4_packet.c
dhcp4_proxy_node.c
dhcp6_client_common_dp.c
dhcp6_ia_na_client_dp.c
+244 -230
View File
File diff suppressed because it is too large Load Diff
+28 -5
View File
@@ -42,6 +42,24 @@ struct dhcp_client_t_;
typedef void (*dhcp_event_cb_t) (u32 client_index,
const struct dhcp_client_t_ * client);
/**
* The set of addresses/mask that contribute forwarding info
* and are installed.
*/
typedef struct dhcp_client_fwd_addresses_t_
{
/** the address assigned to this client and it's mask */
ip4_address_t leased_address;
u32 subnet_mask_width;
/** the address of the DHCP server handing out the address.
this is used to send any unicast messages */
ip4_address_t dhcp_server;
/** The address of this client's default gateway - may not be present */
ip4_address_t router_address;
} dhcp_client_fwd_addresses_t;
typedef struct dhcp_client_t_
{
dhcp_client_state_t state;
@@ -59,11 +77,16 @@ typedef struct dhcp_client_t_
/* DHCP transaction ID, a random number */
u32 transaction_id;
/* leased address, other learned info DHCP */
ip4_address_t leased_address; /* from your_ip_address field */
ip4_address_t dhcp_server;
u32 subnet_mask_width; /* option 1 */
ip4_address_t router_address; /* option 3 */
/**
* leased address, other learned info DHCP
* the learned set is updated by new messages recieved in the DP
* the installed set is what's actually been added
*/
dhcp_client_fwd_addresses_t learned;
dhcp_client_fwd_addresses_t installed;
/* have local Addresses and default route been installed */
u8 addresses_installed;
ip4_address_t *domain_server_address; /* option 6 */
u32 lease_renewal_interval; /* option 51 */
u32 lease_lifetime; /* option 59 */
+1 -1
View File
@@ -105,7 +105,7 @@ define dhcp_plugin_control_ping_reply
@param is_add - add the config if non-zero, else delete
@param insert_circuit_id - option82 suboption 1 fib number
@param dhcp_server[] - server address
@param dhcp_src_address[] - <fix this, need details>
@param dhcp_src_address[] - sc address for packets sent to the server
*/
autoreply define dhcp_proxy_config
{
+122
View File
@@ -0,0 +1,122 @@
/*
* dhcp4_packet.c: dhcp packet format functions
*
* Copyright (c) 2013 Cisco and/or its affiliates.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <dhcp/dhcp4_packet.h>
#include <vnet/ip/format.h>
u8 *
format_dhcp_packet_type (u8 * s, va_list * args)
{
dhcp_packet_type_t pt = va_arg (*args, dhcp_packet_type_t);
switch (pt)
{
case DHCP_PACKET_DISCOVER:
s = format (s, "discover");
break;
case DHCP_PACKET_OFFER:
s = format (s, "offer");
break;
case DHCP_PACKET_REQUEST:
s = format (s, "request");
break;
case DHCP_PACKET_ACK:
s = format (s, "ack");
break;
case DHCP_PACKET_NAK:
s = format (s, "nack");
break;
}
return (s);
}
u8 *
format_dhcp_header (u8 * s, va_list * args)
{
dhcp_header_t *d = va_arg (*args, dhcp_header_t *);
u32 max_bytes = va_arg (*args, u32);
dhcp_option_t *o;
u32 tmp;
s = format (s, "opcode:%s", (d->opcode == 1 ? "request" : "reply"));
s = format (s, " hw[type:%d addr-len:%d addr:%U]",
d->hardware_type, d->hardware_address_length,
format_hex_bytes, d->client_hardware_address,
d->hardware_address_length);
s = format (s, " hops%d", d->hops);
s = format (s, " transaction-ID:0x%x", d->transaction_identifier);
s = format (s, " seconds:%d", d->seconds);
s = format (s, " flags:0x%x", d->flags);
s = format (s, " client:%U", format_ip4_address, &d->client_ip_address);
s = format (s, " your:%U", format_ip4_address, &d->your_ip_address);
s = format (s, " server:%U", format_ip4_address, &d->server_ip_address);
s = format (s, " gateway:%U", format_ip4_address, &d->gateway_ip_address);
s = format (s, " cookie:%U", format_ip4_address, &d->magic_cookie);
o = (dhcp_option_t *) d->options;
while (o->option != 0xFF /* end of options */ &&
(u8 *) o < (u8 *) d + max_bytes)
{
switch (o->option)
{
case 53: /* dhcp message type */
tmp = o->data[0];
s =
format (s, ", option-53: type:%U", format_dhcp_packet_type, tmp);
break;
case 54: /* dhcp server address */
s = format (s, ", option-54: server:%U",
format_ip4_address, &o->data_as_u32[0]);
break;
case 58: /* lease renew time in seconds */
s = format (s, ", option-58: renewal:%d",
clib_host_to_net_u32 (o->data_as_u32[0]));
break;
case 1: /* subnet mask */
s = format (s, ", option-1: subnet-mask:%d",
clib_host_to_net_u32 (o->data_as_u32[0]));
break;
case 3: /* router address */
s = format (s, ", option-3: router:%U",
format_ip4_address, &o->data_as_u32[0]);
break;
case 6: /* domain server address */
s = format (s, ", option-6: domian-server:%U",
format_hex_bytes, o->data, o->length);
break;
case 12: /* hostname */
s = format (s, ", option-12: hostname:%U",
format_hex_bytes, o->data, o->length);
break;
default:
tmp = o->option;
s = format (s, " option-%d: skipped", tmp);
break;
}
o = (dhcp_option_t *) (((u8 *) o) + (o->length + 2));
}
return (s);
}
/*
* fd.io coding-style-patch-verification: ON
*
* Local Variables:
* eval: (c-set-style "gnu")
* End:
*/
+4
View File
@@ -51,6 +51,8 @@ typedef struct
dhcp_option_t options[0];
} dhcp_header_t;
extern u8 *format_dhcp_header (u8 * s, va_list * args);
typedef enum
{
DHCP_PACKET_DISCOVER = 1,
@@ -60,6 +62,8 @@ typedef enum
DHCP_PACKET_NAK,
} dhcp_packet_type_t;
extern u8 *format_dhcp_packet_type (u8 * s, va_list * args);
typedef enum dhcp_packet_option_t_
{
DHCP_PACKET_OPTION_MSG_TYPE = 53,
+1
View File
@@ -29,4 +29,5 @@ dhcp_proxy_error (OPTION_82_VSS_NOT_PROCESSED, "DHCP VSS not processed by DHCP s
dhcp_proxy_error (BAD_YIADDR, "DHCP packets with bad your_ip_address fields")
dhcp_proxy_error (BAD_SVR_FIB_OR_ADDRESS, "DHCP packets not from DHCP server or server FIB.")
dhcp_proxy_error (PKT_TOO_BIG, "DHCP packets which are too big.")
dhcp_proxy_error (FOR_US, "DHCP packets for local client.")
File diff suppressed because it is too large Load Diff
+5 -3
View File
@@ -269,10 +269,12 @@ dhcp_client_lease_encode (vl_api_dhcp_lease_t * lease,
clib_memcpy (&lease->hostname, client->hostname, len);
lease->hostname[len] = 0;
lease->mask_width = client->subnet_mask_width;
clib_memcpy (&lease->host_address.un, (u8 *) & client->leased_address,
lease->mask_width = client->installed.subnet_mask_width;
clib_memcpy (&lease->host_address.un,
(u8 *) & client->installed.leased_address,
sizeof (ip4_address_t));
clib_memcpy (&lease->router_address.un, (u8 *) & client->router_address,
clib_memcpy (&lease->router_address.un,
(u8 *) & client->installed.router_address,
sizeof (ip4_address_t));
lease->count = vec_len (client->domain_server_address);
+44 -18
View File
@@ -1464,6 +1464,25 @@ class TestDHCP(VppTestCase):
self.assertTrue(find_route(self, self.pg3.local_ip4, 24))
self.assertTrue(find_route(self, self.pg3.local_ip4, 32))
#
# read the DHCP client details from a dump
#
clients = self.vapi.dhcp_client_dump()
self.assertEqual(clients[0].client.sw_if_index,
self.pg3.sw_if_index)
self.assertEqual(clients[0].lease.sw_if_index,
self.pg3.sw_if_index)
self.assertEqual(clients[0].client.hostname, hostname)
self.assertEqual(clients[0].lease.hostname, hostname)
# 0 = DISCOVER, 1 = REQUEST, 2 = BOUND
self.assertEqual(clients[0].lease.state, 2)
self.assertEqual(clients[0].lease.mask_width, 24)
self.assertEqual(str(clients[0].lease.router_address),
self.pg3.remote_ip4)
self.assertEqual(str(clients[0].lease.host_address),
self.pg3.local_ip4)
#
# wait for the unicasted renewal
# the first attempt will be an ARP packet, since we have not yet
@@ -1492,6 +1511,25 @@ class TestDHCP(VppTestCase):
l2_bc=False,
broadcast=False)
# send an ACK with different data from the original offer *
self.pg3.generate_remote_hosts(4)
p_ack = (Ether(dst=self.pg3.local_mac, src=self.pg3.remote_mac) /
IP(src=self.pg3.remote_ip4, dst=self.pg3.local_ip4) /
UDP(sport=DHCP4_SERVER_PORT, dport=DHCP4_CLIENT_PORT) /
BOOTP(op=1, yiaddr=self.pg3.remote_hosts[3].ip4,
chaddr=mac_pton(self.pg3.local_mac)) /
DHCP(options=[('message-type', 'ack'),
('subnet_mask', "255.255.255.0"),
('router', self.pg3.remote_hosts[1].ip4),
('server_id', self.pg3.remote_hosts[2].ip4),
('lease_time', 43200),
('renewal_time', 2),
'end']))
self.pg3.add_stream(p_ack)
self.pg_enable_capture(self.pg_interfaces)
self.pg_start()
#
# read the DHCP client details from a dump
#
@@ -1501,23 +1539,15 @@ class TestDHCP(VppTestCase):
self.pg3.sw_if_index)
self.assertEqual(clients[0].lease.sw_if_index,
self.pg3.sw_if_index)
self.assertEqual(clients[0].client.hostname.rstrip('\0'),
hostname)
self.assertEqual(clients[0].lease.hostname.rstrip('\0'),
hostname)
self.assertEqual(clients[0].client.hostname, hostname)
self.assertEqual(clients[0].lease.hostname, hostname)
# 0 = DISCOVER, 1 = REQUEST, 2 = BOUND
self.assertEqual(clients[0].lease.state, 2)
self.assertEqual(clients[0].lease.mask_width, 24)
self.assertEqual(str(clients[0].lease.router_address),
self.pg3.remote_ip4)
self.pg3.remote_hosts[1].ip4)
self.assertEqual(str(clients[0].lease.host_address),
self.pg3.local_ip4)
# 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)
self.pg3.remote_hosts[3].ip4)
#
# remove the DHCP config
@@ -1532,6 +1562,8 @@ class TestDHCP(VppTestCase):
#
# Start the procedure again. Use requested lease time option.
# this time wait for the lease to expire and the client to
# self-destruct
#
hostname += "-2"
self.pg3.admin_down()
@@ -1601,12 +1633,6 @@ class TestDHCP(VppTestCase):
self.assertTrue(find_route(self, self.pg3.local_ip4, 32))
self.assertTrue(find_route(self, self.pg3.local_ip4, 24))
# 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)
#
# the route should be gone after the lease expires
#
+1
View File
@@ -26,6 +26,7 @@
#include <vlibmemory/socket_client.h>
void vl_api_rpc_call_main_thread (void *fp, u8 * data, u32 data_length);
void vl_api_force_rpc_call_main_thread (void *fp, u8 * data, u32 data_length);
u16 vl_client_get_first_plugin_msg_id (const char *plugin_name);
void vl_api_send_pending_rpc_requests (vlib_main_t * vm);
u8 *vl_api_serialize_message_table (api_main_t * am, u8 * vector);
+1 -4
View File
@@ -187,10 +187,7 @@ VLIB_REGISTER_NODE (ip4_not_enabled_node) =
.name = "ip4-not-enabled",
.vector_size = sizeof (u32),
.format_trace = format_ip4_forward_next_trace,
.n_next_nodes = 1,
.next_nodes = {
[0] = "error-drop",
},
.sibling_of = "ip4-drop",
};
VLIB_REGISTER_NODE (ip4_punt_node) =