On Tue, Jan 19, 2021 at 9:43 PM Mark Michelson <[email protected]> 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 <[email protected]> > > 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 <[email protected]> > 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 > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
