From patchwork Thu Jul 30 09:56:08 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: 75030 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 179E3A053C; Thu, 30 Jul 2020 11:56:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9A2AC1C00F; Thu, 30 Jul 2020 11:56:19 +0200 (CEST) Received: from mail-m972.mail.163.com (mail-m972.mail.163.com [123.126.97.2]) by dpdk.org (Postfix) with ESMTP id B403811A2 for ; Thu, 30 Jul 2020 11:56:15 +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=g/80qYG9QsUAVWw2GkmHC 98SgP4rjOUCtgBNk23BbdpNsYN52V90lET8jmgCCgyt8UFsSLJh9BFPb+TJfUpfU aw3EyR5N+tok5vqDGVrFdEWuaDQXjGRTWCKmKU0WX97HFQYc9ZVU1nuH38cjBu6X 4TQhV943zvoYf9sqZE6LwI= Received: from yangyi0100.home.langchao.com (unknown [111.207.123.56]) by smtp2 (Coremail) with SMTP id GtxpCgCXFdk6mSJfhBGTHg--.3074S3; Thu, 30 Jul 2020 17:56:11 +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 17:56:08 +0800 Message-Id: <20200730095610.93384-2-yang_y_yi@163.com> X-Mailer: git-send-email 2.19.2.windows.1 In-Reply-To: <20200730095610.93384-1-yang_y_yi@163.com> References: <20200730095610.93384-1-yang_y_yi@163.com> MIME-Version: 1.0 X-CM-TRANSID: GtxpCgCXFdk6mSJfhBGTHg--.3074S3 X-Coremail-Antispam: 1Uf129KBjvdXoW7JF1rJrWruF1UGF4fAFyxZrb_yoWDKFc_ua 4FkF1ktay7Jr4xZw43Jw4Yga1xurW7Aa18Ww4Igw45Xr4qgFsxu34DJr42qFWDu3yUGFWx WF43uFn2yr1FvjkaLaAFLSUrUUUUUb8apTn2vfkv8UJUUUU8Yxn0WfASr-VFAUDa7-sFnT 9fnUUvcSsGvfC2KfnxnUUI43ZEXa7IUjFAp7UUUUU== X-Originating-IP: [111.207.123.56] X-CM-SenderInfo: 51dqwsp1b1xqqrwthudrp/xtbB0htxi1UMX9WQMQAAsB Subject: [dpdk-dev] [PATCH 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 09:56:09 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: 75031 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 75703A0518; Thu, 30 Jul 2020 11:56:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 185C31C02E; Thu, 30 Jul 2020 11:56:21 +0200 (CEST) Received: from mail-m972.mail.163.com (mail-m972.mail.163.com [123.126.97.2]) by dpdk.org (Postfix) with ESMTP id 55A0C1BFE7 for ; Thu, 30 Jul 2020 11:56:16 +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=Os8fEZCyBdcs/7/4vm75n JyzLP29LZJ5svJFr59I2Ho2anVEI4oVB2rgkY2Y3R9mhrr3LlOKtp+2UZ2MdGCDp I+1+h42qXfzKLox+qxYev2yCHCdUheyHdA+ThIE+KoxAJfRt2ibdvEYvWhdH1Zv0 +wzsaQn2oH7v4ujxa3SYCM= Received: from yangyi0100.home.langchao.com (unknown [111.207.123.56]) by smtp2 (Coremail) with SMTP id GtxpCgCXFdk6mSJfhBGTHg--.3074S4; Thu, 30 Jul 2020 17:56:11 +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 17:56:09 +0800 Message-Id: <20200730095610.93384-3-yang_y_yi@163.com> X-Mailer: git-send-email 2.19.2.windows.1 In-Reply-To: <20200730095610.93384-1-yang_y_yi@163.com> References: <20200730095610.93384-1-yang_y_yi@163.com> MIME-Version: 1.0 X-CM-TRANSID: GtxpCgCXFdk6mSJfhBGTHg--.3074S4 X-Coremail-Antispam: 1Uf129KBjvJXoW3Ww1fCr18CFy5KFyfuF47XFb_yoW3Cw4fpF srCryjkrs8JF48uFs7Jr4Sqrn3KFWkKay7KrWkJ34F9F1YqryvqFWFka48Zr1YgrWkCrZF vF4kJa47Ga18CwUanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07UNjjgUUUUU= X-Originating-IP: [111.207.123.56] X-CM-SenderInfo: 51dqwsp1b1xqqrwthudrp/xtbB0htxi1UMX9WQMwAAsD Subject: [dpdk-dev] [PATCH 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 09:56:10 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: 75032 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 523C3A0518; Thu, 30 Jul 2020 11:56:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 863F51C042; Thu, 30 Jul 2020 11:56:22 +0200 (CEST) Received: from mail-m972.mail.163.com (mail-m972.mail.163.com [123.126.97.2]) by dpdk.org (Postfix) with ESMTP id 657BD1BFFA for ; Thu, 30 Jul 2020 11:56:16 +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=l6L/lh9g4gSmvcrFmD0S/ zHcn7bp+kPA/Ot6JbSA3xzXPZoKc6sQDEoY+6Jkm/8ONR9aYHrGm8ybaPF1wv1yW H1XJSTuG7qqGGjVfV9GMoSCv6aiz8j/SpvpdPI3W2zftHGbAuRsapHceZqLHD1EY COw4jsKjBmgJYldcKlW108= Received: from yangyi0100.home.langchao.com (unknown [111.207.123.56]) by smtp2 (Coremail) with SMTP id GtxpCgCXFdk6mSJfhBGTHg--.3074S5; Thu, 30 Jul 2020 17:56:11 +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 17:56:10 +0800 Message-Id: <20200730095610.93384-4-yang_y_yi@163.com> X-Mailer: git-send-email 2.19.2.windows.1 In-Reply-To: <20200730095610.93384-1-yang_y_yi@163.com> References: <20200730095610.93384-1-yang_y_yi@163.com> MIME-Version: 1.0 X-CM-TRANSID: GtxpCgCXFdk6mSJfhBGTHg--.3074S5 X-Coremail-Antispam: 1Uf129KBjvJXoW7ZrWfXF1DZF1Utw4UArykAFb_yoW8Ar45pF 43Gry5Ar1rJr4xKrsxCr4fK3Z3Kayvk3W7CrZa9w1F9r4xX34xXFykKFWrur13Kr97Ar43 XF40qF1UK3WUuFDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jFrWrUUUUU= X-Originating-IP: [111.207.123.56] X-CM-SenderInfo: 51dqwsp1b1xqqrwthudrp/1tbiMxtxi1Xl5dl34gAAsL Subject: [dpdk-dev] [PATCH 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,