diff mbox series

[v2,mptcp-net] mptcp: relax check on MPC passive fallback

Message ID d97d56012abb6e277c4ffc5042ff2f414dbe7959.1705343249.git.pabeni@redhat.com (mailing list archive)
State Accepted, archived
Commit 8560e02c1efc4399caed65020035f58677740625
Delegated to: Matthieu Baerts
Headers show
Series [v2,mptcp-net] mptcp: relax check on MPC passive fallback | expand

Checks

Context Check Description
matttbe/build success Build and static analysis OK
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug__except_selftest_mptcp_join_ warning Unstable: 1 failed test(s): packetdrill_sockopts
matttbe/KVM_Validation__debug__only_selftest_mptcp_join_ warning Unstable: 1 failed test(s): selftest_mptcp_join

Commit Message

Paolo Abeni Jan. 15, 2024, 6:31 p.m. UTC
While testing the blamed commit below, I was able to miss (!)
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.

Fixes: 724b00c12957 ("mptcp: refine opt_mp_capable determination")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
 - keep it simple don't bother handling DSS OoO pkts (matttbe)
---
 net/mptcp/subflow.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Matthieu Baerts Jan. 15, 2024, 6:35 p.m. UTC | #1
Hi Paolo,

On 15/01/2024 19:31, Paolo Abeni wrote:
> While testing the blamed commit below, I was able to miss (!)
> 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.
> 
> Fixes: 724b00c12957 ("mptcp: refine opt_mp_capable determination")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v1 -> v2:
>  - keep it simple don't bother handling DSS OoO pkts (matttbe)

Thank you for the v2, looks good to me (if the CI is also OK with it)

Reviewed-by: Matthieu Baerts <matttbe@kernel.org>

Cheers,
Matt
MPTCP CI Jan. 15, 2024, 7:34 p.m. UTC | #2
Hi Paolo,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7532921194

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/cff2320d6411


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)
MPTCP CI Jan. 15, 2024, 7:44 p.m. UTC | #3
Hi Paolo,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Unstable: 1 failed test(s): packetdrill_sockopts 
Matthieu Baerts Jan. 16, 2024, 5:36 p.m. UTC | #4
Hi Paolo,

On 15/01/2024 19:31, Paolo Abeni wrote:
> While testing the blamed commit below, I was able to miss (!)
> 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.
> 
> Fixes: 724b00c12957 ("mptcp: refine opt_mp_capable determination")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v1 -> v2:
>  - keep it simple don't bother handling DSS OoO pkts (matttbe)

Thank you for the v2!

I just applied it in our tree (fix for -net):

New patches for t/upstream-net and t/upstream:
- 8560e02c1efc: mptcp: relax check on MPC passive fallback
- Results: 8cd75bfd0036..dd080a859960 (export-net)
- Results: 25ad5c88ddc7..2d5218531f82 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20240116T172013
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240116T172013

Cheers,
Matt
diff mbox series

Patch

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 1117d1e84274..0dcb721c89d1 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)))
 			fallback = true;
 
 	} else if (subflow_req->mp_join) {