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 8F847A0C40; Tue, 27 Jul 2021 21:29:21 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 54821410F0; Tue, 27 Jul 2021 21:29:21 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by mails.dpdk.org (Postfix) with ESMTP id E7E5F410ED for ; Tue, 27 Jul 2021 21:29:19 +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 16RJPjcc018657; Tue, 27 Jul 2021 12:29:18 -0700 Received: from nam11-dm6-obe.outbound.protection.outlook.com (mail-dm6nam11lp2172.outbound.protection.outlook.com [104.47.57.172]) by mx0a-0016f401.pphosted.com with ESMTP id 3a2qg2g6xp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 27 Jul 2021 12:29:18 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BRT5Eq5yNUhJOfZSE+ysFRQWFT6jkcPznnm/UeWXLGoYzShwIQC3oNmQD4+BvAcwkF7TIfmgEvDZIRjBPh3C+MTLIvuEgd0fa/j+OXFZXAdjYtsqhxYT/t/HgQCF5vx4j9nsmMFo96p+d0rBVsyi6CQecBV0wExRBkYxFA4AxtAEG1r4r5SIk1n+e64mKljzyL4SirdpR2/gDg8aMJ0wYImKHoDEp+kf2wD2NGXGrR7X5+bGOnmF+SaECw9g4a7zCg42fcUDTo/DBWNPNnh9OIeXG+IYFxgvlBDHpK0HqvVaKxOrhlYN6KIGaGVpaZhNsi0H2y8wDT/frzcik8Ys2Q== 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=cZXb735YwHhPiEdvtmP8YD9/Mu6fd1ZL6qjAbk3U9kw=; b=VKlVFgC5D5He62mWqCd9afxJcN4SNWsqua4ZBY8LWAkxUjzUHhJ8/+1CD/fFEojkB50yIJgukVorW1fn+pT/I8ikHM3pIOu1uXn2KDw5GeIvcRc2l0vMblothOyw5B/kxKThxt7nb+ndCB60NpdxN3i/HBjuDv0g6nKVCho6M/zo1MJYHJRZE/Kl4x4m9ORtdiB8E/ZAYeHz3gq4atZwXR/h0c02THx4akQc/6uqisLfJOjBfAgEdryvanY+4uPdjGhC+Fwre41HcFtgAHubDloPn8If7IPEp3JTPUwFhnrWAjnTS1BQ22lrYWXRtgbe2ysTMXhK+oceIBLwUV73Tg== 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=cZXb735YwHhPiEdvtmP8YD9/Mu6fd1ZL6qjAbk3U9kw=; b=slrRtBUzvTgZVxe50gk33dsWIJqX/q9V8ZiQ69+lXt6P1KE0DIm6LI4+D7yZxcPLodr5sEAeJQsGL9vkI9tGGF+aqAS+nzyYIo6AZ6EZZFbLAqX2KTBPxPKjIA8CN5sR9q+UGjRhn1hbdn9X5RYjSyu2VyQSZtIDVLggFzD05Q4= Received: from PH0PR18MB4491.namprd18.prod.outlook.com (2603:10b6:510:e6::13) by PH0PR18MB4391.namprd18.prod.outlook.com (2603:10b6:510:2::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4373.18; Tue, 27 Jul 2021 19:29:15 +0000 Received: from PH0PR18MB4491.namprd18.prod.outlook.com ([fe80::d435:ad84:b25e:4ad3]) by PH0PR18MB4491.namprd18.prod.outlook.com ([fe80::d435:ad84:b25e:4ad3%6]) with mapi id 15.20.4352.031; Tue, 27 Jul 2021 19:29:15 +0000 From: Akhil Goyal To: "Ananyev, Konstantin" , Anoob Joseph , "Doherty, Declan" , "Zhang, Roy Fan" , "hemant.agrawal@nxp.com" CC: Jerin Jacob Kollanukkaran , Ankur Dwivedi , Tejasree Kondoj , "dev@dpdk.org" , Archana Muniganti Thread-Topic: [PATCH 2/2] lib/security: add SA lifetime configuration Thread-Index: AQHXfSwXa0JAhHVxJEmykXnTo0RT0atLZDUAgAnryQCAABJkUIABW52AgAB0IQA= Date: Tue, 27 Jul 2021 19:29:15 +0000 Message-ID: References: <1626759974-334-1-git-send-email-anoobj@marvell.com> <1626759974-334-3-git-send-email-anoobj@marvell.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=marvell.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 85072ba6-c98d-4d95-5fa6-08d95134d23c x-ms-traffictypediagnostic: PH0PR18MB4391: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 0OTv693TB6QkQa8hqjAtK7nq2IAuw7aExOY7mlx1XPLmKtU8z7OmuRd0Olc8HMLBap+fU/3ZRHMxR9gLDCDuRgFS3C7cjLQUKhi1IfpXz6R5IVQWoZDPbu8ljVDrIthJaOFd51FWlPeAIOe4eGdwcMq9YFT7PeiO4kU9KxXd2gg/otlaM0qxVhRLdLikMNmQOxEKLNP1AMf/FNAO/NI4ExLBiXh+93o7oA8p4N7MVH5TXHyMa7mlff9HL2QPVAcGV7df7JdHu7RWhvqzhC0rZjk8FedOxvaH+CfUDsgL9xMnaplNWfhpBbtaYRo6TAerMrZUDTGL8g41T3hOZxspAsO0ubu42Zxo+djrZLW0YmwpO/uTJKRZlyrUme+7s/123+Shw8QplJFtFlZs1iUjx6NDiE5WmWvy3NnSvTEdLzs/PF8WyshQHnptt5dJGfFnq77uxHshEczsu8kc29leKKfdezT5YhFPUyyRGDwOnNkrwPCLY64aSZyHnp70gAa34OqCVDuYyBEDuLvigKdAcKZgpoad/t48SV3F4TXa4rr7MJICM7Z0S2VNsYLLRxzFNdQxEeadBchhiHJWbazfQnPnuFXnHZnt+41HHpk3WYNP6f1Sp2ij1bYeJkip4dLNARlG2QgRlQ8JfTm23ULJsam6L65rWLFGGVJ/T5hwiDLl/zbm8UIipw0wJVa9EDz8iv/8QKMyObJq7GHsrzvqkHGM5uUJO80TkYQnsKWlCNb2L5OFgypJaftnjaylq6BNCmtAZbzJOblh8HbijLIshgMtkGNWXmJebwSaNbqwZc3yB/+ktLCsbzn4OILNpcR1koFMVXnt45Tgm7y+rFwusw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR18MB4491.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(136003)(346002)(376002)(396003)(39860400002)(966005)(83380400001)(8936002)(26005)(76116006)(66476007)(64756008)(2906002)(478600001)(66446008)(66556008)(110136005)(54906003)(186003)(66946007)(38070700005)(8676002)(316002)(9686003)(38100700002)(7696005)(15650500001)(107886003)(122000001)(52536014)(55016002)(71200400001)(4326008)(86362001)(55236004)(33656002)(6506007)(5660300002); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Owktbvu5XQL99jSWEt9JDwdCMORZquKa0XcUWbYdoH8HVuc4qPb2mDcEk8aW?= =?us-ascii?Q?LSgm59+kzhUTrpf5e9/ouLq+LKfaDFB4af9tbSFNQ9eArW6qX+JBeG3vvrds?= =?us-ascii?Q?eREpin+RfZ2tYOkYn+eLb2zRwbTYw6hnPE2Oqab0WOpaKRu9mwXfrIA1yzUT?= =?us-ascii?Q?aRjmuz3Ssigzg7hwgJKTHxJUgIBxJyMBONzX08EUomIaOVhkzgGfGwSq+DHP?= =?us-ascii?Q?4kUEEJiWOggm4zO+fOloAQduKdYctNhUXQi5GfSs/5G+elPr9Ac5K1u3K4tn?= =?us-ascii?Q?55Wac20vM2o+hmHOr5r8u8rXkdSkFvWNFCvi3y85W9zUfxHEm2aKm67Ihchi?= =?us-ascii?Q?dJ6zhRO07KsIezbdUccwU93g8K2S3Hr+UIDnK6nKvc8G6VKEQtqr/klNvCi1?= =?us-ascii?Q?udU9bWWRUEoUZO7vx+AjXUDg4uPxO6yMjOEfxhEckiDkjdQw2XBJ1Ud6+FxV?= =?us-ascii?Q?et4Fb8InDnT3HewZaE5EA9UuDAd8Xl4Dgomc36PcmifOEyFGvZyMO0hK+eRV?= =?us-ascii?Q?dBVYZV3Mlm/HhAlaqInLrCG9akWuzJFqJ/ZayBAePfD8gG+vn/jIvnGcUNOk?= =?us-ascii?Q?deodGmh32c+BfFzoJacFtusFMwFHzB8IDanmZ3dXlk+1xYah9fUdiqvMRvdZ?= =?us-ascii?Q?XLs3WIhLX9s8NJGXpdH5fI08yNpIbWvPp/Vduuc3UYvctn+mEwIDMBtHuHpa?= =?us-ascii?Q?X+zOPCBoNINKQo7U0XhdsZMmGZS3IgcgU1rsHROfx8jfooMO9mQkc5MWHreU?= =?us-ascii?Q?HwiqXVh3tVtqREOSndQqxDLWvLXDWy941TMXhtDtWh79soeUymRlIEL4LG/n?= =?us-ascii?Q?GJsEl2hrlyFWrK9sSS90lqUWSHA0ptDKHhVOtiJtigGQaWFb/jHULwalsd+v?= =?us-ascii?Q?OA+YrCXKbwSWQbTaX9K0y8uL2yjJiXEWVEk+uXaZpweoixKkHYk2vXKXFQXm?= =?us-ascii?Q?Okz7ibDGVDa0zIqmUNXQKIjUlrTqVyY2YOgfQPYnvcJUh/a5PsS0c2Z3Ndd2?= =?us-ascii?Q?i7bo2XuuVknXIZLbcNVMLpu+0rU0Fg9qAhXNOyQospx57jbXhaOdDReYw532?= =?us-ascii?Q?8ECCrSTJl76g/uQXYeIm800znia7xAliXwpwpig83bbD8Rx6jDyRnc0ADNpM?= =?us-ascii?Q?T4OEpSgJM6WvKi0RPSmfTM6Lh09yl+xz0c9LYhxv/W0K1obUSdjT/KwkyNQK?= =?us-ascii?Q?75dnm/OpDultifKJoL71jG7bLxJu1ct2WSb1Ai2Bw+o5luaI2qLmhNY5ozkS?= =?us-ascii?Q?nFfrAPif2K9jSh9RtHD960V0oY1BzO/PxWt4Ek1ioBsbu8sNEiKGDYcvY8co?= =?us-ascii?Q?DDqis5JofYCycapB8ZYFnehn?= 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: PH0PR18MB4491.namprd18.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 85072ba6-c98d-4d95-5fa6-08d95134d23c X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Jul 2021 19:29:15.5493 (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: MdHDDJ/GXvaXwvYYA7ldzR0l1qb0G49KYOnqyfje7phV3VrOUIo7KYY5chW7OKVl/seZINYNGLcaYHEfxKlLEA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR18MB4391 X-Proofpoint-GUID: vsZ88lVuQpH7W4DFGxHoWz_8fY8DFx8_ X-Proofpoint-ORIG-GUID: vsZ88lVuQpH7W4DFGxHoWz_8fY8DFx8_ X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.790 definitions=2021-07-27_13:2021-07-27, 2021-07-27 signatures=0 Subject: Re: [dpdk-dev] [PATCH 2/2] lib/security: add SA lifetime configuration 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" Hi Konstantin, > > > > There are two options that we considered, > > > > 1. Extend the enum, rte_crypto_op_status, to cover warnings [1] > > > > 2. There are reserved fields in rte_cryto_op structure. So we can u= se > bits in > > > them to indicate various cases. [2] > > > > > > > > Both the submitted patches follow approach 1 (following how it's do= ne > > > currently), but we can switch to approach 2 if we think there can be > > > > more such "warnings" that can occur simultaneously. Can you share > your > > > thoughts on how we should extend the library to handle such > > > > cases? > > > > > > > > [1] https://doc.dpdk.org/api/rte__crypto_8h.html#afe16508b77c2a8dc5= caf74a4e9850171 > > > > [2] https://doc.dpdk.org/api/rte__crypto_8h_source.html > > > > > > My vote would probably be for option #2 (use one of the reserved fiel= ds > for > > > it). > > > That way - existing code wouldn't need to be changed. > > > > Adding a single enum or multiple enums is the same thing. Right wrt cod= e > changes? > > However, if the check is something like > > If (status !=3D RTE_CRYPTO_OP_STATUS_SUCCESS) > > Report appropriate error number > > App code will need to be updated to take care the warnings in both > options. > > It will be something like > > Option #1 > > If (status !=3D RTE_CRYPTO_OP_STATUS_SUCCESS) { > > If (status < RTE_CRYPTO_OP_STATUS_SUCCESS) > > Report appropriate error number. > > Else > > Report appropriate warning number probably in debug > prints. > > } > > Option #2 > > If (op->status !=3D RTE_CRYPTO_OP_STATUS_SUCCESS) { > > If (op->status =3D=3D RTE_CRYPTO_OP_STATUS_WARNING) { > > Report appropriate warning based on op->reserved[0] > > } else { > > Report appropriate error number > > } > > } > > Here both the options are same wrt performance. > > But in option #2, driver and app need to fill and decode 2 separate > variables > > As against 1 variable in option #1 > > > > In both the options, there will be similar code changes. > > Do you suspect any other code change? >=20 > Hmm, I think there is some sort of contradiction here. > From Anoob original mail: > "Both the above will be an IPsec operation completed successfully but wit= h > additional information > that PMD can pass on to application for indicating status of offloads." > So my understanding after reading Anoob mail was : > a) warnings will be set when crypto-op completed successfully, i.e: > op->status =3D=3D RTE_CRYPTO_OP_STATUS_SUCCESS > b) It is not mandatory for the application to process the warnings. > Yes it is a recommended but still an optional. If we set op->status =3D RTE_CRYPTO_OP_STATUS_SUCCESS And then check for warnings with a separate variable there will be an extra check for every packet even for a success case with no warning. This may not be acceptable. Now, if we introduce RTE_CRYPTO_OP_STATUS_WARNING or any other warning, Then it would mean a SUCCESS but with a specific warning which application = can decide to ignore or process. All the enum fields > RTE_CRYPTO_OP_STATUS_SUCCESS Sh= ould be treated as success. Status is a uint8_t which can hold 255 values, we can start the warning fro= m say 128, Leaving behind scope for more errors which can be added before RTE_CRYPTO_OP_STATUS_SUCCESS >=20 > Though from your mail it seems visa-versa: > Warnings are just some extra error codes (op->status !=3D > RTE_CRYPTO_OP_STATUS_SUCCESS) > and obviously each app have to handle them. >=20 > So could you tell me which approach did you mean? > If these 'warnings' are just new error codes and app is required to handl= e > them, > then why do we need to introduce 'warnings' at all? > Lets treat them as error - add new RTE_CRYPTO_OP_STATUS_ error codes > for them > and that's would be it. We cannot treat warnings as error codes. These are success cases with some caution to inform user that there may be some issue in coming packets, eg s= oft expiry. The patch that Anoob sent and the options that I specified are inline. There may be some confusion with the wordings. I hope all your doubts gets = clarified After this mail. >=20 > If processing them is optional, then I think we better have a new field f= or > them > So app code will look like: > if (op->status =3D=3D RTE_CRYPTO_OP_STATUS_SUCCESS) { > if (op->warning !=3D 0) { > /* handle warning conditions here */ > } > /* do normal success processing */ > } >=20 > In that case existing apps will be continue to work without any modificat= ions. > Yes, they would just ignore these new warnings, but nothing will be broke= n. >=20 The existing apps can still work and but they would treat warnings as error= for the PMDs which can return these warnings. For all other PMDs, it will work = as is. But the application writer knows the features of the PMD which it is using And hence would need to take care of the warnings eventually. Eg: it will configure the soft/hard expiry limits while configuring the ses= sion. Hence it will expect the warning to come. Moreover as I said above also, there will be one extra check for each packe= t even for success cases without any warning which may not be desirable. As I suggested in both the options, the extra check will be there only in c= ase there is error or warning and not on the success case. > > > Again these warnings, it probably needs to be a bit-flags, correct? > > > > We can deal with both bit flags as well as new enums in the status. > > I believe both are same and in fact using enum in application is more > convenient > > for user, instead of decoding bit flags. > > However, it is personal choice. People may differ on that. >=20 > From what I understand from previous mails: same op can have multiple > warnings set. > Let say both SOFT_LIMIT can be reached and L4 checksum is not correct. > That's why I presumed that warnings have to be a bit-flag. We can specify enum names to combine the possible combination of warnings. Eg: RTE_CRYPTO_OP_STATUS_WAR_SE_L4_CSUM