On Fri, Apr 03, 2026 at 06:42:41PM +0200, Laurent Vivier wrote:
> Change udp_update_hdr4() and udp_update_hdr6() to take an iov_tail
> pointing at the UDP frame instead of a contiguous udp_payload_t buffer
> and explicit data length.  This lets vhost-user pass scatter-gather
> virtqueue buffers directly without an intermediate copy.
> 
> The UDP header is built into a local struct udphdr and written back with
> IOV_PUSH_HEADER().  On the tap side, udp_tap_prepare() wraps the
> existing udp_payload_t in a two-element iov to match the new interface.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  iov.c          |  1 -
>  udp.c          | 65 +++++++++++++++++++++------------------
>  udp_internal.h |  4 +--
>  udp_vu.c       | 82 +++++++++++++++++++++++++++-----------------------
>  4 files changed, 82 insertions(+), 70 deletions(-)
> 
> diff --git a/iov.c b/iov.c
> index b1bcdc4649df..c0d9c6d21322 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -368,7 +368,6 @@ void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align)
>   *
>   * Return: number of bytes written
>   */
> -/* cppcheck-suppress unusedFunction */
>  size_t iov_push_header_(struct iov_tail *tail, const void *v, size_t len)
>  {
>  	size_t l;
> diff --git a/udp.c b/udp.c
> index e113b26bc726..46eb199c74c9 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -255,21 +255,22 @@ static void udp_iov_init(const struct ctx *c)
>  /**
>   * udp_update_hdr4() - Update headers for one IPv4 datagram
>   * @ip4h:		Pre-filled IPv4 header (except for tot_len and saddr)
> - * @bp:			Pointer to udp_payload_t to update
> + * @payload:		iov_tail with UDP payload to update

To me "UDP payload" implies it doesn't include the UDP header, but I
think from the code this *will* include the UDP header.

>   * @toside:		Flowside for destination side
>   * @dlen:		Length of UDP payload
>   * @no_udp_csum:	Do not set UDP checksum
>   *
>   * Return: size of IPv4 payload (UDP header + data)
>   */
> -size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
> +size_t udp_update_hdr4(struct iphdr *ip4h, struct iov_tail *payload,
>  		       const struct flowside *toside, size_t dlen,
>  		       bool no_udp_csum)
>  {
>  	const struct in_addr *src = inany_v4(&toside->oaddr);
>  	const struct in_addr *dst = inany_v4(&toside->eaddr);
> -	size_t l4len = dlen + sizeof(bp->uh);
> +	size_t l4len = dlen + sizeof(struct udphdr);

dlen + sizeof(uh) would break the reverse christmas tree a bit, but I
think I'd still prefer it on balance.

>  	size_t l3len = l4len + sizeof(*ip4h);
> +	struct udphdr uh;
>  
>  	assert(src && dst);
>  
> @@ -278,19 +279,18 @@ size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
>  	ip4h->saddr = src->s_addr;
>  	ip4h->check = csum_ip4_header(l3len, IPPROTO_UDP, *src, *dst);
>  
> -	bp->uh.source = htons(toside->oport);
> -	bp->uh.dest = htons(toside->eport);
> -	bp->uh.len = htons(l4len);
> +	uh.source = htons(toside->oport);
> +	uh.dest = htons(toside->eport);
> +	uh.len = htons(l4len);

This is done unconditionally, so it could be an initializer for uh.

>  	if (no_udp_csum) {
> -		bp->uh.check = 0;
> +		uh.check = 0;
>  	} else {
> -		const struct iovec iov = {
> -			.iov_base = bp->data,
> -			.iov_len = dlen
> -		};
> -		struct iov_tail data = IOV_TAIL(&iov, 1, 0);
> -		csum_udp4(&bp->uh, *src, *dst, &data, l4len);
> +		struct iov_tail data = *payload;

Clunky, but I can't see a nicer way around this.

> +
> +		IOV_DROP_HEADER(&data, struct udphdr);
> +		csum_udp4(&uh, *src, *dst, &data, l4len);
>  	}
> +	IOV_PUSH_HEADER(payload, uh);
>  
>  	return l4len;
>  }
> @@ -299,18 +299,19 @@ size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
>   * udp_update_hdr6() - Update headers for one IPv6 datagram
>   * @ip6h:		Pre-filled IPv6 header (except for payload_len and
>   * 			addresses)
> - * @bp:			Pointer to udp_payload_t to update
> + * @payload:		iov_tail with UDP payload to update
>   * @toside:		Flowside for destination side
>   * @dlen:		Length of UDP payload
>   * @no_udp_csum:	Do not set UDP checksum
>   *
>   * Return: size of IPv6 payload (UDP header + data)
>   */
> -size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp,
> +size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct iov_tail *payload,
>  		       const struct flowside *toside, size_t dlen,
>  		       bool no_udp_csum)

Similar comments to the IPv4 version.

>  {
> -	uint16_t l4len = dlen + sizeof(bp->uh);
> +	uint16_t l4len = dlen + sizeof(struct udphdr);
> +	struct udphdr uh;
>  
>  	ip6h->payload_len = htons(l4len);
>  	ip6h->daddr = toside->eaddr.a6;
> @@ -319,24 +320,23 @@ size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp,
>  	ip6h->nexthdr = IPPROTO_UDP;
>  	ip6h->hop_limit = 255;
>  
> -	bp->uh.source = htons(toside->oport);
> -	bp->uh.dest = htons(toside->eport);
> -	bp->uh.len = ip6h->payload_len;
> +	uh.source = htons(toside->oport);
> +	uh.dest = htons(toside->eport);
> +	uh.len = htons(l4len);
>  	if (no_udp_csum) {
>  		/* 0 is an invalid checksum for UDP IPv6 and dropped by
>  		 * the kernel stack, even if the checksum is disabled by virtio
>  		 * flags. We need to put any non-zero value here.
>  		 */
> -		bp->uh.check = 0xffff;
> +		uh.check = 0xffff;
>  	} else {
> -		const struct iovec iov = {
> -			.iov_base = bp->data,
> -			.iov_len = dlen
> -		};
> -		struct iov_tail data = IOV_TAIL(&iov, 1, 0);
> -		csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data,
> -			  l4len);
> +		struct iov_tail data = *payload;
> +
> +		IOV_DROP_HEADER(&data, struct udphdr);
> +		csum_udp6(&uh, &toside->oaddr.a6, &toside->eaddr.a6,
> +			  &data, l4len);
>  	}
> +	IOV_PUSH_HEADER(payload, uh);
>  
>  	return l4len;
>  }
> @@ -375,11 +375,18 @@ static void udp_tap_prepare(const struct mmsghdr *mmh,
>  	struct ethhdr *eh = (*tap_iov)[UDP_IOV_ETH].iov_base;
>  	struct udp_payload_t *bp = &udp_payload[idx];
>  	struct udp_meta_t *bm = &udp_meta[idx];
> +	struct iovec iov[2];
> +	struct iov_tail payload = IOV_TAIL(iov, ARRAY_SIZE(iov), 0);
>  	size_t l4len, l2len;
>  
> +	iov[0].iov_base = &bp->uh;
> +	iov[0].iov_len = sizeof(bp->uh);
> +	iov[1].iov_base = bp->data;
> +	iov[1].iov_len = mmh[idx].msg_len;
> +
>  	eth_update_mac(eh, NULL, tap_omac);
>  	if (!inany_v4(&toside->eaddr) || !inany_v4(&toside->oaddr)) {
> -		l4len = udp_update_hdr6(&bm->ip6h, bp, toside,
> +		l4len = udp_update_hdr6(&bm->ip6h, &payload, toside,
>  					mmh[idx].msg_len, no_udp_csum);
>  
>  		l2len = MAX(l4len + sizeof(bm->ip6h) + ETH_HLEN, ETH_ZLEN);
> @@ -388,7 +395,7 @@ static void udp_tap_prepare(const struct mmsghdr *mmh,
>  		eh->h_proto = htons_constant(ETH_P_IPV6);
>  		(*tap_iov)[UDP_IOV_IP] = IOV_OF_LVALUE(bm->ip6h);
>  	} else {
> -		l4len = udp_update_hdr4(&bm->ip4h, bp, toside,
> +		l4len = udp_update_hdr4(&bm->ip4h, &payload, toside,
>  					mmh[idx].msg_len, no_udp_csum);
>  
>  		l2len = MAX(l4len + sizeof(bm->ip4h) + ETH_HLEN, ETH_ZLEN);
> diff --git a/udp_internal.h b/udp_internal.h
> index 64e457748324..e6cbaab79519 100644
> --- a/udp_internal.h
> +++ b/udp_internal.h
> @@ -25,10 +25,10 @@ struct udp_payload_t {
>  } __attribute__ ((packed, aligned(__alignof__(unsigned int))));
>  #endif
>  
> -size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp,
> +size_t udp_update_hdr4(struct iphdr *ip4h, struct iov_tail *payload,
>  		       const struct flowside *toside, size_t dlen,
>  		       bool no_udp_csum);
> -size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp,
> +size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct iov_tail *payload,
>  		       const struct flowside *toside, size_t dlen,
>  		       bool no_udp_csum);
>  void udp_sock_fwd(const struct ctx *c, int s, int rule_hint,
> diff --git a/udp_vu.c b/udp_vu.c
> index 16086e6c0c03..989751deafe0 100644
> --- a/udp_vu.c
> +++ b/udp_vu.c
> @@ -96,43 +96,52 @@ static ssize_t udp_vu_sock_recv(struct iovec *iov, size_t *cnt, int s, bool v6)
>  /**
>   * udp_vu_prepare() - Prepare the packet header
>   * @c:		Execution context
> - * @iov:	IO vector for the frame (including vnet header)
> + * @data:	IO vector tail for the frame, on return points to the L4 header

This doesn't make it clear which headers @data includes on entry.

>   * @toside:	Address information for one side of the flow
>   * @dlen:	Packet data length
>   *
>   * Return: Layer-4 length
>   */
> -static size_t udp_vu_prepare(const struct ctx *c, const struct iovec *iov,
> -			     const struct flowside *toside, ssize_t dlen)
> +static size_t udp_vu_prepare(const struct ctx *c, struct iov_tail *data,
> +			     const struct flowside *toside, size_t dlen)
>  {
> -	struct ethhdr *eh;
> +	bool ipv4 = inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr);
> +	struct ethhdr eh;
>  	size_t l4len;
>  
>  	/* ethernet header */
> -	eh = vu_eth(iov[0].iov_base);
> +	memcpy(eh.h_dest, c->guest_mac, sizeof(eh.h_dest));
> +	memcpy(eh.h_source, c->our_tap_mac, sizeof(eh.h_source));
>  
> -	memcpy(eh->h_dest, c->guest_mac, sizeof(eh->h_dest));
> -	memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source));
> +	if (ipv4)
> +		eh.h_proto = htons(ETH_P_IP);
> +	else
> +		eh.h_proto = htons(ETH_P_IPV6);
> +	IOV_PUSH_HEADER(data, eh);

From this, I'm assuming it does include the ethernet header, but not
the vnet header.

>  
>  	/* initialize header */
> -	if (inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr)) {
> -		struct iphdr *iph = vu_ip(iov[0].iov_base);
> -		struct udp_payload_t *bp = vu_payloadv4(iov[0].iov_base);
> +	if (ipv4) {
> +		struct iov_tail udp_frame;
> +		struct iphdr iph;
>  
> -		eh->h_proto = htons(ETH_P_IP);
> +		iph = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_UDP);

This could be an initializer, since iph is a local now.

> -		*iph = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_UDP);
> +		udp_frame = *data;

It's not strict, but I generally see the convention as being "frame"
for the L2 object, "packet" for the L3 object and "datagram" for the
L4 object.  So "udp_frame" reads a bit oddly here, I'd call it
"datagram".

> +		IOV_DROP_HEADER(&udp_frame, struct iphdr);
> +		l4len = udp_update_hdr4(&iph, &udp_frame, toside, dlen, true);
>  
> -		l4len = udp_update_hdr4(iph, bp, toside, dlen, true);
> +		IOV_PUSH_HEADER(data, iph);
>  	} else {
> -		struct ipv6hdr *ip6h = vu_ip(iov[0].iov_base);
> -		struct udp_payload_t *bp = vu_payloadv6(iov[0].iov_base);
> +		struct iov_tail udp_frame;
> +		struct ipv6hdr ip6h;
>  
> -		eh->h_proto = htons(ETH_P_IPV6);
> +		ip6h = (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_UDP);
>  
> -		*ip6h = (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_UDP);
> +		udp_frame = *data;
> +		IOV_DROP_HEADER(&udp_frame, struct ipv6hdr);
> +		l4len = udp_update_hdr6(&ip6h, &udp_frame, toside, dlen, true);
>  
> -		l4len = udp_update_hdr6(ip6h, bp, toside, dlen, true);
> +		IOV_PUSH_HEADER(data, ip6h);
>  	}
>  
>  	return l4len;
> @@ -141,29 +150,25 @@ static size_t udp_vu_prepare(const struct ctx *c, const struct iovec *iov,
>  /**
>   * udp_vu_csum() - Calculate and set checksum for a UDP packet
>   * @toside:	Address information for one side of the flow
> - * @iov:	IO vector for the frame
> - * @cnt:	Number of IO vector entries
> + * @data:	IO vector tail for the L4 frame (point to the UDP header)
>   * @l4len:	L4 length
>   */
> -static void udp_vu_csum(const struct flowside *toside, const struct iovec *iov,
> -			size_t cnt, size_t l4len)
> +static void udp_vu_csum(const struct flowside *toside, struct iov_tail *data,
> +			size_t l4len)
>  {
>  	const struct in_addr *src4 = inany_v4(&toside->oaddr);
>  	const struct in_addr *dst4 = inany_v4(&toside->eaddr);
> -	char *base = iov[0].iov_base;
> -	struct udp_payload_t *bp;
> -	struct iov_tail data;
> -
> -	if (src4 && dst4) {
> -		bp = vu_payloadv4(base);
> -		data = IOV_TAIL(iov, cnt, (char *)&bp->data - base);
> -		csum_udp4(&bp->uh, *src4, *dst4, &data, l4len);
> -	} else {
> -		bp = vu_payloadv6(base);
> -		data = IOV_TAIL(iov, cnt, (char *)&bp->data - base);
> -		csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data,
> -			  l4len);
> -	}
> +	struct udphdr *uh, uh_storage;
> +	bool ipv4 = src4 && dst4;
> +
> +	uh = IOV_REMOVE_HEADER(data, uh_storage);
> +	if (!uh)
> +		return;
> +
> +	if (ipv4)
> +		csum_udp4(uh, *src4, *dst4, data, l4len);

This is still implicitly requiring a contiguous UDP header, since
you're writing to the pointer from IOV_REMOVE_HEADER.  Which makes me
think: maybe we should alter iov_peek_header() to return a const
pointer to catch cases like that.

> +	else
> +		csum_udp6(uh, &toside->oaddr.a6, &toside->eaddr.a6, data, l4len);
>  }
>  
>  /**
> @@ -230,9 +235,10 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx)
>  		vu_queue_rewind(vq, elem_cnt - elem_used);
>  
>  		if (iov_cnt > 0) {
> -			size_t l4len = udp_vu_prepare(c, iov_vu, toside, dlen);
> +			struct iov_tail data = IOV_TAIL(iov_vu, iov_cnt, VNET_HLEN);
> +			size_t l4len = udp_vu_prepare(c, &data, toside, dlen);
>  			if (*c->pcap) {
> -				udp_vu_csum(toside, iov_vu, iov_cnt, l4len);
> +				udp_vu_csum(toside, &data, l4len);
>  				pcap_iov(iov_vu, iov_cnt, VNET_HLEN,
>  					 hdrlen + dlen - 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