[PATCH net-next v2 0/3] ipv4/ipv6: Cache pmtu on all paths of a multipath route

[PATCH net-next v2 0/3] ipv4/ipv6: Cache pmtu on all paths of a multipath route

[Vladimir Vdovin]

IPv6 counterpart of commit 7d3f3b4367f3 ("net: ipv4: Cache pmtu for
all packet paths if multipath enabled"), deferred back then:
https://lore.kernel.org/netdev/ad29493c-9e48-4a94-b327-e47af3a9d137@kernel.org/

Patch 1 keeps the IPv4 fan-out from raising a pmtu a path already has,
patch 2 creates or updates the PMTU exception on every path of an IPv6
multipath route, patch 3 adds the IPv6 variant of the pmtu.sh multipath
test and multipath tests with paths of different MTU for both families.
The IPv6 tests fail without patch 2.

Changes in v2, after the Sashiko review of v1:
- Update the PMTU exception a path already has instead of replacing
  it, so a lower pmtu or a gateway learned from a redirect on another
  path is kept. The pmtu is now propagated to the other paths on every
  Packet Too Big, not only on the first one, when the looked up dst is
  still a plain route and not a cached exception yet.
- Don't walk the siblings of a fib6_info that is no longer in the
  tree: after NLM_F_REPLACE the old route and its siblings are freed
  with the sibling list still linked.
- Describe the real mechanism in the commit message: ip6_update_pmtu()
  looks the destination up without the protocol and ports of the
  offending packet, so the multipath hash may pick another path than
  the one that packet took.
- New patch 1: the same "a cached pmtu never goes up" rule for the IPv4
  fan-out in update_or_create_fnhe().
- Add selftests with a different MTU behind each router, so a second,
  lower Packet Too Big is covered as well.
- Rebased on net-next.
v1: https://lore.kernel.org/netdev/20261017210508.71849-1-deliran@verdict.gg/

Vladimir Vdovin (3):
  ipv4: Don't raise a cached pmtu from another path's Packet Too Big
  ipv6: Cache pmtu for all paths of a multipath route
  selftests: net: pmtu: Add IPv6 and asymmetric multipath tests

 net/ipv4/route.c                    |   8 +
 net/ipv6/route.c                    | 124 ++++++++-----
 tools/testing/selftests/net/pmtu.sh | 276 ++++++++++++++++++++++++----
 3 files changed, 328 insertions(+), 80 deletions(-)


base-commit: d26955e57475434b59d43ba372b2a50d33d5e0ed
-- 
2.56.0

[PATCH net-next v2 1/3] ipv4: Don't raise a cached pmtu from another path's Packet Too Big

[Vladimir Vdovin]

Since commit 7d3f3b4367f3 ("net: ipv4: Cache pmtu for all packet paths
if multipath enabled") __ip_rt_update_pmtu() updates the exception of
every path of a multipath route, but update_or_create_fnhe() overwrites
whatever pmtu a path already has. A Packet Too Big from a wider path
thus raises the pmtu a narrower path has learned, and flows on that path
blackhole until it gets its own Packet Too Big again.

The early return in __ip_rt_update_pmtu() already keeps a pmtu from going
up on the path the Packet Too Big was looked up on. Do the same for the
other paths: leave an unexpired exception with a lower pmtu alone.

Signed-off-by: Vladimir Vdovin <deliran@verdict.gg>
---
 net/ipv4/route.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d0a5f6d77094..596af782c1fb 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -676,6 +676,14 @@ static void update_or_create_fnhe(struct fib_nh_common *nhc, __be32 daddr,
 	}
 
 	if (fnhe) {
+		/* __ip_rt_update_pmtu() caches a pmtu on every path of a
+		 * multipath route, don't let a Packet Too Big from one path
+		 * raise the pmtu another path already learned.
+		 */
+		if (pmtu && fnhe->fnhe_genid == genid && fnhe->fnhe_pmtu &&
+		    pmtu > fnhe->fnhe_pmtu &&
+		    time_before(jiffies, fnhe->fnhe_expires))
+			goto out_unlock;
 		if (fnhe->fnhe_genid != genid)
 			fnhe->fnhe_genid = genid;
 		if (gw)
-- 
2.56.0

Re: [PATCH net-next v2 1/3] ipv4: Don't raise a cached pmtu from another path's Packet Too Big

[netdev-bot+sashiko]

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

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

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] The fix does not do "the same" as the early return in
  __ip_rt_update_pmtu(), as the commit message claims.
- [Medium] The new early exit in update_or_create_fnhe() ignores the
  `lock` argument, so it can drop an MTU-lock transition.
- [Low] Missing Fixes: tag.
- [Low] The commit message presents the raise as the normal result of a
  wider path's Fragmentation Needed ("update_or_create_fnhe() overwrites…
- [Low] No selftest exercises the new no-raise check in
  update_or_create_fnhe().
- [Low] The new comment in update_or_create_fnhe() (route.c:679-682)
  explains the check as stopping "a Packet Too Big from one path" from…

--

ipv4: Don't raise a cached pmtu from another path's Packet Too Big

This commit adds a check to update_or_create_fnhe(). If an existing
exception is unexpired, has the same genid and has a lower cached pmtu
than the new value, the check leaves it alone. The goal is to stop the
multipath loop in __ip_rt_update_pmtu(), added by 7d3f3b4367f3, from
raising a narrower path's learned pmtu with a value from a wider path.

> Since commit 7d3f3b4367f3 ("net: ipv4: Cache pmtu for all packet paths
> if multipath enabled") __ip_rt_update_pmtu() updates the exception of
> every path of a multipath route, but update_or_create_fnhe() overwrites
> whatever pmtu a path already has. A Packet Too Big from a wider path
> thus raises the pmtu a narrower path has learned, and flows on that path
> blackhole until it gets its own Packet Too Big again.

[Severity: Low]
The message names 7d3f3b4367f3 as the origin of this behaviour. Should
there be a Fixes tag for it?

Fixes: 7d3f3b4367f3 ("net: ipv4: Cache pmtu for all packet paths if multipath enabled")

That commit is in released kernels. Without the tag, stable and backport
tooling may not connect this change to the trees that carry the multipath
loop in __ip_rt_update_pmtu().

> The early return in __ip_rt_update_pmtu() already keeps a pmtu from going
> up on the path the Packet Too Big was looked up on. Do the same for the
> other paths: leave an unexpired exception with a lower pmtu alone.

[Severity: Low]
Can the sequence in the first paragraph happen in normal operation?

When the narrower path's ICMP (say 1300) is processed, the multipath loop
in __ip_rt_update_pmtu() writes 1300, with the same expiry, into the
exception of every path:

    for (nhsel = 0; nhsel < fib_info_num_path(res.fi); nhsel++) {
        nhc = fib_info_nhc(res.fi, nhsel);
        ...
        update_or_create_fnhe(nhc, fl4->daddr, 0, mtu, lock,
                              jiffies + net->ipv4.ip_rt_mtu_expires);
    }

fill_route_from_fnhe() then updates each bound dst. The wider path's dst
also reports ipv4_mtu() == 1300, so a later, larger ICMP for it is dropped
by the existing early return:

    old_mtu = ipv4_mtu(dst);
    if (old_mtu < mtu)
        return;

So the raise only seems possible once the paths' exceptions have
diverged. That could happen through:

- fnhe_remove_oldest() evicting the exception on one nexthop
- a GFP_ATOMIC allocation failure
- a per-device MTU change via fib_nhc_update_mtu() and fnhe_update_pmtu()
- a nexthop shared between routes in different tables

Could the commit message say which of these scenarios is being fixed?

The message also says the change is for "the other paths", but the check
is in the shared helper. It applies to the single-path call as well:

    nhc = FIB_RES_NHC(res);
    update_or_create_fnhe(nhc, fl4->daddr, 0, mtu, lock,
                          jiffies + net->ipv4.ip_rt_mtu_expires);

It also applies to builds without CONFIG_IP_ROUTE_MULTIPATH. So
non-multipath behaviour changes whenever the dst passed in isn't bound to
the exception, for example when ip_rt_update_pmtu() clears flowi4_oif for
bridged encapsulations. Should the message mention this?

> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index d0a5f6d77094c..596af782c1fbb 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -676,6 +676,14 @@ static void update_or_create_fnhe(struct fib_nh_common *nhc, __be32 daddr,
>  	}
>  
>  	if (fnhe) {
> +		/* __ip_rt_update_pmtu() caches a pmtu on every path of a
> +		 * multipath route, don't let a Packet Too Big from one path
> +		 * raise the pmtu another path already learned.
> +		 */

[Severity: Low]
This isn't a bug, but the comment describes the check as one path versus
another on a multipath route. update_or_create_fnhe() has no idea which
path an update came from. The check applies to every caller that passes
a non-zero pmtu, including the single-path call in __ip_rt_update_pmtu()
and the nexthop the ICMP was actually looked up on. Could the comment
describe the real scope?

Also, "Packet Too Big" is the ICMPv6 message name and doesn't appear
anywhere else in net/ipv4/. On IPv4, updates reach this code from ICMP
Fragmentation Needed and from non-ICMP callers such as tunnel PMTU
handling. The subject and commit message use the same term.

> +		if (pmtu && fnhe->fnhe_genid == genid && fnhe->fnhe_pmtu &&
> +		    pmtu > fnhe->fnhe_pmtu &&
> +		    time_before(jiffies, fnhe->fnhe_expires))
> +			goto out_unlock;

[Severity: Medium]
Does this really do the same as the early return in
__ip_rt_update_pmtu(), as the commit message says?

The early return compares the new mtu with ipv4_mtu() of the looked-up
dst, which includes the device MTU. This check only fires when a sibling
nexthop already has an exception with a non-zero, same-genid, unexpired
fnhe_pmtu.

Some siblings still get the other path's value, with no comparison
against their own device MTU:

- a sibling with no exception (the else branch below)
- a redirect-only exception (fnhe_pmtu == 0)
- an expired exception

fill_route_from_fnhe() copies that value into rt_pmtu, and
ip_dst_mtu_maybe_forward() uses it before falling back to dev->mtu:

    mtu = rt->rt_pmtu;
    if (mtu && time_before(jiffies, READ_ONCE(rt->dst.expires)))
        goto out;

For example, take an ECMP route with path A on a 9000 MTU device, path B
on a 1400 MTU device, and no exception on B. A Fragmentation Needed of
1500 for a flow on A passes A's early return (9000 >= 1500), and the loop
creates an exception on B with pmtu 1500.

Local flows hashed to B then size packets for 1500. That comes from TCP
MSS via dst_mtu() and from the skb->len > mtu check in
__ip_finish_output(). These packets are larger than B's device MTU, so
they are dropped, and no ICMP comes back to correct it. This lasts until
ip_rt_mtu_expires and happens again each time A learns a new PMTU.

The sibling-path raise itself came in with 7d3f3b4367f3. Since this patch
sets out to stop it, can one path's PMTU update still raise another
path's effective MTU and blackhole it this way?

[Severity: Medium]
Can this early exit drop an MTU lock transition?

The check compares only the pmtu values, and it skips the
fnhe_mtu_locked = lock assignment below. In __ip_rt_update_pmtu(), an mtu
below ip_rt_min_pmtu is clamped up using the looked-up dst's MTU, and
that value is sent to every nexthop with lock set:

    if (mtu < net->ipv4.ip_rt_min_pmtu) {
        lock = true;
        mtu = min(old_mtu, net->ipv4.ip_rt_min_pmtu);
    }

The looked-up dst can belong to a different path than the affected flow.
__build_flow_key() passes 0, 0 for the ports to flowi4_init_output(), so
with fib_multipath_hash_policy=1 the lookup can pick path X while the
flow uses path B.

B can have an unexpired, unlocked exception with fnhe_pmtu below
ip_rt_min_pmtu. fnhe_update_pmtu() leaves it that way after B's device
MTU drops:

    if (fnhe->fnhe_mtu_locked) {
        if (new <= fnhe->fnhe_pmtu) {
            fnhe->fnhe_pmtu = new;
            fnhe->fnhe_mtu_locked = false;
        }
    } else if (new < fnhe->fnhe_pmtu || orig == fnhe->fnhe_pmtu) {
        fnhe->fnhe_pmtu = new;
    }

Raising the min_pmtu sysctl after a pmtu was learned has the same effect.

Say min_pmtu=1000, B is unlocked at 600, and X's effective MTU is 1500. A
Fragmentation Needed of 300 for B's flow gives mtu = min(1500, 1000) =
1000 with lock = true. For B, 1000 > 600 and the exception is unexpired,
so the code jumps to out_unlock.

B stays at 600 with fnhe_mtu_locked false. DF stays set while the next
hop only passes 300, so B's flows blackhole. X is now locked, so later
ICMPs that resolve to X hit this:

    if (ip_mtu_locked(dst))
        return;

B isn't corrected until its exception expires. Before this patch, B was
overwritten with 1000 and locked, so DF was cleared and the traffic was
fragmented and delivered.

Should the check only skip when !lock || fnhe->fnhe_mtu_locked? Or could
it set the lock without raising fnhe_pmtu?

[Severity: Low]
Does any selftest reach this check?

pmtu_ipv4_mp_exceptions in tools/testing/selftests/net/pmtu.sh uses 1500
on both legs, so pmtu > fnhe->fnhe_pmtu is never true.

pmtu_ipv4_mp_asym_exceptions comes later in the series ("selftests: net:
pmtu: Add IPv6 and asymmetric multipath tests"). It sends an 1800 byte
ping via R1, which creates both exceptions at 1400 through the else
branch. It then sends a 1350 byte ping via R2, which lowers both to 1300.
Neither step reaches the check.

Both IPv4 tests give the same result with or without this patch. They
would still pass if the check were removed or inverted. The selftest
commit message only says that the IPv6 tests fail without the preceding
fix.

Could a test case be added where a narrower path's unexpired exception
is offered a larger value from another path and has to keep its lower
pmtu?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110095903.84813-1-deliran%40verdict.gg

[PATCH net-next v2 2/3] ipv6: Cache pmtu for all paths of a multipath route

[Vladimir Vdovin]

ip6_update_pmtu() looks the destination up without the protocol and
ports the offending packet was hashed on, so with a multipath route the
exception may be created on another path than the one the packet took.
The flow then keeps sending too big packets on its path, every Packet
Too Big lands on the same wrong path, and the flow blackholes.

Same problem and same fix as for IPv4 in commit 7d3f3b4367f3 ("net:
ipv4: Cache pmtu for all packet paths if multipath enabled"): cache the
pmtu on every path of the route. For a nexthop object walk all of its
fib6_nh, for a legacy multipath route walk the siblings of the fib6_info
the dst came from. A path that already has an exception for the
destination gets it updated in place, so its gateway from a redirect
stays and its pmtu never goes up, and that also covers a dst that is
itself a cached exception, which used to be updated alone.

Link: https://lore.kernel.org/netdev/ad29493c-9e48-4a94-b327-e47af3a9d137@kernel.org/
Signed-off-by: Vladimir Vdovin <deliran@verdict.gg>
---
 net/ipv6/route.c | 124 +++++++++++++++++++++++++++++++----------------
 1 file changed, 82 insertions(+), 42 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 04098cf32eab..dc982400fbab 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2911,12 +2911,52 @@ static bool rt6_cache_allowed_for_pmtu(const struct rt6_info *rt)
 		(rt->rt6i_flags & RTF_PCPU || rcu_access_pointer(rt->from));
 }
 
+struct rt6_pmtu_arg {
+	struct fib6_result	res;
+	const struct in6_addr	*daddr;
+	const struct in6_addr	*saddr;
+	u32			mtu;
+};
+
+/* Cache a pmtu on one path of a route: update the exception the path already
+ * has for daddr, keeping its gateway and never raising its pmtu, or create one.
+ */
+static int rt6_nh_update_pmtu(struct fib6_nh *nh, void *_arg)
+{
+	struct rt6_pmtu_arg *arg = _arg;
+	struct fib6_result *res = &arg->res;
+	struct rt6_info *rt;
+
+	res->nh = nh;
+	rt = rt6_find_cached_rt(res, arg->daddr, arg->saddr);
+	if (rt) {
+		u32 mtu = dst_metric_raw(&rt->dst, RTAX_MTU);
+
+		if (!mtu || arg->mtu < mtu) {
+			rt6_do_update_pmtu(rt, arg->mtu);
+			fib6_nh_update_exception(nh, res->f6i->fib6_src.plen, rt);
+		}
+		return 0;
+	}
+
+	rt = ip6_rt_cache_alloc(res, arg->daddr, arg->saddr);
+	if (rt) {
+		rt6_do_update_pmtu(rt, arg->mtu);
+		if (rt6_insert_exception(rt, res))
+			dst_release_immediate(&rt->dst);
+	}
+
+	return 0;
+}
+
 static void __ip6_rt_update_pmtu(struct dst_entry *dst, const struct sock *sk,
 				 const struct ipv6hdr *iph, u32 mtu,
 				 bool confirm_neigh)
 {
-	const struct in6_addr *daddr, *saddr;
+	struct rt6_pmtu_arg arg = { .mtu = mtu };
 	struct rt6_info *rt6 = dst_rt6_info(dst);
+	const struct in6_addr *daddr, *saddr;
+	struct fib6_info *f6i, *sibling;
 
 	/* Note: do *NOT* check dst_metric_locked(dst, RTAX_MTU)
 	 * IPv6 pmtu discovery isn't optional, so 'mtu lock' cannot disable it.
@@ -2942,52 +2982,52 @@ static void __ip6_rt_update_pmtu(struct dst_entry *dst, const struct sock *sk,
 	if (mtu >= dst6_mtu(dst))
 		return;
 
-	if (!rt6_cache_allowed_for_pmtu(rt6)) {
-		rt6_do_update_pmtu(rt6, mtu);
-		/* update rt6_ex->stamp for cache */
-		if (rt6->rt6i_flags & RTF_CACHE)
-			rt6_update_exception_stamp_rt(rt6);
-	} else if (daddr) {
-		struct fib6_result res = {};
-		struct rt6_info *nrt6;
-
-		rcu_read_lock();
-		res.f6i = rcu_dereference(rt6->from);
-		if (!res.f6i)
-			goto out_unlock;
-
-		res.fib6_flags = res.f6i->fib6_flags;
-		res.fib6_type = res.f6i->fib6_type;
-
-		if (res.f6i->nh) {
-			struct fib6_nh_match_arg arg = {
-				.dev = dst_dev_rcu(dst),
-				.gw = &rt6->rt6i_gateway,
-			};
+	rcu_read_lock();
+	f6i = rcu_dereference(rt6->from);
+	/* fib6_del_route() and a NLM_F_REPLACE clear fib6_node before the
+	 * route and its siblings are released, don't walk them then.
+	 */
+	if (!daddr || !f6i || !rcu_access_pointer(f6i->fib6_node)) {
+		if (!rt6_cache_allowed_for_pmtu(rt6)) {
+			rt6_do_update_pmtu(rt6, mtu);
+			/* update rt6_ex->stamp for cache */
+			if (rt6->rt6i_flags & RTF_CACHE)
+				rt6_update_exception_stamp_rt(rt6);
+		}
+		goto out;
+	}
 
-			nexthop_for_each_fib6_nh(res.f6i->nh,
-						 fib6_nh_find_match, &arg);
+	arg.daddr = daddr;
+	arg.saddr = saddr;
+	arg.res.f6i = f6i;
+	arg.res.fib6_flags = f6i->fib6_flags;
+	arg.res.fib6_type = f6i->fib6_type;
 
-			/* fib6_info uses a nexthop that does not have fib6_nh
-			 * using the dst->dev + gw. Should be impossible.
-			 */
-			if (!arg.match)
-				goto out_unlock;
+	/* ip6_update_pmtu() looks dst up without the protocol and ports the
+	 * too big packet was hashed on, so dst may be another path of a
+	 * multipath route than the one that packet took. Cache the pmtu on
+	 * every path, or that flow would keep sending too big packets.
+	 */
+	if (f6i->nh) {
+		nexthop_for_each_fib6_nh(f6i->nh, rt6_nh_update_pmtu, &arg);
+		goto out;
+	}
 
-			res.nh = arg.match;
-		} else {
-			res.nh = res.f6i->fib6_nh;
-		}
+	rt6_nh_update_pmtu(f6i->fib6_nh, &arg);
 
-		nrt6 = ip6_rt_cache_alloc(&res, daddr, saddr);
-		if (nrt6) {
-			rt6_do_update_pmtu(nrt6, mtu);
-			if (rt6_insert_exception(nrt6, &res))
-				dst_release_immediate(&nrt6->dst);
-		}
-out_unlock:
-		rcu_read_unlock();
+	/* fib6_del_route() zeroes fib6_nsiblings before unlinking f6i, stop
+	 * then as the walk would never get back to its head.
+	 */
+	list_for_each_entry_rcu(sibling, &f6i->fib6_siblings, fib6_siblings) {
+		arg.res.f6i = sibling;
+		arg.res.fib6_flags = sibling->fib6_flags;
+		arg.res.fib6_type = sibling->fib6_type;
+		rt6_nh_update_pmtu(sibling->fib6_nh, &arg);
+		if (!READ_ONCE(f6i->fib6_nsiblings))
+			break;
 	}
+out:
+	rcu_read_unlock();
 }
 
 static void ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
-- 
2.56.0

Re: [PATCH net-next v2 2/3] ipv6: Cache pmtu for all paths of a multipath route

[netdev-bot+sashiko]

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

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

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 in-place update branch of rt6_nh_update_pmtu() can raise
  another path's effective PMTU above that path's link MTU.
- [High] The fix does not close the blackhole it describes.
- [Medium] A passed-in RTF_CACHE dst is no longer updated when the daddr
  used by the walk is not that dst's exception key.
- [Low] No Fixes: tag, even though the commit message presents the change
  as a fix for a user-visible blackhole.

Pre-existing issues:
- [Low] Reported concern: rt6_nh_update_pmtu() reads and then writes an
  exception's RTAX_MTU under only rcu_read_lock().

--

ipv6: Cache pmtu for all paths of a multipath route

This patch changes __ip6_rt_update_pmtu() so that a Packet Too Big pmtu is
cached on every path of a multipath route. It walks every fib6_nh of a
nexthop object, or the fib6_siblings of a legacy multipath route. A path
that already has an exception for the destination is updated in place by
the new rt6_nh_update_pmtu() helper; a path without one gets a new
exception.

Link: https://lore.kernel.org/netdev/ad29493c-9e48-4a94-b327-e47af3a9d137@kernel.org/

> The flow then keeps sending too big packets on its path, every Packet
> Too Big lands on the same wrong path, and the flow blackholes.

[Severity: Low]
This isn't a bug, but the change is described as fixing a user-visible
blackhole. Should it carry a Fixes: tag?

The IPv4 counterpart, commit 7d3f3b4367f3 ("net: ipv4: Cache pmtu for all
packet paths if multipath enabled"), went through net-next without one.
So treating this as a behaviour extension rather than a stable fix may be
intentional.

> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 04098cf32eabe..dc982400fbab2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2911,12 +2911,52 @@ static bool rt6_cache_allowed_for_pmtu(const struct rt6_info *rt)

[ ... ]

> +/* Cache a pmtu on one path of a route: update the exception the path already
> + * has for daddr, keeping its gateway and never raising its pmtu, or create one.
> + */
> +static int rt6_nh_update_pmtu(struct fib6_nh *nh, void *_arg)
> +{
> +	struct rt6_pmtu_arg *arg = _arg;
> +	struct fib6_result *res = &arg->res;
> +	struct rt6_info *rt;
> +
> +	res->nh = nh;
> +	rt = rt6_find_cached_rt(res, arg->daddr, arg->saddr);
> +	if (rt) {
> +		u32 mtu = dst_metric_raw(&rt->dst, RTAX_MTU);
> +
> +		if (!mtu || arg->mtu < mtu) {
> +			rt6_do_update_pmtu(rt, arg->mtu);

[Severity: High]
Can this raise a path's effective pmtu above that path's own link MTU?

A redirect exception normally has a raw RTAX_MTU of 0, because
rt6_do_redirect()->ip6_rt_cache_alloc() inherits the route's metrics.
Raw 0 does not mean unlimited, though. ip6_dst_mtu_maybe_forward() falls
back to the device mtu6:

include/net/ip6_route.h:ip6_dst_mtu_maybe_forward() {
    ...
		mtu = dst_metric_raw(dst, RTAX_MTU);
		if (mtu)
			goto out;
    ...
		mtu = READ_ONCE(idev->cnf.mtu6);
    ...
}

The only upper bound on arg->mtu is this check in __ip6_rt_update_pmtu():

	if (mtu >= dst6_mtu(dst))
		return;

That check is made against the dst that was looked up, not against the
path being written here.

For example, path A is on a 9000 MTU device and path B is on a 1500 MTU
device, and B has a redirect exception for daddr. A Packet Too Big of 8000
that resolves to A passes the check, and the walk then sets RTAX_MTU to
8000 on B's exception. ip6_mtu() for B now returns 8000, so frames larger
than B's device MTU are dropped until ip6_rt_mtu_expires.

The create branch is protected by rt6_insert_exception():

	if (dst_metric_raw(&nrt->dst, RTAX_MTU) >= fib6_mtu(res)) {
		err = -EINVAL;
		goto out;
	}

The in-place branch has no equivalent check. This also seems to
contradict "its pmtu never goes up" in the commit message and in the
comment above.

Does this branch also need a bound such as arg->mtu < fib6_mtu(res)?

[Severity: Low]
This is a pre-existing issue, but the read of RTAX_MTU and the
rt6_do_update_pmtu() call here run under rcu_read_lock() only.

Two concurrent Packet Too Big messages (say 1300 and 1400) could leave
1400 behind. The update could also race with rt6_exceptions_update_pmtu(),
which holds rt6_exception_lock.

The old __ip6_rt_update_pmtu() already did the same unlocked
compare-then-set on RTF_CACHE dsts. A value left too high would also be
lowered by the next Packet Too Big, so the impact looks limited to one
extra round trip.

> +			fib6_nh_update_exception(nh, res->f6i->fib6_src.plen, rt);
> +		}
> +		return 0;
> +	}

[ ... ]

> @@ -2942,52 +2982,52 @@ static void __ip6_rt_update_pmtu(struct dst_entry *dst, const struct sock *sk,
>  	if (mtu >= dst6_mtu(dst))
>  		return;

[Severity: High]
Does this early return still allow the blackhole the commit message
describes?

Here dst is whatever ip6_update_pmtu() found with a lookup that has no
protocol or ports. It can be path B while the flow is on path A. B may
have a lower MTU than the Packet Too Big value. That happens with a
smaller device MTU, a route mtu metric, or lwtunnel headroom. It also
happens when B has an exception that A lacks, for example because:

- A's GFP_ATOMIC insertion failed
- rt6_exception_remove_oldest() evicted A's entry
- a later redirect replaced A's exception
- A was added to the group afterwards

In any of these cases the function returns before the walk, and A never
gets an exception.

The flow-less hash depends only on saddr, daddr, flowlabel and proto 0.
Every later Packet Too Big for that flow therefore lands on B as well.

For example, A is Ethernet at 1500 and B is PPPoE at 1492. A UDP or tunnel
flow on A gets a Packet Too Big of 1496. The lookup returns B, 1496 >= 1492
is true, and nothing is cached on A.

This check predates the patch and mirrors the IPv4 old_mtu < mtu check.
However, the commit message says this patch fixes the case where "every
Packet Too Big lands on the same wrong path, and the flow blackholes".

Could the check be moved into rt6_nh_update_pmtu(), bounded against each
path's MTU, so the update is skipped only when no path would be lowered?

>  
> -	if (!rt6_cache_allowed_for_pmtu(rt6)) {
> -		rt6_do_update_pmtu(rt6, mtu);
> -		/* update rt6_ex->stamp for cache */
> -		if (rt6->rt6i_flags & RTF_CACHE)
> -			rt6_update_exception_stamp_rt(rt6);
> -	} else if (daddr) {

[ ... ]

> +	rcu_read_lock();
> +	f6i = rcu_dereference(rt6->from);
> +	/* fib6_del_route() and a NLM_F_REPLACE clear fib6_node before the
> +	 * route and its siblings are released, don't walk them then.
> +	 */
> +	if (!daddr || !f6i || !rcu_access_pointer(f6i->fib6_node)) {
> +		if (!rt6_cache_allowed_for_pmtu(rt6)) {
> +			rt6_do_update_pmtu(rt6, mtu);
> +			/* update rt6_ex->stamp for cache */
> +			if (rt6->rt6i_flags & RTF_CACHE)
> +				rt6_update_exception_stamp_rt(rt6);
> +		}
> +		goto out;
> +	}

[Severity: Medium]
What happens to a passed-in RTF_CACHE dst whose exception key is not
daddr?

Before this patch, any RTF_CACHE dst was lowered directly. Now, when
daddr, rt6->from and fib6_node are all set, the dst itself is updated only
if rt6_find_cached_rt(res, daddr, saddr) happens to return it.

Take a TCP socket with a routing header (IPV6_RTHDR or SRH).
inet6_csk_route_socket() looks up the first hop after fl6_update_dst(), so
dst can be a first-hop RTF_CACHE exception, for example one created by a
redirect. The call chain is then:

tcp_v6_mtu_reduced()
  inet6_csk_update_pmtu()
    dst->ops->update_pmtu(dst, sk, NULL, mtu, true)
      __ip6_rt_update_pmtu()    /* daddr = &sk->sk_v6_daddr */

The walk looks up the final destination and finds nothing. It then
inserts an unrelated exception, keyed by that destination, on the
first-hop route, and leaves the first-hop exception's MTU unchanged.

The second inet6_csk_route_socket() call in inet6_csk_update_pmtu()
returns the same exception with the old MTU. The icsk_pmtu_cookie >
dst_mtu(dst) test is then false, so the MSS is never reduced.

The same gap applies to the FLOWI_FLAG_KNOWN_NH clones from
ip6_pol_route(). rt6_uncached_list_add() puts them on the uncached list,
and they are never in an exception bucket.

The commit message says "that also covers a dst that is itself a cached
exception, which used to be updated alone". Is that only true when daddr
matches the exception's key?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110095903.84813-1-deliran%40verdict.gg

[PATCH net-next v2 3/3] selftests: net: pmtu: Add IPv6 and asymmetric multipath tests

[Vladimir Vdovin]

Make pmtu_ipv4_mp_exceptions family agnostic and add
pmtu_ipv6_mp_exceptions: one oversized ping over a multipath route
must leave PMTU exceptions on both paths.

Add pmtu_ipv4_mp_asym_exceptions and pmtu_ipv6_mp_asym_exceptions with
an MTU of 1400 behind one router and 1300 behind the other. They pick a
flow through each router (a source address for IPv4, whose multipath
hash covers addresses only, a flow label for IPv6) and check that a
Packet Too Big from the first router is cached on both paths, then
that a flow through the second router lowers the PMTU of both paths.

Without the preceding fix the IPv6 tests fail with both legacy
nexthops and nexthop objects:

  PMTU exception wasn't created after exceeding MTU (veth_A-R2)

Signed-off-by: Vladimir Vdovin <deliran@verdict.gg>
---
 tools/testing/selftests/net/pmtu.sh | 276 ++++++++++++++++++++++++----
 1 file changed, 238 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index c7cd271714ef..6eb79cb76db7 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -203,6 +203,18 @@
 #	on host A and B on lo reachable via both routers. Host A and B
 #	addresses have multipath routes to each other, b_r1 mtu = 1500.
 #	Check that PMTU exceptions are created for both paths.
+#
+# - pmtu_ipv6_mp_exceptions
+#	Same as above but with IPv6
+#
+# - pmtu_ipv4_mp_asym_exceptions
+#	Same topology as pmtu_ipv4_mp_exceptions, with b_r1 mtu = 1400 and
+#	b_r2 mtu = 1300. Pick a flow through each router and check that a
+#	Packet Too Big from R1 is cached on both paths, then that a flow
+#	through R2 lowers the PMTU of both paths
+#
+# - pmtu_ipv6_mp_asym_exceptions
+#	Same as above but with IPv6
 
 source lib.sh
 
@@ -272,7 +284,10 @@ tests="
 	list_flush_ipv6_exception	ipv6: list and flush cached exceptions	1
 	pmtu_ipv4_route_change		ipv4: PMTU exception w/route replace	1
 	pmtu_ipv6_route_change		ipv6: PMTU exception w/route replace	1
-	pmtu_ipv4_mp_exceptions		ipv4: PMTU multipath nh exceptions	1"
+	pmtu_ipv4_mp_exceptions		ipv4: PMTU multipath nh exceptions	1
+	pmtu_ipv6_mp_exceptions		ipv6: PMTU multipath nh exceptions	1
+	pmtu_ipv4_mp_asym_exceptions	ipv4: PMTU multipath nh exceptions, asymmetric MTUs	1
+	pmtu_ipv6_mp_asym_exceptions	ipv6: PMTU multipath nh exceptions, asymmetric MTUs	1"
 
 # Addressing and routing for tests with routers: four network segments, with
 # index SEGMENT between 1 and 4, a common prefix (PREFIX4 or PREFIX6) and an
@@ -350,8 +365,12 @@ tunnel6_b_addr="fd00:2::b"
 tunnel6_mask="64"
 
 host4_a_addr="192.168.99.99"
+host4_a_net="192.168.97"
 host4_b_addr="192.168.88.88"
 
+host6_a_addr="fc00:1000::a"
+host6_b_addr="fc00:1001::b"
+
 dummy6_0_prefix="fc00:1000::"
 dummy6_1_prefix="fc00:1001::"
 dummy6_mask="64"
@@ -994,49 +1013,112 @@ setup_ovs_bridge() {
 }
 
 setup_multipath_new() {
-	# Set up host A with multipath routes to host B host4_b_addr
-	run_cmd ${ns_a} ip addr add ${host4_a_addr} dev lo
-	run_cmd ${ns_a} ip nexthop add id 401 via ${prefix4}.${a_r1}.2 dev veth_A-R1
-	run_cmd ${ns_a} ip nexthop add id 402 via ${prefix4}.${a_r2}.2 dev veth_A-R2
-	run_cmd ${ns_a} ip nexthop add id 403 group 401/402
-	run_cmd ${ns_a} ip route add ${host4_b_addr} src ${host4_a_addr} nhid 403
+	family=${1}
+
+	if [ ${family} -eq 4 ]; then
+		host_a_addr=${host4_a_addr}
+		host_b_addr=${host4_b_addr}
+		nh_a_r1=${prefix4}.${a_r1}.2
+		nh_a_r2=${prefix4}.${a_r2}.2
+		nh_b_r1=${prefix4}.${b_r1}.2
+		nh_b_r2=${prefix4}.${b_r2}.2
+	else
+		host_a_addr=${host6_a_addr}
+		host_b_addr=${host6_b_addr}
+		nh_a_r1=${prefix6}:${a_r1}::2
+		nh_a_r2=${prefix6}:${a_r2}::2
+		nh_b_r1=${prefix6}:${b_r1}::2
+		nh_b_r2=${prefix6}:${b_r2}::2
+	fi
+
+	# Set up host A with multipath routes to host B host_b_addr
+	run_cmd ${ns_a} ip addr add ${host_a_addr} dev lo
+	run_cmd ${ns_a} ip nexthop add id ${family}01 via ${nh_a_r1} dev veth_A-R1
+	run_cmd ${ns_a} ip nexthop add id ${family}02 via ${nh_a_r2} dev veth_A-R2
+	run_cmd ${ns_a} ip nexthop add id ${family}03 group ${family}01/${family}02
+	run_cmd ${ns_a} ip route add ${host_b_addr} src ${host_a_addr} nhid ${family}03
 
-	# Set up host B with multipath routes to host A host4_a_addr
-	run_cmd ${ns_b} ip addr add ${host4_b_addr} dev lo
-	run_cmd ${ns_b} ip nexthop add id 401 via ${prefix4}.${b_r1}.2 dev veth_B-R1
-	run_cmd ${ns_b} ip nexthop add id 402 via ${prefix4}.${b_r2}.2 dev veth_B-R2
-	run_cmd ${ns_b} ip nexthop add id 403 group 401/402
-	run_cmd ${ns_b} ip route add ${host4_a_addr} src ${host4_b_addr} nhid 403
+	# Set up host B with multipath routes to host A host_a_addr
+	run_cmd ${ns_b} ip addr add ${host_b_addr} dev lo
+	run_cmd ${ns_b} ip nexthop add id ${family}01 via ${nh_b_r1} dev veth_B-R1
+	run_cmd ${ns_b} ip nexthop add id ${family}02 via ${nh_b_r2} dev veth_B-R2
+	run_cmd ${ns_b} ip nexthop add id ${family}03 group ${family}01/${family}02
+	run_cmd ${ns_b} ip route add ${host_a_addr} src ${host_b_addr} nhid ${family}03
 }
 
 setup_multipath_old() {
-	# Set up host A with multipath routes to host B host4_b_addr
-	run_cmd ${ns_a} ip addr add ${host4_a_addr} dev lo
-	run_cmd ${ns_a} ip route add ${host4_b_addr} \
-			src ${host4_a_addr} \
-			nexthop via ${prefix4}.${a_r1}.2 weight 1 \
-			nexthop via ${prefix4}.${a_r2}.2 weight 1
+	family=${1}
+
+	if [ ${family} -eq 4 ]; then
+		host_a_addr=${host4_a_addr}
+		host_b_addr=${host4_b_addr}
+		nh_a_r1=${prefix4}.${a_r1}.2
+		nh_a_r2=${prefix4}.${a_r2}.2
+		nh_b_r1=${prefix4}.${b_r1}.2
+		nh_b_r2=${prefix4}.${b_r2}.2
+	else
+		host_a_addr=${host6_a_addr}
+		host_b_addr=${host6_b_addr}
+		nh_a_r1=${prefix6}:${a_r1}::2
+		nh_a_r2=${prefix6}:${a_r2}::2
+		nh_b_r1=${prefix6}:${b_r1}::2
+		nh_b_r2=${prefix6}:${b_r2}::2
+	fi
+
+	# Set up host A with multipath routes to host B host_b_addr
+	run_cmd ${ns_a} ip addr add ${host_a_addr} dev lo
+	run_cmd ${ns_a} ip route add ${host_b_addr} \
+			src ${host_a_addr} \
+			nexthop via ${nh_a_r1} weight 1 \
+			nexthop via ${nh_a_r2} weight 1
 
-	# Set up host B with multipath routes to host A host4_a_addr
-	run_cmd ${ns_b} ip addr add ${host4_b_addr} dev lo
-	run_cmd ${ns_b} ip route add ${host4_a_addr} \
-			src ${host4_b_addr} \
-			nexthop via ${prefix4}.${b_r1}.2 weight 1 \
-			nexthop via ${prefix4}.${b_r2}.2 weight 1
+	# Set up host B with multipath routes to host A host_a_addr
+	run_cmd ${ns_b} ip addr add ${host_b_addr} dev lo
+	run_cmd ${ns_b} ip route add ${host_a_addr} \
+			src ${host_b_addr} \
+			nexthop via ${nh_b_r1} weight 1 \
+			nexthop via ${nh_b_r2} weight 1
 }
 
+
 setup_multipath() {
+	family="$1"
+
+	if [ ${family} -eq 4 ]; then
+		host_a_addr=${host4_a_addr}
+		host_b_addr=${host4_b_addr}
+		nh_r1_a=${prefix4}.${a_r1}.1
+		nh_r1_b=${prefix4}.${b_r1}.1
+		nh_r2_a=${prefix4}.${a_r2}.1
+		nh_r2_b=${prefix4}.${b_r2}.1
+	else
+		host_a_addr=${host6_a_addr}
+		host_b_addr=${host6_b_addr}
+		nh_r1_a=${prefix6}:${a_r1}::1
+		nh_r1_b=${prefix6}:${b_r1}::1
+		nh_r2_a=${prefix6}:${a_r2}::1
+		nh_r2_b=${prefix6}:${b_r2}::1
+	fi
+
 	if [ "$USE_NH" = "yes" ]; then
-		setup_multipath_new
+		setup_multipath_new ${family}
 	else
-		setup_multipath_old
+		setup_multipath_old ${family}
 	fi
 
 	# Set up routers with routes to dummies
-	run_cmd ${ns_r1} ip route add ${host4_a_addr} via ${prefix4}.${a_r1}.1
-	run_cmd ${ns_r2} ip route add ${host4_a_addr} via ${prefix4}.${a_r2}.1
-	run_cmd ${ns_r1} ip route add ${host4_b_addr} via ${prefix4}.${b_r1}.1
-	run_cmd ${ns_r2} ip route add ${host4_b_addr} via ${prefix4}.${b_r2}.1
+	run_cmd ${ns_r1} ip route add ${host_a_addr} via ${nh_r1_a}
+	run_cmd ${ns_r2} ip route add ${host_a_addr} via ${nh_r2_a}
+	run_cmd ${ns_r1} ip route add ${host_b_addr} via ${nh_r1_b}
+	run_cmd ${ns_r2} ip route add ${host_b_addr} via ${nh_r2_b}
+}
+
+setup_multipath4() {
+	setup_multipath 4
+}
+
+setup_multipath6() {
+	setup_multipath 6
 }
 
 setup() {
@@ -2377,8 +2459,18 @@ test_pmtu_ipv6_route_change() {
 	test_pmtu_ipvX_route_change 6
 }
 
-test_pmtu_ipv4_mp_exceptions() {
-	setup namespaces routing multipath || return $ksft_skip
+test_pmtu_ipvX_mp_exceptions() {
+	family=${1}
+
+	if [ ${family} -eq 4 ]; then
+		ping=ping
+		dst="${host4_b_addr}"
+	else
+		ping=${ping6}
+		dst="${host6_b_addr}"
+	fi
+
+	setup namespaces routing multipath${family} || return $ksft_skip
 
 	trace "${ns_a}"  veth_A-R1    "${ns_r1}" veth_R1-A \
 	      "${ns_r1}" veth_R1-B    "${ns_b}"  veth_B-R1 \
@@ -2395,18 +2487,126 @@ test_pmtu_ipv4_mp_exceptions() {
 	mtu "${ns_r2}" veth_R2-A 2000
 	mtu "${ns_r2}" veth_R2-B 1500
 	mtu "${ns_b}"  veth_B-R2 1500
-
 	# Ping and expect two nexthop exceptions for two routes
-	run_cmd ${ns_a} ping -q -M want -i 0.1 -c 1 -s 1800 "${host4_b_addr}"
+	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -c 1 -s 1800 "${dst}"
 
 	# Check that exceptions have been created with the correct PMTU
-	pmtu_a_R1="$(route_get_dst_pmtu_from_exception "${ns_a}" "${host4_b_addr}" oif veth_A-R1)"
-	pmtu_a_R2="$(route_get_dst_pmtu_from_exception "${ns_a}" "${host4_b_addr}" oif veth_A-R2)"
-
+	pmtu_a_R1="$(route_get_dst_pmtu_from_exception "${ns_a}" "${dst}" oif veth_A-R1)"
+	pmtu_a_R2="$(route_get_dst_pmtu_from_exception "${ns_a}" "${dst}" oif veth_A-R2)"
 	check_pmtu_value "1500" "${pmtu_a_R1}" "exceeding MTU (veth_A-R1)" || return 1
 	check_pmtu_value "1500" "${pmtu_a_R2}" "exceeding MTU (veth_A-R2)" || return 1
 }
 
+test_pmtu_ipv4_mp_exceptions() {
+	test_pmtu_ipvX_mp_exceptions 4
+}
+
+# Find a flow from host A to ${2} that goes out through ${3}: a source
+# address out of ${host4_a_net}, added to lo as needed, since the IPv4
+# multipath hash covers addresses only, or an IPv6 flow label for a packet
+# sent from ${host6_a_addr}
+mp_flow_via() {
+	family="${1}"
+	dst="${2}"
+	oif="${3}"
+
+	for i in $(seq 1 32); do
+		if [ ${family} -eq 4 ]; then
+			flow="${host4_a_net}.${i}"
+			${ns_a} ip addr add ${flow}/32 dev lo 2>/dev/null
+			out="$(${ns_a} ip route get ${dst} from ${flow} 2>/dev/null)"
+		else
+			flow="${i}"
+			out="$(${ns_a} ip route get ${dst} from ${host6_a_addr} \
+				ipproto 58 flowlabel ${flow} 2>/dev/null)"
+		fi
+		case "${out}" in
+		*" dev ${oif} "*)
+			echo "${flow}"
+			return 0
+			;;
+		esac
+	done
+
+	return 1
+}
+
+test_pmtu_ipvX_mp_asym_exceptions() {
+	family=${1}
+
+	if [ ${family} -eq 4 ]; then
+		ping=ping
+		dst="${host4_b_addr}"
+	else
+		ping=${ping6}
+		dst="${host6_b_addr}"
+	fi
+
+	setup namespaces routing multipath${family} || return $ksft_skip
+
+	trace "${ns_a}"  veth_A-R1    "${ns_r1}" veth_R1-A \
+	      "${ns_r1}" veth_R1-B    "${ns_b}"  veth_B-R1 \
+	      "${ns_a}"  veth_A-R2    "${ns_r2}" veth_R2-A \
+	      "${ns_r2}" veth_R2-B    "${ns_b}"  veth_B-R2
+
+	if [ ${family} -eq 4 ]; then
+		# Routes back to the source addresses mp_flow_via() picks
+		run_cmd ${ns_r1} ip route add ${host4_a_net}.0/24 via ${prefix4}.${a_r1}.2
+		run_cmd ${ns_r2} ip route add ${host4_a_net}.0/24 via ${prefix4}.${a_r2}.2
+	fi
+
+	# Set up initial MTU values: 1400 behind R1, 1300 behind R2
+	mtu "${ns_a}"  veth_A-R1 2000
+	mtu "${ns_r1}" veth_R1-A 2000
+	mtu "${ns_r1}" veth_R1-B 1400
+	mtu "${ns_b}"  veth_B-R1 1400
+
+	mtu "${ns_a}"  veth_A-R2 2000
+	mtu "${ns_r2}" veth_R2-A 2000
+	mtu "${ns_r2}" veth_R2-B 1300
+	mtu "${ns_b}"  veth_B-R2 1300
+
+	flow_r1="$(mp_flow_via ${family} ${dst} veth_A-R1)" || {
+		err "  no flow through veth_A-R1 found"; return 1; }
+	flow_r2="$(mp_flow_via ${family} ${dst} veth_A-R2)" || {
+		err "  no flow through veth_A-R2 found"; return 1; }
+	if [ ${family} -eq 4 ]; then
+		ping_r1="${ping} -I ${flow_r1}"
+		ping_r2="${ping} -I ${flow_r2}"
+	else
+		ping_r1="${ping} -I ${host6_a_addr} -F ${flow_r1}"
+		ping_r2="${ping} -I ${host6_a_addr} -F ${flow_r2}"
+	fi
+
+	# A Packet Too Big from R1 is cached on both paths
+	run_cmd ${ns_a} ${ping_r1} -q -M want -i 0.1 -c 1 -s 1800 "${dst}"
+
+	pmtu_a_R1="$(route_get_dst_pmtu_from_exception "${ns_a}" "${dst}" oif veth_A-R1)"
+	pmtu_a_R2="$(route_get_dst_pmtu_from_exception "${ns_a}" "${dst}" oif veth_A-R2)"
+	check_pmtu_value "1400" "${pmtu_a_R1}" "exceeding MTU via R1 (veth_A-R1)" || return 1
+	check_pmtu_value "1400" "${pmtu_a_R2}" "exceeding MTU via R1 (veth_A-R2)" || return 1
+
+	# The lower MTU behind R2 lowers the PMTU of both paths
+	run_cmd ${ns_a} ${ping_r2} -q -M want -i 0.1 -c 1 -s 1350 "${dst}"
+
+	pmtu_a_R1="$(route_get_dst_pmtu_from_exception "${ns_a}" "${dst}" oif veth_A-R1)"
+	pmtu_a_R2="$(route_get_dst_pmtu_from_exception "${ns_a}" "${dst}" oif veth_A-R2)"
+	check_pmtu_value "1300" "${pmtu_a_R1}" "exceeding MTU via R2 (veth_A-R1)" || return 1
+	check_pmtu_value "1300" "${pmtu_a_R2}" "exceeding MTU via R2 (veth_A-R2)" || return 1
+}
+
+test_pmtu_ipv4_mp_asym_exceptions() {
+	test_pmtu_ipvX_mp_asym_exceptions 4
+}
+
+test_pmtu_ipv6_mp_asym_exceptions() {
+	test_pmtu_ipvX_mp_asym_exceptions 6
+}
+
+test_pmtu_ipv6_mp_exceptions() {
+	test_pmtu_ipvX_mp_exceptions 6
+}
+
 usage() {
 	echo
 	echo "$0 [OPTIONS] [TEST]..."
-- 
2.56.0

Re: [PATCH net-next v2 3/3] selftests: net: pmtu: Add IPv6 and asymmetric multipath tests

[netdev-bot+sashiko]

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

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

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] In test_pmtu_ipvX_mp_asym_exceptions()
  (tools/testing/selftests/net/pmtu.sh, IPv4 branch), the return routes…
- [Medium] In the IPv6 branch, mp_flow_via()
  (tools/testing/selftests/net/pmtu.sh) depends on `ip route get ${dst}…
- [Low] mp_flow_via() returns a decimal flow label (`flow="${i}"`, i in
  1..32).

--

selftests: net: pmtu: Add IPv6 and asymmetric multipath tests

This patch makes pmtu_ipv4_mp_exceptions family agnostic and adds an IPv6
variant. It also adds IPv4 and IPv6 multipath tests with asymmetric MTUs
(1400 behind R1, 1300 behind R2). These pick a flow through each router,
using a source address for IPv4 and a flow label for IPv6, and check that
the PMTU exceptions on both paths get updated.

> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index c7cd271714ef7..6eb79cb76db71 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh

[ ... ]

> @@ -2395,18 +2487,126 @@ test_pmtu_ipv4_mp_exceptions() {

[ ... ]

> +mp_flow_via() {
> +	family="${1}"
> +	dst="${2}"
> +	oif="${3}"
> +
> +	for i in $(seq 1 32); do
> +		if [ ${family} -eq 4 ]; then
> +			flow="${host4_a_net}.${i}"
> +			${ns_a} ip addr add ${flow}/32 dev lo 2>/dev/null
> +			out="$(${ns_a} ip route get ${dst} from ${flow} 2>/dev/null)"
> +		else
> +			flow="${i}"
> +			out="$(${ns_a} ip route get ${dst} from ${host6_a_addr} \
> +				ipproto 58 flowlabel ${flow} 2>/dev/null)"

[Severity: Medium]

What happens here if the installed iproute2 doesn't support the flowlabel
keyword for ip route get? stderr goes to /dev/null, so every iteration
returns empty output. The " dev ${oif} " pattern never matches, and
mp_flow_via() returns 1 after 32 iterations.

test_pmtu_ipvX_mp_asym_exceptions() then runs:

    err "  no flow through veth_A-R1 found"; return 1;

So pmtu_ipv6_mp_asym_exceptions would be reported as a failure instead of
$ksft_skip. The error message also points away from the real cause.

Other selftests check for this first. For example, fib_rule_tests.sh runs:

    fib_check_iproute_support "flowlabel" "flowlabel"

before it uses flowlabel. Could a similar check go here, returning
$ksft_skip when the keyword isn't supported?

> +		fi
> +		case "${out}" in
> +		*" dev ${oif} "*)
> +			echo "${flow}"
> +			return 0
> +			;;
> +		esac
> +	done
> +
> +	return 1
> +}
> +
> +test_pmtu_ipvX_mp_asym_exceptions() {

[ ... ]

> +	if [ ${family} -eq 4 ]; then
> +		# Routes back to the source addresses mp_flow_via() picks
> +		run_cmd ${ns_r1} ip route add ${host4_a_net}.0/24 via ${prefix4}.${a_r1}.2
> +		run_cmd ${ns_r2} ip route add ${host4_a_net}.0/24 via ${prefix4}.${a_r2}.2
> +	fi

[Severity: Medium]

Are these gateways right? The addressing comment in pmtu.sh gives ID 1 to
hosts and ID 2 to routers. That makes ${prefix4}.${a_r1}.2 R1's own address
on veth_R1-A, not host A's.

In this same patch, setup_multipath() routes the routers back to host A
through the .1 address:

    nh_r1_a=${prefix4}.${a_r1}.1
    ...
    nh_r2_a=${prefix4}.${a_r2}.1

The kernel still accepts a local address as the gateway.
fib_check_nh_v4_gw() allows RTN_LOCAL and sets the nexthop scope to
RT_SCOPE_HOST. rt_set_nexthop() only uses the gateway when the scope is
RT_SCOPE_LINK. So the route acts as an on-link route, and R1/R2 send ARP
requests for 192.168.97.N directly.

That address is on host A's lo, not on veth_A-R1 or veth_A-R2. Host A only
answers the ARP when arp_ignore is 0. pmtu.sh never sets arp_ignore, and new
namespaces inherit the IPv4 devconf from init_net by default.

On a host with net.ipv4.conf.all.arp_ignore >= 1, wouldn't the ICMP
fragmentation needed from R1 and the echo replies both be dropped? The test
would then fail with "PMTU exception wasn't created after exceeding MTU via
R1 (veth_A-R1)", and the R2 step would hit the same problem.

Should these be ${prefix4}.${a_r1}.1 and ${prefix4}.${a_r2}.1 instead?

[ ... ]

> +	flow_r1="$(mp_flow_via ${family} ${dst} veth_A-R1)" || {
> +		err "  no flow through veth_A-R1 found"; return 1; }
> +	flow_r2="$(mp_flow_via ${family} ${dst} veth_A-R2)" || {
> +		err "  no flow through veth_A-R2 found"; return 1; }
> +	if [ ${family} -eq 4 ]; then
> +		ping_r1="${ping} -I ${flow_r1}"
> +		ping_r2="${ping} -I ${flow_r2}"
> +	else
> +		ping_r1="${ping} -I ${host6_a_addr} -F ${flow_r1}"
> +		ping_r2="${ping} -I ${host6_a_addr} -F ${flow_r2}"
> +	fi

[Severity: Low]

mp_flow_via() returns a decimal flow label (flow="${i}", with i from 1 to
32), and that string goes unchanged to ping -F here. ping documents -F as
the flow label in hex, and older iputils ping6 builds parse it as base 16.

If the first matching label is 10 or higher, wouldn't the label on the wire
differ from the one probed with ip route get? For example, "10" becomes
0x10. The IPv6 multipath hash includes the flow label, so the echo request
could take the other nexthop, and the 1400/1300 checks would fail.

This depends on how the installed iputils and iproute2 parse the value. The
multipath hash seed is also random per boot, so failures would be
intermittent.

Would passing the label with a 0x prefix to both tools remove the ambiguity?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110095903.84813-1-deliran%40verdict.gg

Re: [PATCH net-next v2 0/3] ipv4/ipv6: Cache pmtu on all paths of a multipath route

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next v2 0/3] ipv4/ipv6: Cache pmtu on all paths of a multipath route

[Vladimir Vdovin]

On Tue, Nov 10, 2026 at 10:06:44AM +0000, netdev-bot+sinfo wrote:
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.

Patch 2 (ipv6) is the counterpart of the IPv4 issue fixed in commit
7d3f3b4367f3 ("net: ipv4: Cache pmtu for all packet paths if multipath
enabled") [1]: a host with two ECMP uplinks and a smaller MTU further
down the path had flows blackholing after a Packet Too Big. The IPv6
side was deferred back then at David's request [2]. The new pmtu.sh
tests in patch 3 reproduce it and fail without patch 2.

Patch 1 (ipv4) was found by code inspection while reworking v1 after
the Sashiko (LLM) review of it [3]: the same "replace the exception"
step that review flagged in the IPv6 code is in the IPv4 fan-out from
7d3f3b4367f3 as well.

[1] https://lore.kernel.org/netdev/20241108093427.317942-1-deliran@verdict.gg/
[2] https://lore.kernel.org/netdev/ad29493c-9e48-4a94-b327-e47af3a9d137@kernel.org/
[3] https://lore.kernel.org/netdev/179236119085.3135.2701233313088371263@kernel.org/