[PATCH net v2 0/3] net: ioam6: makes IOAM insertion transparent to IPv6 ECMP

[PATCH net v2 0/3] net: ioam6: makes IOAM insertion transparent to IPv6 ECMP

[Hangbin Liu]

This series makes IOAM insertion transparent to IPv6 ECMP, as required
by RFC 9486 (C2): a packet carrying IOAM options must be forwarded
along the same path as the identical packet without them.

Patch 1 fixes the transit side: hash policy 0 hashed the raw IPv6
nexthdr, which becomes NEXTHDR_HOP once an IOAM Hop-by-Hop header is
inserted. The policy-0 key (and the ICMPv6-error flow selection) now
uses the nexthdr the packet would have had if the Hop-by-Hop header
carried only IOAM options and padding.

Patch 2 fixes the injecting node: on an ioam6 output destination-cache
miss, inline mode now reuses orig_dst so the ECMP sibling from the
original socket lookup is kept (the destination does not change).
Encapsulation may switch the destination to tundst, so it still
re-looks up the transformed packet, using the Hop-by-Hop nexthdr for
flowi6_proto (IPPROTO_IPV6 after encap) and precomputing fl6.mp_hash
from the packet so ICMPv6 quoted-flow and L4 keys are not lost when
ip6_route_output() runs with no skb.

Patch 3 adds selftests/net/ioam6_ecmp.sh, covering both places: a
transit forwarder with a multipath route, and an encapsulating node
compared against a plain ip6ip6 baseline.

The issue was found while reviewing RFC9486. It can be reproduced and
verified by the newly added ioam6_ecmp.sh test.

Tested with:
ecmp_rehash.sh
fib_nexthops.sh
ioam6_ecmp.sh
tcp_ecmp_failover.sh
forwarding/gre_inner_v4_multipath.sh
forwarding/gre_inner_v6_multipath.sh
forwarding/gre_multipath_nh_res.sh
forwarding/gre_multipath_nh.sh
forwarding/gre_multipath.sh
forwarding/ip6gre_inner_v4_multipath.sh
forwarding/ip6gre_inner_v6_multipath.sh
forwarding/router_mpath_nh_res.sh
forwarding/router_mpath_nh.sh
forwarding/router_multipath.sh

---
Changes in v2:
- patch 01:
  * use skb_network_offset() as the start of offset.
  * try get inner next header for ICMPv6 errors.
- patch 02:
  * use origin dst for inline mode, as dst is not changed.
  * use HBH nexthdr for encap mode, as dst may change to tundst.
- patch 03:
  * remove `-w` to prevent ping from ignoring `-c` and continuing to send
    packets before receiving replies.
  * remove beta_tnl warm up since the tunnel will be re-create every time.
  * move trap cleanup before setup
  * update some comments
  * remove tunsrc in ioam encap setup
- Link to v1: https://patch.msgid.link/20261028-ipv6_ioam_ecmp-v1-0-9b6715b6f4c0@kylinos.cn

---
Hangbin Liu (3):
      ipv6: route: consider IOAM options in ECMP hash policy 0
      ipv6: ioam6: use transformed packet nexthdr for ECMP re-lookup
      selftests: net: add ioam6 ECMP test (RFC 9486 C2)

 include/uapi/linux/in6.h                  |   1 +
 net/ipv6/ioam6_iptunnel.c                 |  79 ++++---
 net/ipv6/route.c                          |  95 +++++++-
 tools/testing/selftests/net/Makefile      |   1 +
 tools/testing/selftests/net/config        |   2 +
 tools/testing/selftests/net/ioam6_ecmp.sh | 348 ++++++++++++++++++++++++++++++
 6 files changed, 495 insertions(+), 31 deletions(-)
---
base-commit: c3e67bd82b9b9b1a5118f09a429ccefe2aab0741
change-id: 20261020-ipv6_ioam_ecmp-ae4ef0a4bdae

Best regards,
--  
Hangbin Liu <liuhangbin@kylinos.cn>

[PATCH net v2 1/3] ipv6: route: consider IOAM options in ECMP hash policy 0

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

RFC 9486 (C2) requires that the ECMP hash of a packet carrying IOAM
options is the same as the hash of the identical packet without them.

The default multipath hash policy (0) hashes basic.ip_proto, which
ip6_multipath_l3_keys() fills from the raw IPv6 nexthdr. An IOAM
Hop-by-Hop option is inserted ahead of the original next header, so a
transit router sees NEXTHDR_HOP (0) instead of e.g. IPPROTO_TCP and
may pick a different ECMP sibling than for the same packet without
IOAM.

Recover the nexthdr the packet would have had if its Hop-by-Hop
header carried only IOAM options (0x31, and 0x11 for DEX) plus
padding, and use it for the policy-0 key and for the ICMPv6-error
check, so errors that quote a flow keep hashing that flow. Any other
Hop-by-Hop option (Router Alert, Jumbo, ...) keeps hashing exactly as
before. The walk runs for both the nexthdr == NEXTHDR_HOP case and
ICMPv6-quoted inner headers (with fallback if truncated).

Fixes: 3edede08ff37 ("ipv6: ioam: Support for IOAM injection with lwtunnels")
Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
The ECMP path with and without IOAM is verified by
tools/testing/selftests/net/ioam6_ecmp.sh in the next patch.
---
 include/uapi/linux/in6.h |  1 +
 net/ipv6/route.c         | 95 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
index 5a47339ef7d7..2e66dc37e1e5 100644
--- a/include/uapi/linux/in6.h
+++ b/include/uapi/linux/in6.h
@@ -145,6 +145,7 @@ struct in6_flowlabel_req {
 #define IPV6_TLV_PADN		1
 #define IPV6_TLV_ROUTERALERT	5
 #define IPV6_TLV_CALIPSO	7	/* RFC 5570 */
+#define IPV6_TLV_IOAM_DEX	17	/* RFC 9486 */
 #define IPV6_TLV_IOAM		49	/* RFC 9486 */
 #define IPV6_TLV_JUMBO		194
 #define IPV6_TLV_HAO		201	/* home address option */
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 616b86f89f63..675cf2e8ed35 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2342,6 +2342,71 @@ struct dst_entry *ip6_route_input_lookup(struct net *net,
 }
 EXPORT_SYMBOL_GPL(ip6_route_input_lookup);
 
+/* RFC 9486 (C2) requires that the ECMP hash of a packet carrying IOAM
+ * options is the same as the hash of the identical packet without them.
+ * Recover the nexthdr if its Hop-by-Hop header carried only IOAM options
+ * and padding, every other packet is hashed exactly as before.
+ *
+ * iph_off: offset of the IPv6 header from skb->data
+ * nexthdr: iph->nexthdr (NEXTHDR_HOP or not)
+ */
+static u8 ip6_nexthdr_without_ioam(const struct sk_buff *skb,
+				   int iph_off, u8 nexthdr)
+{
+	const struct ipv6_opt_hdr *hhdr;
+	struct ipv6_opt_hdr hbuf;
+	bool has_ioam = false;
+	u8 obuf[2], *opt;
+	int off, len, i;
+
+	if (likely(nexthdr != NEXTHDR_HOP))
+		return nexthdr;
+
+	off = iph_off + sizeof(struct ipv6hdr);
+	hhdr = skb_header_pointer(skb, off, sizeof(hbuf), &hbuf);
+	if (!hhdr)
+		return nexthdr;
+
+	len = (hhdr->hdrlen + 1) << 3;
+
+	for (i = 2; i < len; ) {
+		const u8 *type;
+		u8 type_buf;
+
+		/* Always check PAD1 first as ip6_parse_tlv() accepts Pad1
+		 * as the last byte of the HBH header
+		 */
+		type = skb_header_pointer(skb, off + i, 1, &type_buf);
+		if (!type)
+			return has_ioam ? hhdr->nexthdr : nexthdr;
+		if (*type == IPV6_TLV_PAD1) {
+			i++;
+			continue;
+		}
+
+		opt = skb_header_pointer(skb, off + i, sizeof(obuf), obuf);
+		if (!opt)
+			return nexthdr;
+
+		if (opt[0] == IPV6_TLV_PADN) {
+			i += opt[1] + 2;
+			continue;
+		}
+		if (opt[0] != IPV6_TLV_IOAM && opt[0] != IPV6_TLV_IOAM_DEX)
+			return nexthdr;
+
+		has_ioam = true;
+
+		/* Keep searching for the next TLV. As long as non-IOAM
+		 * options (Router Alert, Jumbo, etc.) exist in the HBH
+		 * header, we should preserve the original NEXTHDR_HOP.
+		 */
+		i += opt[1] + 2;
+	}
+
+	return has_ioam ? hhdr->nexthdr : nexthdr;
+}
+
 static void ip6_multipath_l3_keys(const struct sk_buff *skb,
 				  struct flow_keys *keys,
 				  struct flow_keys *flkeys)
@@ -2349,15 +2414,34 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
 	const struct ipv6hdr *outer_iph = ipv6_hdr(skb);
 	const struct ipv6hdr *key_iph = outer_iph;
 	struct flow_keys *_flkeys = flkeys;
+	const struct ipv6_opt_hdr *opt_hdr;
 	const struct ipv6hdr *inner_iph;
 	const struct icmp6hdr *icmph;
+	struct ipv6_opt_hdr _opt_hdr;
 	struct ipv6hdr _inner_iph;
 	struct icmp6hdr _icmph;
+	int extlen = 0;
+	int inner_off;
+	u8 nexthdr;
 
-	if (likely(outer_iph->nexthdr != IPPROTO_ICMPV6))
+	nexthdr = ip6_nexthdr_without_ioam(skb, skb_network_offset(skb),
+					   outer_iph->nexthdr);
+	if (likely(nexthdr != IPPROTO_ICMPV6))
 		goto out;
 
-	icmph = skb_header_pointer(skb, skb_transport_offset(skb),
+	/* Outer header has IOAM (NEXTHDR_HOP) and the recovered nexthdr is
+	 * IPPROTO_ICMPV6. transport_header may still point at the Hop-by-Hop
+	 * header (e.g. ipv6_hop_ioam() -> ip6_route_input() before
+	 * ipv6_parse_hopopts() advances it).
+	 */
+	if (outer_iph->nexthdr == NEXTHDR_HOP) {
+		opt_hdr = skb_header_pointer(skb, skb_transport_offset(skb),
+					     sizeof(_opt_hdr), &_opt_hdr);
+		if (opt_hdr && opt_hdr->nexthdr == IPPROTO_ICMPV6)
+			extlen = (opt_hdr->hdrlen + 1) << 3;
+	}
+
+	icmph = skb_header_pointer(skb, skb_transport_offset(skb) + extlen,
 				   sizeof(_icmph), &_icmph);
 	if (!icmph)
 		goto out;
@@ -2365,14 +2449,15 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
 	if (!icmpv6_is_err(icmph->icmp6_type))
 		goto out;
 
-	inner_iph = skb_header_pointer(skb,
-				       skb_transport_offset(skb) + sizeof(*icmph),
+	inner_off = skb_transport_offset(skb) + extlen + sizeof(*icmph);
+	inner_iph = skb_header_pointer(skb, inner_off,
 				       sizeof(_inner_iph), &_inner_iph);
 	if (!inner_iph)
 		goto out;
 
 	key_iph = inner_iph;
 	_flkeys = NULL;
+	nexthdr = ip6_nexthdr_without_ioam(skb, inner_off, inner_iph->nexthdr);
 out:
 	if (_flkeys) {
 		keys->addrs.v6addrs.src = _flkeys->addrs.v6addrs.src;
@@ -2383,7 +2468,7 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
 		keys->addrs.v6addrs.src = key_iph->saddr;
 		keys->addrs.v6addrs.dst = key_iph->daddr;
 		keys->tags.flow_label = ip6_flowlabel(key_iph);
-		keys->basic.ip_proto = key_iph->nexthdr;
+		keys->basic.ip_proto = nexthdr;
 	}
 }
 

-- 
2.56.0

[PATCH net v2 2/3] ipv6: ioam6: use transformed packet nexthdr for ECMP re-lookup

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

RFC 9486 (C2) requires that the ECMP hash of a packet carrying IOAM
options is the same as the hash of the identical packet without them.

On a destination-cache miss the old code always re-looked up the
transformed packet under hash policy 0 with NEXTHDR_HOP (and, for
local flows, keys that do not match the original socket lookup), so
it could pick a different ECMP sibling.

Inline insertion leaves the destination unchanged: reuse orig_dst and
cache the fake null_rt, keeping the sibling from the original lookup.

Encapsulation may switch the destination to tundst, so it still
re-looks up. Take flowi6_proto from the transformed packet HBH (41
after encap, so ipproto rules match as for plain ip6ip6) and
precompute fl6.mp_hash from the packet, since ip6_route_output()
carries no skb and would otherwise drop ICMPv6 quoted-flow and L4
keys.

Depends on patch "ipv6: route: consider IOAM options in ECMP hash
policy 0".

Fixes: 8cb3bf8bff3c ("ipv6: ioam: Add support for the ip6ip6 encapsulation")
Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
For AI review:

The mp_hash precompute makes policy-2 hash inner keys while plain ip6ip6
baseline hashes outer — pre-patch also diverged (proto 0 vs 41);
C2 for policy 2 was broken before and after, no new violation class, and
the patches claim/test policy 0 only.
---
 net/ipv6/ioam6_iptunnel.c | 79 +++++++++++++++++++++++++++++++----------------
 1 file changed, 53 insertions(+), 26 deletions(-)

diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index cfb2c41634a0..246533276be8 100644
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -405,34 +405,61 @@ static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 
 	if (unlikely(!dst)) {
 		struct ipv6hdr *hdr = ipv6_hdr(skb);
-		struct flowi6 fl6;
-
-		memset(&fl6, 0, sizeof(fl6));
-		fl6.daddr = hdr->daddr;
-		fl6.saddr = hdr->saddr;
-		fl6.flowlabel = ip6_flowinfo(hdr);
-		fl6.flowi6_mark = skb->mark;
-		fl6.flowi6_proto = hdr->nexthdr;
-
-		dst = ip6_route_output(net, NULL, &fl6);
-		if (dst->error) {
-			err = dst->error;
-			goto drop;
-		}
 
-		/* If the destination is the same after transformation (which is
-		 * a valid use case for IOAM), then we don't want to add it to
-		 * the cache in order to avoid a reference loop. Instead, we add
-		 * our fake dst_entry to the cache as a way to detect this case.
-		 * Otherwise, we add the resolved destination to the cache.
+		/* Inline insertion does not change the destination. Keep
+		 * orig_dst so the ECMP sibling from the original lookup is
+		 * preserved (RFC 9486 C2 for locally generated traffic).
+		 * Cache the fake null_rt so later packets take the same path.
 		 */
-		local_bh_disable();
-		if (orig_dst->lwtstate == dst->lwtstate)
-			dst_cache_set_ip6(&ilwt->cache,
-					  &ilwt->null_rt.dst, &fl6.saddr);
-		else
-			dst_cache_set_ip6(&ilwt->cache, dst, &fl6.saddr);
-		local_bh_enable();
+		if (ilwt->mode == IOAM6_IPTUNNEL_MODE_INLINE ||
+		    (ilwt->mode == IOAM6_IPTUNNEL_MODE_AUTO && !skb->dev)) {
+			dst = orig_dst;
+			dst_hold(dst);
+
+			local_bh_disable();
+			dst_cache_set_ip6(&ilwt->cache, &ilwt->null_rt.dst,
+					  &hdr->saddr);
+			local_bh_enable();
+		} else {
+			/* Encapsulation may change the destination (tundst);
+			 * re-lookup with the transformed packet keys so the
+			 * outer hash matches the identical packet without IOAM.
+			 */
+			const struct ipv6_opt_hdr *hhdr;
+			struct flowi6 fl6;
+
+			hhdr = (const struct ipv6_opt_hdr *)skb_transport_header(skb);
+
+			memset(&fl6, 0, sizeof(fl6));
+			fl6.daddr = hdr->daddr;
+			fl6.saddr = hdr->saddr;
+			fl6.flowlabel = ip6_flowinfo(hdr);
+			fl6.flowi6_mark = skb->mark;
+			fl6.flowi6_proto = hhdr->nexthdr;
+			fl6.mp_hash = rt6_multipath_hash(net, &fl6, skb, NULL);
+
+			dst = ip6_route_output(net, NULL, &fl6);
+			if (dst->error) {
+				err = dst->error;
+				goto drop;
+			}
+
+			/* If the destination is the same after transformation
+			 * (which is a valid use case for IOAM), then we don't
+			 * want to add it to the cache in order to avoid a
+			 * reference loop. Instead, we add our fake dst_entry
+			 * to the cache as a way to detect this case.
+			 * Otherwise, we add the resolved destination to the
+			 * cache.
+			 */
+			local_bh_disable();
+			if (orig_dst->lwtstate == dst->lwtstate)
+				dst_cache_set_ip6(&ilwt->cache,
+						  &ilwt->null_rt.dst, &fl6.saddr);
+			else
+				dst_cache_set_ip6(&ilwt->cache, dst, &fl6.saddr);
+			local_bh_enable();
+		}
 
 		err = skb_cow_head(skb, LL_RESERVED_SPACE(dst_dev(dst)));
 		if (unlikely(err))

-- 
2.56.0

[PATCH net v2 3/3] selftests: net: add ioam6 ECMP test (RFC 9486 C2)

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

Add new test ioam6_ecmp.sh, verifying that the ECMP path taken with
IOAM options is identical to the path of the same packet without them
under the default hash policy 0, as required by RFC 9486 (C2).

  - transit input hashing: the sender inserts an IOAM Hop-by-Hop option
    (inline mode), the forwarder has a multipath route;
  - encapsulating node re-lookup: the forwarder itself encapsulates
    over a multipath route, with a plain ip6ip6 tunnel as the
    identical-packet-without-IOAM baseline.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 tools/testing/selftests/net/Makefile      |   1 +
 tools/testing/selftests/net/config        |   2 +
 tools/testing/selftests/net/ioam6_ecmp.sh | 348 ++++++++++++++++++++++++++++++
 3 files changed, 351 insertions(+)

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 9180b9bec515..d15f118e70a2 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -52,6 +52,7 @@ TEST_PROGS := \
 	icmp_redirect.sh \
 	io_uring_zerocopy_tx.sh \
 	ioam6.sh \
+	ioam6_ecmp.sh \
 	ip6_gre_headroom.sh \
 	ip6_tunnel.sh \
 	ip_defrag.sh \
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index d4416f8bdcfd..e1de8c0410ef 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -38,6 +38,7 @@ CONFIG_IP_NF_NAT=m
 CONFIG_IP_NF_RAW=m
 CONFIG_IP_NF_TARGET_REJECT=m
 CONFIG_IP_NF_TARGET_TTL=m
+CONFIG_IP_ROUTE_MULTIPATH=y
 CONFIG_IP_SCTP=m
 CONFIG_IPV6=y
 CONFIG_IPV6_FOU=m
@@ -52,6 +53,7 @@ CONFIG_IPV6_ROUTER_PREF=y
 CONFIG_IPV6_RPL_LWTUNNEL=y
 CONFIG_IPV6_SEG6_LWTUNNEL=y
 CONFIG_IPV6_SIT=y
+CONFIG_IPV6_TUNNEL=m
 CONFIG_IPV6_VTI=y
 CONFIG_IPVLAN=m
 CONFIG_IPVTAP=m
diff --git a/tools/testing/selftests/net/ioam6_ecmp.sh b/tools/testing/selftests/net/ioam6_ecmp.sh
new file mode 100755
index 000000000000..35a11f94c422
--- /dev/null
+++ b/tools/testing/selftests/net/ioam6_ecmp.sh
@@ -0,0 +1,348 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Verify RFC 9486 C2 for ECMP: with hash policy 0 (the default), the ECMP
+# nexthop chosen for a packet carrying IOAM options must be the same as for
+# the identical packet without them.
+#
+# Two scenarios, matching the two places where IOAM changed the hashed
+# IPv6 nexthdr:
+#
+# 1) transit node: alpha inserts an IOAM Hop-by-Hop option (inline mode);
+#    beta forwards with a multipath route. Policy 0 hashed the raw
+#    nexthdr (NEXTHDR_HOP vs IPPROTO_ICMPV6), so beta could pick a
+#    different ECMP sibling than for the same packet without IOAM.
+#
+# 2) encapsulating node: beta itself encapsulates over a multipath
+#    route. The ioam6 re-lookup hashed NEXTHDR_HOP instead of the
+#    transformed packet's real nexthdr (IPPROTO_IPV6). The baseline is
+#    the same encapsulation without the IOAM option: a plain ip6ip6
+#    tunnel (encaplimit none) over the same multipath route, with the
+#    same outer addresses.
+#
+# Hash policy 0 hashes addresses, the flow label and the nexthdr (not L4
+# ports), so whether both variants land on the same ECMP sibling depends
+# on the per-netns hash seed. Each test therefore repeats with several
+# seeds and fails on the first divergence: pre-fix kernels fail with
+# high probability, fixed kernels always pass.
+#
+# Topology:
+#
+#   alpha                   beta                      gamma
+#   veth0 2001:db8:1::2 <-> veth0 2001:db8:1::1
+#                           veth1 2001:db8:2:1::1 <-> veth0 2001:db8:2:1::2
+#                           veth2 2001:db8:2:2::1 <-> veth1 2001:db8:2:2::2
+#
+# beta's ECMP route: 2001:db8:2:1::2/128
+#   nexthop via 2001:db8:2:1::2 dev veth1
+#   nexthop via 2001:db8:2:2::2 dev veth2
+#
+# Scenario 1 sends to 2001:db8:2:1::2 (gamma). Scenario 2 sends to
+# 2001:db8:3::2, which beta encapsulates towards 2001:db8:2:1::2. The
+# link gamma received the packets on is detected from tc flower
+# counters (read via jq) on the ingress of gamma's veth0/veth1. The
+# filters match the exact measurement packets: ICMPv6 echo requests
+# for scenario 1, and the outer addresses of the ip6ip6 packets for
+# scenario 2 (flower classifies outer headers only).
+
+# shellcheck disable=SC2154,SC2329
+source lib.sh
+
+readonly PKTS=3
+readonly SEEDS="1 2 3 4 5 6 7 8 9 10 11 12"
+
+ip6tnl_loaded=0
+
+cleanup()
+{
+	cleanup_all_ns
+
+	if [ "$ip6tnl_loaded" != 0 ]; then
+		modprobe -r ip6_tunnel &>/dev/null || true
+	fi
+}
+
+# Replace the tc measurement filters on gamma's ECMP links. $1 is the
+# flower match list, e.g. "ip_proto icmpv6 type 128 src_ip <sender>"
+# for plain IPv6 echo requests, or the outer addresses for ip6ip6
+# packets (flower classifies the outer header only, see
+# FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP in cls_flower).
+set_measure_filters()
+{
+	local match=$1
+	local dev
+
+	for dev in veth0 veth1; do
+		tc -n "$ns_gamma" qdisc del dev "$dev" clsact &>/dev/null
+		tc -n "$ns_gamma" qdisc add dev "$dev" clsact || return 1
+		# shellcheck disable=SC2086
+		tc -n "$ns_gamma" filter add dev "$dev" ingress pref 1 \
+			protocol ipv6 flower skip_hw $match action pass || return 1
+	done
+}
+
+# Number of packets counted by the measurement filter on gamma's $1.
+icmp_packets()
+{
+	tc -n "$ns_gamma" -s -j filter show dev "$1" ingress |
+		jq '[.[] | .options.actions[]?.stats |
+		    (.packets // .basic.packets // 0)] | add // 0'
+}
+
+# Send PKTS pings from alpha to $1 and report which of gamma's links
+# received them: "A" (veth0), "B" (veth1), "TIE" or "LOST:<a>:<b>".
+measure_link()
+{
+	local dst=$1
+	local a0 b0 da db
+
+	a0=$(icmp_packets veth0)
+	b0=$(icmp_packets veth1)
+
+	ip netns exec "$ns_alpha" \
+		ping -6 -q -c "$PKTS" -i 0.2 -W 1 "$dst" &>/dev/null || true
+
+	da=$(($(icmp_packets veth0) - a0))
+	db=$(($(icmp_packets veth1) - b0))
+
+	if [ $((da + db)) -lt "$PKTS" ]; then
+		echo "LOST:$da:$db"
+		return
+	fi
+
+	if [ "$da" -gt "$db" ]; then
+		echo "A"
+	elif [ "$db" -gt "$da" ]; then
+		echo "B"
+	else
+		echo "TIE"
+	fi
+}
+
+set_alpha_plain()
+{
+	ip -netns "$ns_alpha" -6 route replace 2001:db8:2:1::/64 \
+		via 2001:db8:1::1 dev veth0
+}
+
+set_alpha_ioam()
+{
+	ip -netns "$ns_alpha" -6 route replace 2001:db8:2:1::/64 \
+		encap ioam6 mode inline trace prealloc \
+		type 0x800000 ns 0 size 12 \
+		via 2001:db8:1::1 dev veth0
+}
+
+set_beta_tnl()
+{
+	# Recreate the tunnel: ip6_tnl caches the outer dst per CPU,
+	# and neither the route change below nor the fib_multipath_hash_seed
+	# write resets it. A stale cache would resolve the outer sibling
+	# under a previous seed (or on another CPU), making the baseline
+	# flip between attempts of one seed.
+	ip -netns "$ns_beta" link del ip6tun0 &>/dev/null
+	ip -netns "$ns_beta" link add name ip6tun0 type ip6tnl \
+		mode ip6ip6 encaplimit none \
+		local 2001:db8:2:1::1 remote 2001:db8:2:1::2
+	ip -netns "$ns_beta" link set ip6tun0 up
+	ip -netns "$ns_beta" -6 route replace 2001:db8:3::2/128 dev ip6tun0
+}
+
+set_beta_ioam_encap()
+{
+	ip -netns "$ns_beta" -6 route replace 2001:db8:3::2/128 \
+		encap ioam6 mode encap tundst 2001:db8:2:1::2 \
+		trace prealloc type 0x800000 ns 0 size 12 dev veth1
+}
+
+# $1: description; $2: function installing the without-IOAM variant;
+# $3: function installing the with-IOAM variant; $4: destination.
+run_scenario()
+{
+	local desc=$1
+	local plain=$2
+	local ioam=$3
+	local dst=$4
+	local seed pl il
+
+	log_info "$desc"
+
+	for seed in $SEEDS; do
+		RET=0
+		ip netns exec "$ns_beta" \
+			sysctl -wq net.ipv4.fib_multipath_hash_seed="$seed"
+
+		if ! $plain; then
+			RET=$ksft_fail
+			log_test "$desc (plain route setup failed, seed $seed)"
+			continue
+		fi
+		pl=$(measure_link "$dst")
+		if ! $ioam; then
+			RET=$ksft_fail
+			log_test "$desc (ioam route setup failed, seed $seed)"
+			continue
+		fi
+		il=$(measure_link "$dst")
+
+		case "$pl$il" in
+		*LOST*)
+			RET=$ksft_fail
+			log_test "$desc (lost packets, seed $seed: without=$pl with=$il)"
+			continue
+			;;
+		*TIE*)
+			RET=$ksft_fail
+			log_test "$desc (no clear link, seed $seed)"
+			continue
+			;;
+		esac
+
+		if [ "$pl" = "$il" ]; then
+			log_test "$desc (seed $seed: same link $pl)"
+		else
+			RET=$ksft_fail
+			log_test "$desc (diverged on seed $seed: without IOAM via link $pl, with IOAM via link $il)"
+		fi
+	done
+}
+
+setup_topology()
+{
+	local dev
+
+	setup_ns ns_alpha ns_beta ns_gamma || return 1
+
+	ip link add veth0 netns "$ns_alpha" type veth \
+		peer name veth0 netns "$ns_beta" || return 1
+	ip link add veth1 netns "$ns_beta" type veth \
+		peer name veth0 netns "$ns_gamma" || return 1
+	ip link add veth2 netns "$ns_beta" type veth \
+		peer name veth1 netns "$ns_gamma" || return 1
+
+	ip -n "$ns_alpha" addr add 2001:db8:1::2/64 dev veth0 nodad
+	ip -n "$ns_beta" addr add 2001:db8:1::1/64 dev veth0 nodad
+	ip -n "$ns_beta" addr add 2001:db8:2:1::1/64 dev veth1 nodad
+	ip -n "$ns_beta" addr add 2001:db8:2:2::1/64 dev veth2 nodad
+	ip -n "$ns_gamma" addr add 2001:db8:2:1::2/64 dev veth0 nodad
+	ip -n "$ns_gamma" addr add 2001:db8:2:2::2/64 dev veth1 nodad
+
+	ip -n "$ns_alpha" link set veth0 up
+	ip -n "$ns_beta" link set veth0 up
+	ip -n "$ns_beta" link set veth1 up
+	ip -n "$ns_beta" link set veth2 up
+	ip -n "$ns_gamma" link set veth0 up
+	ip -n "$ns_gamma" link set veth1 up
+
+	ip netns exec "$ns_alpha" sysctl -wq net.ipv6.auto_flowlabels=0
+	ip netns exec "$ns_alpha" \
+		sysctl -wq net.ipv6.conf.veth0.accept_ra=0
+	ip netns exec "$ns_beta" sysctl -wq net.ipv6.conf.all.forwarding=1
+	ip netns exec "$ns_beta" \
+		sysctl -wq net.ipv6.fib_multipath_hash_policy=0
+	ip netns exec "$ns_gamma" sysctl -wq net.ipv6.conf.veth0.accept_ra=0
+	ip netns exec "$ns_gamma" sysctl -wq net.ipv6.conf.veth1.accept_ra=0
+
+	for dev in veth0 veth1 veth2
+	do
+		ip netns exec "$ns_beta" \
+			sysctl -wq net.ipv6.conf.$dev.accept_ra=0
+		ip netns exec "$ns_beta" \
+			sysctl -wq net.ipv6.conf.$dev.ioam6_enabled=1
+	done
+
+	# The ECMP route under test (scenario 1 hashes its selection at
+	# beta's input; scenario 2 at the encapsulating re-lookup).
+	ip -netns "$ns_beta" -6 route add 2001:db8:2:1::2/128 \
+		nexthop via 2001:db8:2:1::2 dev veth1 \
+		nexthop via 2001:db8:2:2::2 dev veth2 || return 1
+
+	ip -netns "$ns_alpha" -6 route add 2001:db8:2:1::/64 \
+		via 2001:db8:1::1 dev veth0
+	ip -netns "$ns_alpha" -6 route add 2001:db8:3::/64 \
+		via 2001:db8:1::1 dev veth0
+	ip -netns "$ns_gamma" -6 route add 2001:db8:1::/64 \
+		via 2001:db8:2:1::1 dev veth0
+
+	# drop unreachable message when ping 2001:db8:3::2
+	tc -n "$ns_alpha" qdisc add dev veth0 clsact
+	tc -n "$ns_alpha" filter add dev veth0 ingress pref 1 \
+                protocol ipv6 flower skip_hw ip_proto icmpv6 type 1 action drop
+
+}
+
+check_tunnel()
+{
+	if ! lsmod 2>/dev/null | grep -q "ip6_tunnel" && \
+		! ip a 2>/dev/null | grep -q "ip6tnl0"; then
+		modprobe ip6_tunnel &>/dev/null
+		lsmod 2>/dev/null | grep -q "ip6_tunnel" || \
+			ip a 2>/dev/null | grep -q "ip6tnl0" || return 1
+		ip6tnl_loaded=1
+	fi
+
+	ip -netns "$ns_beta" link add name ip6tun0 type ip6tnl \
+		mode ip6ip6 encaplimit none \
+		local 2001:db8:2:1::1 remote 2001:db8:2:1::2 \
+		2>/dev/null || return 1
+	set_beta_ioam_encap || return 1
+}
+
+################################################################################
+#                                                                              #
+#                                     MAIN                                     #
+#                                                                              #
+################################################################################
+
+require_command ip
+require_command jq
+require_command ping
+require_command tc
+
+if [ ! -e /proc/sys/net/ipv4/fib_multipath_hash_seed ]; then
+	log_test_skip "Kernel without CONFIG_IP_ROUTE_MULTIPATH."
+	exit "${EXIT_STATUS}"
+fi
+
+trap cleanup EXIT
+setup_topology || { log_test_skip "Setup failed."; exit "${EXIT_STATUS}"; }
+
+# Compat check: can this kernel insert an IOAM trace into a route?
+if ! ip -netns "$ns_alpha" -6 route replace 2001:db8:2:1::/64 \
+	encap ioam6 mode inline trace prealloc type 0x800000 ns 0 size 12 \
+	via 2001:db8:1::1 dev veth0 &>/dev/null
+then
+	log_test_skip "Cannot attach an IOAM trace to a route"
+	exit "${EXIT_STATUS}"
+fi
+
+log_info "IOAM + ECMP (RFC 9486 C2, hash policy 0)"
+
+# Warm up ECMP links for neighbor/RA activity.
+set_alpha_plain
+ip netns exec "$ns_alpha" ping -6 -q -c "$PKTS" -i 0.2 2001:db8:2:1::2 &>/dev/null
+
+# Scenario 1 counts the ICMPv6 echo requests sent by alpha.
+if ! set_measure_filters "ip_proto icmpv6 type 128 src_ip 2001:db8:1::2"; then
+	log_test_skip "Cannot install tc flower measurement filters"
+	exit "${EXIT_STATUS}"
+fi
+run_scenario \
+	"Transit input ECMP with/without IOAM" \
+	set_alpha_plain set_alpha_ioam 2001:db8:2:1::2
+
+if check_tunnel; then
+	# Scenario 2 counts the outer ip6ip6 packets: the echo request is
+	# not visible to flower (outer headers only), so match the pinned
+	# tunnel addresses instead. No need to check return value again as
+	# we have done it at scenario 1
+	set_measure_filters "src_ip 2001:db8:2:1::1 dst_ip 2001:db8:2:1::2"
+
+	run_scenario \
+		"Encapsulating node re-lookup ECMP with/without IOAM" \
+		set_beta_tnl set_beta_ioam_encap 2001:db8:3::2
+else
+	log_test_skip "Encapsulating node re-lookup ECMP with/without IOAM (no ip6_tunnel)"
+fi
+
+exit "${EXIT_STATUS}"

-- 
2.56.0

Re: [PATCH net v2 1/3] ipv6: route: consider IOAM options in ECMP hash policy 0

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] ip6_nexthdr_without_ioam() (net/ipv6/route.c:2372-2405) steps
  through Hop-by-Hop TLVs with no limit on option count or padding.
- [Low] ip6_multipath_l3_keys() (net/ipv6/route.c:2437-2441) guesses
  whether skb->transport_header still points at the Hop-by-Hop header.
- [Low] The patch adds a permanent UAPI constant `#define
  IPV6_TLV_IOAM_DEX 17 /* RFC 9486 */` to include/uapi/linux/in6.h, and…
- [Low] ip6_nexthdr_without_ioam() (net/ipv6/route.c:2353-2408) says it
  recovers the nexthdr only if the HBH 'carried only IOAM options and…

--

ipv6: route: consider IOAM options in ECMP hash policy 0

RFC 9486 (C2) requires the same ECMP hash with and without IOAM options.
To meet it, this patch has the policy-0 IPv6 multipath hash use the next
header the packet would have without a Hop-by-Hop header that holds only
IOAM options and padding. It adds a TLV walker,
ip6_nexthdr_without_ioam(), applies it to both the outer header and the
inner header quoted by ICMPv6 errors, and adds an IPV6_TLV_IOAM_DEX UAPI
define.

> diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
> index 5a47339ef7d76..2e66dc37e1e50 100644
> --- a/include/uapi/linux/in6.h
> +++ b/include/uapi/linux/in6.h
> @@ -145,6 +145,7 @@ struct in6_flowlabel_req {
>  #define IPV6_TLV_PADN		1
>  #define IPV6_TLV_ROUTERALERT	5
>  #define IPV6_TLV_CALIPSO	7	/* RFC 5570 */
> +#define IPV6_TLV_IOAM_DEX	17	/* RFC 9486 */
>  #define IPV6_TLV_IOAM		49	/* RFC 9486 */

[Severity: Low]
This isn't a bug, but is IPV6_TLV_IOAM_DEX the right name for a permanent
UAPI constant?

In RFC 9486, IPv6 option types 0x11 and 0x31 are both the IOAM option. They
differ only in the chg bit. DEX is an IOAM-Option-Type carried inside the
option (struct ioam6_hdr->type, the IOAM6_TYPE_* values in
include/uapi/linux/ioam6.h). It is not an IPv6 TLV type.

The only user, ip6_nexthdr_without_ioam(), compares opt[0] against this
value and never looks at the IOAM-Option-Type. So it matches every 0x11
option, not just DEX:

    if (opt[0] != IPV6_TLV_IOAM && opt[0] != IPV6_TLV_IOAM_DEX)
        return nexthdr;

No userspace interface needs this value. Would a kernel-private define, or
a name that describes the chg=0 variant, fit better?

The commit message also doesn't mention that a UAPI define is being added.
It only says "0x11 for DEX".

[ ... ]

> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 616b86f89f637..675cf2e8ed353 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2342,6 +2342,71 @@ struct dst_entry *ip6_route_input_lookup(struct net *net,
>  }
>  EXPORT_SYMBOL_GPL(ip6_route_input_lookup);
>  
> +/* RFC 9486 (C2) requires that the ECMP hash of a packet carrying IOAM
> + * options is the same as the hash of the identical packet without them.
> + * Recover the nexthdr if its Hop-by-Hop header carried only IOAM options
> + * and padding, every other packet is hashed exactly as before.
> + *
> + * iph_off: offset of the IPv6 header from skb->data
> + * nexthdr: iph->nexthdr (NEXTHDR_HOP or not)
> + */
> +static u8 ip6_nexthdr_without_ioam(const struct sk_buff *skb,
> +				   int iph_off, u8 nexthdr)
> +{

[ ... ]

> +	off = iph_off + sizeof(struct ipv6hdr);
> +	hhdr = skb_header_pointer(skb, off, sizeof(hbuf), &hbuf);
> +	if (!hhdr)
> +		return nexthdr;
> +
> +	len = (hhdr->hdrlen + 1) << 3;
> +
> +	for (i = 2; i < len; ) {
> +		const u8 *type;
> +		u8 type_buf;
> +
> +		/* Always check PAD1 first as ip6_parse_tlv() accepts Pad1
> +		 * as the last byte of the HBH header
> +		 */
> +		type = skb_header_pointer(skb, off + i, 1, &type_buf);
> +		if (!type)
> +			return has_ioam ? hhdr->nexthdr : nexthdr;
> +		if (*type == IPV6_TLV_PAD1) {
> +			i++;
> +			continue;
> +		}

[Severity: Medium]
What limits the number of iterations of this loop?

Pad1 advances i by 1. A zero-length PadN or IOAM option advances it by 2.
The loop runs up to len = (hdrlen + 1) << 3, which can be 2048.

ip6_parse_tlv() limits this with max_hbh_opts_cnt and by rejecting padding
runs longer than 7 bytes. This walk has no such limit.

The walk also runs on headers that ip6_parse_tlv() never validated.
ip6_route_input() computes the hash for every ICMPv6 packet, whether or not
the route is multipath:

ip6_route_input()
    if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6))
        fl6.mp_hash = rt6_multipath_hash(net, &fl6, skb, flkeys);

With the default policy 0, this reaches the HBH quoted inside an ICMPv6
error:

rt6_multipath_hash()
  ip6_multipath_l3_keys()
    ip6_nexthdr_without_ioam(skb, inner_off, inner_iph->nexthdr)

The outer HBH is also walked past the IOAM option when ipv6_hop_ioam()
calls ip6_route_input() in the middle of ip6_parse_tlv().

A remote sender can send ICMPv6 errors (type 1-4) whose quoted inner IPv6
header has nexthdr 0, followed by an HBH with hdrlen 255 filled with Pad1.
No IOAM option is needed, so has_ioam stays false and the loop keeps
going.

At a 1500 MTU that is about 1400 iterations per packet. Each one does one
or two skb_header_pointer() calls. These fall back to skb_copy_bits() when
the bytes are in page frags, because only the IPv6 header has been pulled
before ip6_route_input().

Before this patch, ip6_multipath_l3_keys() did a fixed amount of work per
packet. Could the walk stop after a padding run longer than 7 bytes, or
after a small number of TLVs as ip6_parse_tlv() does, and return the
original nexthdr?

> +
> +		opt = skb_header_pointer(skb, off + i, sizeof(obuf), obuf);
> +		if (!opt)
> +			return nexthdr;

[Severity: Low]
Do these truncation exits match the "only IOAM options and padding" rule
in the comment above the function?

When has_ioam is already set, the two exits disagree. If the 1-byte read
fails, the recovered nexthdr is returned:

    type = skb_header_pointer(skb, off + i, 1, &type_buf);
    if (!type)
        return has_ioam ? hhdr->nexthdr : nexthdr;

If the 1-byte read succeeds but the 2-byte read above fails, NEXTHDR_HOP
is returned regardless of has_ioam.

A quoted inner HBH can be truncated. The same IOAM-only flow then hashes
with either proto 0 or the L4 proto, depending on whether zero or one byte
of the next TLV survived the cut.

Now take an original HBH of [IOAM ...][Router Alert] whose quote ends
before the Router Alert type byte. The error hashes with the L4 proto,
while the original flow hashes with proto 0.

The option length is also never checked against the remaining HBH length:

    i += opt[1] + 2;

An IOAM option whose length runs past len ends the loop with has_ioam set,
and the recovered nexthdr is returned. ip6_parse_tlv(), which the Pad1
comment refers to, rejects optlen > len.

icmp6_send() limits the quote, so this needs a quoted HBH longer than
about 1192 bytes, or a crafted error. Even so, the commit message says
"errors that quote a flow keep hashing that flow". Doesn't that now depend
on exactly where the quote was cut?

[ ... ]

> @@ -2349,15 +2414,34 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,

[ ... ]

> -	if (likely(outer_iph->nexthdr != IPPROTO_ICMPV6))
> +	nexthdr = ip6_nexthdr_without_ioam(skb, skb_network_offset(skb),
> +					   outer_iph->nexthdr);
> +	if (likely(nexthdr != IPPROTO_ICMPV6))
>  		goto out;
>  
> -	icmph = skb_header_pointer(skb, skb_transport_offset(skb),
> +	/* Outer header has IOAM (NEXTHDR_HOP) and the recovered nexthdr is
> +	 * IPPROTO_ICMPV6. transport_header may still point at the Hop-by-Hop
> +	 * header (e.g. ipv6_hop_ioam() -> ip6_route_input() before
> +	 * ipv6_parse_hopopts() advances it).
> +	 */
> +	if (outer_iph->nexthdr == NEXTHDR_HOP) {
> +		opt_hdr = skb_header_pointer(skb, skb_transport_offset(skb),
> +					     sizeof(_opt_hdr), &_opt_hdr);
> +		if (opt_hdr && opt_hdr->nexthdr == IPPROTO_ICMPV6)
> +			extlen = (opt_hdr->hdrlen + 1) << 3;
> +	}
> +
> +	icmph = skb_header_pointer(skb, skb_transport_offset(skb) + extlen,
>  				   sizeof(_icmph), &_icmph);

[Severity: Low]
Can this check misfire when transport_header has already been advanced
past the HBH?

On the normal forwarding path, ipv6_parse_hopopts() advances it before the
route lookup:

ip6_rcv_core()
  ipv6_parse_hopopts()
    skb->transport_header += extlen;
ip6_rcv_finish_core()
  ip6_route_input()
    fib6_select_path()
      rt6_multipath_hash()
        ip6_multipath_l3_keys()

Here flowi6_proto is NEXTHDR_HOP, so ip6_route_input() leaves mp_hash
unset and fib6_select_path() computes it. At that point, the two bytes
read as opt_hdr are actually the ICMPv6 type and code.

Take a forwarded packet whose HBH holds only IOAM options and padding,
with HBH nexthdr 58 and icmp6_type 58 (unassigned). The check passes and
extlen becomes (icmp6_code + 1) * 8.

The ICMPv6 header is then read from the payload. If that byte is 1..4,
icmpv6_is_err() returns true, and key_iph and ip_proto come from payload
bytes. The same packet without IOAM hashes its outer header with proto 58.

ip6_nexthdr_without_ioam() already knows the HBH length. Could the ICMPv6
offset be computed as skb_network_offset(skb) + sizeof(struct ipv6hdr) +
((hhdr->hdrlen + 1) << 3) instead of being guessed from transport_header?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103-ipv6_ioam_ecmp-v2-0-6ef0318bdb5e%40kylinos.cn

Re: [PATCH net v2 2/3] ipv6: ioam6: use transformed packet nexthdr for ECMP re-lookup

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The encap cache-miss path in ioam6_output()
  (net/ipv6/ioam6_iptunnel.c:439) now sets `fl6.mp_hash =…
- [High] In IOAM6_IPTUNNEL_MODE_AUTO, local packets (inline, !skb->dev)
  and forwarded packets (encap, skb->dev set) share one per-CPU…
- [Medium] The new call `rt6_multipath_hash(net, &fl6, skb, NULL)` in
  ioam6_output() (net/ipv6/ioam6_iptunnel.c:439) passes an output-path…

--

ipv6: ioam6: use transformed packet nexthdr for ECMP re-lookup

This commit changes the dst_cache miss path in ioam6_output(). For inline
insertion it now reuses orig_dst and caches the fake null_rt. For
encapsulation it re-looks up the route with flowi6_proto taken from the
HBH next header and with fl6.mp_hash precomputed from the skb, so that
the ECMP hash matches the identical packet without IOAM (RFC 9486 C2).

> diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
> index cfb2c41634a0b..246533276be82 100644
> --- a/net/ipv6/ioam6_iptunnel.c
> +++ b/net/ipv6/ioam6_iptunnel.c
> @@ -405,34 +405,61 @@ static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  
>  	if (unlikely(!dst)) {
>  		struct ipv6hdr *hdr = ipv6_hdr(skb);
> -		struct flowi6 fl6;

[ ... ]

> +		if (ilwt->mode == IOAM6_IPTUNNEL_MODE_INLINE ||
> +		    (ilwt->mode == IOAM6_IPTUNNEL_MODE_AUTO && !skb->dev)) {
> +			dst = orig_dst;
> +			dst_hold(dst);
> +
> +			local_bh_disable();
> +			dst_cache_set_ip6(&ilwt->cache, &ilwt->null_rt.dst,
> +					  &hdr->saddr);
> +			local_bh_enable();

[Severity: High]
In AUTO mode, can local and forwarded packets end up using each other's
cached dst? The miss path is now mode aware. The hit path at the top of
ioam6_output() still reads one per-CPU cache, whatever the mode:

	dst = dst_cache_get(&ilwt->cache);
	...
	if (dst == &ilwt->null_rt.dst) {
		dst_release(dst);

		dst = orig_dst;
		/* keep refcount balance: dst_release() is called at the end */
		dst_hold(dst);
	}

Suppose a local packet misses first, so null_rt is cached. A later
forwarded packet on that CPU then gets dst = orig_dst and is encapsulated
with daddr = tundst. Because dst is non-NULL, it skips the tundst
re-lookup. Because the lwtstates match, it leaves through
orig_dst->lwtstate->orig_output(). That uses the inner destination's
route and nexthop, not the route for tundst.

Now suppose a forwarded packet misses first. The encap branch caches the
tundst route:

	dst_cache_set_ip6(&ilwt->cache, dst, &fl6.saddr);

Later local packets on that CPU hit this entry and are processed inline.
They are then sent through the tundst route instead of orig_dst:

	if (orig_dst->lwtstate != dst->lwtstate) {
		skb_dst_drop(skb);
		skb_dst_set(skb, dst);
		return dst_output(net, sk, skb);
	}

The shared cache existed before this patch. However, the commit message
says inline will "reuse orig_dst and cache the fake null_rt, keeping the
sibling from the original lookup", and that encapsulation "still
re-looks up". In AUTO mode with mixed traffic, neither seems to hold.
null_rt is DST_OBSOLETE_NONE, so the stale entry stays until the cache is
reset.

Does the hit path need to be mode aware too, for example with separate
caches for the inline and encap cases?

> +		} else {
> +			/* Encapsulation may change the destination (tundst);
> +			 * re-lookup with the transformed packet keys so the
> +			 * outer hash matches the identical packet without IOAM.
> +			 */
> +			const struct ipv6_opt_hdr *hhdr;
> +			struct flowi6 fl6;
> +
> +			hhdr = (const struct ipv6_opt_hdr *)skb_transport_header(skb);
> +
> +			memset(&fl6, 0, sizeof(fl6));
> +			fl6.daddr = hdr->daddr;
> +			fl6.saddr = hdr->saddr;
> +			fl6.flowlabel = ip6_flowinfo(hdr);
> +			fl6.flowi6_mark = skb->mark;
> +			fl6.flowi6_proto = hhdr->nexthdr;
> +			fl6.mp_hash = rt6_multipath_hash(net, &fl6, skb, NULL);

[Severity: High]
Does computing mp_hash from the skb break the C2 equivalence that the
comment above claims? When fl6->mp_hash is non-zero, fib6_select_path()
uses it as is, so this skb-based value chooses the sibling.

The comment says the outer hash "matches the identical packet without
IOAM". A plain ip6ip6 tunnel, ip6_tnl_xmit(), routes with
ip6_route_output(net, NULL, fl6) and no skb, so it hashes the outer fl6
keys. With an skb, rt6_multipath_hash() behaves differently for each
policy.

Policy 1 returns the inner flow's hash early. ioam6_do_encap() never
clears that hash:

	if (skb->l4_hash)
		return skb_get_hash_raw(skb) >> 1;

Policy 2 dissects through the ip6ip6 header and hashes the inner
addresses, flowlabel and proto:

	skb_flow_dissect_flow_keys(skb, &keys, 0);

Policy 3 calls rt6_multipath_custom_hash_skb(), which returns
jhash_2words(mhash, mhash_inner, 0). Plain ip6ip6 goes through
rt6_multipath_custom_hash_fl6() instead.

For policy 0 with flowi6_proto = 41, the skb result looks the same as
the fl6-only hash, so the precompute appears to do nothing there.
Without the companion patch "ipv6: route: consider IOAM options in ECMP
hash policy 0", it would hash ip_proto = NEXTHDR_HOP and undo the
flowi6_proto = 41 change. That matters for backports of this
Fixes:-tagged patch.

Setting fl6.flowi6_proto = hhdr->nexthdr on its own, without the
precompute, seems to match plain ip6ip6 under every policy. Would it be
better to drop the mp_hash precompute?

The commit message also says:

    precompute fl6.mp_hash from the packet, since ip6_route_output()
    carries no skb and would otherwise drop ICMPv6 quoted-flow and L4
    keys.

Can the ICMPv6 quoted-flow case happen here? ioam6_do_encap() always sets
tuninfo->eh.nexthdr = NEXTHDR_IPV6, so the header after the HBH is
always 41.

[Severity: Medium]
Can this call run the flow dissector without a netns? Under policy 2,
policy 3, and policy 1 when skb->l4_hash is clear, rt6_multipath_hash()
calls skb_flow_dissect_flow_keys(), which passes net = NULL.
__skb_flow_dissect() then takes the netns only from skb->dev or skb->sk:

	if (skb) {
		if (!net) {
			if (skb->dev)
				net = dev_net_rcu(skb->dev);
			else if (skb->sk)
				net = sock_net(skb->sk);
		}
	}

	DEBUG_NET_WARN_ON_ONCE(!net);

In ENCAP mode, locally generated packets can reach this branch with both
fields NULL. One example is an RST sent for a closed port:

tcp_v6_send_reset(NULL, ...)
  tcp_v6_send_response()
    ip6_xmit(ctl_sk, buff, ...)
      NF_HOOK(..., dst_output)
        lwtunnel_output()
          ioam6_output()
            rt6_multipath_hash(net, &fl6, skb, NULL)

When sk is NULL, buff comes from alloc_skb() with no socket attached.
ip6_xmit() sets skb->dev only on the EMSGSIZE path, after NF_HOOK has
returned.

On CONFIG_DEBUG_NET kernels this warns on the first cache miss on each
CPU. On all kernels, an attached netns BPF flow dissector is skipped for
these packets. Before this patch, ioam6_output() never passed an skb to
the multipath hash.

> +
> +			dst = ip6_route_output(net, NULL, &fl6);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103-ipv6_ioam_ecmp-v2-0-6ef0318bdb5e%40kylinos.cn

Re: [PATCH net v2 3/3] selftests: net: add ioam6 ECMP test (RFC 9486 C2)

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] check_tunnel() (lines 276-281) decides whether ip6_tunnel is
  available from two checks: `lsmod | grep ip6_tunnel`, and `ip a | grep…
- [Low] The header of tools/testing/selftests/net/ioam6_ecmp.sh (lines
  8-9) says there are "Two scenarios, matching the two places where IOAM…
- [Low] The header (line 44) says "The filters match the exact
  measurement packets".

--

selftests: net: add ioam6 ECMP test (RFC 9486 C2)

This adds ioam6_ecmp.sh. Under hash policy 0, it checks that a packet
carrying IOAM options takes the same ECMP nexthop as the same packet
without them. It covers transit input hashing after inline IOAM
insertion, and the re-lookup on a node that encapsulates with IOAM over a
multipath route, with a plain ip6ip6 tunnel as the baseline.

> diff --git a/tools/testing/selftests/net/ioam6_ecmp.sh b/tools/testing/selftests/net/ioam6_ecmp.sh
> new file mode 100755
> index 0000000000000..35a11f94c4223
> --- /dev/null
> +++ b/tools/testing/selftests/net/ioam6_ecmp.sh
> @@ -0,0 +1,348 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Verify RFC 9486 C2 for ECMP: with hash policy 0 (the default), the ECMP
> +# nexthop chosen for a packet carrying IOAM options must be the same as for
> +# the identical packet without them.
> +#
> +# Two scenarios, matching the two places where IOAM changed the hashed
> +# IPv6 nexthdr:

[Severity: Low]
Is "the two places" accurate here? The series patch "ipv6: ioam6: use
transformed packet nexthdr for ECMP re-lookup" changes a third place too:
the inline path (and the AUTO path with !skb->dev) of ioam6_output() on
the node that inserts the IOAM option.

net/ipv6/ioam6_iptunnel.c:ioam6_output() {
    ...
		if (ilwt->mode == IOAM6_IPTUNNEL_MODE_INLINE ||
		    (ilwt->mode == IOAM6_IPTUNNEL_MODE_AUTO && !skb->dev)) {
			dst = orig_dst;
    ...
}

Before that change, a dst cache miss re-ran ip6_route_output() with
fl6.flowi6_proto = hdr->nexthdr. After inline insertion that is
NEXTHDR_HOP, so the inserting node itself could pick a different ECMP
sibling.

In this test the inserting node is alpha (set_alpha_ioam()). Alpha only
gets single-path routes from setup_topology():

	ip -netns "$ns_alpha" -6 route add 2001:db8:2:1::/64 \
		via 2001:db8:1::1 dev veth0

Wouldn't this test still pass if the orig_dst reuse branch regressed?
Could a scenario with an ECMP route on the inserting node be added? If
not, could the comment be reworded so it doesn't suggest every affected
place is covered?

[ ... ]

> +check_tunnel()
> +{
> +	if ! lsmod 2>/dev/null | grep -q "ip6_tunnel" && \
> +		! ip a 2>/dev/null | grep -q "ip6tnl0"; then
> +		modprobe ip6_tunnel &>/dev/null
> +		lsmod 2>/dev/null | grep -q "ip6_tunnel" || \
> +			ip a 2>/dev/null | grep -q "ip6tnl0" || return 1

[Severity: Medium]
Can this heuristic skip scenario 2 even when ip6ip6 tunnels work?

With CONFIG_IPV6_TUNNEL=y, ip6_tunnel never appears in lsmod.

The ip6tnl0 fallback device is also missing from the runner's netns in
two cases: net.core.fb_tunnels_only_for_init_net=2, or the sysctl is 1
and the runner is not in init_net.

include/linux/netdevice.h:net_has_fallback_tunnels() {
    ...
	return !fb_tunnels_only_for_init_net ||
		(net_eq(net, &init_net) && fb_tunnels_only_for_init_net == 1);
    ...
}

In that setup modprobe ip6_tunnel does nothing and both greps fail again.
check_tunnel() then returns 1 before it reaches the ip link add probe
below, which would work (and would also autoload a modular ip6_tunnel).

Main then calls log_test_skip() for scenario 2. ksft_exit_status_merge()
ranks skip above pass, so the whole test exits with SKIP even after
scenario 1 passed.

Could check_tunnel() rely on the ip link add probe instead?

> +		ip6tnl_loaded=1
> +	fi
> +
> +	ip -netns "$ns_beta" link add name ip6tun0 type ip6tnl \
> +		mode ip6ip6 encaplimit none \
> +		local 2001:db8:2:1::1 remote 2001:db8:2:1::2 \
> +		2>/dev/null || return 1
> +	set_beta_ioam_encap || return 1
> +}

[ ... ]

> +if check_tunnel; then
> +	# Scenario 2 counts the outer ip6ip6 packets: the echo request is
> +	# not visible to flower (outer headers only), so match the pinned
> +	# tunnel addresses instead. No need to check return value again as
> +	# we have done it at scenario 1
> +	set_measure_filters "src_ip 2001:db8:2:1::1 dst_ip 2001:db8:2:1::2"

[Severity: Low]
The header comment says "The filters match the exact measurement
packets". Does this filter match only the ip6ip6 packets, though?

It matches any IPv6 packet from beta's veth1 address to gamma's veth0
address. Beta's unicast NUD probes use that same pair, because
ndisc_solicit() takes the source address from the queued skb:

net/ipv6/ndisc.c:ndisc_solicit() {
    ...
	if (skb && ipv6_chk_addr_and_flags(dev_net(dev), &ipv6_hdr(skb)->saddr,
					   dev, false, 1,
					   IFA_F_TENTATIVE|IFA_F_OPTIMISTIC))
		saddr = &ipv6_hdr(skb)->saddr;
    ...
		ndisc_send_ns(dev, target, target, saddr, 0);
    ...
}

Beta's Neighbor Advertisements to gamma also match. Gamma sends
solicitations when it has traffic for 2001:db8:2:1::1, for example
ICMPv6 errors for ip6ip6 packets it cannot decapsulate.

These packets add to the veth0 count in measure_link():

	if [ $((da + db)) -lt "$PKTS" ]; then
		echo "LOST:$da:$db"

With PKTS=3, two tunnel packets plus one ND packet already reach the
threshold, so a lost tunnel packet goes unreported. The stray packets
also count in the A/B majority vote.

Could the filter also match the encapsulated protocol, or otherwise
exclude ICMPv6/ND, so it is as exact as the comment says?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103-ipv6_ioam_ecmp-v2-0-6ef0318bdb5e%40kylinos.cn