Re: [PATCH v6 4/5] tcp: Update TCP checksum using an iovec array

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 3 Oct 2024 16:55:56 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 03/10/2024 16:51, Laurent Vivier wrote:
> > TCP header and payload are supposed to be in the same buffer,
> > and tcp_update_check_tcp4()/tcp_update_check_tcp6() compute
> > the checksum from the base address of the header using the
> > length of the IP payload.
> > 
> > In the future (for vhost-user) we need to dispatch the TCP header and
> > the TCP payload through several buffers. To be able to manage that, we
> > provide an iovec array that points to the data of the TCP frame.
> > We provide also an offset to be able to provide an array that contains
> > the TCP frame embedded in an lower level frame, and this offset points
> > to the TCP header inside the iovec array.
> > 
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> > 
> > Notes:
> >      v6:
> >        - fix clang-tidy error with ptr
> >        - add missing static to tcp_update_check_tcp[4|6]
> >      
> >      v5:
> >        - s/IPv6/IPv4/
> >        - reintroduce ip6h and iph to avoid iov_size()
> >        - check pointer alignment before casting to the type
> >      
> >      v4:
> >        - replace die() by err() in tcp_update_check_tcp6() too
> >      
> >      v3:
> >        - replace die() by err() and return
> >        - add more information in the error message
> >      
> >      v2:
> >        - s/payload_offset/l4offset/
> >        - check memory address of the checksum (alignment, iovec boundaries)
> > 
> >   checksum.c |   1 -
> >   iov.c      |   1 -
> >   tcp.c      | 118 +++++++++++++++++++++++++++++++++++++++++++++--------
> >   3 files changed, 100 insertions(+), 20 deletions(-)
> >   
> ...
> > diff --git a/iov.c b/iov.c
> > index 3f9e229a305f..9116dda94247 100644
> > --- a/iov.c
> > +++ b/iov.c
> > @@ -25,7 +25,6 @@
> >   #include "util.h"
> >   #include "iov.h"
> >   
> > -
> >   /* iov_skip_bytes() - Skip leading bytes of an IO vector
> >    * @iov:	IO vector
> >    * @n:		Number of entries in @iov  
> 
> This is an unwanted change... could you remove it on merge?

Sure.

I still have to figure out what causes tests to fail before merging. By
applying just this series I haven't seen failures anymore, but I just
ran tests twice. It might take a bit.

-- 
Stefano