From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 318151DA4 for ; Fri, 8 Mar 2019 11:48:50 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Mar 2019 02:48:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,455,1544515200"; d="scan'208";a="280905151" Received: from dpdk-tbie.sh.intel.com ([10.67.104.173]) by orsmga004.jf.intel.com with ESMTP; 08 Mar 2019 02:48:48 -0800 From: Tiwei Bie To: yskoh@mellanox.com, stable@dpdk.org Cc: maxime.coquelin@redhat.com, zhihong.wang@intel.com, Olivier Matz Date: Fri, 8 Mar 2019 18:48:05 +0800 Message-Id: <20190308104805.30001-1-tiwei.bie@intel.com> X-Mailer: git-send-email 2.17.1 Subject: [dpdk-stable] [PATCH 17.11] net/virtio: fix resuming port with Rx vector path X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Mar 2019 10:48:51 -0000 [ backported from upstream commit 478574706638ff78cbc7e82731a4eae743322ac6 ] 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") Signed-off-by: Maxime Coquelin Signed-off-by: Olivier Matz Signed-off-by: Tiwei Bie Reviewed-by: Jianfeng Tan --- Hi Yongseok, This fix patch is backported from v18.02, and targets for v17.11.6. It fixes a port resuming issue for Rx vector path in virtio PMD. Thanks, Tiwei drivers/net/virtio/virtio_rxtx.c | 38 ++++++++++++++----------- drivers/net/virtio/virtio_rxtx.h | 3 -- drivers/net/virtio/virtio_rxtx_simple.c | 30 +------------------ drivers/net/virtio/virtio_rxtx_simple.h | 2 +- 4 files changed, 23 insertions(+), 50 deletions(-) diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c index 390c137c8..275b65f43 100644 --- a/drivers/net/virtio/virtio_rxtx.c +++ b/drivers/net/virtio/virtio_rxtx.c @@ -59,6 +59,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) @@ -465,6 +466,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)); @@ -474,30 +477,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 54f1e849b..4143551a1 100644 --- a/drivers/net/virtio/virtio_rxtx.h +++ b/drivers/net/virtio/virtio_rxtx.h @@ -88,7 +88,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 b5bc1c49f..86737e416 100644 --- a/drivers/net/virtio/virtio_rxtx_simple.c +++ b/drivers/net/virtio/virtio_rxtx_simple.c @@ -56,34 +56,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; - - 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) @@ -106,7 +78,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 f531c5428..5f6dabc8a 100644 --- a/drivers/net/virtio/virtio_rxtx_simple.h +++ b/drivers/net/virtio/virtio_rxtx_simple.h @@ -89,7 +89,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.17.1