Skip to content

Commit

Permalink
ovn: Encode dhcpv6 PACKET_IN userdata as big endian.
Browse files Browse the repository at this point in the history 
The packet in userdata generated by ovn-controller when translating the
put_dhcpv6_opt action includes 16-bit integers.

Currently these 16-bit integers are encoded with native endianness.
This is ok becase the only consumer of that userdata is ovn-controller
itself, but it means that the OpenFlow action we're generating might
not really be the same on different hosts.

I think it's better to encode the userdata as big-endian, like the rest
of OpenFlow messages.

This fixes a test failure on big-endian platforms, because the generated
OpenFlow bytes were different than expected (the expectation was
generated on a little endian machine).

Now 'struct dhcp_opt6_header' is identical to 'struct
dhcpv6_opt_header', but I chose to keep them separate, because they
have different purposes.  I also renamed the members to avoid confusion.

I haven't tested this on a real setup.

CC: Numan Siddique <[email protected]>
Reported-at: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=840770
Signed-off-by: Daniele Di Proietto <[email protected]>
Acked-by: Numan Siddique <[email protected]>
  • Loading branch information
@ddiproietto
ddiproietto committed Dec 12, 2016
1 parent fe11b9e commit a55daca
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 28 deletions.
28 changes: 13 additions & 15 deletions ovn/controller/pinctrl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,13 @@ compose_out_dhcpv6_opts(struct ofpbuf *userdata,
return false;
}

uint8_t *userdata_opt_data = ofpbuf_try_pull(userdata,
userdata_opt->len);
size_t size = ntohs(userdata_opt->size);
uint8_t *userdata_opt_data = ofpbuf_try_pull(userdata, size);
if (!userdata_opt_data) {
return false;
}

switch (userdata_opt->code) {
switch (ntohs(userdata_opt->opt_code)) {
case DHCPV6_OPT_SERVER_ID_CODE:
{
/* The Server Identifier option carries a DUID
Expand All @@ -405,7 +405,7 @@ compose_out_dhcpv6_opts(struct ofpbuf *userdata,
out_dhcpv6_opts, sizeof *opt_server_id);

opt_server_id->opt.code = htons(DHCPV6_OPT_SERVER_ID_CODE);
opt_server_id->opt.len = htons(userdata_opt->len + 4);
opt_server_id->opt.len = htons(size + 4);
opt_server_id->duid_type = htons(DHCPV6_DUID_LL);
opt_server_id->hw_type = htons(DHCPV6_HW_TYPE_ETH);
memcpy(&opt_server_id->mac, userdata_opt_data,
Expand All @@ -415,7 +415,7 @@ compose_out_dhcpv6_opts(struct ofpbuf *userdata,

case DHCPV6_OPT_IA_ADDR_CODE:
{
if (userdata_opt->len != sizeof(struct in6_addr)) {
if (size != sizeof(struct in6_addr)) {
return false;
}

Expand Down Expand Up @@ -444,9 +444,8 @@ compose_out_dhcpv6_opts(struct ofpbuf *userdata,
struct dhcpv6_opt_ia_addr *opt_ia_addr = ofpbuf_put_zeros(
out_dhcpv6_opts, sizeof *opt_ia_addr);
opt_ia_addr->opt.code = htons(DHCPV6_OPT_IA_ADDR_CODE);
opt_ia_addr->opt.len = htons(userdata_opt->len + 8);
memcpy(opt_ia_addr->ipv6.s6_addr, userdata_opt_data,
userdata_opt->len);
opt_ia_addr->opt.len = htons(size + 8);
memcpy(opt_ia_addr->ipv6.s6_addr, userdata_opt_data, size);
opt_ia_addr->t1 = OVS_BE32_MAX;
opt_ia_addr->t2 = OVS_BE32_MAX;
break;
Expand All @@ -457,8 +456,8 @@ compose_out_dhcpv6_opts(struct ofpbuf *userdata,
struct dhcpv6_opt_header *opt_dns = ofpbuf_put_zeros(
out_dhcpv6_opts, sizeof *opt_dns);
opt_dns->code = htons(DHCPV6_OPT_DNS_SERVER_CODE);
opt_dns->len = htons(userdata_opt->len);
ofpbuf_put(out_dhcpv6_opts, userdata_opt_data, userdata_opt->len);
opt_dns->len = htons(size);
ofpbuf_put(out_dhcpv6_opts, userdata_opt_data, size);
break;
}

Expand All @@ -467,11 +466,10 @@ compose_out_dhcpv6_opts(struct ofpbuf *userdata,
struct dhcpv6_opt_header *opt_dsl = ofpbuf_put_zeros(
out_dhcpv6_opts, sizeof *opt_dsl);
opt_dsl->code = htons(DHCPV6_OPT_DOMAIN_SEARCH_CODE);
opt_dsl->len = htons(userdata_opt->len + 2);
uint8_t *data = ofpbuf_put_zeros(out_dhcpv6_opts,
userdata_opt->len + 2);
*data = userdata_opt->len;
memcpy(data + 1, userdata_opt_data, userdata_opt->len);
opt_dsl->len = htons(size + 2);
uint8_t *data = ofpbuf_put_zeros(out_dhcpv6_opts, size + 2);
*data = size;
memcpy(data + 1, userdata_opt_data, size);
break;
}

Expand Down
19 changes: 12 additions & 7 deletions ovn/lib/actions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1527,21 +1527,26 @@ encode_put_dhcpv6_option(const struct ovnact_dhcp_option *o,
struct ofpbuf *ofpacts)
{
struct dhcp_opt6_header *opt = ofpbuf_put_uninit(ofpacts, sizeof *opt);
opt->code = o->option->code;

const union expr_constant *c = o->value.values;
size_t n_values = o->value.n_values;
size_t size;

opt->opt_code = htons(o->option->code);

if (!strcmp(o->option->type, "ipv6")) {
opt->len = n_values * sizeof(struct in6_addr);
size = n_values * sizeof(struct in6_addr);
opt->size = htons(size);
for (size_t i = 0; i < n_values; i++) {
ofpbuf_put(ofpacts, &c[i].value.ipv6, sizeof(struct in6_addr));
}
} else if (!strcmp(o->option->type, "mac")) {
opt->len = sizeof(struct eth_addr);
ofpbuf_put(ofpacts, &c->value.mac, opt->len);
size = sizeof(struct eth_addr);
opt->size = htons(size);
ofpbuf_put(ofpacts, &c->value.mac, size);
} else if (!strcmp(o->option->type, "str")) {
opt->len = strlen(c->string);
ofpbuf_put(ofpacts, c->string, opt->len);
size = strlen(c->string);
opt->size = htons(size);
ofpbuf_put(ofpacts, c->string, size);
}
}

Expand Down
5 changes: 3 additions & 2 deletions ovn/lib/ovn-dhcp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,10 @@ dhcp_opts_destroy(struct hmap *dhcp_opts)
hmap_destroy(dhcp_opts);
}

/* Used in the OpenFlow PACKET_IN userdata */
struct dhcp_opt6_header {
uint16_t code;
uint16_t len;
ovs_be16 opt_code;
ovs_be16 size;
};

/* Supported DHCPv6 Message Types */
Expand Down
8 changes: 4 additions & 4 deletions tests/ovn.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -950,17 +950,17 @@ put_nd(inport, nd.target, nd.sll);

# put_dhcpv6_opts
reg1[0] = put_dhcpv6_opts(ia_addr = ae70::4, server_id = 00:00:00:00:10:02);
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40.05.00.10.00.ae.70.00.00.00.00.00.00.00.00.00.00.00.00.00.04.02.00.06.00.00.00.00.00.10.02,pause)
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40.00.05.00.10.ae.70.00.00.00.00.00.00.00.00.00.00.00.00.00.04.00.02.00.06.00.00.00.00.10.02,pause)
reg1[0] = put_dhcpv6_opts();
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40,pause)
reg1[0] = put_dhcpv6_opts(dns_server={ae70::1,ae70::2});
formats as reg1[0] = put_dhcpv6_opts(dns_server = {ae70::1, ae70::2});
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40.17.00.20.00.ae.70.00.00.00.00.00.00.00.00.00.00.00.00.00.01.ae.70.00.00.00.00.00.00.00.00.00.00.00.00.00.02,pause)
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40.00.17.00.20.ae.70.00.00.00.00.00.00.00.00.00.00.00.00.00.01.ae.70.00.00.00.00.00.00.00.00.00.00.00.00.00.02,pause)
reg1[0] = put_dhcpv6_opts(server_id=12:34:56:78:9a:bc, dns_server={ae70::1,ae89::2});
formats as reg1[0] = put_dhcpv6_opts(server_id = 12:34:56:78:9a:bc, dns_server = {ae70::1, ae89::2});
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40.02.00.06.00.12.34.56.78.9a.bc.17.00.20.00.ae.70.00.00.00.00.00.00.00.00.00.00.00.00.00.01.ae.89.00.00.00.00.00.00.00.00.00.00.00.00.00.02,pause)
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40.00.02.00.06.12.34.56.78.9a.bc.00.17.00.20.ae.70.00.00.00.00.00.00.00.00.00.00.00.00.00.01.ae.89.00.00.00.00.00.00.00.00.00.00.00.00.00.02,pause)
reg1[0] = put_dhcpv6_opts(domain_search = "ovn.org");
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40.18.00.07.00.6f.76.6e.2e.6f.72.67,pause)
encodes as controller(userdata=00.00.00.05.00.00.00.00.00.01.de.10.00.00.00.40.00.18.00.07.6f.76.6e.2e.6f.72.67,pause)
reg1[0] = put_dhcpv6_opts(x = 1.2.3.4);
Syntax error at `x' expecting DHCPv6 option name.
reg1[0] = put_dhcpv6_opts(ia_addr=ae70::4, "hi");
Expand Down

0 comments on commit a55daca

Please sign in to comment.