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

[Eugenio Perez Martin <eperezma@redhat.com>]

On Thu, Jul 24, 2025 at 2:51 AM David Gibson
<david@gibson.dropbear.id.au> wrote:
>
> 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.
>

Documenting for the next version.

> > +{
> > +     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