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 9CD07A046B for ; Fri, 28 Jun 2019 09:10:40 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6BDD21B99D; Fri, 28 Jun 2019 09:10:39 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by dpdk.org (Postfix) with ESMTP id 12AFB1B99C for ; Fri, 28 Jun 2019 09:10:37 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x5S71tr6019464; Fri, 28 Jun 2019 00:10:37 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pfpt0818; bh=bxixx27tTgnOfUeJbOUooLdePQmpAbL12uXhm82mm08=; b=x+WinHeaDMmxVMyWD0mJPxCb7YY7eWVcK+E5KMfl8HrAzFjrV5cHF4HIUOaDsaDQaTCp 7GPpPQpSsFzWa6UO/viS3IM5UvwJc0WttNp6SZlY/C8mUD6Pn9XukfXUANJAfgFaTkA/ cEVTfFBrSLs0pnzuk04CiMzncLkr+DS5WZFJ6IxO3uZjYMfa5jQvuNt4r1n3o/EFIzoh EaFOb+lR4lvfcTEJIOP7Hnb95RCQ2ewkIt0+8XtFG/FERQiDr4w8oMgPAjlqOZ3aAj9+ LyAn4AOvjKkFHOfkImaogouJ0GZSGwe90eV0x7fUxTh0YF7aIoOzGCqMNRwVM5rD4gvg kg== Received: from sc-exch02.marvell.com ([199.233.58.182]) by mx0b-0016f401.pphosted.com with ESMTP id 2tcvnhc2qa-7 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Fri, 28 Jun 2019 00:10:36 -0700 Received: from SC-EXCH02.marvell.com (10.93.176.82) by SC-EXCH02.marvell.com (10.93.176.82) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Fri, 28 Jun 2019 00:09:53 -0700 Received: from NAM05-CO1-obe.outbound.protection.outlook.com (104.47.48.56) by SC-EXCH02.marvell.com (10.93.176.82) with Microsoft SMTP Server (TLS) id 15.0.1367.3 via Frontend Transport; Fri, 28 Jun 2019 00:09:53 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector2-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=bxixx27tTgnOfUeJbOUooLdePQmpAbL12uXhm82mm08=; b=gldzzQD98IUSps9hZYXu/jQzWvRbDiIui+mO43bOjykGRRVlWqX5QAAoFMUU5Ciwi5AeXk8DIozR3ne9SETiugX5wp3RAeo08J0z4isPnhCFDJUfDJEXChwCBy6INzcWdWn8/YXgbH8wwgfK7U4wN+qwzTOGONpcorEP2DGCjcg= Received: from MN2PR18MB2877.namprd18.prod.outlook.com (20.179.20.218) by MN2PR18MB3231.namprd18.prod.outlook.com (10.255.237.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2032.17; Fri, 28 Jun 2019 07:09:48 +0000 Received: from MN2PR18MB2877.namprd18.prod.outlook.com ([fe80::595e:3b6c:3d12:7285]) by MN2PR18MB2877.namprd18.prod.outlook.com ([fe80::595e:3b6c:3d12:7285%7]) with mapi id 15.20.2008.018; Fri, 28 Jun 2019 07:09:48 +0000 From: Anoob Joseph To: Anoob Joseph , Akhil Goyal , Junxiao Shi , "dev@dpdk.org" CC: "De Lara Guarch, Pablo" Thread-Topic: [dpdk-dev] [EXT] [PATCH] cryptodev: free memzone when releasing cryptodev Thread-Index: AQHVLYB44dK2dRrdt0mSgoIgk6d9pQ== Date: Fri, 28 Jun 2019 07:09:47 +0000 Message-ID: References: <201905301724.x4UHOJN7016223@lectura.cs.arizona.edu> In-Reply-To: Accept-Language: en-IN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [115.113.156.2] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: e98de291-ffb6-49b1-288c-08d6fb979b6e x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(7168020)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328)(7193020); SRVR:MN2PR18MB3231; x-ms-traffictypediagnostic: MN2PR18MB3231: x-ms-exchange-purlcount: 1 x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:7219; x-forefront-prvs: 00826B6158 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(366004)(396003)(346002)(136003)(376002)(39860400002)(189003)(199004)(13464003)(102836004)(74316002)(9686003)(55016002)(446003)(6306002)(6116002)(3846002)(25786009)(256004)(316002)(6246003)(66556008)(64756008)(66446008)(73956011)(66946007)(6436002)(26005)(66476007)(66066001)(14454004)(71200400001)(71190400001)(966005)(110136005)(2940100002)(86362001)(68736007)(2501003)(6506007)(476003)(8676002)(11346002)(305945005)(186003)(14444005)(5024004)(33656002)(229853002)(4326008)(486006)(53546011)(53936002)(81156014)(99286004)(76176011)(55236004)(8936002)(81166006)(7696005)(2906002)(52536014)(478600001)(76116006)(5660300002)(7736002); DIR:OUT; SFP:1101; SCL:1; SRVR:MN2PR18MB3231; H:MN2PR18MB2877.namprd18.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: marvell.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: QAcmz91Ztl7uM8Jpekism09WfJBo8ySkOpMfikDiv7Ule/T5Tw4zm3ec9b3XWzaYQ2LTtbmQd1jkeg2XCCtJ3SePuQE97t02b/9/yjSLhR9SFl0qZPxWRZea0vKIdG1iNViA4Ye4VR0xDoElAJg50fY+9fD4v4hnlwNDa4Of0zd1fa6JbqKAlVzBvlY2XvDTU51WcuV6gE8TBrALCU+D7tUTXFz52Z6HSA8PyhB/06QfpHh6mZfy4CpMWTAzE4MJoSLPT7wuigqQlBWOTC3EW1R3B+R5+mwJLBhilsbOBeCWb+b7oSx7pwVgVkN6DLHuxFXOqTd1+xQakktvJ1Uw/x/ptSkxYUuUNdrDrXldk+OhreifTGkpeYTBCVXG8SpMmqJakNkjifMV0EO+3gYUB2OJWMcxaoicdEcrbKUrbiQ= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: e98de291-ffb6-49b1-288c-08d6fb979b6e X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Jun 2019 07:09:48.3306 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: anoobj@marvell.com X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR18MB3231 X-OriginatorOrg: marvell.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-06-28_02:, , signatures=0 Subject: Re: [dpdk-dev] [EXT] [PATCH] cryptodev: free memzone when releasing cryptodev 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" Hi Akhil, One correction. Please see inline. Thanks, Anoob > -----Original Message----- > From: dev On Behalf Of Anoob Joseph > Sent: Friday, June 28, 2019 12:34 PM > To: Akhil Goyal ; Junxiao Shi > ; dev@dpdk.org > Cc: De Lara Guarch, Pablo > Subject: Re: [dpdk-dev] [EXT] [PATCH] cryptodev: free memzone when > releasing cryptodev >=20 > Hi Akhil, >=20 > Please see inline. >=20 > Thanks, > Anoob >=20 > > -----Original Message----- > > From: Akhil Goyal > > Sent: Friday, June 28, 2019 11:45 AM > > To: Anoob Joseph ; Junxiao Shi > > ; dev@dpdk.org > > Cc: De Lara Guarch, Pablo > > Subject: RE: [EXT] [dpdk-dev] [PATCH] cryptodev: free memzone when > > releasing cryptodev > > > > Hi Anoob, > > > > > > > > ------------------------------------------------------------------ > > > > -- > > > > -- When a cryptodev is created in a primary process, > > > > rte_cryptodev_data_alloc reserves a memzone. > > > > However, this memzone was not released when the cryptodev is > > uninitialized. > > > > After that, new cryptodev cannot be created due to memzone name > > conflict. > > > > > > > > This commit frees the memzone when a cryptodev is uninitialized, > > > > fixing this bug. This approach is chosen instead of keeping and > > > > reusing the old memzone, because the new cryptodev could belong to > > > > a > > different NUMA socket. > > > > > > > > Also, rte_cryptodev_data pointer is now properly recorded in > > > > cryptodev_globals.data array. > > > > > > > > Bugzilla ID: 105 > > > > > > > > Signed-off-by: Junxiao Shi > > > > --- > > > > lib/librte_cryptodev/rte_cryptodev.c | 44 > > > > +++++++++++++++++++++++++++++++----- > > > > 1 file changed, 38 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/lib/librte_cryptodev/rte_cryptodev.c > > > > b/lib/librte_cryptodev/rte_cryptodev.c > > > > index 00c2cf4..666dfea 100644 > > > > --- a/lib/librte_cryptodev/rte_cryptodev.c > > > > +++ b/lib/librte_cryptodev/rte_cryptodev.c > > > > @@ -653,6 +653,31 @@ rte_cryptodev_data_alloc(uint8_t dev_id, > > > > struct rte_cryptodev_data **data, > > > > return 0; > > > > } > > > > > > > > +static inline int > > > > +rte_cryptodev_data_free(uint8_t dev_id, struct rte_cryptodev_data > > > > +**data) { > > > > + char mz_name[RTE_CRYPTODEV_NAME_MAX_LEN]; >=20 > [Anoob] Shouldn't we use RTE_MEMZONE_NAMESIZE instead? I guess this is > also coming from the existing code in rte_cryptodev_data_alloc(). May be > we should fix that as well? >=20 > > > > + const struct rte_memzone *mz; > > > > + int n; > > > > + > > > > + /* generate memzone name */ > > > > + n =3D snprintf(mz_name, sizeof(mz_name), > > "rte_cryptodev_data_%u", > > > > dev_id); > > > > + if (n >=3D (int)sizeof(mz_name)) > > > > + return -EINVAL; > > > > > > [Anoob] Is the above check needed? > > I believe this being used while creating the memzone, so same logic is > > used while freeing it. > > Just to be safe. > > >=20 > [Anoob] Thinking bit more, it seems like we are trying to capture a situa= tion > when the name is getting truncated because of insufficient buffer space. = So > it is safe to have I guess. But even in that case, 'n' will not be greate= r than the > "size" field passed (which happens to be sizeof(mz_name) in our case). >=20 > My opinion is '=3D=3D' might make more sense. But I leave that to your > judgement. [Anoob] The check has to be retained.=20 "The number of characters that would have been written if n had been suffic= iently large, not counting the terminating null character."=20 Please ignore my earlier comments. >=20 > > > > > > > + > > > > + mz =3D rte_memzone_lookup(mz_name); > > > > + if (mz =3D=3D NULL) > > > > + return -ENOMEM; > > > > > > [Anoob] Is the return value correct? Shouldn't it be -EINVAL? > > > > > > @Akhil, thoughts? > > > > > > I believe ENOMEM is correct, as there is no memory associated with the > > cryptodev_data. >=20 > [Anoob] Agreed. >=20 > > > > > > > > > + > > > > + RTE_ASSERT(*data =3D=3D mz->addr); > > > > + *data =3D NULL; > > > > + > > > > + if (rte_eal_process_type() =3D=3D RTE_PROC_PRIMARY) > > > > + return rte_memzone_free(mz); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > static uint8_t > > > > rte_cryptodev_find_free_device_index(void) > > > > { > > > > @@ -687,16 +712,16 @@ rte_cryptodev_pmd_allocate(const char > > *name, > > > > int > > > > socket_id) > > > > cryptodev =3D rte_cryptodev_pmd_get_dev(dev_id); > > > > > > > > if (cryptodev->data =3D=3D NULL) { > > > > - struct rte_cryptodev_data *cryptodev_data =3D > > > > - cryptodev_globals.data[dev_id]; > > > > + struct rte_cryptodev_data **cryptodev_data =3D > > > > + &cryptodev_globals.data[dev_id]; > > > > > > > > - int retval =3D rte_cryptodev_data_alloc(dev_id, > > &cryptodev_data, > > > > + int retval =3D rte_cryptodev_data_alloc(dev_id, > > cryptodev_data, > > > > socket_id); > > > > > > > > - if (retval < 0 || cryptodev_data =3D=3D NULL) > > > > + if (retval < 0 || *cryptodev_data =3D=3D NULL) > > > > return NULL; > > > > > > > > - cryptodev->data =3D cryptodev_data; > > > > + cryptodev->data =3D *cryptodev_data; > > > > > > > > strlcpy(cryptodev->data->name, name, > > > > RTE_CRYPTODEV_NAME_MAX_LEN); > > > > @@ -724,13 +749,20 @@ rte_cryptodev_pmd_release_device(struct > > > > rte_cryptodev *cryptodev) > > > > if (cryptodev =3D=3D NULL) > > > > return -EINVAL; > > > > > > > > + uint8_t dev_id =3D cryptodev->data->dev_id; > > > > + > > > > > > [Anoob] Variables need to be declared at the start of the function. > > > https://doc.dpdk.org/guides/contributing/coding_style.html > > > > > > > /* Close device only if device operations have been set */ > > > > if (cryptodev->dev_ops) { > > > > - ret =3D rte_cryptodev_close(cryptodev->data->dev_id); > > > > + ret =3D rte_cryptodev_close(dev_id); > > > > if (ret < 0) > > > > return ret; > > > > } > > > > > > > > + struct rte_cryptodev_data **cryptodev_data =3D > > > > &cryptodev_globals.data[dev_id]; > > > > > > [Anoob] Same comment as above > > > > > > > + ret =3D rte_cryptodev_data_free(dev_id, cryptodev_data); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > cryptodev->attached =3D RTE_CRYPTODEV_DETACHED; > > > > cryptodev_globals.nb_devs--; > > > > return 0; > > > > -- > > > > 2.7.4