[v2] mptcp: just another xmit path refactor

[PATCH v2 mptcp-next 0/4] mptcp: just another xmit path refactor

Posted by Paolo Abeni 4 years ago

Eric want to revert the tcp_tx_skb_cache. MPTCP relies on it
for skb allocation. Before the revert we need to refactor our
xmit path.

Patch 1 exposes some needed helpers (endorsed by Eric)
Patch 2 contains the nasty new code
Patch 3 revert some core TCP changes not needed anymore
and patch 4 is Eric's revert.

This iteration should hopefully solve the OoB issues I observed with
the previous attempt, changes documented in patch 2.

Eric Dumazet (1):
  tcp: remove sk_{tr}x_skb_cache

Paolo Abeni (3):
  tcp: expose the tcp_mark_push() and skb_entail() helpers
  mptcp: stop relaying on tcp_tx_skb_cache.
  Partially revert "tcp: factor out tcp_build_frag()"

 Documentation/networking/ip-sysctl.rst |   8 --
 include/net/sock.h                     |  19 ----
 include/net/tcp.h                      |   4 +-
 net/ipv4/af_inet.c                     |   4 -
 net/ipv4/sysctl_net_ipv4.c             |  12 --
 net/ipv4/tcp.c                         | 147 +++++++++----------------
 net/ipv4/tcp_ipv4.c                    |   6 -
 net/ipv6/tcp_ipv6.c                    |   6 -
 net/mptcp/protocol.c                   | 132 +++++++++++++---------
 9 files changed, 132 insertions(+), 206 deletions(-)

-- 
2.26.3

Re: [PATCH v2 mptcp-next 0/4] mptcp: just another xmit path refactor

Posted by Mat Martineau 4 years ago

On Thu, 2 Sep 2021, Paolo Abeni wrote:

> Eric want to revert the tcp_tx_skb_cache. MPTCP relies on it
> for skb allocation. Before the revert we need to refactor our
> xmit path.
>
> Patch 1 exposes some needed helpers (endorsed by Eric)
> Patch 2 contains the nasty new code
> Patch 3 revert some core TCP changes not needed anymore
> and patch 4 is Eric's revert.
>
> This iteration should hopefully solve the OoB issues I observed with
> the previous attempt, changes documented in patch 2.
>
> Eric Dumazet (1):
>  tcp: remove sk_{tr}x_skb_cache
>
> Paolo Abeni (3):
>  tcp: expose the tcp_mark_push() and skb_entail() helpers
>  mptcp: stop relaying on tcp_tx_skb_cache.
>  Partially revert "tcp: factor out tcp_build_frag()"
>
> Documentation/networking/ip-sysctl.rst |   8 --
> include/net/sock.h                     |  19 ----
> include/net/tcp.h                      |   4 +-
> net/ipv4/af_inet.c                     |   4 -
> net/ipv4/sysctl_net_ipv4.c             |  12 --
> net/ipv4/tcp.c                         | 147 +++++++++----------------
> net/ipv4/tcp_ipv4.c                    |   6 -
> net/ipv6/tcp_ipv6.c                    |   6 -
> net/mptcp/protocol.c                   | 132 +++++++++++++---------
> 9 files changed, 132 insertions(+), 206 deletions(-)
>
> -- 
> 2.26.3

It's ok to apply the series to the export branch. Before we upstream it, 
can Paolo send a squash-to patch for #2 that removes the 
WARN_ON_ONCE(!mpext)?

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>



--
Mat Martineau
Intel

Re: [PATCH v2 mptcp-next 0/4] mptcp: just another xmit path refactor

Posted by Matthieu Baerts 4 years ago

Hi Paolo, Mat,

On 03/09/2021 22:11, Mat Martineau wrote:
> On Thu, 2 Sep 2021, Paolo Abeni wrote:
> 
>> Eric want to revert the tcp_tx_skb_cache. MPTCP relies on it
>> for skb allocation. Before the revert we need to refactor our
>> xmit path.
>>
>> Patch 1 exposes some needed helpers (endorsed by Eric)
>> Patch 2 contains the nasty new code
>> Patch 3 revert some core TCP changes not needed anymore
>> and patch 4 is Eric's revert.
>>
>> This iteration should hopefully solve the OoB issues I observed with
>> the previous attempt, changes documented in patch 2.
>>
>> Eric Dumazet (1):
>>  tcp: remove sk_{tr}x_skb_cache
>>
>> Paolo Abeni (3):
>>  tcp: expose the tcp_mark_push() and skb_entail() helpers
>>  mptcp: stop relaying on tcp_tx_skb_cache.
>>  Partially revert "tcp: factor out tcp_build_frag()"
>>
>> Documentation/networking/ip-sysctl.rst |   8 --
>> include/net/sock.h                     |  19 ----
>> include/net/tcp.h                      |   4 +-
>> net/ipv4/af_inet.c                     |   4 -
>> net/ipv4/sysctl_net_ipv4.c             |  12 --
>> net/ipv4/tcp.c                         | 147 +++++++++----------------
>> net/ipv4/tcp_ipv4.c                    |   6 -
>> net/ipv6/tcp_ipv6.c                    |   6 -
>> net/mptcp/protocol.c                   | 132 +++++++++++++---------
>> 9 files changed, 132 insertions(+), 206 deletions(-)
>>
>> -- 
>> 2.26.3

Thank you for the patches and the reviews

> It's ok to apply the series to the export branch. Before we upstream it,
> can Paolo send a squash-to patch for #2 that removes the
> WARN_ON_ONCE(!mpext)?
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

These patches are now in our tree -- features for net-next -- with:

- Mat's RvB tag on patch 2/4
- Mat's Ack on patches 1/4 and 3-4/4
- Paolo's Ack on patch 4/4
- fixed a typo and removed dot in the title of patch 2/4
- a small checkpatch fix in patch 2/4: Unnecessary parentheses
- without checkpatch fixes in patch 3/4: bring back old and "non
compliant" code in TCP
- (I hope it is OK) reduce commit SHA lengths in patch 4/4's message to
fix checkpatch errors.

Commits:

- ef9706ecd17e: tcp: expose the tcp_mark_push() and skb_entail() helpers

- 0ed0c4ad3483: mptcp: stop relying on tcp_tx_skb_cache

- ee51bf92cdcc: Partially revert "tcp: factor out tcp_build_frag()"

- ead8eb1fc223: tcp: remove sk_{tr}x_skb_cache
- Results: 7755d2ada382..d692ad305b2a

WARN_ON_ONCE(!mpext):

- 618f6578f614: mptcp: remove no longer needed check
- Results: d692ad305b2a..7a2d18a25e65

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210904T080009
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210904T080009

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