1. Patchew
  2. MPTCP
  3. [MPTCP next v3 00/12] mptcp: receive path improvement
  4. View patch

[MPTCP next v3 09/12] mptcp: cleanup fallback dummy mapping generation.

Paolo Abeni posted 12 patches 3 weeks ago
[MPTCP next v3 09/12] mptcp: cleanup fallback dummy mapping generation.
Posted by Paolo Abeni 3 weeks ago 
MPTCP currently access ack_seq outside the msk socket log scope to
generate the dummy mapping for fallback socket. Soon we are going
to introduce backlog usage and even for fallback socket the ack_seq
value will be significantly off outside of the msk socket lock scope.

Avoid relying on ack_seq for dummy mapping generation, using instead
the subflow sequence number. Note that in case of disconnect() and
(re)connect() we must ensure that any previous state is re-set.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v2 -> v3:
 - reordered before the backlog introduction to avoid transiently
   break the fallback
 - explicitly reset ack_seq
---
 net/mptcp/protocol.c | 3 +++
 net/mptcp/subflow.c  | 8 +++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index fce70cdad2a7f..c8b02048126a9 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3224,6 +3224,9 @@ static int mptcp_disconnect(struct sock *sk, int flags)
 	msk->bytes_retrans = 0;
 	msk->rcvspace_init = 0;
 
+	/* for fallback's sake */
+	WRITE_ONCE(msk->ack_seq, 0);
+
 	WRITE_ONCE(sk->sk_shutdown, 0);
 	sk_error_report(sk);
 	return 0;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index b9455c04e8a46..ac8616e7521e8 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -491,6 +491,9 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
 	mptcp_crypto_key_sha(subflow->remote_key, NULL, &subflow->iasn);
 	subflow->iasn++;
 
+	/* for fallback's sake */
+	subflow->map_seq = subflow->iasn;
+
 	WRITE_ONCE(msk->remote_key, subflow->remote_key);
 	WRITE_ONCE(msk->ack_seq, subflow->iasn);
 	WRITE_ONCE(msk->can_ack, true);
@@ -1435,9 +1438,12 @@ static bool subflow_check_data_avail(struct sock *ssk)
 
 	skb = skb_peek(&ssk->sk_receive_queue);
 	subflow->map_valid = 1;
-	subflow->map_seq = READ_ONCE(msk->ack_seq);
 	subflow->map_data_len = skb->len;
 	subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq - subflow->ssn_offset;
+	subflow->map_seq = __mptcp_expand_seq(subflow->map_seq,
+					      subflow->iasn +
+					      TCP_SKB_CB(skb)->seq -
+					      subflow->ssn_offset - 1);
 	WRITE_ONCE(subflow->data_avail, true);
 	return true;
 }
-- 
2.51.0
Re: [MPTCP next v3 09/12] mptcp: cleanup fallback dummy mapping generation.
Posted by Geliang Tang 3 weeks ago 
On Fri, 2025-09-19 at 17:53 +0200, Paolo Abeni wrote:
> MPTCP currently access ack_seq outside the msk socket log scope to
> generate the dummy mapping for fallback socket. Soon we are going
> to introduce backlog usage and even for fallback socket the ack_seq
> value will be significantly off outside of the msk socket lock scope.
> 
> Avoid relying on ack_seq for dummy mapping generation, using instead
> the subflow sequence number. Note that in case of disconnect() and
> (re)connect() we must ensure that any previous state is re-set.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v2 -> v3:
>  - reordered before the backlog introduction to avoid transiently
>    break the fallback
>  - explicitly reset ack_seq
> ---
>  net/mptcp/protocol.c | 3 +++
>  net/mptcp/subflow.c  | 8 +++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index fce70cdad2a7f..c8b02048126a9 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3224,6 +3224,9 @@ static int mptcp_disconnect(struct sock *sk,
> int flags)
>  	msk->bytes_retrans = 0;
>  	msk->rcvspace_init = 0;
>  
> +	/* for fallback's sake */
> +	WRITE_ONCE(msk->ack_seq, 0);
> +
>  	WRITE_ONCE(sk->sk_shutdown, 0);
>  	sk_error_report(sk);
>  	return 0;
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index b9455c04e8a46..ac8616e7521e8 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -491,6 +491,9 @@ static void subflow_set_remote_key(struct
> mptcp_sock *msk,
>  	mptcp_crypto_key_sha(subflow->remote_key, NULL, &subflow-
> >iasn);
>  	subflow->iasn++;
>  
> +	/* for fallback's sake */
> +	subflow->map_seq = subflow->iasn;
> +
>  	WRITE_ONCE(msk->remote_key, subflow->remote_key);
>  	WRITE_ONCE(msk->ack_seq, subflow->iasn);
>  	WRITE_ONCE(msk->can_ack, true);
> @@ -1435,9 +1438,12 @@ static bool subflow_check_data_avail(struct
> sock *ssk)
>  
>  	skb = skb_peek(&ssk->sk_receive_queue);
>  	subflow->map_valid = 1;
> -	subflow->map_seq = READ_ONCE(msk->ack_seq);

nit:

How about replacing this line in place? I mean, do not move it to a
later position:

+	subflow->map_seq = __mptcp_expand_seq(subflow->map_seq,
+					      subflow->iasn +
+					      TCP_SKB_CB(skb)->seq -
+					      subflow->ssn_offset -
1);

Reviewed-by: Geliang Tang <geliang@kernel.org>
Tested-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

>  	subflow->map_data_len = skb->len;
>  	subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq -
> subflow->ssn_offset;
> +	subflow->map_seq = __mptcp_expand_seq(subflow->map_seq,
> +					      subflow->iasn +
> +					      TCP_SKB_CB(skb)->seq -
> +					      subflow->ssn_offset -
> 1);
>  	WRITE_ONCE(subflow->data_avail, true);
>  	return true;
>  }
Re: [MPTCP next v3 09/12] mptcp: cleanup fallback dummy mapping generation.
Posted by Geliang Tang 2 weeks, 6 days ago 
On Sat, 2025-09-20 at 08:06 +0800, Geliang Tang wrote:
> On Fri, 2025-09-19 at 17:53 +0200, Paolo Abeni wrote:
> > MPTCP currently access ack_seq outside the msk socket log scope to
> > generate the dummy mapping for fallback socket. Soon we are going
> > to introduce backlog usage and even for fallback socket the ack_seq
> > value will be significantly off outside of the msk socket lock
> > scope.
> > 
> > Avoid relying on ack_seq for dummy mapping generation, using
> > instead
> > the subflow sequence number. Note that in case of disconnect() and
> > (re)connect() we must ensure that any previous state is re-set.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > v2 -> v3:
> >  - reordered before the backlog introduction to avoid transiently
> >    break the fallback
> >  - explicitly reset ack_seq
> > ---
> >  net/mptcp/protocol.c | 3 +++
> >  net/mptcp/subflow.c  | 8 +++++++-
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index fce70cdad2a7f..c8b02048126a9 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -3224,6 +3224,9 @@ static int mptcp_disconnect(struct sock *sk,
> > int flags)
> >  	msk->bytes_retrans = 0;
> >  	msk->rcvspace_init = 0;
> >  
> > +	/* for fallback's sake */
> > +	WRITE_ONCE(msk->ack_seq, 0);
> > +
> >  	WRITE_ONCE(sk->sk_shutdown, 0);
> >  	sk_error_report(sk);
> >  	return 0;
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index b9455c04e8a46..ac8616e7521e8 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -491,6 +491,9 @@ static void subflow_set_remote_key(struct
> > mptcp_sock *msk,
> >  	mptcp_crypto_key_sha(subflow->remote_key, NULL, &subflow-
> > > iasn);
> >  	subflow->iasn++;
> >  
> > +	/* for fallback's sake */
> > +	subflow->map_seq = subflow->iasn;
> > +
> >  	WRITE_ONCE(msk->remote_key, subflow->remote_key);
> >  	WRITE_ONCE(msk->ack_seq, subflow->iasn);
> >  	WRITE_ONCE(msk->can_ack, true);
> > @@ -1435,9 +1438,12 @@ static bool subflow_check_data_avail(struct
> > sock *ssk)
> >  
> >  	skb = skb_peek(&ssk->sk_receive_queue);
> >  	subflow->map_valid = 1;
> > -	subflow->map_seq = READ_ONCE(msk->ack_seq);
> 
> nit:
> 
> How about replacing this line in place? I mean, do not move it to a
> later position:
> 
> +	subflow->map_seq = __mptcp_expand_seq(subflow->map_seq,
> +					      subflow->iasn +
> +					      TCP_SKB_CB(skb)->seq -
> +					      subflow->ssn_offset -
> 1);
> 
> Reviewed-by: Geliang Tang <geliang@kernel.org>
> Tested-by: Geliang Tang <geliang@kernel.org>

Please delete the sentence in the subject of this patch too.

Thanks,
-Geliang

> 
> Thanks,
> -Geliang
> 
> >  	subflow->map_data_len = skb->len;
> >  	subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq -
> > subflow->ssn_offset;
> > +	subflow->map_seq = __mptcp_expand_seq(subflow->map_seq,
> > +					      subflow->iasn +
> > +					      TCP_SKB_CB(skb)->seq
> > -
> > +					      subflow->ssn_offset
> > -
> > 1);
> >  	WRITE_ONCE(subflow->data_avail, true);
> >  	return true;
> >  }

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.