[PATCH mptcp-net 0/2] mptcp: restore zero window probe

[PATCH mptcp-net 0/2] mptcp: restore zero window probe

[Paolo Abeni]

It turns out that the issue reported by Geliang on the pending backlog
patches is actually on older one that the new code made more easy
reproducible.

The first patch in the series addresses it[1], the 2nd one introduce new
mibs to hopefully catch this problem sooned.

I have a pktdrill to validate this scenario. I'll share that soon.

@Geliang could you please validate the above in your testbed, on top of
the backlog and splice patches?

[1] at least here, at least with the packet drill reproducer ;)

Paolo Abeni (2):
  mptcp: restore window probe
  mptcp: zero window probe mib

 net/mptcp/mib.c      |  1 +
 net/mptcp/mib.h      |  1 +
 net/mptcp/protocol.c | 15 +++++++++++----
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.51.0

[PATCH mptcp-net 1/2] mptcp: restore window probe

[Paolo Abeni]

Since commit 72377ab2d671 ("mptcp: more conservative check for zero
probes") the MPTCP-level zero window probe check is always disabled, as
the TCP-level write queue always contains at least the newly allocated
skb.

Refine the relevant check tacking in account that the above condition
and that such skb can have zero length.

Fixes: 72377ab2d671 ("mptcp: more conservative check for zero probes")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index bde76b7311f986..fe029359b7d7a2 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1276,6 +1276,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 	u64 data_seq = dfrag->data_seq + info->sent;
 	int offset = dfrag->offset + info->sent;
 	struct mptcp_sock *msk = mptcp_sk(sk);
+	struct tcp_sock *tp = tcp_sk(ssk);
 	bool zero_window_probe = false;
 	struct mptcp_ext *mpext = NULL;
 	bool can_coalesce = false;
@@ -1311,14 +1312,14 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 		mpext = mptcp_get_ext(skb);
 		if (!mptcp_skb_can_collapse_to(data_seq, skb, mpext)) {
 			TCP_SKB_CB(skb)->eor = 1;
-			tcp_mark_push(tcp_sk(ssk), skb);
+			tcp_mark_push(tp, skb);
 			goto alloc_skb;
 		}
 
 		i = skb_shinfo(skb)->nr_frags;
 		can_coalesce = skb_can_coalesce(skb, i, dfrag->page, offset);
 		if (!can_coalesce && i >= READ_ONCE(net_hotdata.sysctl_max_skb_frags)) {
-			tcp_mark_push(tcp_sk(ssk), skb);
+			tcp_mark_push(tp, skb);
 			goto alloc_skb;
 		}
 
@@ -1339,7 +1340,12 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 	if (copy == 0) {
 		u64 snd_una = READ_ONCE(msk->snd_una);
 
-		if (snd_una != msk->snd_nxt || tcp_write_queue_tail(ssk)) {
+		/* No need for zero probe if there are any data pending
+		 * either at the msk or ssk level; skb is the current write
+		 * queue tail and can be empty at this point.
+		 */
+		if (snd_una != msk->snd_nxt || skb->len ||
+		    skb != tcp_send_head(ssk)) {
 			tcp_remove_empty_skb(ssk);
 			return 0;
 		}
@@ -1367,7 +1373,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 	skb->truesize += copy;
 	sk_wmem_queued_add(ssk, copy);
 	sk_mem_charge(ssk, copy);
-	WRITE_ONCE(tcp_sk(ssk)->write_seq, tcp_sk(ssk)->write_seq + copy);
+	WRITE_ONCE(tp->write_seq, tp->write_seq + copy);
 	TCP_SKB_CB(skb)->end_seq += copy;
 	tcp_skb_pcount_set(skb, 0);
 
-- 
2.51.0

Re: [PATCH mptcp-net 1/2] mptcp: restore window probe

[Mat Martineau]

On Mon, 20 Oct 2025, Paolo Abeni wrote:

> Since commit 72377ab2d671 ("mptcp: more conservative check for zero
> probes") the MPTCP-level zero window probe check is always disabled, as
> the TCP-level write queue always contains at least the newly allocated
> skb.
>
> Refine the relevant check tacking in account that the above condition
> and that such skb can have zero length.
>
> Fixes: 72377ab2d671 ("mptcp: more conservative check for zero probes")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/protocol.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index bde76b7311f986..fe029359b7d7a2 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1276,6 +1276,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> 	u64 data_seq = dfrag->data_seq + info->sent;
> 	int offset = dfrag->offset + info->sent;
> 	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct tcp_sock *tp = tcp_sk(ssk);

Hi Paolo -

I appreciate the urge to tidy up the tcp_sk(ssk)s in this function, but it 
does distract from the meaningful code change - and the updated zero probe 
logic doesn't use the new tp variable...

Then again, if a netdev maintainer wants this in a -net patch, that 
opinion does carry some weight :)

> 	bool zero_window_probe = false;
> 	struct mptcp_ext *mpext = NULL;
> 	bool can_coalesce = false;
> @@ -1311,14 +1312,14 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> 		mpext = mptcp_get_ext(skb);
> 		if (!mptcp_skb_can_collapse_to(data_seq, skb, mpext)) {
> 			TCP_SKB_CB(skb)->eor = 1;
> -			tcp_mark_push(tcp_sk(ssk), skb);
> +			tcp_mark_push(tp, skb);
> 			goto alloc_skb;
> 		}
>
> 		i = skb_shinfo(skb)->nr_frags;
> 		can_coalesce = skb_can_coalesce(skb, i, dfrag->page, offset);
> 		if (!can_coalesce && i >= READ_ONCE(net_hotdata.sysctl_max_skb_frags)) {
> -			tcp_mark_push(tcp_sk(ssk), skb);
> +			tcp_mark_push(tp, skb);
> 			goto alloc_skb;
> 		}
>
> @@ -1339,7 +1340,12 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> 	if (copy == 0) {
> 		u64 snd_una = READ_ONCE(msk->snd_una);
>
> -		if (snd_una != msk->snd_nxt || tcp_write_queue_tail(ssk)) {
> +		/* No need for zero probe if there are any data pending
> +		 * either at the msk or ssk level; skb is the current write
> +		 * queue tail and can be empty at this point.
> +		 */
> +		if (snd_una != msk->snd_nxt || skb->len ||
> +		    skb != tcp_send_head(ssk)) {

LGTM

Reviewed-by: Mat Martineau <martineau@kernel.org>

> 			tcp_remove_empty_skb(ssk);
> 			return 0;
> 		}
> @@ -1367,7 +1373,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> 	skb->truesize += copy;
> 	sk_wmem_queued_add(ssk, copy);
> 	sk_mem_charge(ssk, copy);
> -	WRITE_ONCE(tcp_sk(ssk)->write_seq, tcp_sk(ssk)->write_seq + copy);
> +	WRITE_ONCE(tp->write_seq, tp->write_seq + copy);
> 	TCP_SKB_CB(skb)->end_seq += copy;
> 	tcp_skb_pcount_set(skb, 0);
>
> -- 
> 2.51.0
>
>
>

Re: [PATCH mptcp-net 1/2] mptcp: restore window probe

[Paolo Abeni]

On 10/21/25 2:12 AM, Mat Martineau wrote:
> On Mon, 20 Oct 2025, Paolo Abeni wrote:
>> Since commit 72377ab2d671 ("mptcp: more conservative check for zero
>> probes") the MPTCP-level zero window probe check is always disabled, as
>> the TCP-level write queue always contains at least the newly allocated
>> skb.
>>
>> Refine the relevant check tacking in account that the above condition
>> and that such skb can have zero length.
>>
>> Fixes: 72377ab2d671 ("mptcp: more conservative check for zero probes")
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>> net/mptcp/protocol.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index bde76b7311f986..fe029359b7d7a2 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -1276,6 +1276,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
>> 	u64 data_seq = dfrag->data_seq + info->sent;
>> 	int offset = dfrag->offset + info->sent;
>> 	struct mptcp_sock *msk = mptcp_sk(sk);
>> +	struct tcp_sock *tp = tcp_sk(ssk);
> 
> Hi Paolo -
> 
> I appreciate the urge to tidy up the tcp_sk(ssk)s in this function, but it 
> does distract from the meaningful code change - and the updated zero probe 
> logic doesn't use the new tp variable...

Whoops, due to a bit of rush on my side after spending ~1w looping on
the splice selftests ;) A previous iteration of this patch introduced a
new tcp_sock() usage hence the larger diffstat.

I'll send a v2 with the minimal fix.

/P

[PATCH mptcp-net 2/2] mptcp: zero window probe mib

[Paolo Abeni]

Explicitly account for MPTCP-level zero windows probe, to catch
hopefully earlier issues alike the one addressed by the previous
patch.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/mib.c      | 1 +
 net/mptcp/mib.h      | 1 +
 net/mptcp/protocol.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index 6003e47c770a7c..eb4645a9c5ac07 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -85,6 +85,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("DssFallback", MPTCP_MIB_DSSFALLBACK),
 	SNMP_MIB_ITEM("SimultConnectFallback", MPTCP_MIB_SIMULTCONNFALLBACK),
 	SNMP_MIB_ITEM("FallbackFailed", MPTCP_MIB_FALLBACKFAILED),
+	SNMP_MIB_ITEM("MPTcpWinProbe", MPTCP_MIB_MPTCPWINPROBE),
 };
 
 /* mptcp_mib_alloc - allocate percpu mib counters
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 309bac6fea3252..f83a113700522e 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -88,6 +88,7 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_DSSFALLBACK,		/* Bad or missing DSS */
 	MPTCP_MIB_SIMULTCONNFALLBACK,	/* Simultaneous connect */
 	MPTCP_MIB_FALLBACKFAILED,	/* Can't fallback due to msk status */
+	MPTCP_MIB_MPTCPWINPROBE,	/* MPTCP-level zero window probe */
 	__MPTCP_MIB_MAX
 };
 
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index fe029359b7d7a2..67919ae774cc40 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1396,6 +1396,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 		 mpext->dsn64);
 
 	if (zero_window_probe) {
+		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_MPTCPWINPROBE);
 		mptcp_subflow_ctx(ssk)->rel_write_seq += copy;
 		mpext->frozen = 1;
 		if (READ_ONCE(msk->csum_enabled))
-- 
2.51.0

Re: [PATCH mptcp-net 2/2] mptcp: zero window probe mib

[Mat Martineau]

On Mon, 20 Oct 2025, Paolo Abeni wrote:

> Explicitly account for MPTCP-level zero windows probe, to catch
> hopefully earlier issues alike the one addressed by the previous
> patch.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Helpful metric to track, looks good.

Reviewed-by: Mat Martineau <martineau@kernel.org>

> ---
> net/mptcp/mib.c      | 1 +
> net/mptcp/mib.h      | 1 +
> net/mptcp/protocol.c | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
> index 6003e47c770a7c..eb4645a9c5ac07 100644
> --- a/net/mptcp/mib.c
> +++ b/net/mptcp/mib.c
> @@ -85,6 +85,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
> 	SNMP_MIB_ITEM("DssFallback", MPTCP_MIB_DSSFALLBACK),
> 	SNMP_MIB_ITEM("SimultConnectFallback", MPTCP_MIB_SIMULTCONNFALLBACK),
> 	SNMP_MIB_ITEM("FallbackFailed", MPTCP_MIB_FALLBACKFAILED),
> +	SNMP_MIB_ITEM("MPTcpWinProbe", MPTCP_MIB_MPTCPWINPROBE),
> };
>
> /* mptcp_mib_alloc - allocate percpu mib counters
> diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
> index 309bac6fea3252..f83a113700522e 100644
> --- a/net/mptcp/mib.h
> +++ b/net/mptcp/mib.h
> @@ -88,6 +88,7 @@ enum linux_mptcp_mib_field {
> 	MPTCP_MIB_DSSFALLBACK,		/* Bad or missing DSS */
> 	MPTCP_MIB_SIMULTCONNFALLBACK,	/* Simultaneous connect */
> 	MPTCP_MIB_FALLBACKFAILED,	/* Can't fallback due to msk status */
> +	MPTCP_MIB_MPTCPWINPROBE,	/* MPTCP-level zero window probe */
> 	__MPTCP_MIB_MAX
> };
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index fe029359b7d7a2..67919ae774cc40 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1396,6 +1396,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> 		 mpext->dsn64);
>
> 	if (zero_window_probe) {
> +		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_MPTCPWINPROBE);
> 		mptcp_subflow_ctx(ssk)->rel_write_seq += copy;
> 		mpext->frozen = 1;
> 		if (READ_ONCE(msk->csum_enabled))
> -- 
> 2.51.0
>
>
>

Re: [PATCH mptcp-net 0/2] mptcp: restore zero window probe

[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): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Unstable: 1 failed test(s): selftest_mptcp_join 🔴
- 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/18658193252

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


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)