From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id CF1B6A04DB;
	Fri, 16 Oct 2020 10:31:35 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id ACB0A1EB5E;
	Fri, 16 Oct 2020 10:31:34 +0200 (CEST)
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by dpdk.org (Postfix) with ESMTP id 824231EB5D
 for <dev@dpdk.org>; Fri, 16 Oct 2020 10:31:31 +0200 (CEST)
IronPort-SDR: XJbihRaRi/KAtrDoVSFXLxFhLKbkCXIBshFzNQN0muKw3n08I//Bj4Lp83jmw77D+WIdjvadkW
 tkDa/qyaIXjw==
X-IronPort-AV: E=McAfee;i="6000,8403,9775"; a="165788723"
X-IronPort-AV: E=Sophos;i="5.77,382,1596524400"; d="scan'208";a="165788723"
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga006.fm.intel.com ([10.253.24.20])
 by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 16 Oct 2020 01:31:30 -0700
IronPort-SDR: tsRgbGUzKh/t2cRQbfK6xARqtVTbFrN0DhQUCrkn8PF8vc4ojJJUQs07CRzLXxJmKedE3dy1Ee
 B7VWZ318oLTA==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.77,382,1596524400"; d="scan'208";a="521101009"
Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81])
 by fmsmga006.fm.intel.com with ESMTP; 16 Oct 2020 01:31:29 -0700
Received: from fmsmsx609.amr.corp.intel.com (10.18.126.89) by
 fmsmsx601.amr.corp.intel.com (10.18.126.81) 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 01:31:29 -0700
Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by
 fmsmsx609.amr.corp.intel.com (10.18.126.89) 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 01:31:28 -0700
Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) 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; Fri, 16 Oct 2020 01:31:28 -0700
Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.108)
 by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.1.1713.5; Fri, 16 Oct 2020 01:31:27 -0700
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=leM3gYpEfdRO3Ufc+yCvap+Mnhy7jz8wH39jl2CevFbfcwcjR6XyCdQN3rc4QKlYJu86l31ll8UoQlb1W30w5OHA6/swA8TIcl5ILhTu/p/A+rgYyP86pS71z5aShG5qI4YGYg8mE0jfqCY9cP6jVv99HRsxzIpMEJLpdXU8bPV5dK+eSacQbftPtIbhZ2ONk58TrTWBvuFNm25UbjZXQ6KEPJIyg70pGXlCoEbgpspeIjUS7ekhl61rZuxWmIcY3W81g4p3pxGKs7s741s7ksQkf7gCGrHbwhDgrmCj8Ea1Xo0C0UeoX/bYUx2X6ydSdpIxsh7FQFlA1HHZ1PUvKA==
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=KeryxU/7HdQjnyfyhdTIL5GnJ+mNYXSWOSOWvA30NjI=;
 b=RgWeRPwSG4ldkZuFjEimZ5b55Ahzf4jAgd4mCOKxYL+NH/lF3AQXtJxlY3N+Y+nkvBeee4xpwZ/KBaMZE+Av7jdl6EI9krub2IuVKa3DeNhp5qCchHnEm4tiMH/PWYmMbWCpk6/jbSA4zlkfrCNsyclbwxaasURASgWKiFRFbklu23S4koxoUdsydSFRIe94eqWfvxaGAImmkh3EU02BMyyFQCMf0koFcU1AmGiPvdBqvBQCBZ9Luj22oyNRNmMkLgumoSypFljhqOBLBDKdwJjI0McB1HNXg8rtHR1w+pZ4kSqC4lwJJUdeE0bkK72kYDJDlgLCf+IqLTz9kOVdDw==
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=KeryxU/7HdQjnyfyhdTIL5GnJ+mNYXSWOSOWvA30NjI=;
 b=gGi6omvnvWj+L7CiT1NtFZ5uuVE2T+tiLvzgXMtUAp3obLqogpt3h0oRKXy2I0b8uT+P56qqztSnGmAYcwt1Z24/X1Z98hUUoq1t0uEkT3OPAAvpMzHLhwRauBCIX9ocIlxn26xLBr3YU00JjZOr1AqYrdFSf1Xnjnjqk0esNgM=
Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26)
 by BYAPR11MB3221.namprd11.prod.outlook.com (2603:10b6:a03:1c::12)
 with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.21; Fri, 16 Oct
 2020 08:31:23 +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.3477.021; Fri, 16 Oct 2020
 08:31:23 +0000
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Hu, Jiayu" <jiayu.hu@intel.com>, yang_y_yi <yang_y_yi@163.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "olivier.matz@6wind.com"
 <olivier.matz@6wind.com>, "thomas@monjalon.net" <thomas@monjalon.net>,
 "yangyi01@inspur.com" <yangyi01@inspur.com>
Thread-Topic: Re:RE: [PATCH] gso: fix free issue of mbuf gso segments attach to
Thread-Index: AQHWnrL85KBHJVyvm02xWITKaS0ypKmVJ4OAgACCzBCAAKMIAIAAIGYAgACLbLCAAS2VgIAAt6cAgACRmACAAHz1MA==
Date: Fri, 16 Oct 2020 08:31:22 +0000
Message-ID: <BYAPR11MB3301D13C95E1E43AFEB203979A030@BYAPR11MB3301.namprd11.prod.outlook.com>
References: <20201010031020.349516-1-yang_y_yi@163.com>
 <43f71e6c9d2f4d5ba3ab56a921c5912d@intel.com>
 <BYAPR11MB3301BB7E22FF7268741D094F9A040@BYAPR11MB3301.namprd11.prod.outlook.com>
 <6167423037704e3382f85275be79de30@intel.com>
 <9239b2e.2116.17525095531.Coremail.yang_y_yi@163.com>
 <SN6PR11MB3310BA07C362EEAA2235DD899A050@SN6PR11MB3310.namprd11.prod.outlook.com>
 <cf33f4b88dcf44cf890fb8e6163ddb36@intel.com>
 <BYAPR11MB330178813E4F9DC71E5507EA9A020@BYAPR11MB3301.namprd11.prod.outlook.com>
 <29f24642c9fe498db60c8c3fa6ec0f1d@intel.com>
In-Reply-To: <29f24642c9fe498db60c8c3fa6ec0f1d@intel.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: intel.com; dkim=none (message not signed)
 header.d=none;intel.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: 46a55ffd-dd9f-40a2-614b-08d871addd76
x-ms-traffictypediagnostic: BYAPR11MB3221:
x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <BYAPR11MB32218613C058A574AE942EB79A030@BYAPR11MB3221.namprd11.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:8273;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: ztR83fxwunWV32ei61ub+I5YmBhxWXXKJ9VPZhzfLrCMwKJphIVytlWtoJGbZLxZD06zADL/qPmCG9ZfDy9+4a06BWAvHAj0NXHjjOORNDyRqq85OIlPQDre4/Yk3JTK5auIBbiw8TtBrEUOPUKuYuD+2VfmiDIoHAp5FHK5lLZiDJBC+TIwcA6angBHbR0RZrLDXh00gytNVw6xBSkHGXIj5f0YZaHsLWitAI0h6xxyQkU73QG/9kcCV9T7Ef46k2U2ZBL40CtCnYcc6EmGvdZOY8UgjncWJqc3uyJ1kRwlWEQHcIGr1iC0jpm9HZCEPZ5KppC6jLOisVeJCaNyYQ==
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)(376002)(346002)(39860400002)(136003)(366004)(396003)(66946007)(7696005)(83380400001)(86362001)(4326008)(71200400001)(186003)(8936002)(55016002)(2906002)(26005)(8676002)(52536014)(6506007)(66556008)(66476007)(33656002)(9686003)(64756008)(76116006)(316002)(110136005)(54906003)(478600001)(5660300002)(66446008);
 DIR:OUT; SFP:1102; 
x-ms-exchange-antispam-messagedata: iKVDP1ANJ61vQjvj3fmQTJfMHk6Ez8SFflWT25KpZliljGQiJ22Us2TdUD1vy0BTjxOTYkNG9biFJVPORa8Gzh904b2cC0aXMiVNnau/Ofh2KYydd+PU4bY1itIjgg+Udl3nx0NxH0VJrIyDXl/3y9og8Z8xCrFv7iCJi70HgBA1r/63d8mMYFD6iz5B8L3AFojuxouu7ZjMWKXEHtTXowqqdBw3er7m51qYvNvtgjdt5TQqtBgap3Q+AvMoZI7+37Lt/GZGDkkz1bEe8Q/vJIkKV/f8sr7cOumWTOyOYhrK442+2eh1vdOwt2vT29FULNps3IlMfXyiq3Wdlda7zZA6yMlYmg4RgbswPRsTRh78WF/6R/pITyFGjuzf6rS/3x2NdTVVxQgUZds8xqq6+dVYm6WOWxj5O2ferp/Lri6msqpYHSZ4NJnrcGxbIEKUTtV9K/Yl9oFNNzgph9j3r2X59OyghOkwVUUn3DRFTMNKRtWKEUWLcvXkz//FhppYB3/M4zLN7irf/NMrwua9w30QdTdDy85CGWcjmYK1L7HD6Lcr0bFcho0q9Tfl/wzJxKNxhepXb6Ks/6CWzfTnxKNpR6OqkGPm1+wtEme5oC1cn5PzzZyZN79qOW32TFcOuYYTWrvI2Tx/jVFlKY/m0Q==
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: 46a55ffd-dd9f-40a2-614b-08d871addd76
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Oct 2020 08:31:22.9420 (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: /N0EnMRi1i3XfponBGVF6HWh2EtLXojMJ1SLhWDqPBn3MIfzMHNlFEAbOx/8WfPzDixjl7ryzYTCYjRYyEVl4dcF3xQ70jRe51/hcYpYE08=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3221
X-OriginatorOrg: intel.com
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>



>=20
> > > > >
> > > > > 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 bet=
ter 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 i=
nput
> > 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 ex=
ample?
> >
> > 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:
> >
> > 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;
> >         }
> >
> >         /* 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;
> >         }
> >
> > That's why in rte_gso_segment() we update refcnt only when ret > 1.
>=20
> But in these cases, the value of ret is 1. So we can free input pkt only =
when
> ret > 1. Like:
>=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 > 1)
> +               rte_pktmbuf_free(pkt);
> +       else if (ret < 0) {
>                 /* Revert the ol_flags in the event of failure. */
>                 pkt->ol_flags =3D ol_flags;
>         }

Yes, definitely. I am not arguing about that.
My question was to the author of the original patch:
He suggests not to free input packet inside gso function and leave it to th=
e user.
So, in his proposition, would it also become user responsibility to determi=
ne
when input packet can be freed (it is not present in pkt_out[]) or not?

>=20
> Thanks,
> Jiayu
> >
> >
> >
>=20