On 7/8/26 2:00 PM, Dumitru Ceara via dev wrote:
> 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')")
Ugh, the indentation is off here, but I can fix that in v2 or when
merging it if this patch is acked.
> +
> +echo $nd_ns > expected_nd_ns
> +OVN_CHECK_PACKETS_CONTAIN([hv/br-phys_n1-tx.pcap], [expected_nd_ns])
> +
> +OVN_CLEANUP([hv])
> +AT_CLEANUP
> +])
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
