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 7AB61A04DB; Fri, 16 Oct 2020 02:53:10 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E8BA71DA6B; Fri, 16 Oct 2020 02:53:08 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id DAC7A1DA80 for ; Fri, 16 Oct 2020 02:53:05 +0200 (CEST) IronPort-SDR: Gj7qp/QQJsAH3VI1KsagM9vWAO4D2Q8fkAwWtX2AYy0Le88voFjrjI5Rq4tSt7bVjljg6XfdrR 8t7qSsPPbgVQ== X-IronPort-AV: E=McAfee;i="6000,8403,9775"; a="166598739" X-IronPort-AV: E=Sophos;i="5.77,380,1596524400"; d="scan'208";a="166598739" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Oct 2020 17:53:03 -0700 IronPort-SDR: NMyaRD70FxRWXIyXIpcrqlN8Y+PUvfjalcJqWu6mXgbAcTdhy2sy8Zz4A9f9CUS/kkCeo1hOQY 2CAkV2QNyjqg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,380,1596524400"; d="scan'208";a="346351849" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by fmsmga004.fm.intel.com with ESMTP; 15 Oct 2020 17:53:02 -0700 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 15 Oct 2020 17:53:02 -0700 Received: from shsmsx606.ccr.corp.intel.com (10.109.6.216) by SHSMSX601.ccr.corp.intel.com (10.109.6.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 16 Oct 2020 08:53:00 +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; Fri, 16 Oct 2020 08:53:00 +0800 From: "Hu, Jiayu" To: "Ananyev, Konstantin" , yang_y_yi CC: "dev@dpdk.org" , "olivier.matz@6wind.com" , "thomas@monjalon.net" , "yangyi01@inspur.com" Thread-Topic: Re:RE: [PATCH] gso: fix free issue of mbuf gso segments attach to Thread-Index: AQHWnrL4kCD/a91T7EmI02LJCsZxX6mVGaMwgAAQ4wCAAR+qUP//nXEAgACZjACAAaD1kIAAN1CAgAEV1GA= Date: Fri, 16 Oct 2020 00:53:00 +0000 Message-ID: <29f24642c9fe498db60c8c3fa6ec0f1d@intel.com> References: <20201010031020.349516-1-yang_y_yi@163.com> <43f71e6c9d2f4d5ba3ab56a921c5912d@intel.com> <6167423037704e3382f85275be79de30@intel.com> <9239b2e.2116.17525095531.Coremail.yang_y_yi@163.com> In-Reply-To: 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: Ananyev, Konstantin > Sent: Friday, October 16, 2020 12:16 AM > To: Hu, Jiayu ; yang_y_yi > Cc: dev@dpdk.org; olivier.matz@6wind.com; thomas@monjalon.net; > yangyi01@inspur.com > Subject: RE: Re:RE: [PATCH] gso: fix free issue of mbuf gso segments atta= ch to >=20 >=20 >=20 > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Wednesday, October 14, 2020 8:06 PM > > > To: yang_y_yi ; Hu, Jiayu > > > Cc: dev@dpdk.org; olivier.matz@6wind.com; thomas@monjalon.net; > > > yangyi01@inspur.com > > > Subject: RE: Re:RE: [PATCH] gso: fix free issue of mbuf gso segments > attach to > > > > > > > > > > From: yang_y_yi > > > > Sent: Wednesday, October 14, 2020 3:56 AM > > > > To: Hu, Jiayu > > > > Cc: Ananyev, Konstantin ; > dev@dpdk.org; > > > olivier.matz@6wind.com; thomas@monjalon.net; > > > > yangyi01@inspur.com > > > > Subject: Re:RE: [PATCH] gso: fix free issue of mbuf gso segments at= tach > to > > > > > > > > I think it isn't a good idea to free it in rte_gso_segment, maybe > application > > > will continue to use this pkt for other purpose, rte_gso_segment > > > > can't make decision for application without any notice, it is bette= r to > return > > > this decision right backt to application. > > > > > > > > > > I think, if user wants to keep original packet, he can always call > > > rte_pktmbuf_refcnt_update(pkt, 1) > > > just before calling gso function. > > > > > > Also, as I remember in some cases it is not safe to do free() for inp= ut > packet > > > (as pkt_out[] can contain input pkt itself). Would it also be user > responsibility > > > to determine > > > such situations? > > > > In what case will pkt_out[] contain the input pkt? Can you give an exam= ple? >=20 > As I can read the code, whenever gso code decides that > no segmentation is not really needed, or it is not capable > of doing it properly. > Let say: >=20 > gso_tcp4_segment(struct rte_mbuf *pkt, > uint16_t gso_size, > uint8_t ipid_delta, > struct rte_mempool *direct_pool, > struct rte_mempool *indirect_pool, > struct rte_mbuf **pkts_out, > uint16_t nb_pkts_out) > { > ... > /* Don't process the fragmented packet */ > ipv4_hdr =3D (struct rte_ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *= ) + > pkt->l2_len); > frag_off =3D rte_be_to_cpu_16(ipv4_hdr->fragment_offset); > if (unlikely(IS_FRAGMENTED(frag_off))) { > pkts_out[0] =3D pkt; > return 1; > } >=20 > /* Don't process the packet without data */ > hdr_offset =3D pkt->l2_len + pkt->l3_len + pkt->l4_len; > if (unlikely(hdr_offset >=3D pkt->pkt_len)) { > pkts_out[0] =3D pkt; > return 1; > } >=20 > That's why in rte_gso_segment() we update refcnt only when ret > 1. But in these cases, the value of ret is 1. So we can free input pkt only wh= en ret > 1. Like: - 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 > 1) + rte_pktmbuf_free(pkt); + else if (ret < 0) { /* Revert the ol_flags in the event of failure. */ pkt->ol_flags =3D ol_flags; } Thanks, Jiayu >=20 >=20 >=20