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

[Matthieu Baerts <matttbe@kernel.org>]

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.