While testing the blamed commit below, I was able to miss noticing(!)
packetdrill failures in the fastopen test-cases.

On passive fastopen the child socket is created by incoming TCP MPC syn,
allow for both MPC_SYN and MPC_ACK header.

While at it, additionally allow for real OoO packets, as the RFC
mandates.

Fixes: 724b00c12957 ("mptcp: refine opt_mp_capable determination")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/subflow.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 1117d1e84274..90984b053adc 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -783,7 +783,8 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 		 * options.
 		 */
 		mptcp_get_options(skb, &mp_opt);
-		if (!(mp_opt.suboptions & OPTION_MPTCP_MPC_ACK))
+		if (!(mp_opt.suboptions &
+		      (OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_ACK | OPTION_MPTCP_DSS)))
 			fallback = true;
 
 	} else if (subflow_req->mp_join) {
-- 
2.43.0

Hi Paolo,

On 15/01/2024 15:56, Paolo Abeni wrote:
> While testing the blamed commit below, I was able to miss noticing(!)
> packetdrill failures in the fastopen test-cases.
> 
> On passive fastopen the child socket is created by incoming TCP MPC syn,
> allow for both MPC_SYN and MPC_ACK header.

Thank you for the fix!

> While at it, additionally allow for real OoO packets, as the RFC
> mandates.

Should this be split into another patch? (it is not a fix for the commit
mentioned below)

Also, the comment just above the code you modified seems to suggest we
should do a fallback if some DSS are detected, not the opposite, no?

  /* (...)
   * Even OoO DSS packets coming legitly after dropped or
   * reordered MPC will cause fallback, but we don't have other
   * options.
   */

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

On Mon, 2024-01-15 at 16:52 +0100, Matthieu Baerts wrote:
> Hi Paolo,
> 
> On 15/01/2024 15:56, Paolo Abeni wrote:
> > While testing the blamed commit below, I was able to miss noticing(!)
> > packetdrill failures in the fastopen test-cases.
> > 
> > On passive fastopen the child socket is created by incoming TCP MPC syn,
> > allow for both MPC_SYN and MPC_ACK header.
> 
> Thank you for the fix!
> 
> > While at it, additionally allow for real OoO packets, as the RFC
> > mandates.
> 
> Should this be split into another patch? (it is not a fix for the commit
> mentioned below)
> 
> Also, the comment just above the code you modified seems to suggest we
> should do a fallback if some DSS are detected, not the opposite, no?
> 
>   /* (...)
>    * Even OoO DSS packets coming legitly after dropped or
>    * reordered MPC will cause fallback, but we don't have other
>    * options.
>    */

Oops, I forgot to update the comment. 

The point is that we should be able to accept OoO DSS packets, and
switch to fully established when receiving the MPC_ACK, but I agree you
are right, that much of change is dangerous, let's just accept SYN &&
ACK and ev. handle better the thing later

I'll send a v2.

Cheers,

Paolo