Hi Anoob, On 2/1/2019 5:19 PM, Trahe, Fiona wrote: > Hi Thomas, Akhil, Anoob, > >> -----Original Message----- >> From: Thomas Monjalon [mailto:tho...@monjalon.net] >> Sent: Friday, February 1, 2019 11:14 AM >> To: dev@dpdk.org >> Cc: Akhil Goyal <akhil.go...@nxp.com>; Anoob Joseph <ano...@marvell.com>; De >> Lara Guarch, Pablo >> <pablo.de.lara.gua...@intel.com>; Trahe, Fiona <fiona.tr...@intel.com>; >> Jerin Jacob Kollanukkaran >> <jer...@marvell.com>; Narayana Prasad Raju Athreya <pathr...@marvell.com>; >> Shally Verma >> <shal...@marvell.com>; Doherty, Declan <declan.dohe...@intel.com> >> Subject: Re: [dpdk-dev] [PATCH] doc: announce ABI change for cryptodev config >> >> There is only one ack for this change. >> A deprecation requires more agreement (3 valuable acks). >> Other opinions? >> >> >> 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 <ano...@marvell.com> >>>> >>> Acked-by: Akhil Goyal <akhil.go...@nxp.com> > [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_HW_ACCELERATED. > For some, PMDs would need to add performance impacting forks on the data path > to check for disabled features. E.g. if an applic disables the > RTE_CRYPTODEV_FF_CPU_AESNI flag, what does it expect the PMD to do? Not use > the aesni 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 these and reject them? > It seems there are only 3 of the 16 flags that it's desirable to disable, > based 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 > application is allowed to disable - only the above 3. Then PMDs only need to > implement enable/disable functionality for that subset of flags.
could you send a new version of this patch as per the comments from Fiona. Thanks, Akhil
