From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4D6DFA04DD for ; Wed, 28 Oct 2020 11:54:49 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4086FCA3E; Wed, 28 Oct 2020 11:54:48 +0100 (CET) Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by dpdk.org (Postfix) with ESMTP id 27311CA4D for ; Wed, 28 Oct 2020 11:54:46 +0100 (CET) Received: by mail-wr1-f65.google.com with SMTP id g12so5138515wrp.10 for ; Wed, 28 Oct 2020 03:54:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=iBHkWf5W5xDJrMz2eKU3PRL8e1UZUopdrkaq5Q3wQHg=; b=HuTVYIvr+tJ8Sr+3eP6VW79wuQRAglYt6MtGo9na2B4kJHMPwnrBFZPmHBK3S2wZ0j OarcWUYJmZWlFYjnA3TOH9nnna1xppt9yV3OJfAvetAekv45gYKIm1qFCVZ2ls+/D+iz snFKTEzUCAOUXilcVkx+B8kP6AhR7DkaqT3D3p6YUgphNpk2y8nfuLlod+pX1fEcbiLY O3TnIHxYayIGFazCFCewCOzhApbxO49Y7e4vCd3e93d3dd0kmdYC/ccEJrB17JCogtRL fn/dpG3FTHXnUKOUcrOc41BoPcHTqoaTga2TjZaPh4Ez4jiZOMLYk0O9LH4840DXscP4 krTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=iBHkWf5W5xDJrMz2eKU3PRL8e1UZUopdrkaq5Q3wQHg=; b=XGdAfUkIv/RdT5vf3VIGtEaiZWajH/7HKE7ca4Z6q6rwtQQDN/Tyge2Gvw2Oy9tNrx pn+nd1ZEF5TxvuMfjexhlUYuz2R6ZF5+/JWI/V66lkXX9Ir4oR9RQkEKauIYyhSivcBc SDdF9qYh4On6LugbRmZnOu/kUeINpQ7AXjlUZU3a6xHT9cf7+Mrumrw2ApDlVIlPXc7M /3tzm7uymi+ZxnPdK0V6M9e0TgMDDePPzUMRq8VobUK9H6gsxFJZWY6es0PdILNjPkZv 4TVwJ9wZMzKucb7YQNJoDEmll0nZkEn/tA/qrb4DmAiu80Sj0NftPSGl4Xhi7BdyLmqB OvxQ== X-Gm-Message-State: AOAM53251COvmN15oCU03nMUdgZBWv+eC6FfxlQV4Z0aZBK7IzIfx652 TsON+xM7cTVip0F2MBeu/5/nkioHb3/fpfX6 X-Google-Smtp-Source: ABdhPJxxHiemG1MG6ld+/JpEbus5sjz9mo0WRA7WJNOVMyOJh6OlRsrRvci78NLMtKARnN8s72hgxg== X-Received: by 2002:a5d:4409:: with SMTP id z9mr8047067wrq.236.1603882485901; Wed, 28 Oct 2020 03:54:45 -0700 (PDT) Received: from localhost ([88.98.246.218]) by smtp.gmail.com with ESMTPSA id n4sm4107027wmi.32.2020.10.28.03.54.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Oct 2020 03:54:45 -0700 (PDT) From: luca.boccassi@gmail.com To: Olivier Matz Cc: Maxime Coquelin , dpdk stable Date: Wed, 28 Oct 2020 10:45:20 +0000 Message-Id: <20201028104606.3504127-161-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20201028104606.3504127-1-luca.boccassi@gmail.com> References: <20201028104606.3504127-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'vhost: fix external mbuf creation' has been queued to stable release 19.11.6 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: , Errors-To: stable-bounces@dpdk.org Sender: "stable" 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 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 Reviewed-by: Maxime Coquelin --- 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 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@dpdk.org Signed-off-by: Olivier Matz Reviewed-by: Maxime Coquelin @@ -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; }