All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow
@ 2026-10-25 13:46 Abdul Wasey
  2026-10-25 13:46 ` [RFC PATCH mptcp-next 1/1] " Abdul Wasey
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Abdul Wasey @ 2026-10-25 13:46 UTC (permalink / raw)
  To: mptcp; +Cc: Matthieu Baerts, Mat Martineau, Geliang Tang, Paolo Abeni

This is an attempt at issue #332, the "opportunistic retransmission" of
section 4.2 of the NSDI 2012 paper, which the out-of-tree kernel had.

I found it from a multi-WAN router: a fast link next to a slow one gets
much less than the fast link alone, because data sent on the slow link
holds back the DATA_ACK until the receive window or the send buffer is
used up. With the default tcp_rmem, a 400 Mbit link next to a
15 Mbit/240 ms one gave 67-82 Mbit in total with BBR, and 30-142 with
CUBIC. With this patch it is 320-345, and uploads gain the same.

When a DATA_ACK shows that no new data can go out and an active subflow
is idle, the data the slower subflows hold is resent on it: the ranges
from the DSS mappings in their queues, not everything from snd_una. It
only does so when the copy can arrive at least one RTT of the idle
subflow sooner, and no more than the idle subflow can send in that time.
Without these limits it cost about 1% on simult_flows.sh's unbalanced
cases, and even waking the worker for nothing showed there, so the
decision is made before scheduling it.

A subflow that had a retransmission timeout and no progress since is
left alone: the copies would keep the MPTCP-level timeout from firing,
and with it the stale detection (mptcp_join.sh's "multiple flows,
signal, link failure" caught that).

Tested in netns on the current export, against the same kernel without
the patch: nine 2-path cases with BBR and CUBIC, 6 and 32 MB tcp_rmem,
uploads and downloads, 3 runs each and 16 for the slow lossy path.
mptcp_connect.sh, mptcp_join.sh (twice) and simult_flows.sh (16 runs)
pass as they do without the patch. The 2-path cases, mptcp_connect.sh
and simult_flows.sh also ran on a KASAN + lockdep kernel, with no
report, and on a KCSAN one, whose reports are the same data races in
mptcp_subflow_get_send() and mptcp_subflow_active() that it shows
without the patch.

Questions, since this is an RFC:
- It is in the core, for every scheduler. Should it rather be a
  scheduler hook (#344), with the default scheduler using it?
- The rules (slower, one RTT of gain, budget) come from my tests. Is
  there a better signal to use here?
- mptcp_oppr_check() runs on each DATA_ACK and reads the other
  subflows' queues and RTTs without their locks: is that acceptable as a
  heuristic, or should it be done differently?
- I could not find a short simult_flows.sh case where the stock kernel
  is clearly slower: the stall needs a few seconds of traffic to build
  up. Would a counter based test (OppReinject moving on a lossy slow
  path, with the data checked) be enough?

Abdul Wasey (1):
  mptcp: opportunistic reinjection on an idle subflow

 net/mptcp/mib.c      |   2 +
 net/mptcp/mib.h      |   2 +
 net/mptcp/options.c  |   1 +
 net/mptcp/protocol.c | 291 +++++++++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.h |   3 +
 5 files changed, 299 insertions(+)

-- 
2.43.0


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

* [RFC PATCH mptcp-next 1/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-25 13:46 [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow Abdul Wasey
@ 2026-10-25 13:46 ` Abdul Wasey
  2026-10-25 14:05   ` sashiko-bot
  2026-10-25 14:54 ` [RFC PATCH mptcp-next 0/1] " MPTCP CI
  2026-10-25 15:12 ` Matthieu Baerts
  2 siblings, 1 reply; 10+ messages in thread
From: Abdul Wasey @ 2026-10-25 13:46 UTC (permalink / raw)
  To: mptcp; +Cc: Matthieu Baerts, Mat Martineau, Geliang Tang, Paolo Abeni

With subflows of different speeds, data sent on a slow or lossy one holds
back the DATA_ACK while the fast one keeps delivering what comes after
it. Once the receive window or the send buffer is used up, nothing new
can be sent: the fast subflow drains and stays idle until the slow one
delivers, or until the MPTCP-level retransmission timer fires.

This is the "opportunistic retransmission" of section 4.2 of Raiciu et
al., NSDI 2012, which the out-of-tree kernel had in
mptcp_rcv_buf_optimization(). When a DATA_ACK shows that no new data can
go out while an active subflow has nothing queued or in flight, resend
on that subflow the DSN ranges the other subflows still hold, read from
the DSS mappings in their queues: resending everything from snd_una
would mostly duplicate data the peer already has.

Only the data of a subflow that is gone, or of one slower than the idle
subflow and with an RTT long enough for the copies to arrive at least
one RTT of the idle subflow sooner, is resent. A subflow that had a
retransmission timeout and no progress since may be broken: it is left
to the stale detection, which needs the MPTCP-level timeouts the copies
would avoid. At most once per snd_una value, and at most what the idle
subflow can send in the time saved: between paths of similar RTTs the
copies would only take capacity from the idle subflow.

In a 2-path netns setup with the default tcp_rmem, a single download
(Mbit/s, runs without / with the patch):
- 400 Mbit/20 ms + 15 Mbit/240 ms: 67-82 -> 320-332 with BBR (6 / 6),
  30-142 -> 322-345 with CUBIC (6 / 3);
- 40 Mbit/20 ms + 2 Mbit/500 ms with 5% loss: median 19 -> 34 with BBR
  (16 / 16), 21 -> 37 with CUBIC (16 / 13). 3 of the 16 BBR runs gain
  less (14-23).
Uploads gain the same: 72-76 -> 321-328 and 9-28 -> 34-37 with BBR,
32-134 -> 317-331 and 4-18 -> 37 with CUBIC (3 runs each).
Balanced paths, two lossy paths, a slow path with a deep buffer and
jittery 4G-like paths stay within the noise in both directions, also
with a 32 MB tcp_rmem, and simult_flows.sh times are the same as without
this patch (16 runs each; its opposed, unbalanced delay case missed the
time limit once with and once without it).

Two new MIB counters, OppReinject and OppReinjectBytes, count the events
and the bytes resent.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/332
Signed-off-by: Abdul Wasey <w453y.me@gmail.com>
---
 net/mptcp/mib.c      |   2 +
 net/mptcp/mib.h      |   2 +
 net/mptcp/options.c  |   1 +
 net/mptcp/protocol.c | 291 +++++++++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.h |   3 +
 5 files changed, 299 insertions(+)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index 608cb568897c..5ef574679bd1 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -95,6 +95,8 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("BacklogDrop", MPTCP_MIB_BACKLOGDROP),
 	SNMP_MIB_ITEM("RcvPruned", MPTCP_MIB_RCVPRUNED),
 	SNMP_MIB_ITEM("OFOPruned", MPTCP_MIB_OFOPRUNED),
+	SNMP_MIB_ITEM("OppReinject", MPTCP_MIB_OPPREINJ),
+	SNMP_MIB_ITEM("OppReinjectBytes", MPTCP_MIB_OPPREINJBYTES),
 };
 
 /* mptcp_mib_alloc - allocate percpu mib counters
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 1ebdb55e9534..e700b17b8e5d 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -98,6 +98,8 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_BACKLOGDROP,		/* Backlog over memory limit */
 	MPTCP_MIB_RCVPRUNED,		/* Dropped due to memory constraints */
 	MPTCP_MIB_OFOPRUNED,		/* MPTCP-level OoO queue pruned */
+	MPTCP_MIB_OPPREINJ,		/* Opportunistic reinjections on an idle subflow */
+	MPTCP_MIB_OPPREINJBYTES,	/* Bytes sent by opportunistic reinjections */
 	__MPTCP_MIB_MAX
 };
 
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index adbf8e0773db..b2b4e6b23b6b 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1154,6 +1154,7 @@ static void ack_update_msk(struct mptcp_sock *msk,
 		__mptcp_data_acked(sk);
 	}
 	msk->last_ack_recv = tcp_jiffies32;
+	mptcp_oppr_check(sk);
 	mptcp_data_unlock(sk);
 
 	trace_ack_update_msk(mp_opt->data_ack,
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 02a4f479d9d8..cd9aebd66b1e 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2964,6 +2964,294 @@ static int __mptcp_push_retrans(struct sock *sk, struct mptcp_data_frag *dfrag,
 	return len;
 }
 
+/* Opportunistic reinjection, "opportunistic retransmission" in section 4.2
+ * of Raiciu et al., NSDI 2012.
+ *
+ * With paths of different speeds, data sent on a slow or lossy path holds
+ * back the DATA_ACK while the fast path keeps delivering what comes after
+ * it. Once the receive window or the send buffer is used up, nothing new
+ * can be sent: the fast path drains and stays idle until the slow path
+ * delivers, or until the MPTCP-level RTO fires.
+ *
+ * That state can be seen when a DATA_ACK comes in: no new data can go out,
+ * and an active subflow has nothing queued or in flight. The data at
+ * snd_una is then held by another subflow, so resend on the idle subflow
+ * the ranges that the other subflows still hold, taken from the DSS
+ * mappings in their queues, instead of everything from snd_una: most of
+ * that already reached the peer.
+ *
+ * Only data held by a subflow that is gone, or that is slower than the
+ * idle one and whose RTT is long enough for a copy to arrive at least one
+ * RTT of the idle subflow sooner. Below that, the copies cost the idle
+ * subflow more than they save. This is done at most once per snd_una
+ * value, and at most what the idle subflow can send in the time saved.
+ */
+#define MPTCP_OPPR_MIN		(16 * 1024)
+#define MPTCP_OPPR_BUDGET	(256 * 1024)
+#define MPTCP_OPPR_MAX_RANGES	32
+
+struct mptcp_oppr_range {
+	u64 start;
+	u64 end;
+};
+
+/* the active subflow with nothing to send or retransmit and the lowest RTT */
+static struct sock *mptcp_oppr_idle_ssk(struct mptcp_sock *msk)
+{
+	struct mptcp_subflow_context *subflow;
+	struct sock *pick = NULL;
+	u32 best = U32_MAX;
+
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+		u32 rtt = READ_ONCE(tcp_sk(ssk)->srtt_us);
+
+		if (!__mptcp_subflow_active(subflow) || subflow->backup ||
+		    subflow->request_bkup || subflow->stale || !rtt)
+			continue;
+		if (!tcp_rtx_and_write_queues_empty(ssk))
+			continue;
+		if (rtt < best) {
+			best = rtt;
+			pick = ssk;
+		}
+	}
+	return pick;
+}
+
+static bool mptcp_oppr_blocked(struct sock *sk)
+{
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	u64 snd_una = READ_ONCE(msk->snd_una);
+
+	if (snd_una == READ_ONCE(msk->snd_nxt) ||
+	    snd_una == READ_ONCE(msk->oppr_una) ||
+	    __mptcp_check_fallback(msk))
+		return false;
+
+	/* new data can still go out: the scheduler will use the idle subflow */
+	if (mptcp_send_head(sk) &&
+	    mptcp_wnd_end(msk) - READ_ONCE(msk->snd_nxt) >= MPTCP_OPPR_MIN)
+		return false;
+	return true;
+}
+
+/* How much earlier, in us, a copy sent now on @idle would reach the peer
+ * than the data held by @ssk: about half an RTT of each, the current one of
+ * @ssk and the base one of @idle, whose queue is empty. 0 if @ssk is not
+ * slower than @idle, or if the copy would not save at least one RTT of
+ * @idle: with a smaller gain, the copies cost more than they save.
+ */
+static u32 mptcp_oppr_gain(const struct sock *ssk, const struct sock *idle)
+{
+	u32 srtt = READ_ONCE(tcp_sk(ssk)->srtt_us) >> 3;
+	u32 min_rtt = tcp_min_rtt(tcp_sk(idle));
+
+	/* A retransmission timeout without progress since: the subflow may
+	 * be broken, which the stale detection handles (it needs the
+	 * MPTCP-level timeouts that copies would avoid).
+	 */
+	if (READ_ONCE(inet_csk(ssk)->icsk_retransmits))
+		return 0;
+	if (READ_ONCE(ssk->sk_pacing_rate) >= READ_ONCE(idle->sk_pacing_rate) ||
+	    min_rtt == ~0U || srtt <= 3 * min_rtt)
+		return 0;
+	return (srtt - min_rtt) / 2;
+}
+
+/* Does a subflow other than @idle hold data worth resending on it: one
+ * that is gone, or one slow enough for the copies to arrive sooner?
+ */
+static bool mptcp_oppr_worth(struct mptcp_sock *msk, struct sock *idle)
+{
+	struct mptcp_subflow_context *subflow;
+
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+		if (ssk == idle || tcp_rtx_and_write_queues_empty(ssk))
+			continue;
+		if (!__mptcp_subflow_active(subflow) ||
+		    mptcp_oppr_gain(ssk, idle))
+			return true;
+	}
+	return false;
+}
+
+/* called on each DATA_ACK, under the msk data lock */
+void mptcp_oppr_check(struct sock *sk)
+{
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	struct sock *idle;
+
+	if (!mptcp_oppr_blocked(sk))
+		return;
+	idle = mptcp_oppr_idle_ssk(msk);
+	if (!idle || !mptcp_oppr_worth(msk, idle))
+		return;
+	if (!test_and_set_bit(MPTCP_WORK_OPPR, &msk->flags))
+		mptcp_schedule_work(sk);
+}
+
+static int mptcp_oppr_add_skb(const struct sk_buff *skb, u64 una, u64 *last,
+			      struct mptcp_oppr_range *r, int n)
+{
+	const struct mptcp_ext *mpext = mptcp_get_ext(skb);
+	u64 start, end;
+
+	if (!mpext || !mpext->use_map || !mpext->data_len ||
+	    mpext->data_seq == *last || n >= MPTCP_OPPR_MAX_RANGES)
+		return n;
+
+	/* one mapping can span several skbs after a TCP split */
+	*last = mpext->data_seq;
+	start = mpext->data_seq;
+	end = start + mpext->data_len;
+	if (!after64(end, una))
+		return n;
+
+	r[n].start = before64(start, una) ? una : start;
+	r[n].end = end;
+	return n + 1;
+}
+
+/* The DSN ranges at or after @una that the subflows slower than @idle still
+ * hold, sent or not: the data the peer is waiting for. Sorted by start.
+ * @gain is set to the largest time the copies can save.
+ */
+static int mptcp_oppr_collect(struct mptcp_sock *msk, struct sock *idle,
+			      u64 una, struct mptcp_oppr_range *r, u32 *gain)
+{
+	struct mptcp_subflow_context *subflow;
+	int n = 0, i, k;
+
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+		struct sk_buff *skb;
+		u64 last = 0;
+		u32 g = 0;
+
+		if (ssk == idle)
+			continue;
+		if (__mptcp_subflow_active(subflow)) {
+			g = mptcp_oppr_gain(ssk, idle);
+			if (!g)
+				continue;
+		}
+		*gain = max(*gain, g);
+
+		lock_sock(ssk);
+		skb_rbtree_walk(skb, &ssk->tcp_rtx_queue)
+			n = mptcp_oppr_add_skb(skb, una, &last, r, n);
+		skb_queue_walk(&ssk->sk_write_queue, skb)
+			n = mptcp_oppr_add_skb(skb, una, &last, r, n);
+		release_sock(ssk);
+	}
+
+	for (i = 1; i < n; i++) {
+		struct mptcp_oppr_range t = r[i];
+
+		for (k = i; k > 0 && after64(r[k - 1].start, t.start); k--)
+			r[k] = r[k - 1];
+		r[k] = t;
+	}
+	return n;
+}
+
+/* resend [start, end) on @ssk from the msk rtx queue; < 0 on a send error */
+static int mptcp_oppr_send_range(struct sock *sk, struct sock *ssk, u64 start,
+				 u64 end, int budget,
+				 struct mptcp_sendmsg_info *info)
+{
+	struct mptcp_data_frag *dfrag;
+	int copied = 0, ret;
+
+	list_for_each_entry(dfrag, &mptcp_sk(sk)->rtx_queue, list) {
+		if (!dfrag->already_sent || !before64(dfrag->data_seq, end))
+			break;
+		if (!after64(dfrag->data_seq + dfrag->already_sent, start))
+			continue;
+
+		info->sent = before64(dfrag->data_seq, start) ?
+			     start - dfrag->data_seq : 0;
+		info->limit = min_t(u64, dfrag->already_sent,
+				    end - dfrag->data_seq);
+		while (info->sent < info->limit && copied < budget) {
+			ret = mptcp_sendmsg_frag(sk, ssk, dfrag, info);
+			if (ret <= 0)
+				return copied ? : -1;
+			copied += ret;
+			info->sent += ret;
+		}
+		if (copied >= budget)
+			break;
+	}
+	return copied;
+}
+
+static void __mptcp_oppr(struct sock *sk)
+{
+	struct mptcp_sendmsg_info info = { .data_lock_held = true, };
+	struct mptcp_oppr_range r[MPTCP_OPPR_MAX_RANGES];
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	int copied = 0, budget, ret, n, i;
+	struct sock *ssk;
+	u32 gain = 0;
+	u64 una;
+
+	mptcp_data_lock(sk);
+	__mptcp_clean_una_wakeup(sk);
+	mptcp_data_unlock(sk);
+
+	/* with csum, the retransmission could send new data */
+	if (!mptcp_oppr_blocked(sk) || READ_ONCE(msk->csum_enabled))
+		return;
+	ssk = mptcp_oppr_idle_ssk(msk);
+	if (!ssk || !mptcp_rtx_head(sk))
+		return;
+
+	una = msk->snd_una;
+	WRITE_ONCE(msk->oppr_una, una);
+	n = mptcp_oppr_collect(msk, ssk, una, r, &gain);
+	if (!n)
+		return;
+
+	lock_sock(ssk);
+	/* The copies delay what @ssk sends next: no more than it can send in
+	 * the time they save. A subflow that is gone saves an MPTCP RTO.
+	 */
+	budget = min_t(int, MPTCP_OPPR_BUDGET,
+		       tcp_snd_cwnd(tcp_sk(ssk)) * tcp_sk(ssk)->mss_cache);
+	if (gain)
+		budget = min_t(u64, budget,
+			       div_u64((u64)READ_ONCE(ssk->sk_pacing_rate) * gain,
+				       USEC_PER_SEC));
+
+	spin_lock_bh(&msk->fallback_lock);
+	if (__mptcp_check_fallback(msk) || !msk->allow_subflows)
+		goto unlock;
+
+	for (i = 0; i < n && copied < budget; i++) {
+		ret = mptcp_oppr_send_range(sk, ssk, r[i].start, r[i].end,
+					    budget - copied, &info);
+		if (ret < 0)
+			break;
+		copied += ret;
+	}
+
+	if (copied) {
+		tcp_push(ssk, 0, info.mss_now, tcp_sk(ssk)->nonagle,
+			 info.size_goal);
+		msk->allow_infinite_fallback = false;
+		msk->bytes_retrans += copied;
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OPPREINJ);
+		MPTCP_ADD_STATS(sock_net(sk), MPTCP_MIB_OPPREINJBYTES, copied);
+	}
+unlock:
+	spin_unlock_bh(&msk->fallback_lock);
+	release_sock(ssk);
+}
+
 static void __mptcp_retrans(struct sock *sk)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
@@ -3202,6 +3490,9 @@ static void mptcp_worker(struct work_struct *work)
 	if (test_and_clear_bit(MPTCP_WORK_RTX, &msk->flags))
 		__mptcp_retrans(sk);
 
+	if (test_and_clear_bit(MPTCP_WORK_OPPR, &msk->flags))
+		__mptcp_oppr(sk);
+
 	fail_tout = msk->first ? READ_ONCE(mptcp_subflow_ctx(msk->first)->fail_tout) : 0;
 	if (fail_tout && time_after(jiffies, fail_tout))
 		mptcp_mp_fail_no_response(msk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3be60da12924..5bd0f0479f9e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -117,6 +117,7 @@
 #define MPTCP_FALLBACK_DONE	2
 #define MPTCP_WORK_CLOSE_SUBFLOW 3
 #define MPTCP_RTX_ENABLED	4
+#define MPTCP_WORK_OPPR		5
 
 /* MPTCP socket release cb flags */
 #define MPTCP_PUSH_PENDING	1
@@ -310,6 +311,7 @@ struct mptcp_sock {
 	atomic64_t	rcv_wnd_sent;
 	u64		rcv_data_fin_seq;
 	u64		bytes_retrans;
+	u64		oppr_una;	/* snd_una at the last opportunistic reinjection */
 	u64		bytes_consumed;
 	int		snd_burst;
 	int		old_wspace;
@@ -1003,6 +1005,7 @@ static inline u64 mptcp_expand_seq(u64 old_seq, u64 cur_seq, bool use_64bit)
 }
 void __mptcp_check_push(struct sock *sk, struct sock *ssk);
 void __mptcp_data_acked(struct sock *sk);
+void mptcp_oppr_check(struct sock *sk);
 void __mptcp_error_report(struct sock *sk);
 bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq, bool use_64bit);
 static inline bool mptcp_data_fin_enabled(const struct mptcp_sock *msk)
-- 
2.43.0


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

* Re: [RFC PATCH mptcp-next 1/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-25 13:46 ` [RFC PATCH mptcp-next 1/1] " Abdul Wasey
@ 2026-10-25 14:05   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-25 14:05 UTC (permalink / raw)
  To: Abdul Wasey, mptcp

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] mptcp: race on msk->conn_list in mptcp_oppr_check() when socket owned
- [Medium] mptcp: missing csum_enabled check in mptcp_oppr_blocked()
- [Medium] mptcp: mptcp_oppr_send_range() does not cap info->limit to budget
- [Medium] mptcp: mptcp_oppr_collect() does not merge overlapping DSN ranges
- [Medium] mptcp: flawed gain and oppr_una tracking in mptcp_oppr_collect()
--

commit 44cedfbaa2769a1dea51657a670dc2d6b650eaf1
Author: Abdul Wasey <w453y.me@gmail.com>

mptcp: opportunistic reinjection on an idle subflow

This commit implements opportunistic reinjection for MPTCP when a connection
cannot send new data while an active subflow is idle and another slower or
inactive subflow still holds unacknowledged data. It collects the unacked
DSN ranges from the other subflows and resends them on the idle subflow via
the MPTCP worker.

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index adbf8e0773dbb..b2b4e6b23b6b8 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1154,6 +1154,7 @@ static void ack_update_msk(struct mptcp_sock *msk,
>  		__mptcp_data_acked(sk);
>  	}
>  	msk->last_ack_recv = tcp_jiffies32;
> +	mptcp_oppr_check(sk);
>  	mptcp_data_unlock(sk);
>  
>  	trace_ack_update_msk(mp_opt->data_ack,
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 02a4f479d9d8c..cd9aebd66b1e3 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c

[ ... ]

> +/* the active subflow with nothing to send or retransmit and the lowest RTT */
> +static struct sock *mptcp_oppr_idle_ssk(struct mptcp_sock *msk)
> +{
> +	struct mptcp_subflow_context *subflow;
> +	struct sock *pick = NULL;
> +	u32 best = U32_MAX;
> +
> +	mptcp_for_each_subflow(msk, subflow) {
> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +		u32 rtt = READ_ONCE(tcp_sk(ssk)->srtt_us);
> +
> +		if (!__mptcp_subflow_active(subflow) || subflow->backup ||
> +		    subflow->request_bkup || subflow->stale || !rtt)
> +			continue;
> +		if (!tcp_rtx_and_write_queues_empty(ssk))
> +			continue;
> +		if (rtt < best) {
> +			best = rtt;
> +			pick = ssk;
> +		}
> +	}
> +	return pick;
> +}
> +
> +static bool mptcp_oppr_blocked(struct sock *sk)
> +{
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	u64 snd_una = READ_ONCE(msk->snd_una);
> +
> +	if (snd_una == READ_ONCE(msk->snd_nxt) ||
> +	    snd_una == READ_ONCE(msk->oppr_una) ||
> +	    __mptcp_check_fallback(msk))
> +		return false;

[Severity: Medium]
Should mptcp_oppr_blocked() also check READ_ONCE(msk->csum_enabled) here?

In __mptcp_oppr(), the function returns early when msk->csum_enabled is set,
before msk->oppr_una is updated to msk->snd_una:

    if (!mptcp_oppr_blocked(sk) || READ_ONCE(msk->csum_enabled))
            return;
    ...
    una = msk->snd_una;
    WRITE_ONCE(msk->oppr_una, una);

Because mptcp_oppr_blocked() does not check msk->csum_enabled,
ack_update_msk() -> mptcp_oppr_check() will walk msk->conn_list twice and
schedule mptcp_worker on every incoming DATA_ACK while blocked when data
checksums are enabled, only for __mptcp_oppr() to immediately return without
updating msk->oppr_una.

> +
> +	/* new data can still go out: the scheduler will use the idle subflow */
> +	if (mptcp_send_head(sk) &&
> +	    mptcp_wnd_end(msk) - READ_ONCE(msk->snd_nxt) >= MPTCP_OPPR_MIN)
> +		return false;
> +	return true;
> +}

[ ... ]

> +static bool mptcp_oppr_worth(struct mptcp_sock *msk, struct sock *idle)
> +{
> +	struct mptcp_subflow_context *subflow;
> +
> +	mptcp_for_each_subflow(msk, subflow) {
> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +
> +		if (ssk == idle || tcp_rtx_and_write_queues_empty(ssk))
> +			continue;
> +		if (!__mptcp_subflow_active(subflow) ||
> +		    mptcp_oppr_gain(ssk, idle))
> +			return true;
> +	}
> +	return false;
> +}
> +
> +/* called on each DATA_ACK, under the msk data lock */
> +void mptcp_oppr_check(struct sock *sk)
> +{
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct sock *idle;
> +
> +	if (!mptcp_oppr_blocked(sk))
> +		return;
> +	idle = mptcp_oppr_idle_ssk(msk);
> +	if (!idle || !mptcp_oppr_worth(msk, idle))
> +		return;
> +	if (!test_and_set_bit(MPTCP_WORK_OPPR, &msk->flags))
> +		mptcp_schedule_work(sk);
> +}

[Severity: Critical]
Can calling mptcp_oppr_check() from ack_update_msk() race with concurrent
modifications to msk->conn_list and subflow teardown when the msk socket is
owned by user context?

In ack_update_msk(), only mptcp_data_lock(sk) (sk->sk_lock.slock) is held.
When a process-context thread holds lock_sock(sk), sk->sk_lock.owned is set
while sk->sk_lock.slock is released. Under lock_sock(sk) and without holding
mptcp_data_lock(sk), msk->conn_list is modified with non-RCU list operations
and subflows are closed and freed, for instance in __mptcp_close_ssk():

    dispose_it = msk->free_first || ssk != msk->first;
    if (dispose_it)
            list_del(&subflow->node);
    ...
    __tcp_close(ssk, 0);
    ...
    sock_put(ssk);

As well as list_add_tail() / list_del() in __mptcp_subflow_connect(),
list_move_tail() in __mptcp_flush_join_list(), and list_del() in
mptcp_subflow_drop_ctx(). Similarly, msk->first_pending (read via
mptcp_send_head(sk) in mptcp_oppr_blocked()) is updated under the msk socket
lock without holding mptcp_data_lock(sk).

Unlike __mptcp_check_push() and __mptcp_data_acked() in ack_update_msk(),
which check !sock_owned_by_user(sk) and defer work via msk->cb_flags when
the socket is owned, mptcp_oppr_check() walks msk->conn_list in
mptcp_oppr_idle_ssk() and mptcp_oppr_worth() and dereferences subflow and
ssk unconditionally. Could this dereference LIST_POISON1/LIST_POISON2 or a
freed ssk/subflow in softirq context?

[ ... ]

> +/* The DSN ranges at or after @una that the subflows slower than @idle still
> + * hold, sent or not: the data the peer is waiting for. Sorted by start.
> + * @gain is set to the largest time the copies can save.
> + */
> +static int mptcp_oppr_collect(struct mptcp_sock *msk, struct sock *idle,
> +			      u64 una, struct mptcp_oppr_range *r, u32 *gain)
> +{
> +	struct mptcp_subflow_context *subflow;
> +	int n = 0, i, k;
> +
> +	mptcp_for_each_subflow(msk, subflow) {
> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +		struct sk_buff *skb;
> +		u64 last = 0;
> +		u32 g = 0;
> +
> +		if (ssk == idle)
> +			continue;
> +		if (__mptcp_subflow_active(subflow)) {
> +			g = mptcp_oppr_gain(ssk, idle);
> +			if (!g)
> +				continue;
> +		}
> +		*gain = max(*gain, g);
> +
> +		lock_sock(ssk);
> +		skb_rbtree_walk(skb, &ssk->tcp_rtx_queue)
> +			n = mptcp_oppr_add_skb(skb, una, &last, r, n);
> +		skb_queue_walk(&ssk->sk_write_queue, skb)
> +			n = mptcp_oppr_add_skb(skb, una, &last, r, n);
> +		release_sock(ssk);
> +	}

[Severity: Medium]
Does updating *gain = max(*gain, g) before walking the queues of ssk cause
two issues with how gain is tracked?

First, unlike mptcp_oppr_worth(), this loop does not check whether ssk
actually holds any unacked ranges after una before updating *gain, so an
active slow subflow with empty queues can inflate *gain.

Second, for an inactive subflow (!__mptcp_subflow_active(subflow)), g is
left at 0 so that __mptcp_oppr() does not cap budget by the pacing rate
(since a subflow that is gone saves an MPTCP RTO). However, if another
active subflow on the connection has g > 0, *gain = max(*gain, g) overwrites
the 0 sentinel with g, causing __mptcp_oppr() to cap the budget for the
inactive subflow's data.

In addition, in __mptcp_oppr(), WRITE_ONCE(msk->oppr_una, una) is executed
before calling mptcp_oppr_collect() and before checking whether any bytes
were actually copied. If mptcp_oppr_collect() returns 0 or no bytes are
sent, should msk->oppr_una remain unchanged so reinjection is not
permanently suppressed for that snd_una?

> +
> +	for (i = 1; i < n; i++) {
> +		struct mptcp_oppr_range t = r[i];
> +
> +		for (k = i; k > 0 && after64(r[k - 1].start, t.start); k--)
> +			r[k] = r[k - 1];
> +		r[k] = t;
> +	}
> +	return n;
> +}

[Severity: Medium]
What happens if two non-idle subflows hold overlapping or identical DSN
ranges in their queues (for example, after an earlier retransmission or
subflow closure)?

mptcp_oppr_collect() sorts r[] by start sequence number, but it does not
merge overlapping ranges where before64(r[i].start, r[i - 1].end). When
__mptcp_oppr() iterates over r[] and calls mptcp_oppr_send_range() for each
entry, won't it retransmit the overlapping DSN bytes multiple times on the
same idle subflow and consume the budget on duplicate copies?

> +
> +/* resend [start, end) on @ssk from the msk rtx queue; < 0 on a send error */
> +static int mptcp_oppr_send_range(struct sock *sk, struct sock *ssk, u64 start,
> +				 u64 end, int budget,
> +				 struct mptcp_sendmsg_info *info)
> +{
> +	struct mptcp_data_frag *dfrag;
> +	int copied = 0, ret;
> +
> +	list_for_each_entry(dfrag, &mptcp_sk(sk)->rtx_queue, list) {
> +		if (!dfrag->already_sent || !before64(dfrag->data_seq, end))
> +			break;
> +		if (!after64(dfrag->data_seq + dfrag->already_sent, start))
> +			continue;
> +
> +		info->sent = before64(dfrag->data_seq, start) ?
> +			     start - dfrag->data_seq : 0;
> +		info->limit = min_t(u64, dfrag->already_sent,
> +				    end - dfrag->data_seq);
> +		while (info->sent < info->limit && copied < budget) {
> +			ret = mptcp_sendmsg_frag(sk, ssk, dfrag, info);

[Severity: Medium]
Can mptcp_oppr_send_range() exceed the caller's budget when a dfrag range is
larger than the remaining budget?

In mptcp_sendmsg_frag(), the amount copied in a single call is bounded by
info->limit - info->sent:

    copy = info->size_goal;
    ...
    copy = min_t(size_t, copy, info->limit - info->sent);

Because mptcp_oppr_send_range() sets info->limit from dfrag->already_sent
and end - dfrag->data_seq without clamping it to info->sent + budget -
copied, a single call to mptcp_sendmsg_frag() can copy up to info->size_goal
(or the entire dfrag) before the while loop rechecks copied < budget. Should
info->limit also be capped by info->sent + budget - copied?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025134643.1537069-1-w453y.me@gmail.com?part=1

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

* Re: [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-25 13:46 [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow Abdul Wasey
  2026-10-25 13:46 ` [RFC PATCH mptcp-next 1/1] " Abdul Wasey
@ 2026-10-25 14:54 ` MPTCP CI
  2026-10-25 15:12 ` Matthieu Baerts
  2 siblings, 0 replies; 10+ messages in thread
From: MPTCP CI @ 2026-10-25 14:54 UTC (permalink / raw)
  To: Abdul Wasey; +Cc: mptcp

Hi Abdul,

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): Success! ✅
- 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): Unstable: 1 failed test(s): bpftest_test_progs_mptcp ⚠️ 
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/40200388982

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


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] 10+ messages in thread

* Re: [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-25 13:46 [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow Abdul Wasey
  2026-10-25 13:46 ` [RFC PATCH mptcp-next 1/1] " Abdul Wasey
  2026-10-25 14:54 ` [RFC PATCH mptcp-next 0/1] " MPTCP CI
@ 2026-10-25 15:12 ` Matthieu Baerts
  2026-10-25 15:58   ` Abdul Wasey
  2 siblings, 1 reply; 10+ messages in thread
From: Matthieu Baerts @ 2026-10-25 15:12 UTC (permalink / raw)
  To: Abdul Wasey, mptcp
  Cc: Mat Martineau, Geliang Tang, Paolo Abeni, Shardul Bankar

Hi Abdul,

(+cc Shardul)

On 25/10/2026 14:46, Abdul Wasey wrote:
> This is an attempt at issue #332, the "opportunistic retransmission" of
> section 4.2 of the NSDI 2012 paper, which the out-of-tree kernel had.
> 
> I found it from a multi-WAN router: a fast link next to a slow one gets
> much less than the fast link alone, because data sent on the slow link
> holds back the DATA_ACK until the receive window or the send buffer is
> used up. With the default tcp_rmem, a 400 Mbit link next to a
> 15 Mbit/240 ms one gave 67-82 Mbit in total with BBR, and 30-142 with
> CUBIC. With this patch it is 320-345, and uploads gain the same.
> 
> When a DATA_ACK shows that no new data can go out and an active subflow
> is idle, the data the slower subflows hold is resent on it: the ranges
> from the DSS mappings in their queues, not everything from snd_una. It
> only does so when the copy can arrive at least one RTT of the idle
> subflow sooner, and no more than the idle subflow can send in that time.
> Without these limits it cost about 1% on simult_flows.sh's unbalanced
> cases, and even waking the worker for nothing showed there, so the
> decision is made before scheduling it.
> 
> A subflow that had a retransmission timeout and no progress since is
> left alone: the copies would keep the MPTCP-level timeout from firing,
> and with it the stale detection (mptcp_join.sh's "multiple flows,
> signal, link failure" caught that).
> 
> Tested in netns on the current export, against the same kernel without
> the patch: nine 2-path cases with BBR and CUBIC, 6 and 32 MB tcp_rmem,
> uploads and downloads, 3 runs each and 16 for the slow lossy path.
> mptcp_connect.sh, mptcp_join.sh (twice) and simult_flows.sh (16 runs)
> pass as they do without the patch. The 2-path cases, mptcp_connect.sh
> and simult_flows.sh also ran on a KASAN + lockdep kernel, with no
> report, and on a KCSAN one, whose reports are the same data races in
> mptcp_subflow_get_send() and mptcp_subflow_active() that it shows
> without the patch.

Thank you for working on this.

That's indeed something we need to improve.

Before replying to the questions below, I think it would be good if you
rebase your work on top of Shardul's one:


https://lore.kernel.org/20260911-mptcp_penalise_send_v2-v4-0-820f2be4a8fe@mpiricsoftware.com

Note: this work looks good to me, but I was waiting for external reviews
(because I helped a bit) before applying it (and other urgent fixes/work
kept us busy...)

> Questions, since this is an RFC:
> - It is in the core, for every scheduler. Should it rather be a
>   scheduler hook (#344), with the default scheduler using it?

Yes, I think it should (and moved to sched.c), but having a wait for
others schedulers to re-use that. At the end, that's a decision a
scheduler can take depending on the use case. But we want that by default.

When discussing #332 with Shardul, it was clear that the current MPTCP
packet scheduler API is not flexible enough to stop sending on one
subflow and force a reinjection on another subflow. I think it would be
best to take over the following old series, rebase it, address the
comments, and validate it:


https://lore.kernel.org/all/20240527-sched_per_packet-v1-0-09a41d405f7c@gmail.com/t/#u

Once it is there, I think it should be easier to modify the scheduler to
force a reinjection, e.g. set a flag to stop pushing, then find a way to
force using another subflows (e.g. the stale flag or another one). WDYT?
Also, if this work is in place, it would help to add BPF support for the
MPTCP packet scheduler.

Note that I don't know if Shardul is still looking at that. (BTW, sorry
for delaying the work you did with #345 ... note that, in between, I
managed to get Clashiko (Netdev Sashiko) doing reviews not to be blocked
when upstreaming them: so probably we should also check what Clashiko
thinks about this series by resending the last version, to "be prepared"
or to fix issues if any we missed during dev/review).

> - The rules (slower, one RTT of gain, budget) come from my tests. Is
>   there a better signal to use here?

I guess imitating what the old fork was doing is a good starting point.

> - mptcp_oppr_check() runs on each DATA_ACK and reads the other
>   subflows' queues and RTTs without their locks: is that acceptable as a
>   heuristic, or should it be done differently?

Was the fork not limiting this to once per RTT? In any cases, it will be
needed to measure the impact on high perf transfers.

> - I could not find a short simult_flows.sh case where the stock kernel
>   is clearly slower: the stall needs a few seconds of traffic to build
>   up. Would a counter based test (OppReinject moving on a lossy slow
>   path, with the data checked) be enough?

I'm (still :( ) working on a perf lab that might help to measure the
behaviour in different conditions. Would that help here? I don't think
we can have such tests in the selftests: stability needs to be measured
on long runs and in "stable" environments I guess.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


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

* Re: [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-25 15:12 ` Matthieu Baerts
@ 2026-10-25 15:58   ` Abdul Wasey
  2026-10-26 10:13     ` Matthieu Baerts
  0 siblings, 1 reply; 10+ messages in thread
From: Abdul Wasey @ 2026-10-25 15:58 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: mptcp, Mat Martineau, Geliang Tang, Paolo Abeni, Shardul Bankar,
	Gregory Detal

Hi Matt,

I'll rebase on Shardul's series. I'll also test his v4 on my setup and
reply on that thread, since it's still waiting for reviews.

Moving it to the scheduler makes sense. The CI also caught it on v1: the
BPF "first" test saw bytes on its second subflow because of the
reinjection.

Gregory, are you still planning to continue the scheduler API series? If
not, I can pick it up. Shardul, same if you already started on it.

About once per RTT: in the old fork that limit was on the cwnd halving.
The reinjection itself only took the head of the rtx queue, when the
holder's srtt was more than 4x the idle subflow's. I'll check the cost on
fast transfers.

The perf lab would help, yes. I can share my netns cases if that's
useful.

I've fixed what Sashiko found locally, and will fold it into the
scheduler version instead of sending a v2 of this one.

Thanks,
Abdul Wasey

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

* Re: [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-25 15:58   ` Abdul Wasey
@ 2026-10-26 10:13     ` Matthieu Baerts
  2026-10-28  6:29       ` Shardul Bankar
  2026-10-29 19:05       ` Matthieu Baerts
  0 siblings, 2 replies; 10+ messages in thread
From: Matthieu Baerts @ 2026-10-26 10:13 UTC (permalink / raw)
  To: Abdul Wasey
  Cc: mptcp, Mat Martineau, Geliang Tang, Paolo Abeni, Shardul Bankar,
	Gregory Detal

Hi Abdul,

Thank you for your reply.

(Do you mind quoting the text you are replying to, please: it helps to
remember the context, especially when switching between many topics per
day :) )

https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying

While at it, it usually helps to send one version per day, and ideally
wait for Netdev's Sashiko review that can take ~48h, then reply to the
comments on the list before sending a new version. Except if there is an
urgency to fix something of course. By doing that, it's easier for human
reviewers to follow and give guidance. Without that, people might only
look at the last version, and likely after a while or waiting for a
comment to say "Sashiko is happy, please review now". Note that AI
reviews can still miss things or focus on details instead of the big
picture: they might guide a dev to a wrong path.

On 25/10/2026 16:58, Abdul Wasey wrote:
> I'll rebase on Shardul's series. I'll also test his v4 on my setup and
> reply on that thread, since it's still waiting for reviews.

Thank you, that's very helpful!

> Moving it to the scheduler makes sense. The CI also caught it on v1: the
> BPF "first" test saw bytes on its second subflow because of the
> reinjection.
> 
> Gregory, are you still planning to continue the scheduler API series? If
> not, I can pick it up.

For Gregory, I'm sure he is not planning to continue working on that.

> Shardul, same if you already started on it.
> 
> About once per RTT: in the old fork that limit was on the cwnd halving.
> The reinjection itself only took the head of the rtx queue, when the
> holder's srtt was more than 4x the idle subflow's. I'll check the cost on
> fast transfers.

Thanks!

Please also note that for optimisation purposes, the current
implementation tries to push as much as possible on the selected
subflow: it can be quickly costly to stop the delegation and alternate
between subflows.

(Of course, this cost might not be a problem in some cases, e.g. a
"redundant" scheduler is not designed for fast transfers, but for low
latency.)

> The perf lab would help, yes. I can share my netns cases if that's
> useful.

That could indeed. But I'm going to try to push what I have today: it
might be better to work on improving the same thing.

> I've fixed what Sashiko found locally, and will fold it into the
> scheduler version instead of sending a v2 of this one.
Thank you!

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


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

* Re: [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-26 10:13     ` Matthieu Baerts
@ 2026-10-28  6:29       ` Shardul Bankar
  2026-10-28 22:40         ` Abdul Wasey
  2026-10-29 19:05       ` Matthieu Baerts
  1 sibling, 1 reply; 10+ messages in thread
From: Shardul Bankar @ 2026-10-28  6:29 UTC (permalink / raw)
  To: Matthieu Baerts, Abdul Wasey
  Cc: mptcp, Mat Martineau, Geliang Tang, Paolo Abeni, Gregory Detal,
	janak, kalpan.jani

Hi Abdul, Matt,

On Mon, 2026-10-26 at 11:13 +0100, Matthieu Baerts wrote:
> 
> 
> > Moving it to the scheduler makes sense. The CI also caught it on
> > v1: the
> > BPF "first" test saw bytes on its second subflow because of the
> > reinjection.
> > 
> > Gregory, are you still planning to continue the scheduler API
> > series? If
> > not, I can pick it up.
> 
> For Gregory, I'm sure he is not planning to continue working on that.
> 
> > Shardul, same if you already started on it.
> > 

I have not started on it, so please go ahead. I am happy to review the
series as it comes.

Matt, sorry for going quiet on this after we discussed #332.

As you suggested, I am resending the penalize subflow series rebased on
the current export so that Clashiko can have a look.

Abdul, one point on the RFC that should still apply once it moves to
the scheduler, since you asked whether there is a better signal:

mptcp_oppr_blocked() treats an empty send queue (mptcp_send_head() ==
NULL) as "blocked". first_pending is NULL once everything the
application has written so far has been handed to a subflow, which
happens when the send buffer is full, but also when the application
simply has nothing more to write at that moment. In the second case
nothing is stalled, yet the data still held by the slower subflow gets
resent on the idle one at the next DATA_ACK. The commit message
describes the trigger as the receive window or the send buffer being
used up, and the window case is covered by the mptcp_wnd_end() check;
the send buffer case is not checked as such.

Requiring the msk send buffer to be (nearly) full on that branch, i.e.
sk_wmem_queued >= sk_sndbuf as __sk_stream_memory_free() tests, would
separate the two. Note that mptcp_stream_memory_free() alone would not
help here: it only counts the bytes not yet sent, which are zero in
exactly this case.

One more observation: #345 and this work both act on "this subflow is
slow", one by halving its cwnd and the other by copying its data
elsewhere. Once both are in the scheduler it may be worth looking at
how they interact, e.g. whether a subflow being penalised should also
be a source for reinjection. Happy to help with that part.

> 
Thanks,
Shardul

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

* Re: [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-28  6:29       ` Shardul Bankar
@ 2026-10-28 22:40         ` Abdul Wasey
  0 siblings, 0 replies; 10+ messages in thread
From: Abdul Wasey @ 2026-10-28 22:40 UTC (permalink / raw)
  To: Shardul Bankar
  Cc: Matthieu Baerts, mptcp, Mat Martineau, Geliang Tang, Paolo Abeni,
	Gregory Detal, janak, kalpan.jani

Hi Shardul,

On Wed, 2026-10-28 at 11:59 +0530, Shardul Bankar wrote:
> I have not started on it, so please go ahead. I am happy to review the
> series as it comes.

Thanks, I will take it over.

> mptcp_oppr_blocked() treats an empty send queue (mptcp_send_head() ==
> NULL) as "blocked". first_pending is NULL once everything the
> application has written so far has been handed to a subflow, which
> happens when the send buffer is full, but also when the application
> simply has nothing more to write at that moment.
[...]
> Requiring the msk send buffer to be (nearly) full on that branch, i.e.
> sk_wmem_queued >= sk_sndbuf as __sk_stream_memory_free() tests, would
> separate the two.

Right, it also fires when the app has nothing to write, and the commit
message doesn't say that. I tested that case before changing anything.

Same netns setup as my v4 report, download, BBR, 5 runs. The client
asks for 1 MB, reads it, waits 100 ms and repeats for 20 s. p95 time
per request, in ms:

                         export   RFC   RFC + your check
  40M/10 + 2M/250 5%      2396    317    1448
  400M/10 + 15M/120        530    104     528
  40M/10 + 5M/100          530    305     473

A paced stream (64 KB every 50 ms) shows the same thing. With the
check it's back to about the export numbers. The end of each burst
waits on the slow subflow while the fast one has nothing to send, and
that's where most of the gain comes from. The copies cost 1 to 3% of
the bytes, and nothing on two equal paths.

The check also lowers the bulk result on the 40M + 2M pair from 33-38
to 26-31 Mbit/s, because sk_wmem_queued drops below sk_sndbuf as soon
as a DATA_ACK frees space.

So I'd keep it and fix the commit message. In the scheduler series I
can pass the reason (window, send buffer or app limited) to the
scheduler, so it can skip the app limited case if it wants.

> One more observation: #345 and this work both act on "this subflow is
> slow", one by halving its cwnd and the other by copying its data
> elsewhere.
[...]

Agreed. I'll base the series on your v5 and look at that then.

Thanks,
Abdul Wasey

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

* Re: [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow
  2026-10-26 10:13     ` Matthieu Baerts
  2026-10-28  6:29       ` Shardul Bankar
@ 2026-10-29 19:05       ` Matthieu Baerts
  1 sibling, 0 replies; 10+ messages in thread
From: Matthieu Baerts @ 2026-10-29 19:05 UTC (permalink / raw)
  To: Abdul Wasey
  Cc: mptcp, Mat Martineau, Geliang Tang, Paolo Abeni, Shardul Bankar

Hi Abdul,

On 26/10/2026 11:13, Matthieu Baerts wrote:

(...)

> On 25/10/2026 16:58, Abdul Wasey wrote:
>> I'll rebase on Shardul's series. I'll also test his v4 on my setup and
>> reply on that thread, since it's still waiting for reviews.
> 
> Thank you, that's very helpful!

Do not hesitate to send what you have without waiting for the last
details from Shardul's series to be fixed. You can simply mention the
base series in your cover letter as explained here:

  https://github.com/multipath-tcp/mptcp_net-next/wiki/CI

e.g.

  Based-on: <20261028-mptcp_penalise_send_v2-v5-0-32fb379ce117@mpiricsoftware.com>

(Sashiko also supports b4's dependencies tags, but not Patchew -- it is
 fine to add both.)

(...)

>> The perf lab would help, yes. I can share my netns cases if that's
>> useful.
> 
> That could indeed. But I'm going to try to push what I have today: it
> might be better to work on improving the same thing.

It took me a bit more time to fix the last details, but it should be
usable now:

  https://github.com/multipath-tcp/mptcp-upstream-virtme-docker/blob/latest/PERF.md#tracking-regressions-locally

There are many use-cases (based on previous work). The idea is that
the regressions are found automatically by looking at the distribution
over multiple runs. So it works best if you execute the tests a lot of
time on a base branch, then check on top of your modifications (always
under the same conditions).

The whole suite might take a lot of time (1 to 2h), but it is possible
to execute only some parts. Do not hesitate to suggest modifications or
send PRs ;)
Or tell me if that doesn't work for what you are doing.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


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

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

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-25 13:46 [RFC PATCH mptcp-next 0/1] mptcp: opportunistic reinjection on an idle subflow Abdul Wasey
2026-10-25 13:46 ` [RFC PATCH mptcp-next 1/1] " Abdul Wasey
2026-10-25 14:05   ` sashiko-bot
2026-10-25 14:54 ` [RFC PATCH mptcp-next 0/1] " MPTCP CI
2026-10-25 15:12 ` Matthieu Baerts
2026-10-25 15:58   ` Abdul Wasey
2026-10-26 10:13     ` Matthieu Baerts
2026-10-28  6:29       ` Shardul Bankar
2026-10-28 22:40         ` Abdul Wasey
2026-10-29 19:05       ` Matthieu Baerts

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.