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 E58DBA04B6; Tue, 13 Oct 2020 09:28:42 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 666841D970; Tue, 13 Oct 2020 09:28:41 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 0ADDD1D94E for ; Tue, 13 Oct 2020 09:28:38 +0200 (CEST) IronPort-SDR: YX0ybyCwBjHFt4l/PsYS5prVEWswSj08b1w6X6Qdiv3WM/CYxy9qHcoqL1taFqCODAejvadKvg jZpB+gWRVqXA== X-IronPort-AV: E=McAfee;i="6000,8403,9772"; a="152797046" X-IronPort-AV: E=Sophos;i="5.77,369,1596524400"; d="scan'208";a="152797046" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Oct 2020 00:28:36 -0700 IronPort-SDR: Kxg4I5sfq7KkM9b5Bp6R80FXssVg7+eoig5OPv4GN0jfh/cPYyvj4c5rhYk7xZidrylgtPQr0W fGWs4QypeP9Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,369,1596524400"; d="scan'208";a="420453635" Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by fmsmga001.fm.intel.com with ESMTP; 13 Oct 2020 00:28:36 -0700 Received: from shsmsx602.ccr.corp.intel.com (10.109.6.142) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 13 Oct 2020 00:28:35 -0700 Received: from shsmsx606.ccr.corp.intel.com (10.109.6.216) by SHSMSX602.ccr.corp.intel.com (10.109.6.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 13 Oct 2020 15:28:33 +0800 Received: from shsmsx606.ccr.corp.intel.com ([10.109.6.216]) by SHSMSX606.ccr.corp.intel.com ([10.109.6.216]) with mapi id 15.01.1713.004; Tue, 13 Oct 2020 15:28:33 +0800 From: "Hu, Jiayu" To: "yang_y_yi@163.com" , "dev@dpdk.org" , "Ananyev, Konstantin" CC: "mark.b.kavanagh@intel.com" , "olivier.matz@6wind.com" , "thomas@monjalon.net" , "yangyi01@inspur.com" Thread-Topic: [PATCH] gso: fix free issue of mbuf gso segments attach to Thread-Index: AQHWnrL4kCD/a91T7EmI02LJCsZxX6mVGaMw Date: Tue, 13 Oct 2020 07:28:33 +0000 Message-ID: <43f71e6c9d2f4d5ba3ab56a921c5912d@intel.com> References: <20201010031020.349516-1-yang_y_yi@163.com> In-Reply-To: <20201010031020.349516-1-yang_y_yi@163.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-reaction: no-action dlp-version: 11.5.1.3 dlp-product: dlpe-windows x-originating-ip: [10.239.127.36] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] gso: fix free issue of mbuf gso segments attach to 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" > -----Original Message----- > From: yang_y_yi@163.com > Sent: Saturday, October 10, 2020 11:10 AM > To: dev@dpdk.org > Cc: Hu, Jiayu ; mark.b.kavanagh@intel.com; Ananyev, > Konstantin ; olivier.matz@6wind.com; > thomas@monjalon.net; yangyi01@inspur.com; yang_y_yi@163.com > Subject: [PATCH] gso: fix free issue of mbuf gso segments attach to >=20 > From: Yi Yang >=20 > rte_gso_segment decreased refcnt of pkt by one, but > it is wrong if pkt is external mbuf, pkt won't be > freed because of incorrect refcnt, the result is > application can't allocate mbuf from mempool because > mbufs in mempool are run out of. >=20 > One correct way is application should call > rte_pktmbuf_free after calling rte_gso_segment to free > pkt explicitly. rte_gso_segment mustn't handle it, this > should be responsibility of application. GSO doesn't support the input pktmbuf has external buffer. Indeed, requiring users to free the input pktmbuf can avoid memory leak, but I'm afraid that it also changes the semantic of rte_gso_segment() which is defined in rte_gso.h. @Konstantin, any suggestions? Thanks, Jiayu >=20 > Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO") > Signed-off-by: Yi Yang > --- > doc/guides/prog_guide/generic_segmentation_offload_lib.rst | 7 +++++-- > lib/librte_gso/rte_gso.c | 9 +-------- > 2 files changed, 6 insertions(+), 10 deletions(-) >=20 > diff --git a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst > b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst > index 205cb8a..8577572 100644 > --- a/doc/guides/prog_guide/generic_segmentation_offload_lib.rst > +++ b/doc/guides/prog_guide/generic_segmentation_offload_lib.rst > @@ -25,8 +25,9 @@ Bearing that in mind, the GSO library enables DPDK > applications to segment > packets in software. Note however, that GSO is implemented as a > standalone > library, and not via a 'fallback' mechanism (i.e. for when TSO is unsupp= orted > in the underlying hardware); that is, applications must explicitly invok= e the > -GSO library to segment packets. The size of GSO segments ``(segsz)`` is > -configurable by the application. > +GSO library to segment packets, they also must call ``rte_pktmbuf_free()= `` to > +free mbuf GSO segments attach to after calling ``rte_gso_segment()``. Th= e > size > +of GSO segments ``(segsz)`` is configurable by the application. >=20 > Limitations > ----------- > @@ -233,6 +234,8 @@ To segment an outgoing packet, an application must: >=20 > #. Invoke the GSO segmentation API, ``rte_gso_segment()``. >=20 > +#. Call ``rte_pktmbuf_free()`` to free mbuf ``rte_gso_segment()`` segmen= ts. > + > #. If required, update the L3 and L4 checksums of the newly-created > segments. > For tunneled packets, the outer IPv4 headers' checksums should also b= e > updated. Alternatively, the application may offload checksum calculat= ion > diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.c > index 751b5b6..0d6cae5 100644 > --- a/lib/librte_gso/rte_gso.c > +++ b/lib/librte_gso/rte_gso.c > @@ -30,7 +30,6 @@ > uint16_t nb_pkts_out) > { > struct rte_mempool *direct_pool, *indirect_pool; > - struct rte_mbuf *pkt_seg; > uint64_t ol_flags; > uint16_t gso_size; > uint8_t ipid_delta; > @@ -80,13 +79,7 @@ > return 1; > } >=20 > - if (ret > 1) { > - pkt_seg =3D pkt; > - while (pkt_seg) { > - rte_mbuf_refcnt_update(pkt_seg, -1); > - pkt_seg =3D pkt_seg->next; > - } > - } else if (ret < 0) { > + if (ret < 0) { > /* Revert the ol_flags in the event of failure. */ > pkt->ol_flags =3D ol_flags; > } > -- > 1.8.3.1