Re: [PATCH mptcp-next RFC v4 2/2] mptcp: send MP_FAIL and infinite mapping on the same ACK

[Matthieu Baerts <matttbe@kernel.org>]

Hi Chenguang,

Thank you for your reply!

On 25/08/2026 08:33, Chenguang Zhao wrote:
> 
> 在 2026/8/20 00:06, Matthieu Baerts 写道:
>> Hi Chenguang,
>>
>> On 19/08/2026 13:16, Chenguang Zhao wrote:
>>> 在 2026/8/19 00:19, Matthieu Baerts 写道:
>>>> Hi Chenguang,
>>>>
>>>> On 18/08/2026 11:14, Chenguang Zhao wrote:
>>>>> From: Chenguang Zhao <zhaochenguang@kylinos.cn>
>>>>>
>>>>> On a single subflow, accepting MP_FAIL must leave MPTCP mode at once
>>>>> (RFC8684 §3.7). Emit the infinite mapping on the MP_FAIL reply ACK,
>>>>> then call mptcp_try_fallback() immediately after.
>>>> I think Sashiko is right about the issue of lacking option space to have
>>>> both the MP_FAIL and the infinite mapping sent together.
>>> Yes, Sashiko is right, I did not consider all the aspects.
>>>> Please also note this from RFC 8684 §3.7.11 [1]:
>>>>
>>>>> This infinite mapping will be a DSS option (Section 3.3) on the first
>>>>> new packet, containing a Data Sequence Mapping that acts retroactively
>>>> The "on the first new packet" seems to suggest the infinite mapping
>>>> shouldn't be attached to the MP_FAIL.
>>>>
>>>> [1] https://datatracker.ietf.org/doc/html/rfc8684#section-3.7-11
>>>>
>>>>
>>>> Maybe a fallback for the single subflow without csum is enough? Or maybe
>>>> nothing needs to be done? Or maybe this fallback is just an
>>>> optimisation, and can be done in net-next (and both commits can be
>>>> combined)? WDYT?
>>> Hi Matthieu
>>>
>>> I am getting a bit confused, so let me summarize my current understanding to verify if I got this right.
>>> In the existing kernel code, fallback will be triggered upon receiving MP_FAIL. However, the FALLBACK_DONE
>>> flag is currently set inside mptcp_update_infinite_map(), which only gets invoked when transmitting a data packet.
>>> This creates a timing window between receiving MP_FAIL and sending the first subsequent packet, resulting in this minor bug.
>>> Our goal is to close this timing window: set the FALLBACK_DONE flag immediately after MP_FAIL is received.
>>> Besides, add a __mptcp_check_fallback() check within mptcp_can_accept_new_subflow() to reject MP_JOIN requests ahead of time.
>>>
>>> Is my understanding correct? I will send out a v5 RFC based on the above ideas.      
>> I'm not sure that it can work: if there is a fallback, no MPTCP options
>> are supposed to be sent after that point. That's why the fallback should
>> be done after having sent the infinite mapping.
>>
>> I didn't check in the details: *maybe* the fully_established bit could
>> be reset earlier in some cases, but it feels like it can only be done
>> when a fallback can be done, so never had extra subflows or reinjection,
>> and no checksum. But then again, in this case, we are not supposed to
>> receive an MP_FAIL. Then why bother and not simply fallback, and that's
>> it, no?
>>
>> Cheers,
>> Matt
> 
> Hi Matt,
> 
> Thanks for your feedback. I'd like to share some thoughts on the
> 
> practical impact of the extra_subflow window.While looking into this, 
> 
> I realized that the window where the leak can occur is actually quite
> 
> narrow in practice:
> 
> 	1. MP_FAIL itself is rare — it requires a checksum validation failure,
> 
> 	   which typically only happens when a middlebox corrupts in-flight
> 
> 	   data.
> 
> 	2. It only applies to single-subflow connections — for multi-subflow
> 
> 	   connections, allow_infinite_fallback is cleared by
> 
> 	   mptcp_subflow_joined() when the second subflow joins, so the
> 
> 	   fallback path in mptcp_pm_mp_fail_received() is never reached.
> 
> 	3. The window is very short — it spans from the MP_FAIL ACK to the
> 
> 	   first data packet carrying the infinite mapping, which is on the
> 
> 	   order of microseconds under normal conditions.
> 	
> 	4. Even deferring the fallback to the first data packet (as the
> 	   original code does) does not appear to cause issues in practice,
> 	   since the infinite mapping is still sent correctly and the
> 	   connection eventually transitions to TCP.
> 
> Given the above, I'm inclined to put this issue on hold for now and
> 
> revisit it if a concrete need arises. I'd appreciate your thoughts on
> 
> whether that makes sense.
Fine by me. For this specific case (single-subflow, no csum), either we:

- do nothing special because it shouldn't happen and what we are doing
is not wrong

- or we do the minimum, i.e. a fallback without infinite mapping, e.g.

  if (!READ_ONCE(msk->csum_enabled) && mptcp_try_fallback(...))
      (...)

Just one last thing while we are talking about MP_FAIL, I wonder if we
do fully respect the RFC, e.g.:

- when multiple subflows are in used, an MP_FAIL should send an MP_FAIL
back (on the same subflow! is it always the case?) + reset the current
subflow (OK I think), but no infinite mapping to send in this case + new
subflows are still allowed

  https://datatracker.ietf.org/doc/html/rfc8684#section-3.7-8


- when an MP_FAIL is received on the single subflow case and it is not
possible to send an infinite mapping, do we reset it?

  https://datatracker.ietf.org/doc/html/rfc8684#section-3.7-12


If issues are confirmed, it would be good to create new tickets on
GitHub, not to forget about them (except if you are already looking at
that). Also, that's a shame we don't have any packetdrill tests
involving MP_FAIL: that would be great to cover all these cases with
packetdrill (hopefully MP_FAIL is fully supported there).

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