On Mon, Oct 19, 2026 at 10:52 PM Mark Michelson via dev < [email protected]> wrote:
> In the current code, en-lflow checks the valid nexthops on a routing > policy to ensure they all have the same configured address family just > before writing the policy logical flows. > > This commit performs the check in en-route-policies instead, so that by > the time logical flows are being written, it is guaranteed that all > valid nexthops are of the same address family. > > Signed-off-by: Mark Michelson <[email protected]> > --- > v1 -> v2: > * The address matching check is now tolerant of empty strings. Empty > nexthops strings are ignored when testing address family. > * Changed the type of a loop iteration counter from uint16_t to size_t. > --- > northd/en-route-policies.c | 31 ++++++++++++++++++++++++++++++- > northd/northd.c | 20 -------------------- > 2 files changed, 30 insertions(+), 21 deletions(-) > > diff --git a/northd/en-route-policies.c b/northd/en-route-policies.c > index 2110b8b60..0cac2fddc 100644 > --- a/northd/en-route-policies.c > +++ b/northd/en-route-policies.c > @@ -238,6 +238,36 @@ build_route_policies(struct ovn_datapath *od, > continue; > } > > + /* Check that all the nexthops belong to the same addr > family. */ > + bool is_ipv4 = true; > + bool ips_match = true; > + for (size_t j = 0; j < rule->n_nexthops; j++) { > + const char *nexthop = rule->nexthops[j]; > + if (!nexthop || !nexthop[0]) { > + continue; > + } > + > + bool nexthop_is_ipv4 = !!strchr(nexthop, '.'); > + > + if (j == 0) { > + is_ipv4 = nexthop_is_ipv4; > + } > + > + if (nexthop_is_ipv4 != is_ipv4) { > + static struct vlog_rate_limit rl = > + VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "nexthop [%s] of the router policy > with " > + "the match [%s] does not belong to the > same " > + "address family as other next hops", > + rule->nexthops[j], rule->match); > + ips_match = false; > + break; > + } > + } > + if (!ips_match) { > + continue; > + } > + > valid_nexthops = xcalloc(rule->n_nexthops, sizeof > *valid_nexthops); > for (size_t j = 0; j < rule->n_nexthops; j++) { > char *nexthop = rule->nexthops[j]; > @@ -246,7 +276,6 @@ build_route_policies(struct ovn_datapath *od, > } > > struct ovn_port *out_port = NULL; > - bool is_ipv4 = strchr(nexthop, '.') ? true : false; > > if (!find_policy_outport(od, rule, nexthop, is_ipv4, NULL, > &out_port)) { > diff --git a/northd/northd.c b/northd/northd.c > index 734d39a3a..3335db520 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -12500,29 +12500,9 @@ build_ecmp_routing_policy_flows(struct > lflow_table *lflows, > uint16_t ecmp_group_id, > struct lflow_ref *lflow_ref) > { > - bool nexthops_is_ipv4 = true; > const struct nbrec_logical_router_policy *rule = rp->rule; > ovs_assert(rp->n_valid_nexthops > 1); > > - /* Check that all the nexthops belong to the same addr family before > - * adding logical flows. */ > - for (uint16_t i = 0; i < rp->n_valid_nexthops; i++) { > - bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > - > - if (i == 0) { > - nexthops_is_ipv4 = is_ipv4; > - } > - > - if (is_ipv4 != nexthops_is_ipv4) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "nexthop [%s] of the router policy with " > - "the match [%s] do not belong to the same > address " > - "family as other next hops", > - rp->valid_nexthops[i], rule->match); > - return; > - } > - } > - > struct ds match = DS_EMPTY_INITIALIZER; > struct ds actions = DS_EMPTY_INITIALIZER; > > -- > 2.55.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
