As reported by Mat, the in kernel PM can, in some edge scenarios,
unexpectedly create multiple subflows with the same local and remote
address.

The real fix is implemented by patch 4/4 with some more accurate check
at subflow creation time.

Patches 1-3 are roughly optional pre-requisities, added to avoid
introducing more data-races with the actual fix. Patch 1/4 is a bit
debatable, as it changes the existing ULP API, but I could not find a
better solution and there is some similar prior art:
commit 0df48c26d841 ("tcp: add tcpi_bytes_acked to tcp_info")

This address feedback from Mat on v1, see the patches changelog for 
the details (no changes in patch 1/4).

Paolo Abeni (4):
  mptcp: fix lockless access in subflow ULP diag
  mptcp: fix data races on local_id
  mptcp: fix data races on remote_id
  mptcp: fix duplicate subflow creation

 include/net/tcp.h        |  2 +-
 net/mptcp/diag.c         |  8 +++++--
 net/mptcp/pm_netlink.c   | 45 +++++++++++++++++++++-------------------
 net/mptcp/pm_userspace.c |  2 +-
 net/mptcp/protocol.c     |  2 +-
 net/mptcp/protocol.h     | 15 +++++++++++---
 net/mptcp/subflow.c      | 15 +++++++-------
 net/tls/tls_main.c       |  2 +-
 8 files changed, 54 insertions(+), 37 deletions(-)

-- 
2.43.0

Hi Paolo, Mat,

On 08/02/2024 21:42, Paolo Abeni wrote:
> As reported by Mat, the in kernel PM can, in some edge scenarios,
> unexpectedly create multiple subflows with the same local and remote
> address.
> 
> The real fix is implemented by patch 4/4 with some more accurate check
> at subflow creation time.
> 
> Patches 1-3 are roughly optional pre-requisities, added to avoid
> introducing more data-races with the actual fix. Patch 1/4 is a bit
> debatable, as it changes the existing ULP API, but I could not find a
> better solution and there is some similar prior art:
> commit 0df48c26d841 ("tcp: add tcpi_bytes_acked to tcp_info")
> 
> This address feedback from Mat on v1, see the patches changelog for 
> the details (no changes in patch 1/4).

Thank you for the modifications and the reviews!

Now in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- ce4271cba41b: mptcp: fix lockless access in subflow ULP diag
- bb35405f2e28: mptcp: fix data races on local_id
- ed0e0e7b2325: mptcp: fix data races on remote_id
- 22e3b19337f7: mptcp: fix duplicate subflow creation
- Results: bef0b46af378..8abab82d59f2 (export-net)
- Results: 78a4d8e40bf5..0c8d1475f726 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20240209T141835
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240209T141835

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

On Thu, 8 Feb 2024, Paolo Abeni wrote:

> As reported by Mat, the in kernel PM can, in some edge scenarios,
> unexpectedly create multiple subflows with the same local and remote
> address.
>
> The real fix is implemented by patch 4/4 with some more accurate check
> at subflow creation time.
>
> Patches 1-3 are roughly optional pre-requisities, added to avoid
> introducing more data-races with the actual fix. Patch 1/4 is a bit
> debatable, as it changes the existing ULP API, but I could not find a
> better solution and there is some similar prior art:
> commit 0df48c26d841 ("tcp: add tcpi_bytes_acked to tcp_info")
>
> This address feedback from Mat on v1, see the patches changelog for
> the details (no changes in patch 1/4).
>
> Paolo Abeni (4):
>  mptcp: fix lockless access in subflow ULP diag
>  mptcp: fix data races on local_id
>  mptcp: fix data races on remote_id
>  mptcp: fix duplicate subflow creation
>

Hi Paolo -

v2 LGTM, thanks:

Reviewed-by: Mat Martineau <martineau@kernel.org>

> include/net/tcp.h        |  2 +-
> net/mptcp/diag.c         |  8 +++++--
> net/mptcp/pm_netlink.c   | 45 +++++++++++++++++++++-------------------
> net/mptcp/pm_userspace.c |  2 +-
> net/mptcp/protocol.c     |  2 +-
> net/mptcp/protocol.h     | 15 +++++++++++---
> net/mptcp/subflow.c      | 15 +++++++-------
> net/tls/tls_main.c       |  2 +-
> 8 files changed, 54 insertions(+), 37 deletions(-)
>
> -- 
> 2.43.0
>
>
>