This is a batch of changes I had sitting in my local tree for a while.
Why another refactor you may ask? Two main resons:

- currently the mptcp RX path introduces quite a bit of 'exceptional'
 accounting/locking processing WRT to plain TCP, adding up to the
 implementation complexity in a misurable way
- the performance gap WRT plain TCP for single subflow connections is
 quite measurable.

The present refactor addresses both the above items: most of the
additional complexity is dropped, and single stream performances
increase measurably - from 55Gbps to 71Gbps in my loopback test. As a
reference, plain TCP is around 84Gps on the same host.

The above comes to a price: the patch are invasive, even in subtle ways:
the chance of destabilizing the implementation is real (ence the
additional, intentional '-next' into the subj).

In any case keeping the patch hidden for longer was not going to do any
good, so here we are.

Paolo Abeni (3):
  mptcp: consolidate subflow cleanup
  mptcp: move the whole rx path under msk socket lock protection
  mptcp: cleanup mem accounting.

 net/mptcp/fastopen.c |   4 +-
 net/mptcp/protocol.c | 202 +++++++++----------------------------------
 net/mptcp/protocol.h |   6 +-
 net/mptcp/subflow.c  |  33 +++----
 4 files changed, 66 insertions(+), 179 deletions(-)

-- 
2.45.2

Hi Paolo,

On 29/11/2024 18:45, Paolo Abeni wrote:
> This is a batch of changes I had sitting in my local tree for a while.
> Why another refactor you may ask? Two main resons:
> 
> - currently the mptcp RX path introduces quite a bit of 'exceptional'
>  accounting/locking processing WRT to plain TCP, adding up to the
>  implementation complexity in a misurable way
> - the performance gap WRT plain TCP for single subflow connections is
>  quite measurable.
> 
> The present refactor addresses both the above items: most of the
> additional complexity is dropped, and single stream performances
> increase measurably - from 55Gbps to 71Gbps in my loopback test. As a
> reference, plain TCP is around 84Gps on the same host.

Nice clean-up, with an excellent improvement!

> The above comes to a price: the patch are invasive, even in subtle ways:
> the chance of destabilizing the implementation is real (ence the
> additional, intentional '-next' into the subj).

Because we are only at the beginning of a new cycle, do we need to have
this in '-next'? If we can have syzkaller validating this for a couple
of weeks, would it not be OK to send that to netdev, to have it in
linux-next for a few weeks?

> In any case keeping the patch hidden for longer was not going to do any
> good, so here we are.
> 
> Paolo Abeni (3):
>   mptcp: consolidate subflow cleanup
>   mptcp: move the whole rx path under msk socket lock protection
>   mptcp: cleanup mem accounting.

The series look good to me: the modifications seem to make sense, but I
don't know well this part that we usually avoid modifying :)

@Mat: do you mind having a look please?

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

Hi Paolo,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Unstable: 1 failed test(s): selftest_mptcp_connect 🔴
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/12088981326

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/818e1f7b5c07
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=913372


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

On 11/29/24 19:52, MPTCP CI wrote:
> Thank you for your modifications, that's great!
> 
> Our CI did some validations and here is its report:
> 
> - KVM Validation: normal: Success! ✅
> - KVM Validation: debug: Unstable: 1 failed test(s): selftest_mptcp_connect 🔴

This is somewhat 'funny'; I can't reproduce it locally, and other tests
do the connection successfully. I guess there is some race due to the
slow env?!? Is virtme still running without KVM ?

Thanks,

Paolo