Re: [dpdk-dev] [PATCH v2 3/3] net/virtio: improve batching in mergeable path

[Jens Freimann <jfreimann@redhat.com>]

On Tue, Dec 11, 2018 at 02:48:04PM +0100, Maxime Coquelin wrote:
>This patch improves both descriptors dequeue and refill,
>by using the same bathing strategy as done in in-order path.

s/bathing/batching/

>
>Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>---
> drivers/net/virtio/virtio_rxtx.c | 237 ++++++++++++++++---------------
> 1 file changed, 126 insertions(+), 111 deletions(-)
>
>diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>index ebe5c74b5..59bcac2f7 100644
>--- a/drivers/net/virtio/virtio_rxtx.c
>+++ b/drivers/net/virtio/virtio_rxtx.c
>@@ -267,41 +267,42 @@ virtqueue_enqueue_refill_inorder(struct virtqueue *vq,
> }
>
> static inline int
>-virtqueue_enqueue_recv_refill(struct virtqueue *vq, struct rte_mbuf *cookie)
>+virtqueue_enqueue_recv_refill(struct virtqueue *vq, struct rte_mbuf **cookie,
>+				uint16_t num)
> {
> 	struct vq_desc_extra *dxp;
> 	struct virtio_hw *hw = vq->hw;
>-	struct vring_desc *start_dp;
>-	uint16_t needed = 1;
>-	uint16_t head_idx, idx;
>+	struct vring_desc *start_dp = vq->vq_ring.desc;
>+	uint16_t idx, i;
>
> 	if (unlikely(vq->vq_free_cnt == 0))
> 		return -ENOSPC;
>-	if (unlikely(vq->vq_free_cnt < needed))
>+	if (unlikely(vq->vq_free_cnt < num))
> 		return -EMSGSIZE;
>
>-	head_idx = vq->vq_desc_head_idx;
>-	if (unlikely(head_idx >= vq->vq_nentries))
>+	if (unlikely(vq->vq_desc_head_idx >= vq->vq_nentries))
> 		return -EFAULT;
>
>-	idx = head_idx;
>-	dxp = &vq->vq_descx[idx];
>-	dxp->cookie = (void *)cookie;
>-	dxp->ndescs = needed;
>+	for (i = 0; i < num; i++) {
>+		idx = vq->vq_desc_head_idx;
>+		dxp = &vq->vq_descx[idx];
>+		dxp->cookie = (void *)cookie[i];

I think code is safe as it is, but should we check if cookie actually points to something?

I tested this patch and saw the same performance improvement, so

Tested-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Jens Freimann <jfreimann@redhat.com>

regards,
Jens