On Sat, Mar 21, 2026 at 5:12 AM Han Zhou <[email protected]> wrote: > add_ecmp_symmetric_reply_flows() chose REG_SRC_IPV4 vs REG_SRC_IPV6 from > the route prefix, which is wrong. It should be based on the nexthop > address family instead. For IPv4 prefix + IPv6 nexthop this assigns > reg5 with a 128-bit constant. This patch fixes it by using the existing > check against nexthop. > > Fixes: 4fdca656857d ("Add ECMP symmetric replies.") > Assisted-by: Cursor, with model: claude-4.6-opus-high > Signed-off-by: Han Zhou <[email protected]> > --- > northd/northd.c | 8 +++++--- > tests/ovn-northd.at | 7 +++++++ > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 4b87c2305c20..ac236141722d 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -12566,6 +12566,7 @@ static void > add_ecmp_symmetric_reply_flows(struct lflow_table *lflows, > const struct ovn_datapath *od, > const char *port_ip, > + bool is_ipv4_nexthop, > const struct ovn_port *out_port, > const struct parsed_route *route, > struct ds *route_match, > @@ -12620,8 +12621,7 @@ add_ecmp_symmetric_reply_flows(struct lflow_table > *lflows, > ds_put_format(&actions, "ip.ttl--; flags.loopback = 1; " > "eth.src = %s; %s = %s; outport = %s; next;", > out_port->lrp_networks.ea_s, > - IN6_IS_ADDR_V4MAPPED(&route->prefix) ? > - REG_SRC_IPV4 : REG_SRC_IPV6, > + is_ipv4_nexthop ? REG_SRC_IPV4 : REG_SRC_IPV6, > port_ip, out_port->json_key); > ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10300, > ds_cstr(&match), > ds_cstr(&actions), lflow_ref, > WITH_HINT(route->source_hint)); > @@ -12744,6 +12744,8 @@ build_ecmp_route_flow(struct lflow_table *lflows, > continue; > } > > + bool is_ipv4_nexthop = IN6_IS_ADDR_V4MAPPED(route->nexthop); > + > /* Symmetric ECMP reply is only usable on gateway routers. > * It is NOT usable on distributed routers with a gateway port. > */ > @@ -12751,12 +12753,12 @@ build_ecmp_route_flow(struct lflow_table *lflows, > route->ecmp_symmetric_reply && sset_add(&visited_ports, > > route->out_port->key)) { > add_ecmp_symmetric_reply_flows(lflows, od, route->lrp_addr_s, > + is_ipv4_nexthop, > route->out_port, > route, &route_match, > lflow_ref); > } > > - bool is_ipv4_nexthop = IN6_IS_ADDR_V4MAPPED(route->nexthop); > ds_put_format(&actions, "%s = ", > is_ipv4_nexthop ? REG_NEXT_HOP_IPV4 : > REG_NEXT_HOP_IPV6); > ipv6_format_mapped(route->nexthop, &actions); > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 587ce94148e4..2ccf7a196999 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -7429,6 +7429,13 @@ AT_CHECK([grep -e "lr_in_arp_request" lr0flows | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_arp_request ), priority=200 , match=(eth.dst == > 00:00:00:00:00:00 && reg9[[9]] == 0 && xxreg0 == 2001:db8::20), > action=(nd_ns { eth.dst = 33:33:ff:00:00:20; ip6.dst = ff02::1:ff00:20; > nd.target = 2001:db8::20; output; }; next;) > ]) > > +# ECMP symmetric reply with IPv4 prefix + IPv6 nexthop. > +# It should select the first IPv6 and assign to REG_SRC_IPV6 (xxreg1) > +check ovn-nbctl set nb_global . options:ecmp_nexthop_monitor_enable="true" > +check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 > 12.0.0.0/24 2001:db8::10 > +ovn-sbctl dump-flows lr0 > lr0flows > +AT_CHECK([grep "ct_mark.ecmp_reply_port" lr0flows | grep "12.0.0.0/24" | > grep -q "xxreg1 = 2001:db8::1"], [0], [ignore]) > + > OVN_CLEANUP_NORTHD > AT_CLEANUP > ]) > -- > 2.38.1 > > _______________________________________________ > 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
