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 1A73FA04DD; Thu, 22 Oct 2020 15:16:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5A91DA9C5; Thu, 22 Oct 2020 15:16:52 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 37411A9C0; Thu, 22 Oct 2020 15:16:49 +0200 (CEST) IronPort-SDR: e1vfugXripmt7Y+fhYRJehugSChRontozboJ2dCf8YKDP1/sdOBtvEd2nYC1jBfaPhbrhx0w7y BmMMiOYShiag== X-IronPort-AV: E=McAfee;i="6000,8403,9781"; a="229151965" X-IronPort-AV: E=Sophos;i="5.77,404,1596524400"; d="scan'208";a="229151965" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Oct 2020 06:16:46 -0700 IronPort-SDR: /4C4XcFdCVXKuwU1LuAOw/dnyEjUKlVDcH36h5Xj2AGFw16crqjv98rsBrMEiC5vg//9AndInO htrUI7GCxLag== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,404,1596524400"; d="scan'208";a="533962799" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by orsmga005.jf.intel.com with ESMTP; 22 Oct 2020 06:16:46 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) 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, 22 Oct 2020 06:16:46 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 22 Oct 2020 06:16:45 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Thu, 22 Oct 2020 06:16:45 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.46) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Thu, 22 Oct 2020 06:16:45 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=G4/ElkWD4kHE6zMyPU2UfEY1/fsg5aILQ/omqyg3VwuHCxm1K5iGlhp832lNpanWLJtkHQeoQQHYGuCTS6S9n3jyPuu+5p/azn06oy1LCdkEDlnbhEi4Wl6/VIaZF8vfp+sF0rCYp8VK4GJJC0Jw4wyqsjeinUbNyf78lbnZTX0ip/EDdJE+4APCnvEaunI4u3sJ/2Gpuwb/uSKAVHfUY6eBWs7AMy1I9qmAAUt2oUMmZxi4Tkn83fi13dbW+BsaKDmS4uJAx8NY2J3bIDSW72oYTUQ6/WNL+L+U2UWrCrYd85zB4KPIIkM2eZLgmZQ3PL2N1txseL09/Ui0NFWjsg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=RShsLcfd5KXWzTfft7oHtbwGhGniEK39LbeO5bWO10o=; b=b6l28ljpQN+AV8KXwnXv9L/XCLPR7ZiBFVFDOuyLik2x9oLsbJcGe4KWAC/ymC8EcycPccNiuaB/W4hA6fvx5HbiroQVsPJmIIRvxgijxIRqOHNsZc0p3NMxTfQS6EpRWqCs+WqtryCZ7Yatn1hbZ8vs5HmGPv2JZqoxOF/W9dyShVwTcR6HOvfhCxq3EaVUhro/w4w4zJBFQv5v1yRwUpl9ydMF/slFhL29RxweLzXoMtrqmnjCcdvXhVn0E5GxegA7qZZcOukt/V1NXk+gdKDsrcNV42nXUfgUO/FymXq6GGaa8fJOVFk9F0F7C7crerCHTtreX+x+F+ANKg51HQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=RShsLcfd5KXWzTfft7oHtbwGhGniEK39LbeO5bWO10o=; b=jcJwdmRa2x4gGR4f14kpRvLZYRhC8XJ2SuNyxtW9SboGAP4aH8ujP+5P4FOkXZftsCgh+XFBdu9014WQ6McU9N8uhQW7GiZnm4wXWsJrhCIbym7XegINfWzo3YZRuLnCdBwVfz5QWCM+7RwwtgxVDrB2yNMB0U4bvEE03Gn7/t4= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by SJ0PR11MB5134.namprd11.prod.outlook.com (2603:10b6:a03:2de::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.25; Thu, 22 Oct 2020 13:16:43 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f5a4:3f6b:ade3:296b]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f5a4:3f6b:ade3:296b%3]) with mapi id 15.20.3499.018; Thu, 22 Oct 2020 13:16:43 +0000 From: "Ananyev, Konstantin" To: "yang_y_yi@163.com" , "dev@dpdk.org" CC: "Hu, Jiayu" , "techboard@dpdk.org" , "thomas@monjalon.net" , "yangyi01@inspur.com" Thread-Topic: [PATCH v2] gso: fix free issue of mbuf gso segments attach to Thread-Index: AQHWqD/tzleaP+GQ60mK7fZdnSpuYqmjlw5w Date: Thu, 22 Oct 2020 13:16:43 +0000 Message-ID: References: <20201022065151.10108-1-yang_y_yi@163.com> In-Reply-To: <20201022065151.10108-1-yang_y_yi@163.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: 163.com; dkim=none (message not signed) header.d=none;163.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [46.7.39.127] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 02fb3a3b-c85a-4a8c-19e8-08d8768cb87c x-ms-traffictypediagnostic: SJ0PR11MB5134: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: rLXw/ODZXPZ0h9UZUJ8bdCLu280xgV4PM0pn3I768Ac0dcW1WivhatAxqOOBFsXNZWmsV9t1LR3AsHNCfkrFDmOEyA2m4e/rouRFUBxjF0I127hIjQ1g0IbdMCrVCMoOFfpGg5VJG7XdipTkPv5lqtCazlm0IztcfZvqChAUYLFXidzwHFJ084ANk0mEGjFPF5oiPeazRkebJd+QMU6DqMSMuEq+YDMFRFU9NNp8wVTqvoIdyiIqPUwMZGrpqHSXoX9Yl3L90+iyZn/FhbwGwHAQVOSKO1MbJOFs5+02mdSoFXzJv40uSeo00B17xfXy2cWHggJ/tywGKBhH7yf/gw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(346002)(376002)(366004)(136003)(396003)(39860400002)(66446008)(33656002)(186003)(71200400001)(478600001)(5660300002)(110136005)(2906002)(316002)(9686003)(86362001)(76116006)(6506007)(83380400001)(8676002)(54906003)(55016002)(66556008)(64756008)(4326008)(66476007)(52536014)(7696005)(26005)(66946007)(8936002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: XhYEjJIwYA4ob3sZghkSf5HZ0aHle2YspvyRzEIcI2RDvMzyllx6zng+AQh5AKTDYq0VF+VNsCPeiH9dS3JVa4fJWik/g6Rc8/FWOZlTglGrE6lnrvDV2gRi+MJusM2bJyeP3Hsm8QQZuG3bCthguYRQarPat1VEqST/hznAgwl3OagegAfE+XAte9UFB1lbFvKAmZ3l3LBICz0QgvER3+CtNkF8EpZ+6YiyRKwXLWgF0n9deYiWC8MhMeVONB/p32iF5oKBEhr7l9JiWE30L5wXTMxWhvhNnmZbXL0LPyfcfax+U8y7XMH2CwLAMkV/UFW/lmZ72oKUiohjJ6Vx0d0XRRTw3enZTA1Pz4yjQV3hIzDhelPBPHnCoo68gsfqEGWbuKujh6TEqvwL+atjkEiYpwmGY42sFLk9Tx47MDiCzeNSpXNme3U3dFKj8IsY/yBiMuUJEEY3OSPDRbRhlDXWs+vm0XmjRqKjPByqXEdn4pbtNJLuZDwpgUWKWS0nyxjBxEPvsG74uIpc7WN7h7UW0suDk9jbsZmco0DVLkzX/cX7Z1M/m4+0vA0Tt59BOOv4CippavUSUxAFyiYxspI6OaG2pSWspGGa/GVyZlGXHDI4BOy/hcz4hN8sEq2GxQEKOcXV4dyMQ56mWqooEQ== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3301.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 02fb3a3b-c85a-4a8c-19e8-08d8768cb87c X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Oct 2020 13:16:43.4225 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: XXYKc1Rbb0XvnM8M2owEG3ANpRWK7j6BWfqSHGGEKQzjlDxH0ezuHVwkHZDbzfCjGA5VVTe5RndsErmqhjwyIa9S75t4aZUWFTVL0VkeoIs= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5134 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v2] 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" >=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. Probably needs to be stated clearly: It is a change in functional behaviour. Without deprecation note in advance. TB members: please provide your opinion on that patch. >=20 > Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO") > Signed-off-by: Yi Yang > --- > Changelog: >=20 > v1->v2: > - update description of rte_gso_segment(). > - change code which calls rte_gso_segment() to > fix free issue. >=20 > --- > app/test-pmd/csumonly.c | 3 ++- > doc/guides/prog_guide/generic_segmentation_offload_lib.rst | 7 +++++-- I think release notes also have to be updated. > lib/librte_gso/rte_gso.c | 9 +-------- > lib/librte_gso/rte_gso.h | 7 +++++-- > 4 files changed, 13 insertions(+), 13 deletions(-) >=20 > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c > index 3d7d244..829e07f 100644 > --- a/app/test-pmd/csumonly.c > +++ b/app/test-pmd/csumonly.c > @@ -1080,11 +1080,12 @@ struct simple_gre_hdr { > ret =3D rte_gso_segment(pkts_burst[i], gso_ctx, > &gso_segments[nb_segments], > GSO_MAX_PKT_BURST - nb_segments); > + /* pkts_burst[i] can be freed safely here. */ > + rte_pktmbuf_free(pkts_burst[i]); It doesn't look correct to me. I think it should be: If (ret > 1) rte_pktmbuf_free(pkts_burst[i]); > if (ret >=3D 0) > nb_segments +=3D ret; > else { > TESTPMD_LOG(DEBUG, "Unable to segment packet"); > - rte_pktmbuf_free(pkts_burst[i]); > } > } About drivers/net/tap/rte_eth_tap.c: I think it has to be modified too, as here: /* free original mbuf */ rte_pktmbuf_free(mbuf_in); /* free tso mbufs */ if (num_tso_mbufs > 0) rte_pktmbuf_free_bulk(mbuf, num_tso_mbufs); if mbuf[0] =3D=3D mbuf_in Will have a double free() for the same mbuf. >=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 appl= ications to segment > packets in software. Note however, that GSO is implemented as a standalo= ne > 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()``. Probably worth to mention that if return code =3D=3D 1, then output mbuf will point to input mbuf and extra care with free() is required= . > The 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 segm= ents. > 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; > } > diff --git a/lib/librte_gso/rte_gso.h b/lib/librte_gso/rte_gso.h > index 3aab297..f6694f9 100644 > --- a/lib/librte_gso/rte_gso.h > +++ b/lib/librte_gso/rte_gso.h > @@ -89,8 +89,11 @@ struct rte_gso_ctx { > * the GSO segments are sent to should support transmission of multi-seg= ment > * packets. > * > - * If the input packet is GSO'd, its mbuf refcnt reduces by 1. Therefore= , > - * when all GSO segments are freed, the input packet is freed automatica= lly. > + * If the input packet is GSO'd, all the indirect segments are attached = to the > + * input packet. > + * > + * rte_gso_segment() will not free the input packet no matter whether it= is > + * GSO'd or not, the application should free it after call rte_gso_segme= nt(). > * > * If the memory space in pkts_out or MBUF pools is insufficient, this > * function fails, and it returns (-1) * errno. Otherwise, GSO succeeds, > -- > 1.8.3.1