From patchwork Thu Jul 30 12:08:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: yang_y_yi X-Patchwork-Id: 75036 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DFEFEA052B; Thu, 30 Jul 2020 14:09:16 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1D1862BC7; Thu, 30 Jul 2020 14:09:08 +0200 (CEST) Received: from mail-m974.mail.163.com (mail-m974.mail.163.com [123.126.97.4]) by dpdk.org (Postfix) with ESMTP id 7B7871023 for ; Thu, 30 Jul 2020 14:09:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:Subject:Date:Message-Id:MIME-Version; bh=Z0xEf AcIlG7UazUOoxO0z8+F9G+0Lb/6Ky1JI6Ak4OA=; b=iYx6B9EoNPcrE4+G7nHf0 /0znqkPXhl/eYXTCpHLUBa20+cq0h47L99zCWSvKrLQYyfHMgqGPXcywbCBVu27R H5TgMzP3BrI6TSNYpFVRvEAKBnM36IOhIHjTKoFjrX136Is3l2jTd9dxOk0gRzBQ 1wZwsHCT2ONicWtY5cPK/k= Received: from yangyi0100.home.langchao.com (unknown [61.48.210.155]) by smtp4 (Coremail) with SMTP id HNxpCgBXekNcuCJfZo1vIA--.3698S3; Thu, 30 Jul 2020 20:09:02 +0800 (CST) From: yang_y_yi@163.com To: dev@dpdk.org Cc: jiayu.hu@intel.com, thomas@monjalon.net, yangyi01@inspur.com, yang_y_yi@163.com Date: Thu, 30 Jul 2020 20:08:58 +0800 Message-Id: <20200730120900.108232-2-yang_y_yi@163.com> X-Mailer: git-send-email 2.19.2.windows.1 In-Reply-To: <20200730120900.108232-1-yang_y_yi@163.com> References: <20200730120900.108232-1-yang_y_yi@163.com> MIME-Version: 1.0 X-CM-TRANSID: HNxpCgBXekNcuCJfZo1vIA--.3698S3 X-Coremail-Antispam: 1Uf129KBjvdXoW7JF1rJrWruF1UGF4fAFyxZrb_yoWDKFc_ua 4FkF1ktay7Jr4xZw43Jw4Yga1xurW7Aa18Ww4Igw45Xr4qgFsxu34DJr42qFWDu3yUGFWx WF43uFn2yr1FvjkaLaAFLSUrUUUUUb8apTn2vfkv8UJUUUU8Yxn0WfASr-VFAUDa7-sFnT 9fnUUvcSsGvfC2KfnxnUUI43ZEXa7IU8wSdDUUUUU== X-Originating-IP: [61.48.210.155] X-CM-SenderInfo: 51dqwsp1b1xqqrwthudrp/xtbB0h5xi1UMX9eJ+wAAsV Subject: [dpdk-dev] [PATCH V1 1/3] gso: fix refcnt update issue for external mbuf X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Yi Yang rte_gso_segment will segment original mbuf to multiple small size mbufs, every mbuf of them has two segments, the second segment will attach to original mbuf, if original mbuf is external, rte_gso_segment should update refcnt of mbuf->shinfo but not refcnt of mbuf. Otherwise, original mbuf will be invalid on doing sanity check because refcnt of mbuf is 0. Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO") Signed-off-by: Yi Yang --- lib/librte_gso/rte_gso.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.c index 751b5b6..b5e8b2a 100644 --- a/lib/librte_gso/rte_gso.c +++ b/lib/librte_gso/rte_gso.c @@ -83,7 +83,10 @@ if (ret > 1) { pkt_seg = pkt; while (pkt_seg) { - rte_mbuf_refcnt_update(pkt_seg, -1); + if (RTE_MBUF_HAS_EXTBUF(pkt_seg)) + rte_mbuf_ext_refcnt_update(pkt_seg->shinfo, -1); + else + rte_mbuf_refcnt_update(pkt_seg, -1); pkt_seg = pkt_seg->next; } } else if (ret < 0) { From patchwork Thu Jul 30 12:08:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: yang_y_yi X-Patchwork-Id: 75035 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id D65B2A052B; Thu, 30 Jul 2020 14:09:07 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AA172E07; Thu, 30 Jul 2020 14:09:06 +0200 (CEST) Received: from mail-m974.mail.163.com (mail-m974.mail.163.com [123.126.97.4]) by dpdk.org (Postfix) with ESMTP id 684F7255 for ; Thu, 30 Jul 2020 14:09:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:Subject:Date:Message-Id:MIME-Version; bh=QmRC5 GXh3xemHC43lm06RloUCxVu8+cRnAdujpGeuoc=; b=HnIFCRw/avrWWeKMcI8KO 5zmuHpX/SjOrhFQaLUq5/ByxiToL8rq6xizh3tDkar9jBzm4Cyqtt6i62ie9Ien9 lYsJEgpcFY2HNfSGd9uPo9hqEFp2KQ1eW5v57814hECeXt7WMaD6JI1E4U66UpLH P8pz48k5TT15y/5sNgkx9E= Received: from yangyi0100.home.langchao.com (unknown [61.48.210.155]) by smtp4 (Coremail) with SMTP id HNxpCgBXekNcuCJfZo1vIA--.3698S4; Thu, 30 Jul 2020 20:09:02 +0800 (CST) From: yang_y_yi@163.com To: dev@dpdk.org Cc: jiayu.hu@intel.com, thomas@monjalon.net, yangyi01@inspur.com, yang_y_yi@163.com Date: Thu, 30 Jul 2020 20:08:59 +0800 Message-Id: <20200730120900.108232-3-yang_y_yi@163.com> X-Mailer: git-send-email 2.19.2.windows.1 In-Reply-To: <20200730120900.108232-1-yang_y_yi@163.com> References: <20200730120900.108232-1-yang_y_yi@163.com> MIME-Version: 1.0 X-CM-TRANSID: HNxpCgBXekNcuCJfZo1vIA--.3698S4 X-Coremail-Antispam: 1Uf129KBjvJXoW3Ww1fCr18CFy5KFyfuF47XFb_yoW3Cw4fpF srCryjkrs8JF48uFs7Jr4Sqrn3KFWkKay7KrWkJ34F9F1YqryvqFWFka48Zr1YgrWkCrZF vF4kJa47Ga18CwUanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jsManUUUUU= X-Originating-IP: [61.48.210.155] X-CM-SenderInfo: 51dqwsp1b1xqqrwthudrp/1tbiqBBxi1c7RBg7zAAAsF Subject: [dpdk-dev] [PATCH V1 2/3] mbuf: change free_cb interface to adapt to GSO case X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Yi Yang In GSO case, segmented mbufs are attached to original mbuf which can't be freed when it is external. The issue is free_cb doesn't know original mbuf and doesn't free it when refcnt of shinfo is 0. Original mbuf can be freed by rte_pktmbuf_free segmented mbufs or by rte_pktmbuf_free original mbuf. Two kind of cases should have different behaviors. free_cb won't explicitly call rte_pktmbuf_free to free original mbuf if it is freed by rte_pktmbuf_free original mbuf, but it has to call rte_pktmbuf_free to free original mbuf if it is freed by rte_pktmbuf_free segmented mbufs. In order to fix this issue, free_cb interface has to been changed, __rte_pktmbuf_free_extbuf must deliver called mbuf pointer to free_cb, argument opaque can be defined as a custom struct by user, it can includes original mbuf pointer, user-defined free_cb can compare caller mbuf with mbuf in opaque struct, free_cb should free original mbuf if they are not same, this corresponds to rte_pktmbuf_free segmented mbufs case, otherwise, free_cb won't free original mbuf because the caller explicitly called rte_pktmbuf_free to free it. Here is pseduo code to show two kind of cases. case 1. rte_pktmbuf_free segmented mbufs nb_tx = rte_gso_segment(original_mbuf, /* original mbuf */ &gso_ctx, /* segmented mbuf */ (struct rte_mbuf **)&gso_mbufs, MAX_GSO_MBUFS); rte_eth_tx_burst(dev->port_id, qid, gso_mbufs, nb_tx); case 2. rte_pktmbuf_free original mbuf rte_eth_tx_burst(dev->port_id, qid, original_mbuf, 1); Here are user defined free_cb and opaque. struct shinfo_arg { void *buf; struct rte_mbuf *mbuf; }; custom_free_cb(struct rte_mbuf *caller_m, void *opaque) { struct shinfo_arg *arg = (struct shinfo_arg *)opaque; rte_free(arg->buf); if (caller_m != arg->mbuf) rte_pktmbuf_free(arg->mbuf); rte_free(arg); } struct shinfo_arg *arg = (struct shinfo_arg *) rte_malloc(NULL, sizeof(struct shinfo_arg), RTE_CACHE_LINE_SIZE); arg->buf = buf; arg->mbuf = pkt; shinfo->free_cb = custom_free_cb; shinfo->fcb_opaque = arg; Signed-off-by: Yi Yang --- app/test-compress-perf/comp_perf_test_common.c | 2 +- app/test/test_compressdev.c | 2 +- app/test/test_mbuf.c | 2 +- drivers/net/mlx5/mlx5_rxtx.c | 2 +- drivers/net/mlx5/mlx5_rxtx.h | 2 +- drivers/net/netvsc/hn_rxtx.c | 3 ++- lib/librte_mbuf/rte_mbuf.c | 4 ++-- lib/librte_mbuf/rte_mbuf.h | 2 +- lib/librte_mbuf/rte_mbuf_core.h | 2 +- lib/librte_vhost/virtio_net.c | 2 +- 10 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/test-compress-perf/comp_perf_test_common.c b/app/test-compress-perf/comp_perf_test_common.c index b402a0d..b1eb733 100644 --- a/app/test-compress-perf/comp_perf_test_common.c +++ b/app/test-compress-perf/comp_perf_test_common.c @@ -115,7 +115,7 @@ struct cperf_buffer_info { } static void -comp_perf_extbuf_free_cb(void *addr __rte_unused, void *opaque __rte_unused) +comp_perf_extbuf_free_cb(struct rte_mbuf *m __rte_unused, void *opaque __rte_unused) { } diff --git a/app/test/test_compressdev.c b/app/test/test_compressdev.c index 0571c17..a4a5d7c 100644 --- a/app/test/test_compressdev.c +++ b/app/test/test_compressdev.c @@ -778,7 +778,7 @@ struct test_private_arrays { } static void -extbuf_free_callback(void *addr __rte_unused, void *opaque __rte_unused) +extbuf_free_callback(struct rte_mbuf *m __rte_unused, void *opaque __rte_unused) { } diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index 06e44f0..f9e5ca5 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -2300,7 +2300,7 @@ struct test_case { /* Define a free call back function to be used for external buffer */ static void -ext_buf_free_callback_fn(void *addr __rte_unused, void *opaque) +ext_buf_free_callback_fn(struct rte_mbuf *caller_m __rte_unused, void *opaque) { void *ext_buf_addr = opaque; diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index 3eb0243..f084488 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1622,7 +1622,7 @@ enum mlx5_txcmp_code { } void -mlx5_mprq_buf_free_cb(void *addr __rte_unused, void *opaque) +mlx5_mprq_buf_free_cb(struct rte_mbuf *caller_m __rte_unused, void *opaque) { struct mlx5_mprq_buf *buf = opaque; diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h index c02a007..8c230c6 100644 --- a/drivers/net/mlx5/mlx5_rxtx.h +++ b/drivers/net/mlx5/mlx5_rxtx.h @@ -499,7 +499,7 @@ struct mlx5_txq_ctrl *mlx5_txq_hairpin_new uint16_t mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n); void mlx5_rxq_initialize(struct mlx5_rxq_data *rxq); __rte_noinline int mlx5_rx_err_handle(struct mlx5_rxq_data *rxq, uint8_t vec); -void mlx5_mprq_buf_free_cb(void *addr, void *opaque); +void mlx5_mprq_buf_free_cb(struct rte_mbuf *caller_m, void *opaque); void mlx5_mprq_buf_free(struct mlx5_mprq_buf *buf); uint16_t mlx5_rx_burst_mprq(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n); diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index 86a4c0d..30af404 100644 --- a/drivers/net/netvsc/hn_rxtx.c +++ b/drivers/net/netvsc/hn_rxtx.c @@ -519,7 +519,8 @@ int hn_dev_tx_descriptor_status(void *arg, uint16_t offset) return 0; } -static void hn_rx_buf_free_cb(void *buf __rte_unused, void *opaque) +static void hn_rx_buf_free_cb(struct rte_mbuf *caller_m __rte_unused, + void *opaque) { struct hn_rx_bufinfo *rxb = opaque; struct hn_data *hv = rxb->hv; diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index 8a456e5..52445b3 100644 --- a/lib/librte_mbuf/rte_mbuf.c +++ b/lib/librte_mbuf/rte_mbuf.c @@ -118,11 +118,11 @@ * buffer. */ static void -rte_pktmbuf_free_pinned_extmem(void *addr, void *opaque) +rte_pktmbuf_free_pinned_extmem(struct rte_mbuf *caller_m, void *opaque) { struct rte_mbuf *m = opaque; - RTE_SET_USED(addr); + RTE_SET_USED(caller_m); RTE_ASSERT(RTE_MBUF_HAS_EXTBUF(m)); RTE_ASSERT(RTE_MBUF_HAS_PINNED_EXTBUF(m)); RTE_ASSERT(m->shinfo->fcb_opaque == m); diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 7259575..5f8626f 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -1193,7 +1193,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *m) RTE_ASSERT(m->shinfo != NULL); if (rte_mbuf_ext_refcnt_update(m->shinfo, -1) == 0) - m->shinfo->free_cb(m->buf_addr, m->shinfo->fcb_opaque); + m->shinfo->free_cb(m, m->shinfo->fcb_opaque); } /** diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h index 8cd7137..d194429 100644 --- a/lib/librte_mbuf/rte_mbuf_core.h +++ b/lib/librte_mbuf/rte_mbuf_core.h @@ -671,7 +671,7 @@ struct rte_mbuf { /** * Function typedef of callback to free externally attached buffer. */ -typedef void (*rte_mbuf_extbuf_free_callback_t)(void *addr, void *opaque); +typedef void (*rte_mbuf_extbuf_free_callback_t)(struct rte_mbuf *, void *); /** * Shared data at the end of an external buffer. diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index 95a0bc1..e663fd4 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -2137,7 +2137,7 @@ uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id, } static void -virtio_dev_extbuf_free(void *addr __rte_unused, void *opaque) +virtio_dev_extbuf_free(struct rte_mbuf * caller_m __rte_unused, void *opaque) { rte_free(opaque); } From patchwork Thu Jul 30 12:09:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: yang_y_yi X-Patchwork-Id: 75037 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 319C6A052B; Thu, 30 Jul 2020 14:09:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9FFC61C00D; Thu, 30 Jul 2020 14:09:09 +0200 (CEST) Received: from mail-m974.mail.163.com (mail-m974.mail.163.com [123.126.97.4]) by dpdk.org (Postfix) with ESMTP id 7FFD910A3 for ; Thu, 30 Jul 2020 14:09:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:Subject:Date:Message-Id:MIME-Version; bh=lFDu0 tBdjS+Pgn1WClXzcYpD7IRxZ1x0v3sOwg3KcrI=; b=YWRNUYZJRlvWIshMTsyfq VrflWZUCmlLD0ORjK6CiPg46Ukckx9cYyoQbUNj943W5e927X0NIDtkY+IMmVaZy lMrVlwERYnYznZ4O0MPEtXUv4bi7iQ6Pt6cj8TiO88FRQYCrfTBaAZbvcprZYPaP 8obQxOGmomfljEgM+uif3U= Received: from yangyi0100.home.langchao.com (unknown [61.48.210.155]) by smtp4 (Coremail) with SMTP id HNxpCgBXekNcuCJfZo1vIA--.3698S5; Thu, 30 Jul 2020 20:09:02 +0800 (CST) From: yang_y_yi@163.com To: dev@dpdk.org Cc: jiayu.hu@intel.com, thomas@monjalon.net, yangyi01@inspur.com, yang_y_yi@163.com Date: Thu, 30 Jul 2020 20:09:00 +0800 Message-Id: <20200730120900.108232-4-yang_y_yi@163.com> X-Mailer: git-send-email 2.19.2.windows.1 In-Reply-To: <20200730120900.108232-1-yang_y_yi@163.com> References: <20200730120900.108232-1-yang_y_yi@163.com> MIME-Version: 1.0 X-CM-TRANSID: HNxpCgBXekNcuCJfZo1vIA--.3698S5 X-Coremail-Antispam: 1Uf129KBjvJXoW7ZrWfXF1DZF1Utw4UArykAFb_yoW8Ar45pF 43Gry5Ar1rJr4xKrsxCr4fK3Z3Kayvk3W7CrZa9w1F9r4xX34xXFykKFWrur13Kr97Ar43 XF40qF1UK3WUuFDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jBeHgUUUUU= X-Originating-IP: [61.48.210.155] X-CM-SenderInfo: 51dqwsp1b1xqqrwthudrp/1tbiqBBxi1c7RBg70AAAsZ Subject: [dpdk-dev] [PATCH V1 3/3] vhost: use new free_cb interface to fix mbuf free issue X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Yi Yang New free_cb interface can help fix original external mbuf free issue in GSO case, it still can be compatible with normal non-GSO case. dpdkvhostuser port can work both in GSO case and in non-GSO case by this fix. Signed-off-by: Yi Yang --- lib/librte_vhost/virtio_net.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index e663fd4..3b69cbb 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -2136,10 +2136,20 @@ uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id, return NULL; } +struct shinfo_arg { + void *buf; + struct rte_mbuf *mbuf; +}; + static void -virtio_dev_extbuf_free(struct rte_mbuf * caller_m __rte_unused, void *opaque) +virtio_dev_extbuf_free(struct rte_mbuf *caller_m, void *opaque) { - rte_free(opaque); + struct shinfo_arg *arg = (struct shinfo_arg *)opaque; + + rte_free(arg->buf); + if (caller_m != arg->mbuf) + rte_pktmbuf_free(arg->mbuf); + rte_free(arg); } static int @@ -2172,8 +2182,14 @@ uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id, /* Initialize shinfo */ if (shinfo) { + struct shinfo_arg *arg = (struct shinfo_arg *) + rte_malloc(NULL, sizeof(struct shinfo_arg), + RTE_CACHE_LINE_SIZE); + + arg->buf = buf; + arg->mbuf = pkt; shinfo->free_cb = virtio_dev_extbuf_free; - shinfo->fcb_opaque = buf; + shinfo->fcb_opaque = arg; rte_mbuf_ext_refcnt_set(shinfo, 1); } else { shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len,