On Fri, 6 Sep 2024 17:34:23 -0400 Jon Maloy <jmaloy(a)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