From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 789511B591 for ; Wed, 19 Dec 2018 13:01:15 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9FEC73C2CE2; Wed, 19 Dec 2018 12:01:14 +0000 (UTC) Received: from [10.36.112.26] (ovpn-112-26.ams2.redhat.com [10.36.112.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 00D264144; Wed, 19 Dec 2018 12:01:12 +0000 (UTC) To: Tiwei Bie Cc: dev@dpdk.org, jfreimann@redhat.com, zhihong.wang@intel.com References: <20181211134804.10318-1-maxime.coquelin@redhat.com> <20181211134804.10318-4-maxime.coquelin@redhat.com> <20181219111818.GA3443@dpdk-tbie.sh.intel.com> From: Maxime Coquelin Message-ID: Date: Wed, 19 Dec 2018 13:01:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <20181219111818.GA3443@dpdk-tbie.sh.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 19 Dec 2018 12:01:14 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 3/3] net/virtio: improve batching in mergeable path X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Dec 2018 12:01:15 -0000 On 12/19/18 12:18 PM, Tiwei Bie wrote: > 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. >> >> Signed-off-by: Maxime Coquelin >> --- >> 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]; >> + dxp->ndescs = 1; >> >> - start_dp = vq->vq_ring.desc; >> - start_dp[idx].addr = >> - VIRTIO_MBUF_ADDR(cookie, vq) + >> - RTE_PKTMBUF_HEADROOM - hw->vtnet_hdr_size; >> - start_dp[idx].len = >> - cookie->buf_len - RTE_PKTMBUF_HEADROOM + hw->vtnet_hdr_size; >> - start_dp[idx].flags = VRING_DESC_F_WRITE; >> - idx = start_dp[idx].next; >> - vq->vq_desc_head_idx = idx; >> - if (vq->vq_desc_head_idx == VQ_RING_DESC_CHAIN_END) >> - vq->vq_desc_tail_idx = idx; >> - vq->vq_free_cnt = (uint16_t)(vq->vq_free_cnt - needed); >> - vq_update_avail_ring(vq, head_idx); >> + start_dp[idx].addr = >> + VIRTIO_MBUF_ADDR(cookie[i], vq) + >> + RTE_PKTMBUF_HEADROOM - hw->vtnet_hdr_size; >> + start_dp[idx].len = >> + cookie[i]->buf_len - RTE_PKTMBUF_HEADROOM + >> + hw->vtnet_hdr_size; >> + start_dp[idx].flags = VRING_DESC_F_WRITE; >> + vq->vq_desc_head_idx = start_dp[idx].next; >> + if (vq->vq_desc_head_idx == VQ_RING_DESC_CHAIN_END) >> + vq->vq_desc_tail_idx = vq->vq_desc_head_idx; > > It's better to break the loop here. Or maybe move > this check out of the loop. Right, I vote for breaking, else we might get an out of bound access. Thanks! >> + vq_update_avail_ring(vq, idx); >> + } >> + >> + vq->vq_free_cnt = (uint16_t)(vq->vq_free_cnt - num); >> >> return 0; >> } > [...] >