> 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

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

Reply via email to