Re: [PATCH v3 4/6] vhost-user: Add queue pair parameter throughout the network stack

[Stefano Brivio <sbrivio@redhat.com>]

On Wed,  3 Dec 2025 19:54:32 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> diff --git a/vu_common.c b/vu_common.c
> index b13b7c308fd8..80d9a30f6f71 100644
> --- a/vu_common.c
> +++ b/vu_common.c
> @@ -196,11 +196,11 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
>  
>  		data = IOV_TAIL(elem[count].out_sg, elem[count].out_num, 0);
>  		if (IOV_DROP_HEADER(&data, struct virtio_net_hdr_mrg_rxbuf))
> -			tap_add_packet(vdev->context, &data, now);
> +			tap_add_packet(vdev->context, 0, &data, now);
>  
>  		count++;
>  	}
> -	tap_handler(vdev->context, now);
> +	tap_handler(vdev->context, 0, now);
>  
>  	if (count) {
>  		int i;
> @@ -235,23 +235,26 @@ void vu_kick_cb(struct vu_dev *vdev, union epoll_ref ref,
>  }
>  
>  /**
> - * vu_send_single() - Send a buffer to the front-end using the RX virtqueue
> + * vu_send_single() - Send a buffer to the front-end using a specified virtqueue
>   * @c:		execution context
> + * @qpair:	Queue pair on which to send the buffer
>   * @buf:	address of the buffer
>   * @size:	size of the buffer
>   *
>   * Return: number of bytes sent, -1 if there is an error
>   */
> -int vu_send_single(const struct ctx *c, const void *buf, size_t size)
> +int vu_send_single(const struct ctx *c, unsigned int qpair, const void *buf, size_t size)
>  {
>  	struct vu_dev *vdev = c->vdev;
> -	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
>  	struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
>  	struct iovec in_sg[VIRTQUEUE_MAX_SIZE];
> +	struct vu_virtq *vq;
>  	size_t total;
>  	int elem_cnt;
>  	int i;
>  
> +	vq = &vdev->vq[qpair << 1];

<< 1 instead of * 2 is a bit surprising here, for a few seconds I
thought you swapped qpair and 1.

Then I started thinking that somebody is likely to mix up (probably not
you) indices of RX and TX queues at some point. So... what about some
macros, say (let's see if I got it right this time):

#define VHOST_SEND_QUEUE(pair)	((pair) * 2)
#define VHOST_RECV_QUEUE(pair)	(pair)

and:

#define VHOST_QUEUE_PAIR(q)	((q) % 2) ? (q) : (q) / 2)

...are they correct? A short description or "Theory of operation"
section somewhere with a recap of how queue indices are used would be
nice to have.

And maybe also something explaining that 0 that's now appearing in
argument lists:

#define VHOST_NO_QUEUE		0

?

> +
>  	trace("vu_send_single size %zu", size);
>  
>  	if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) {
> diff --git a/vu_common.h b/vu_common.h
> index f538f237790b..9ceb8034a9a5 100644
> --- a/vu_common.h
> +++ b/vu_common.h
> @@ -56,6 +56,7 @@ void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq,
>  	      struct vu_virtq_element *elem, int elem_cnt);
>  void vu_kick_cb(struct vu_dev *vdev, union epoll_ref ref,
>  		const struct timespec *now);
> -int vu_send_single(const struct ctx *c, const void *buf, size_t size);
> +int vu_send_single(const struct ctx *c, unsigned int qpair, const void *buf,
> +		   size_t size);
>  
>  #endif /* VU_COMMON_H */

I'm still reviewing the rest, currently at 5/6.

-- 
Stefano