Re: [PATCH 1/7] tcp: Pass TCP header and payload separately to tcp_update_check_tcp[46]()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2598 bytes --]

On Tue, Oct 29, 2024 at 02:02:25PM +1100, David Gibson wrote:
> On Mon, Oct 28, 2024 at 07:42:54PM +0100, Stefano Brivio wrote:
> > On Mon, 28 Oct 2024 20:40:44 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > 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.
> > 
> > We couldn't do this, and also what you're now doing in 5/7, because
> > with vhost-user the TCP header is not aligned, so we can't pass it
> > around as a pointer, see:
> > 
> >   <ZeUpxEY-sn64NLE5@zatzit>
> >   https://archives.passt.top/passt-dev/ZeUpxEY-sn64NLE5@zatzit/
> > 
> > and following. That one is about IP headers, but the same applies to
> > TCP and UDP headers.
> 
> Hrm.  I'm aware it theoretically need not be aligned, but I thought it
> was in practice.. and that we were already relying on that.
> 
> In fact, I'm pretty sure the second part is true, although more subtly
> than here.  v8 of the vhost-user patches calls tcp_fill_headers[46]()
> with the bp parameter set to the offset of the TCP header.  If
> creating a tcphdr * there is a problem, then creating a tcp_payload_t
> * can't be any better.
> 
> > Of course the current solution is not elegant and it would be nice to
> > find another way to deal with it, but we couldn't come up with anything
> > better back then.
> > 
> > The rest of the series looks good to me, but I'm afraid that without
> > this one and 5/7 the other changes will be a bit more complicated to
> > implement (if at all possible).
> 
> Definitely.  I have so ideas for approaches more robust to
> misalignment, but they're substantially more complicated.  I was
> hoping we could avoid it at least for now.

I had a closer look at that earlier message now.  I believe at the
time I was aiming for fully robust handling of misaligned user
buffers.  AIUI, we've given up on that for the time being: instead
we'll just *test* for suitable alignment and we can do the hard work
of handling it if it ever arises in practice.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]