[ovs-dev] [PATCH ovn] Honour router_preference for solicited RA

Numan Siddique numans at ovn.org
Tue Jun 9 07:44:50 UTC 2020


On Sat, Jun 6, 2020 at 4:54 PM Gabriele Cerami <gcerami at redhat.com> wrote:

> Replies to router solicitation follow a different flow than periodic RA.
> This flow currently does not honour the router_preference configuration.
>
> This patch modifies the flow to honour the flag, and send
> router preference indications in the reply RA following RFC4191
> specifications
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1804576
> Signed-off-by: Gabriele Cerami <gcerami at redhat.com>
>

Hi Gabriele,

Thank for the fix.

The patch LGTM. However the compilation fails with the below error when
configured with "--enable-sparse --enable-Werror"

******
de -I /home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/include -I
/home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/_gcc/include -I
/home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/lib -I
/home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/_gcc/lib -I
/home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs -I
/home/nusiddiq/workspace_cpp/openvswitch/ovn_split/ovs/_gcc -I ../lib -I
./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
-Wswitch-bool -Wlogical-not-parentheses -Wsizeof-array-argument
-Wbool-compare -Wshift-negative-value -Wduplicated-cond -Wshadow
-Wmultistatement-macros -Wcast-align=strict -Werror -Werror -g -O2 -MT
lib/actions.lo -MD -MP -MF lib/.deps/actions.Tpo -c ../lib/actions.c -o
lib/actions.o
../lib/actions.c:2643:27: error: symbol 'ra' shadows an earlier one
../lib/actions.c:2592:23: originally declared here
make[1]: *** [Makefile:2016: lib/actions.lo] Error 1

*****

Thanks
Numan


> ---
>  lib/actions.c       | 30 +++++++++++++++++++++++++++---
>  lib/ovn-l7.h        |  5 +++++
>  northd/ovn-northd.c |  6 ++++++
>  tests/ovn.at        | 16 +++++++++++-----
>  4 files changed, 49 insertions(+), 8 deletions(-)
>
> diff --git a/lib/actions.c b/lib/actions.c
> index c50615177..38a3c0ef0 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -2535,6 +2535,12 @@ parse_put_nd_ra_opts(struct action_context *ctx,
> const struct expr_field *dst,
>              }
>              break;
>
> +        case ND_RA_FLAG_PRF:
> +            ok = (c->string && (!strcmp(c->string, "MEDIUM") ||
> +                                !strcmp(c->string, "HIGH") ||
> +                                !strcmp(c->string, "LOW")));
> +            break;
> +
>          case ND_OPT_SOURCE_LINKADDR:
>              ok = c->format == LEX_F_ETHERNET;
>              slla_present = true;
> @@ -2583,15 +2589,27 @@ encode_put_nd_ra_option(const struct
> ovnact_gen_option *o,
>                          struct ofpbuf *ofpacts, ptrdiff_t ra_offset)
>  {
>      const union expr_constant *c = o->value.values;
> +    struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
>
>      switch (o->option->code) {
>      case ND_RA_FLAG_ADDR_MODE:
>      {
> -        struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
>          if (!strcmp(c->string, "dhcpv6_stateful")) {
> -            ra->mo_flags = IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
> +            ra->mo_flags |= IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
>          } else if (!strcmp(c->string, "dhcpv6_stateless")) {
> -            ra->mo_flags = IPV6_ND_RA_FLAG_OTHER_ADDR_CONFIG;
> +            ra->mo_flags |= IPV6_ND_RA_FLAG_OTHER_ADDR_CONFIG;
> +        }
> +        break;
> +    }
> +
> +    case ND_RA_FLAG_PRF:
> +    {
> +        if (!strcmp(c->string, "LOW")) {
> +            ra->mo_flags |= IPV6_ND_RA_OPT_PRF_LOW;
> +        } else if (!strcmp(c->string, "HIGH")) {
> +            ra->mo_flags |= IPV6_ND_RA_OPT_PRF_HIGH;
> +        } else {
> +            ra->mo_flags |= IPV6_ND_RA_OPT_PRF_NORMAL;
>          }
>          break;
>      }
> @@ -2640,6 +2658,12 @@ encode_put_nd_ra_option(const struct
> ovnact_gen_option *o,
>          break;
>      }
>      }
> +
> +    /* RFC4191 section 2.2 */
> +    if (ntohs(ra->router_lifetime) == 0x0) {
> +        ra->mo_flags &= IPV6_ND_RA_OPT_PRF_RESET_MASK;
> +    }
> +
>  }
>
>  static void
> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> index cc63d822d..d5c6feaeb 100644
> --- a/lib/ovn-l7.h
> +++ b/lib/ovn-l7.h
> @@ -309,6 +309,9 @@ nd_ra_opts_destroy(struct hmap *nd_ra_opts)
>
>
>  #define ND_RA_FLAG_ADDR_MODE    0
> +/* all small numbers seems to be all already taken but nothing guarantees
> this
> + * code will not be assigned by IANA to another option */
> +#define ND_RA_FLAG_PRF          255
>
>
>  /* Default values of various IPv6 Neighbor Discovery protocol options and
> @@ -330,11 +333,13 @@ nd_ra_opts_destroy(struct hmap *nd_ra_opts)
>  #define IPV6_ND_RA_OPT_PRF_NORMAL                   0x00
>  #define IPV6_ND_RA_OPT_PRF_HIGH                     0x08
>  #define IPV6_ND_RA_OPT_PRF_LOW                      0x18
> +#define IPV6_ND_RA_OPT_PRF_RESET_MASK               0xe7
>
>  static inline void
>  nd_ra_opts_init(struct hmap *nd_ra_opts)
>  {
>      nd_ra_opt_add(nd_ra_opts, "addr_mode", ND_RA_FLAG_ADDR_MODE, "str");
> +    nd_ra_opt_add(nd_ra_opts, "router_preference", ND_RA_FLAG_PRF, "str");
>      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");
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index eb78f317e..e7815bfc3 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -9452,6 +9452,12 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>              ds_put_format(&actions, ", mtu = %u", mtu);
>          }
>
> +        const char *prf = smap_get_def(
> +            &op->nbrp->ipv6_ra_configs, "router_preference", "MEDIUM");
> +        if (strcmp(prf, "MEDIUM")) {
> +            ds_put_format(&actions, ", router_preference = \"%s\"", prf);
> +        }
> +
>          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 15b40ca1e..029027586 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -10619,13 +10619,16 @@ reset_pcap_file hv1-vif1 hv1/vif1
>  reset_pcap_file hv1-vif2 hv1/vif2
>  reset_pcap_file hv1-vif3 hv1/vif3
>
> -# Set the MTU to 1500
> +# Set the MTU to 1500, send_periodic to false, preference to LOW
>  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"
>
>  # 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"`])
>
> -addr_mode=00
> +# addr_mode byte also includes router preference information
> +addr_mode=18
>  default_prefix_option_config=030440c0ffffffffffffffff00000000
>  src_mac=fa163e000003
>  src_lla=fe80000000000000f8163efffe000003
> @@ -10650,12 +10653,14 @@ reset_pcap_file hv1-vif1 hv1/vif1
>  reset_pcap_file hv1-vif2 hv1/vif2
>  reset_pcap_file hv1-vif3 hv1/vif3
>
> -# Set the address mode to dhcpv6_stateful
> +# 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"
>  # 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"`])
>
> -addr_mode=80
> +# addr_mode byte also includes router preference information
> +addr_mode=88
>  default_prefix_option_config=03044080ffffffffffffffff00000000
>  src_mac=fa163e000004
>  src_lla=fe80000000000000f8163efffe000004
> @@ -10680,8 +10685,9 @@ reset_pcap_file hv1-vif1 hv1/vif1
>  reset_pcap_file hv1-vif2 hv1/vif2
>  reset_pcap_file hv1-vif3 hv1/vif3
>
> -# Set the address mode to dhcpv6_stateless
> +# 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"
>  # 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"`])
>
> --
> 2.11.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list