This is the 2nd preparatory series for the TCP_NOTSENT_LOWAT support.
The latter will need tracking more state under the msk data lock

The mptcp locking schema is already quite complex. We need to clarify
and make consistent the lockless access already there or later changes
will be even harder to follow and understand.

This series goes through all the msk fields accessed in the RX and TX
path and makes the lockless annotation consistent with the in-use
locking schema.

As a bonus this should fix data races possibly found by fuzzers - even
if we haven't seen many such reports so far.

Patch 1/5 hints we could remove local_key and remote_key from the
subflow context, and always use the ones in the msk socket, possibly
reducing the context memory usage. That change is left over as a
possible follow-up.

Paolo Abeni (5):
  mptcp: annotate access for msk keys
  mptcp: annotate lockless access for the tx path
  mptcp: annotate lockless access for RX path fields.
  mptcp: annotate lockless access for token
  mptcp: annotate lockless accesses around read-mostly fields

 net/mptcp/options.c    | 20 ++++++++---------
 net/mptcp/pm.c         |  2 +-
 net/mptcp/pm_netlink.c | 10 ++++-----
 net/mptcp/protocol.c   | 51 +++++++++++++++++++++---------------------
 net/mptcp/protocol.h   |  8 ++++---
 net/mptcp/sockopt.c    |  2 +-
 net/mptcp/subflow.c    | 10 +++++----
 7 files changed, 54 insertions(+), 49 deletions(-)

-- 
2.43.0

Hi Paolo, Mat,

On 16/01/2024 19:16, Paolo Abeni wrote:
> This is the 2nd preparatory series for the TCP_NOTSENT_LOWAT support.
> The latter will need tracking more state under the msk data lock
> 
> The mptcp locking schema is already quite complex. We need to clarify
> and make consistent the lockless access already there or later changes
> will be even harder to follow and understand.
> 
> This series goes through all the msk fields accessed in the RX and TX
> path and makes the lockless annotation consistent with the in-use
> locking schema.
> 
> As a bonus this should fix data races possibly found by fuzzers - even
> if we haven't seen many such reports so far.
> 
> Patch 1/5 hints we could remove local_key and remote_key from the
> subflow context, and always use the ones in the msk socket, possibly
> reducing the context memory usage. That change is left over as a
> possible follow-up.
> 
> Paolo Abeni (5):
>   mptcp: annotate access for msk keys
>   mptcp: annotate lockless access for the tx path
>   mptcp: annotate lockless access for RX path fields.
>   mptcp: annotate lockless access for token
>   mptcp: annotate lockless accesses around read-mostly fields

Thank you for the patches and the reviews!

Now in our tree (feat. for net-next) without a typo spotted by
checkpatch.pl --codespell.

New patches for t/upstream:
- 39c9cb5e8865: mptcp: annotate access for msk keys
- 483c63a2857c: mptcp: annotate lockless access for the tx path
- 817a9559ee7c: mptcp: annotate lockless access for RX path fields
- 5cda40c71dad: mptcp: annotate lockless access for token
- ce87356496c6: mptcp: annotate lockless accesses around read-mostly fields
- Results: 81c0f68b0316..2710fa51b8b5 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240130T121817

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

On Tue, 16 Jan 2024, Paolo Abeni wrote:

> This is the 2nd preparatory series for the TCP_NOTSENT_LOWAT support.
> The latter will need tracking more state under the msk data lock
>
> The mptcp locking schema is already quite complex. We need to clarify
> and make consistent the lockless access already there or later changes
> will be even harder to follow and understand.
>
> This series goes through all the msk fields accessed in the RX and TX
> path and makes the lockless annotation consistent with the in-use
> locking schema.
>
> As a bonus this should fix data races possibly found by fuzzers - even
> if we haven't seen many such reports so far.
>
> Patch 1/5 hints we could remove local_key and remote_key from the
> subflow context, and always use the ones in the msk socket, possibly
> reducing the context memory usage. That change is left over as a
> possible follow-up.
>

Thanks for the reply to my question on patch 2, series LGTM:

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

> Paolo Abeni (5):
>  mptcp: annotate access for msk keys
>  mptcp: annotate lockless access for the tx path
>  mptcp: annotate lockless access for RX path fields.
>  mptcp: annotate lockless access for token
>  mptcp: annotate lockless accesses around read-mostly fields
>
> net/mptcp/options.c    | 20 ++++++++---------
> net/mptcp/pm.c         |  2 +-
> net/mptcp/pm_netlink.c | 10 ++++-----
> net/mptcp/protocol.c   | 51 +++++++++++++++++++++---------------------
> net/mptcp/protocol.h   |  8 ++++---
> net/mptcp/sockopt.c    |  2 +-
> net/mptcp/subflow.c    | 10 +++++----
> 7 files changed, 54 insertions(+), 49 deletions(-)
>
> -- 
> 2.43.0
>
>
>