Hi Dumitru, Looks good to me, thanks!
Acked-by: Lucas Vargas Dias <[email protected]> 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 > > -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
