Factor out all the skb initialization step in a new helper and
use it. Note that this change moves the MPTCP CB initialization
earlier: we can do such step as soon as the skb leaves the
subflow socket receive queues.

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
  - drop subflow argument from mptcp_init_skb()
  - change msk args to sock arg in __mptcp_move_skb()
---
 net/mptcp/protocol.c | 46 ++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c9fcdbaf50874..3aa03da781ba3 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -326,27 +326,11 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
 		mptcp_rcvbuf_grow(sk);
 }
 
-static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
-			     struct sk_buff *skb, unsigned int offset,
-			     size_t copy_len)
+static void mptcp_init_skb(struct sock *ssk,
+			   struct sk_buff *skb, int offset, int copy_len)
 {
-	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
-	struct sock *sk = (struct sock *)msk;
-	struct sk_buff *tail;
-	bool has_rxtstamp;
-
-	__skb_unlink(skb, &ssk->sk_receive_queue);
-
-	skb_ext_reset(skb);
-	skb_orphan(skb);
-
-	/* try to fetch required memory from subflow */
-	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
-		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
-		goto drop;
-	}
-
-	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
+	const struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
 
 	/* the skb map_seq accounts for the skb offset:
 	 * mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
@@ -358,6 +342,24 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
 	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
 	MPTCP_SKB_CB(skb)->cant_coalesce = 0;
 
+	__skb_unlink(skb, &ssk->sk_receive_queue);
+
+	skb_ext_reset(skb);
+	skb_dst_drop(skb);
+}
+
+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;
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	struct sk_buff *tail;
+
+	/* try to fetch required memory from subflow */
+	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
+		goto drop;
+	}
+
 	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
 		/* in sequence */
 		msk->bytes_received += copy_len;
@@ -678,7 +680,9 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
 		if (offset < skb->len) {
 			size_t len = skb->len - offset;
 
-			ret = __mptcp_move_skb(msk, ssk, skb, offset, len) || ret;
+			mptcp_init_skb(ssk, skb, offset, len);
+			skb_orphan(skb);
+			ret = __mptcp_move_skb(sk, skb) || ret;
 			seq += len;
 
 			if (unlikely(map_remaining < len)) {
-- 
2.51.0

Hi Paolo,

Thank you for the v3. All tests pass on my end, including my "implement
mptcp read_sock v11" test case.

I have a few minor cleanup comments to make the patch clean. There is
no need to send a v4 or block the merging of this series. Matt can make
the changes when he merges it, or I can send some squash-to patches to
address these after merging. Either approach would be fine.

On Fri, 2025-09-19 at 17:53 +0200, Paolo Abeni wrote:
> Factor out all the skb initialization step in a new helper and
> use it. Note that this change moves the MPTCP CB initialization
> earlier: we can do such step as soon as the skb leaves the
> subflow socket receive queues.
> 
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v1 -> v2:
>   - drop subflow argument from mptcp_init_skb()
>   - change msk args to sock arg in __mptcp_move_skb()
> ---
>  net/mptcp/protocol.c | 46 ++++++++++++++++++++++++------------------
> --
>  1 file changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index c9fcdbaf50874..3aa03da781ba3 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -326,27 +326,11 @@ static void mptcp_data_queue_ofo(struct
> mptcp_sock *msk, struct sk_buff *skb)
>  		mptcp_rcvbuf_grow(sk);
>  }
>  
> -static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock
> *ssk,
> -			     struct sk_buff *skb, unsigned int
> offset,
> -			     size_t copy_len)
> +static void mptcp_init_skb(struct sock *ssk,
> +			   struct sk_buff *skb, int offset, int
> copy_len)

nit:

static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb,
                           int offset, int copy_len)

is better.

>  {
> -	struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> -	struct sock *sk = (struct sock *)msk;
> -	struct sk_buff *tail;
> -	bool has_rxtstamp;
> -
> -	__skb_unlink(skb, &ssk->sk_receive_queue);
> -
> -	skb_ext_reset(skb);
> -	skb_orphan(skb);
> -
> -	/* try to fetch required memory from subflow */
> -	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> -		goto drop;
> -	}
> -
> -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> +	const struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> +	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;

nit, how about keeping it as the original code to make this patch
smaller:

-	struct mptcp_subflow_context *subflow =
mptcp_subflow_ctx(ssk);
-	bool has_rxtstamp;
-
-	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;

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

Thanks,
-Geliang

>  
>  	/* the skb map_seq accounts for the skb offset:
>  	 * mptcp_subflow_get_mapped_dsn() is based on the current
> tp->copied_seq
> @@ -358,6 +342,24 @@ static bool __mptcp_move_skb(struct mptcp_sock
> *msk, struct sock *ssk,
>  	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
>  	MPTCP_SKB_CB(skb)->cant_coalesce = 0;
>  
> +	__skb_unlink(skb, &ssk->sk_receive_queue);
> +
> +	skb_ext_reset(skb);
> +	skb_dst_drop(skb);
> +}
> +
> +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;
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct sk_buff *tail;
> +
> +	/* try to fetch required memory from subflow */
> +	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> +		goto drop;
> +	}
> +
>  	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
>  		/* in sequence */
>  		msk->bytes_received += copy_len;
> @@ -678,7 +680,9 @@ static bool __mptcp_move_skbs_from_subflow(struct
> mptcp_sock *msk,
>  		if (offset < skb->len) {
>  			size_t len = skb->len - offset;
>  
> -			ret = __mptcp_move_skb(msk, ssk, skb,
> offset, len) || ret;
> +			mptcp_init_skb(ssk, skb, offset, len);
> +			skb_orphan(skb);
> +			ret = __mptcp_move_skb(sk, skb) || ret;
>  			seq += len;
>  
>  			if (unlikely(map_remaining < len)) {

On Sat, 2025-09-20 at 08:03 +0800, Geliang Tang wrote:
> Hi Paolo,
> 
> Thank you for the v3. All tests pass on my end, including my
> "implement
> mptcp read_sock v11" test case.
> 
> I have a few minor cleanup comments to make the patch clean. There is
> no need to send a v4 or block the merging of this series. Matt can
> make
> the changes when he merges it, or I can send some squash-to patches
> to
> address these after merging. Either approach would be fine.
> 
> On Fri, 2025-09-19 at 17:53 +0200, Paolo Abeni wrote:
> > Factor out all the skb initialization step in a new helper and
> > use it. Note that this change moves the MPTCP CB initialization
> > earlier: we can do such step as soon as the skb leaves the
> > subflow socket receive queues.
> > 
> > Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > v1 -> v2:
> >   - drop subflow argument from mptcp_init_skb()
> >   - change msk args to sock arg in __mptcp_move_skb()
> > ---
> >  net/mptcp/protocol.c | 46 ++++++++++++++++++++++++----------------
> > --
> > --
> >  1 file changed, 25 insertions(+), 21 deletions(-)
> > 
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index c9fcdbaf50874..3aa03da781ba3 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -326,27 +326,11 @@ static void mptcp_data_queue_ofo(struct
> > mptcp_sock *msk, struct sk_buff *skb)
> >  		mptcp_rcvbuf_grow(sk);
> >  }
> >  
> > -static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock
> > *ssk,
> > -			     struct sk_buff *skb, unsigned int
> > offset,
> > -			     size_t copy_len)
> > +static void mptcp_init_skb(struct sock *ssk,
> > +			   struct sk_buff *skb, int offset, int
> > copy_len)
> 
> nit:
> 
> static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb,
>                            int offset, int copy_len)
> 
> is better.
> 
> >  {
> > -	struct mptcp_subflow_context *subflow =
> > mptcp_subflow_ctx(ssk);
> > -	struct sock *sk = (struct sock *)msk;
> > -	struct sk_buff *tail;
> > -	bool has_rxtstamp;
> > -
> > -	__skb_unlink(skb, &ssk->sk_receive_queue);
> > -
> > -	skb_ext_reset(skb);
> > -	skb_orphan(skb);
> > -
> > -	/* try to fetch required memory from subflow */
> > -	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> > -		MPTCP_INC_STATS(sock_net(sk),
> > MPTCP_MIB_RCVPRUNED);
> > -		goto drop;
> > -	}
> > -
> > -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> > +	const struct mptcp_subflow_context *subflow =
> > mptcp_subflow_ctx(ssk);
> > +	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> 
> nit, how about keeping it as the original code to make this patch
> smaller:
> 
> -	struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> -	bool has_rxtstamp;
> -
> -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> 
> Reviewed-by: Geliang Tang <geliang@kernel.org>
> Tested-by: Geliang Tang <geliang@kernel.org>
> 
> Thanks,
> -Geliang
> 
> >  
> >  	/* the skb map_seq accounts for the skb offset:
> >  	 * mptcp_subflow_get_mapped_dsn() is based on the current
> > tp->copied_seq
> > @@ -358,6 +342,24 @@ static bool __mptcp_move_skb(struct mptcp_sock
> > *msk, struct sock *ssk,
> >  	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
> >  	MPTCP_SKB_CB(skb)->cant_coalesce = 0;
> >  
> > +	__skb_unlink(skb, &ssk->sk_receive_queue);
> > +
> > +	skb_ext_reset(skb);
> > +	skb_dst_drop(skb);
> > +}
> > +
> > +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;
> > +	struct mptcp_sock *msk = mptcp_sk(sk);
> > +	struct sk_buff *tail;
> > +
> > +	/* try to fetch required memory from subflow */
> > +	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> > +		MPTCP_INC_STATS(sock_net(sk),
> > MPTCP_MIB_RCVPRUNED);
> > +		goto drop;
> > +	}
> > +
> >  	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
> >  		/* in sequence */
> >  		msk->bytes_received += copy_len;
> > @@ -678,7 +680,9 @@ static bool
> > __mptcp_move_skbs_from_subflow(struct
> > mptcp_sock *msk,
> >  		if (offset < skb->len) {
> >  			size_t len = skb->len - offset;
> >  
> > -			ret = __mptcp_move_skb(msk, ssk, skb,
> > offset, len) || ret;
> > +			mptcp_init_skb(ssk, skb, offset, len);
> > +			skb_orphan(skb);

One more small request: could you also put skb_orphan(skb); into
mptcp_init_skb, placing it on the last line, and then replace it in
patch 12.

Thanks,
-Geliang

> > +			ret = __mptcp_move_skb(sk, skb) || ret;
> >  			seq += len;
> >  
> >  			if (unlikely(map_remaining < len)) {

On Sun, 2025-09-21 at 08:23 +0800, Geliang Tang wrote:
> On Sat, 2025-09-20 at 08:03 +0800, Geliang Tang wrote:
> > Hi Paolo,
> > 
> > Thank you for the v3. All tests pass on my end, including my
> > "implement
> > mptcp read_sock v11" test case.
> > 
> > I have a few minor cleanup comments to make the patch clean. There
> > is
> > no need to send a v4 or block the merging of this series. Matt can
> > make
> > the changes when he merges it, or I can send some squash-to patches
> > to
> > address these after merging. Either approach would be fine.
> > 
> > On Fri, 2025-09-19 at 17:53 +0200, Paolo Abeni wrote:
> > > Factor out all the skb initialization step in a new helper and
> > > use it. Note that this change moves the MPTCP CB initialization
> > > earlier: we can do such step as soon as the skb leaves the
> > > subflow socket receive queues.
> > > 
> > > Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > ---
> > > v1 -> v2:
> > >   - drop subflow argument from mptcp_init_skb()
> > >   - change msk args to sock arg in __mptcp_move_skb()
> > > ---
> > >  net/mptcp/protocol.c | 46 ++++++++++++++++++++++++--------------
> > > --
> > > --
> > > --
> > >  1 file changed, 25 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > > index c9fcdbaf50874..3aa03da781ba3 100644
> > > --- a/net/mptcp/protocol.c
> > > +++ b/net/mptcp/protocol.c
> > > @@ -326,27 +326,11 @@ static void mptcp_data_queue_ofo(struct
> > > mptcp_sock *msk, struct sk_buff *skb)
> > >  		mptcp_rcvbuf_grow(sk);
> > >  }
> > >  
> > > -static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock
> > > *ssk,
> > > -			     struct sk_buff *skb, unsigned int
> > > offset,
> > > -			     size_t copy_len)
> > > +static void mptcp_init_skb(struct sock *ssk,
> > > +			   struct sk_buff *skb, int offset, int
> > > copy_len)
> > 
> > nit:
> > 
> > static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb,
> >                            int offset, int copy_len)
> > 
> > is better.
> > 
> > >  {
> > > -	struct mptcp_subflow_context *subflow =
> > > mptcp_subflow_ctx(ssk);
> > > -	struct sock *sk = (struct sock *)msk;
> > > -	struct sk_buff *tail;
> > > -	bool has_rxtstamp;
> > > -
> > > -	__skb_unlink(skb, &ssk->sk_receive_queue);
> > > -
> > > -	skb_ext_reset(skb);
> > > -	skb_orphan(skb);
> > > -
> > > -	/* try to fetch required memory from subflow */
> > > -	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> > > -		MPTCP_INC_STATS(sock_net(sk),
> > > MPTCP_MIB_RCVPRUNED);
> > > -		goto drop;
> > > -	}
> > > -
> > > -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> > > +	const struct mptcp_subflow_context *subflow =
> > > mptcp_subflow_ctx(ssk);
> > > +	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> > 
> > nit, how about keeping it as the original code to make this patch
> > smaller:
> > 
> > -	struct mptcp_subflow_context *subflow =
> > mptcp_subflow_ctx(ssk);
> > -	bool has_rxtstamp;
> > -
> > -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> > 
> > Reviewed-by: Geliang Tang <geliang@kernel.org>
> > Tested-by: Geliang Tang <geliang@kernel.org>
> > 
> > Thanks,
> > -Geliang
> > 
> > >  
> > >  	/* the skb map_seq accounts for the skb offset:
> > >  	 * mptcp_subflow_get_mapped_dsn() is based on the
> > > current
> > > tp->copied_seq
> > > @@ -358,6 +342,24 @@ static bool __mptcp_move_skb(struct
> > > mptcp_sock
> > > *msk, struct sock *ssk,
> > >  	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
> > >  	MPTCP_SKB_CB(skb)->cant_coalesce = 0;
> > >  
> > > +	__skb_unlink(skb, &ssk->sk_receive_queue);
> > > +
> > > +	skb_ext_reset(skb);
> > > +	skb_dst_drop(skb);
> > > +}
> > > +
> > > +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;
> > > +	struct mptcp_sock *msk = mptcp_sk(sk);
> > > +	struct sk_buff *tail;
> > > +
> > > +	/* try to fetch required memory from subflow */
> > > +	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> > > +		MPTCP_INC_STATS(sock_net(sk),
> > > MPTCP_MIB_RCVPRUNED);
> > > +		goto drop;
> > > +	}
> > > +
> > >  	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
> > >  		/* in sequence */
> > >  		msk->bytes_received += copy_len;
> > > @@ -678,7 +680,9 @@ static bool
> > > __mptcp_move_skbs_from_subflow(struct
> > > mptcp_sock *msk,
> > >  		if (offset < skb->len) {
> > >  			size_t len = skb->len - offset;
> > >  
> > > -			ret = __mptcp_move_skb(msk, ssk, skb,
> > > offset, len) || ret;
> > > +			mptcp_init_skb(ssk, skb, offset, len);
> > > +			skb_orphan(skb);
> 
> One more small request: could you also put skb_orphan(skb); into
> mptcp_init_skb, placing it on the last line, and then replace it in
> patch 12.

And please delete the sentence in the subject of this patch too.

Thanks,
-Geliang

> 
> Thanks,
> -Geliang
> 
> > > +			ret = __mptcp_move_skb(sk, skb) || ret;
> > >  			seq += len;
> > >  
> > >  			if (unlikely(map_remaining < len)) {

Hi Paolo,

Thank you for the v3. All tests pass on my end, including my "implement
mptcp read_sock v11" test case.

I have a few minor cleanup comments to make the patch clean. There is
no need to send a v4 or block the merging of this series. Matt can make
the changes when he merges it, or I can send some squash-to patches to
address these after merging. Either approach would be fine.

On Fri, 2025-09-19 at 17:53 +0200, Paolo Abeni wrote:
> Factor out all the skb initialization step in a new helper and
> use it. Note that this change moves the MPTCP CB initialization
> earlier: we can do such step as soon as the skb leaves the
> subflow socket receive queues.
> 
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v1 -> v2:
>   - drop subflow argument from mptcp_init_skb()
>   - change msk args to sock arg in __mptcp_move_skb()
> ---
>  net/mptcp/protocol.c | 46 ++++++++++++++++++++++++------------------
> --
>  1 file changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index c9fcdbaf50874..3aa03da781ba3 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -326,27 +326,11 @@ static void mptcp_data_queue_ofo(struct
> mptcp_sock *msk, struct sk_buff *skb)
>  		mptcp_rcvbuf_grow(sk);
>  }
>  
> -static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock
> *ssk,
> -			     struct sk_buff *skb, unsigned int
> offset,
> -			     size_t copy_len)
> +static void mptcp_init_skb(struct sock *ssk,
> +			   struct sk_buff *skb, int offset, int
> copy_len)

nit:

static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb,
                           int offset, int copy_len)

is better.

>  {
> -	struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> -	struct sock *sk = (struct sock *)msk;
> -	struct sk_buff *tail;
> -	bool has_rxtstamp;
> -
> -	__skb_unlink(skb, &ssk->sk_receive_queue);
> -
> -	skb_ext_reset(skb);
> -	skb_orphan(skb);
> -
> -	/* try to fetch required memory from subflow */
> -	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> -		goto drop;
> -	}
> -
> -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> +	const struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> +	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;

nit, how about keeping it as the original code to make this patch
smaller:

-	struct mptcp_subflow_context *subflow =
mptcp_subflow_ctx(ssk);
-	bool has_rxtstamp;
-
-	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;

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

Thanks,
-Geliang

>  
>  	/* the skb map_seq accounts for the skb offset:
>  	 * mptcp_subflow_get_mapped_dsn() is based on the current
> tp->copied_seq
> @@ -358,6 +342,24 @@ static bool __mptcp_move_skb(struct mptcp_sock
> *msk, struct sock *ssk,
>  	MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
>  	MPTCP_SKB_CB(skb)->cant_coalesce = 0;
>  
> +	__skb_unlink(skb, &ssk->sk_receive_queue);
> +
> +	skb_ext_reset(skb);
> +	skb_dst_drop(skb);
> +}
> +
> +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;
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct sk_buff *tail;
> +
> +	/* try to fetch required memory from subflow */
> +	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> +		goto drop;
> +	}
> +
>  	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
>  		/* in sequence */
>  		msk->bytes_received += copy_len;
> @@ -678,7 +680,9 @@ static bool __mptcp_move_skbs_from_subflow(struct
> mptcp_sock *msk,
>  		if (offset < skb->len) {
>  			size_t len = skb->len - offset;
>  
> -			ret = __mptcp_move_skb(msk, ssk, skb,
> offset, len) || ret;
> +			mptcp_init_skb(ssk, skb, offset, len);
> +			skb_orphan(skb);
> +			ret = __mptcp_move_skb(sk, skb) || ret;
>  			seq += len;
>  
>  			if (unlikely(map_remaining < len)) {

On 9/20/25 2:01 AM, Geliang Tang wrote:
> On Fri, 2025-09-19 at 17:53 +0200, Paolo Abeni wrote:
>> Factor out all the skb initialization step in a new helper and
>> use it. Note that this change moves the MPTCP CB initialization
>> earlier: we can do such step as soon as the skb leaves the
>> subflow socket receive queues.
>>
>> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>> v1 -> v2:
>>   - drop subflow argument from mptcp_init_skb()
>>   - change msk args to sock arg in __mptcp_move_skb()
>> ---
>>  net/mptcp/protocol.c | 46 ++++++++++++++++++++++++------------------
>> --
>>  1 file changed, 25 insertions(+), 21 deletions(-)
>>
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index c9fcdbaf50874..3aa03da781ba3 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -326,27 +326,11 @@ static void mptcp_data_queue_ofo(struct
>> mptcp_sock *msk, struct sk_buff *skb)
>>  		mptcp_rcvbuf_grow(sk);
>>  }
>>  
>> -static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock
>> *ssk,
>> -			     struct sk_buff *skb, unsigned int
>> offset,
>> -			     size_t copy_len)
>> +static void mptcp_init_skb(struct sock *ssk,
>> +			   struct sk_buff *skb, int offset, int
>> copy_len)
> 
> nit:
> 
> static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb,
>                            int offset, int copy_len)
> 
> is better.
> 
>>  {
>> -	struct mptcp_subflow_context *subflow =
>> mptcp_subflow_ctx(ssk);
>> -	struct sock *sk = (struct sock *)msk;
>> -	struct sk_buff *tail;
>> -	bool has_rxtstamp;
>> -
>> -	__skb_unlink(skb, &ssk->sk_receive_queue);
>> -
>> -	skb_ext_reset(skb);
>> -	skb_orphan(skb);
>> -
>> -	/* try to fetch required memory from subflow */
>> -	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
>> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
>> -		goto drop;
>> -	}
>> -
>> -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
>> +	const struct mptcp_subflow_context *subflow =
>> mptcp_subflow_ctx(ssk);
>> +	bool has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> 
> nit, how about keeping it as the original code to make this patch
> smaller:
> 
> -	struct mptcp_subflow_context *subflow =
> mptcp_subflow_ctx(ssk);
> -	bool has_rxtstamp;
> -
> -	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;

I would like to keep the 'struct mptcp_subflow_context' constantness,
helps GCC generating better code.

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

Thanks,

Paolo