The mptcp protocol maintains an additional struct socket per connection,
mainly to be able to use easily tcp-level struct socket operations.

That lead to several ill side effects, beyond the quite unfortunate/
confusing field name
- active and passive sockets behavior is incosistent, as only active
  ones have not NULL msk->subflow, leading to different error handling
  (and different error code returned to the user-space) in several
  places.
- active sockets uses an unneeded, larger amount of memory
- passive sockets can't successfully go through accept()/disconnect()
  accept()

This series address all the above finally getting rid of the blamed
field. The first patch is a minor clean-up, in the next 11 patches
msk->subflow usage is sistematically removed from the mptcp protocol,
replacing it with direct msk->first usage, eventually introducing new
core helpers as needed.

The final patch finally dispose the field, and it's the only patch in
the series intened to produce functional changes.

v1 -> v2:
 - dropped first patch (already applied)
 - a bunch of typos and comments, the only patches with code changes
   are 7/13, 9/13, 13/13, see the individual changelog for the
   details.

Paolo Abeni (13):
  mptcp: avoid unneeded mptcp_token_destroy() calls
  mptcp: avoid additional __inet_stream_connect() call
  mptcp: avoid subflow socket usage in mptcp_get_port()
  net: factor out inet{,6}_bind_sk helpers
  mptcp: mptcp: avoid additional indirection in mptcp_bind()
  net: factor out __inet_listen_sk() helper
  mptcp: avoid additional indirection in mptcp_listen()
  mptcp: avoid additional indirection in mptcp_poll()
  mptcp: avoid unneeded indirection in mptcp_stream_accept()
  mptcp: avoid additional indirection in sockopt
  mptcp: avoid ssock usage in mptcp_pm_nl_create_listen_socket()
  mptcp: change the mpc check helper to return a sk
  mptcp: get rid of msk->subflow

 include/net/inet_common.h |   2 +
 include/net/ipv6.h        |   1 +
 net/ipv4/af_inet.c        |  46 ++++++----
 net/ipv6/af_inet6.c       |  10 ++-
 net/mptcp/pm_netlink.c    |  30 ++++---
 net/mptcp/protocol.c      | 177 +++++++++++++++++++-------------------
 net/mptcp/protocol.h      |  15 ++--
 net/mptcp/sockopt.c       |  65 +++++++-------
 8 files changed, 181 insertions(+), 165 deletions(-)

-- 
2.41.0

Hi Paolo, Mat,

On 14/07/2023 13:29, Paolo Abeni wrote:
> The mptcp protocol maintains an additional struct socket per connection,
> mainly to be able to use easily tcp-level struct socket operations.
> 
> That lead to several ill side effects, beyond the quite unfortunate/
> confusing field name
> - active and passive sockets behavior is incosistent, as only active
>   ones have not NULL msk->subflow, leading to different error handling
>   (and different error code returned to the user-space) in several
>   places.
> - active sockets uses an unneeded, larger amount of memory
> - passive sockets can't successfully go through accept()/disconnect()
>   accept()
> 
> This series address all the above finally getting rid of the blamed
> field. The first patch is a minor clean-up, in the next 11 patches
> msk->subflow usage is sistematically removed from the mptcp protocol,
> replacing it with direct msk->first usage, eventually introducing new
> core helpers as needed.
> 
> The final patch finally dispose the field, and it's the only patch in
> the series intened to produce functional changes.

Thank you for the patches and the reviews!

Now in our tree (feat. for net-next) with Mat's RvB tag:

New patches for t/upstream:
- 26b3767aaa72: mptcp: avoid unneeded mptcp_token_destroy() calls
- ae27baea9cbe: mptcp: avoid additional __inet_stream_connect() call
- c52207f85c8f: mptcp: avoid subflow socket usage in mptcp_get_port()
- a3beb4bf1bf4: net: factor out inet{,6}_bind_sk helpers
- f805626e3cf4: mptcp: mptcp: avoid additional indirection in mptcp_bind()
- 5bbcd109ff64: net: factor out __inet_listen_sk() helper
- 86211f9d790c: mptcp: avoid additional indirection in mptcp_listen()
- 4e7fe9d2245f: mptcp: avoid additional indirection in mptcp_poll()
- 36d4933b1c0a: mptcp: avoid unneeded indirection in mptcp_stream_accept()
- 6684c443b5a9: mptcp: avoid additional indirection in sockopt
- ac388877cff7: mptcp: avoid ssock usage in
mptcp_pm_nl_create_listen_socket()
- 3bdc46b9f3dd: mptcp: change the mpc check helper to return a sk
- 0b266479d09c: mptcp: get rid of msk->subflow
- Results: 87ca9f6d4f74..a989acf037d0 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230717T135639

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

On Fri, 14 Jul 2023, Paolo Abeni wrote:

> The mptcp protocol maintains an additional struct socket per connection,
> mainly to be able to use easily tcp-level struct socket operations.
>
> That lead to several ill side effects, beyond the quite unfortunate/
> confusing field name
> - active and passive sockets behavior is incosistent, as only active
>  ones have not NULL msk->subflow, leading to different error handling
>  (and different error code returned to the user-space) in several
>  places.
> - active sockets uses an unneeded, larger amount of memory
> - passive sockets can't successfully go through accept()/disconnect()
>  accept()
>
> This series address all the above finally getting rid of the blamed
> field. The first patch is a minor clean-up, in the next 11 patches
> msk->subflow usage is sistematically removed from the mptcp protocol,
> replacing it with direct msk->first usage, eventually introducing new
> core helpers as needed.
>
> The final patch finally dispose the field, and it's the only patch in
> the series intened to produce functional changes.
>
> v1 -> v2:
> - dropped first patch (already applied)
> - a bunch of typos and comments, the only patches with code changes
>   are 7/13, 9/13, 13/13, see the individual changelog for the
>   details.
>

Thanks Paolo, v2 looks good to me.

For the "mptcp: *" patches:

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

I'll ack the "net: *" patches separately.


- Mat


> Paolo Abeni (13):
>  mptcp: avoid unneeded mptcp_token_destroy() calls
>  mptcp: avoid additional __inet_stream_connect() call
>  mptcp: avoid subflow socket usage in mptcp_get_port()
>  net: factor out inet{,6}_bind_sk helpers
>  mptcp: mptcp: avoid additional indirection in mptcp_bind()
>  net: factor out __inet_listen_sk() helper
>  mptcp: avoid additional indirection in mptcp_listen()
>  mptcp: avoid additional indirection in mptcp_poll()
>  mptcp: avoid unneeded indirection in mptcp_stream_accept()
>  mptcp: avoid additional indirection in sockopt
>  mptcp: avoid ssock usage in mptcp_pm_nl_create_listen_socket()
>  mptcp: change the mpc check helper to return a sk
>  mptcp: get rid of msk->subflow
>
> include/net/inet_common.h |   2 +
> include/net/ipv6.h        |   1 +
> net/ipv4/af_inet.c        |  46 ++++++----
> net/ipv6/af_inet6.c       |  10 ++-
> net/mptcp/pm_netlink.c    |  30 ++++---
> net/mptcp/protocol.c      | 177 +++++++++++++++++++-------------------
> net/mptcp/protocol.h      |  15 ++--
> net/mptcp/sockopt.c       |  65 +++++++-------
> 8 files changed, 181 insertions(+), 165 deletions(-)
>
> -- 
> 2.41.0
>
>
>