Re: [PATCH 0/4] tcp: unify IPv4 and IPv6 l2 tap queues

[Stefano Brivio <sbrivio@redhat.com>]

On Fri,  6 Sep 2024 17:34:23 -0400
Jon Maloy <jmaloy@redhat.com> wrote:

> This should save us some memory and code.
> 
> Jon Maloy (4):
>   tcp: create unified struct for IPv4 and IPv6 header
>   tcp: update ip address in l2 tap queues on the fly
>   tcp: unify l2 TCPv4 and TCPv6 queues and structures
>   tcp: change prefix tcp4_ to tcp_ where applicable
> 
>  tcp.c     |  22 ++---
>  tcp_buf.c | 259 +++++++++++++++++-------------------------------------
>  tcp_buf.h |   3 +
>  3 files changed, 98 insertions(+), 186 deletions(-)

The saving in lines of code is nice (68 lines excluding comments), I'm
not quite sure about memory savings and overhead.

Memory-wise, if one IP version is disabled, we won't initialise any
related buffer, so in that case we won't save any memory with these
changes.

If both IPv4 and IPv6 are enabled, we'll halve the memory usage for
inbound payload buffers (about 8 MiB instead of 16 MiB):

  $ nm -td -P passt.avx2 | grep "tcp6_payload "
  tcp6_payload b 48607968 8388608

but that also halves the total amount of available buffers. If we have
occasional usage of one IP family, these changes decrease waste, but if
both IPv4 and IPv6 are used consistently, this simply means we're
cutting the amount of buffers in half. However, we could double
TCP_FRAMES_MEM and have, as a result, a more optimised management of
buffers.

But I'm not sure it makes sense considered the potential overhead.

That is, there's a reason why I originally added two separate sets of
buffers for IPv4 and IPv6: to enable pre-computations of headers, so
that we avoid populating the full headers at every received packet.
This series reverses that.

There's the possibility that improvements in data locality and lower
data cache usage outweigh that (as David mentioned in his comment to
1/4), but that should be demonstrated first.

By the way, I tested this series, the clang-tidy test fails (see
David's comment to 2/4) and the "TCP throughput over IPv6: guest to
host" test consistently hangs as we switch to the 65520 bytes MTU. It
looks like the iperf3 client fails to connect altogether, but I
didn't really investigate.

-- 
Stefano