Re: [PATCH v2 09/10] tcp: Pass explicit data length to tcp_fill_headers()

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

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

On Fri, Apr 03, 2026 at 06:38:10PM +0200, Laurent Vivier wrote:
> tcp_fill_headers() computed the TCP payload length from iov_tail_size(),
> but with vhost-user multibuffer frames, the iov_tail will be larger than
> the actual data.  Pass the data length explicitly so that IP total
> length, pseudo-header, and checksum computations use the correct value.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tcp.c          | 5 +++--
>  tcp_buf.c      | 3 ++-
>  tcp_internal.h | 2 +-
>  tcp_vu.c       | 9 +++++----
>  4 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 49c6fb57ce16..6b0e25f33bf1 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -945,6 +945,7 @@ static void tcp_fill_header(struct tcphdr *th,
>   * @ip6h:		Pointer to IPv6 header, or NULL
>   * @th:			Pointer to TCP header
>   * @payload:		TCP payload
> + * @dlen:		TCP payload length
>   * @ip4_check:		IPv4 checksum, if already known
>   * @seq:		Sequence number for this segment
>   * @no_tcp_csum:	Do not set TCP checksum
> @@ -955,11 +956,11 @@ size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn,
>  			struct ethhdr *eh,
>  			struct iphdr *ip4h, struct ipv6hdr *ip6h,
>  			struct tcphdr *th, struct iov_tail *payload,
> -			const uint16_t *ip4_check, uint32_t seq,
> +			size_t dlen, const uint16_t *ip4_check, uint32_t seq,
>  			bool no_tcp_csum)
>  {
>  	const struct flowside *tapside = TAPFLOW(conn);
> -	size_t l4len = iov_tail_size(payload) + sizeof(*th);
> +	size_t l4len = dlen + sizeof(*th);
>  	uint8_t *omac = conn->f.tap_omac;
>  	size_t l3len = l4len;
>  	uint32_t psum = 0;
> diff --git a/tcp_buf.c b/tcp_buf.c
> index 41965b107567..27151854033c 100644
> --- a/tcp_buf.c
> +++ b/tcp_buf.c
> @@ -190,7 +190,8 @@ static void tcp_l2_buf_fill_headers(const struct ctx *c,
>  	else
>  		ip6h = iov[TCP_IOV_IP].iov_base;
>  
> -	l2len = tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &tail, check, seq,
> +	l2len = tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &tail,
> +				 iov_tail_size(&tail), check, seq,
>  				 no_tcp_csum);
>  	tap_hdr_update(taph, l2len);
>  }
> diff --git a/tcp_internal.h b/tcp_internal.h
> index d9408852571f..a0fa19f4ed11 100644
> --- a/tcp_internal.h
> +++ b/tcp_internal.h
> @@ -187,7 +187,7 @@ size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn,
>  			struct ethhdr *eh,
>  			struct iphdr *ip4h, struct ipv6hdr *ip6h,
>  			struct tcphdr *th, struct iov_tail *payload,
> -			const uint16_t *ip4_check, uint32_t seq,
> +			size_t dlen, const uint16_t *ip4_check, uint32_t seq,
>  			bool no_tcp_csum);
>  
>  int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 4eba7b8a5190..8c1894dca7fe 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -136,7 +136,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>  		seq--;
>  
>  	tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload,
> -			 NULL, seq, !*c->pcap);
> +			 optlen, NULL, seq, !*c->pcap);
>  
>  	vu_pad(&flags_elem[0].in_sg[0], l2len);
>  
> @@ -280,12 +280,13 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>   * @conn:		Connection pointer
>   * @iov:		Pointer to the array of IO vectors
>   * @iov_cnt:		Number of entries in @iov
> + * @dlen:		Data length
>   * @check:		Checksum, if already known
>   * @no_tcp_csum:	Do not set TCP checksum
>   * @push:		Set PSH flag, last segment in a batch
>   */
>  static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
> -			   struct iovec *iov, size_t iov_cnt,
> +			   struct iovec *iov, size_t iov_cnt, size_t dlen,
>  			   const uint16_t **check, bool no_tcp_csum, bool push)
>  {
>  	const struct flowside *toside = TAPFLOW(conn);
> @@ -329,7 +330,7 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
>  	th->ack = 1;
>  	th->psh = push;
>  
> -	tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload,
> +	tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload, dlen,
>  			 *check, conn->seq_to_tap, no_tcp_csum);
>  	if (ip4h)
>  		*check = &ip4h->check;
> @@ -457,7 +458,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
>  			check = NULL;
>  		previous_dlen = dlen;
>  
> -		tcp_vu_prepare(c, conn, iov, buf_cnt, &check, !*c->pcap, push);
> +		tcp_vu_prepare(c, conn, iov, buf_cnt, dlen, &check, !*c->pcap, push);
>  
>  		/* Pad first/single buffer only, it's at least ETH_ZLEN long */
>  		l2len = dlen + hdrlen - VNET_HLEN;
> -- 
> 2.53.0
> 

-- 
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 --]