Re: [PATCH 6/7] tap: Implement the pasta vhost-net to-guest path

[Laurent Vivier <lvivier@redhat.com>]

On 9/4/26 23:28, aerosouund wrote:
> From: Ammar Yasser <aerosound161@gmail.com>
> 
> Add a vhost argument to tap_send_frames_pasta(). Callers pass it to say
> whether this send should go through vhost-net, and when it does sending
> goes through tap_send_frames_vhost(), which takes a descriptor from the
> queue shared with the kernel for each iov and points it at that iov's
> base address, chaining the bufs_per_frame descriptors of a frame
> together so the guest receives it in one go.
> 
> tx_reap() reclaims the descriptors the kernel has finished with, walking
> each used chain to its end so that every descriptor in it is counted as
> free again.
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Ammar Yasser <aerosound161@gmail.com>
> ---
>   tap.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 142 insertions(+), 3 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index c730535..0f51e62 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -359,12 +359,146 @@ void tap_icmp6_send(const struct ctx *c,
>   	tap_send_single(c, buf, l4len + ((char *)icmp6h - buf));
>   }
>   
> +/**
> + * 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 (vhost_vq_state[1].last_used_idx != used_idx) {
> +		uint16_t last_used, desc_id;
> +
> +		last_used = vhost_vq_state[1].last_used_idx % VHOST_NDESCS;
> +		desc_id = le32toh(used->ring[last_used].id);

desc_id is uint16_t  but used->ring[last_used].id is 32bit. should we use an uint32_t too?

> +
> +		for (;;) {
> +			/* keep going until we find a descriptor without the
> +			 * next flag
> +			 */
> +			vhost_vq_state[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);
> +		}
> +
> +		vhost_vq_state[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
> + * @nframes:		Number of frames to send
> + *
> + * @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
> + */
> +static size_t tap_send_frames_vhost(const struct ctx *c,
> +				    const struct iovec *iov,
> +				    size_t bufs_per_frame, size_t nframes)
> +{
> +	struct vring_avail *avail = &vring_avail_all[1].avail;
> +	size_t processed_frames = 0;
> +	size_t i;
> +
> +	/* reclaim descriptors if we don't have enough available buffers to
> +	 * perform this send
> +	 */
> +	if (vhost_vq_state[1].num_free < bufs_per_frame * nframes)
> +		tx_reap();
> +
> +	for (i = 0; i < nframes; i++) {
> +		uint16_t head;
> +		size_t j;
> +
> +		/* it's likely that tx_reap returned to us less than
> +		 * bufs_per_frame descs
> +		 */
> +		if (vhost_vq_state[1].num_free < bufs_per_frame)
> +			break;
> +
> +		/* set the index of the avail ring in the tx queue to be our
> +		 * last_used_idx
> +		 */
> +		head = vhost_vq_state[1].next_free % VHOST_NDESCS;
> +		avail->ring[(avail->idx + i) % VHOST_NDESCS] = htole16(head);
> +
> +		/* we will be consuming bufs_per_frame descriptors for every
> +		 * frame, decrement the local num_free
> +		 */
> +		vhost_vq_state[1].num_free -= bufs_per_frame;
> +
> +		for (j = 0; j < bufs_per_frame; ++j) {
> +			uint16_t next = vhost_vq_state[1].next_free %
> +					VHOST_NDESCS;
> +			/* get the last_used_idx descriptor */
> +			struct vring_desc *desc = &vring_desc[1][next];
> +			const struct iovec *iov_i;
> +
> +			/* 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)
> +			 */
> +			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);
> +
> +			vhost_vq_state[1].next_free++;
> +		}
> +
> +		processed_frames++;
> +	}
> +
> +	/* we didn't process any frames, no need to notify the kernel */
> +	if (!processed_frames)
> +		return 0;
> +
> +	smp_wmb();
> +
> +	/* we will have used nframes descriptor chains */
> +	avail->idx = htole16(le16toh(avail->idx) + processed_frames);
> +
> +	vhost_kick(&vring_used_all[1].used, c->vhost.vq[1].kick_fd);
> +
> +	/* wait until the kernel finishes processing this send */
> +	while (avail->idx != vring_used_all[1].used.idx)
> +		;
> +
> +	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:		Send through vhost-net rather than writing to the tap
>    *
>    * @iov must have total length @bufs_per_frame * @nframes, with each set of
>    * @bufs_per_frame contiguous buffers representing a single frame.
> @@ -375,11 +509,15 @@ void tap_icmp6_send(const struct ctx *c,
>    */
>   static size_t tap_send_frames_pasta(const struct ctx *c,
>   				    const struct iovec *iov,
> -				    size_t bufs_per_frame, size_t nframes)
> +				    size_t bufs_per_frame, size_t nframes,
> +				    bool vhost)
>   {
>   	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);
> @@ -515,7 +653,7 @@ void tap_send_single(const struct ctx *c, const void *data, size_t l2len)
>   		iov[iovcnt].iov_len = l2len;
>   		iovcnt++;
>   
> -		m = tap_send_frames_pasta(c, iov, iovcnt, 1);
> +		m = tap_send_frames_pasta(c, iov, iovcnt, 1, false);

You keep false, you should use "c->vhost.fd != -1" here

>   		break;
>   	case MODE_VU:
>   		vu_send_single(c, data, l2len);
> @@ -553,7 +691,8 @@ size_t tap_send_frames(const struct ctx *c, const struct iovec *iov,
>   
>   	switch (c->mode) {
>   	case MODE_PASTA:
> -		m = tap_send_frames_pasta(c, iov, bufs_per_frame, nframes);
> +		m = tap_send_frames_pasta(c, iov, bufs_per_frame, nframes,
> +					  c->vhost.fd != -1);
>   		break;
>   	case MODE_PASST:
>   		m = tap_send_frames_passt(c, iov, bufs_per_frame, nframes);