Re: [RFC v3 8/8] tap: Implement pasta vhost TX

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

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

On Sun, Aug 02, 2026 at 01:21:55PM +0000, Ammar Yasser wrote:
> Callers will specify whether vhost should be used or no by passing the
> vhost argument to tap_send_frames_pasta. If specified, sending will go
> through a function called tap_send_frames_vhost, which pops a descriptor
> from the queue shared with the kernel and sets the address of that
> descriptor to be the base address of a single iov from the group of
>  buffers_per_frame * nframes iovs we pass to the function
> 
> Signed-off-by: Ammar Yasser <aerosound161@gmail.com>

Sorry, didn't spot this earlier in the series: if these are based on
Eugenio's earlier patches, they should have his Signed-off-by in
addition to your own.

> ---
>  tap.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/tap.c b/tap.c
> index e177eef..66bb71d 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -361,7 +361,32 @@ void tap_icmp6_send(const struct ctx *c,
>  }
>  
>  /**
> - * tap_send_frames_pasta() - Send multiple frames to the pasta tap
> + * tx_reap() - Reclaim the descriptors the kernel has already processed
> + */
> +static void tx_reap(void) {
> +	struct vring_used *used = &vring_used_all[1].used;
> +	uint16_t used_idx = le16toh(used->idx);
> +
> +	smp_rmb();
> +
> +	/* increment last_used_idx until it reaches the kernel's used index */
> +	while (vqs[1].last_used_idx != used_idx)  {
> +		uint16_t desc_id = le32toh(used->ring[vqs[1].last_used_idx % VHOST_NDESCS].id);
> +		
> +		for (;;) {
> +			/* keep going until we find a descriptor without the next flag */
> +			vqs[1].num_free++;
> +			if (!(le16toh(vring_desc[1][desc_id].flags) & VRING_DESC_F_NEXT))
> +				break;
> +			/* this descriptor wasn't the last, set desc_id to the next one and keep going */
> +			desc_id = le16toh(vring_desc[1][desc_id].next);
> +		}
> +		vqs[1].last_used_idx++;
> +	}
> +}
> +
> +/**
> + * tap_send_frames_vhost() - Send multiple frames to the pasta tap
>   * @c:			Execution context
>   * @iov:		Array of buffers
>   * @bufs_per_frame:	Number of buffers (iovec entries) per frame
> @@ -371,6 +396,84 @@ void tap_icmp6_send(const struct ctx *c,
>   * @bufs_per_frame contiguous buffers representing a single frame.
>   *
>   * Return: number of frames successfully sent
> + */
> +static size_t tap_send_frames_vhost(const struct ctx *c,
> +				    const struct iovec *iov,
> +				    size_t bufs_per_frame, size_t nframes)
> +{
> +	size_t i;
> +	size_t processed_frames = 0;

Reverse christmas tree.

> +
> +	/* update our local counters first */
> +	tx_reap();
> +
> +	#define AVAIL_Q(i)(vring_avail_all[i].avail)

# for preprocessor directives should always go in column 0.

> +
> +	for (i = 0; i < nframes; i++) {
> +		size_t j;
> +
> +		if (vqs[1].num_free < bufs_per_frame)
> +			break;

IIRC, we were discussing during the last call that at least for the
first version it's fine if we synchronously wait for buffers to be
available.  Since then it occurred to me that another acceptable
option would be to simply drop frames if there are no available
buffers (this is kind of how IP is designed to work - congestion is
reported implicitly as packet loss).  Do whichever is easier for now
and it can be refined later.

> +
> +		/* set the index of the avail ring in the tx queue to be our last_used_idx */
> +		uint16_t head = vqs[1].next_free % VHOST_NDESCS;

No inline decls.

> +		AVAIL_Q(1).ring[(AVAIL_Q(1).idx + i) % VHOST_NDESCS] = htole16(head);
> +
> +		/* we will be consuming bufs_per_frame descriptors for every frame, decrement the local num_free */
> +		vqs[1].num_free -= bufs_per_frame;
> +
> +		for (j = 0; j < bufs_per_frame; ++j) {
> +			/* get the last_used_idx descriptor */
> +			struct vring_desc *desc = &vring_desc[1][vqs[1].next_free % VHOST_NDESCS];
> +			/*
> +			 * the iov variable contains the iovecs for all frames we will send.
> +			 * access a single fragment of a frame (each fragment is one of tcp_iov_parts)
> +			 * denoted by iov at index i (index of the frame being processed) * bufs_per_frame 
> +			 * plus j (the index of the fragment being processed)
> +			 */	
> +			const struct iovec *iov_i = &iov[i * bufs_per_frame + j];
> +
> +			/* 
> +			 * set that descriptor's address to the base of the iov and set the VRING_DESC_F_NEXT
> +			 * flag on the descriptor if its not the last frame fragment, so that the
> +			 * guest would recieve the entire frame in one go. 
> +			 */
> +			desc->addr = (uint64_t)iov_i->iov_base;
> +			desc->len = iov_i->iov_len;
> +			desc->flags = (j == bufs_per_frame - 1) ? 0 : htole16(VRING_DESC_F_NEXT);
> +			vqs[1].next_free++;
> +		}
> +
> +		processed_frames++;
> +	}
> +
> +	/* we didn't process any frames, no need to notify the kernel */
> +	if ((processed_frames == 0))
> +		return 0;
> +
> +	smp_wmb();
> +	/* we will have used nframes descriptor chains */
> +	AVAIL_Q(1).idx = htole16(le16toh(AVAIL_Q(1).idx)+processed_frames);
> +	#undef AVAIL_Q
> +
> +	vhost_kick(&vring_used_all[1].used, c->vq[1].kick_fd);
> +
> +	return processed_frames;
> +}
> +
> +
> +/**
> + * tap_send_frames_pasta() - Send multiple frames to the pasta tap
> + * @c:			Execution context
> + * @iov:		Array of buffers
> + * @bufs_per_frame:	Number of buffers (iovec entries) per frame
> + * @nframes:		Number of frames to send
> + * @vhost:             Use vhost-kernel or not
> + *
> + * @iov must have total length @bufs_per_frame * @nframes, with each set of
> + * @bufs_per_frame contiguous buffers representing a single frame.
> + *
> + * Return: number of frames successfully sent (or queued)
>   *
>   * #syscalls:pasta write
>   */
> @@ -381,6 +484,9 @@ static size_t tap_send_frames_pasta(const struct ctx *c,
>  	size_t nbufs = bufs_per_frame * nframes;
>  	size_t i;
>  
> +	if (vhost)
> +		return tap_send_frames_vhost(c, iov, bufs_per_frame, nframes);
> +
>  	for (i = 0; i < nbufs; i += bufs_per_frame) {
>  		ssize_t rc = writev(c->fd_tap, iov + i, bufs_per_frame);
>  		size_t framelen = iov_size(iov + i, bufs_per_frame);
> -- 
> 2.34.1
> 

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