On 7/8/26 7:43 PM, Lucas Vargas Dias wrote: > Hi Dumitru, > > Looks good to me, thanks! > > Acked-by: Lucas Vargas Dias <[email protected]> >
Hi Lucas, Lorenzo, Xavier, Thanks for the reviews! I applied this patch to main and 26.03. Regards, Dumitru > Regards, > Lucas > > Em qua., 8 de jul. de 2026 às 09:01, Dumitru Ceara <[email protected]> > escreveu: > >> Commit 4f562e9346c2 ("ovn: Fix the invalid eth.dst and ip6.dst >> set by nd_ns action for certain cases.") added priority-200 >> flows in lr_in_arp_request that pre-computed the solicited-node >> multicast address for each IPv6 static route nexthop. Those >> flows were needed because pinctrl_handle_nd_ns() was using >> ip_flow->ipv6_dst (the original packet's IPv6 destination) >> instead of the nexthop from xxreg0 to derive the solicited-node >> multicast. >> >> After d682437f99a7 ("pinctrl: Use correct IPv6 dst for nd_ns >> action for IPv4 over v6.") fixed pinctrl to read xxreg0 for the >> cross-AF case, extend pinctrl to ALWAYS read xxreg0. This is >> correct because the generic priority-100 nd_ns flow stores the >> nexthop in xxreg0 for all cases, and compose_nd_ns() already >> derives the solicited-node multicast internally when called >> with multicast=true. >> >> Remove the now-redundant priority-200 flows from northd along >> with their documentation. Simplify pinctrl_handle_nd_ns() to >> unconditionally read xxreg0, removing the dl_type branch and >> the manual in6_addr_solicited_node() call. >> >> Assisted-by: Claude Opus 4.6, OpenCode >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> Documentation/ref/ovn-logical-flows.7.rst | 15 ------ >> controller/pinctrl.c | 37 +++++--------- >> northd/northd.c | 47 +---------------- >> tests/ovn-northd.at | 2 - >> tests/ovn.at | 62 +++++++++++++++++++++++ >> 5 files changed, 75 insertions(+), 88 deletions(-) >> >> diff --git a/Documentation/ref/ovn-logical-flows.7.rst >> b/Documentation/ref/ovn-logical-flows.7.rst >> index 3d9936414f..95f7e0d7e1 100644 >> --- a/Documentation/ref/ovn-logical-flows.7.rst >> +++ b/Documentation/ref/ovn-logical-flows.7.rst >> @@ -3585,21 +3585,6 @@ or IPv6 Neighbor Solicitation request. It holds >> the following flows: >> output; >> }; >> >> - Unknown MAC address. For each IPv6 static route associated with the >> router >> - with the nexthop IP: *G*, a priority-200 flow for IPv6 packets with >> match >> - ``eth.dst == 00:00:00:00:00:00 && xxreg0 == G`` with the following >> actions is >> - added:: >> - >> - nd_ns { >> - eth.dst = E; >> - ip6.dst = I >> - nd.target = G; >> - output; >> - }; >> - >> - Where *E* is the multicast mac derived from the Gateway IP, *I* is the >> - solicited-node multicast address corresponding to the target address >> *G*. >> - >> Unknown MAC address. A priority-100 flow for IPv6 packets with match >> ``eth.dst == 00:00:00:00:00:00`` has the following actions:: >> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> index 1728aa7901..f898846285 100644 >> --- a/controller/pinctrl.c >> +++ b/controller/pinctrl.c >> @@ -6553,32 +6553,19 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const >> struct flow *ip_flow, >> >> in6_generate_lla(ip_flow->dl_src, &ipv6_src); >> >> - /* We might be here without actually currently handling an IPv6 >> packet. >> - * This can happen in the case where we route IPv4 packets over an >> IPv6 >> - * link (e.g. RFC 5549 / BGP unnumbered, where an IPv4 destination is >> - * resolved via an IPv6 link-local nexthop). >> - * >> - * In that case we have no destination IPv6 address in the trigger >> packet >> - * to reuse. compose_nd_ns() needs a valid destination so it can >> derive >> - * the correct solicited-node multicast (ff02::1:ff{addr[13:16]}) for >> - * eth.dst and ip6.dst -- userdata only sets nd.target on the new >> packet >> - * and does not rewrite ip6.dst, so a wrong ipv6_dst here egresses on >> the >> - * wire as-is. >> - * >> - * The fallback nd_ns logical flow in S_ROUTER_IN_ARP_REQUEST stores >> the >> - * actual IPv6 nexthop in xxreg0 (REG_NEXT_HOP_IPV6) before invoking >> the >> - * nd_ns action, so for the IPv4-over-IPv6 case read xxreg0 from the >> - * trigger packet's flow metadata. */ >> - struct in6_addr ipv6_dst = IN6ADDR_EXACT_INIT; >> - if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) { >> - ipv6_dst = ip_flow->ipv6_dst; >> - } else { >> - ovs_be128 nexthop_be = >> - hton128(flow_get_xxreg(&pin->flow_metadata.flow, 0)); >> - memcpy(&ipv6_dst, &nexthop_be, sizeof ipv6_dst); >> - } >> + /* The nd_ns logical flow in S_ROUTER_IN_ARP_REQUEST stores the actual >> + * IPv6 nexthop in xxreg0 (REG_NEXT_HOP_IPV6) before invoking the >> + * nd_ns action. Read the nexthop and pass it to compose_nd_ns() >> + * which derives the solicited-node multicast for eth.dst and >> + * ip6.dst. */ >> + struct in6_addr ipv6_nh = IN6ADDR_EXACT_INIT; >> + >> + ovs_be128 nexthop_be = >> + hton128(flow_get_xxreg(&pin->flow_metadata.flow, 0)); >> + memcpy(&ipv6_nh, &nexthop_be, sizeof ipv6_nh); >> + >> compose_nd_ns(&packet, true, ip_flow->dl_src, eth_addr_zero, >> &ipv6_src, >> - &ipv6_dst); >> + &ipv6_nh); >> >> /* Reload previous packet metadata and set actions from userdata. */ >> set_actions_and_enqueue_msg(swconn, &packet, >> diff --git a/northd/northd.c b/northd/northd.c >> index 62270893ab..e4cd93969a 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -16458,53 +16458,10 @@ build_lr_gateway_redirect_flows_for_nats( >> static void >> build_arp_request_flows_for_lrouter( >> struct ovn_datapath *od, struct lflow_table *lflows, >> - struct ds *match, struct ds *actions, >> const struct shash *meter_groups, >> struct lflow_ref *lflow_ref) >> { >> ovs_assert(od->nbr); >> - for (int i = 0; i < od->nbr->n_static_routes; i++) { >> - const struct nbrec_logical_router_static_route *route; >> - >> - route = od->nbr->static_routes[i]; >> - struct in6_addr gw_ip6; >> - unsigned int plen; >> - char *error = ipv6_parse_cidr(route->nexthop, &gw_ip6, &plen); >> - if (error || plen != 128) { >> - free(error); >> - continue; >> - } >> - >> - ds_clear(match); >> - ds_put_format(match, "eth.dst == 00:00:00:00:00:00 && " >> - REGBIT_NEXTHOP_IS_IPV4" == 0 && " >> - REG_NEXT_HOP_IPV6 " == %s", >> - route->nexthop); >> - struct in6_addr sn_addr; >> - struct eth_addr eth_dst; >> - in6_addr_solicited_node(&sn_addr, &gw_ip6); >> - ipv6_multicast_to_ethernet(ð_dst, &sn_addr); >> - >> - char sn_addr_s[INET6_ADDRSTRLEN + 1]; >> - ipv6_string_mapped(sn_addr_s, &sn_addr); >> - >> - ds_clear(actions); >> - ds_put_format(actions, >> - "nd_ns { " >> - "eth.dst = "ETH_ADDR_FMT"; " >> - "ip6.dst = %s; " >> - "nd.target = %s; " >> - "output; " >> - "}; next;", ETH_ADDR_ARGS(eth_dst), sn_addr_s, >> - route->nexthop); >> - >> - ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 200, >> - ds_cstr(match), ds_cstr(actions), lflow_ref, >> - WITH_CTRL_METER(copp_meter_get(COPP_ND_NS_RESOLVE, >> - od->nbr->copp, >> - meter_groups)), >> - WITH_HINT(&route->header_)); >> - } >> >> ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 100, >> "eth.dst == 00:00:00:00:00:00 && " >> @@ -19716,9 +19673,7 @@ build_lswitch_and_lrouter_iterate_by_lr(struct >> ovn_datapath *od, >> build_gateway_redirect_flows_for_lrouter(od, lsi->lflows, &lsi->match, >> &lsi->actions, >> od->datapath_lflows); >> - build_arp_request_flows_for_lrouter(od, lsi->lflows, &lsi->match, >> - &lsi->actions, >> - lsi->meter_groups, >> + build_arp_request_flows_for_lrouter(od, lsi->lflows, >> lsi->meter_groups, >> od->datapath_lflows); >> build_ecmp_stateful_egr_flows_for_lrouter(od, lsi->lflows, >> od->datapath_lflows); >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index f82902c61d..886dd9aba0 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -7717,8 +7717,6 @@ AT_CHECK([grep -e "lr_in_arp_request" lr0flows | >> ovn_strip_lflows], [0], [dnl >> table=??(lr_in_arp_request ), priority=0 , match=(1), action=(next;) >> table=??(lr_in_arp_request ), priority=100 , match=(eth.dst == >> 00:00:00:00:00:00 && reg9[[9]] == 0), action=(nd_ns { nd.target = xxreg0; >> output; }; next;) >> table=??(lr_in_arp_request ), priority=100 , match=(eth.dst == >> 00:00:00:00:00:00 && reg9[[9]] == 1), action=(arp { eth.dst = >> ff:ff:ff:ff:ff:ff; arp.spa = reg5; arp.tpa = reg0; arp.op = 1; output; }; >> next;) >> - table=??(lr_in_arp_request ), priority=200 , match=(eth.dst == >> 00:00:00:00:00:00 && reg9[[9]] == 0 && xxreg0 == 2001:db8::10), >> action=(nd_ns { eth.dst = 33:33:ff:00:00:10; ip6.dst = ff02::1:ff00:10; >> nd.target = 2001:db8::10; output; }; next;) >> - 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. >> diff --git a/tests/ovn.at b/tests/ovn.at >> index abd12f0ddb..31a97aead9 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -46586,3 +46586,65 @@ OVN_CLEANUP([hv]) >> AT_CLEANUP >> ]) >> >> +OVN_FOR_EACH_NORTHD([ >> +AT_SETUP([IPv6 static route ND NS resolution]) >> +CHECK_SCAPY >> +ovn_start >> + >> +net_add n1 >> +sim_add hv >> +ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.1 >> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys >> + >> +check ovn-nbctl ls-add sw0 >> +check ovn-nbctl lsp-add sw0 sw0-port1 >> +check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 >> 2001:db8::3" >> + >> +check ovn-nbctl lr-add lr0 >> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 2001:db8::1/64 >> +check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0 >> + >> +check ovn-nbctl ls-add public >> +check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 2001:db8:1::1/64 >> +check ovn-nbctl lsp-add-router-port public public-lr0 lr0-public >> + >> +# localnet port >> +check ovn-nbctl lsp-add-localnet-port public ln-public phys >> + >> +check ovn-nbctl lrp-set-gateway-chassis lr0-public hv 20 >> + >> +# Add an IPv6 static route with an IPv6 nexthop whose MAC is unknown. >> +check ovn-nbctl --wait=hv lr-route-add lr0 2001:db8:2::/64 >> 2001:db8:1::cafe >> + >> +ovs-vsctl -- add-port br-int vif1 -- \ >> + set interface vif1 external-ids:iface-id=sw0-port1 \ >> + options:tx_pcap=hv/vif1-tx.pcap \ >> + options:rxq_pcap=hv/vif1-rx.pcap >> + >> +wait_for_ports_up >> + >> +# Send an IPv6 packet from sw0-port1 destined for an address behind the >> +# static route. The nexthop 2001:db8:1::cafe has no MAC binding, so the >> +# router must emit an ND NS to resolve it. >> +packet=$(fmt_pkt "Ether(dst='00:00:00:00:ff:01', >> src='50:54:00:00:00:01')/ \ >> + IPv6(src='2001:db8::3', dst='2001:db8:2::42', hlim=64)/ >> \ >> + ICMPv6EchoRequest()") >> +check as hv ovs-appctl netdev-dummy/receive vif1 $packet >> + >> +# Verify the ND NS has correct solicited-node multicast addresses derived >> +# from the nexthop 2001:db8:1::cafe: >> +# eth.dst = 33:33:ff:00:ca:fe (multicast MAC from solicited-node) >> +# ip6.dst = ff02::1:ff00:cafe (solicited-node multicast) >> +# nd.target = 2001:db8:1::cafe (the actual nexthop) >> +nd_ns=$(fmt_pkt "Ether(dst='33:33:ff:00:ca:fe', src='00:00:20:20:12:13')/ >> \ >> + IPv6(src='fe80::200:20ff:fe20:1213', \ >> + >> dst='ff02::1:ff00:cafe')/ICMPv6ND_NS(tgt='2001:db8:1::cafe')/\ >> + ICMPv6NDOptSrcLLAddr(lladdr='00:00:20:20:12:13')") >> + >> +echo $nd_ns > expected_nd_ns >> +OVN_CHECK_PACKETS_CONTAIN([hv/br-phys_n1-tx.pcap], [expected_nd_ns]) >> + >> +OVN_CLEANUP([hv]) >> +AT_CLEANUP >> +]) >> -- >> 2.54.0 >> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
