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 B50F1A034F; Wed, 28 Jul 2021 12:59:54 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3181340E64; Wed, 28 Jul 2021 12:59:54 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id 6480F40142 for ; Wed, 28 Jul 2021 12:59:52 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10058"; a="199859509" X-IronPort-AV: E=Sophos;i="5.84,276,1620716400"; d="scan'208";a="199859509" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jul 2021 03:59:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.84,276,1620716400"; d="scan'208";a="435080097" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by orsmga002.jf.intel.com with ESMTP; 28 Jul 2021 03:59:50 -0700 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10; Wed, 28 Jul 2021 03:59:50 -0700 Received: from orsmsx604.amr.corp.intel.com (10.22.229.17) by ORSMSX607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10; Wed, 28 Jul 2021 03:59:50 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx604.amr.corp.intel.com (10.22.229.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10 via Frontend Transport; Wed, 28 Jul 2021 03:59:50 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.105) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.10; Wed, 28 Jul 2021 03:59:50 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=B+d0z6qoSFmaDh9fPpGXJQzN1gO5O7NzvCf3QKHxzRfuk3Xz3NeahTqOvGc/C78P/qYWQ/V/2fw97M45JRmjLr2uSWZXDHPxKMP2ZBJ2XL907Z1tirHjtw6Gm9qoSvAFGAjGVvxBuBN4Xl5ngjGE8YG6zb9ypgovOB0c4/xlAzgh9I6n9ljhLmP+B6MKCua+BCdMhM2S+j8LiTjWq7RSQt9qDMZ61a8CBhLSfH873c3MEUCcY7WZanc+Yuucu1qcakJXH0yR7XEd4Nj7oeRQhdLXo7GqexXvuHlpClWlAHQ+eoHk5KmYD1WHKXJ/MGjSdC7XA/q4N/sUyqUK4DFPPw== 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=21AtJslD8OhZ0bWawfjPjz25f0pZBfN58NMoFW5hnLo=; b=bgzSPk5DrYrxUHnukLNmVfMgKCczExmCpbMftDmGUiQGOKGmioM38ABEr/Xioi1xm5vM2krf8Y0W29K0hCB4GZt7Psu8V/RIj9gFksv9Oqlr0mxZL1hLRJ/SGpVOrjjBEqa+XJxco0/0h/F5rh7J7PTfM/y2UcnD415luKfWcleYS0KC2w6TU5AuRVeVgi9eYxgJwG9R4caeONKyu+YxYy86js5feIdeXMfc07kEqV4bJWy93HWMl8KEmm15BqLIGdMdUlFNGU1ehCnWCH3ATgpk+BiwDiUzIFsuHyCgOYH4c2lBMq/Fb3SB7l26dTS1iAIOF29OHHPjogwemJwZ3A== 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=21AtJslD8OhZ0bWawfjPjz25f0pZBfN58NMoFW5hnLo=; b=atwbFdekkQWXjUNwW9ees0pXMQc0aWcuaa4wZXt6OV3Tf/SU2jw0Z5LfzVLMAypnjbIcSZrSzXBPTc8ZZKL/4xchgEXZximlGKQ7NIQvglg3H+WSXiH2qzmQg5hssjoKdhxZZ7Rg8GEnvrxE2UZAkFYnuVY2uL+k+Hb/+X5DOO8= Received: from DM6PR11MB4491.namprd11.prod.outlook.com (2603:10b6:5:204::19) by DM6PR11MB2921.namprd11.prod.outlook.com (2603:10b6:5:70::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4352.25; Wed, 28 Jul 2021 10:59:48 +0000 Received: from DM6PR11MB4491.namprd11.prod.outlook.com ([fe80::7dc4:66b0:f76b:6d48]) by DM6PR11MB4491.namprd11.prod.outlook.com ([fe80::7dc4:66b0:f76b:6d48%7]) with mapi id 15.20.4352.031; Wed, 28 Jul 2021 10:59:48 +0000 From: "Ananyev, Konstantin" To: Akhil Goyal , 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: AQHXfSwfZZ8LQxx0IUyMwgdib0ZEGKtLZDUAgAnqBmCAACM3AIABQWuwgACOKYCAAO2NgA== Date: Wed, 28 Jul 2021 10:59:48 +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-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: marvell.com; dkim=none (message not signed) header.d=none;marvell.com; dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: da757ff8-6000-48ee-3767-08d951b6d13a x-ms-traffictypediagnostic: DM6PR11MB2921: 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: /XJfrx4CIu7R8OzOiEpOiFSAt2jsYWODDV3TqNtmQ8fbzXuAasM7ROZZngshebzlNnrI1TCqQVk0JK9GbLvhivfnE30lw78CxFY6H0N5w4M5lAOgXKelKRW3nVRfrTE0KCbY4gs2FbzB4LvIoUtvWR4L3ZV0VDhNFL/+N4OBl5/LnLOzwfsWcs4SbmyHipZuAgv0HKsG2VN3z4L/1df2V8uNS9kVdIeK5kQ8n+KBIjQj6bC49oF+yb3MVSFPjFfU0be75m7RKwHGDmvhDXmv3qPGoY+Agr90ph7GSOso/ZenwBRYFfMSWfGZqv4rN9w/HDtdpUUKlnlqXOF5LNkKS/svJvvmEfAawHz2m9LYoyKX1KMQSq67qDODQ79wFsO3x2yp//xlXHxm1te9Y2Oa2mS6MP/tG+uagij3qBHLm763D2Ynvi00fyw5Ha0QWAgQDRUIUCAgL7LzsXSM39kMljRqZSMqTiq9ffvNadzagHanY1L1NHhLhcS7aNCJar4sppg1YS8Sw3uatmKiMD0gf3/uGf67v1NGgvwnqf76QVqR/B/+ItlW3zTloTTutMhj9tr4nalxFaYlRj9AtMZAKnkSBzCvuHon/g5c/l1PgRwper/3JSsMcsHQKfRNcQxAekRBdcsFvDcAKID/wHu6vpBkBXalJxJRYc5qkUHFHHRrDNSUK/AtRB5P4NMk2qiJ1vbGftfgUyoLquysSNHg0rh6fjhAJb/H9TXX2HxUjqOoqyA+7hnv6og+slW/iKad63UCC520hwbCoxhCKntg8dDqB04BgWyky65xyeNoAB4FVRc0LbaEIucGdc/nYk3c/kymG4aOYpBgj1/fYNcgHQ== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR11MB4491.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(396003)(39860400002)(136003)(376002)(346002)(54906003)(110136005)(15650500001)(55236004)(4326008)(38100700002)(64756008)(5660300002)(76116006)(33656002)(66556008)(38070700005)(66476007)(66446008)(86362001)(52536014)(122000001)(6506007)(966005)(83380400001)(7696005)(9686003)(55016002)(8936002)(66946007)(316002)(2906002)(8676002)(186003)(71200400001)(478600001)(26005); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?WKmrYQlYMxL1TEcvB9v3/caANHR/ChXBxaO+2QsILRKdZc4lMFlKpRfO/NKc?= =?us-ascii?Q?ykAqsE9+tKAMKwQLZsahFxkLZlg0uh+DWNG3UXIHhoFPFsPHPxZtuybb3uBC?= =?us-ascii?Q?CoPsl69QqJ8q8hEyVMJSHGxxdPAl1wTFaCu/CPg6NifBOxxMnU6TTx7x4sG3?= =?us-ascii?Q?+iM+8kL8Qr0pu9Uh8Sn3Hvu7JpBf56WlsLNQI3yxR5cGsTWCca00DO7eCso/?= =?us-ascii?Q?saAWq5k/vh5nKaa2mHYsI+P5SvaK/vh/ImlkB5uy3aO57TWfaUKx9lwZ2ixd?= =?us-ascii?Q?q5u3pGW5zkUGAvuOeC0JsZXevCLnQc7fff90Da3YXJvx4xOWLHceWDIUR0dv?= =?us-ascii?Q?snJltpPjPKJB2oC3lo2zWdQ/7kJYvBsSwgfjTLvtYMUze5JzcoVgoCXArucp?= =?us-ascii?Q?yEzdB7EqZiXhYp0d1sE6qvVyRxjFsVjlr9XUPTzFWfd5q9KTlw7uglI8gzSZ?= =?us-ascii?Q?KRw3JZhhtv3I8Ey4bu03O/TaecLQpyJjJYh3X8tRdRkYZ2EKmzTW00BUDgV8?= =?us-ascii?Q?w/hhXib7mBmqisQaj4ypPMLyvDV/igiQCSY/bFlYlGjEU5xe5VoNy5RwGj5G?= =?us-ascii?Q?lT9rYqU37FFmjsB80iBDPXjz68e++eAV8iZEUYv3ih2n9stF539awtV71rjl?= =?us-ascii?Q?Xsbn9uL6Q9KMQRtC/1VCESp32HkC/ftuJc5IDSyHYUN74475Zxrkczh/5Gne?= =?us-ascii?Q?qALdaF3Bc+UGqcnghLZBwEaFeZD2jTxPnB8FeW1mvk89FlLFrlL459s1saF2?= =?us-ascii?Q?z8rqR91g6Lob/5wMEwphaOOsZx6H3DVBNdZU1VTxoEZCdxX0r3ZyF3W2htir?= =?us-ascii?Q?lI7WG8yZtZav6JB4eMr5lGaJEcDwH4gsGrNfMvc7fWLVQ+Am0CbhTeeaE+LQ?= =?us-ascii?Q?Q4N0ZhnJtV+vniPa0NLF4juN8y7Aorhovz+gWl51SC3cfPFPQ32M5Y2Dp8pd?= =?us-ascii?Q?aWq6/GG3EOjXTKsB4SKDtx0TV7R/lhxmNNb0SgM26isXm1HAF1aaGmd9aoTc?= =?us-ascii?Q?e7oe0vDqMriBQk80MZ7YNkTaYGCyJ80HWPq90XHB1opr7WZtD448HGgKNOmo?= =?us-ascii?Q?BTU3cGU09d5ST9TDVknGn7OxnaDMZdxrdIX5I1Ox854GQiTZpUh/DrV1KKJL?= =?us-ascii?Q?7Hl6Onqt3L/eZ70NcrDphK9O3ZJ/9GMeTmvvwMtCgj9exO9CFvfnwBUylthC?= =?us-ascii?Q?OU+Uyc2H+hbfH5WHnY+tB057xv0Fefj/YGpdOpb8UxiwTFvjkOVR+W+j0bfQ?= =?us-ascii?Q?ZExfTP29GkPKE72Su3lqUsFT2J32dQGperQZAZu69VyejgP+QPfTs7uFXW44?= =?us-ascii?Q?ydLOeHCxZaYUo3T2R+sscx7e?= 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: DM6PR11MB4491.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: da757ff8-6000-48ee-3767-08d951b6d13a X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Jul 2021 10:59:48.4677 (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: u5NgOZflPO2xeefRY3k3UEDrsPUzSmjoaNR9ZMfS9LBbNmY50lhJfAXjk9X2uh7K3TG9w4xy8rNxTeQZzqpxuiRvQbncIkld9l7vJy61jCI= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB2921 X-OriginatorOrg: intel.com 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 Akhil, > > > > > 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= use > > bits in > > > > them to indicate various cases. [2] > > > > > > > > > > Both the submitted patches follow approach 1 (following how it's = done > > > > currently), but we can switch to approach 2 if we think there can b= e > > > > > 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#afe16508b77c2a8d= c5caf74a4e9850171 > > > > > [2] https://doc.dpdk.org/api/rte__crypto_8h_source.html > > > > > > > > My vote would probably be for option #2 (use one of the reserved fi= elds > > 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 c= ode > > 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? > > > > 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 w= ith > > 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. >=20 > 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. Not really. warning will be within the same 32/64 bits as status. Compilers these days are smart enough to generate code that would read an check them as one value: https://godbolt.org/z/M3f9891zq > This may not be acceptable. I don't think there would be any performance regression, see above. If you are still nervous about possibility of this extra load, I think we c= an go even one step further and reorder crypto_op fields a bit to have 'status' and 'warning' fields consequent, and put them into one struct to make such optimizations = explicit. I.E: union { uint16_t status_warning; struct {uint8_t status; uint8_t warning;} };=20 =20 > 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 applicatio= n can decide > to ignore or process. All the enum fields > RTE_CRYPTO_OP_STATUS_SUCCESS = Should be > treated as success. > Status is a uint8_t which can hold 255 values, we can start the warning f= rom 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. > > > > So could you tell me which approach did you mean? > > If these 'warnings' are just new error codes and app is required to han= dle > > 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. >=20 > We cannot treat warnings as error codes. These are success cases with som= e > caution to inform user that there may be some issue in coming packets, eg= soft 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 get= s clarified > After this mail. >=20 > > > > If processing them is optional, then I think we better have a new field= for > > 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 */ > > } > > > > In that case existing apps will be continue to work without any modific= ations. > > Yes, they would just ignore these new warnings, but nothing will be bro= ken. > > > The existing apps can still work and but they would treat warnings as err= or for > the PMDs which can return these warnings.=20 > For all other PMDs, it will work as is. > But the application writer knows the features of the PMD which it is usin= g > And hence would need to take care of the warnings eventually. > Eg: it will configure the soft/hard expiry limits while configuring the s= ession. > Hence it will expect the warning to come. So, PMD will generate warnings only when particular offloads will be enable= d, =20 and existing apps wouldn't need to be changed to keep working, right? That's a good thing. Though I still don't like the idea to implicitly re-define op->status behav= iour, depending on some offloads enable/disable.=20 Warning as separate filed looks much more sane and clear to me. > Moreover as I said above also, there will be one extra check for each pac= ket 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= case > there is error or warning and not on the success case. >=20 > > > > 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. > > > > 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. >=20 > We can specify enum names to combine the possible combination of warnings= . > Eg: RTE_CRYPTO_OP_STATUS_WAR_SE_L4_CSUM With just 2 warnings defined it is ok, but imagine in future there would be let say 5 or 6 different warnings, and nearly all combinations will be poss= ible. With enum it would become a real pain.=20