[PATCH v9 mptcp-next 0/6] mptcp: address stall under memory pressure

[PATCH v9 mptcp-next 0/6] mptcp: address stall under memory pressure

[Paolo Abeni]

This an attempt to fix the data transfer stall reported by Geliang and
Gang more carefully enforcing memory constraints at the MPTCP level.

This iteration introduces has no significant change over the previous
one; the already merged patches has been dropped, and the remaining ones
rebases on top. The main delta is the commit message for patch 4, here
completely reworderd, as its content become obsoleted in the past
reworks.

Note that `multi_chunk_sendfile` and `multiproc*` test cases in
mptcp_data *may* require longer timeout than default[1].

Patch 1 is actually a fix for a pre-existing issues targeting net,
included here just for my convenience.

Patch 2 and 3 make the admission check much more strict for incoming
packets exceeding the memory limits, with some exception for fallback
sockets.
Patch 4 implements OoO queue pruning for MPTCP.
Finally patch 5 and 6 improve the MPTCP-level retransmission schema to
make recovery from memory pressure/after MPTCP-level drop significantly
faster.

[1] In my testing on v8 and v9 mptcp_data survived a few hundred
iterations with the default timeout. Some independent testing would be
appreciated.
---
v8 -> v9:
  - dropped already merged old patches 1-3
  - reworded commit message in patch 4

Paolo Abeni (6):
  mptcp: allow subflow rcv wnd to shrink
  mptcp: explicitly drop over memory limits
  mptcp: enforce hard limit on backlog flushing
  mptcp: implemented OoO queue pruning
  mptcp: move the retrans loop to a separate helper
  mptcp: let the retrans scheduler do its job.

 net/mptcp/mib.c      |   3 +
 net/mptcp/mib.h      |   3 +
 net/mptcp/options.c  |  35 +++++-
 net/mptcp/protocol.c | 247 +++++++++++++++++++++++++++++++------------
 4 files changed, 218 insertions(+), 70 deletions(-)

-- 
2.54.0

[PATCH v9 mptcp-next 1/6] mptcp: allow subflow rcv wnd to shrink

[Paolo Abeni]

In MPTCP connection, the `window` field in the TCP header refers to the
MPTCP-level rcv_nxt and it's right edge should not move backward. Such
constraint is enforced at DSS option generation time.

At the same time, the TCP stack ensures independently that the TCP-level
rcv wnd right's edge does not move backward. That in turn causes artificial
inflating of the MPTCP rcv window when the incoming data is acked at the
TCP level and is OoO in the MPTCP sequence space (or lands in the backlog).

As a consequence, the incoming traffic can exceed the receiver rcvbuf size
even when the sender is not misbehaving.

Prevent such scenario forcibly allowing the TCP subflow to shrink the
TCP-level rcv wnd regardless of the current netns setting.

Fixes: f3589be0c420 ("mptcp: never shrink offered window")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/options.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 4d72f286a485..97ea4aa37b33 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -566,6 +566,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
 	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
+	struct tcp_sock *tp = tcp_sk(sk);
 	unsigned int dss_size = 0;
 	struct mptcp_ext *mpext;
 	unsigned int ack_size;
@@ -614,6 +615,12 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
 	if (dss_size == 0)
 		ack_size += TCPOLEN_MPTCP_DSS_BASE;
 
+	/* The caller is __tcp_transmit_skb(), and will compute the new rcv
+	 * wnd soon: ensure that the window can shrink.
+	 */
+	if (skb)
+		tp->rcv_wnd = tp->rcv_nxt - tp->rcv_wup;
+
 	dss_size += ack_size;
 
 	*size = ALIGN(dss_size, 4);
-- 
2.54.0

[PATCH v9 mptcp-next 2/6] mptcp: explicitly drop over memory limits

[Paolo Abeni]

Currently the enforcement of the rcvbuf constraint is implemented
when moving the skbs into the msk receive or OoO queue, keeping the
incoming skbs in the subflow queue when over limit.

Under significant memory pressure the above can cause permanent data
transfer stalls, as the skb needed to make forward progress can be
stuck in a subflow queue.

Over memory limits, drop the incoming skb, relaying on MPTCP-level
retransmissions.

Note that fallback socket must perform the limit before the skb reaches
the subflow-level queue, as dropping an in-sequence already acked skb
would break the stream.

This is not a complete fix for the stall issue, as the drop strategy
needs refinements that will come in the next patches.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v7 -> v8:
 - removed non fallback check in mptcp_incoming_option(): that is an
   tput optimization (avoid rejections) for a slowpath case (sender
   is misbehaving) and needs too much additional complexity.
 - move here from later patches mibs definition

v6 -> v7:
 - fix sign extension issues

v4 -> v5:
 - fix possible u32 overflow in mptcp_over_limit

v3 -> v4:
 - schedule TCP ack on drop
 - enforce limits in __mptcp_move_skb() and __mptcp_add_backlog(), too
   but only if not fallback.

v1 -> v2:
 - deal correctly with tcp fin and zero win probe

RFC -> v1:
 - limit vs actual buffer size
 - use CB info instead of skb->len

Note that:
 - this needs the follow-up patches to really fix the stall
 - sashiko can assume ZWP carries unacked data and may be silently dropped.
   AFAIK that is false.
 - sashiko apparently can't graps mptcp subflow never hit the tcp rx
   fastpath, and the mptcp_incoming_options in tcp_rcv_state_process
   is hit, the peer can't transmit any more data.
 - the memory comparison is intentionally very rough, as
   the msk socket lock is not currently held where the condition is
   now enforced. This should require some refinement, shared as-is
   to avoid more latency on my side
---
 net/mptcp/mib.c      |  2 ++
 net/mptcp/mib.h      |  2 ++
 net/mptcp/options.c  | 28 +++++++++++++++++++++++++---
 net/mptcp/protocol.c | 31 +++++++++++++++++++++++--------
 4 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index f23fda0c55a7..ef65e2df709f 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -85,6 +85,8 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("SimultConnectFallback", MPTCP_MIB_SIMULTCONNFALLBACK),
 	SNMP_MIB_ITEM("FallbackFailed", MPTCP_MIB_FALLBACKFAILED),
 	SNMP_MIB_ITEM("WinProbe", MPTCP_MIB_WINPROBE),
+	SNMP_MIB_ITEM("BacklogDrop", MPTCP_MIB_BACKLOGDROP),
+	SNMP_MIB_ITEM("RcvPruned", MPTCP_MIB_RCVPRUNED),
 };
 
 /* mptcp_mib_alloc - allocate percpu mib counters
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 812218b5ed2b..c84eb853d499 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -88,6 +88,8 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_SIMULTCONNFALLBACK,	/* Simultaneous connect */
 	MPTCP_MIB_FALLBACKFAILED,	/* Can't fallback due to msk status */
 	MPTCP_MIB_WINPROBE,		/* MPTCP-level zero window probe */
+	MPTCP_MIB_BACKLOGDROP,		/* Backlog over memory limit */
+	MPTCP_MIB_RCVPRUNED,		/* Dropped due to memory constrains */
 	__MPTCP_MIB_MAX
 };
 
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 97ea4aa37b33..2b35bdc113a5 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1161,8 +1161,30 @@ static bool add_addr_hmac_valid(struct mptcp_sock *msk,
 	return hmac == mp_opt->ahmac;
 }
 
-/* Return false in case of error (or subflow has been reset),
- * else return true.
+static bool mptcp_over_limit(struct sock *sk, struct sock *ssk,
+			     const struct sk_buff *skb)
+{
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	u64 mem = sk_rmem_alloc_get(sk);
+
+	mem += READ_ONCE(msk->backlog_len);
+	if (likely(mem <= READ_ONCE(sk->sk_rcvbuf)))
+		return false;
+
+	/* Avoid silently dropping pure acks, fin or zero win probes. */
+	if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq ||
+	    TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN ||
+	    !after(TCP_SKB_CB(skb)->end_seq, tcp_sk(ssk)->rcv_nxt))
+		return false;
+
+	/* Dropped due to memory constraints, schedule an ack. */
+	inet_csk(ssk)->icsk_ack.pending |= ICSK_ACK_NOMEM | ICSK_ACK_NOW;
+	inet_csk_schedule_ack(ssk);
+	return true;
+}
+
+/* Return false when the caller must drop the packet, i.e. in case of error,
+ * subflow has been reset, or over memory limits.
  */
 bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 {
@@ -1188,7 +1210,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 
 		__mptcp_data_acked(subflow->conn);
 		mptcp_data_unlock(subflow->conn);
-		return true;
+		return !mptcp_over_limit(subflow->conn, sk, skb);
 	}
 
 	mptcp_get_options(skb, &mp_opt);
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1d67728d4233..26a70b3b9566 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -381,6 +381,16 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
 
 	mptcp_borrow_fwdmem(sk, skb);
 
+	/* Can't drop packets for fallback socket this late, or the stream
+	 * will break.
+	 */
+	if (unlikely(sk_rmem_alloc_get(sk) > READ_ONCE(sk->sk_rcvbuf)) &&
+	    !__mptcp_check_fallback(msk)) {
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
+		mptcp_drop(sk, skb);
+		return false;
+	}
+
 	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
 		/* in sequence */
 		msk->bytes_received += copy_len;
@@ -675,6 +685,7 @@ static void __mptcp_add_backlog(struct sock *sk,
 	struct sk_buff *tail = NULL;
 	struct sock *ssk = skb->sk;
 	bool fragstolen;
+	u64 limit;
 	int delta;
 
 	if (unlikely(sk->sk_state == TCP_CLOSE)) {
@@ -682,6 +693,16 @@ static void __mptcp_add_backlog(struct sock *sk,
 		return;
 	}
 
+	/* Similar additional allowance as plain TCP. */
+	limit = READ_ONCE(sk->sk_rcvbuf);
+	limit += (limit >> 1) + 64 * 1024;
+	limit = min_t(u64, limit, UINT_MAX);
+	if (msk->backlog_len > limit && !__mptcp_check_fallback(msk)) {
+		__MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_BACKLOGDROP);
+		kfree_skb_reason(skb, SKB_DROP_REASON_SOCKET_BACKLOG);
+		return;
+	}
+
 	/* Try to coalesce with the last skb in our backlog */
 	if (!list_empty(&msk->backlog_list))
 		tail = list_last_entry(&msk->backlog_list, struct sk_buff, list);
@@ -753,7 +774,7 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 
 			mptcp_init_skb(ssk, skb, offset, len);
 
-			if (own_msk && sk_rmem_alloc_get(sk) < sk->sk_rcvbuf) {
+			if (own_msk) {
 				mptcp_subflow_lend_fwdmem(subflow, skb);
 				ret |= __mptcp_move_skb(sk, skb);
 			} else {
@@ -2211,10 +2232,6 @@ static bool __mptcp_move_skbs(struct sock *sk, struct list_head *skbs, u32 *delt
 
 	*delta = 0;
 	while (1) {
-		/* If the msk recvbuf is full stop, don't drop */
-		if (sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
-			break;
-
 		prefetch(skb->next);
 		list_del(&skb->list);
 		*delta += skb->truesize;
@@ -2242,9 +2259,7 @@ static bool mptcp_can_spool_backlog(struct sock *sk, struct list_head *skbs)
 	DEBUG_NET_WARN_ON_ONCE(msk->backlog_unaccounted && sk->sk_socket &&
 			       mem_cgroup_from_sk(sk));
 
-	/* Don't spool the backlog if the rcvbuf is full. */
-	if (list_empty(&msk->backlog_list) ||
-	    sk_rmem_alloc_get(sk) > sk->sk_rcvbuf)
+	if (list_empty(&msk->backlog_list))
 		return false;
 
 	INIT_LIST_HEAD(skbs);
-- 
2.54.0

[PATCH v9 mptcp-next 3/6] mptcp: enforce hard limit on backlog flushing

[Paolo Abeni]

Currently a wild producer could keep the backlog flushing operation
spinning for an unbound time.

Since the previous patch the amount of data present in the backlog is
hard-limited. Move the backlog len update at the end of the flush loop to
prevent it spinning forever.

Also, no need to splice back the remaining skbs list into the backlog, as
such list is always empty after each backlog processing loop.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 26a70b3b9566..b0a0c51e0a13 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2230,7 +2230,6 @@ static bool __mptcp_move_skbs(struct sock *sk, struct list_head *skbs, u32 *delt
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	bool moved = false;
 
-	*delta = 0;
 	while (1) {
 		prefetch(skb->next);
 		list_del(&skb->list);
@@ -2267,20 +2266,12 @@ static bool mptcp_can_spool_backlog(struct sock *sk, struct list_head *skbs)
 	return true;
 }
 
-static void mptcp_backlog_spooled(struct sock *sk, u32 moved,
-				  struct list_head *skbs)
-{
-	struct mptcp_sock *msk = mptcp_sk(sk);
-
-	WRITE_ONCE(msk->backlog_len, msk->backlog_len - moved);
-	list_splice(skbs, &msk->backlog_list);
-}
-
 static bool mptcp_move_skbs(struct sock *sk)
 {
+	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct list_head skbs;
 	bool enqueued = false;
-	u32 moved;
+	u32 moved = 0;
 
 	mptcp_data_lock(sk);
 	while (mptcp_can_spool_backlog(sk, &skbs)) {
@@ -2288,8 +2279,8 @@ static bool mptcp_move_skbs(struct sock *sk)
 		enqueued |= __mptcp_move_skbs(sk, &skbs, &moved);
 
 		mptcp_data_lock(sk);
-		mptcp_backlog_spooled(sk, moved, &skbs);
 	}
+	WRITE_ONCE(msk->backlog_len, msk->backlog_len - moved);
 	mptcp_data_unlock(sk);
 
 	if (enqueued && mptcp_epollin_ready(sk))
@@ -3680,12 +3671,12 @@ static void mptcp_release_cb(struct sock *sk)
 	__must_hold(&sk->sk_lock.slock)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
+	u32 moved = 0;
 
 	for (;;) {
 		unsigned long flags = (msk->cb_flags & MPTCP_FLAGS_PROCESS_CTX_NEED);
 		struct list_head join_list, skbs;
 		bool spool_bl;
-		u32 moved;
 
 		spool_bl = mptcp_can_spool_backlog(sk, &skbs);
 		if (!flags && !spool_bl)
@@ -3718,9 +3709,9 @@ static void mptcp_release_cb(struct sock *sk)
 
 		cond_resched();
 		spin_lock_bh(&sk->sk_lock.slock);
-		if (spool_bl)
-			mptcp_backlog_spooled(sk, moved, &skbs);
 	}
+	if (moved)
+		WRITE_ONCE(msk->backlog_len, msk->backlog_len - moved);
 
 	if (__test_and_clear_bit(MPTCP_CLEAN_UNA, &msk->cb_flags))
 		__mptcp_clean_una_wakeup(sk);
-- 
2.54.0

[PATCH v9 mptcp-next 4/6] mptcp: implemented OoO queue pruning

[Paolo Abeni]

When moving incoming skbs in the msk receive queue and the latter
is above limits, prune it as needed quite alike what TCP is doing
at the subflow level. The main difference relies in the stop condition:
since MPTCP does not perform collapsing, it's better off dropping the
bare minimum to fit the (newer) incoming packet.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v8 -> v9:
 - reworded the (obsoleted) commit message

v6 -> v7:
 - fix u64 -> u32 truncation

v2 -> v3:
 - deal with unsynced TFO skb at prune time - only possible when pruning
   in mptcp_over_limit()

v1 -> v2:
 - collapse rcv queue, too
 - deal with MPC map, too
 - drop left-over sentence in the commit message

RFC -> v1:
 - use data_seq only when available
 - avoid ack_seq lockless access
 - drop limit on fallback
 - collapse rcvqueue, too
 - drop only when pruning is not possible and over rcvbuf * 2

Note:
 - sashiko can be confused about fwd memory lifecycle (I can
 understand that :). Any exceeding amount of fwd allocated memory
 is always released by the next sk_mem_uncharge() - i.e. fwd memory
 is not tied to the current skb.
 - AFAICS KASAN handles bitmap variables in a sane way, and sashiko
 doesn't know about that
---
 net/mptcp/mib.c      |  1 +
 net/mptcp/mib.h      |  1 +
 net/mptcp/protocol.c | 48 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index ef65e2df709f..d9bd4f4afcc0 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -87,6 +87,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("WinProbe", MPTCP_MIB_WINPROBE),
 	SNMP_MIB_ITEM("BacklogDrop", MPTCP_MIB_BACKLOGDROP),
 	SNMP_MIB_ITEM("RcvPruned", MPTCP_MIB_RCVPRUNED),
+	SNMP_MIB_ITEM("OfoPruned", MPTCP_MIB_OFO_PRUNED),
 };
 
 /* mptcp_mib_alloc - allocate percpu mib counters
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index c84eb853d499..18f35f7e0a2d 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -90,6 +90,7 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_WINPROBE,		/* MPTCP-level zero window probe */
 	MPTCP_MIB_BACKLOGDROP,		/* Backlog over memory limit */
 	MPTCP_MIB_RCVPRUNED,		/* Dropped due to memory constrains */
+	MPTCP_MIB_OFO_PRUNED,		/* MPTCP-level OoO queue pruned */
 	__MPTCP_MIB_MAX
 };
 
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b0a0c51e0a13..29cb10c02ed8 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -373,6 +373,45 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
 	skb_dst_drop(skb);
 }
 
+/* "Inspired" from the TCP version; main difference: stop as soon as the MPTCP
+ * socket is under memory limit.
+ */
+static void mptcp_prune_ofo_queue(struct sock *sk, u64 seq)
+{
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	struct rb_node *node, *prev;
+	bool pruned = false;
+	u64 mem;
+
+	if (RB_EMPTY_ROOT(&msk->out_of_order_queue))
+		return;
+
+	node = &msk->ooo_last_skb->rbnode;
+
+	do {
+		struct sk_buff *skb = rb_to_skb(node);
+
+		/* Stop pruning if the incoming skb would land in OoO tail. */
+		if (after64(seq, MPTCP_SKB_CB(skb)->map_seq))
+			break;
+
+		pruned = true;
+		prev = rb_prev(node);
+		rb_erase(node, &msk->out_of_order_queue);
+		mptcp_drop(sk, skb);
+		msk->ooo_last_skb = rb_to_skb(prev);
+
+		mem = (unsigned int)atomic_read(&sk->sk_rmem_alloc);
+		if (mem < sk->sk_rcvbuf)
+			break;
+
+		node = prev;
+	} while (node);
+
+	if (pruned)
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFO_PRUNED);
+}
+
 static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
 {
 	u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
@@ -386,9 +425,12 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
 	 */
 	if (unlikely(sk_rmem_alloc_get(sk) > READ_ONCE(sk->sk_rcvbuf)) &&
 	    !__mptcp_check_fallback(msk)) {
-		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
-		mptcp_drop(sk, skb);
-		return false;
+		mptcp_prune_ofo_queue(sk, MPTCP_SKB_CB(skb)->map_seq);
+		if (sk_rmem_alloc_get(sk) > READ_ONCE(sk->sk_rcvbuf)) {
+			MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
+			mptcp_drop(sk, skb);
+			return false;
+		}
 	}
 
 	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
-- 
2.54.0

[PATCH v9 mptcp-next 5/6] mptcp: move the retrans loop to a separate helper

[Paolo Abeni]

This is a cleanup in order to make the next patch simpler.
No functional change intended.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 74 +++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 31 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 29cb10c02ed8..e21ace787a32 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2828,41 +2828,14 @@ static void mptcp_check_fastclose(struct mptcp_sock *msk)
 	sk_error_report(sk);
 }
 
-static void __mptcp_retrans(struct sock *sk)
+/* Retransmit the specified data fragment on all the selected subflows. */
+static int __mptcp_push_retrans(struct sock *sk, struct mptcp_data_frag *dfrag)
 {
 	struct mptcp_sendmsg_info info = { .data_lock_held = true, };
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct mptcp_subflow_context *subflow;
-	struct mptcp_data_frag *dfrag;
 	struct sock *ssk;
-	int ret, err;
-	u16 len = 0;
-
-	mptcp_clean_una_wakeup(sk);
-
-	/* first check ssk: need to kick "stale" logic */
-	err = mptcp_sched_get_retrans(msk);
-	dfrag = mptcp_rtx_head(sk);
-	if (!dfrag) {
-		if (mptcp_data_fin_enabled(msk)) {
-			struct inet_connection_sock *icsk = inet_csk(sk);
-
-			WRITE_ONCE(icsk->icsk_retransmits,
-				   icsk->icsk_retransmits + 1);
-			mptcp_set_datafin_timeout(sk);
-			mptcp_send_ack(msk);
-
-			goto reset_timer;
-		}
-
-		if (!mptcp_send_head(sk))
-			goto clear_scheduled;
-
-		goto reset_timer;
-	}
-
-	if (err)
-		goto reset_timer;
+	int ret, len = 0;
 
 	mptcp_for_each_subflow(msk, subflow) {
 		if (READ_ONCE(subflow->scheduled)) {
@@ -2890,7 +2863,7 @@ static void __mptcp_retrans(struct sock *sk)
 			    !msk->allow_subflows) {
 				spin_unlock_bh(&msk->fallback_lock);
 				release_sock(ssk);
-				goto clear_scheduled;
+				return -1;
 			}
 
 			while (info.sent < info.limit) {
@@ -2913,6 +2886,45 @@ static void __mptcp_retrans(struct sock *sk)
 			release_sock(ssk);
 		}
 	}
+	return len;
+}
+
+static void __mptcp_retrans(struct sock *sk)
+{
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	struct mptcp_subflow_context *subflow;
+	struct mptcp_data_frag *dfrag;
+	int err, len;
+
+	mptcp_clean_una_wakeup(sk);
+
+	/* first check ssk: need to kick "stale" logic */
+	err = mptcp_sched_get_retrans(msk);
+	dfrag = mptcp_rtx_head(sk);
+	if (!dfrag) {
+		if (mptcp_data_fin_enabled(msk)) {
+			struct inet_connection_sock *icsk = inet_csk(sk);
+
+			WRITE_ONCE(icsk->icsk_retransmits,
+				   icsk->icsk_retransmits + 1);
+			mptcp_set_datafin_timeout(sk);
+			mptcp_send_ack(msk);
+
+			goto reset_timer;
+		}
+
+		if (!mptcp_send_head(sk))
+			goto clear_scheduled;
+
+		goto reset_timer;
+	}
+
+	if (err)
+		goto reset_timer;
+
+	len = __mptcp_push_retrans(sk, dfrag);
+	if (len < 0)
+		goto clear_scheduled;
 
 	msk->bytes_retrans += len;
 	dfrag->already_sent = max(dfrag->already_sent, len);
-- 
2.54.0

[PATCH v9 mptcp-next 6/6] mptcp: let the retrans scheduler do its job.

[Paolo Abeni]

Currently the MPTCP core enforces that when MPTCP-level retrans timer
fires, at most a single dfrag is retransmitted. If some corner-cases it
may be necessary retransmit multiple dfrags, and the MPTCP socket will
need to wait multiple retrans timeout to accomplish that.

Remove the mentioned constraint, allowing to transmit multiple dfrags per
retrans period, as long as the scheduler keeps selecting subflows for
retransmissions and pending data is available in the rtx queue.
The default scheduler will transmit a dfrag per available subflow.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v7 -> v8
  - fix corner-case retrans_seq update

v4 -> v5:
  - fixed already_sent update

v3 -> v4:
  - avoid quadratic behavior, fix retrans_seq update
  - fix rtx timer re-schedule miss

v2 -> v3:
  - fix infinite loop issue (should address tls tests failures)

v1 -> v2:
  - fix retrans sequence update (sashiko)

Note:
 - sashiko see issues when dfrag = mptcp_rtx_head(sk) != NULL and
   dfrag->already_sent == 0. That condition should not possible: if
   mptcp_rtx_head() is not NULL there should be some data already
   sent.
---
 net/mptcp/protocol.c | 117 +++++++++++++++++++++++++++++++------------
 1 file changed, 85 insertions(+), 32 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index e21ace787a32..51509c062768 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1199,13 +1199,6 @@ static void __mptcp_clean_una_wakeup(struct sock *sk)
 	mptcp_write_space(sk);
 }
 
-static void mptcp_clean_una_wakeup(struct sock *sk)
-{
-	mptcp_data_lock(sk);
-	__mptcp_clean_una_wakeup(sk);
-	mptcp_data_unlock(sk);
-}
-
 static void mptcp_enter_memory_pressure(struct sock *sk)
 {
 	struct mptcp_subflow_context *subflow;
@@ -2828,8 +2821,12 @@ static void mptcp_check_fastclose(struct mptcp_sock *msk)
 	sk_error_report(sk);
 }
 
-/* Retransmit the specified data fragment on all the selected subflows. */
-static int __mptcp_push_retrans(struct sock *sk, struct mptcp_data_frag *dfrag)
+/*
+ * Retransmit the specified data fragment on all the selected subflows,
+ * starting from the specified sequence
+ */
+static int __mptcp_push_retrans(struct sock *sk, struct mptcp_data_frag *dfrag,
+				u64 sent_seq)
 {
 	struct mptcp_sendmsg_info info = { .data_lock_held = true, };
 	struct mptcp_sock *msk = mptcp_sk(sk);
@@ -2839,6 +2836,7 @@ static int __mptcp_push_retrans(struct sock *sk, struct mptcp_data_frag *dfrag)
 
 	mptcp_for_each_subflow(msk, subflow) {
 		if (READ_ONCE(subflow->scheduled)) {
+			u16 offset = sent_seq - dfrag->data_seq;
 			u16 copied = 0;
 
 			mptcp_subflow_set_scheduled(subflow, false);
@@ -2848,9 +2846,12 @@ static int __mptcp_push_retrans(struct sock *sk, struct mptcp_data_frag *dfrag)
 			lock_sock(ssk);
 
 			/* limit retransmission to the bytes already sent on some subflows */
-			info.sent = 0;
+			info.sent = offset;
 			info.limit = READ_ONCE(msk->csum_enabled) ? dfrag->data_len :
 								    dfrag->already_sent;
+			DEBUG_NET_WARN_ON_ONCE(!before64(sent_seq,
+							 dfrag->data_seq +
+							 info.limit));
 
 			/*
 			 * make the whole retrans decision, xmit, disallow
@@ -2894,45 +2895,97 @@ static void __mptcp_retrans(struct sock *sk)
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct mptcp_subflow_context *subflow;
 	struct mptcp_data_frag *dfrag;
+	bool retransmitted = false;
+	u64 retrans_seq;
 	int err, len;
 
-	mptcp_clean_una_wakeup(sk);
-
-	/* first check ssk: need to kick "stale" logic */
-	err = mptcp_sched_get_retrans(msk);
+	mptcp_data_lock(sk);
+	__mptcp_clean_una_wakeup(sk);
+	retrans_seq = msk->snd_una;
 	dfrag = mptcp_rtx_head(sk);
+	mptcp_data_unlock(sk);
+	if (!dfrag)
+		goto check_data_fin;
+
+	for (;;) {
+		bool already_retrans;
+		u64 sent_seq;
+
+		/* The scheduler may clean the RTX queue. */
+		get_page(dfrag->page);
+
+		/* The default scheduler will kick "stale" logic. */
+		err = mptcp_sched_get_retrans(msk);
+		if (err) {
+			put_page(dfrag->page);
+			break;
+		}
+
+		/* Incoming acks can have moved retrans sequence after
+		 * the current dfrag, if so try to start again from RTX head.
+		 */
+		mptcp_data_lock(sk);
+		already_retrans = !dfrag->already_sent ||
+				  !before64(msk->snd_una, dfrag->data_seq +
+					    dfrag->already_sent);
+		put_page(dfrag->page);
+		if (already_retrans) {
+			__mptcp_clean_una_wakeup(sk);
+			retrans_seq = msk->snd_una;
+			dfrag = mptcp_rtx_head(sk);
+		} else if (after64(msk->snd_una, retrans_seq)) {
+			retrans_seq = msk->snd_una;
+		}
+		mptcp_data_unlock(sk);
+		if (!dfrag)
+			break;
+
+		len = __mptcp_push_retrans(sk, dfrag, retrans_seq);
+		if (len < 0)
+			goto clear_scheduled;
+
+		retransmitted = true;
+		retrans_seq += len;
+		msk->bytes_retrans += len;
+		dfrag->already_sent = max_t(u16, dfrag->already_sent,
+					    retrans_seq - dfrag->data_seq);
+
+		/* With csum enabled retransmission can send new data. */
+		sent_seq = dfrag->already_sent + dfrag->data_seq;
+		if (after64(sent_seq, msk->snd_nxt))
+			WRITE_ONCE(msk->snd_nxt, sent_seq);
+
+		/* Attempt the next fragment only if the current one is
+		 * completely retransmitted.
+		 */
+		if (before64(retrans_seq, dfrag->data_seq + dfrag->data_len))
+			break;
+
+		dfrag = list_is_last(&dfrag->list, &msk->rtx_queue) ?
+				NULL : list_next_entry(dfrag, list);
+		if (!dfrag || !dfrag->already_sent)
+			break;
+	}
+
+	/* Data fin retransmission needed only if no data retransmission took
+	 * place, and RTX queue is empty.
+	 */
+check_data_fin:
 	if (!dfrag) {
-		if (mptcp_data_fin_enabled(msk)) {
+		if (!retransmitted && mptcp_data_fin_enabled(msk)) {
 			struct inet_connection_sock *icsk = inet_csk(sk);
 
 			WRITE_ONCE(icsk->icsk_retransmits,
 				   icsk->icsk_retransmits + 1);
 			mptcp_set_datafin_timeout(sk);
 			mptcp_send_ack(msk);
-
 			goto reset_timer;
 		}
 
 		if (!mptcp_send_head(sk))
 			goto clear_scheduled;
-
-		goto reset_timer;
 	}
 
-	if (err)
-		goto reset_timer;
-
-	len = __mptcp_push_retrans(sk, dfrag);
-	if (len < 0)
-		goto clear_scheduled;
-
-	msk->bytes_retrans += len;
-	dfrag->already_sent = max(dfrag->already_sent, len);
-
-	/* With csum enabled retransmission can send new data. */
-	if (after64(dfrag->already_sent + dfrag->data_seq, msk->snd_nxt))
-		WRITE_ONCE(msk->snd_nxt, dfrag->already_sent + dfrag->data_seq);
-
 reset_timer:
 	mptcp_check_and_set_pending(sk);
 
-- 
2.54.0

Re: [PATCH v9 mptcp-next 0/6] mptcp: address stall under memory pressure

[MPTCP CI]

Hi Paolo,

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): selftest_simult_flows ⚠️ 
- 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! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/26507260311

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


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)

Re: [PATCH v9 mptcp-next 2/6] mptcp: explicitly drop over memory limits

[Paolo Abeni]

On 5/27/26 12:45 PM, Paolo Abeni wrote:
> Currently the enforcement of the rcvbuf constraint is implemented
> when moving the skbs into the msk receive or OoO queue, keeping the
> incoming skbs in the subflow queue when over limit.
> 
> Under significant memory pressure the above can cause permanent data
> transfer stalls, as the skb needed to make forward progress can be
> stuck in a subflow queue.
> 
> Over memory limits, drop the incoming skb, relaying on MPTCP-level
> retransmissions.
> 
> Note that fallback socket must perform the limit before the skb reaches
> the subflow-level queue, as dropping an in-sequence already acked skb
> would break the stream.
> 
> This is not a complete fix for the stall issue, as the drop strategy
> needs refinements that will come in the next patches.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v7 -> v8:
>  - removed non fallback check in mptcp_incoming_option(): that is an
>    tput optimization (avoid rejections) for a slowpath case (sender
>    is misbehaving) and needs too much additional complexity.
>  - move here from later patches mibs definition
> 
> v6 -> v7:
>  - fix sign extension issues
> 
> v4 -> v5:
>  - fix possible u32 overflow in mptcp_over_limit
> 
> v3 -> v4:
>  - schedule TCP ack on drop
>  - enforce limits in __mptcp_move_skb() and __mptcp_add_backlog(), too
>    but only if not fallback.
> 
> v1 -> v2:
>  - deal correctly with tcp fin and zero win probe
> 
> RFC -> v1:
>  - limit vs actual buffer size
>  - use CB info instead of skb->len
> 
> Note that:
>  - this needs the follow-up patches to really fix the stall
>  - sashiko can assume ZWP carries unacked data and may be silently dropped.
>    AFAIK that is false.
>  - sashiko apparently can't graps mptcp subflow never hit the tcp rx
>    fastpath, and the mptcp_incoming_options in tcp_rcv_state_process
>    is hit, the peer can't transmit any more data.
>  - the memory comparison is intentionally very rough, as
>    the msk socket lock is not currently held where the condition is
>    now enforced. This should require some refinement, shared as-is
>    to avoid more latency on my side

Sashiko has some comments on this patch; TL;DR: can be safely ignored.

---
Does this code incorrectly handle transient negative values?
sk_rmem_alloc_get() returns a signed int. If sk_rmem_alloc transiently drops
below zero, assigning it to a u64 causes the negative integer to sign-extend
into a very large positive 64-bit value.
This would cause the subsequent limit check (mem <= sk_rcvbuf) to
evaluate to
false, erroneously dropping valid packets.
---

negative sk_rmem_alloc() values are only possible when the amount of
memory in the queue is above INT_MAX, i.e. the value is really a large
positive one. The above concern does not apply.

sk_rmem_alloc() can raise above INT_MAX when sk_rcvbuf is INT_MAX as the
RX path accepts one more packet above sk_rcvbuf.

---
Does this code leak memory charges and sk_forward_alloc credits?
mptcp_borrow_fwdmem() transfers the skb's truesize from the subflow's memory
to the MPTCP socket's sk_forward_alloc. However, the skb's destructor was
cleared by mptcp_subflow_lend_fwdmem() prior to this.
If the skb is then dropped via mptcp_drop(sk, skb) before skb_set_owner_r()
is called, the skb is freed without triggering a destructor. The truesize is
never subtracted from sk_forward_alloc, and global tcp_memory_allocated is
not replenished.
Could this allow an attacker to repeatedly hit this path by sending data
while
the buffer is full, inflating sk_forward_alloc infinitely and causing a
system
memory exhaustion regression?
---

Pre existing because we already have the exact same drop path.

May deserve a follow-up patch.

/P

Re: [PATCH v9 mptcp-next 6/6] mptcp: let the retrans scheduler do its job.

[Paolo Abeni]

 5/27/26 12:45 PM, Paolo Abeni wrote:
> Currently the MPTCP core enforces that when MPTCP-level retrans timer
> fires, at most a single dfrag is retransmitted. If some corner-cases it
> may be necessary retransmit multiple dfrags, and the MPTCP socket will
> need to wait multiple retrans timeout to accomplish that.
> 
> Remove the mentioned constraint, allowing to transmit multiple dfrags per
> retrans period, as long as the scheduler keeps selecting subflows for
> retransmissions and pending data is available in the rtx queue.
> The default scheduler will transmit a dfrag per available subflow.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Sashiko concerns on this patch is real, and a new revision is needed.
@Matttbe: could you please apply patches 1-4, as they makes sense as a
whole? I'll resubmit a new version for the last 2. Patch 5 is correct,
but makes no sense without this one.

/P

Re: [PATCH v9 mptcp-next 2/6] mptcp: explicitly drop over memory limits

[Matthieu Baerts]

Hi Paolo,

On 27/05/2026 20:45, Paolo Abeni wrote:
> Currently the enforcement of the rcvbuf constraint is implemented
> when moving the skbs into the msk receive or OoO queue, keeping the
> incoming skbs in the subflow queue when over limit.
> 
> Under significant memory pressure the above can cause permanent data
> transfer stalls, as the skb needed to make forward progress can be
> stuck in a subflow queue.
> 
> Over memory limits, drop the incoming skb, relaying on MPTCP-level
> retransmissions.
> 
> Note that fallback socket must perform the limit before the skb reaches
> the subflow-level queue, as dropping an in-sequence already acked skb
> would break the stream.
> 
> This is not a complete fix for the stall issue, as the drop strategy
> needs refinements that will come in the next patches.

Thank you for working on that!

(...)

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 97ea4aa37b33..2b35bdc113a5 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1161,8 +1161,30 @@ static bool add_addr_hmac_valid(struct mptcp_sock *msk,
>  	return hmac == mp_opt->ahmac;
>  }
>  
> -/* Return false in case of error (or subflow has been reset),
> - * else return true.
> +static bool mptcp_over_limit(struct sock *sk, struct sock *ssk,
> +			     const struct sk_buff *skb)
> +{
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	u64 mem = sk_rmem_alloc_get(sk);
> +
> +	mem += READ_ONCE(msk->backlog_len);
> +	if (likely(mem <= READ_ONCE(sk->sk_rcvbuf)))
> +		return false;
> +
> +	/* Avoid silently dropping pure acks, fin or zero win probes. */
> +	if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq ||
> +	    TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN ||
> +	    !after(TCP_SKB_CB(skb)->end_seq, tcp_sk(ssk)->rcv_nxt))
> +		return false;
> +
> +	/* Dropped due to memory constraints, schedule an ack. */
> +	inet_csk(ssk)->icsk_ack.pending |= ICSK_ACK_NOMEM | ICSK_ACK_NOW;
> +	inet_csk_schedule_ack(ssk);

Just to be sure, no need to increment a MIB counter here?

> +	return true;
> +}
> +
> +/* Return false when the caller must drop the packet, i.e. in case of error,
> + * subflow has been reset, or over memory limits.
>   */
>  bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>  {
> @@ -1188,7 +1210,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>  
>  		__mptcp_data_acked(subflow->conn);
>  		mptcp_data_unlock(subflow->conn);
> -		return true;
> +		return !mptcp_over_limit(subflow->conn, sk, skb);
>  	}
>  
>  	mptcp_get_options(skb, &mp_opt);
(...)

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

Re: [PATCH v9 mptcp-next 2/6] mptcp: explicitly drop over memory limits

[Paolo Abeni]

On 5/28/26 10:21 AM, Matthieu Baerts wrote:
> On 27/05/2026 20:45, Paolo Abeni wrote:
>> Currently the enforcement of the rcvbuf constraint is implemented
>> when moving the skbs into the msk receive or OoO queue, keeping the
>> incoming skbs in the subflow queue when over limit.
>>
>> Under significant memory pressure the above can cause permanent data
>> transfer stalls, as the skb needed to make forward progress can be
>> stuck in a subflow queue.
>>
>> Over memory limits, drop the incoming skb, relaying on MPTCP-level
>> retransmissions.
>>
>> Note that fallback socket must perform the limit before the skb reaches
>> the subflow-level queue, as dropping an in-sequence already acked skb
>> would break the stream.
>>
>> This is not a complete fix for the stall issue, as the drop strategy
>> needs refinements that will come in the next patches.
> 
> Thank you for working on that!
> 
> (...)
> 
>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index 97ea4aa37b33..2b35bdc113a5 100644
>> --- a/net/mptcp/options.c
>> +++ b/net/mptcp/options.c
>> @@ -1161,8 +1161,30 @@ static bool add_addr_hmac_valid(struct mptcp_sock *msk,
>>  	return hmac == mp_opt->ahmac;
>>  }
>>  
>> -/* Return false in case of error (or subflow has been reset),
>> - * else return true.
>> +static bool mptcp_over_limit(struct sock *sk, struct sock *ssk,
>> +			     const struct sk_buff *skb)
>> +{
>> +	struct mptcp_sock *msk = mptcp_sk(sk);
>> +	u64 mem = sk_rmem_alloc_get(sk);
>> +
>> +	mem += READ_ONCE(msk->backlog_len);
>> +	if (likely(mem <= READ_ONCE(sk->sk_rcvbuf)))
>> +		return false;
>> +
>> +	/* Avoid silently dropping pure acks, fin or zero win probes. */
>> +	if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq ||
>> +	    TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN ||
>> +	    !after(TCP_SKB_CB(skb)->end_seq, tcp_sk(ssk)->rcv_nxt))
>> +		return false;
>> +
>> +	/* Dropped due to memory constraints, schedule an ack. */
>> +	inet_csk(ssk)->icsk_ack.pending |= ICSK_ACK_NOMEM | ICSK_ACK_NOW;
>> +	inet_csk_schedule_ack(ssk);
> 
> Just to be sure, no need to increment a MIB counter here?

Uhm... I did not consider that.

I guess:

	NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);

could be added here, but still the caller will do __kfree_skb instead of
using a proper drop reason.

I think/guess an eventual follow-up patch could do?

/P

Re: [PATCH v9 mptcp-next 2/6] mptcp: explicitly drop over memory limits

[Matthieu Baerts]

On 28/05/2026 18:26, Paolo Abeni wrote:
> On 5/28/26 10:21 AM, Matthieu Baerts wrote:
>> On 27/05/2026 20:45, Paolo Abeni wrote:
>>> Currently the enforcement of the rcvbuf constraint is implemented
>>> when moving the skbs into the msk receive or OoO queue, keeping the
>>> incoming skbs in the subflow queue when over limit.
>>>
>>> Under significant memory pressure the above can cause permanent data
>>> transfer stalls, as the skb needed to make forward progress can be
>>> stuck in a subflow queue.
>>>
>>> Over memory limits, drop the incoming skb, relaying on MPTCP-level
>>> retransmissions.
>>>
>>> Note that fallback socket must perform the limit before the skb reaches
>>> the subflow-level queue, as dropping an in-sequence already acked skb
>>> would break the stream.
>>>
>>> This is not a complete fix for the stall issue, as the drop strategy
>>> needs refinements that will come in the next patches.
>>
>> Thank you for working on that!
>>
>> (...)
>>
>>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>>> index 97ea4aa37b33..2b35bdc113a5 100644
>>> --- a/net/mptcp/options.c
>>> +++ b/net/mptcp/options.c
>>> @@ -1161,8 +1161,30 @@ static bool add_addr_hmac_valid(struct mptcp_sock *msk,
>>>  	return hmac == mp_opt->ahmac;
>>>  }
>>>  
>>> -/* Return false in case of error (or subflow has been reset),
>>> - * else return true.
>>> +static bool mptcp_over_limit(struct sock *sk, struct sock *ssk,
>>> +			     const struct sk_buff *skb)
>>> +{
>>> +	struct mptcp_sock *msk = mptcp_sk(sk);
>>> +	u64 mem = sk_rmem_alloc_get(sk);
>>> +
>>> +	mem += READ_ONCE(msk->backlog_len);
>>> +	if (likely(mem <= READ_ONCE(sk->sk_rcvbuf)))
>>> +		return false;
>>> +
>>> +	/* Avoid silently dropping pure acks, fin or zero win probes. */
>>> +	if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq ||
>>> +	    TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN ||
>>> +	    !after(TCP_SKB_CB(skb)->end_seq, tcp_sk(ssk)->rcv_nxt))
>>> +		return false;
>>> +
>>> +	/* Dropped due to memory constraints, schedule an ack. */
>>> +	inet_csk(ssk)->icsk_ack.pending |= ICSK_ACK_NOMEM | ICSK_ACK_NOW;
>>> +	inet_csk_schedule_ack(ssk);
>>
>> Just to be sure, no need to increment a MIB counter here?
> 
> Uhm... I did not consider that.

Thank you for your reply!

> I guess:
> 
> 	NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
> 
> could be added here, but still the caller will do __kfree_skb instead of
> using a proper drop reason.

Or a new MPTCP specific one. Or abusing MPTCP_MIB_RCVPRUNED?

> I think/guess an eventual follow-up patch could do?

Of course. It can also be a "Squash-to" patch if preferred.

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

Re: [PATCH v9 mptcp-next 4/6] mptcp: implemented OoO queue pruning

[Matthieu Baerts]

Hi Paolo,

On 27/05/2026 20:45, Paolo Abeni wrote:
> When moving incoming skbs in the msk receive queue and the latter
> is above limits, prune it as needed quite alike what TCP is doing
> at the subflow level. The main difference relies in the stop condition:
> since MPTCP does not perform collapsing, it's better off dropping the
> bare minimum to fit the (newer) incoming packet.

Thank you for the patch!

I have two small ideas below, but they are not blocking. If you think
they are valid, feel free to add a Squash-to patch in the v10, or ask me
to do the modifications.

I will apply the first 4 patches, adding this tag:

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Plus this one only on this patch:

Tested-by: Gang Yan <yangang@kylinos.cn>

(...)

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index b0a0c51e0a13..29cb10c02ed8 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -373,6 +373,45 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
>  	skb_dst_drop(skb);
>  }
>  
> +/* "Inspired" from the TCP version; main difference: stop as soon as the MPTCP
> + * socket is under memory limit.
> + */
> +static void mptcp_prune_ofo_queue(struct sock *sk, u64 seq)
> +{
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct rb_node *node, *prev;
> +	bool pruned = false;
> +	u64 mem;
> +
> +	if (RB_EMPTY_ROOT(&msk->out_of_order_queue))
> +		return;
> +
> +	node = &msk->ooo_last_skb->rbnode;
> +
> +	do {
> +		struct sk_buff *skb = rb_to_skb(node);
> +
> +		/* Stop pruning if the incoming skb would land in OoO tail. */
> +		if (after64(seq, MPTCP_SKB_CB(skb)->map_seq))
> +			break;
> +
> +		pruned = true;
> +		prev = rb_prev(node);
> +		rb_erase(node, &msk->out_of_order_queue);
> +		mptcp_drop(sk, skb);
> +		msk->ooo_last_skb = rb_to_skb(prev);
> +
> +		mem = (unsigned int)atomic_read(&sk->sk_rmem_alloc);

Detail: why not using sk_rmem_alloc_get() here, like you did in
__mptcp_move_skb(). Also there, sk->sk_rcvbuf is used with READ_ONCE(),
but not here. Is it on purpose?

> +		if (mem < sk->sk_rcvbuf)
> +			break;
> +
> +		node = prev;
> +	} while (node);
> +
> +	if (pruned)
> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFO_PRUNED);

Here, why not returning sk_rmem_alloc_get(sk) > READ_ONCE(sk->sk_rcvbuf)
...

> +}
> +
>  static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
>  {
>  	u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
> @@ -386,9 +425,12 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
>  	 */
>  	if (unlikely(sk_rmem_alloc_get(sk) > READ_ONCE(sk->sk_rcvbuf)) &&
>  	    !__mptcp_check_fallback(msk)) {

... so here you can simply add mptcp_prune_ofo_queue(...) and not change
what is below?

> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> -		mptcp_drop(sk, skb);
> -		return false;
> +		mptcp_prune_ofo_queue(sk, MPTCP_SKB_CB(skb)->map_seq);
> +		if (sk_rmem_alloc_get(sk) > READ_ONCE(sk->sk_rcvbuf)) {
> +			MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> +			mptcp_drop(sk, skb);
> +			return false;
> +		}
>  	}
>  
>  	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {

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

Re: [PATCH v9 mptcp-next 6/6] mptcp: let the retrans scheduler do its job.

[Matthieu Baerts]

Hi Paolo,

On 28/05/2026 17:45, Paolo Abeni wrote:
>  5/27/26 12:45 PM, Paolo Abeni wrote:
>> Currently the MPTCP core enforces that when MPTCP-level retrans timer
>> fires, at most a single dfrag is retransmitted. If some corner-cases it
>> may be necessary retransmit multiple dfrags, and the MPTCP socket will
>> need to wait multiple retrans timeout to accomplish that.
>>
>> Remove the mentioned constraint, allowing to transmit multiple dfrags per
>> retrans period, as long as the scheduler keeps selecting subflows for
>> retransmissions and pending data is available in the rtx queue.
>> The default scheduler will transmit a dfrag per available subflow.
>>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> 
> Sashiko concerns on this patch is real, and a new revision is needed.
> @Matttbe: could you please apply patches 1-4, as they makes sense as a
> whole? I'll resubmit a new version for the last 2. Patch 5 is correct,
> but makes no sense without this one.

Just did a quick review (I have a comment on patch 4), and applied
patches 1-4:

New patches for t/upstream-net and t/upstream:
- e1da5203df71: mptcp: allow subflow rcv wnd to shrink
- Results: 9ed1e5bcaf67..9f3fb817e1a6 (export-net)
- Results: c154734ef126..b0745f4933ef (export)

New patches for t/upstream:
- acd6981b76f5: mptcp: explicitly drop over memory limits
- 152803991ff0: mptcp: enforce hard limit on backlog flushing
- 58f9ba52dd21: mptcp: implemented OoO queue pruning
- Results: b0745f4933ef..e439b5a4647e (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/b581e1e5e4658bbef17e336e94acea9a7062244c/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/6394b9805eeb481113927da4d259c2798685114a/checks

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

Re: [PATCH v9 mptcp-next 2/6] mptcp: explicitly drop over memory limits

[Paolo Abeni]

On 5/28/26 10:32 AM, Matthieu Baerts wrote:
> On 28/05/2026 18:26, Paolo Abeni wrote:
>> I guess:
>>
>> 	NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
>>
>> could be added here, but still the caller will do __kfree_skb instead of
>> using a proper drop reason.
>
> Or a new MPTCP specific one. Or abusing MPTCP_MIB_RCVPRUNED?

The dropped skb is not pruned (removed from a queue were it was already
sitting). In theory we could add a MPTCPRCVQDROP MIB, but technically
the skb is dropped before reaching the _tcp_ receive queue, so I think
LINUX_MIB_TCPRCVQDROP could make sense.

>> I think/guess an eventual follow-up patch could do?
> 
> Of course. It can also be a "Squash-to" patch if preferred.

/P

Re: [PATCH v9 mptcp-next 4/6] mptcp: implemented OoO queue pruning

[Paolo Abeni]

On 5/28/26 10:45 AM, Matthieu Baerts wrote:
> On 27/05/2026 20:45, Paolo Abeni wrote:
>> When moving incoming skbs in the msk receive queue and the latter
>> is above limits, prune it as needed quite alike what TCP is doing
>> at the subflow level. The main difference relies in the stop condition:
>> since MPTCP does not perform collapsing, it's better off dropping the
>> bare minimum to fit the (newer) incoming packet.
> 
> Thank you for the patch!
> 
> I have two small ideas below, but they are not blocking. If you think
> they are valid, feel free to add a Squash-to patch in the v10, or ask me
> to do the modifications.
> 
> I will apply the first 4 patches, adding this tag:
> 
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> Plus this one only on this patch:
> 
> Tested-by: Gang Yan <yangang@kylinos.cn>
> 
> (...)
> 
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index b0a0c51e0a13..29cb10c02ed8 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -373,6 +373,45 @@ static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset,
>>  	skb_dst_drop(skb);
>>  }
>>  
>> +/* "Inspired" from the TCP version; main difference: stop as soon as the MPTCP
>> + * socket is under memory limit.
>> + */
>> +static void mptcp_prune_ofo_queue(struct sock *sk, u64 seq)
>> +{
>> +	struct mptcp_sock *msk = mptcp_sk(sk);
>> +	struct rb_node *node, *prev;
>> +	bool pruned = false;
>> +	u64 mem;
>> +
>> +	if (RB_EMPTY_ROOT(&msk->out_of_order_queue))
>> +		return;
>> +
>> +	node = &msk->ooo_last_skb->rbnode;
>> +
>> +	do {
>> +		struct sk_buff *skb = rb_to_skb(node);
>> +
>> +		/* Stop pruning if the incoming skb would land in OoO tail. */
>> +		if (after64(seq, MPTCP_SKB_CB(skb)->map_seq))
>> +			break;
>> +
>> +		pruned = true;
>> +		prev = rb_prev(node);
>> +		rb_erase(node, &msk->out_of_order_queue);
>> +		mptcp_drop(sk, skb);
>> +		msk->ooo_last_skb = rb_to_skb(prev);
>> +
>> +		mem = (unsigned int)atomic_read(&sk->sk_rmem_alloc);
> 
> Detail: why not using sk_rmem_alloc_get() here, like you did in
> __mptcp_move_skb(). Also there, sk->sk_rcvbuf is used with READ_ONCE(),
> but not here. Is it on purpose?

The READ_ONCE is omitted on purpose - this is under the msk socket lock,
can't race with write.

Raw sk_rmem_alloc access comes tcp code from C&P (inlining the
tcp_can_ingest() helper). I guess it could be replaced with
sk_rmem_alloc_get() for consistency.

>> +		if (mem < sk->sk_rcvbuf)
>> +			break;
>> +
>> +		node = prev;
>> +	} while (node);
>> +
>> +	if (pruned)
>> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFO_PRUNED);
> 
> Here, why not returning sk_rmem_alloc_get(sk) > READ_ONCE(sk->sk_rcvbuf)
> ...
> 
>> +}
>> +
>>  static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
>>  {
>>  	u64 copy_len = MPTCP_SKB_CB(skb)->end_seq - MPTCP_SKB_CB(skb)->map_seq;
>> @@ -386,9 +425,12 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
>>  	 */
>>  	if (unlikely(sk_rmem_alloc_get(sk) > READ_ONCE(sk->sk_rcvbuf)) &&
>>  	    !__mptcp_check_fallback(msk)) {
> 
> ... so here you can simply add mptcp_prune_ofo_queue(...) and not change
> what is below?

Looks good. Should I send a squash-to patch?

/P