From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 724082B86 for ; Tue, 19 Jul 2016 14:32:11 +0200 (CEST) Received: from glumotte.dev.6wind.com (unknown [10.16.0.195]) by proxy.6wind.com (Postfix) with ESMTP id 3E0B427878; Tue, 19 Jul 2016 14:32:11 +0200 (CEST) From: Olivier Matz To: dev@dpdk.org, jianfeng.tan@intel.com, huawei.xie@intel.com, yuanhan.liu@linux.intel.com Date: Tue, 19 Jul 2016 14:31:59 +0200 Message-Id: <1468931519-7381-1-git-send-email-olivier.matz@6wind.com> X-Mailer: git-send-email 2.8.1 Subject: [dpdk-dev] [PATCH] virtio: fix packet corruption X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jul 2016 12:32:11 -0000 The support of virtio-user changed the way the mbuf dma address is retrieved, using a physical address in case of virtio-pci and a virtual address in case of virtio-user. This change introduced some possible memory corruption in packets, replacing: m->buf_physaddr + RTE_PKTMBUF_HEADROOM by: m->buf_physaddr + m->data_off (through a macro) This patch fixes this issue, restoring the original behavior. By the way, it also rework the macros, adding a "VIRTIO_" prefix and API comments. Fixes: f24f8f9fee8a ("net/virtio: allow virtual address to fill vring descriptors") Signed-off-by: Olivier Matz --- drivers/net/virtio/virtio_ethdev.c | 2 +- drivers/net/virtio/virtio_rxtx.c | 5 +++-- drivers/net/virtio/virtio_rxtx_simple.c | 13 +++++++------ drivers/net/virtio/virtqueue.h | 25 +++++++++++++++++-------- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 850e3ba..fcc996e 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -454,7 +454,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, /* For virtio-user case (that is when dev->pci_dev is NULL), we use * virtual address. And we need properly set _offset_, please see - * MBUF_DATA_DMA_ADDR in virtqueue.h for more information. + * VIRTIO_MBUF_DATA_DMA_ADDR in virtqueue.h for more information. */ if (dev->pci_dev) vq->offset = offsetof(struct rte_mbuf, buf_physaddr); diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c index a27208e..9aba044 100644 --- a/drivers/net/virtio/virtio_rxtx.c +++ b/drivers/net/virtio/virtio_rxtx.c @@ -193,7 +193,8 @@ virtqueue_enqueue_recv_refill(struct virtqueue *vq, struct rte_mbuf *cookie) start_dp = vq->vq_ring.desc; start_dp[idx].addr = - MBUF_DATA_DMA_ADDR(cookie, vq->offset) - hw->vtnet_hdr_size; + 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; @@ -265,7 +266,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie, } do { - start_dp[idx].addr = MBUF_DATA_DMA_ADDR(cookie, vq->offset); + start_dp[idx].addr = VIRTIO_MBUF_DATA_DMA_ADDR(cookie, vq); start_dp[idx].len = cookie->data_len; start_dp[idx].flags = cookie->next ? VRING_DESC_F_NEXT : 0; idx = start_dp[idx].next; diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c index d8fcc15..6517aa8 100644 --- a/drivers/net/virtio/virtio_rxtx_simple.c +++ b/drivers/net/virtio/virtio_rxtx_simple.c @@ -80,8 +80,9 @@ virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq, vq->sw_ring[desc_idx] = cookie; start_dp = vq->vq_ring.desc; - start_dp[desc_idx].addr = MBUF_DATA_DMA_ADDR(cookie, vq->offset) - - vq->hw->vtnet_hdr_size; + 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; @@ -120,8 +121,8 @@ virtio_rxq_rearm_vec(struct virtnet_rx *rxvq) *(uint64_t *)p = rxvq->mbuf_initializer; start_dp[i].addr = - MBUF_DATA_DMA_ADDR(sw_ring[i], vq->offset) - - vq->hw->vtnet_hdr_size; + VIRTIO_MBUF_ADDR(sw_ring[i], vq) + + RTE_PKTMBUF_HEADROOM - vq->hw->vtnet_hdr_size; start_dp[i].len = sw_ring[i]->buf_len - RTE_PKTMBUF_HEADROOM + vq->hw->vtnet_hdr_size; } @@ -371,7 +372,7 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts, vq->vq_descx[desc_idx + i].cookie = tx_pkts[i]; for (i = 0; i < nb_tail; i++) { start_dp[desc_idx].addr = - MBUF_DATA_DMA_ADDR(*tx_pkts, vq->offset); + VIRTIO_MBUF_DATA_DMA_ADDR(*tx_pkts, vq); start_dp[desc_idx].len = (*tx_pkts)->pkt_len; tx_pkts++; desc_idx++; @@ -383,7 +384,7 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts, vq->vq_descx[desc_idx + i].cookie = tx_pkts[i]; for (i = 0; i < nb_commit; i++) { start_dp[desc_idx].addr = - MBUF_DATA_DMA_ADDR(*tx_pkts, vq->offset); + VIRTIO_MBUF_DATA_DMA_ADDR(*tx_pkts, vq); start_dp[desc_idx].len = (*tx_pkts)->pkt_len; tx_pkts++; desc_idx++; diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h index 455aaaf..c452d04 100644 --- a/drivers/net/virtio/virtqueue.h +++ b/drivers/net/virtio/virtqueue.h @@ -67,12 +67,21 @@ struct rte_mbuf; #define VIRTQUEUE_MAX_NAME_SZ 32 #ifdef RTE_VIRTIO_USER -#define MBUF_DATA_DMA_ADDR(mb, offset) \ - ((uint64_t)((uintptr_t)(*(void **)((uintptr_t)mb + offset)) \ - + (mb)->data_off)) -#else /* RTE_VIRTIO_USER */ -#define MBUF_DATA_DMA_ADDR(mb, offset) rte_mbuf_data_dma_addr(mb) -#endif /* RTE_VIRTIO_USER */ +/** + * Return the physical address (or virtual address in case of + * virtio-user) of mbuf data buffer. + */ +#define VIRTIO_MBUF_ADDR(mb, vq) (*(uint64_t *)((uintptr_t)(mb) + (vq)->offset)) +#else +#define VIRTIO_MBUF_ADDR(mb, vq) ((mb)->buf_physaddr) +#endif + +/** + * Return the physical address (or virtual address in case of + * virtio-user) of mbuf data buffer, taking care of mbuf data offset + */ +#define VIRTIO_MBUF_DATA_DMA_ADDR(mb, vq) \ + (VIRTIO_MBUF_ADDR(mb, vq) + (mb)->data_off) #define VTNET_SQ_RQ_QUEUE_IDX 0 #define VTNET_SQ_TQ_QUEUE_IDX 1 @@ -182,8 +191,8 @@ struct virtqueue { void *vq_ring_virt_mem; /**< linear address of vring*/ unsigned int vq_ring_size; - phys_addr_t vq_ring_mem; /**< physical address of vring */ - /**< use virtual address for virtio-user. */ + phys_addr_t vq_ring_mem; /**< physical address of vring, + * or virtual address for virtio-user. */ /** * Head of the free chain in the descriptor table. If -- 2.8.1