[RESEND PATCH mptcp-net v2] mptcp: fix stall because of data_ready

[RESEND PATCH mptcp-net v2] mptcp: fix stall because of data_ready

[Gang Yan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3350 bytes --]

From: Gang Yan <yangang@kylinos.cn>

This patch fixes the second type of backlog_list stall issue that
occurs when the data_ready callback attempts to trigger data transfer.
The issue reproduces at approximately a 20% rate (once every five runs)
when running multi_chunk.sh tests.

The stall occurs under the following conditions:
1. A large amount of out-of-order (OFO) data causes sk_rmem_alloc to
   exceed sk_rcvbuf
2. The skb matching the current ack_seq is not present in backlog_list
3. Data reception relies on data_ready callback notification

In this scenario, the data_ready callback (via mptcp_data_ready() ->
sk->sk_data_ready()) attempts to trigger data movement, but
__mptcp_move_skbs_from_subflow() repeatedly moves the ack_seq skb into
backlog_list and returns false:

'''
msk->ack_seq:3442119990924456661, map_seq:3442119990924456661, offset:0, fin:0
[MPTCP_BACKLOG] #0 map_seq=3442119990924655746 end_seq=3442119990924660542 len=4796
[MPTCP_BACKLOG] #1 map_seq=3442119990924491850 end_seq=3442119990924500364 len=8514
[MPTCP_BACKLOG] #2 map_seq=3442119990924660542 end_seq=3442119990924726001 len=65459
[MPTCP_BACKLOG] #3 map_seq=3442119990924508114 end_seq=3442119990924514971 len=6857
[MPTCP_BACKLOG] #4 map_seq=3442119990924726001 end_seq=3442119990924731093 len=5092
[MPTCP_BACKLOG] #5 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
[MPTCP_BACKLOG] #6 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
[MPTCP_BACKLOG] #7 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
[MPTCP_BACKLOG] #8 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
[MPTCP_BACKLOG] #9 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
[MPTCP_BACKLOG] #10 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
[MPTCP_BACKLOG] #11 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
[MPTCP_BACKLOG] #12 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
...
'''

The fix allows moving in-sequence SKBs (map_seq == ack_seq) even when
the receive buffer is full, preventing the stall condition.

Fixes: 6228efe0cc01 ("mptcp: leverage the backlog for RX packet processing")
Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---

Notes:
    changelog:
      v2:
       - As suggested by Paolo, the previous empty‑queue check on the
         receive queue has been replaced with a check that permits
         in‑sequence SKBs.
      v1:
       - Paolo has reviewed at:
	 https://patchwork.kernel.org/project/mptcp/patch/de0a7e3ea6a3cac75b3c2c811695c5d674d929ef.1770273341.git.yangang@kylinos.cn/

 net/mptcp/protocol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c020882521b4..25ed246db7fb 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -739,7 +739,8 @@ 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 && (sk_rmem_alloc_get(sk) < sk->sk_rcvbuf ||
+					MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq)) {
 				mptcp_subflow_lend_fwdmem(subflow, skb);
 				ret |= __mptcp_move_skb(sk, skb);
 			} else {
-- 
2.43.0

Re: [RESEND PATCH mptcp-net v2] mptcp: fix stall because of data_ready

[Paolo Abeni]

On 2/28/26 2:15 AM, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
> 
> This patch fixes the second type of backlog_list stall issue that
> occurs when the data_ready callback attempts to trigger data transfer.
> The issue reproduces at approximately a 20% rate (once every five runs)
> when running multi_chunk.sh tests.
> 
> The stall occurs under the following conditions:
> 1. A large amount of out-of-order (OFO) data causes sk_rmem_alloc to
>    exceed sk_rcvbuf
> 2. The skb matching the current ack_seq is not present in backlog_list
> 3. Data reception relies on data_ready callback notification
> 
> In this scenario, the data_ready callback (via mptcp_data_ready() ->
> sk->sk_data_ready()) attempts to trigger data movement, but
> __mptcp_move_skbs_from_subflow() repeatedly moves the ack_seq skb into
> backlog_list and returns false:
> 
> '''
> msk->ack_seq:3442119990924456661, map_seq:3442119990924456661, offset:0, fin:0
> [MPTCP_BACKLOG] #0 map_seq=3442119990924655746 end_seq=3442119990924660542 len=4796
> [MPTCP_BACKLOG] #1 map_seq=3442119990924491850 end_seq=3442119990924500364 len=8514
> [MPTCP_BACKLOG] #2 map_seq=3442119990924660542 end_seq=3442119990924726001 len=65459
> [MPTCP_BACKLOG] #3 map_seq=3442119990924508114 end_seq=3442119990924514971 len=6857
> [MPTCP_BACKLOG] #4 map_seq=3442119990924726001 end_seq=3442119990924731093 len=5092
> [MPTCP_BACKLOG] #5 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
> [MPTCP_BACKLOG] #6 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
> [MPTCP_BACKLOG] #7 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
> [MPTCP_BACKLOG] #8 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
> [MPTCP_BACKLOG] #9 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
> [MPTCP_BACKLOG] #10 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
> [MPTCP_BACKLOG] #11 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
> [MPTCP_BACKLOG] #12 map_seq=3442119990924456661 end_seq=3442119990924464310 len=7649
> ...
> '''
> 
> The fix allows moving in-sequence SKBs (map_seq == ack_seq) even when
> the receive buffer is full, preventing the stall condition.
> 
> Fixes: 6228efe0cc01 ("mptcp: leverage the backlog for RX packet processing")
> Co-developed-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
> 
> Notes:
>     changelog:
>       v2:
>        - As suggested by Paolo, the previous empty‑queue check on the
>          receive queue has been replaced with a check that permits
>          in‑sequence SKBs.
>       v1:
>        - Paolo has reviewed at:
> 	 https://patchwork.kernel.org/project/mptcp/patch/de0a7e3ea6a3cac75b3c2c811695c5d674d929ef.1770273341.git.yangang@kylinos.cn/
> 
>  net/mptcp/protocol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index c020882521b4..25ed246db7fb 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -739,7 +739,8 @@ 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 && (sk_rmem_alloc_get(sk) < sk->sk_rcvbuf ||
> +					MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq)) {
>  				mptcp_subflow_lend_fwdmem(subflow, skb);
>  				ret |= __mptcp_move_skb(sk, skb);
>  			} else {

It just occurred to me since commit 6787b7e350d ("mptcp: avoid
processing packet if a subflow reset") we could early drop OoB skb in
mptcp_incoming_options() and possibly/likely completely remove the check
from here and elsewhere, something alike the following:

---
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index ac16e4bd496f..4834b2cd8142 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1167,6 +1167,9 @@ bool mptcp_incoming_options(struct sock *sk,
struct sk_buff *skb)
 	struct mptcp_options_received mp_opt;
 	struct mptcp_ext *mpext;

+	if (sk_rmem_alloc_get(msk) > READ_ONCE(subflow->conn->sk_rcvbuf))
+		return false;
+
 	if (__mptcp_check_fallback(msk)) {
 		/* Keep it simple and unconditionally trigger send data cleanup and
 		 * pending queue spooling. We will need to acquire the data lock
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c020882521b4..78ce00b14b23 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -739,7 +739,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 {
@@ -2176,10 +2176,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;
@@ -2207,9 +2203,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);
---

Note there could be unexpected side effects, it would need a bit of
testing in fast and slow env with/without drops, but on the flip side it
should also solve the more problematic "receive stalls when 'ack_seq' in
backlog_list"

/P