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
