From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 0A0081B4B3 for ; Fri, 1 Feb 2019 12:49:40 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Feb 2019 03:49:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,548,1539673200"; d="scan'208";a="296466840" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by orsmga005.jf.intel.com with ESMTP; 01 Feb 2019 03:49:36 -0800 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.213]) by IRSMSX153.ger.corp.intel.com ([169.254.9.115]) with mapi id 14.03.0415.000; Fri, 1 Feb 2019 11:49:35 +0000 From: "Trahe, Fiona" To: Thomas Monjalon , "dev@dpdk.org" CC: Akhil Goyal , Anoob Joseph , "De Lara Guarch, Pablo" , "Jerin Jacob Kollanukkaran" , Narayana Prasad Raju Athreya , Shally Verma , "Doherty, Declan" , "Trahe, Fiona" Thread-Topic: [dpdk-dev] [PATCH] doc: announce ABI change for cryptodev config Thread-Index: AQHUrkiNRSYeo6tWnUGaZWm74s8S2KXJOPMAgAGo/ICAAAHZwA== Date: Fri, 1 Feb 2019 11:49:34 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B435896D7CF9@IRSMSX101.ger.corp.intel.com> References: <1547717928-21203-1-git-send-email-anoobj@marvell.com> <3553801.FxY0opz6cb@xps> In-Reply-To: <3553801.FxY0opz6cb@xps> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjQ1OGRlNDUtNjhiYS00MWM0LWIxNjctY2M2YzU2ZDVjZjM2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYmpQSldwVm5udUNsYk9cL204UjR4d1BXajQ1UUNCT2ZMcFwvSEdsWVNtOGVNRjBnMFVcL2wwK2M5a3RzVitVdXRDayJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] doc: announce ABI change for cryptodev config 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: , X-List-Received-Date: Fri, 01 Feb 2019 11:49:41 -0000 Hi Thomas, Akhil, Anoob, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Friday, February 1, 2019 11:14 AM > To: dev@dpdk.org > Cc: Akhil Goyal ; Anoob Joseph ;= De Lara Guarch, Pablo > ; Trahe, Fiona ; J= erin Jacob Kollanukkaran > ; Narayana Prasad Raju Athreya = ; Shally Verma > ; Doherty, Declan > Subject: Re: [dpdk-dev] [PATCH] doc: announce ABI change for cryptodev co= nfig >=20 > There is only one ack for this change. > A deprecation requires more agreement (3 valuable acks). > Other opinions? >=20 >=20 > 31/01/2019 10:53, Akhil Goyal: > > On 1/17/2019 3:09 PM, Anoob Joseph wrote: > > > Add new field ff_enable in rte_cryptodev_config. This enables > > > applications to control the features enabled on the crypto device. > > > > > > Proposed new layout: > > > > > > /** Crypto device configuration structure */ > > > struct rte_cryptodev_config { > > > int socket_id; /**< Socket to allocate resources on *= / > > > uint16_t nb_queue_pairs; > > > /**< Number of queue pairs to configure on device */ > > > + uint64_t ff_enable; > > > + /**< Feature flags to be enabled on the device. Only the features= set > > > + * on rte_cryptodev_info.feature_flags are allowed to be set. > > > + */ > > > }; > > > > > > For eth devices, rte_eth_conf.rx_mode.offloads and > > > rte_eth_conf.tx_mode.offloads fields are used by applications to > > > control the offloads enabled on the eth device. This proposal adds a > > > similar ability for the crypto device. > > > > > > Signed-off-by: Anoob Joseph > > > > > Acked-by: Akhil Goyal [Fiona] Keeping consistent with ethdev is a lower priority that adding a param that works well for crypto. As proposed this ff_enable is problematic= for crypto as it makes no sense to enable/disable most of the flags. For some there's no sensible action a PMD can take, e.g. RTE_CRYPTODEV_FF_H= W_ACCELERATED. For some, PMDs would need to add performance impacting forks on the data pa= th to check for disabled features. E.g. if an applic disables the RTE_CRYPT= ODEV_FF_CPU_AESNI flag, what does it expect the PMD to do? Not use the aesn= i capability of the CPU? Fork and do a less performant implementation? Or if this flag is set, RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT or RTE_CRYPTODEV_= FF_SYM_OPERATION_CHAINING, does the PMD need to check for operations like t= hese and reject them? It seems there are only 3 of the 16 flags that it's desirable to disable, b= ased on the earlier thread RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO RTE_CRYPTODEV_FF_SECURITY So I propose this param should be called ff_disable. It should documented - and maybe provide a mask for - the flags the applica= tion is allowed to disable - only the above 3. Then PMDs only need to imple= ment enable/disable functionality for that subset of flags.