When building our route policies, we evaluate whether the configured nexthops are valid based on whether we can find a legitimate output port for the policy, and whether the BFD state for that nexthop is up. By the time we are writing logical flows, the northbound database's configured nexthops are irrelevant since we only want to use the routing policy's valid_nexthops for evaluation.
When determining whether to evaluate a policy as ECMP or not, we should use the number of valid nexthops as the determiner, not the northbound rule's number of nexthops. It's possible that the northbound rule intends for the policy to be an ECMP reroute to a number of nexthops. However, if only one configured nexthop is valid, then we can just program the logical flow as if this were a non-ECMP reroute. We can skip the need for the OpenFlow select since there is only one valid nexthop. Signed-off-by: Mark Michelson <[email protected]> --- northd/northd.c | 30 ++++++++++++------------------ tests/system-ovn.at | 5 ++++- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index c23b4ffc1..91e2fa601 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -12256,7 +12256,7 @@ build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, struct ds actions = DS_EMPTY_INITIALIZER; if (!strcmp(rule->action, "reroute")) { - ovs_assert(rule->n_nexthops <= 1); + ovs_assert(rp->n_valid_nexthops <= 1); if (!rp->n_valid_nexthops) { return; @@ -12330,7 +12330,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, { bool nexthops_is_ipv4 = true; const struct nbrec_logical_router_policy *rule = rp->rule; - ovs_assert(rule->n_nexthops > 1); + ovs_assert(rp->n_valid_nexthops > 1); /* Check that all the nexthops belong to the same addr family before * adding logical flows. */ @@ -12396,24 +12396,18 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, } ds_clear(&actions); - if (rp->n_valid_nexthops > 1) { - ds_put_format(&actions, "%s = %"PRIu16 - "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id, - REG_ECMP_MEMBER_ID); - - for (size_t i = 0; i < rp->n_valid_nexthops; i++) { - if (i > 0) { - ds_put_cstr(&actions, ", "); - } + ds_put_format(&actions, "%s = %"PRIu16 + "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id, + REG_ECMP_MEMBER_ID); - ds_put_format(&actions, "%"PRIuSIZE, i + 1); + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { + if (i > 0) { + ds_put_cstr(&actions, ", "); } - ds_put_cstr(&actions, ");"); - } else { - ds_put_format(&actions, "%s = %"PRIu16 - "; %s = 1; next;", REG_ECMP_GROUP_ID, - ecmp_group_id, REG_ECMP_MEMBER_ID); + + ds_put_format(&actions, "%"PRIuSIZE, i + 1); } + ds_put_cstr(&actions, ");"); ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY, rule->priority, rule->match, ds_cstr(&actions), lflow_ref, WITH_HINT(&rule->header_)); cleanup: @@ -15765,7 +15759,7 @@ build_ingress_policy_flows_for_lrouter( route_policies) { const struct nbrec_logical_router_policy *rule = rp->rule; bool is_ecmp_reroute = - (!strcmp(rule->action, "reroute") && rule->n_nexthops > 1); + (!strcmp(rule->action, "reroute") && rp->n_valid_nexthops > 1); if (is_ecmp_reroute) { build_ecmp_routing_policy_flows(lflows, od, rp, ecmp_group_id, diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 13e62bf9b..07991d495 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -7349,7 +7349,10 @@ OVS_WAIT_UNTIL([ip netns exec server bfdd-control status | grep -qi state=Down]) check ovn-nbctl --bfd lr-policy-add R1 100 "ip4.src == 200.0.0.0/8" reroute 172.16.1.50,172.16.1.60 wait_column "up" nb:bfd status dst_ip=172.16.1.50 -OVS_WAIT_UNTIL([ovn-sbctl dump-flows R1 | grep lr_in_policy_ecmp | grep -q 172.16.1.50]) + +# Even though the policy is an ECMP policy, only one route is actually valid. +# Therefore, the policy is installed in lr_in_policy instead of lr_in_policy_ecmp. +OVS_WAIT_UNTIL([ovn-sbctl dump-flows R1 | grep 'lr_in_policy[[^_]]' | grep -q 172.16.1.50]) check ovn-nbctl lr-policy-del R1 wait_column "admin_down" nb:bfd status dst_ip=172.16.1.50 -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
