On Fri, Mar 20, 2026 at 6:53 AM Lorenzo Bianconi <
[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.
> >
> > Assisted-by: Cursor, with model: claude-4.6-opus-high
> > Signed-off-by: Han Zhou <[email protected]>
>
> This patch does not compile since is_ipv4_nexthop is defined below
> add_ecmp_symmetric_reply_flows():
>
> make all-am
> make[1]: Entering directory '/home/lorenzo/workspace/ovn'
> depbase=`echo northd/northd.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
> gcc -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I
./include -I ./lib -I ./lib -I /home/lorenzo/workspace/ovn/ovs/include -I
/home/lorenzo/workspace/ovn/ovs/include -I
/home/lorenzo/workspace/ovn/ovs/lib -I /home/lorenzo/workspace/ovn/ovs/lib
-I /home/lorenzo/workspace/ovn/ovs -I /home/lorenzo/workspace/ovn/ovs
-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
-Wswitch-bool -Wlogical-not-parentheses -Wsizeof-array-argument
-Wbool-compare -Wshift-negative-value -Wduplicated-cond -Wshadow
-Wmultistatement-macros -Wcast-align=strict -g -O2 -MT northd/northd.o
-MD -MP -MF $depbase.Tpo -c -o northd/northd.o northd/northd.c &&\
> mv -f $depbase.Tpo $depbase.Po
> northd/northd.c: In function ‘build_ecmp_route_flow’:
> northd/northd.c:12754:44: error: ‘is_ipv4_nexthop’ undeclared (first use
in this function)
> 12754 | is_ipv4_nexthop,
> | ^~~~~~~~~~~~~~~
> northd/northd.c:12754:44: note: each undeclared identifier is reported
only once for each function it appears in
> northd/northd.c:12760:14: warning: declaration of ‘is_ipv4_nexthop’
shadows previous non-variable [-Wshadow]
> 12760 | bool is_ipv4_nexthop =
IN6_IS_ADDR_V4MAPPED(route->nexthop);
> | ^~~~~~~~~~~~~~~
> make[1]: *** [Makefile:2793: northd/northd.o] Error 1
> make[1]: Leaving directory '/home/lorenzo/workspace/ovn'
> make: *** [Makefile:1802: all] Error 2
>
Ah, I missed a rebase with the latest main which contains a change
("northd: Drop traffic for ECMP group with "discard" route.") that moved
the definition of the is_ipv4_nexthop. Fixing in v2.
Thanks,
Han
> Regards,
> Lorenzo
>
> > ---
> > northd/northd.c | 5 +++--
> > tests/ovn-northd.at | 7 +++++++
> > 2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 297892a1630e..2024bf9c6fed 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -12563,6 +12563,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,
> > @@ -12617,8 +12618,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));
> > @@ -12734,6 +12734,7 @@ 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);
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 0fb59f68ffc3..978e10f7c8b4 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
> >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
