From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id 504F11B374; Mon, 12 Feb 2018 16:46:43 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D6DCC40A0971; Mon, 12 Feb 2018 15:46:42 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-42.ams2.redhat.com [10.36.112.42]) by smtp.corp.redhat.com (Postfix) with ESMTP id 47B022026DFD; Mon, 12 Feb 2018 15:46:40 +0000 (UTC) From: Maxime Coquelin To: tiwei.bie@intel.com, yliu@fridaylinux.org, ferruh.yigit@intel.com, victork@redhat.com, thomas@monjalon.net, olivier.matz@6wind.com, jianfeng.tan@intel.com Cc: dev@dpdk.org, stable@dpdk.org, zhihong.wang@intel.com, qian.q.xu@intel.com, lei.a.yao@intel.com, Maxime Coquelin Date: Mon, 12 Feb 2018 16:46:11 +0100 Message-Id: <20180212154612.5297-3-maxime.coquelin@redhat.com> In-Reply-To: <20180212154612.5297-1-maxime.coquelin@redhat.com> References: <20180212154612.5297-1-maxime.coquelin@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Mon, 12 Feb 2018 15:46:42 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Mon, 12 Feb 2018 15:46:42 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:'' Subject: [dpdk-dev] [PATCH v2 2/3] virtio: fix resuming port with rx vector 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: Mon, 12 Feb 2018 15:46:43 -0000 Since commit efc83a1e7fc3 ("net/virtio: fix queue setup consistency"), when resuming a virtio port, the rx rings are refilled with new mbufs until they are full (vq->vq_free_cnt == 0). This is done without ensuring that the descriptor index remains a multiple of RTE_VIRTIO_VPMD_RX_REARM_THRESH, which is a prerequisite when using the vector mode. This can cause an out of bound access in the rx ring. This commit changes the vector refill method from virtqueue_enqueue_recv_refill_simple() to virtio_rxq_rearm_vec(), which properly checks that the refill is done by batch of RTE_VIRTIO_VPMD_RX_REARM_THRESH. As virtqueue_enqueue_recv_refill_simple() is no more used, this patch also removes the function. Fixes: efc83a1e7fc3 ("net/virtio: fix queue setup consistency") Reviewed-by: Jianfeng Tan Signed-off-by: Tiwei Bie Signed-off-by: Olivier Matz Signed-off-by: Maxime Coquelin --- drivers/net/virtio/virtio_rxtx.c | 38 ++++++++++++++++++--------------- drivers/net/virtio/virtio_rxtx.h | 3 --- drivers/net/virtio/virtio_rxtx_simple.c | 31 +-------------------------- drivers/net/virtio/virtio_rxtx_simple.h | 2 +- 4 files changed, 23 insertions(+), 51 deletions(-) diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c index 854af399e..8dbf2a30e 100644 --- a/drivers/net/virtio/virtio_rxtx.c +++ b/drivers/net/virtio/virtio_rxtx.c @@ -30,6 +30,7 @@ #include "virtio_pci.h" #include "virtqueue.h" #include "virtio_rxtx.h" +#include "virtio_rxtx_simple.h" #ifdef RTE_LIBRTE_VIRTIO_DEBUG_DUMP #define VIRTIO_DUMP_PACKET(m, len) rte_pktmbuf_dump(stdout, m, len) @@ -437,6 +438,8 @@ virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, uint16_t queue_idx) vq->vq_ring.desc[desc_idx].flags = VRING_DESC_F_WRITE; } + + virtio_rxq_vec_setup(rxvq); } memset(&rxvq->fake_mbuf, 0, sizeof(rxvq->fake_mbuf)); @@ -446,30 +449,31 @@ virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, uint16_t queue_idx) &rxvq->fake_mbuf; } - while (!virtqueue_full(vq)) { - m = rte_mbuf_raw_alloc(rxvq->mpool); - if (m == NULL) - break; + if (hw->use_simple_rx) { + while (vq->vq_free_cnt >= RTE_VIRTIO_VPMD_RX_REARM_THRESH) { + virtio_rxq_rearm_vec(rxvq); + nbufs += RTE_VIRTIO_VPMD_RX_REARM_THRESH; + } + } else { + while (!virtqueue_full(vq)) { + m = rte_mbuf_raw_alloc(rxvq->mpool); + if (m == NULL) + break; - /* Enqueue allocated buffers */ - if (hw->use_simple_rx) - error = virtqueue_enqueue_recv_refill_simple(vq, m); - else + /* Enqueue allocated buffers */ error = virtqueue_enqueue_recv_refill(vq, m); - - if (error) { - rte_pktmbuf_free(m); - break; + if (error) { + rte_pktmbuf_free(m); + break; + } + nbufs++; } - nbufs++; - } - vq_update_avail_idx(vq); + vq_update_avail_idx(vq); + } PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs); - virtio_rxq_vec_setup(rxvq); - VIRTQUEUE_DUMP(vq); return 0; diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h index 49e9d98ee..685cc4f81 100644 --- a/drivers/net/virtio/virtio_rxtx.h +++ b/drivers/net/virtio/virtio_rxtx.h @@ -60,7 +60,4 @@ struct virtnet_ctl { int virtio_rxq_vec_setup(struct virtnet_rx *rxvq); -int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq, - struct rte_mbuf *m); - #endif /* _VIRTIO_RXTX_H_ */ diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c index b1f610ffa..515207581 100644 --- a/drivers/net/virtio/virtio_rxtx_simple.c +++ b/drivers/net/virtio/virtio_rxtx_simple.c @@ -27,35 +27,6 @@ #pragma GCC diagnostic ignored "-Wcast-qual" #endif -int __attribute__((cold)) -virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq, - struct rte_mbuf *cookie) -{ - struct vq_desc_extra *dxp; - struct vring_desc *start_dp; - uint16_t desc_idx; - - cookie->port = vq->rxq.port_id; - cookie->data_off = RTE_PKTMBUF_HEADROOM; - - desc_idx = vq->vq_avail_idx & (vq->vq_nentries - 1); - dxp = &vq->vq_descx[desc_idx]; - dxp->cookie = (void *)cookie; - vq->sw_ring[desc_idx] = cookie; - - start_dp = vq->vq_ring.desc; - start_dp[desc_idx].addr = - VIRTIO_MBUF_ADDR(cookie, vq) + - RTE_PKTMBUF_HEADROOM - vq->hw->vtnet_hdr_size; - start_dp[desc_idx].len = cookie->buf_len - - RTE_PKTMBUF_HEADROOM + vq->hw->vtnet_hdr_size; - - vq->vq_free_cnt--; - vq->vq_avail_idx++; - - return 0; -} - uint16_t virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) @@ -78,7 +49,7 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts, rte_compiler_barrier(); if (nb_used >= VIRTIO_TX_FREE_THRESH) - virtio_xmit_cleanup(vq); + virtio_xmit_cleanup_simple(vq); nb_commit = nb_pkts = RTE_MIN((vq->vq_free_cnt >> 1), nb_pkts); desc_idx = (uint16_t)(vq->vq_avail_idx & desc_idx_max); diff --git a/drivers/net/virtio/virtio_rxtx_simple.h b/drivers/net/virtio/virtio_rxtx_simple.h index 2d8e6b14a..303904d64 100644 --- a/drivers/net/virtio/virtio_rxtx_simple.h +++ b/drivers/net/virtio/virtio_rxtx_simple.h @@ -60,7 +60,7 @@ virtio_rxq_rearm_vec(struct virtnet_rx *rxvq) #define VIRTIO_TX_FREE_NR 32 /* TODO: vq->tx_free_cnt could mean num of free slots so we could avoid shift */ static inline void -virtio_xmit_cleanup(struct virtqueue *vq) +virtio_xmit_cleanup_simple(struct virtqueue *vq) { uint16_t i, desc_idx; uint32_t nb_free = 0; -- 2.14.3