[ovs-dev] [PATCH ovn] ovn-nbctl: add ecmp/ecmp-symmetric-reply to lr-route-list command

Numan Siddique numans at ovn.org
Mon Jan 25 11:53:55 UTC 2021


On Tue, Jan 19, 2021 at 9:43 PM Mark Michelson <mmichels at redhat.com> wrote:

> Excellent. I tried to make it angry but it remained calm no matter what
> I threw at it. The only "flaw" is that if you add just one route with
> "--ecmp" then it will not display "ecmp" when you list the route.
> ovn-northd doesn't do any special ECMP handling with this single route,
> either, so the output is accurate in this case, even if possibly
> unexpected.
>
> Acked-by: Mark Michelson <mmichels at redhat.com>
>
> On 1/18/21 11:22 AM, Lorenzo Bianconi wrote:
> > Explicitly add ecmp/ecmp-symmetric-reply info to ovn-nbctl
> > lr-route-list command
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1915958
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
>

Hi Lorenzo,

This patch needs a rebase.

Thanks
Numan


> > ---
> > This patch is based on "ovn-nbctl: add bfd report to lr-route-list
> command"
> > http://patchwork.ozlabs.org/project/ovn/list/?series=224464
> > ---
> >   tests/ovn-nbctl.at    | 36 ++++++++++++++---------
> >   utilities/ovn-nbctl.c | 67 +++++++++++++++++++++++++++++++++++--------
> >   2 files changed, 78 insertions(+), 25 deletions(-)
> >
> > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> > index 2827b223c..e4a2a9695 100644
> > --- a/tests/ovn-nbctl.at
> > +++ b/tests/ovn-nbctl.at
> > @@ -1544,27 +1544,27 @@ AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0
> 10.0.0.0/24 11.0.0.3])
> >   AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 10.0.0.0/24 11.0.0.3 lp0])
> >   AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
> >   IPv4 Routes
> > -              10.0.0.0/24                  11.0.0.1 dst-ip
> > -              10.0.0.0/24                  11.0.0.2 dst-ip
> > -              10.0.0.0/24                  11.0.0.2 dst-ip
> > -              10.0.0.0/24                  11.0.0.3 dst-ip
> > -              10.0.0.0/24                  11.0.0.3 dst-ip lp0
> > +              10.0.0.0/24                  11.0.0.1 dst-ip ecmp
> > +              10.0.0.0/24                  11.0.0.2 dst-ip ecmp
> > +              10.0.0.0/24                  11.0.0.2 dst-ip ecmp
> > +              10.0.0.0/24                  11.0.0.3 dst-ip ecmp
> > +              10.0.0.0/24                  11.0.0.3 dst-ip lp0 ecmp
> >   ])
> >
> >   dnl Delete ecmp routes
> >   AT_CHECK([ovn-nbctl lr-route-del lr0 10.0.0.0/24 11.0.0.1])
> >   AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
> >   IPv4 Routes
> > -              10.0.0.0/24                  11.0.0.2 dst-ip
> > -              10.0.0.0/24                  11.0.0.2 dst-ip
> > -              10.0.0.0/24                  11.0.0.3 dst-ip
> > -              10.0.0.0/24                  11.0.0.3 dst-ip lp0
> > +              10.0.0.0/24                  11.0.0.2 dst-ip ecmp
> > +              10.0.0.0/24                  11.0.0.2 dst-ip ecmp
> > +              10.0.0.0/24                  11.0.0.3 dst-ip ecmp
> > +              10.0.0.0/24                  11.0.0.3 dst-ip lp0 ecmp
> >   ])
> >   AT_CHECK([ovn-nbctl lr-route-del lr0 10.0.0.0/24 11.0.0.2])
> >   AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
> >   IPv4 Routes
> > -              10.0.0.0/24                  11.0.0.3 dst-ip
> > -              10.0.0.0/24                  11.0.0.3 dst-ip lp0
> > +              10.0.0.0/24                  11.0.0.3 dst-ip ecmp
> > +              10.0.0.0/24                  11.0.0.3 dst-ip lp0 ecmp
> >   ])
> >   AT_CHECK([ovn-nbctl lr-route-del lr0 10.0.0.0/24 11.0.0.3 lp0])
> >   AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
> > @@ -1605,7 +1605,12 @@ AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.1.1/24
> 11.0.1.1 lp0])
> >   AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.1/24 11.0.0.1])
> >   AT_CHECK([ovn-nbctl lr-route-add lr0 0:0:0:0:0:0:0:0/0
> 2001:0db8:0:f101::1])
> >   AT_CHECK([ovn-nbctl lr-route-add lr0 2001:0db8:0::/64
> 2001:0db8:0:f102::1 lp0])
> > -AT_CHECK([ovn-nbctl lr-route-add lr0 2001:0db8:1::/64
> 2001:0db8:0:f103::1])
> > +AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 2001:0db8:1::/64
> 2001:0db8:0:f103::1])
> > +AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 2001:0db8:1::/64
> 2001:0db8:0:f103::2])
> > +AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 2001:0db8:1::/64
> 2001:0db8:0:f103::3])
> > +AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 2001:0db8:1::/64
> 2001:0db8:0:f103::4])
> > +AT_CHECK([ovn-nbctl lr-route-add lr0 2002:0db8:1::/64
> 2001:0db8:0:f103::5])
> > +AT_CHECK([ovn-nbctl --ecmp-symmetric-reply lr-route-add lr0
> 2003:0db8:1::/64 2001:0db8:0:f103::6])
> >
> >   AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
> >   IPv4 Routes
> > @@ -1615,7 +1620,12 @@ IPv4 Routes
> >
> >   IPv6 Routes
> >               2001:db8::/64        2001:db8:0:f102::1 dst-ip lp0
> > -          2001:db8:1::/64        2001:db8:0:f103::1 dst-ip
> > +          2001:db8:1::/64        2001:db8:0:f103::1 dst-ip ecmp
> > +          2001:db8:1::/64        2001:db8:0:f103::2 dst-ip ecmp
> > +          2001:db8:1::/64        2001:db8:0:f103::3 dst-ip ecmp
> > +          2001:db8:1::/64        2001:db8:0:f103::4 dst-ip ecmp
> > +          2002:db8:1::/64        2001:db8:0:f103::5 dst-ip
> > +          2003:db8:1::/64        2001:db8:0:f103::6 dst-ip
> ecmp-symmetric-reply
> >                        ::/0        2001:db8:0:f101::1 dst-ip
> >   ])
> >
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index abd138313..f43967477 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -5443,17 +5443,27 @@ struct ipv4_route {
> >       const struct nbrec_logical_router_static_route *route;
> >   };
> >
> > +static int
> > +__ipv4_route_cmp(const struct ipv4_route *r1, const struct ipv4_route
> *r2)
> > +{
> > +    if (r1->priority != r2->priority) {
> > +        return r1->priority > r2->priority ? -1 : 1;
> > +    }
> > +    if (r1->addr != r2->addr) {
> > +        return ntohl(r1->addr) < ntohl(r2->addr) ? -1 : 1;
> > +    }
> > +    return 0;
> > +}
> > +
> >   static int
> >   ipv4_route_cmp(const void *route1_, const void *route2_)
> >   {
> >       const struct ipv4_route *route1p = route1_;
> >       const struct ipv4_route *route2p = route2_;
> >
> > -    if (route1p->priority != route2p->priority) {
> > -        return route1p->priority > route2p->priority ? -1 : 1;
> > -    }
> > -    if (route1p->addr != route2p->addr) {
> > -        return ntohl(route1p->addr) < ntohl(route2p->addr) ? -1 : 1;
> > +    int ret = __ipv4_route_cmp(route1p, route2p);
> > +    if (ret) {
> > +        return ret;
> >       }
> >       return route_cmp_details(route1p->route, route2p->route);
> >   }
> > @@ -5464,16 +5474,22 @@ struct ipv6_route {
> >       const struct nbrec_logical_router_static_route *route;
> >   };
> >
> > +static int
> > +__ipv6_route_cmp(const struct ipv6_route *r1, const struct ipv6_route
> *r2)
> > +{
> > +    if (r1->priority != r2->priority) {
> > +        return r1->priority > r2->priority ? -1 : 1;
> > +    }
> > +    return memcmp(&r1->addr, &r2->addr, sizeof(r1->addr));
> > +}
> > +
> >   static int
> >   ipv6_route_cmp(const void *route1_, const void *route2_)
> >   {
> >       const struct ipv6_route *route1p = route1_;
> >       const struct ipv6_route *route2p = route2_;
> >
> > -    if (route1p->priority != route2p->priority) {
> > -        return route1p->priority > route2p->priority ? -1 : 1;
> > -    }
> > -    int ret = memcmp(&route1p->addr, &route2p->addr,
> sizeof(route1p->addr));
> > +    int ret = __ipv6_route_cmp(route1p, route2p);
> >       if (ret) {
> >           return ret;
> >       }
> > @@ -5481,7 +5497,8 @@ ipv6_route_cmp(const void *route1_, const void
> *route2_)
> >   }
> >
> >   static void
> > -print_route(const struct nbrec_logical_router_static_route *route,
> struct ds *s)
> > +print_route(const struct nbrec_logical_router_static_route *route,
> > +            struct ds *s, bool ecmp)
> >   {
> >
> >       char *prefix = normalize_prefix_str(route->ip_prefix);
> > @@ -5504,6 +5521,14 @@ print_route(const struct
> nbrec_logical_router_static_route *route, struct ds *s)
> >           ds_put_format(s, " (learned)");
> >       }
> >
> > +    if (ecmp) {
> > +        ds_put_format(s, " %s", "ecmp");
> > +    }
> > +
> > +    if (smap_get_bool(&route->options, "ecmp_symmetric_reply", false)) {
> > +        ds_put_format(s, " %s", "ecmp-symmetric-reply");
> > +    }
> > +
> >       if (route->bfd) {
> >           ds_put_format(s, " %s", "bfd");
> >       }
> > @@ -5573,7 +5598,16 @@ nbctl_lr_route_list(struct ctl_context *ctx)
> >           ds_put_cstr(&ctx->output, "IPv4 Routes\n");
> >       }
> >       for (int i = 0; i < n_ipv4_routes; i++) {
> > -        print_route(ipv4_routes[i].route, &ctx->output);
> > +        bool ecmp = false;
> > +        if (i < n_ipv4_routes - 1 &&
> > +            !__ipv4_route_cmp(&ipv4_routes[i], &ipv4_routes[i + 1])) {
> > +            ecmp = true;
> > +        } else if (i > 0 &&
> > +                   !__ipv4_route_cmp(&ipv4_routes[i],
> > +                                     &ipv4_routes[i - 1])) {
> > +            ecmp = true;
> > +        }
> > +        print_route(ipv4_routes[i].route, &ctx->output, ecmp);
> >       }
> >
> >       if (n_ipv6_routes) {
> > @@ -5581,7 +5615,16 @@ nbctl_lr_route_list(struct ctl_context *ctx)
> >                         n_ipv4_routes ?  "\n" : "");
> >       }
> >       for (int i = 0; i < n_ipv6_routes; i++) {
> > -        print_route(ipv6_routes[i].route, &ctx->output);
> > +        bool ecmp = false;
> > +        if (i < n_ipv6_routes - 1 &&
> > +            !__ipv6_route_cmp(&ipv6_routes[i], &ipv6_routes[i + 1])) {
> > +            ecmp = true;
> > +        } else if (i > 0 &&
> > +                   !__ipv6_route_cmp(&ipv6_routes[i],
> > +                                     &ipv6_routes[i - 1])) {
> > +            ecmp = true;
> > +        }
> > +        print_route(ipv6_routes[i].route, &ctx->output, ecmp);
> >       }
> >
> >       free(ipv4_routes);
> >
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list