[PATCH mptcp-net v2 0/4] mptcp: fix duplicate subflow creation

Paolo Abeni posted 4 patches 5 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/cover.1707418323.git.pabeni@redhat.com
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(-)
[PATCH mptcp-net v2 0/4] mptcp: fix duplicate subflow creation
Posted by Paolo Abeni 5 months, 1 week ago
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
Re: [PATCH mptcp-net v2 0/4] mptcp: fix duplicate subflow creation
Posted by Matthieu Baerts 5 months, 1 week ago
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.
Re: [PATCH mptcp-net v2 0/4] mptcp: fix duplicate subflow creation
Posted by Mat Martineau 5 months, 1 week ago
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
>
>
>