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 D6CEFA0C42; Wed, 12 May 2021 08:47:35 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 609124003F; Wed, 12 May 2021 08:47:35 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by mails.dpdk.org (Postfix) with ESMTP id AD7EA4003E for ; Wed, 12 May 2021 08:47:33 +0200 (CEST) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 14C6kkuC031597; Tue, 11 May 2021 23:47:32 -0700 Received: from nam10-dm6-obe.outbound.protection.outlook.com (mail-dm6nam10lp2109.outbound.protection.outlook.com [104.47.58.109]) by mx0a-0016f401.pphosted.com with ESMTP id 38fw8yape5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 11 May 2021 23:47:32 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hsFA/WuCBl0Obi2YmQiPjO0b0mfEKa/6fW0EcLMijSvbgllsRiTz2PLxmMVhoEsU7qHED1Uk8j0ZH+H0uh8enhIc6grF8uTGRnTBy2iI1HWuMp74FBD1CReLslePQ15zGhU02vkt/OAVYKs5McwtUn5nkvqugaT3QdQeZqShmqg9uX8pXVeu2vakZymiZ6FsnUA+AD4HyzKLtT88XxisPDMuKrDSYX/KAa/XXrEaJrokGm6nYC0QhHJlxxREguVW6BuvD6loMWbPoTzMkhWsPva0zPjtkXPdKkN6hgv0fVUT5Nn+BBcOg4oId9RHDZV6YvaGVjmdQf+t15ii5L53WA== 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=267Kr+kMBCtbht9yU1db7K7Dg+k/Xoc/eh6XPFsLbPU=; b=K30nEvXnTPuzMkp5oD/nUHy/owMGkSDuz/HILj+3mzYm2ojMCQN/XVZO0Lq1i917MJdJflj1IK85NyJrmsgjFfdKHum+zpqFCCM/WD0Oeh3s8qGkQnKyRuC8cBg+VfM2dIdJjcO/mTXnxakBi2zON/INbFIQgJWc9FG/DWW6mj3E0mj/vFIzNHkP9XzKcWrXqCBV3dN5ztSEZnr8eOwpt2JfWwsImhD/upsraolpMw2fHUOLiB3YZZsahsC+ROFocUxb1q++OvzZQlw4+9eqLuBPYHSKKFEAElglqKj7hivPOrHIuYOZP/en9p5IXJiITgaqZkoWNzlYvsvVcB98jQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=267Kr+kMBCtbht9yU1db7K7Dg+k/Xoc/eh6XPFsLbPU=; b=ItigBx7f14jeX7yM+j6RQlCLzBbL9m74tUTU/++5rqe+jnASfLk7qgH+605umppVgGGRHsUCok2ZsDbB2ulJDERkcI1nX1UG2PNaN6DyVVn6BDXfeq9o0G/dTd/CK2vR00o5IDqiXRKqPgnW+2dBTR+q2y2NmWkPbqOJkBTQH40= Received: from MW2PR18MB2284.namprd18.prod.outlook.com (2603:10b6:907:10::16) by MWHPR18MB1535.namprd18.prod.outlook.com (2603:10b6:300:d2::17) 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 06:47:30 +0000 Received: from MW2PR18MB2284.namprd18.prod.outlook.com ([fe80::3168:cb00:6607:743f]) by MW2PR18MB2284.namprd18.prod.outlook.com ([fe80::3168:cb00:6607:743f%7]) with mapi id 15.20.4129.025; Wed, 12 May 2021 06:47:30 +0000 From: Akhil Goyal To: Matan Azrad , "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: AQHXQSnJ4KJlusD3MUiFAPGamVq8bKrZjGyggAFE9QCAABU0oIAAVUcAgANXCnCAANBegIAADQoA Date: Wed, 12 May 2021 06:47:30 +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: nvidia.com; dkim=none (message not signed) header.d=none;nvidia.com; dmarc=none action=none header.from=marvell.com; x-originating-ip: [171.48.51.1] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 8b3fdb2b-f624-4814-8c96-08d91511d07c x-ms-traffictypediagnostic: MWHPR18MB1535: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: ARv5i+ONDXLzrgeYwhOpA4MRmcgXAVJ9h0Z4K3n84JbLyWJTbqqMk2LArnFZe5Vip5RqvCsXyd5xSTMrso/Th0pf0yyyOUdovYhodPp+QuF9qY9Cy66DISnOFC7Dy5Z7L8fP/sDGCUycy0GQ38mAJH5FNss0i3l92fi0+2XKu5D+1pz4LplngKIAOXw6ChRXV6ZMVEDdZDCASenmkIWrH6h70OiCE8enFTJChaMoF3PFH5FV94ADpoBpDYi/1rhsFsx0c3JwfpC1u6T5NhJ4c2APQrKETeC+F0kVT3RafZMxQ/vBCcAFEKO3HOvQRjpmRykItsiNLb6u1VY5UePTg4/ubZAZffKMeGgaSJvWZIv5DdiLBrglwlzZ2NpUnnf2jHRxL1hutv+l4VMBGtciCkjorHydRvgDxd/GiB440R+3rPUnwnSQd3mWPcb9X9aZCC10MhvQJmSZz5DipQbzleQuIINgrgkXvR7d89op4LCfoLWb1Y21iz2W5HYmX9X3OXdj5YFnRFbtPoFP/eXK3XPgSWKWfBPyX/1Cm7KDtYhbx7tFsUJjIH/fshm9wXTneZx7OOq7a4jkcYzR0JSCi0y/Qdt1XZQb2JfkTUs2Cmc= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MW2PR18MB2284.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(396003)(376002)(366004)(39850400004)(346002)(136003)(33656002)(86362001)(38100700002)(122000001)(76116006)(64756008)(71200400001)(66946007)(66446008)(66556008)(66476007)(478600001)(9686003)(5660300002)(2906002)(4326008)(110136005)(316002)(55016002)(54906003)(8676002)(186003)(26005)(8936002)(52536014)(6506007)(7696005); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?fOFJrBLF/NJ5ABOagumrQQzJ6DFIIW+/2Ic64Ox2JUGgfIJ/2AzEdkn201wE?= =?us-ascii?Q?xP3LXfPb8J7wk421ITVmGcX4j1jdjXralhoktrscDFjbaz0RmZgctvrT3hwJ?= =?us-ascii?Q?s7ZN75tf23J+pcS6HXixY4dPqTjiqxIVzGkgBJ+3AnlARRUPUL32g68vvNUv?= =?us-ascii?Q?sE7XvXtzX1u3AuaJHvDJS3SzIJ+ufiXjsN+SYlyAMSgpb2exMmhxPBQTgHrR?= =?us-ascii?Q?ieyza66rXJqi2V+/2xGM4ec0rHeGc8miL09ZVZ0iw/d3/HrFEoNJCKpgQrvR?= =?us-ascii?Q?miMSuIy4/25qjRCSIgVLOACrh2kDeT84QVm9TY77apk1lyJq6VtmtCSUzREX?= =?us-ascii?Q?CCoiBA/RbB1JEIxLXLL7fo0BIWxU/vv/h4z0lgTjye3NBtxm79m596QtQgds?= =?us-ascii?Q?ZqwUHu7ZTKAdNGFU/74BpqyG0VUkDw3KHIGuefr7wfT6jpAMCJdis/d/WaJN?= =?us-ascii?Q?v4BhAlhX+sneclYLX/fxgSBlotweizWgxMc75vzarcxCgCFA6Y2Wv2TxSlh/?= =?us-ascii?Q?v6mekzhOCMBHr8gkCbWbaMt4VfDX6z/2ltBWR+S7/zEzlFsXy0nmE28ZRg0n?= =?us-ascii?Q?kdB4F1x3ssdRmyFmjKUItVbdmPetxDnQMM0b1gWe9W5+IGtYlIx618XxHEHH?= =?us-ascii?Q?lM4rfKqcPFx9ZjO70oZW/8gvZFmAM4IqicfV3FGUXmIrqJs7orTtvGZv88u5?= =?us-ascii?Q?m2Z5oSFWeYTzEPSmdhKi2vjwFZdKo/C1uAUfqQQqZxE8N9iEKtdmeXvtuiVp?= =?us-ascii?Q?wYLPs/3Gi03d27ScT7B/Y7PQGVr/mmMS2CpApJH/LatA/EIGOmffuvPk6Ghy?= =?us-ascii?Q?mZzUxI7jcZpGq/KwjGrpd0r5a5SWfoV2cVmt9tZmxEwZ4R4q7+JMQGzH3eV7?= =?us-ascii?Q?Zi4TtGzPxYGqub07D2VJwKao92222EY5iS/bD6mAOpIfclZYCve7JK9ENel2?= =?us-ascii?Q?DhfxkyihGRJn/nydiHmBueSmDgRbcDU1eYY3y+vV76C53eGwRCDA6qcBQgcA?= =?us-ascii?Q?KJEUhEEFrGJteZja4KtPqC+xfohrge8juwfbdxpmjaKAluc57LjSqJlKWbne?= =?us-ascii?Q?ag8tWD505nvpLR3TEg/rxLMHnw3ra+7u0BuU8NO0Zn+xrRX2wZNOi7YHhdVQ?= =?us-ascii?Q?OST/a0pqVKmbQN3MhbEXGhUQpWCj8D8fJ3qgkoGiJwqmAWNihKQ2pdPi52Ne?= =?us-ascii?Q?edGGWKcZxLjYFbj27NXmJs4XmYbLEH3MnanBkQYQEpNl0KWNMqQDjGvE4dB0?= =?us-ascii?Q?6oYB1FhutAVetG8dBQvQJ2u9mDqGJ1pREBvGmMum/QhDg5DKUUnUnhY+MKau?= =?us-ascii?Q?duk=3D?= x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: marvell.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW2PR18MB2284.namprd18.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8b3fdb2b-f624-4814-8c96-08d91511d07c X-MS-Exchange-CrossTenant-originalarrivaltime: 12 May 2021 06:47:30.4287 (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: 94A3TE8wdIGAYHG9OMH5TGybWL5GqZc3t/7Uzwu/6xTo8MNGjiVwFQf1A66SeCT08uZUpGN1ME0wtGugh99+Fw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR18MB1535 X-Proofpoint-GUID: YZHud4qNywzgxEH0-nFnIB-Yx6YCvx82 X-Proofpoint-ORIG-GUID: YZHud4qNywzgxEH0-nFnIB-Yx6YCvx82 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.761 definitions=2021-05-12_03:2021-05-11, 2021-05-12 signatures=0 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 > > > > > > > +static void > > > > > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev, > > > > > > > + struct rte_cryptodev_sym_session = *sess) { > > > > > > > + struct mlx5_crypto_priv *priv =3D dev->data->dev_privat= e; > > > > > > > + 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 private = 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 app. > > > > > > > > > > The only app we tested until now is l2fwd_crypto and it works fin= e! > > > > > 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. >=20 > We can, but this is not our plan. > We met all the DPDK rules to push it on time. On time!! Really? Incomplete v1 was submitted before RC1, The complete PMD was submitted only during RC2. >=20 > > I believe test application makes > > The PMD look robust as per the DPDK crypto PMD API usage. >=20 > Every test will add robustness to the PMD. >=20 > > I haven't seen a PMD getting merged without test app. >=20 > compress/mlx5, vdpa/mlx5, regex/mlx5, net/mlx5, vdev_netvsc.... >=20 All these are 'not' crypto PMDs. For crypto, UT is a must. I would like to add a statement in the guidelines= if it is not there. > > And I apologize I did not mentioned it earlier, but it is kind of obvio= us thing > to > > run test app before sending it to upstream. >=20 > In fact, no, I added more than one PMD, no one require specific test. >=20 > > L2fwd-crypto is not doing data validation hence you cannot be sure that= it > is > > working fine as per other standard stacks like Linux. >=20 > 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. >=20 > For us, the current validation is enough, we don't support a lot of thing= s in > the crypto PMD currently, only one algorithm in the most basic modes. >=20 > For sure we will continue to add tests and to increase robustness. > Also adding more features is in plan for future. >=20 > If you postpone it, it yours, we don't agree with it. >=20 Please fix the review comments by today, if you want it to be merged in RC= 3. IMO, the driver is not ready to be merged.=20 I want to postpone it as it is not feasible to fix all the comments by RC3 = timeline. 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