On Wed, Dec 12, 2018 at 04:26:37PM +0000, Gregory Smith wrote:
> See RFC 2131, section 4.3.2. When handling a DHCPREQUEST message, the
> server should validate that the client's requested IP matches the
> offered IP. If not, the server should reply with a DHCPNAK. The client's
> requested IP is either specified as the Requested IP Address (option
> 50), or as the ciaddr, depending on the client's state.
>
> Signed-off-by: Gregory Smith <[email protected]>
Thanks for v6!
This version still has a couple of ALIGNED_CASTs of pointers that are
not necessarily aligned. Please allow me to suggest another strategy.
First, let's make dhcp_header a packed struct. Packing can have
performance implications, as I understand it, but DHCP packet processing
is not performance-critical. Second, let's take advantage of that to
avoid needing to assume alignment.
I'm going to post a 2-patch series where the first patch packs
dhcp_header and the second one is a small variant of yours. Here's an
incremental showing how I'm proposing to change your patch:
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index bc712209838e..49cd0af51e36 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -617,8 +617,8 @@ pinctrl_handle_put_dhcp_opts(
goto exit;
}
- const struct dhcp_header *in_dhcp_data =
- ALIGNED_CAST(const struct dhcp_header *, in_dhcp_ptr);
+ const struct dhcp_header *in_dhcp_data
+ = (const struct dhcp_header *) in_dhcp_ptr;
in_dhcp_ptr += sizeof *in_dhcp_data;
if (in_dhcp_ptr > end) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -636,18 +636,16 @@ pinctrl_handle_put_dhcp_opts(
* options is the DHCP magic cookie followed by the actual DHCP options.
*/
ovs_be32 magic_cookie = htonl(DHCP_MAGIC_COOKIE);
- const ovs_be32 *in_dhcp_cookie =
- ALIGNED_CAST(const ovs_be32 *, in_dhcp_ptr);
- in_dhcp_ptr += sizeof magic_cookie;
- if (in_dhcp_ptr > end ||
- get_unaligned_be32(in_dhcp_cookie) != magic_cookie) {
+ if (in_dhcp_ptr + sizeof magic_cookie > end ||
+ get_unaligned_be32((const void *) in_dhcp_ptr) != magic_cookie) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
VLOG_WARN_RL(&rl, "DHCP magic cookie not present in the DHCP packet");
goto exit;
}
+ in_dhcp_ptr += sizeof magic_cookie;
const uint8_t *in_dhcp_msg_type = NULL;
- const ovs_be32 *in_dhcp_request_ip = NULL;
+ ovs_be32 request_ip = in_dhcp_data->ciaddr;
while (in_dhcp_ptr < end) {
const struct dhcp_opt_header *in_dhcp_opt =
(const struct dhcp_opt_header *)in_dhcp_ptr;
@@ -670,22 +668,17 @@ pinctrl_handle_put_dhcp_opts(
switch (in_dhcp_opt->code) {
case DHCP_OPT_MSG_TYPE:
if (in_dhcp_opt->len == 1) {
- in_dhcp_msg_type = (const uint8_t *)&in_dhcp_opt[1];
+ in_dhcp_msg_type = DHCP_OPT_PAYLOAD(in_dhcp_opt);
}
break;
case DHCP_OPT_REQ_IP:
if (in_dhcp_opt->len == 4) {
- in_dhcp_request_ip =
- ALIGNED_CAST(const ovs_be32 *, &in_dhcp_opt[1]);
+ request_ip = get_unaligned_be32(DHCP_OPT_PAYLOAD(in_dhcp_opt));
}
break;
default:
break;
}
-
- if (in_dhcp_msg_type && in_dhcp_request_ip) {
- break;
- }
}
/* Check that the DHCP Message Type (opt 53) is present or not with
@@ -712,13 +705,7 @@ pinctrl_handle_put_dhcp_opts(
* requested IP address may be supplied either via Requested IP Address
* (opt 50) or via ciaddr, depending on the client's state.
*/
- ovs_be32 request_ip;
msg_type = DHCP_MSG_ACK;
- if (in_dhcp_request_ip) {
- request_ip = get_unaligned_be32(in_dhcp_request_ip);
- } else {
- request_ip = get_unaligned_be32(&in_dhcp_data->ciaddr);
- }
if (request_ip != *offer_ip) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
VLOG_WARN_RL(&rl, "DHCPREQUEST requested IP "IP_FMT" does not "
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
