[ovs-dev] [PATCH 2/3] ovn: Encode dhcpv6 PACKET_IN userdata as big endian.
Numan Siddique
nusiddiq at redhat.com
Fri Dec 9 13:44:57 UTC 2016
On Fri, Dec 9, 2016 at 8:20 AM, Daniele Di Proietto <diproiettod at vmware.com>
wrote:
> 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 <nusiddiq at redhat.com>
> Reported-at: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=840770
> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
> ---
>
Thanks. I tested it and it works as expected.
Acked-by: Numan Siddique <nusiddiq at redhat.com>
> 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
>
>
More information about the dev
mailing list