mbox series

[mptcp-net,0/4] mptcp: locking cleanup

Message ID cover.1705331716.git.pabeni@redhat.com (mailing list archive)
Headers show
Series mptcp: locking cleanup | expand

Message

Paolo Abeni Jan. 15, 2024, 3:16 p.m. UTC
This is a preparatory work for the TCP_NOTSENT_LOWAT support. The latter
will need tracking more state under the msk data lock.

Since the locking msk locking schema is already quite complex, do a long
awaited clean-up step, moving several confusing lockless initialization
under the relevant locks.

Note that patches 2-4 carry fixes tag, and could target the net tree,
but AFACIS no real race could really happen even prior to such patches
as the MPTCP-level state machine implicitly ensure proper serialization
of the write accesses, even lacking explicit lock.

Patch 1 has no fixes, but still is logically tied to the other patches.
Possibly we could target net-next for the whole series.

Paolo Abeni (4):
  mptcp: drop the push_pending field
  mptcp: fix rcv space initialization
  mptcp: fix more tx path fields initialization
  mptcp: corner case locking for rx path fields initialization

 net/mptcp/fastopen.c |  6 ++--
 net/mptcp/options.c  |  9 +++---
 net/mptcp/protocol.c | 31 ++++++++++---------
 net/mptcp/protocol.h | 13 ++++----
 net/mptcp/subflow.c  | 71 +++++++++++++++++++++++++++-----------------
 5 files changed, 75 insertions(+), 55 deletions(-)

Comments

Mat Martineau Jan. 17, 2024, 2:32 a.m. UTC | #1
On Mon, 15 Jan 2024, Paolo Abeni wrote:

> This is a preparatory work for the TCP_NOTSENT_LOWAT support. The latter
> will need tracking more state under the msk data lock.
>
> Since the locking msk locking schema is already quite complex, do a long
> awaited clean-up step, moving several confusing lockless initialization
> under the relevant locks.
>
> Note that patches 2-4 carry fixes tag, and could target the net tree,
> but AFACIS no real race could really happen even prior to such patches
> as the MPTCP-level state machine implicitly ensure proper serialization
> of the write accesses, even lacking explicit lock.
>
> Patch 1 has no fixes, but still is logically tied to the other patches.
> Possibly we could target net-next for the whole series.
>
> Paolo Abeni (4):
>  mptcp: drop the push_pending field
>  mptcp: fix rcv space initialization
>  mptcp: fix more tx path fields initialization
>  mptcp: corner case locking for rx path fields initialization
>

Hi Paolo -

The series LGTM, assuming the CI failure is the issue fixed by "mptcp: 
relax check on MPC passive fallback":

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

> net/mptcp/fastopen.c |  6 ++--
> net/mptcp/options.c  |  9 +++---
> net/mptcp/protocol.c | 31 ++++++++++---------
> net/mptcp/protocol.h | 13 ++++----
> net/mptcp/subflow.c  | 71 +++++++++++++++++++++++++++-----------------
> 5 files changed, 75 insertions(+), 55 deletions(-)
>
> -- 
> 2.43.0
>
>
>
Paolo Abeni Jan. 17, 2024, 10:30 a.m. UTC | #2
On Tue, 2024-01-16 at 18:32 -0800, Mat Martineau wrote:
> On Mon, 15 Jan 2024, Paolo Abeni wrote:
> 
> > This is a preparatory work for the TCP_NOTSENT_LOWAT support. The latter
> > will need tracking more state under the msk data lock.
> > 
> > Since the locking msk locking schema is already quite complex, do a long
> > awaited clean-up step, moving several confusing lockless initialization
> > under the relevant locks.
> > 
> > Note that patches 2-4 carry fixes tag, and could target the net tree,
> > but AFACIS no real race could really happen even prior to such patches
> > as the MPTCP-level state machine implicitly ensure proper serialization
> > of the write accesses, even lacking explicit lock.
> > 
> > Patch 1 has no fixes, but still is logically tied to the other patches.
> > Possibly we could target net-next for the whole series.
> > 
> > Paolo Abeni (4):
> >  mptcp: drop the push_pending field
> >  mptcp: fix rcv space initialization
> >  mptcp: fix more tx path fields initialization
> >  mptcp: corner case locking for rx path fields initialization
> > 
> 
> Hi Paolo -
> 
> The series LGTM, assuming the CI failure is the issue fixed by "mptcp: 
> relax check on MPC passive fallback":

AFAIK, they are: I tested locally this series on top of the fixes,
without any problem.

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

Thanks!

Paolo
Matthieu Baerts (NGI0) Jan. 17, 2024, 11:58 a.m. UTC | #3
Hi Paolo, Mat,

On 15/01/2024 16:16, Paolo Abeni wrote:
> This is a preparatory work for the TCP_NOTSENT_LOWAT support. The latter
> will need tracking more state under the msk data lock.
> 
> Since the locking msk locking schema is already quite complex, do a long
> awaited clean-up step, moving several confusing lockless initialization
> under the relevant locks.
> 
> Note that patches 2-4 carry fixes tag, and could target the net tree,
> but AFACIS no real race could really happen even prior to such patches
> as the MPTCP-level state machine implicitly ensure proper serialization
> of the write accesses, even lacking explicit lock.
> 
> Patch 1 has no fixes, but still is logically tied to the other patches.
> Possibly we could target net-next for the whole series.

Thank you for the patches and the review!

Now in our tree (fixes for -net), without a typo spotted by
"checkpatch.pl --codespell" (and Vim's "set spell spelllang=en_gb")

New patches for t/upstream-net and t/upstream:
- c6d26639b0a9: mptcp: drop the push_pending field
- 2541670ad929: mptcp: fix rcv space initialization
- a6f5bfc5abf7: mptcp: fix more tx path fields initialization
- 7e6e9b2c5e9d: mptcp: corner case locking for rx path fields initialization
- Results: dd080a859960..50a3b99c60bd (export-net)
- Results: 2d5218531f82..387cadd951df (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20240117T115551
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240117T115551

Cheers,
Matt