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]> --- ovn/controller/pinctrl.c | 28 +++++++++++++--------------- ovn/lib/actions.c | 19 ++++++++++++------- ovn/lib/ovn-dhcp.h | 5 +++-- tests/ovn.at | 8 ++++---- 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index db9e441..849fbce 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -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 @@ -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, @@ -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; } @@ -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; @@ -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; } @@ -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; } diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index df526c0..fa8f175 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -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); } } diff --git a/ovn/lib/ovn-dhcp.h b/ovn/lib/ovn-dhcp.h index ecfcc03..d5561ed 100644 --- a/ovn/lib/ovn-dhcp.h +++ b/ovn/lib/ovn-dhcp.h @@ -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 */ diff --git a/tests/ovn.at b/tests/ovn.at index 38fd1e5..cb21210 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -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"); -- 2.10.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
