MPTCP Linux Development
 help / color / mirror / Atom feed
* [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid
@ 2026-09-14  9:19 Kalpan Jani
  2026-09-14  9:19 ` [PATCH mptcp-next v4 1/2] mptcp: sched: add subflow avoid flag and enforce it in default sched Kalpan Jani
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Kalpan Jani @ 2026-09-14  9:19 UTC (permalink / raw)
  To: mptcp; +Cc: matttbe, martineau, pabeni, shardul.b, janak, kalpanjani009

The core benches a subflow via the "stale" bit after
net.mptcp.stale_loss_cnt retransmission intervals without progress.
That bit belongs to the core and is tightly coupled to the RTO: it
gets cleared the moment a packet is acked on the subflow. A packet
scheduler has no way to bench a subflow for its own reasons, such as
too high latency or instability, and keep it benched independently of
ack traffic.

Reusing "stale" doesn't work, and "scheduled" doesn't either: the core
clears it right after every send/retrans pass, so it can't express a
standing "leave this subflow alone for now" decision, and nothing
outside the scheduler that set it can ever see it.

This adds a scheduler-owned "avoid" bool that stays set until the
scheduler clears it, is honoured by the default in-kernel subflow
picker on both the send and retransmit paths, and is visible through
MPTCP diag alongside the existing BKUP/FULLY_ESTABLISHED flags.

The avoid state is kept as a standalone bool instead of using the
adjacent flag bitfield. This avoids a race with 
mptcp_subflow_data_available(),
which clears map_valid in the same bitfield from the receive path while
the scheduler can update avoid from BPF context. The avoid accesses use
READ_ONCE()/WRITE_ONCE().

The default in-kernel subflow picker skips avoided subflows for both
send and retransmit operations. Custom schedulers can explicitly
honour the avoid state according to their own scheduling policy.

The avoid state is exposed to BPF struct_ops schedulers through
mptcp_subflow_set_avoid(), and is also exposed through MPTCP diag as
MPTCP_SUBFLOW_FLAG_AVOID. The kfunc is marked __bpf_kfunc so it remains
available for BPF BTF resolution.

The BPF selftest sets the avoid flag once on the first get_send() call
and relies on the state remaining set across subsequent scheduler
rounds. The test uses per-socket BPF storage for the scheduler state
and validates the avoided-subflow behaviour across rounds.

Probing an avoided subflow to decide when to clear it (mentioned in
issue 348) is left out on purpose: that's scheduler policy, not core
mechanism, and belongs to whoever writes the actual latency/stability
heuristics on top of this.

Patch 1 adds the flag, the kfunc, the default-scheduler enforcement,
and diag support.

Patch 2 adds the BPF selftest exercising persistence of the avoid state
across scheduler rounds.

The patches have been reviewed with the concurrency concern raised by
Sashiko addressed, and the resulting patch series passes checkpatch
with no errors. The remaining warning is the standard MAINTAINERS
warning for the newly added selftest file.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/349

Kalpan Jani (2):
mptcp: sched: add subflow avoid flag and enforce it in default sched
selftests: mptcp: bpf: exercise the subflow avoid flag across rounds


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH mptcp-next v4 1/2] mptcp: sched: add subflow avoid flag and enforce it in default sched
  2026-09-14  9:19 [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid Kalpan Jani
@ 2026-09-14  9:19 ` Kalpan Jani
  2026-09-14  9:20 ` [PATCH mptcp-next v4 2/2] selftests: mptcp: bpf: exercise the subflow avoid flag across rounds Kalpan Jani
  2026-09-14 10:29 ` [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid MPTCP CI
  2 siblings, 0 replies; 5+ messages in thread
From: Kalpan Jani @ 2026-09-14  9:19 UTC (permalink / raw)
  To: mptcp; +Cc: matttbe, martineau, pabeni, shardul.b, janak, kalpanjani009

The core benches a subflow via the "stale" bit after
net.mptcp.stale_loss_cnt retransmission intervals without progress.
That bit belongs to the core and is tightly coupled to the RTO: it
gets cleared as soon as a packet is acked on the subflow
(mptcp_subflow_active() calls mptcp_subflow_set_active() once
rcv_tstamp moves forward).

"scheduled" doesn't cover this either. The core clears it right after
every send/retrans pass the scheduler set it in, so there's no way to
express "leave this subflow alone across future rounds" with it, and
nothing outside the scheduler that set it ever sees the value.

Add a scheduler-owned "avoid" bool to mptcp_subflow_context:

 - only mptcp_subflow_set_avoid() touches it, never the core. It's
   exposed to BPF struct_ops schedulers as a kfunc with the same
   registration and filter as mptcp_subflow_set_scheduled(), so only
   a scheduler can call it;

 - it is a standalone bool, alongside "scheduled" and "data_avail",
   rather than a bit in the adjacent flag bitfield.
   mptcp_subflow_data_available()
   clears map_valid, a bit in that bitfield, from the subflow's own
   receive path without holding the msk lock, while set_avoid() runs
   under the msk lock from BPF scheduler context. Bitfield writes are
   a non-atomic read-modify-write on the whole containing word, so
   placing avoid there would race with that update and risk silently
   corrupting map_valid or other neighbouring bits. The standalone
   bools next to it are accessed with READ_ONCE()/WRITE_ONCE() for
   the same reason;

 - the default in-kernel subflow picker, mptcp_subflow_get_send() and
   mptcp_subflow_get_retrans(), now skips an avoided subflow next to
   the existing active-subflow check. This only applies when no
   custom scheduler is loaded (mptcp_sched_default_get_send() /
   _get_retrans() are what msk->sched->get_send() falls back to); a
   custom BPF scheduler bypasses this default picker entirely, so it
   only honours avoid if it explicitly checks subflow->avoid itself,
   exactly as the selftest scheduler in patch 2 does. avoid defaults
   to false and only an explicit set_avoid() call flips it, so this
   is a no-op for anyone who doesn't use it;

 - it's surfaced through MPTCP diag as MPTCP_SUBFLOW_FLAG_AVOID,
   alongside the existing BKUP/FULLY_ESTABLISHED flags.

set_avoid() has no C callers in-tree, unlike set_scheduled() which
the core send/retrans/close paths call directly -- mark it
__bpf_kfunc so it survives LTO/dead-code-elimination before
resolve_btfids looks for it in BTF.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/349
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>

Changes since v3:
- moved avoid back out of the __unused bitfield padding into a
  standalone bool, and restored READ_ONCE()/WRITE_ONCE() on every
  access. Sashiko flagged that mptcp_subflow_data_available() clears
  map_valid, a bit in the same bitfield word, without the msk lock
  held, so a scheduler setting avoid under the msk lock could race
  with it and corrupt neighbouring bits via the non-atomic
  read-modify-write. This reintroduces the 4-byte struct hole v2
  removed; correctness takes priority over that optimisation.
- corrected the commit message: avoid is only enforced by the
  default in-kernel subflow picker, not by every custom scheduler
  unconditionally. mptcp_sched_get_send()/_get_retrans() call the
  registered scheduler's own get_send()/get_retrans() directly when
  one is loaded, bypassing mptcp_subflow_get_send()/_get_retrans()
  entirely. A custom scheduler only honours avoid if it checks the
  field itself.

Changes since v2:
- rebased onto the current export tree. v2 was generated against a
  stale local snapshot that predated several unrelated commits
  touching the same bitfield region in mptcp_subflow_context; applying
  v2 there produced a merge conflict in protocol.h.

Changes since v1:
- moved avoid from a standalone bool into the existing __unused
  bitfield padding: the extra bool pushed lent_mem_frag onto a
  4-byte-aligned boundary, adding a 4-byte hole. The bitfield slot was
  already reserved and unused. (Reverted in v4, see above.)
- marked mptcp_subflow_set_avoid() __bpf_kfunc: no C callers, unlike
  set_scheduled(), so it could be dropped under LTO before
  resolve_btfids finds it in BTF.
- mptcp_subflow_get_send() and mptcp_subflow_get_retrans() now skip a
  subflow with avoid set, next to the existing active-subflow check:
  previously nothing in the core consulted the flag.
- exposed avoid via MPTCP diag as MPTCP_SUBFLOW_FLAG_AVOID, next to
  the existing BKUP_LOC/FULLY_ESTABLISHED flags.

v3: https://lore.kernel.org/all/20260831094651.2682660-2-kalpan.jani@mpiricsoftware.com/
v2: https://lore.kernel.org/all/20260824102741.347492-2-kalpan.jani@mpiricsoftware.com/
v1: https://lore.kernel.org/all/20260715101148.2601045-2-kalpan.jani@mpiricsoftware.com/
---
 include/uapi/linux/mptcp.h | 1 +
 net/mptcp/bpf.c            | 1 +
 net/mptcp/diag.c           | 2 ++
 net/mptcp/protocol.c       | 4 ++--
 net/mptcp/protocol.h       | 3 +++
 net/mptcp/sched.c          | 6 ++++++
 6 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 72a5d030154e..b4ca97e07445 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -22,6 +22,7 @@
 #define MPTCP_SUBFLOW_FLAG_FULLY_ESTABLISHED	_BITUL(6)
 #define MPTCP_SUBFLOW_FLAG_CONNECTED		_BITUL(7)
 #define MPTCP_SUBFLOW_FLAG_MAPVALID		_BITUL(8)
+#define MPTCP_SUBFLOW_FLAG_AVOID		_BITUL(9)
 
 #define MPTCP_PM_CMD_GRP_NAME	"mptcp_pm_cmds"
 #define MPTCP_PM_EV_GRP_NAME	"mptcp_pm_events"
diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 82b0ad25f700..3a229e28960d 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -318,6 +318,7 @@ BTF_KFUNCS_START(bpf_mptcp_common_kfunc_ids)
 BTF_ID_FLAGS(func, bpf_mptcp_subflow_ctx, KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_mptcp_subflow_tcp_sock, KF_RET_NULL)
 BTF_ID_FLAGS(func, mptcp_subflow_set_scheduled)
+BTF_ID_FLAGS(func, mptcp_subflow_set_avoid)
 BTF_ID_FLAGS(func, mptcp_subflow_active)
 BTF_ID_FLAGS(func, mptcp_set_timeout)
 BTF_ID_FLAGS(func, mptcp_wnd_end)
diff --git a/net/mptcp/diag.c b/net/mptcp/diag.c
index 70cf9ebce833..7a2c40a63dfa 100644
--- a/net/mptcp/diag.c
+++ b/net/mptcp/diag.c
@@ -49,6 +49,8 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin
 		flags |= MPTCP_SUBFLOW_FLAG_BKUP_LOC;
 	if (READ_ONCE(sf->fully_established))
 		flags |= MPTCP_SUBFLOW_FLAG_FULLY_ESTABLISHED;
+	if (READ_ONCE(sf->avoid))
+		flags |= MPTCP_SUBFLOW_FLAG_AVOID;
 	if (sf->conn_finished)
 		flags |= MPTCP_SUBFLOW_FLAG_CONNECTED;
 	if (sf->map_valid)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f22d64ab1c53..56c9f1f878fb 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1645,7 +1645,7 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
 
 		trace_mptcp_subflow_get_send(subflow);
 		ssk =  mptcp_subflow_tcp_sock(subflow);
-		if (!mptcp_subflow_active(subflow))
+		if (!mptcp_subflow_active(subflow) || READ_ONCE(subflow->avoid))
 			continue;
 
 		tout = max(tout, mptcp_timeout_from_subflow(subflow));
@@ -2563,7 +2563,7 @@ struct sock *mptcp_subflow_get_retrans(struct mptcp_sock *msk)
 	mptcp_for_each_subflow(msk, subflow) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 
-		if (!__mptcp_subflow_active(subflow))
+		if (!__mptcp_subflow_active(subflow) || READ_ONCE(subflow->avoid))
 			continue;
 
 		/* still data outstanding at TCP level? skip this */
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3d250e8204d5..e6135ab87e3e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -589,6 +589,7 @@ struct mptcp_subflow_context {
 		__unused : 9;
 	bool	data_avail;
 	bool	scheduled;
+	bool	avoid;		    /* pkt scheduler: skip subflow if possible */
 	bool	pm_listener;	    /* a listener managed by the kernel PM? */
 	bool	fully_established;  /* path validated */
 	u32	lent_mem_frag;
@@ -902,6 +903,8 @@ static inline bool __mptcp_subflow_active(struct mptcp_subflow_context *subflow)
 
 void mptcp_subflow_set_active(struct mptcp_subflow_context *subflow);
 
+void mptcp_subflow_set_avoid(struct mptcp_subflow_context *subflow, bool avoid);
+
 bool mptcp_subflow_active(struct mptcp_subflow_context *subflow);
 
 void mptcp_subflow_drop_ctx(struct sock *ssk);
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index 1e59072d478c..0ce59eb093df 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -165,6 +165,12 @@ void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow,
 	WRITE_ONCE(subflow->scheduled, scheduled);
 }
 
+__bpf_kfunc void mptcp_subflow_set_avoid(struct mptcp_subflow_context *subflow,
+					 bool avoid)
+{
+	WRITE_ONCE(subflow->avoid, avoid);
+}
+
 int mptcp_sched_get_send(struct mptcp_sock *msk)
 {
 	struct mptcp_subflow_context *subflow;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH mptcp-next v4 2/2] selftests: mptcp: bpf: exercise the subflow avoid flag across rounds
  2026-09-14  9:19 [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid Kalpan Jani
  2026-09-14  9:19 ` [PATCH mptcp-next v4 1/2] mptcp: sched: add subflow avoid flag and enforce it in default sched Kalpan Jani
@ 2026-09-14  9:20 ` Kalpan Jani
  2026-09-14  9:32   ` sashiko-bot
  2026-09-14 10:29 ` [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid MPTCP CI
  2 siblings, 1 reply; 5+ messages in thread
From: Kalpan Jani @ 2026-09-14  9:20 UTC (permalink / raw)
  To: mptcp; +Cc: matttbe, martineau, pabeni, shardul.b, janak, kalpanjani009

Add a BPF struct_ops scheduler selftest for mptcp_subflow_set_avoid(),
and add the required extern declarations for it and for
mptcp_subflow_active(), both declared locally in this file rather
than in the shared mptcp_bpf.h. mptcp_subflow_active()'s extern was
missing here even though the other BPF scheduler progs already have
it (see mptcp_bpf_burst.c).

"scheduled" gets cleared by the core after every single send/retrans
pass, so a scheduler has to set it again every round. This test's
scheduler does the opposite on purpose: it marks a subflow avoided
exactly once, the first time get_send() runs, and never calls
set_avoid() again after that. Every later round skips the marked
subflow purely by reading back state set earlier, and the harness's
own byte-count checks confirm the avoided path carried no data across
the whole transfer.

Per-msk state (whether avoid has been asserted yet) is kept in
BPF_MAP_TYPE_SK_STORAGE keyed on msk, the same pattern
mptcp_bpf_rr.c already uses, rather than in BPF global variables:
globals are process-wide, not per-socket, so they would be shared
and reset across any concurrent connections attached to this
scheduler.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/349
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>

Changes since v3:
- moved the avoid_marked flag from a BPF global variable into
  per-msk BPF_MAP_TYPE_SK_STORAGE, mirroring mptcp_bpf_rr.c. Sashiko
  flagged that globals are shared across every socket the scheduler
  attaches to, so concurrent connections could reset each other's
  state.
- dropped the get_send_calls counter and its assertion: it lived in
  the same global state and can't be read back from userspace once
  the counted storage is per-socket and freed on release. The test
  still exercises multiple scheduling rounds implicitly -- the
  harness's data-transfer size requires several send calls, and the
  addr1/addr2 byte-count checks already confirm the avoided path
  carried no data throughout.
- reformatted the two multi-line comments to put the opening /* on
  its own line, per BPF subsystem comment style.

Changes since v2:
- rebased onto the current export tree, no functional change from v2.

Changes since v1:
- reworked the scheduler to set avoid once, on the first get_send()
  call, and rely on reading it back on every later call rather than
  re-asserting it: the previous version set and read the flag within
  the same call, which doesn't exercise standing state across
  scheduling rounds.
- added the missing extern __ksym declaration for
  mptcp_subflow_active(), matching the existing pattern in
  mptcp_bpf_burst.c.
- declared the mptcp_subflow_set_avoid() extern in this file instead
  of mptcp_bpf.h: checkpatch flags new externs in shared headers, and
  this kfunc has only one caller.

v3: https://lore.kernel.org/all/20260831094651.2682660-3-kalpan.jani@mpiricsoftware.com/
v2: https://lore.kernel.org/all/20260824102741.347492-3-kalpan.jani@mpiricsoftware.com/
v1: https://lore.kernel.org/all/20260715101148.2601045-3-kalpan.jani@mpiricsoftware.com/
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 15 ++++
 .../selftests/bpf/progs/mptcp_bpf_avoid.c     | 86 +++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/mptcp_bpf_avoid.c

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index d77c9f8c53c7..3bff30d0d530 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -18,6 +18,7 @@
 #include "mptcp_bpf_rr.skel.h"
 #include "mptcp_bpf_red.skel.h"
 #include "mptcp_bpf_burst.skel.h"
+#include "mptcp_bpf_avoid.skel.h"
 
 #define NS_TEST "mptcp_ns"
 #define ADDR_1	"10.0.1.1"
@@ -820,6 +821,18 @@ static void test_burst(void)
 	mptcp_bpf_burst__destroy(skel);
 }
 
+static void test_avoid(void)
+{
+	struct mptcp_bpf_avoid *skel;
+
+	skel = mptcp_bpf_avoid__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load: avoid"))
+		return;
+
+	test_bpf_sched(skel->maps.avoid, "avoid", WITH_DATA, WITHOUT_DATA);
+	mptcp_bpf_avoid__destroy(skel);
+}
+
 void test_mptcp(void)
 {
 	if (test__start_subtest("base"))
@@ -842,4 +855,6 @@ void test_mptcp(void)
 		test_red();
 	if (test__start_subtest("burst"))
 		test_burst();
+	if (test__start_subtest("avoid"))
+		test_avoid();
 }
diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_avoid.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_avoid.c
new file mode 100644
index 000000000000..359b51580087
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_avoid.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026, MPTCP. */
+
+#include "mptcp_bpf.h"
+#include <bpf/bpf_tracing.h>
+
+extern bool mptcp_subflow_active(struct mptcp_subflow_context *subflow) __ksym;
+extern void mptcp_subflow_set_avoid(struct mptcp_subflow_context *subflow,
+				    bool avoid) __ksym;
+
+char _license[] SEC("license") = "GPL";
+
+struct mptcp_avoid_storage {
+	bool avoid_marked;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, struct mptcp_avoid_storage);
+} mptcp_avoid_map SEC(".maps");
+
+SEC("struct_ops")
+void BPF_PROG(mptcp_sched_avoid_init, struct mptcp_sock *msk)
+{
+	bpf_sk_storage_get(&mptcp_avoid_map, msk, 0,
+			   BPF_LOCAL_STORAGE_GET_F_CREATE);
+}
+
+SEC("struct_ops")
+void BPF_PROG(mptcp_sched_avoid_release, struct mptcp_sock *msk)
+{
+	bpf_sk_storage_delete(&mptcp_avoid_map, msk);
+}
+
+SEC("struct_ops")
+int BPF_PROG(bpf_avoid_get_send, struct mptcp_sock *msk)
+{
+	struct mptcp_subflow_context *subflow;
+	struct mptcp_avoid_storage *ptr;
+
+	ptr = bpf_sk_storage_get(&mptcp_avoid_map, msk, 0,
+				 BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!ptr)
+		return -1;
+
+	/*
+	 * set once, on the first scheduling round, never touched again --
+	 * this is what makes "avoid" different from "scheduled": nothing
+	 * here re-asserts the decision, and the core doesn't clear it
+	 * either.
+	 */
+	if (!ptr->avoid_marked) {
+		int i = 0;
+
+		/* bench every subflow but the first, once */
+		bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk) {
+			if (i++ > 0)
+				mptcp_subflow_set_avoid(subflow, true);
+		}
+		ptr->avoid_marked = true;
+	}
+
+	/*
+	 * every round -- including this one -- just reads back state set
+	 * at most once above. no re-assertion, ever.
+	 */
+	bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk) {
+		if (subflow->avoid)
+			continue;
+		if (mptcp_subflow_active(subflow)) {
+			mptcp_subflow_set_scheduled(subflow, true);
+			break;
+		}
+	}
+	return 0;
+}
+
+SEC(".struct_ops.link")
+struct mptcp_sched_ops avoid = {
+	.init		= (void *)mptcp_sched_avoid_init,
+	.release	= (void *)mptcp_sched_avoid_release,
+	.get_send	= (void *)bpf_avoid_get_send,
+	.name		= "bpf_avoid",
+};
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH mptcp-next v4 2/2] selftests: mptcp: bpf: exercise the subflow avoid flag across rounds
  2026-09-14  9:20 ` [PATCH mptcp-next v4 2/2] selftests: mptcp: bpf: exercise the subflow avoid flag across rounds Kalpan Jani
@ 2026-09-14  9:32   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-14  9:32 UTC (permalink / raw)
  To: Kalpan Jani; +Cc: mptcp

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `test_avoid` selftest contains fundamental logic flaws that cause it to silently pass without ever exercising the `avoid` functionality, resulting in a false positive test.
--

commit 23085ecdc8efb175d47c716d935708a8072a4cd5
Author: Kalpan Jani <kalpan.jani@mpiricsoftware.com>

selftests: mptcp: bpf: exercise the subflow avoid flag across rounds

This commit adds a BPF struct_ops scheduler selftest for 
mptcp_subflow_set_avoid(). It attempts to mark subflows as avoided during the
first scheduling round and verifies they are skipped in subsequent rounds.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/349

> diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_avoid.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_avoid.c
> new file mode 100644
> index 0000000000000..359b515800875
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_avoid.c

[ ... ]

> +SEC("struct_ops")
> +int BPF_PROG(bpf_avoid_get_send, struct mptcp_sock *msk)
> +{
> +	struct mptcp_subflow_context *subflow;
> +	struct mptcp_avoid_storage *ptr;
> +
> +	ptr = bpf_sk_storage_get(&mptcp_avoid_map, msk, 0,
> +				 BPF_LOCAL_STORAGE_GET_F_CREATE);
> +	if (!ptr)
> +		return -1;
> +
> +	/*
> +	 * set once, on the first scheduling round, never touched again --
> +	 * this is what makes "avoid" different from "scheduled": nothing
> +	 * here re-asserts the decision, and the core doesn't clear it
> +	 * either.
> +	 */
> +	if (!ptr->avoid_marked) {
> +		int i = 0;
> +
> +		/* bench every subflow but the first, once */
> +		bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk) {
> +			if (i++ > 0)
> +				mptcp_subflow_set_avoid(subflow, true);
> +		}
> +		ptr->avoid_marked = true;
> +	}

[Severity: Medium]
Since bpf_avoid_get_send() is called immediately when the first subflow is 
established (and before the second subflow finishes its MP_JOIN), is `i++ > 0` 
ever true here?

If there is only one subflow when this initialization block runs, `avoid` will 
never be set for any subflow, but `avoid_marked` becomes true, which permanently 
locks out this block.

> +
> +	/*
> +	 * every round -- including this one -- just reads back state set
> +	 * at most once above. no re-assertion, ever.
> +	 */
> +	bpf_for_each(mptcp_subflow, subflow, (struct sock *)msk) {
> +		if (subflow->avoid)
> +			continue;
> +		if (mptcp_subflow_active(subflow)) {
> +			mptcp_subflow_set_scheduled(subflow, true);
> +			break;
> +		}
> +	}
> +	return 0;
> +}

[Severity: Medium]
Because the first subflow is never marked to be avoided, does this loop always 
break on the first active subflow?

Since the first subflow naturally handles the entire data transfer, it is always 
selected, and the second subflow is never evaluated. Does this cause the test 
to artificially pass and provide a false positive result, regardless of whether 
the `avoid` feature actually works?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914092000.1306968-1-kalpan.jani@mpiricsoftware.com?part=2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid
  2026-09-14  9:19 [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid Kalpan Jani
  2026-09-14  9:19 ` [PATCH mptcp-next v4 1/2] mptcp: sched: add subflow avoid flag and enforce it in default sched Kalpan Jani
  2026-09-14  9:20 ` [PATCH mptcp-next v4 2/2] selftests: mptcp: bpf: exercise the subflow avoid flag across rounds Kalpan Jani
@ 2026-09-14 10:29 ` MPTCP CI
  2 siblings, 0 replies; 5+ messages in thread
From: MPTCP CI @ 2026-09-14 10:29 UTC (permalink / raw)
  To: Kalpan Jani; +Cc: mptcp

Hi Kalpan,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Unstable: 1 failed test(s): packetdrill_fastclose ⚠️ 
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/34828601595

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/b7129410a677
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1164353


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-14 10:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14  9:19 [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid Kalpan Jani
2026-09-14  9:19 ` [PATCH mptcp-next v4 1/2] mptcp: sched: add subflow avoid flag and enforce it in default sched Kalpan Jani
2026-09-14  9:20 ` [PATCH mptcp-next v4 2/2] selftests: mptcp: bpf: exercise the subflow avoid flag across rounds Kalpan Jani
2026-09-14  9:32   ` sashiko-bot
2026-09-14 10:29 ` [PATCH mptcp-next v4 0/2] mptcp: sched: let schedulers mark a subflow to avoid MPTCP CI

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox