Re: [PATCH 3/7] tcp_vu: Use iov_tail helpers to build headers in tcp_vu_prepare()

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

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

On Mon, Mar 23, 2026 at 05:52:55PM +0100, Laurent Vivier wrote:
> Replace direct pointer arithmetic using vu_eth(), vu_ip(),
> vu_payloadv4() and vu_payloadv6() with the iov_tail abstraction
> to build Ethernet, IP and TCP headers in tcp_vu_prepare().
> 
> This removes the assumption that all headers reside in a single
> contiguous iov entry.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

LGTM, with the exception of my general concerns about with_header().

> ---
>  tcp_vu.c | 61 ++++++++++++++++++++++++++------------------------------
>  1 file changed, 28 insertions(+), 33 deletions(-)
> 
> diff --git a/tcp_vu.c b/tcp_vu.c
> index c6206b7a689c..a39f6ea018e9 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -222,6 +222,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>  		/* reserve space for headers in iov */
>  		iov = &elem[elem_cnt].in_sg[0];
>  		assert(iov->iov_len >= hdrlen);
> +

Unrelated change (usually I don't mind if it's a reasonable change
adjacent to what you're doing, but this is an entirely different
function).

>  		iov->iov_base = (char *)iov->iov_base + hdrlen;
>  		iov->iov_len -= hdrlen;
>  		head[(*head_cnt)++] = elem_cnt;
> @@ -285,51 +286,45 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
>  			   int *check, bool no_tcp_csum, bool push)
>  {
>  	const struct flowside *toside = TAPFLOW(conn);
> -	bool v6 = !(inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr));
> -	size_t hdrlen = tcp_vu_hdrlen(v6);
> -	char *base = iov[0].iov_base;
> -	struct ipv6hdr *ip6h = NULL;
> -	struct iphdr *ip4h = NULL;
> +	bool ipv4 = inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr);
>  	struct iov_tail payload;
> -	struct tcphdr *th;
> -	struct ethhdr *eh;
> -
> -	/* we guess the first iovec provided by the guest can embed
> -	 * all the headers needed by L2 frame, including any padding
> -	 */
> -	assert(iov[0].iov_len >= hdrlen);
> -
> -	eh = vu_eth(base);
>  
> -	memcpy(eh->h_dest, c->guest_mac, sizeof(eh->h_dest));
> +	payload = IOV_TAIL(iov, iov_cnt, VNET_HLEN);
> +	with_header(struct ethhdr, eh, &payload)
> +		memcpy(eh->h_dest, c->guest_mac, sizeof(eh->h_dest));
> +	IOV_DROP_HEADER(&payload, struct ethhdr);
>  
>  	/* initialize header */
>  
> -	if (!v6) {
> -		eh->h_proto = htons(ETH_P_IP);
> -
> -		ip4h = vu_ip(base);
> -		*ip4h = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
> -		th = vu_payloadv4(base);
> +	if (ipv4) {
> +		with_header(struct iphdr, ip4h, &payload)
> +			*ip4h = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
> +		IOV_DROP_HEADER(&payload, struct iphdr);
>  	} else {
> -		eh->h_proto = htons(ETH_P_IPV6);
> -
> -		ip6h = vu_ip(base);
> -		*ip6h = (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_TCP);
> -
> -		th = vu_payloadv6(base);
> +		with_header(struct ipv6hdr, ip6h, &payload)
> +			*ip6h = (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_TCP);
> +		IOV_DROP_HEADER(&payload, struct ipv6hdr);
>  	}
>  
> -	memset(th, 0, sizeof(*th));
> -	th->doff = sizeof(*th) / 4;
> -	th->ack = 1;
> -	th->psh = push;
> +	with_header(struct tcphdr, th, &payload) {
> +		memset(th, 0, sizeof(*th));
> +		th->doff = sizeof(*th) / 4;
> +		th->ack = 1;
> +		th->psh = push;
> +	}
>  
>  	payload = IOV_TAIL(iov, iov_cnt, VNET_HLEN);
> -	tcp_fill_headers(c, conn, !v6, &payload, *check, conn->seq_to_tap,
> +	tcp_fill_headers(c, conn, ipv4, &payload, *check, conn->seq_to_tap,
>  			 no_tcp_csum);
> -	if (ip4h)
> +	if (ipv4) {
> +		struct iphdr ip4h_storage;
> +		const struct iphdr *ip4h;
> +
> +		IOV_DROP_HEADER(&payload, struct ethhdr);
> +		ip4h = IOV_PEEK_HEADER(&payload, ip4h_storage);
> +
>  		*check = ip4h->check;
> +	}
>  }
>  
>  /**
> -- 
> 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 --]