[PATCH v2 mptcp-net 0/2] mptcp: a couple of fixes

[PATCH v2 mptcp-net 0/2] mptcp: a couple of fixes

[Paolo Abeni]

The first patch addresses a recently filed issue, 2nd one is due to
code inspection while investigating the mentioned issue.

v1 -> v2:
  - fix pktdrill failures due to missing rcv_wnd_sent initialization

Paolo Abeni (2):
  mptcp: do not fallback when OoO is present
  mptcp: do not drop partial packets.

 net/mptcp/protocol.c | 30 ++++++++++++++++++++++--------
 net/mptcp/subflow.c  |  3 ++-
 2 files changed, 24 insertions(+), 9 deletions(-)

-- 
2.51.1

[PATCH v2 mptcp-net 1/2] mptcp: do not fallback when OoO is present

[Paolo Abeni]

In case of DSS corruption, the MPTCP protocol tries to avoid the
subflow reset if fallback is possible. Such corruptions happen in
the receive path; to ensure fallback is possible the stack additionally
need to check for OoO data, otherwise the fallback will break the data
stream.

Fixes: e32d262c89e2 ("mptcp: handle consistently DSS corruption")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/598
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Note: this does not avoid the WARN(), but fixes the inconsistend
read() behavior; the ingress data is OoO, we should not ack it
---
 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 d6b08e1de358..7b966f105f89 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -646,7 +646,8 @@ static void mptcp_check_data_fin(struct sock *sk)
 
 static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
 {
-	if (!mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
+	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue) ||
+	    !mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DSSCORRUPTIONRESET);
 		mptcp_subflow_reset(ssk);
 	}
-- 
2.51.1

[PATCH v2 mptcp-net 2/2] mptcp: do not drop partial packets.

[Paolo Abeni]

Currently MPTCP drops partial packets for no good reason at all.
Instead we should just skip the already acked bytes.

Also add a missing check for zero window, which in turn requires
properly initializing the rcv_wnd_sent at connection creation time.

Fixes: ab174ad8ef76 ("mptcp: move ooo skbs into msk out of order queue.")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Notes:
- We should also add a MIB for zerowin drop, but that should be a follow-up
patch, I think.
- based on top of 'mptcp: autotune related improvement', but targeting net,
  will have a conflict in __mptcp_move_skb().
---
 net/mptcp/protocol.c | 27 ++++++++++++++++++++-------
 net/mptcp/subflow.c  |  3 ++-
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7b966f105f89..e3330141a6da 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -378,6 +378,10 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
 
 	mptcp_borrow_fwdmem(sk, skb);
 
+	/* Check for zero window.*/
+	if (atomic64_read(&msk->rcv_wnd_sent) == msk->ack_seq)
+		goto drop;
+
 	if (MPTCP_SKB_CB(skb)->map_seq == msk->ack_seq) {
 		/* in sequence */
 		msk->bytes_received += copy_len;
@@ -386,18 +390,27 @@ static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb)
 		if (tail && mptcp_try_coalesce(sk, tail, skb))
 			return true;
 
-		skb_set_owner_r(skb, sk);
-		__skb_queue_tail(&sk->sk_receive_queue, skb);
-		return true;
+		goto enqueue;
 	} else if (after64(MPTCP_SKB_CB(skb)->map_seq, msk->ack_seq)) {
 		mptcp_data_queue_ofo(msk, skb);
 		return false;
 	}
 
-	/* old data, keep it simple and drop the whole pkt, sender
-	 * will retransmit as needed, if needed.
-	 */
-	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
+	/* Check for old data. */
+	if (!after64(MPTCP_SKB_CB(skb)->end_seq, msk->ack_seq)) {
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
+		goto drop;
+	}
+
+	/* Partial packet, seq < rcv_next < end_seq. */
+	MPTCP_SKB_CB(skb)->offset += msk->ack_seq - MPTCP_SKB_CB(skb)->map_seq;
+
+enqueue:
+	skb_set_owner_r(skb, sk);
+	__skb_queue_tail(&sk->sk_receive_queue, skb);
+	return true;
+
+drop:
 	mptcp_drop(sk, skb);
 	return false;
 }
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 39bf69e73975..0db832b5ddca 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -496,7 +496,8 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
 	WRITE_ONCE(msk->remote_key, subflow->remote_key);
 	WRITE_ONCE(msk->ack_seq, subflow->iasn);
 	WRITE_ONCE(msk->can_ack, true);
-	atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
+	atomic64_set(&msk->rcv_wnd_sent, subflow->iasn +
+		     tcp_receive_window(tcp_sk(subflow->tcp_sock)));
 }
 
 static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
-- 
2.51.1

Re: [PATCH v2 mptcp-net 0/2] mptcp: a couple of fixes

[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): Unstable: 2 failed test(s): selftest_mptcp_connect selftest_mptcp_connect_checksum 🔴
- 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): Unstable: 2 failed test(s): bpftest_test_progs-cpuv4_mptcp bpftest_test_progs-no_alu32_mptcp 🔴
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/19258711974

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


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)

Re: [PATCH v2 mptcp-net 1/2] mptcp: do not fallback when OoO is present

[Matthieu Baerts]

Hi Paolo,

On 11/11/2025 08:24, Paolo Abeni wrote:
> In case of DSS corruption, the MPTCP protocol tries to avoid the
> subflow reset if fallback is possible. Such corruptions happen in
> the receive path; to ensure fallback is possible the stack additionally
> need to check for OoO data, otherwise the fallback will break the data
> stream.

Thank you for the fix!
> Fixes: e32d262c89e2 ("mptcp: handle consistently DSS corruption")
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/598
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> Note: this does not avoid the WARN(), but fixes the inconsistend
> read() behavior; the ingress data is OoO, we should not ack it
> ---
>  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 d6b08e1de358..7b966f105f89 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -646,7 +646,8 @@ static void mptcp_check_data_fin(struct sock *sk)
>  
>  static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
>  {
> -	if (!mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
> +	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue) ||
> +	    !mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {

Does it mean we should check the OoO queue each time mptcp_try_fallback
is called?

Should we not eventually set msk->allow_infinite_fallback to false in
mptcp_data_queue_ofo()?

>  		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DSSCORRUPTIONRESET);
>  		mptcp_subflow_reset(ssk);
>  	}

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH v2 mptcp-net 1/2] mptcp: do not fallback when OoO is present

[Paolo Abeni]

On 11/11/25 6:50 PM, Matthieu Baerts wrote:
> On 11/11/2025 08:24, Paolo Abeni wrote:
>> In case of DSS corruption, the MPTCP protocol tries to avoid the
>> subflow reset if fallback is possible. Such corruptions happen in
>> the receive path; to ensure fallback is possible the stack additionally
>> need to check for OoO data, otherwise the fallback will break the data
>> stream.
> 
> Thank you for the fix!
>> Fixes: e32d262c89e2 ("mptcp: handle consistently DSS corruption")
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/598
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>> Note: this does not avoid the WARN(), but fixes the inconsistend
>> read() behavior; the ingress data is OoO, we should not ack it
>> ---
>>  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 d6b08e1de358..7b966f105f89 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -646,7 +646,8 @@ static void mptcp_check_data_fin(struct sock *sk)
>>  
>>  static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
>>  {
>> -	if (!mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
>> +	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue) ||
>> +	    !mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
> 
> Does it mean we should check the OoO queue each time mptcp_try_fallback
> is called?
> 
> Should we not eventually set msk->allow_infinite_fallback to false in
> mptcp_data_queue_ofo()?

Good question. According to the RFC here we should unconditionally reset
the subflow. "historically" we try hard to fallback - even because in
early releases fallback was a bit to easy to obtain.

Setting msk->allow_infinite_fallback = false in mptcp_data_queue_ofo()
could possibly hit performances. queue_ofo is basically fastpath with
multiple streams and we will likely need to acquire the fallback lock to
to the thing race free.

I'm tempted to just do a plain reset here. WDYT?

Thanks,

Paolo

Re: [PATCH v2 mptcp-net 1/2] mptcp: do not fallback when OoO is present

[Matthieu Baerts]

Hi Paolo,

On 13/11/2025 01:02, Paolo Abeni wrote:
> On 11/11/25 6:50 PM, Matthieu Baerts wrote:
>> On 11/11/2025 08:24, Paolo Abeni wrote:
>>> In case of DSS corruption, the MPTCP protocol tries to avoid the
>>> subflow reset if fallback is possible. Such corruptions happen in
>>> the receive path; to ensure fallback is possible the stack additionally
>>> need to check for OoO data, otherwise the fallback will break the data
>>> stream.
>>
>> Thank you for the fix!
>>> Fixes: e32d262c89e2 ("mptcp: handle consistently DSS corruption")
>>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/598
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>> ---
>>> Note: this does not avoid the WARN(), but fixes the inconsistend
>>> read() behavior; the ingress data is OoO, we should not ack it
>>> ---
>>>  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 d6b08e1de358..7b966f105f89 100644
>>> --- a/net/mptcp/protocol.c
>>> +++ b/net/mptcp/protocol.c
>>> @@ -646,7 +646,8 @@ static void mptcp_check_data_fin(struct sock *sk)
>>>  
>>>  static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
>>>  {
>>> -	if (!mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
>>> +	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue) ||
>>> +	    !mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
>>
>> Does it mean we should check the OoO queue each time mptcp_try_fallback
>> is called?
>>
>> Should we not eventually set msk->allow_infinite_fallback to false in
>> mptcp_data_queue_ofo()?
> 
> Good question. According to the RFC here we should unconditionally reset
> the subflow. "historically" we try hard to fallback - even because in
> early releases fallback was a bit to easy to obtain.
> 
> Setting msk->allow_infinite_fallback = false in mptcp_data_queue_ofo()
> could possibly hit performances. queue_ofo is basically fastpath with
> multiple streams and we will likely need to acquire the fallback lock to
> to the thing race free.
> 
> I'm tempted to just do a plain reset here. WDYT?

Can mptcp_dss_corruption() not be called before being in fully
established mode? In this case, should we not fallback instead? e.g. if
a middlebox start to alter MPTCP options after the 3WHS?

Or maybe moving this RB_EMPTY_ROOT(&msk->out_of_order_queue) check to
mptcp_try_fallback()? (mmh, we don't have the msk)

Or maybe just a plain reset is OK here?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH v2 mptcp-net 1/2] mptcp: do not fallback when OoO is present

[Paolo Abeni]

On 11/13/25 10:08 AM, Matthieu Baerts wrote:
> On 13/11/2025 01:02, Paolo Abeni wrote:
>> On 11/11/25 6:50 PM, Matthieu Baerts wrote:
>>> On 11/11/2025 08:24, Paolo Abeni wrote:
>>>> In case of DSS corruption, the MPTCP protocol tries to avoid the
>>>> subflow reset if fallback is possible. Such corruptions happen in
>>>> the receive path; to ensure fallback is possible the stack additionally
>>>> need to check for OoO data, otherwise the fallback will break the data
>>>> stream.
>>>
>>> Thank you for the fix!
>>>> Fixes: e32d262c89e2 ("mptcp: handle consistently DSS corruption")
>>>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/598
>>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>>> ---
>>>> Note: this does not avoid the WARN(), but fixes the inconsistend
>>>> read() behavior; the ingress data is OoO, we should not ack it
>>>> ---
>>>>  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 d6b08e1de358..7b966f105f89 100644
>>>> --- a/net/mptcp/protocol.c
>>>> +++ b/net/mptcp/protocol.c
>>>> @@ -646,7 +646,8 @@ static void mptcp_check_data_fin(struct sock *sk)
>>>>  
>>>>  static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
>>>>  {
>>>> -	if (!mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
>>>> +	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue) ||
>>>> +	    !mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
>>>
>>> Does it mean we should check the OoO queue each time mptcp_try_fallback
>>> is called?
>>>
>>> Should we not eventually set msk->allow_infinite_fallback to false in
>>> mptcp_data_queue_ofo()?
>>
>> Good question. According to the RFC here we should unconditionally reset
>> the subflow. "historically" we try hard to fallback - even because in
>> early releases fallback was a bit to easy to obtain.
>>
>> Setting msk->allow_infinite_fallback = false in mptcp_data_queue_ofo()
>> could possibly hit performances. queue_ofo is basically fastpath with
>> multiple streams and we will likely need to acquire the fallback lock to
>> to the thing race free.
>>
>> I'm tempted to just do a plain reset here. WDYT?
> 
> Can mptcp_dss_corruption() not be called before being in fully
> established mode? In this case, should we not fallback instead? e.g. if
> a middlebox start to alter MPTCP options after the 3WHS?

Even if we would be before reaching fully established status, the DSS
corruption caused by the pktdrill test causes MPTCP-level OoO with data
already acked at the TCP level. We can't drop it nor we can safely
fallback, we have do reset (if ofo queue) is not empty.
> Or maybe moving this RB_EMPTY_ROOT(&msk->out_of_order_queue) check to
> mptcp_try_fallback()? (mmh, we don't have the msk)
> 
> Or maybe just a plain reset is OK here?
I think the main question is if constraining such resets to OoO cases
(in the attempt to do fallback in other scenarios) or always reset here.

I propose to keep it simple, minimize the difference from current status
and always check OoO.

/P

Re: [PATCH v2 mptcp-net 1/2] mptcp: do not fallback when OoO is present

[Matthieu Baerts]

On 13/11/2025 18:32, Paolo Abeni wrote:
> On 11/13/25 10:08 AM, Matthieu Baerts wrote:
>> On 13/11/2025 01:02, Paolo Abeni wrote:
>>> On 11/11/25 6:50 PM, Matthieu Baerts wrote:
>>>> On 11/11/2025 08:24, Paolo Abeni wrote:
>>>>> In case of DSS corruption, the MPTCP protocol tries to avoid the
>>>>> subflow reset if fallback is possible. Such corruptions happen in
>>>>> the receive path; to ensure fallback is possible the stack additionally
>>>>> need to check for OoO data, otherwise the fallback will break the data
>>>>> stream.
>>>>
>>>> Thank you for the fix!
>>>>> Fixes: e32d262c89e2 ("mptcp: handle consistently DSS corruption")
>>>>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/598
>>>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>>>> ---
>>>>> Note: this does not avoid the WARN(), but fixes the inconsistend
>>>>> read() behavior; the ingress data is OoO, we should not ack it
>>>>> ---
>>>>>  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 d6b08e1de358..7b966f105f89 100644
>>>>> --- a/net/mptcp/protocol.c
>>>>> +++ b/net/mptcp/protocol.c
>>>>> @@ -646,7 +646,8 @@ static void mptcp_check_data_fin(struct sock *sk)
>>>>>  
>>>>>  static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
>>>>>  {
>>>>> -	if (!mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
>>>>> +	if (!RB_EMPTY_ROOT(&msk->out_of_order_queue) ||
>>>>> +	    !mptcp_try_fallback(ssk, MPTCP_MIB_DSSCORRUPTIONFALLBACK)) {
>>>>
>>>> Does it mean we should check the OoO queue each time mptcp_try_fallback
>>>> is called?
>>>>
>>>> Should we not eventually set msk->allow_infinite_fallback to false in
>>>> mptcp_data_queue_ofo()?
>>>
>>> Good question. According to the RFC here we should unconditionally reset
>>> the subflow. "historically" we try hard to fallback - even because in
>>> early releases fallback was a bit to easy to obtain.
>>>
>>> Setting msk->allow_infinite_fallback = false in mptcp_data_queue_ofo()
>>> could possibly hit performances. queue_ofo is basically fastpath with
>>> multiple streams and we will likely need to acquire the fallback lock to
>>> to the thing race free.
>>>
>>> I'm tempted to just do a plain reset here. WDYT?
>>
>> Can mptcp_dss_corruption() not be called before being in fully
>> established mode? In this case, should we not fallback instead? e.g. if
>> a middlebox start to alter MPTCP options after the 3WHS?
> 
> Even if we would be before reaching fully established status, the DSS
> corruption caused by the pktdrill test causes MPTCP-level OoO with data
> already acked at the TCP level. We can't drop it nor we can safely
> fallback, we have do reset (if ofo queue) is not empty.

Yes, OK to reset in this case.

>> Or maybe moving this RB_EMPTY_ROOT(&msk->out_of_order_queue) check to
>> mptcp_try_fallback()? (mmh, we don't have the msk)
>>
>> Or maybe just a plain reset is OK here?
>
> I think the main question is if constraining such resets to OoO cases
> (in the attempt to do fallback in other scenarios) or always reset here.
> 
> I propose to keep it simple, minimize the difference from current status
> and always check OoO.

Just to be sure I understand this correctly: your prefer keeping the
patch as it is, and there is no need to move the OoO check to the
__mptcp_try_fallback() help to check it in other cases?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.