Re: [dpdk-dev] [PATCH] doc: announce ABI change for cryptodev config

[Anoob Joseph <anoobj@marvell.com>]

Hi Fiona,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: 17 January 2019 17:07
> To: Anoob Joseph <anoobj@marvell.com>; Akhil Goyal <akhil.goyal@nxp.com>;
> De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Narayana Prasad Raju
> Athreya <pathreya@marvell.com>; Shally Verma <shallyv@marvell.com>;
> dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [PATCH] doc: announce ABI change for cryptodev config
> 
> Hi Joseph,
> 
> > -----Original Message-----
> > From: Anoob Joseph [mailto:anoobj@marvell.com]
> > Sent: Thursday, January 17, 2019 9:40 AM
> > To: Akhil Goyal <akhil.goyal@nxp.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>
> > Cc: Anoob Joseph <anoobj@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> > <pathreya@marvell.com>; Shally Verma <shallyv@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.

> 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.