[PATCH 0/7] TCP buffer handling cleanups (including vhost)
Here's a current version of my IOV tail and some cleanups to TCP buffer handling based on it. Now rebased on top of v14 of the vhost-user patches. This was aimed at sharing more code between the "buffer" and vhost-user paths, but that turned out to be trickier than I anticipated, so it hasn't really been accomplished. Nonetheless I think these are reasonable cleanups on their own merits, and may yet make sharing some more code between the paths easier in future. David Gibson (7): iov: iov tail helpers iov, checksum: Replace csum_iov() with csum_iov_tail() tcp: Pass TCP header and payload separately to tcp_update_check_tcp[46]() tcp: Pass TCP header and payload separately to tcp_fill_headers[46]() tcp: Merge tcp_update_check_tcp[46]() tcp: Merge tcp_fill_headers[46]() with each other tcp_vu: Remove unnecessary tcp_vu_update_check() function checksum.c | 58 +++++-------- checksum.h | 8 +- iov.c | 91 ++++++++++++++++++++ iov.h | 76 ++++++++++++++++ tap.c | 6 +- tcp.c | 229 +++++++++++++------------------------------------ tcp_buf.c | 33 ++++--- tcp_internal.h | 21 ++--- tcp_vu.c | 120 ++++++++++---------------- udp.c | 7 +- udp_vu.c | 9 +- 11 files changed, 338 insertions(+), 320 deletions(-) -- 2.47.0
In the vhost-user code we have a number of places where we need to locate
a particular header within the guest-supplied IO vector. We need to work
out which buffer the header is in, and verify that it's contiguous and
aligned as we need. At the moment this is open-coded, but introduce a
helper to make this more straightforward.
We add a new datatype 'struct iov_tail' representing an IO vector from
which we've logically consumed some number of headers. The IOV_PULL_HEADER
macro consumes a new header from the vector, returning a pointer and
updating the iov_tail.
Signed-off-by: David Gibson
We usually want to checksum only the tail part of a frame, excluding at
least some headers. csum_iov() does that for a frame represented as an
IO vector, not actually summing the entire IO vector. We now have struct
iov_tail to explicitly represent this construct, so replace csum_iov()
with csum_iov_tail() taking that representation rather than 3 parameters.
We propagate the same change to csum_udp4() and csum_udp6() which take
similar parameters. This slightly simplifies the code, and will allow some
further simplifications as struct iov_tail is more widely used.
Signed-off-by: David Gibson
Currently these expects both the TCP header and payload in a single IOV,
and goes to some trouble to locate the checksum field within it. In the
current caller we've already know where the TCP header is, so we might as
well just pass it in. This will need to work a bit differently for
vhost-user, but that code already needs to locate the TCP header for other
reasons, so again we can just pass it in.
Signed-off-by: David Gibson
At the moment these take separate pointers to the tap specific and IP
headers, but expect the TCP header and payload as a single tcp_payload_t.
As well as being slightly inconsistent, this involves some slightly iffy
pointer shenanigans when called on the flags path with a tcp_flags_t
instead of a tcp_payload_t.
More importantly, it's inconvenient for the upcoming vhost-user case, where
the TCP header and payload might not be contiguous. Furthermore, the
payload itself might not be contiguous.
So, pass the TCP header as its own pointer, and the TCP payload as an IO
vector.
Signed-off-by: David Gibson
The only reason we need separate functions for the IPv4 and IPv6 case is
to calculate the checksum of the IP pseudo-header, which is different for
the two cases. However, the caller already knows which path it's on and
can access the values needed for the pseudo-header partial sum more easily
than tcp_update_check_tcp[46]() can.
So, merge these functions into a single tcp_update_csum() function that
just takes the pseudo-header partial sum, calculated in the caller.
Signed-off-by: David Gibson
We have different versions of this function for IPv4 and IPv6, but the
caller already requires some IP version specific code to get the right
header pointers. Instead, have a common function that fills either an
IPv4 or an IPv6 header based on which header pointer it is passed. This
allows us to remove a small amount of code duplication and make a few
slightly ugly conditionals.
Signed-off-by: David Gibson
Because the vhost-user <-> virtio-net path ignores checksums, we usually
don't calculate them when sending packets to the guest. So, we always
pass no_tcp_csum=true to tcp_fill_headers(). We do want accurate
checksums when capturing packets though, so the captures don't show bogus
values.
Currently we handle this by updating the checksum field immediately before
writing the packet to the capture file, using tcp_vu_update_check(). This
is unnecessary, though: in each case tcp_fill_headers() is called not very
long before, so we can alter its no_tcp_csum parameter pased on whether
we're generating captures or not.
Signed-off-by: David Gibson
On Wed, 27 Nov 2024 14:54:03 +1100
David Gibson
Here's a current version of my IOV tail and some cleanups to TCP buffer handling based on it. Now rebased on top of v14 of the vhost-user patches.
This was aimed at sharing more code between the "buffer" and vhost-user paths, but that turned out to be trickier than I anticipated, so it hasn't really been accomplished. Nonetheless I think these are reasonable cleanups on their own merits, and may yet make sharing some more code between the paths easier in future.
David Gibson (7): iov: iov tail helpers iov, checksum: Replace csum_iov() with csum_iov_tail() tcp: Pass TCP header and payload separately to tcp_update_check_tcp[46]() tcp: Pass TCP header and payload separately to tcp_fill_headers[46]() tcp: Merge tcp_update_check_tcp[46]() tcp: Merge tcp_fill_headers[46]() with each other tcp_vu: Remove unnecessary tcp_vu_update_check() function
Applied. -- Stefano
participants (2)
-
David Gibson
-
Stefano Brivio