| Message ID | 29b75921d858bbd5372ccb33a80879f4ebfee6b8.1643738069.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] introduce rdnss, dnssl and route_info opt in put_nd_ra_opts action | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Hi Lorenzo, the change is mostly good but I have a few findings down below: On 2/1/22 12:59, Lorenzo Bianconi wrote: > Send non-periodic Router Advertisement with rdnss, dnssl and route_info > options if configured by the user. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1851788 > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > controller/pinctrl.c | 37 ++--------- > include/ovn/actions.h | 1 + > lib/actions.c | 144 ++++++++++++++++++++++++++++++++++++++++++ > lib/ovn-l7.h | 3 + > northd/northd.c | 16 +++++ > tests/ovn.at | 30 +++++++-- > 6 files changed, 197 insertions(+), 34 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index d2bb7f441..247d53d8c 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -3705,38 +3705,17 @@ static void > packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime, > char *dnssl_data) > { > - char *dnssl_list, *t0, *r0 = NULL, dnssl[255] = {}; > size_t prev_l4_size = dp_packet_l4_size(b); > size_t size = sizeof(struct ovs_nd_dnssl); > - int i = 0; > + char dnssl[255] = {}; > + int dnssl_size; > > - dnssl_list = xstrdup(dnssl_data); > - > - /* Multiple DNS Search List must be 'comma' separated > - * (e.g. "a.b.c, d.e.f"). Domain names must be encoded > - * as described in Section 3.1 of RFC1035. > - * (e.g if dns list is a.b.c,www.ovn.org, it will be encoded as: > - * 01 61 01 62 01 63 00 03 77 77 77 03 6f 76 63 03 6f 72 67 00 > - */ > - for (t0 = strtok_r(dnssl_list, ",", &r0); t0; > - t0 = strtok_r(NULL, ",", &r0)) { > - char *t1, *r1 = NULL; > - > - size += strlen(t0) + 2; > - if (size > sizeof(dnssl)) { > - goto out; > - } > - > - for (t1 = strtok_r(t0, ".", &r1); t1; > - t1 = strtok_r(NULL, ".", &r1)) { > - dnssl[i++] = strlen(t1); > - memcpy(&dnssl[i], t1, strlen(t1)); > - i += strlen(t1); > - } > - dnssl[i++] = 0; > + dnssl_size = encode_ra_dnssl_opt(dnssl_data, dnssl, sizeof(dnssl)); Should you pass a copy of dnssl_data instead of passing dnssl_data directly? encode_ra_dnssl_opt uses strtok_r, which will modify dnssl_data. Since dnssl_data is a pointer to ra->config->dnssl, wouldn't that mean that DNSSL packets will be encoded wrongly for every one sent after the first? > + if (dnssl_size < 0) { > + return; > } > - size = ROUND_UP(size, 8); > > + size += dnssl_size; Note: The size calculation is a bit different now, but it actually works out OK since sizeof(ovs_nd_dnssl) == 8. It used to be: 1. Add sizeof(ovs_nd_dnssl) to calculated dnssl packet size 2. Round result up to nearest 8 bytes. Now it's: 1. Calculate dnssl packet size 2. Round that up to nearest 8 bytes 3. Add sizeof(ovs_nd_dnssl). If sizeof(ovs_nd_dnssl) weren't a multiple of 8 this would result in a different calculation. I think it may be worth adding a comment so that some eagle-eyed person in the future doesn't try to "correct" this or report it as a bug. > struct ip6_hdr *nh = dp_packet_l3(b); > nh->ip6_plen = htons(prev_l4_size + size); > > @@ -3746,15 +3725,13 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime, > nd_dnssl->reserved = 0; > put_16aligned_be32(&nd_dnssl->lifetime, lifetime); > > - dp_packet_put(b, dnssl, size - sizeof *nd_dnssl); > + dp_packet_put(b, dnssl, dnssl_size); > > struct ovs_ra_msg *ra = dp_packet_l4(b); > ra->icmph.icmp6_cksum = 0; > uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b)); > ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra, > prev_l4_size + size)); > -out: > - free(dnssl_list); > } > > static void > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index cdef5fb03..0641b927e 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -807,5 +807,6 @@ void ovnacts_encode(const struct ovnact[], size_t ovnacts_len, > > void ovnacts_free(struct ovnact[], size_t ovnacts_len); > char *ovnact_op_to_string(uint32_t); > +int encode_ra_dnssl_opt(char *data, char *buf, int buf_len); > > #endif /* ovn/actions.h */ > diff --git a/lib/actions.c b/lib/actions.c > index d5d8391bb..0d3f78704 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -40,6 +40,7 @@ > #include "simap.h" > #include "uuid.h" > #include "socket-util.h" > +#include "lib/ovn-util.h" > > VLOG_DEFINE_THIS_MODULE(actions); > > @@ -2992,6 +2993,15 @@ parse_put_nd_ra_opts(struct action_context *ctx, const struct expr_field *dst, > case ND_OPT_MTU: > ok = c->format == LEX_F_DECIMAL; > break; > + > + case ND_OPT_RDNSS: > + ok = c->format == LEX_F_IPV6; > + break; > + > + case ND_OPT_ROUTE_INFO_TYPE: > + case ND_OPT_DNSSL: > + ok = !!c->string; > + break; > } > > if (!ok) { > @@ -3022,6 +3032,98 @@ format_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po, > format_put_opts("put_nd_ra_opts", po, s); > } > > +int encode_ra_dnssl_opt(char *data, char *buf, int buf_len) > +{ > + char *t0, *r0 = NULL; > + size_t size = 0; > + int i = 0; > + > + /* Multiple DNS Search List must be 'comma' separated > + * (e.g. "a.b.c, d.e.f"). Domain names must be encoded > + * as described in Section 3.1 of RFC1035. > + * (e.g if dns list is a.b.c,www.ovn.org, it will be encoded as: > + * 01 61 01 62 01 63 00 03 77 77 77 03 6f 76 63 03 6f 72 67 00 > + */ > + for (t0 = strtok_r(data, ",", &r0); t0; > + t0 = strtok_r(NULL, ",", &r0)) { > + char *t1, *r1 = NULL; > + > + size += strlen(t0) + 2; > + if (size > buf_len) { > + return -EINVAL; > + } > + > + for (t1 = strtok_r(t0, ".", &r1); t1; > + t1 = strtok_r(NULL, ".", &r1)) { > + buf[i++] = strlen(t1); I realize that this is not something new that you have introduced, but should there be a check to ensure that strlen(t1) is not greater than UINT8_MAX? > + memcpy(&buf[i], t1, strlen(t1)); > + i += strlen(t1); > + } > + buf[i++] = 0; > + } > + > + return ROUND_UP(size, 8); > +} > + > +static void > +encode_ra_route_info_opt(struct ofpbuf *ofpacts, char *route_data) > +{ > + char *t0, *r0 = NULL; > + > + for (t0 = strtok_r(route_data, ",", &r0); t0; > + t0 = strtok_r(NULL, ",", &r0)) { > + struct ovs_nd_route_info nd_rinfo; > + char *t1, *r1 = NULL; > + int index; > + > + for (t1 = strtok_r(t0, "-", &r1), index = 0; t1; > + t1 = strtok_r(NULL, "-", &r1), index++) { > + > + nd_rinfo.type = ND_OPT_ROUTE_INFO_TYPE; > + nd_rinfo.route_lifetime = htonl(0xffffffff); > + > + switch (index) { > + case 0: > + if (!strcmp(t1, "HIGH")) { > + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_HIGH; > + } else if (!strcmp(t1, "LOW")) { > + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_LOW; > + } else { > + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_NORMAL; > + } > + break; > + case 1: { > + struct lport_addresses route; > + uint8_t plen; > + > + if (!extract_ip_addresses(t1, &route)) { > + break; > + } > + if (!route.n_ipv6_addrs) { > + destroy_lport_addresses(&route); > + break; > + } > + > + nd_rinfo.prefix_len = route.ipv6_addrs->plen; > + plen = DIV_ROUND_UP(nd_rinfo.prefix_len, 64); > + nd_rinfo.len = 1 + plen; > + struct ovs_nd_route_info *rinfo_opt = > + ofpbuf_put_uninit(ofpacts, sizeof *rinfo_opt); > + memcpy(rinfo_opt, &nd_rinfo, sizeof *rinfo_opt); > + void *data = ofpbuf_put_uninit(ofpacts, plen * 8); > + memcpy(data, &route.ipv6_addrs->network, plen * 8); > + > + destroy_lport_addresses(&route); > + index = 0; > + break; > + } > + default: > + break; > + } > + } > + } > +} > + > static void > encode_put_nd_ra_option(const struct ovnact_gen_option *o, > struct ofpbuf *ofpacts, ptrdiff_t ra_offset) > @@ -3096,6 +3198,48 @@ encode_put_nd_ra_option(const struct ovnact_gen_option *o, > sizeof(ovs_be32[4])); > break; > } > + > + case ND_OPT_RDNSS: > + { > + struct nd_rdnss_opt *rdnss_opt = > + ofpbuf_put_uninit(ofpacts, sizeof *rdnss_opt); > + rdnss_opt->type = ND_OPT_RDNSS; > + rdnss_opt->len = 3; > + rdnss_opt->reserved = htons(0); > + put_16aligned_be32(&rdnss_opt->lifetime, htonl(0xffffffff)); > + void *dns = ofpbuf_put_uninit(ofpacts, sizeof(ovs_be32[4])); > + memcpy(dns, &c->value.be128[7].be32, sizeof(ovs_be32[4])); > + break; > + } > + > + case ND_OPT_DNSSL: > + { > + size_t size = sizeof(struct ovs_nd_dnssl); > + char dnssl[255] = {}; > + int dnssl_size; > + > + dnssl_size = encode_ra_dnssl_opt(c->string, dnssl, sizeof(dnssl)); Like in pinctrl, I think a copy of c->string should be passed to encode_ra_dnssl_opt(). Seeing that c is of const type, it seems like it is not intended for its contents to be modified. > + if (dnssl_size < 0) { > + break; > + } > + > + size += dnssl_size; > + struct ovs_nd_dnssl *nd_dnssl = > + ofpbuf_put_uninit(ofpacts, sizeof *nd_dnssl); > + nd_dnssl->type = ND_OPT_DNSSL; > + nd_dnssl->len = size / 8; > + nd_dnssl->reserved = 0; > + put_16aligned_be32(&nd_dnssl->lifetime, htonl(0xffffffff)); > + void *p = ofpbuf_put_uninit(ofpacts, dnssl_size); > + memcpy(p, dnssl, dnssl_size); > + break; > + } > + > + case ND_OPT_ROUTE_INFO_TYPE: > + { > + encode_ra_route_info_opt(ofpacts, c->string); Pass a copy of c->string to encode_ra_route_info_opt() > + break; > + } > } > } > > diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h > index 256e963d9..49ecea81f 100644 > --- a/lib/ovn-l7.h > +++ b/lib/ovn-l7.h > @@ -409,6 +409,9 @@ nd_ra_opts_init(struct hmap *nd_ra_opts) > nd_ra_opt_add(nd_ra_opts, "slla", ND_OPT_SOURCE_LINKADDR, "mac"); > nd_ra_opt_add(nd_ra_opts, "prefix", ND_OPT_PREFIX_INFORMATION, "ipv6"); > nd_ra_opt_add(nd_ra_opts, "mtu", ND_OPT_MTU, "uint32"); > + nd_ra_opt_add(nd_ra_opts, "rdnss", ND_OPT_RDNSS, "ipv6"); > + nd_ra_opt_add(nd_ra_opts, "dnssl", ND_OPT_DNSSL, "str"); > + nd_ra_opt_add(nd_ra_opts, "route_info", ND_OPT_ROUTE_INFO_TYPE, "str"); > } > > #define EMPTY_LB_VIP 1 > diff --git a/northd/northd.c b/northd/northd.c > index fc7a64f99..861812259 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -10855,6 +10855,22 @@ build_ND_RA_flows_for_lrouter_port( > ds_put_format(actions, ", router_preference = \"%s\"", prf); > } > > + const char *ra_rdnss = smap_get(&op->nbrp->ipv6_ra_configs, "rdnss"); > + if (ra_rdnss) { > + ds_put_format(actions, ", rdnss = %s", ra_rdnss); > + } > + > + const char *ra_dnssl = smap_get(&op->nbrp->ipv6_ra_configs, "dnssl"); > + if (ra_dnssl) { > + ds_put_format(actions, ", dnssl = \"%s\"", ra_dnssl); > + } > + > + const char *route_info = smap_get(&op->nbrp->ipv6_ra_configs, > + "route_info"); > + if (route_info) { > + ds_put_format(actions, ", route_info = \"%s\"", route_info); > + } > + > bool add_rs_response_flow = false; > > for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { > diff --git a/tests/ovn.at b/tests/ovn.at > index 957eb7850..33685109f 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -11902,9 +11902,10 @@ options:rxq_pcap=${pcap_file}-rx.pcap > OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) > > # This shell function sends a Router Solicitation packet. > -# test_ipv6_ra INPORT SRC_MAC SRC_LLA ADDR_MODE MTU RA_PREFIX_OPT > +# test_ipv6_ra INPORT SRC_MAC SRC_LLA ADDR_MODE MTU RA_PREFIX_OPT RDNSS DNSSL ROUTE_INFO > test_ipv6_ra() { > local inport=$1 src_mac=$2 src_lla=$3 addr_mode=$4 mtu=$5 prefix_opt=$6 > + local rdnss=$7 dnssl=$8 route_info=$9 > local request=333300000002${src_mac}86dd6000000000103aff${src_lla}ff02000000000000000000000000000285000efc000000000101${src_mac} > > local len=24 > @@ -11914,6 +11915,18 @@ test_ipv6_ra() { > mtu_opt=05010000${mtu} > fi > > + if test ${#rdnss} != 0; then > + len=`expr $len + ${#rdnss} / 2` > + fi > + > + if test ${#dnssl} != 0; then > + len=`expr $len + ${#dnssl} / 2` > + fi > + > + if test ${#route_info} != 0; then > + len=`expr $len + ${#route_info} / 2` > + fi > + > if test ${#prefix_opt} != 0; then > prefix_opt=${prefix_opt}fdad1234567800000000000000000000 > len=`expr $len + ${#prefix_opt} / 2` > @@ -11922,7 +11935,7 @@ test_ipv6_ra() { > len=$(printf "%x" $len) > local lrp_mac=fa163e000001 > local lrp_lla=fe80000000000000f8163efffe000001 > - local reply=${src_mac}${lrp_mac}86dd6000000000${len}3aff${lrp_lla}${src_lla}8600XXXXff${addr_mode}ffff00000000000000000101${lrp_mac}${mtu_opt}${prefix_opt} > + local reply=${src_mac}${lrp_mac}86dd6000000000${len}3aff${lrp_lla}${src_lla}8600XXXXff${addr_mode}ffff00000000000000000101${lrp_mac}${mtu_opt}${rdnss}${dnssl}${route_info}${prefix_opt} > echo $reply >> $inport.expected > > as hv1 ovs-appctl netdev-dummy/receive hv1-vif${inport} $request > @@ -11960,6 +11973,9 @@ reset_pcap_file hv1-vif3 hv1/vif3 > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:mtu=1500 > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:send_periodic="false" > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="LOW" > +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:rdnss=1000::11 > +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:dnssl=aa.bb.cc > +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:route_info=HIGH-1001::11/48,LOW-1002::11/96 > > # Make sure that ovn-controller has installed the corresponding OF Flow. > OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) > @@ -11970,8 +11986,11 @@ default_prefix_option_config=030440c0ffffffffffffffff00000000 > src_mac=fa163e000003 > src_lla=fe80000000000000f8163efffe000003 > mtu=000005dc > +rdnss=19030000ffffffff10000000000000000000000000000011 > +dnssl=1f030000ffffffff02616102626202636300000000000000 > +route_info=18023008ffffffff100100000000000018036018ffffffff10020000000000000000000000000000 > > -test_ipv6_ra 2 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config > +test_ipv6_ra 2 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config $rdnss $dnssl $route_info > > # NXT_RESUME should be 2. > OVS_WAIT_UNTIL([test 2 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -11993,6 +12012,8 @@ reset_pcap_file hv1-vif3 hv1/vif3 > # Set the address mode to dhcpv6_stateful, router_preference to HIGH > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:address_mode=dhcpv6_stateful > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="HIGH" > +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs rdnss > +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs route_info > # Make sure that ovn-controller has installed the corresponding OF Flow. > OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) > > @@ -12003,7 +12024,7 @@ src_mac=fa163e000004 > src_lla=fe80000000000000f8163efffe000004 > mtu=000005dc > > -test_ipv6_ra 3 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config > +test_ipv6_ra 3 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config "" $dnssl > > # NXT_RESUME should be 3. > OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > @@ -12025,6 +12046,7 @@ reset_pcap_file hv1-vif3 hv1/vif3 > # Set the address mode to dhcpv6_stateless, reset router preference to default > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:address_mode=dhcpv6_stateless > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="MEDIUM" > +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs dnssl > # Make sure that ovn-controller has installed the corresponding OF Flow. > OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) > >
> Hi Lorenzo, the change is mostly good but I have a few findings down below: Hi Mark, thx for the review. > > On 2/1/22 12:59, Lorenzo Bianconi wrote: > > Send non-periodic Router Advertisement with rdnss, dnssl and route_info > > options if configured by the user. > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1851788 > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > --- > > controller/pinctrl.c | 37 ++--------- > > include/ovn/actions.h | 1 + > > lib/actions.c | 144 ++++++++++++++++++++++++++++++++++++++++++ > > lib/ovn-l7.h | 3 + > > northd/northd.c | 16 +++++ > > tests/ovn.at | 30 +++++++-- > > 6 files changed, 197 insertions(+), 34 deletions(-) > > > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > index d2bb7f441..247d53d8c 100644 > > --- a/controller/pinctrl.c > > +++ b/controller/pinctrl.c > > @@ -3705,38 +3705,17 @@ static void > > packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime, > > char *dnssl_data) > > { > > - char *dnssl_list, *t0, *r0 = NULL, dnssl[255] = {}; > > size_t prev_l4_size = dp_packet_l4_size(b); > > size_t size = sizeof(struct ovs_nd_dnssl); > > - int i = 0; > > + char dnssl[255] = {}; > > + int dnssl_size; > > - dnssl_list = xstrdup(dnssl_data); > > - > > - /* Multiple DNS Search List must be 'comma' separated > > - * (e.g. "a.b.c, d.e.f"). Domain names must be encoded > > - * as described in Section 3.1 of RFC1035. > > - * (e.g if dns list is a.b.c,www.ovn.org, it will be encoded as: > > - * 01 61 01 62 01 63 00 03 77 77 77 03 6f 76 63 03 6f 72 67 00 > > - */ > > - for (t0 = strtok_r(dnssl_list, ",", &r0); t0; > > - t0 = strtok_r(NULL, ",", &r0)) { > > - char *t1, *r1 = NULL; > > - > > - size += strlen(t0) + 2; > > - if (size > sizeof(dnssl)) { > > - goto out; > > - } > > - > > - for (t1 = strtok_r(t0, ".", &r1); t1; > > - t1 = strtok_r(NULL, ".", &r1)) { > > - dnssl[i++] = strlen(t1); > > - memcpy(&dnssl[i], t1, strlen(t1)); > > - i += strlen(t1); > > - } > > - dnssl[i++] = 0; > > + dnssl_size = encode_ra_dnssl_opt(dnssl_data, dnssl, sizeof(dnssl)); > > Should you pass a copy of dnssl_data instead of passing dnssl_data directly? > encode_ra_dnssl_opt uses strtok_r, which will modify dnssl_data. Since > dnssl_data is a pointer to ra->config->dnssl, wouldn't that mean that DNSSL > packets will be encoded wrongly for every one sent after the first? ack, right. I will fix it in v2. > > > + if (dnssl_size < 0) { > > + return; > > } > > - size = ROUND_UP(size, 8); > > + size += dnssl_size; > > Note: The size calculation is a bit different now, but it actually works out > OK since sizeof(ovs_nd_dnssl) == 8. > > It used to be: > 1. Add sizeof(ovs_nd_dnssl) to calculated dnssl packet size > 2. Round result up to nearest 8 bytes. > > Now it's: > 1. Calculate dnssl packet size > 2. Round that up to nearest 8 bytes > 3. Add sizeof(ovs_nd_dnssl). > > If sizeof(ovs_nd_dnssl) weren't a multiple of 8 this would result in a > different calculation. > > I think it may be worth adding a comment so that some eagle-eyed person in > the future doesn't try to "correct" this or report it as a bug. I guess we can sum sizeof(ovs_nd_dnssl) in encode_ra_dnssl_opt. > > > struct ip6_hdr *nh = dp_packet_l3(b); > > nh->ip6_plen = htons(prev_l4_size + size); > > @@ -3746,15 +3725,13 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime, > > nd_dnssl->reserved = 0; > > put_16aligned_be32(&nd_dnssl->lifetime, lifetime); > > - dp_packet_put(b, dnssl, size - sizeof *nd_dnssl); > > + dp_packet_put(b, dnssl, dnssl_size); > > struct ovs_ra_msg *ra = dp_packet_l4(b); > > ra->icmph.icmp6_cksum = 0; > > uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b)); > > ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra, > > prev_l4_size + size)); > > -out: > > - free(dnssl_list); > > } > > static void > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > > index cdef5fb03..0641b927e 100644 > > --- a/include/ovn/actions.h > > +++ b/include/ovn/actions.h > > @@ -807,5 +807,6 @@ void ovnacts_encode(const struct ovnact[], size_t ovnacts_len, > > void ovnacts_free(struct ovnact[], size_t ovnacts_len); > > char *ovnact_op_to_string(uint32_t); > > +int encode_ra_dnssl_opt(char *data, char *buf, int buf_len); > > #endif /* ovn/actions.h */ > > diff --git a/lib/actions.c b/lib/actions.c > > index d5d8391bb..0d3f78704 100644 > > --- a/lib/actions.c > > +++ b/lib/actions.c > > @@ -40,6 +40,7 @@ > > #include "simap.h" > > #include "uuid.h" > > #include "socket-util.h" > > +#include "lib/ovn-util.h" > > VLOG_DEFINE_THIS_MODULE(actions); > > > > @@ -2992,6 +2993,15 @@ parse_put_nd_ra_opts(struct action_context *ctx, const struct expr_field *dst, > > case ND_OPT_MTU: > > ok = c->format == LEX_F_DECIMAL; > > break; > > + > > + case ND_OPT_RDNSS: > > + ok = c->format == LEX_F_IPV6; > > + break; > > + > > + case ND_OPT_ROUTE_INFO_TYPE: > > + case ND_OPT_DNSSL: > > + ok = !!c->string; > > + break; > > } > > if (!ok) { > > @@ -3022,6 +3032,98 @@ format_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po, > > format_put_opts("put_nd_ra_opts", po, s); > > } > > +int encode_ra_dnssl_opt(char *data, char *buf, int buf_len) > > +{ > > + char *t0, *r0 = NULL; > > + size_t size = 0; > > + int i = 0; > > + > > + /* Multiple DNS Search List must be 'comma' separated > > + * (e.g. "a.b.c, d.e.f"). Domain names must be encoded > > + * as described in Section 3.1 of RFC1035. > > + * (e.g if dns list is a.b.c,www.ovn.org, it will be encoded as: > > + * 01 61 01 62 01 63 00 03 77 77 77 03 6f 76 63 03 6f 72 67 00 > > + */ > > + for (t0 = strtok_r(data, ",", &r0); t0; > > + t0 = strtok_r(NULL, ",", &r0)) { > > + char *t1, *r1 = NULL; > > + > > + size += strlen(t0) + 2; > > + if (size > buf_len) { > > + return -EINVAL; > > + } > > + > > + for (t1 = strtok_r(t0, ".", &r1); t1; > > + t1 = strtok_r(NULL, ".", &r1)) { > > + buf[i++] = strlen(t1); > > I realize that this is not something new that you have introduced, but > should there be a check to ensure that strlen(t1) is not greater than > UINT8_MAX? ack, fine. I will add it in v2. > > > + memcpy(&buf[i], t1, strlen(t1)); > > + i += strlen(t1); > > + } > > + buf[i++] = 0; > > + } > > + > > + return ROUND_UP(size, 8); > > +} > > + > > +static void > > +encode_ra_route_info_opt(struct ofpbuf *ofpacts, char *route_data) > > +{ > > + char *t0, *r0 = NULL; > > + > > + for (t0 = strtok_r(route_data, ",", &r0); t0; > > + t0 = strtok_r(NULL, ",", &r0)) { > > + struct ovs_nd_route_info nd_rinfo; > > + char *t1, *r1 = NULL; > > + int index; > > + > > + for (t1 = strtok_r(t0, "-", &r1), index = 0; t1; > > + t1 = strtok_r(NULL, "-", &r1), index++) { > > + > > + nd_rinfo.type = ND_OPT_ROUTE_INFO_TYPE; > > + nd_rinfo.route_lifetime = htonl(0xffffffff); > > + > > + switch (index) { > > + case 0: > > + if (!strcmp(t1, "HIGH")) { > > + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_HIGH; > > + } else if (!strcmp(t1, "LOW")) { > > + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_LOW; > > + } else { > > + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_NORMAL; > > + } > > + break; > > + case 1: { > > + struct lport_addresses route; > > + uint8_t plen; > > + > > + if (!extract_ip_addresses(t1, &route)) { > > + break; > > + } > > + if (!route.n_ipv6_addrs) { > > + destroy_lport_addresses(&route); > > + break; > > + } > > + > > + nd_rinfo.prefix_len = route.ipv6_addrs->plen; > > + plen = DIV_ROUND_UP(nd_rinfo.prefix_len, 64); > > + nd_rinfo.len = 1 + plen; > > + struct ovs_nd_route_info *rinfo_opt = > > + ofpbuf_put_uninit(ofpacts, sizeof *rinfo_opt); > > + memcpy(rinfo_opt, &nd_rinfo, sizeof *rinfo_opt); > > + void *data = ofpbuf_put_uninit(ofpacts, plen * 8); > > + memcpy(data, &route.ipv6_addrs->network, plen * 8); > > + > > + destroy_lport_addresses(&route); > > + index = 0; > > + break; > > + } > > + default: > > + break; > > + } > > + } > > + } > > +} > > + > > static void > > encode_put_nd_ra_option(const struct ovnact_gen_option *o, > > struct ofpbuf *ofpacts, ptrdiff_t ra_offset) > > @@ -3096,6 +3198,48 @@ encode_put_nd_ra_option(const struct ovnact_gen_option *o, > > sizeof(ovs_be32[4])); > > break; > > } > > + > > + case ND_OPT_RDNSS: > > + { > > + struct nd_rdnss_opt *rdnss_opt = > > + ofpbuf_put_uninit(ofpacts, sizeof *rdnss_opt); > > + rdnss_opt->type = ND_OPT_RDNSS; > > + rdnss_opt->len = 3; > > + rdnss_opt->reserved = htons(0); > > + put_16aligned_be32(&rdnss_opt->lifetime, htonl(0xffffffff)); > > + void *dns = ofpbuf_put_uninit(ofpacts, sizeof(ovs_be32[4])); > > + memcpy(dns, &c->value.be128[7].be32, sizeof(ovs_be32[4])); > > + break; > > + } > > + > > + case ND_OPT_DNSSL: > > + { > > + size_t size = sizeof(struct ovs_nd_dnssl); > > + char dnssl[255] = {}; > > + int dnssl_size; > > + > > + dnssl_size = encode_ra_dnssl_opt(c->string, dnssl, sizeof(dnssl)); > > Like in pinctrl, I think a copy of c->string should be passed to > encode_ra_dnssl_opt(). Seeing that c is of const type, it seems like it is > not intended for its contents to be modified. ack, I will fix it in v2. > > > + if (dnssl_size < 0) { > > + break; > > + } > > + > > + size += dnssl_size; > > + struct ovs_nd_dnssl *nd_dnssl = > > + ofpbuf_put_uninit(ofpacts, sizeof *nd_dnssl); > > + nd_dnssl->type = ND_OPT_DNSSL; > > + nd_dnssl->len = size / 8; > > + nd_dnssl->reserved = 0; > > + put_16aligned_be32(&nd_dnssl->lifetime, htonl(0xffffffff)); > > + void *p = ofpbuf_put_uninit(ofpacts, dnssl_size); > > + memcpy(p, dnssl, dnssl_size); > > + break; > > + } > > + > > + case ND_OPT_ROUTE_INFO_TYPE: > > + { > > + encode_ra_route_info_opt(ofpacts, c->string); > > Pass a copy of c->string to encode_ra_route_info_opt() ack, I will fix it in v2. Regards, Lorenzo > > > + break; > > + } > > } > > } > > diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h > > index 256e963d9..49ecea81f 100644 > > --- a/lib/ovn-l7.h > > +++ b/lib/ovn-l7.h > > @@ -409,6 +409,9 @@ nd_ra_opts_init(struct hmap *nd_ra_opts) > > nd_ra_opt_add(nd_ra_opts, "slla", ND_OPT_SOURCE_LINKADDR, "mac"); > > nd_ra_opt_add(nd_ra_opts, "prefix", ND_OPT_PREFIX_INFORMATION, "ipv6"); > > nd_ra_opt_add(nd_ra_opts, "mtu", ND_OPT_MTU, "uint32"); > > + nd_ra_opt_add(nd_ra_opts, "rdnss", ND_OPT_RDNSS, "ipv6"); > > + nd_ra_opt_add(nd_ra_opts, "dnssl", ND_OPT_DNSSL, "str"); > > + nd_ra_opt_add(nd_ra_opts, "route_info", ND_OPT_ROUTE_INFO_TYPE, "str"); > > } > > #define EMPTY_LB_VIP 1 > > diff --git a/northd/northd.c b/northd/northd.c > > index fc7a64f99..861812259 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -10855,6 +10855,22 @@ build_ND_RA_flows_for_lrouter_port( > > ds_put_format(actions, ", router_preference = \"%s\"", prf); > > } > > + const char *ra_rdnss = smap_get(&op->nbrp->ipv6_ra_configs, "rdnss"); > > + if (ra_rdnss) { > > + ds_put_format(actions, ", rdnss = %s", ra_rdnss); > > + } > > + > > + const char *ra_dnssl = smap_get(&op->nbrp->ipv6_ra_configs, "dnssl"); > > + if (ra_dnssl) { > > + ds_put_format(actions, ", dnssl = \"%s\"", ra_dnssl); > > + } > > + > > + const char *route_info = smap_get(&op->nbrp->ipv6_ra_configs, > > + "route_info"); > > + if (route_info) { > > + ds_put_format(actions, ", route_info = \"%s\"", route_info); > > + } > > + > > bool add_rs_response_flow = false; > > for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 957eb7850..33685109f 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -11902,9 +11902,10 @@ options:rxq_pcap=${pcap_file}-rx.pcap > > OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) > > # This shell function sends a Router Solicitation packet. > > -# test_ipv6_ra INPORT SRC_MAC SRC_LLA ADDR_MODE MTU RA_PREFIX_OPT > > +# test_ipv6_ra INPORT SRC_MAC SRC_LLA ADDR_MODE MTU RA_PREFIX_OPT RDNSS DNSSL ROUTE_INFO > > test_ipv6_ra() { > > local inport=$1 src_mac=$2 src_lla=$3 addr_mode=$4 mtu=$5 prefix_opt=$6 > > + local rdnss=$7 dnssl=$8 route_info=$9 > > local request=333300000002${src_mac}86dd6000000000103aff${src_lla}ff02000000000000000000000000000285000efc000000000101${src_mac} > > local len=24 > > @@ -11914,6 +11915,18 @@ test_ipv6_ra() { > > mtu_opt=05010000${mtu} > > fi > > + if test ${#rdnss} != 0; then > > + len=`expr $len + ${#rdnss} / 2` > > + fi > > + > > + if test ${#dnssl} != 0; then > > + len=`expr $len + ${#dnssl} / 2` > > + fi > > + > > + if test ${#route_info} != 0; then > > + len=`expr $len + ${#route_info} / 2` > > + fi > > + > > if test ${#prefix_opt} != 0; then > > prefix_opt=${prefix_opt}fdad1234567800000000000000000000 > > len=`expr $len + ${#prefix_opt} / 2` > > @@ -11922,7 +11935,7 @@ test_ipv6_ra() { > > len=$(printf "%x" $len) > > local lrp_mac=fa163e000001 > > local lrp_lla=fe80000000000000f8163efffe000001 > > - local reply=${src_mac}${lrp_mac}86dd6000000000${len}3aff${lrp_lla}${src_lla}8600XXXXff${addr_mode}ffff00000000000000000101${lrp_mac}${mtu_opt}${prefix_opt} > > + local reply=${src_mac}${lrp_mac}86dd6000000000${len}3aff${lrp_lla}${src_lla}8600XXXXff${addr_mode}ffff00000000000000000101${lrp_mac}${mtu_opt}${rdnss}${dnssl}${route_info}${prefix_opt} > > echo $reply >> $inport.expected > > as hv1 ovs-appctl netdev-dummy/receive hv1-vif${inport} $request > > @@ -11960,6 +11973,9 @@ reset_pcap_file hv1-vif3 hv1/vif3 > > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:mtu=1500 > > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:send_periodic="false" > > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="LOW" > > +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:rdnss=1000::11 > > +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:dnssl=aa.bb.cc > > +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:route_info=HIGH-1001::11/48,LOW-1002::11/96 > > # Make sure that ovn-controller has installed the corresponding OF Flow. > > OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) > > @@ -11970,8 +11986,11 @@ default_prefix_option_config=030440c0ffffffffffffffff00000000 > > src_mac=fa163e000003 > > src_lla=fe80000000000000f8163efffe000003 > > mtu=000005dc > > +rdnss=19030000ffffffff10000000000000000000000000000011 > > +dnssl=1f030000ffffffff02616102626202636300000000000000 > > +route_info=18023008ffffffff100100000000000018036018ffffffff10020000000000000000000000000000 > > -test_ipv6_ra 2 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config > > +test_ipv6_ra 2 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config $rdnss $dnssl $route_info > > # NXT_RESUME should be 2. > > OVS_WAIT_UNTIL([test 2 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -11993,6 +12012,8 @@ reset_pcap_file hv1-vif3 hv1/vif3 > > # Set the address mode to dhcpv6_stateful, router_preference to HIGH > > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:address_mode=dhcpv6_stateful > > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="HIGH" > > +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs rdnss > > +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs route_info > > # Make sure that ovn-controller has installed the corresponding OF Flow. > > OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) > > @@ -12003,7 +12024,7 @@ src_mac=fa163e000004 > > src_lla=fe80000000000000f8163efffe000004 > > mtu=000005dc > > -test_ipv6_ra 3 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config > > +test_ipv6_ra 3 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config "" $dnssl > > # NXT_RESUME should be 3. > > OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) > > @@ -12025,6 +12046,7 @@ reset_pcap_file hv1-vif3 hv1/vif3 > > # Set the address mode to dhcpv6_stateless, reset router preference to default > > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:address_mode=dhcpv6_stateless > > ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="MEDIUM" > > +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs dnssl > > # Make sure that ovn-controller has installed the corresponding OF Flow. > > OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) > > >
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index d2bb7f441..247d53d8c 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -3705,38 +3705,17 @@ static void packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime, char *dnssl_data) { - char *dnssl_list, *t0, *r0 = NULL, dnssl[255] = {}; size_t prev_l4_size = dp_packet_l4_size(b); size_t size = sizeof(struct ovs_nd_dnssl); - int i = 0; + char dnssl[255] = {}; + int dnssl_size; - dnssl_list = xstrdup(dnssl_data); - - /* Multiple DNS Search List must be 'comma' separated - * (e.g. "a.b.c, d.e.f"). Domain names must be encoded - * as described in Section 3.1 of RFC1035. - * (e.g if dns list is a.b.c,www.ovn.org, it will be encoded as: - * 01 61 01 62 01 63 00 03 77 77 77 03 6f 76 63 03 6f 72 67 00 - */ - for (t0 = strtok_r(dnssl_list, ",", &r0); t0; - t0 = strtok_r(NULL, ",", &r0)) { - char *t1, *r1 = NULL; - - size += strlen(t0) + 2; - if (size > sizeof(dnssl)) { - goto out; - } - - for (t1 = strtok_r(t0, ".", &r1); t1; - t1 = strtok_r(NULL, ".", &r1)) { - dnssl[i++] = strlen(t1); - memcpy(&dnssl[i], t1, strlen(t1)); - i += strlen(t1); - } - dnssl[i++] = 0; + dnssl_size = encode_ra_dnssl_opt(dnssl_data, dnssl, sizeof(dnssl)); + if (dnssl_size < 0) { + return; } - size = ROUND_UP(size, 8); + size += dnssl_size; struct ip6_hdr *nh = dp_packet_l3(b); nh->ip6_plen = htons(prev_l4_size + size); @@ -3746,15 +3725,13 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime, nd_dnssl->reserved = 0; put_16aligned_be32(&nd_dnssl->lifetime, lifetime); - dp_packet_put(b, dnssl, size - sizeof *nd_dnssl); + dp_packet_put(b, dnssl, dnssl_size); struct ovs_ra_msg *ra = dp_packet_l4(b); ra->icmph.icmp6_cksum = 0; uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b)); ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra, prev_l4_size + size)); -out: - free(dnssl_list); } static void diff --git a/include/ovn/actions.h b/include/ovn/actions.h index cdef5fb03..0641b927e 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -807,5 +807,6 @@ void ovnacts_encode(const struct ovnact[], size_t ovnacts_len, void ovnacts_free(struct ovnact[], size_t ovnacts_len); char *ovnact_op_to_string(uint32_t); +int encode_ra_dnssl_opt(char *data, char *buf, int buf_len); #endif /* ovn/actions.h */ diff --git a/lib/actions.c b/lib/actions.c index d5d8391bb..0d3f78704 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -40,6 +40,7 @@ #include "simap.h" #include "uuid.h" #include "socket-util.h" +#include "lib/ovn-util.h" VLOG_DEFINE_THIS_MODULE(actions); @@ -2992,6 +2993,15 @@ parse_put_nd_ra_opts(struct action_context *ctx, const struct expr_field *dst, case ND_OPT_MTU: ok = c->format == LEX_F_DECIMAL; break; + + case ND_OPT_RDNSS: + ok = c->format == LEX_F_IPV6; + break; + + case ND_OPT_ROUTE_INFO_TYPE: + case ND_OPT_DNSSL: + ok = !!c->string; + break; } if (!ok) { @@ -3022,6 +3032,98 @@ format_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po, format_put_opts("put_nd_ra_opts", po, s); } +int encode_ra_dnssl_opt(char *data, char *buf, int buf_len) +{ + char *t0, *r0 = NULL; + size_t size = 0; + int i = 0; + + /* Multiple DNS Search List must be 'comma' separated + * (e.g. "a.b.c, d.e.f"). Domain names must be encoded + * as described in Section 3.1 of RFC1035. + * (e.g if dns list is a.b.c,www.ovn.org, it will be encoded as: + * 01 61 01 62 01 63 00 03 77 77 77 03 6f 76 63 03 6f 72 67 00 + */ + for (t0 = strtok_r(data, ",", &r0); t0; + t0 = strtok_r(NULL, ",", &r0)) { + char *t1, *r1 = NULL; + + size += strlen(t0) + 2; + if (size > buf_len) { + return -EINVAL; + } + + for (t1 = strtok_r(t0, ".", &r1); t1; + t1 = strtok_r(NULL, ".", &r1)) { + buf[i++] = strlen(t1); + memcpy(&buf[i], t1, strlen(t1)); + i += strlen(t1); + } + buf[i++] = 0; + } + + return ROUND_UP(size, 8); +} + +static void +encode_ra_route_info_opt(struct ofpbuf *ofpacts, char *route_data) +{ + char *t0, *r0 = NULL; + + for (t0 = strtok_r(route_data, ",", &r0); t0; + t0 = strtok_r(NULL, ",", &r0)) { + struct ovs_nd_route_info nd_rinfo; + char *t1, *r1 = NULL; + int index; + + for (t1 = strtok_r(t0, "-", &r1), index = 0; t1; + t1 = strtok_r(NULL, "-", &r1), index++) { + + nd_rinfo.type = ND_OPT_ROUTE_INFO_TYPE; + nd_rinfo.route_lifetime = htonl(0xffffffff); + + switch (index) { + case 0: + if (!strcmp(t1, "HIGH")) { + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_HIGH; + } else if (!strcmp(t1, "LOW")) { + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_LOW; + } else { + nd_rinfo.flags = IPV6_ND_RA_OPT_PRF_NORMAL; + } + break; + case 1: { + struct lport_addresses route; + uint8_t plen; + + if (!extract_ip_addresses(t1, &route)) { + break; + } + if (!route.n_ipv6_addrs) { + destroy_lport_addresses(&route); + break; + } + + nd_rinfo.prefix_len = route.ipv6_addrs->plen; + plen = DIV_ROUND_UP(nd_rinfo.prefix_len, 64); + nd_rinfo.len = 1 + plen; + struct ovs_nd_route_info *rinfo_opt = + ofpbuf_put_uninit(ofpacts, sizeof *rinfo_opt); + memcpy(rinfo_opt, &nd_rinfo, sizeof *rinfo_opt); + void *data = ofpbuf_put_uninit(ofpacts, plen * 8); + memcpy(data, &route.ipv6_addrs->network, plen * 8); + + destroy_lport_addresses(&route); + index = 0; + break; + } + default: + break; + } + } + } +} + static void encode_put_nd_ra_option(const struct ovnact_gen_option *o, struct ofpbuf *ofpacts, ptrdiff_t ra_offset) @@ -3096,6 +3198,48 @@ encode_put_nd_ra_option(const struct ovnact_gen_option *o, sizeof(ovs_be32[4])); break; } + + case ND_OPT_RDNSS: + { + struct nd_rdnss_opt *rdnss_opt = + ofpbuf_put_uninit(ofpacts, sizeof *rdnss_opt); + rdnss_opt->type = ND_OPT_RDNSS; + rdnss_opt->len = 3; + rdnss_opt->reserved = htons(0); + put_16aligned_be32(&rdnss_opt->lifetime, htonl(0xffffffff)); + void *dns = ofpbuf_put_uninit(ofpacts, sizeof(ovs_be32[4])); + memcpy(dns, &c->value.be128[7].be32, sizeof(ovs_be32[4])); + break; + } + + case ND_OPT_DNSSL: + { + size_t size = sizeof(struct ovs_nd_dnssl); + char dnssl[255] = {}; + int dnssl_size; + + dnssl_size = encode_ra_dnssl_opt(c->string, dnssl, sizeof(dnssl)); + if (dnssl_size < 0) { + break; + } + + size += dnssl_size; + struct ovs_nd_dnssl *nd_dnssl = + ofpbuf_put_uninit(ofpacts, sizeof *nd_dnssl); + nd_dnssl->type = ND_OPT_DNSSL; + nd_dnssl->len = size / 8; + nd_dnssl->reserved = 0; + put_16aligned_be32(&nd_dnssl->lifetime, htonl(0xffffffff)); + void *p = ofpbuf_put_uninit(ofpacts, dnssl_size); + memcpy(p, dnssl, dnssl_size); + break; + } + + case ND_OPT_ROUTE_INFO_TYPE: + { + encode_ra_route_info_opt(ofpacts, c->string); + break; + } } } diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h index 256e963d9..49ecea81f 100644 --- a/lib/ovn-l7.h +++ b/lib/ovn-l7.h @@ -409,6 +409,9 @@ nd_ra_opts_init(struct hmap *nd_ra_opts) nd_ra_opt_add(nd_ra_opts, "slla", ND_OPT_SOURCE_LINKADDR, "mac"); nd_ra_opt_add(nd_ra_opts, "prefix", ND_OPT_PREFIX_INFORMATION, "ipv6"); nd_ra_opt_add(nd_ra_opts, "mtu", ND_OPT_MTU, "uint32"); + nd_ra_opt_add(nd_ra_opts, "rdnss", ND_OPT_RDNSS, "ipv6"); + nd_ra_opt_add(nd_ra_opts, "dnssl", ND_OPT_DNSSL, "str"); + nd_ra_opt_add(nd_ra_opts, "route_info", ND_OPT_ROUTE_INFO_TYPE, "str"); } #define EMPTY_LB_VIP 1 diff --git a/northd/northd.c b/northd/northd.c index fc7a64f99..861812259 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -10855,6 +10855,22 @@ build_ND_RA_flows_for_lrouter_port( ds_put_format(actions, ", router_preference = \"%s\"", prf); } + const char *ra_rdnss = smap_get(&op->nbrp->ipv6_ra_configs, "rdnss"); + if (ra_rdnss) { + ds_put_format(actions, ", rdnss = %s", ra_rdnss); + } + + const char *ra_dnssl = smap_get(&op->nbrp->ipv6_ra_configs, "dnssl"); + if (ra_dnssl) { + ds_put_format(actions, ", dnssl = \"%s\"", ra_dnssl); + } + + const char *route_info = smap_get(&op->nbrp->ipv6_ra_configs, + "route_info"); + if (route_info) { + ds_put_format(actions, ", route_info = \"%s\"", route_info); + } + bool add_rs_response_flow = false; for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { diff --git a/tests/ovn.at b/tests/ovn.at index 957eb7850..33685109f 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -11902,9 +11902,10 @@ options:rxq_pcap=${pcap_file}-rx.pcap OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) # This shell function sends a Router Solicitation packet. -# test_ipv6_ra INPORT SRC_MAC SRC_LLA ADDR_MODE MTU RA_PREFIX_OPT +# test_ipv6_ra INPORT SRC_MAC SRC_LLA ADDR_MODE MTU RA_PREFIX_OPT RDNSS DNSSL ROUTE_INFO test_ipv6_ra() { local inport=$1 src_mac=$2 src_lla=$3 addr_mode=$4 mtu=$5 prefix_opt=$6 + local rdnss=$7 dnssl=$8 route_info=$9 local request=333300000002${src_mac}86dd6000000000103aff${src_lla}ff02000000000000000000000000000285000efc000000000101${src_mac} local len=24 @@ -11914,6 +11915,18 @@ test_ipv6_ra() { mtu_opt=05010000${mtu} fi + if test ${#rdnss} != 0; then + len=`expr $len + ${#rdnss} / 2` + fi + + if test ${#dnssl} != 0; then + len=`expr $len + ${#dnssl} / 2` + fi + + if test ${#route_info} != 0; then + len=`expr $len + ${#route_info} / 2` + fi + if test ${#prefix_opt} != 0; then prefix_opt=${prefix_opt}fdad1234567800000000000000000000 len=`expr $len + ${#prefix_opt} / 2` @@ -11922,7 +11935,7 @@ test_ipv6_ra() { len=$(printf "%x" $len) local lrp_mac=fa163e000001 local lrp_lla=fe80000000000000f8163efffe000001 - local reply=${src_mac}${lrp_mac}86dd6000000000${len}3aff${lrp_lla}${src_lla}8600XXXXff${addr_mode}ffff00000000000000000101${lrp_mac}${mtu_opt}${prefix_opt} + local reply=${src_mac}${lrp_mac}86dd6000000000${len}3aff${lrp_lla}${src_lla}8600XXXXff${addr_mode}ffff00000000000000000101${lrp_mac}${mtu_opt}${rdnss}${dnssl}${route_info}${prefix_opt} echo $reply >> $inport.expected as hv1 ovs-appctl netdev-dummy/receive hv1-vif${inport} $request @@ -11960,6 +11973,9 @@ reset_pcap_file hv1-vif3 hv1/vif3 ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:mtu=1500 ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:send_periodic="false" ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="LOW" +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:rdnss=1000::11 +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:dnssl=aa.bb.cc +ovn-nbctl --wait=hv set Logical_Router_port lrp0 ipv6_ra_configs:route_info=HIGH-1001::11/48,LOW-1002::11/96 # Make sure that ovn-controller has installed the corresponding OF Flow. OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) @@ -11970,8 +11986,11 @@ default_prefix_option_config=030440c0ffffffffffffffff00000000 src_mac=fa163e000003 src_lla=fe80000000000000f8163efffe000003 mtu=000005dc +rdnss=19030000ffffffff10000000000000000000000000000011 +dnssl=1f030000ffffffff02616102626202636300000000000000 +route_info=18023008ffffffff100100000000000018036018ffffffff10020000000000000000000000000000 -test_ipv6_ra 2 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config +test_ipv6_ra 2 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config $rdnss $dnssl $route_info # NXT_RESUME should be 2. OVS_WAIT_UNTIL([test 2 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -11993,6 +12012,8 @@ reset_pcap_file hv1-vif3 hv1/vif3 # Set the address mode to dhcpv6_stateful, router_preference to HIGH ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:address_mode=dhcpv6_stateful ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="HIGH" +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs rdnss +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs route_info # Make sure that ovn-controller has installed the corresponding OF Flow. OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`]) @@ -12003,7 +12024,7 @@ src_mac=fa163e000004 src_lla=fe80000000000000f8163efffe000004 mtu=000005dc -test_ipv6_ra 3 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config +test_ipv6_ra 3 $src_mac $src_lla $addr_mode $mtu $default_prefix_option_config "" $dnssl # NXT_RESUME should be 3. OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`]) @@ -12025,6 +12046,7 @@ reset_pcap_file hv1-vif3 hv1/vif3 # Set the address mode to dhcpv6_stateless, reset router preference to default ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:address_mode=dhcpv6_stateless ovn-nbctl --wait=hv set Logical_Router_Port lrp0 ipv6_ra_configs:router_preference="MEDIUM" +ovn-nbctl --wait=hv remove Logical_Router_Port lrp0 ipv6_ra_configs dnssl # Make sure that ovn-controller has installed the corresponding OF Flow. OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | grep -c "ipv6_dst=ff02::2,nw_ttl=255,icmp_type=133,icmp_code=0"`])
Send non-periodic Router Advertisement with rdnss, dnssl and route_info options if configured by the user. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1851788 Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- controller/pinctrl.c | 37 ++--------- include/ovn/actions.h | 1 + lib/actions.c | 144 ++++++++++++++++++++++++++++++++++++++++++ lib/ovn-l7.h | 3 + northd/northd.c | 16 +++++ tests/ovn.at | 30 +++++++-- 6 files changed, 197 insertions(+), 34 deletions(-)