Re: [PATCH net] vhost: rewind next_avail_head while discarding descriptors

On Thu, Nov 13, 2025 at 4:13 PM Michael S. Tsirkin mst@redhat.com wrote:

On Thu, Nov 13, 2025 at 09:54:20AM +0800, Jason Wang wrote:

...

When discarding descriptors with IN_ORDER, we should rewind next_avail_head otherwise it would run out of sync with last_avail_idx. This would cause driver to report "id X is not a head".

Fixing this by returning the number of descriptors that is used for each buffer via vhost_get_vq_desc_n() so caller can use the value while discarding descriptors.

Fixes: 67a873df0c41 ("vhost: basic in order support") Cc: stable@vger.kernel.org Signed-off-by: Jason Wang jasowang@redhat.com

Wow that change really caused a lot of fallout.

Thanks for the patch! Yet something to improve:

...

---

drivers/vhost/net.c | 53 ++++++++++++++++++++++++++----------------- drivers/vhost/vhost.c | 43 ++++++++++++++++++++++++----------- drivers/vhost/vhost.h | 9 +++++++- 3 files changed, 70 insertions(+), 35 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 35ded4330431..8f7f50acb6d6 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -592,14 +592,15 @@ static void vhost_net_busy_poll(struct vhost_net *net, static int vhost_net_tx_get_vq_desc(struct vhost_net *net, struct vhost_net_virtqueue *tnvq, unsigned int *out_num, unsigned int *in_num,

•                             struct msghdr *msghdr, bool *busyloop_intr)
  

•                             struct msghdr *msghdr, bool *busyloop_intr,
  

•                             unsigned int *ndesc)
  

{ struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX]; struct vhost_virtqueue *rvq = &rnvq->vq; struct vhost_virtqueue *tvq = &tnvq->vq;

• int r = vhost_get_vq_desc(tvq, tvq->iov, ARRAY_SIZE(tvq->iov),
  

•                           out_num, in_num, NULL, NULL);
  

• int r = vhost_get_vq_desc_n(tvq, tvq->iov, ARRAY_SIZE(tvq->iov),
  

•                             out_num, in_num, NULL, NULL, ndesc);
  
  if (r == tvq->num && tvq->busyloop_timeout) {
          /* Flush batched packets first */
  

@@ -610,8 +611,8 @@ static int vhost_net_tx_get_vq_desc(struct vhost_net *net,

          vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, false);

•         r = vhost_get_vq_desc(tvq, tvq->iov, ARRAY_SIZE(tvq->iov),
  

•                               out_num, in_num, NULL, NULL);
  

•         r = vhost_get_vq_desc_n(tvq, tvq->iov, ARRAY_SIZE(tvq->iov),
  

•                                 out_num, in_num, NULL, NULL, ndesc);
  }
  
  return r;
  

@@ -642,12 +643,14 @@ static int get_tx_bufs(struct vhost_net *net, struct vhost_net_virtqueue *nvq, struct msghdr *msg, unsigned int *out, unsigned int *in,

•                size_t *len, bool *busyloop_intr)
  

•                size_t *len, bool *busyloop_intr,
  

•                unsigned int *ndesc)
  

{ struct vhost_virtqueue *vq = &nvq->vq; int ret;

• ret = vhost_net_tx_get_vq_desc(net, nvq, out, in, msg, busyloop_intr);
  

• ret = vhost_net_tx_get_vq_desc(net, nvq, out, in, msg,
  

•                                busyloop_intr, ndesc);
  
  if (ret < 0 || ret == vq->num)
          return ret;
  

@@ -766,6 +769,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock) int sent_pkts = 0; bool sock_can_batch = (sock->sk->sk_sndbuf == INT_MAX); bool in_order = vhost_has_feature(vq, VIRTIO_F_IN_ORDER);

• unsigned int ndesc = 0;
  
  do {
          bool busyloop_intr = false;
  

@@ -774,7 +778,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock) vhost_tx_batch(net, nvq, sock, &msg);

          head = get_tx_bufs(net, nvq, &msg, &out, &in, &len,

•                            &busyloop_intr);
  

•                            &busyloop_intr, &ndesc);
          /* On error, stop handling until the next kick. */
          if (unlikely(head < 0))
                  break;
  

@@ -806,7 +810,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock) goto done; } else if (unlikely(err != -ENOSPC)) { vhost_tx_batch(net, nvq, sock, &msg);

•                         vhost_discard_vq_desc(vq, 1);
  

•                         vhost_discard_vq_desc(vq, 1, ndesc);
                          vhost_net_enable_vq(net, vq);
                          break;
                  }
  

@@ -829,7 +833,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock) err = sock->ops->sendmsg(sock, &msg, len); if (unlikely(err < 0)) { if (err == -EAGAIN || err == -ENOMEM || err == -ENOBUFS) {

•                         vhost_discard_vq_desc(vq, 1);
  

•                         vhost_discard_vq_desc(vq, 1, ndesc);
                          vhost_net_enable_vq(net, vq);
                          break;
                  }
  

@@ -868,6 +872,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock) int err; struct vhost_net_ubuf_ref *ubufs; struct ubuf_info_msgzc *ubuf;

• unsigned int ndesc = 0;
  bool zcopy_used;
  int sent_pkts = 0;
  
  

@@ -879,7 +884,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)

          busyloop_intr = false;
          head = get_tx_bufs(net, nvq, &msg, &out, &in, &len,

•                            &busyloop_intr);
  

•                            &busyloop_intr, &ndesc);
          /* On error, stop handling until the next kick. */
          if (unlikely(head < 0))
                  break;
  

@@ -941,7 +946,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock) vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN; } if (retry) {

•                         vhost_discard_vq_desc(vq, 1);
  

•                         vhost_discard_vq_desc(vq, 1, ndesc);
                          vhost_net_enable_vq(net, vq);
                          break;
                  }
  

@@ -1045,11 +1050,12 @@ static int get_rx_bufs(struct vhost_net_virtqueue *nvq, unsigned *iovcount, struct vhost_log *log, unsigned *log_num,

•                unsigned int quota)
  

•                unsigned int quota,
  

•                unsigned int *ndesc)
  

{ struct vhost_virtqueue *vq = &nvq->vq; bool in_order = vhost_has_feature(vq, VIRTIO_F_IN_ORDER);

• unsigned int out, in;
  

• unsigned int out, in, desc_num, n = 0;
  int seg = 0;
  int headcount = 0;
  unsigned d;
  

@@ -1064,9 +1070,9 @@ static int get_rx_bufs(struct vhost_net_virtqueue *nvq, r = -ENOBUFS; goto err; }

•         r = vhost_get_vq_desc(vq, vq->iov + seg,
  

•                               ARRAY_SIZE(vq->iov) - seg, &out,
  

•                               &in, log, log_num);
  

•         r = vhost_get_vq_desc_n(vq, vq->iov + seg,
  

•                                 ARRAY_SIZE(vq->iov) - seg, &out,
  

•                                 &in, log, log_num, &desc_num);
          if (unlikely(r < 0))
                  goto err;
  
  

@@ -1093,6 +1099,7 @@ static int get_rx_bufs(struct vhost_net_virtqueue *nvq, ++headcount; datalen -= len; seg += in;

•         n += desc_num;
  }
  
  *iovcount = seg;
  

@@ -1113,9 +1120,11 @@ static int get_rx_bufs(struct vhost_net_virtqueue *nvq, nheads[0] = headcount; }

• *ndesc = n;
  

• return headcount;
  

err:

• vhost_discard_vq_desc(vq, headcount);
  

• vhost_discard_vq_desc(vq, headcount, n);
  

So here ndesc and n are the same, but below in vhost_discard_vq_desc they are different. Fun.

Not necessarily the same, a buffer could contain more than 1 descriptor.

...

  return r;

}

@@ -1151,6 +1160,7 @@ static void handle_rx(struct vhost_net *net) struct iov_iter fixup; __virtio16 num_buffers; int recv_pkts = 0;

• unsigned int ndesc;
  
  mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_RX);
  sock = vhost_vq_get_backend(vq);
  

@@ -1182,7 +1192,8 @@ static void handle_rx(struct vhost_net *net) headcount = get_rx_bufs(nvq, vq->heads + count, vq->nheads + count, vhost_len, &in, vq_log, &log,

•                                 likely(mergeable) ? UIO_MAXIOV : 1);
  

•                                 likely(mergeable) ? UIO_MAXIOV : 1,
  

•                                 &ndesc);
          /* On error, stop handling until the next kick. */
          if (unlikely(headcount < 0))
                  goto out;
  

@@ -1228,7 +1239,7 @@ static void handle_rx(struct vhost_net *net) if (unlikely(err != sock_len)) { pr_debug("Discarded rx packet: " " len %d, expected %zd\n", err, sock_len);

•                 vhost_discard_vq_desc(vq, headcount);
  

•                 vhost_discard_vq_desc(vq, headcount, ndesc);
                  continue;
          }
          /* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */
  

@@ -1252,7 +1263,7 @@ static void handle_rx(struct vhost_net *net) copy_to_iter(&num_buffers, sizeof num_buffers, &fixup) != sizeof num_buffers) { vq_err(vq, "Failed num_buffers write");

•                 vhost_discard_vq_desc(vq, headcount);
  

•                 vhost_discard_vq_desc(vq, headcount, ndesc);
                  goto out;
          }
          nvq->done_idx += headcount;
  

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 8570fdf2e14a..b56568807588 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2792,18 +2792,11 @@ static int get_indirect(struct vhost_virtqueue *vq, return 0; }

-/* This looks in the virtqueue and for the first available buffer, and converts

• • it to an iovec for convenient access. Since descriptors consist of some
• • number of output then some number of input descriptors, it's actually two
• • iovecs, but we pack them into one and note how many of each there were.
• • This function returns the descriptor number found, or vq->num (which is
• • never a valid descriptor number) if none was found. A negative code is
• • returned on error. */

A new module API with no docs at all is not good. Please add documentation to this one. vhost_get_vq_desc is a subset and could refer to it.

Fixed.

...

-int vhost_get_vq_desc(struct vhost_virtqueue *vq,

•               struct iovec iov[], unsigned int iov_size,
  

•               unsigned int *out_num, unsigned int *in_num,
  

•               struct vhost_log *log, unsigned int *log_num)
  

+int vhost_get_vq_desc_n(struct vhost_virtqueue *vq,

•                 struct iovec iov[], unsigned int iov_size,
  

•                 unsigned int *out_num, unsigned int *in_num,
  

•                 struct vhost_log *log, unsigned int *log_num,
  

•                 unsigned int *ndesc)
  

...

{ bool in_order = vhost_has_feature(vq, VIRTIO_F_IN_ORDER); struct vring_desc desc; @@ -2921,16 +2914,40 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq, vq->last_avail_idx++; vq->next_avail_head += c;

• if (ndesc)
  

•         *ndesc = c;
  

• /* Assume notifications from guest are disabled at this point,
   * if they aren't we would need to update avail_event index. */
  BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
  return head;
  

} +EXPORT_SYMBOL_GPL(vhost_get_vq_desc_n);

+/* This looks in the virtqueue and for the first available buffer, and converts

• • it to an iovec for convenient access. Since descriptors consist of some
• • number of output then some number of input descriptors, it's actually two
• • iovecs, but we pack them into one and note how many of each there were.
• • This function returns the descriptor number found, or vq->num (which is
• • never a valid descriptor number) if none was found. A negative code is
• • returned on error.
• */

+int vhost_get_vq_desc(struct vhost_virtqueue *vq,

•               struct iovec iov[], unsigned int iov_size,
  

•               unsigned int *out_num, unsigned int *in_num,
  

•               struct vhost_log *log, unsigned int *log_num)
  

+{

• return vhost_get_vq_desc_n(vq, iov, iov_size, out_num, in_num,
  

•                            log, log_num, NULL);
  

+} EXPORT_SYMBOL_GPL(vhost_get_vq_desc);

/* Reverse the effect of vhost_get_vq_desc. Useful for error handling. */ -void vhost_discard_vq_desc(struct vhost_virtqueue *vq, int n) +void vhost_discard_vq_desc(struct vhost_virtqueue *vq, int n,

•                    unsigned int ndesc)
  

ndesc is number of descriptors? And n is what, in that case?

The semantic of n is not changed which is the number of buffers, ndesc is the number of descriptors.

...

{

• vq->next_avail_head -= ndesc;
  vq->last_avail_idx -= n;
  

} EXPORT_SYMBOL_GPL(vhost_discard_vq_desc); diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 621a6d9a8791..69a39540df3d 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -230,7 +230,14 @@ int vhost_get_vq_desc(struct vhost_virtqueue *, struct iovec iov[], unsigned int iov_size, unsigned int *out_num, unsigned int *in_num, struct vhost_log *log, unsigned int *log_num); -void vhost_discard_vq_desc(struct vhost_virtqueue *, int n);

+int vhost_get_vq_desc_n(struct vhost_virtqueue *vq,

•                 struct iovec iov[], unsigned int iov_size,
  

•                 unsigned int *out_num, unsigned int *in_num,
  

•                 struct vhost_log *log, unsigned int *log_num,
  

•                 unsigned int *ndesc);
  

+void vhost_discard_vq_desc(struct vhost_virtqueue *, int n, unsigned int ndesc);

bool vhost_vq_work_queue(struct vhost_virtqueue *vq, struct vhost_work *work); bool vhost_vq_has_work(struct vhost_virtqueue *vq); -- 2.31.1

Thanks