In the current ovn-nbctl lr-route-add implementation is possible to add multiple duplicated routes adding --ecmp or --ecmp-symmetric-reply option, e.g:
$ovn-nbctl --ecmp-symmetric-reply lr-route-add lr0 10.244.0.7/32 172.18.0.4 $ovn-nbctl --ecmp-symmetric-reply lr-route-add lr0 10.244.0.7/32 172.18.0.4 $ovn-nbctl --ecmp-symmetric-reply lr-route-add lr0 10.244.0.7/32 172.18.0.4 https://bugzilla.redhat.com/show_bug.cgi?id=1916842 Fix the issue checking nexthop for ECMP routes. Signed-off-by: Lorenzo Bianconi <[email protected]> --- Changes since v1: - rename nbctl_lr_get_route_prefix() in nbctl_lr_get_route() - drop normalize_prefix_str routine in nbctl_lr_route_add() - remove unnecessary strcmp() in nbctl_lr_route_add() --- tests/ovn-nbctl.at | 19 ++++++----- utilities/ovn-nbctl.c | 77 ++++++++++++++++++++++++++----------------- 2 files changed, 58 insertions(+), 38 deletions(-) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index e4a2a9695..6d91aa4c5 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1539,34 +1539,34 @@ AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl dnl Add ecmp routes AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.0/24 11.0.0.1]) AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 10.0.0.0/24 11.0.0.2]) -AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 10.0.0.0/24 11.0.0.2]) 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 --ecmp lr-route-add lr0 10.0.0.0/24 11.0.0.4 lp0]) AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl IPv4 Routes 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 + 10.0.0.0/24 11.0.0.4 dst-ip lp0 ecmp +]) +AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 10.0.0.0/24 11.0.0.2], [1], [], + [ovn-nbctl: duplicate nexthop for the same ECMP route ]) 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 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 + 10.0.0.0/24 11.0.0.4 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 ecmp - 10.0.0.0/24 11.0.0.3 dst-ip lp0 ecmp + 10.0.0.0/24 11.0.0.4 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-del lr0 10.0.0.0/24 11.0.0.4 lp0]) AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl IPv4 Routes 10.0.0.0/24 11.0.0.3 dst-ip @@ -1611,6 +1611,9 @@ 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 --ecmp-symmetric-reply lr-route-add lr0 2003:0db8:1::/64 2001:0db8:0:f103::6], [1], [], + [ovn-nbctl: duplicate nexthop for the same ECMP route +]) AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl IPv4 Routes diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 3f28ba11a..2c77f4ba7 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -3917,6 +3917,47 @@ nbctl_lr_policy_list(struct ctl_context *ctx) } free(policies); } + +static struct nbrec_logical_router_static_route * +nbctl_lr_get_route(const struct nbrec_logical_router *lr, char *prefix, + char *next_hop, bool is_src_route, bool ecmp) +{ + for (int i = 0; i < lr->n_static_routes; i++) { + struct nbrec_logical_router_static_route *route = lr->static_routes[i]; + + /* Compare route policy. */ + char *nb_policy = route->policy; + bool nb_is_src_route = false; + if (nb_policy && !strcmp(nb_policy, "src-ip")) { + nb_is_src_route = true; + } + if (is_src_route != nb_is_src_route) { + continue; + } + + /* Compare route prefix. */ + char *rt_prefix = normalize_prefix_str(route->ip_prefix); + if (!rt_prefix) { + /* Ignore existing prefix we couldn't parse. */ + continue; + } + + if (strcmp(rt_prefix, prefix)) { + free(rt_prefix); + continue; + } + + if (ecmp && strcmp(next_hop, route->nexthop)) { + free(rt_prefix); + continue; + } + + free(rt_prefix); + return route; + } + return NULL; +} + static void nbctl_lr_route_add(struct ctl_context *ctx) @@ -3988,39 +4029,14 @@ nbctl_lr_route_add(struct ctl_context *ctx) "--ecmp-symmetric-reply") != NULL; bool ecmp = shash_find(&ctx->options, "--ecmp") != NULL || ecmp_symmetric_reply; + struct nbrec_logical_router_static_route *route = + nbctl_lr_get_route(lr, prefix, next_hop, is_src_route, ecmp); if (!ecmp) { - for (int i = 0; i < lr->n_static_routes; i++) { - const struct nbrec_logical_router_static_route *route - = lr->static_routes[i]; - char *rt_prefix; - - /* Compare route policy. */ - char *nb_policy = lr->static_routes[i]->policy; - bool nb_is_src_route = false; - if (nb_policy && !strcmp(nb_policy, "src-ip")) { - nb_is_src_route = true; - } - if (is_src_route != nb_is_src_route) { - continue; - } - - /* Compare route prefix. */ - rt_prefix = normalize_prefix_str(lr->static_routes[i]->ip_prefix); - if (!rt_prefix) { - /* Ignore existing prefix we couldn't parse. */ - continue; - } - - if (strcmp(rt_prefix, prefix)) { - free(rt_prefix); - continue; - } - + if (route) { if (!may_exist) { ctl_error(ctx, "duplicate prefix: %s (policy: %s). Use option" " --ecmp to allow this for ECMP routing.", prefix, is_src_route ? "src-ip" : "dst-ip"); - free(rt_prefix); goto cleanup; } @@ -4049,12 +4065,13 @@ nbctl_lr_route_add(struct ctl_context *ctx) } nbrec_logical_router_static_route_set_bfd(route, nb_bt); } - free(rt_prefix); goto cleanup; } + } else if (route) { + ctl_error(ctx, "duplicate nexthop for the same ECMP route"); + goto cleanup; } - struct nbrec_logical_router_static_route *route; route = nbrec_logical_router_static_route_insert(ctx->txn); nbrec_logical_router_static_route_set_ip_prefix(route, prefix); nbrec_logical_router_static_route_set_nexthop(route, next_hop); -- 2.29.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
