Re: [PATCH export v3 2/4] mptcp: reject joins after fallback in mptcp_is_fully_established

Hi Gang,

On 14/08/2026 05:02, [email protected] wrote:
> August 14, 2026 at 1:42 AM, "Matthieu Baerts" <[email protected] mailto:[email protected]?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:
>>
>> In addition, in the case we are trying to fix here, the MP_FAIL is
>> received before switching to fully established: the RFC doesn't explain
>> this particular case, but it sounds like there is no need to send the
>> infinite mapping, and a fallback can be done directly *I think*. The
>> thing is this situation shouldn't happen: we are not supposed to receive
>> an MP_FAIL at that stage, no data has been exchanged at the MPTCP level,
>> so let's do the minimal -- i.e. just a fallback -- to avoid issues and
>> reduce the complexity, no?
>>
> 
> Hi Matt, 
> 
> Thanks for your explanation. I have a quick thought:
> 
> Can we check the 'subflow->send_mp_fail' in 'subflow_finish_connect'? like:
> 
> '''
> ...
>  543         if (subflow->request_mptcp) {
>  544                 if (!(mp_opt.suboptions & OPTION_MPTCP_MPC_SYNACK)) {
>  545                         if (!mptcp_try_fallback(sk,
>  546                                                 MPTCP_MIB_MPCAPABLEACTIVEFALLBACK)) {
>  547                                 MPTCP_INC_STATS(sock_net(sk),
>  548                                                 MPTCP_MIB_FALLBACKFAILED);
>  549                                 goto do_reset;
>  550                         }
>  551 
>  552                         goto fallback;
>  553                 }
>              if (subflow->send_mp_fail)
>                              goto fallback;
>  554 
> ...
> '''

I don't know: it doesn't feel right to check this field there. Sending
the MP_FAIL reply and the infinite mapping in the same ACK, as you
suggested, seems better.

> Or, check msk->fully_established in pm_fail_received like this:
> '''
> @@ -901,6 +898,12 @@  void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
>  		subflow->send_mp_fail = 1;
>  		subflow->send_infinite_map = 1;
>  		tcp_send_ack(sk);
> +
> +		if (!msk->fully_established && !mptcp_try_fallback(sk, MPTCP_MIB_MPFAILFALLBACK)) {
> +			MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_FALLBACKFAILED);
> +			mptcp_subflow_reset(sk);
> +		}
> ''' 

Paolo noticed (in another email from this thread) this will not cover
the case we are trying to fix as we were already in fully_established.

But maybe we could try to fallback if that's possible, and if checksum
was not being used? I didn't check if it would be a good solution.

> Because the MP_FAIL specified in the RFC does not seem to actively trigger a fallback
> on its own; instead, it is triggered via the infinite mapping. If we modify the
> behavior of MP_FAIL in the regular path, I think it might be over-design and could
> introduce additional risks. What do you think?

Yes, but here, I'm only proposing doing a fallback for the cases not
covered by the RFC, i.e. single subflow, no checksum.

I think the simplest solution should be picked for this use-case that
shouldn't happen. Bounding MP_FAIL reply and infinite mapping in the
same ack seems interesting, and not just for this case, but it might be
"hacky" as you don't have an mpext in this case, no? Maybe a fallback is
"cleaner"?

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