Re: [RFC v2 08/11] tap: add tap_free_old_xmit

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

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

On Wed, Jul 09, 2025 at 07:47:45PM +0200, Eugenio Pérez wrote:
> As pasta cannot modify the TCP sent buffers until vhost-kernel does not
> use them anymore, we need a way to report the caller the buffers that
> can be overriden.
> 
> Let's start by following the same pattern as in tap write(2): wait until
> pasta can override the buffers.  We can add async cleaning on top.
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  tap.c | 26 ++++++++++++++++++++++++++
>  tap.h |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/tap.c b/tap.c
> index 7ccac86..55357e3 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -128,6 +128,11 @@ static struct {
>  	/* Number of free descriptors */
>  	uint16_t num_free;
>  
> +	/* Last used_idx in the used ring.
> +	 * Duplicate here allows to check for proper vhost usage, and avoid
> +	 * false sharing between pasta and kernel. */
> +	uint16_t shadow_used_idx;
> +
>  	/* Last used idx processed */
>  	uint16_t last_used_idx;
>  
> @@ -467,6 +472,27 @@ static void vhost_kick(struct vring_used *used, int kick_fd) {
>  		eventfd_write(kick_fd, 1);
>  }
>  
> +/* n = target */
> +void tap_free_old_xmit(size_t n)

This function is introduced without either a caller or a descriptive
comment.  That makes it pretty hard to review - I'm not sure what it
is supposed to be doing.

> +{
> +	size_t r = 0;
> +
> +	while (r < n) {
> +		uint16_t used_idx = vqs[1].last_used_idx;
> +		if (vqs[1].shadow_used_idx == used_idx) {
> +		       vqs[1].shadow_used_idx = le16toh(*(volatile uint16_t*)&vring_used_1.used.idx);
> +
> +		       if (vqs[1].shadow_used_idx == used_idx)
> +			       continue;
> +		}
> +
> +		/* assert in-order */
> +		assert(vring_used_1.used.ring[used_idx % VHOST_NDESCS].id == vring_avail_1.avail.ring[used_idx % VHOST_NDESCS]);
> +		vqs[1].num_free += vqs[1].ndescs[used_idx % VHOST_NDESCS];
> +		vqs[1].last_used_idx++;
> +		r++;
> +	}
> +}
>  
>  /**
>   * tap_send_frames_vhost() - Send multiple frames to the pasta tap
> diff --git a/tap.h b/tap.h
> index e924dfb..7ca0fb0 100644
> --- a/tap.h
> +++ b/tap.h
> @@ -112,6 +112,7 @@ void tap_icmp6_send(const struct ctx *c,
>  		    const struct in6_addr *src, const struct in6_addr *dst,
>  		    const void *in, size_t l4len);
>  void tap_send_single(const struct ctx *c, const void *data, size_t l2len, bool vhost);
> +void tap_free_old_xmit(size_t n);
>  size_t tap_send_frames(const struct ctx *c, const struct iovec *iov,
>  		       size_t bufs_per_frame, size_t nframes, bool vhost);
>  void eth_update_mac(struct ethhdr *eh,

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