patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] vhost: fix dequeue zero copy
@ 2017-04-19  5:26 Yuanhan Liu
  2017-04-19  8:43 ` Yuanhan Liu
  0 siblings, 1 reply; 2+ messages in thread
From: Yuanhan Liu @ 2017-04-19  5:26 UTC (permalink / raw)
  To: dev; +Cc: Maxime Coquelin, Yuanhan Liu, stable

For zero copy mode, we need pin the mbuf to not let the underlaying PMD
driver (or the app) free the mbuf. Currently, only the heading mbuf is
pinned. However, the mbuf free function would try to free all mbufs
in the mbuf chain (-1 to the refcnt). This may lead the head mbuf being
still pinned, while the other subsequent mbufs are actually freed. Which
is wrong.

It becomes more fatal after the mbuf refactor, more specificly, after
the commit 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool"). The
refcnt resets to 1 after the last real reference. OTOH, it leads to a
situtation that we never know one mbuf is actually freed or not. This
would result the mbuf __just__ after the heading mbuf being freed twice:
it's firstly freed (and put back to mempool) when the underlaying PMD
finishes the DMA.  Later, it will then be freed again when vhost unpins
it. Meaning, one mbuf may be returned to the mempool twice, while in
turn, being allocated twice later. Something uncertain may happen then.
For example, the VM2VM case becomes broken.

Fixes: b0a985d1f340 ("vhost: add dequeue zero copy")

Cc: stable@dpdk.org
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/virtio_net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index b9f2168..d3d4424 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -871,6 +871,8 @@ static inline int __attribute__((always_inline))
 					"allocate memory for mbuf.\n");
 				return -1;
 			}
+			if (unlikely(dev->dequeue_zero_copy))
+				rte_mbuf_refcnt_update(cur, 1);
 
 			prev->next = cur;
 			prev->data_len = mbuf_offset;
-- 
1.9.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [dpdk-stable] [PATCH] vhost: fix dequeue zero copy
  2017-04-19  5:26 [dpdk-stable] [PATCH] vhost: fix dequeue zero copy Yuanhan Liu
@ 2017-04-19  8:43 ` Yuanhan Liu
  0 siblings, 0 replies; 2+ messages in thread
From: Yuanhan Liu @ 2017-04-19  8:43 UTC (permalink / raw)
  To: dev; +Cc: Maxime Coquelin, stable

On Wed, Apr 19, 2017 at 01:26:01PM +0800, Yuanhan Liu wrote:
> For zero copy mode, we need pin the mbuf to not let the underlaying PMD
> driver (or the app) free the mbuf. Currently, only the heading mbuf is
> pinned. However, the mbuf free function would try to free all mbufs
> in the mbuf chain (-1 to the refcnt). This may lead the head mbuf being
> still pinned, while the other subsequent mbufs are actually freed. Which
> is wrong.
> 
> It becomes more fatal after the mbuf refactor, more specificly, after
> the commit 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool"). The
> refcnt resets to 1 after the last real reference. OTOH, it leads to a
> situtation that we never know one mbuf is actually freed or not. This
> would result the mbuf __just__ after the heading mbuf being freed twice:
> it's firstly freed (and put back to mempool) when the underlaying PMD
> finishes the DMA.  Later, it will then be freed again when vhost unpins
> it. Meaning, one mbuf may be returned to the mempool twice, while in
> turn, being allocated twice later. Something uncertain may happen then.
> For example, the VM2VM case becomes broken.
> 
> Fixes: b0a985d1f340 ("vhost: add dequeue zero copy")
> 
> Cc: stable@dpdk.org
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Applied to dpdk-next-virtio.

	--yliu

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-04-19  8:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-19  5:26 [dpdk-stable] [PATCH] vhost: fix dequeue zero copy Yuanhan Liu
2017-04-19  8:43 ` Yuanhan Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).