1. Patchew
  2. MPTCP
  3. [PATCH mptcp-net 0/2] mptcp: restore zero window probe
  4. View patch

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

Paolo Abeni posted 2 patches 1 week, 4 days ago
There is a newer version of this series
[PATCH mptcp-net 1/2] mptcp: restore window probe
Posted by Paolo Abeni 1 week, 4 days ago 
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
Posted by Mat Martineau 1 week, 3 days ago 
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
Posted by Paolo Abeni 1 week, 2 days ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.