From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 90843A0C42; Wed, 12 May 2021 09:25:09 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0FF84410DE; Wed, 12 May 2021 09:25:09 +0200 (CEST) Received: from NAM12-MW2-obe.outbound.protection.outlook.com (mail-mw2nam12on2077.outbound.protection.outlook.com [40.107.244.77]) by mails.dpdk.org (Postfix) with ESMTP id 3380A4003E for ; Wed, 12 May 2021 09:25:07 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Qhm6N9HufBVE18eW2DVh6vsQPkIIxrgFmbXwqmjFc2CuLOw+fCXtE3syXbpRAuOW3VeqdVUKZHEMaaQWMnkkcS1NvghUT2ennkGjYFDpsIcrUHvYbO8ZvY8nKRMvAbVda6xg5XkHgOIApbVz2kHlu/pDiJl5fTs3a1V6YP9UbHfJMrMkrlUsGIiAMFdN+Xfig/9TIXkvdTVxGUKF/24E8Rt/khUfjr6Kx9tdVwQYRSVEw8DUofNJU6yQUrccTCg7thxVj2t7ZpiMekJEJadFYRkSjbkZzLnPKgt2HnQnEaqTpbfti8OsRqnJL2A369T1MDrqXWCBmEpYZXWREjK5rA== 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=98k9n/rP/2ukqloE5n3tY6Q+3wKyVWdDBMI7FIREF5k=; b=fVfnHJqy9uDxKTSWjzYaZBcqvxDP/NI+WHkcLOB2U5/BY3cTRcB2WjODyJNMTk+C6cqpOs/xKukRDP4RlCIg2WxpcvRlOWfkpe7AnY/EwSrVdYY8/s+OsY6MkPPNcUIexFksBhL0ZOazAkV5RPFbcSsNYbt/KSFv4rNUGFV7yQYDeBCgj1U/T3JVt/2k33D0+YQTyarCggwKMLGKyKNI3s3myPd9QDJXxP1PWElDHPlEXQWxq9gCnDdJoN7gBvEj1zv63riXMqxuzcnP0p7WnMCZYYW9W+ttEn8UnV6gnQAuvsVGaW0jHuxXPqN3oYt+TQWkMLrwScA0lhAwPO16bg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=98k9n/rP/2ukqloE5n3tY6Q+3wKyVWdDBMI7FIREF5k=; b=GX7r6edTbviJS3YCyx5TGqCyiicdoex4Fq95SU+b1G86Y7y2pAZrx2V1t8dCu1vLf2i1457Nw/ROIAoCILIfbAiv7Id/Kwrz9i1OYHyhftaMvhojs9sHjryBT3X0BT1czGday5kll/6TiBuWpi/O+Zp+BWroaYggIyOUPrmeBE/9HITje3E07JWw7zZtOzV6AcXUITonfli4ljxQdnaeCU50NY7Gam9/ZKZLd9IDZR/XNLa9neEpVx9/F+I/7EuflJ023jKxP4ZVdBESNg5p3pwjYISrQ41hK7yoam94+k0PfnqYluKqog3PO69gtFVyEHh7a/5HPoBDqWuVliZscw== Received: from DM4PR12MB5389.namprd12.prod.outlook.com (2603:10b6:5:39a::7) by DM6PR12MB5567.namprd12.prod.outlook.com (2603:10b6:5:1ba::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4129.25; Wed, 12 May 2021 07:25:05 +0000 Received: from DM4PR12MB5389.namprd12.prod.outlook.com ([fe80::70df:ef1e:d98c:ce4]) by DM4PR12MB5389.namprd12.prod.outlook.com ([fe80::70df:ef1e:d98c:ce4%6]) with mapi id 15.20.4108.031; Wed, 12 May 2021 07:25:05 +0000 From: Matan Azrad To: Akhil Goyal , "dev@dpdk.org" , NBU-Contact-Thomas Monjalon CC: Suanming Mou , Shiri Kuzin Thread-Topic: [EXT] [PATCH v3 03/15] crypto/mlx5: support session operations Thread-Index: AQHXQSnFZsqHi0ROBkyLX1cinh9IRKrZjXoAgAFCeWCAABcSgIAAVLhQgANYqACAAMmmIIAAFPwAgAACqBA= Date: Wed, 12 May 2021 07:25:05 +0000 Message-ID: References: <20210429154712.2820159-1-matan@nvidia.com> <20210504210857.3398397-1-matan@nvidia.com> <20210504210857.3398397-4-matan@nvidia.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: marvell.com; dkim=none (message not signed) header.d=none;marvell.com; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [79.179.12.22] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 7eec89c7-856c-4204-36ee-08d9151710a6 x-ms-traffictypediagnostic: DM6PR12MB5567: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,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: ZqDZRKmCCaiPQIUGyeBjVZHpTLrxVR9Ztv0T5DcG+uw9cNBARJwBdf5zoJaa7Jf2YCZeW6RYtgbfka+8aiA1gS8KLNMw4qI/al/HRCONcCGoiBObGevb1X32xeBFSGvRgFtL/1+BL0WCB+n6w5OLTr4wc23/1YLF7wGru08fVwpTrD8MC+psdQCpHGQYEwxIQYZ8j2mLaxbGOGrr5NKfHbPX6+7choLUZKica9zKtHVIwc2hCumS42VQs2JzQ703mGHBH9yCRTGwIEQU8uMv7n+FgC7aHTg9lOS1F4s9mx0hK+pc+vYc7+zNCjEAzZpnrA8zoNlHBHYB96XvePq5S8riVQr9S8vsfm5IAH34NrIdNKjj6fEuyNIbkVPlsTG2poXMCM+giIR1kgzuuTG8tCfHm1lJJKkSyQvmgkjlOpdUcEesmMpv8qCERjocYNQxxX20I7AQST+prPWI1UuaYKjnfolJqOG3pynURA050sEDHAf+Put1kOTxue6uaFMIxN7Lb9e+aGSVLmAdaRFDUsH9cnKysb+CxRIF1pOPLt+x/lBXH7G/6tQXPRZCPr1OSILYw6GVQii3M9CwQe7O4SQU9khenmxOYecwwMwKo6s= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM4PR12MB5389.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(346002)(39860400002)(396003)(136003)(376002)(366004)(8676002)(54906003)(9686003)(110136005)(55016002)(316002)(186003)(5660300002)(478600001)(107886003)(4326008)(2906002)(6506007)(7696005)(52536014)(26005)(8936002)(66476007)(66946007)(38100700002)(122000001)(33656002)(86362001)(64756008)(71200400001)(66446008)(66556008)(76116006); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?UpYoxJS+ulHCJ/CAScrozAehWB5OA6OtX9G8BNywx5S9M6J8idP0JoTCn5MK?= =?us-ascii?Q?i1h6NUhanfWgySPJW80l8ZrBXSf8qeiux5oP00Wxx3yJVo4hoBnFtWcEevW3?= =?us-ascii?Q?hjviRVlnGf7RBRa2PiDLFsEvJuCVXYtC93Pi/1CvaBQSQ5WcttDQz03Hx4Sv?= =?us-ascii?Q?oDz4p2LSXGkArRI3ta4xCWlxYO7mPm7VsBVpJzWu1IKWdEUrwldzc2mtutGY?= =?us-ascii?Q?10bJgekt2+8+r0fvl10omUJXgEUIzf4yMKAw7SnnRI74FPrEHVyKd5lUA4z8?= =?us-ascii?Q?7qDQe37lvOz//10MNmIfoH6I08083xhr6F2mYFWfOpgltcHt8/did4vazRhW?= =?us-ascii?Q?jNENC1Txa14Gl9b9BTnuzndDI1eKMYDPqeYlINz6j8Gx3Wc+iMxFEPaZfMqr?= =?us-ascii?Q?4MJYeJuZj9vNnfdALD5C7FQxO7k1qpfj3ZtugtdWlgsCMzn4j059Hho6iMNK?= =?us-ascii?Q?wfT5cc0RbdCAloiSSO8Y8s6D8oY2b5726QIf8GeB+NwQbvGMHMIvsog2/OmU?= =?us-ascii?Q?9dzrOKvRDFLZijhCFQKF3wdGeJgqFjgAoBoSxPiIpHEBP3yFKujqzHZtL83I?= =?us-ascii?Q?EcalYm20e1CvN1I9/c3BtyK/S7WO6YTYi5vhdN4aiFkZ2XhfPYATgu5snJdC?= =?us-ascii?Q?lL7GGfvrRcX9vLpb+veIKaHje2oi0/MBTOF+re2sEVhVzjRdSC4BoTvkbwZD?= =?us-ascii?Q?G4E1CNMVnaNkjjXV+/Xz6E6OEDfZeYpY2LXeme7iWXYcyO5w1BAhBkyyLwo8?= =?us-ascii?Q?p02ATXMuKduugLmkulpW0KrCepNtakSV58LMGwEiNGgb9Q6f0t7kRVyL12lX?= =?us-ascii?Q?AW37SLISzYwP1z8LGAEU2FiOFP8xCh4B5XpMnl0eOwois2Dj6Te10EzJIEN0?= =?us-ascii?Q?D96Sz15MKgFu+5wqvwxSOO33UpMMjxWOnXjACXeU0dzb8eywC3cbz0wLZ7ZY?= =?us-ascii?Q?ARFWBiSfUpjZSqNHOD3XJZlS/EVii1AtrGcwi5TEak4VeD8LvLhKoGTMqqzv?= =?us-ascii?Q?LS6kY2xI6C05RVLj9GspiKmOBmIdQ/wcgqZv/bWdGSoinr7NNodgSpGMqdEX?= =?us-ascii?Q?r6w4YAfdv5N2GOUoOK7T8XFVkfQl2Eb2/48EmFH5a/I40QlcI3T662bViDWH?= =?us-ascii?Q?eB5MXAx+igxp4Mk76wyNbWqRxR8mK1bW0LGt8rklU2cPo+3oc9uCIlvSL1sI?= =?us-ascii?Q?f/eWInJVu6oU7eIqo3XoyCCyCThklQzc8qY550yQ5HtHxJUyl7waCDM+HJO7?= =?us-ascii?Q?rP/Py6KL3mIxSn8Sl15jj19wkbpMCPdnyThmDX0tlk2G4c+renn5MSiwXRGl?= =?us-ascii?Q?7dA=3D?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5389.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7eec89c7-856c-4204-36ee-08d9151710a6 X-MS-Exchange-CrossTenant-originalarrivaltime: 12 May 2021 07:25:05.5732 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: ko5GesGnUFb9R0PIaWl7g94pzRaB5b6bdAtwn7maHeNhicfjq1GOM2fl8rGN7hjwWhlKrp0/eYBZP2WKWezvlw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB5567 Subject: Re: [dpdk-dev] [EXT] [PATCH v3 03/15] crypto/mlx5: support session operations X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" From: Akhil Goyal > > From: Akhil Goyal > > > > > > > > +static void > > > > > > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev, > > > > > > > > + struct rte_cryptodev_sym_sessio= n *sess) { > > > > > > > > + struct mlx5_crypto_priv *priv =3D dev->data->dev_priv= ate; > > > > > > > > + struct mlx5_crypto_session *sess_private_data =3D > > > > > > > > + get_sym_session_private_data(sess, > > > > > > > > +dev->driver_id); > > > > > > > > + > > > > > > > > + if (unlikely(sess_private_data =3D=3D NULL)) { > > > > > > > > + DRV_LOG(ERR, "Failed to get session %p privat= e data.", > > > > > > > > + sess_private_data); > > > > > > > > + return; > > > > > > > > + } > > > > > > > > + mlx5_crypto_dek_destroy(priv, sess_private_data->dek)= ; > > > > > > > > + DRV_LOG(DEBUG, "Session %p was cleared.", > > > > > > > > + sess_private_data); > > > > } > > > > > > > > > > > > > > Memory leakage, mempool is not freed. > > > > > > > > > > > > Yes, good catch, this part was missed. > > > > > > > > > > > > > IMO, this driver is not properly tested with the unit test ap= p. > > > > > > > > > > > > The only app we tested until now is l2fwd_crypto and it works f= ine! > > > > > > We can add it to doc. > > > > > > > > > > > > > Please add a note in the documentation that it is tested > > > > > > > with > > autotest. > > > > > > > > > > > > > > > > > > The next app we want to test with, is test-crypto-perf. > > > > > > > > > > > I would recommend to run the test app first. > > > > > It will catch most of your basic bugs like the one above. > > > > > > > > It is too late for this, will add the test adjustment later. > > > > > > Can we postpone the PMD to next release. > > > > We can, but this is not our plan. > > We met all the DPDK rules to push it on time. >=20 > On time!! Really? Incomplete v1 was submitted before RC1, The complete > PMD was submitted only during RC2. Yes, on time, you got draft in RC1 -- most of your critical comments are re= levant for that version.=20 =20 > > > I believe test application makes > > > The PMD look robust as per the DPDK crypto PMD API usage. > > > > Every test will add robustness to the PMD. > > > > > I haven't seen a PMD getting merged without test app. > > > > compress/mlx5, vdpa/mlx5, regex/mlx5, net/mlx5, vdev_netvsc.... > > > All these are 'not' crypto PMDs. > For crypto, UT is a must. I would like to add a statement in the guidelin= es if it > is not there. >=20 > > > And I apologize I did not mentioned it earlier, but it is kind of > > > obvious thing > > to > > > run test app before sending it to upstream. > > > > In fact, no, I added more than one PMD, no one require specific test. > > > > > L2fwd-crypto is not doing data validation hence you cannot be sure > > > that it > > is > > > working fine as per other standard stacks like Linux. > > > > It is not do data validation, but we check that the packet payload > > return back has the expected encrypted\decrypted data using open-ssl. > > Also for us it was mandatory requirement to check it. > > > > For us, the current validation is enough, we don't support a lot of > > things in the crypto PMD currently, only one algorithm in the most basi= c > modes. > > > > For sure we will continue to add tests and to increase robustness. > > Also adding more features is in plan for future. > > > > If you postpone it, it yours, we don't agree with it. > > > Please fix the review comments by today, if you want it to be merged in = RC3. > IMO, the driver is not ready to be merged. > I want to postpone it as it is not feasible to fix all the comments by RC= 3 > timeline. Yes, it is too late. Let's continue in next release. Thanks for your time. > I believe all my comments are valid and need to be addressed. > Can you get somebody else(outside Nvidia) to ack your patch and counter > my comments? > I will merge it without review based on that Ack. > Regards, > Akhil