[dpdk-stable] patch 'vhost: fix external mbuf creation' has been queued to stable release 19.11.6

[luca.boccassi at gmail.com]

Hi,

FYI, your patch has been queued to stable release 19.11.6

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 10/30/20. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Thanks.

Luca Boccassi

---
>From 04acc850841d45a8d0bd2def14387b3f229af3a4 Mon Sep 17 00:00:00 2001
From: Olivier Matz <olivier.matz at 6wind.com>
Date: Wed, 7 Oct 2020 14:53:18 +0200
Subject: [PATCH] vhost: fix external mbuf creation

[ upstream commit fa5054c4bb994311278d4a99cccf09be69296177 ]

In virtio_dev_extbuf_alloc(), the shinfo structure used to store
the reference counter and the free callback of the external buffer
is by default stored inside the mbuf data.

This is wrong because the mbuf (and its data) can be freed before
the external buffer, for instance in the following situation:

  pkt2 = rte_pktmbuf_alloc(mp);
  rte_pktmbuf_attach(pkt2, pkt);
  rte_pktmbuf_free(pkt);

After this, pkt is freed, but it still contains shinfo, which is
referenced by pkt2.

Fix this by always storing the shinfo beside the external buffer.

Fixes: c3ff0ac70acb ("vhost: improve performance by supporting large buffer")

Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
---
 lib/librte_vhost/virtio_net.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index a6c106c13c..b8237f2eb4 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -1595,16 +1595,8 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)
 	rte_iova_t iova;
 	void *buf;
 
-	/* Try to use pkt buffer to store shinfo to reduce the amount of memory
-	 * required, otherwise store shinfo in the new buffer.
-	 */
-	if (rte_pktmbuf_tailroom(pkt) >= sizeof(*shinfo))
-		shinfo = rte_pktmbuf_mtod(pkt,
-					  struct rte_mbuf_ext_shared_info *);
-	else {
-		total_len += sizeof(*shinfo) + sizeof(uintptr_t);
-		total_len = RTE_ALIGN_CEIL(total_len, sizeof(uintptr_t));
-	}
+	total_len += sizeof(*shinfo) + sizeof(uintptr_t);
+	total_len = RTE_ALIGN_CEIL(total_len, sizeof(uintptr_t));
 
 	if (unlikely(total_len > UINT16_MAX))
 		return -ENOSPC;
@@ -1615,18 +1607,12 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)
 		return -ENOMEM;
 
 	/* Initialize shinfo */
-	if (shinfo) {
-		shinfo->free_cb = virtio_dev_extbuf_free;
-		shinfo->fcb_opaque = buf;
-		rte_mbuf_ext_refcnt_set(shinfo, 1);
-	} else {
-		shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len,
-					      virtio_dev_extbuf_free, buf);
-		if (unlikely(shinfo == NULL)) {
-			rte_free(buf);
-			RTE_LOG(ERR, VHOST_DATA, "Failed to init shinfo\n");
-			return -1;
-		}
+	shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len,
+						virtio_dev_extbuf_free, buf);
+	if (unlikely(shinfo == NULL)) {
+		rte_free(buf);
+		RTE_LOG(ERR, VHOST_DATA, "Failed to init shinfo\n");
+		return -1;
 	}
 
 	iova = rte_malloc_virt2iova(buf);
-- 
2.20.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2020-10-28 10:35:16.726942443 +0000
+++ 0161-vhost-fix-external-mbuf-creation.patch	2020-10-28 10:35:11.760833792 +0000
@@ -1,8 +1,10 @@
-From fa5054c4bb994311278d4a99cccf09be69296177 Mon Sep 17 00:00:00 2001
+From 04acc850841d45a8d0bd2def14387b3f229af3a4 Mon Sep 17 00:00:00 2001
 From: Olivier Matz <olivier.matz at 6wind.com>
 Date: Wed, 7 Oct 2020 14:53:18 +0200
 Subject: [PATCH] vhost: fix external mbuf creation
 
+[ upstream commit fa5054c4bb994311278d4a99cccf09be69296177 ]
+
 In virtio_dev_extbuf_alloc(), the shinfo structure used to store
 the reference counter and the free callback of the external buffer
 is by default stored inside the mbuf data.
@@ -20,7 +22,6 @@
 Fix this by always storing the shinfo beside the external buffer.
 
 Fixes: c3ff0ac70acb ("vhost: improve performance by supporting large buffer")
-Cc: stable at dpdk.org
 
 Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
 Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
@@ -29,10 +30,10 @@
  1 file changed, 8 insertions(+), 22 deletions(-)
 
 diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
-index 0a0bea1a5a..008f5ceb04 100644
+index a6c106c13c..b8237f2eb4 100644
 --- a/lib/librte_vhost/virtio_net.c
 +++ b/lib/librte_vhost/virtio_net.c
-@@ -2098,16 +2098,8 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)
+@@ -1595,16 +1595,8 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)
  	rte_iova_t iova;
  	void *buf;
  
@@ -51,7 +52,7 @@
  
  	if (unlikely(total_len > UINT16_MAX))
  		return -ENOSPC;
-@@ -2118,18 +2110,12 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)
+@@ -1615,18 +1607,12 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)
  		return -ENOMEM;
  
  	/* Initialize shinfo */
@@ -64,14 +65,14 @@
 -					      virtio_dev_extbuf_free, buf);
 -		if (unlikely(shinfo == NULL)) {
 -			rte_free(buf);
--			VHOST_LOG_DATA(ERR, "Failed to init shinfo\n");
+-			RTE_LOG(ERR, VHOST_DATA, "Failed to init shinfo\n");
 -			return -1;
 -		}
 +	shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len,
 +						virtio_dev_extbuf_free, buf);
 +	if (unlikely(shinfo == NULL)) {
 +		rte_free(buf);
-+		VHOST_LOG_DATA(ERR, "Failed to init shinfo\n");
++		RTE_LOG(ERR, VHOST_DATA, "Failed to init shinfo\n");
 +		return -1;
  	}