HI Fiona, Anoob >-----Original Message----- >From: Anoob Joseph >Sent: 17 January 2019 19:17 >To: Trahe, Fiona <fiona.tr...@intel.com>; Akhil Goyal <akhil.go...@nxp.com>; >De Lara Guarch, Pablo ><pablo.de.lara.gua...@intel.com> >Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Narayana Prasad Raju >Athreya <pathr...@marvell.com>; Shally Verma ><shal...@marvell.com>; dev@dpdk.org >Subject: RE: [PATCH] doc: announce ABI change for cryptodev config > >Hi Fiona, > >Please see inline. > >Thanks, >Anoob > >> -----Original Message----- >> From: Trahe, Fiona <fiona.tr...@intel.com> >> Sent: 17 January 2019 17:07 >> To: Anoob Joseph <ano...@marvell.com>; Akhil Goyal <akhil.go...@nxp.com>; >> De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com> >> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Narayana Prasad Raju >> Athreya <pathr...@marvell.com>; Shally Verma <shal...@marvell.com>; >> dev@dpdk.org; Trahe, Fiona <fiona.tr...@intel.com> >> Subject: RE: [PATCH] doc: announce ABI change for cryptodev config >> >> Hi Joseph, >> >> > -----Original Message----- >> > From: Anoob Joseph [mailto:ano...@marvell.com] >> > Sent: Thursday, January 17, 2019 9:40 AM >> > To: Akhil Goyal <akhil.go...@nxp.com>; De Lara Guarch, Pablo >> > <pablo.de.lara.gua...@intel.com>; Trahe, Fiona <fiona.tr...@intel.com> >> > Cc: Anoob Joseph <ano...@marvell.com>; Jerin Jacob Kollanukkaran >> > <jer...@marvell.com>; Narayana Prasad Raju Athreya >> > <pathr...@marvell.com>; Shally Verma <shal...@marvell.com>; >> > dev@dpdk.org >> > Subject: [PATCH] doc: announce ABI change for cryptodev config >> > >> > 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. >> [Fiona] I'm unfamiliar with eth config so can you explain a bit more how this >> works? > >[Anoob] For eth devices, application would first do rte_eth_dev_info_get() and >gets the capabilities. The device would expose it's >capabilities using dev_info.rx_offload_capa & dev_info.tx_offload_capa. The >application can enable/disable these features while >configuring the eth ports. > >From ipsec-secgw: >https://git.dpdk.org/dpdk/tree/examples/ipsec-secgw/ipsec-secgw.c#n1866 > >if (frame_size) { > local_port_conf.rxmode.max_rx_pkt_len = frame_size; > local_port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME; > } > ><snip> > >ret = rte_eth_dev_configure(portid, nb_rx_queue, nb_tx_queue, > &local_port_conf); > ><snip> > >This way application can choose to disable unwanted offloads. > >Port init in ipsec-secgw: >https://git.dpdk.org/dpdk/tree/examples/ipsec-secgw/ipsec-secgw.c#n1826 > >> e.g. if a crypto device's info says it supports >> RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO >> RTE_CRYPTODEV_FF_SYM_OPERATION_CHAINING >> RTE_CRYPTODEV_FF_CPU_AESNI >> RTE_CRYPTODEV_FF_SECURITY >> RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT >> these things are all already enabled. >> Is the param a way to disable some of them? > >[Anoob] Yes. There are few other flags in addition to the above. Like >RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO. > >> If so, it would be better to call it ff_disable. > >[Anoob] Calling it ff_enable is to make it similar to how it's done with eth >devices. Either way should be fine. [Shally] keeping it as "ff_enable" will require application to set these flags to configure PMD. If we define it other way around, then it would be mean to mask out unwanted features which can be quite many. Though purpose can be achieved both ways but keeping it as "enable" looks more easy to use, readable and inline with how ethdev handle multi feature support. So I would prefer to keep it as "ff_enable"
Thanks Shally > >> And to limit it to the subset of features that it makes sense to disable. >> i.e. why would an application disable chaining or any of the SGL flags? It >> would >> just add extra cycles in the PMD to error check on these cases, instead the >> appl >> can just not send such commands. >> And it doesn't make sense to disable AESNI or >> RTE_CRYPTODEV_FF_HW_ACCELERATED. >> Actually I can't see what an application would want to achieve by disabling >> any >> flag? > >[Anoob] Features like ASYMMETRIC or SECURITY is not required for every >application. SECURITY is added for eth devices also. But >since the feature can be disabled while configuring the port, applications are >given a choice to disable it. That way apps like l2fwd >doesn't enable SECURITY. With crypto this option is not available. > >If the unused offloads can be communicated to the PMD before initialization, >the PMD will be allowed to optimize hardware usage. >Otherwise, supporting more features would affect performance, even if >application is not making use of those features. > >Ex: test-crypto-perf doesn't use ASYM/SECURITY. Now adding these features >would affect the performance of this app.
