[mptcp-net] mptcp: pm: do not remove closing subflows

Commit Message

Matthieu Baerts (NGI0) Aug. 30, 2024, 3:36 p.m. UTC

In a previous fix, the in-kernel path-manager has been modified not to
retrigger the removal of a subflow if it was already closed, e.g. when
the initial subflow is removed, but kept in the subflows list.

To be complete, this fix should also skip the subflows that are in any
closing state: mptcp_close_ssk() will initiate the closure, but the
switch to the TCP_CLOSE state depends on the other peer.

Suggested-by: Paolo Abeni <[e-mail]>
Fixes: Fixes: 58e1b66b4e4b ("mptcp: pm: do not remove already closed subflows")
Signed-off-by: Matthieu Baerts (NGI0) <[e-mail]>
---
Notes:
- I'm not sure looking for TCPF_CLOSING is really needed in this case.
  I can remove it if it is not needed.
---
 net/mptcp/pm_netlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


---
base-commit: 6b336f0e1c2b1f649f86fde82abeae5b87ce09b4
change-id: 20240830-mptcp-pm-minor-fixes-3578d0338438

Best regards,

Comments

MPTCP CI Sept. 2, 2024, 5:43 p.m. UTC | #1

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/10670883525

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/5cf12ffb55bd
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=885285


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)

Matthieu Baerts (NGI0) Oct. 7, 2024, 9:19 a.m. UTC | #2

Hello,

On 30/08/2024 17:36, Matthieu Baerts (NGI0) wrote:
> In a previous fix, the in-kernel path-manager has been modified not to
> retrigger the removal of a subflow if it was already closed, e.g. when
> the initial subflow is removed, but kept in the subflows list.
> 
> To be complete, this fix should also skip the subflows that are in any
> closing state: mptcp_close_ssk() will initiate the closure, but the
> switch to the TCP_CLOSE state depends on the other peer.

As discussed at the last meeting, the patch looks OK. I then just
applied it with Paolo's Acked-by:

New patches for t/upstream-net and t/upstream:
- 0986f9be4a5c: mptcp: pm: do not remove closing subflows
- Results: 98b1e473ae39..400d24177d84 (export-net)
- Results: 48a08655247c..2ce52e0f26d3 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/7ff625c9345c4313b66dba10977bbfa1f5350a3f/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/f024f47bc584b29fa1f211c82317c41ef7ac2d01/checks

> Suggested-by: Paolo Abeni <[e-mail]>
> Fixes: Fixes: 58e1b66b4e4b ("mptcp: pm: do not remove already closed subflows")

... and I removed the duplicated 'Fixes' word.

Cheers,
Matt

Patch

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index a006ce39d41a..f52718e1c30c 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -862,7 +862,8 @@  static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk,
 			int how = RCV_SHUTDOWN | SEND_SHUTDOWN;
 			u8 id = subflow_get_local_id(subflow);
 
-			if (inet_sk_state_load(ssk) == TCP_CLOSE)
+			if ((1 << inet_sk_state_load(ssk)) &
+			    (TCPF_FIN_WAIT1 | TCPF_FIN_WAIT2 | TCPF_CLOSING | TCPF_CLOSE))
 				continue;
 			if (rm_type == MPTCP_MIB_RMADDR && remote_id != rm_id)
 				continue;