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

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

[Anoob Joseph]

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 <anoobj@marvell.com>
---
 doc/guides/rel_notes/deprecation.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 5f03443..e289c2d 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -66,3 +66,13 @@ Deprecation Notices
 
 * crypto/aesni_mb: the minimum supported intel-ipsec-mb library version will be
   changed from 0.49.0 to 0.52.0.
+
+* cryptodev: New member in ``rte_cryptodev_config`` to allow applications to
+  specify the features to be enabled on the crypto device. For eth devices,
+  applications can use ``rte_eth_conf.rxmode.offloads`` and
+  ``rte_eth_conf.txmode.offloads`` to control the offloads enabled. Adding
+  a similar field to facilitate efficient usage of HW/SW offloads.
+
+  - Member ``uint64_t ff_enable`` in ``rte_cryptodev_config``
+
+  The field would be added in v19.05.
-- 
2.7.4

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

[Trahe, Fiona]

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?
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?
If so, it would be better to call it ff_disable.
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?

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

[Anoob Joseph]

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.

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

[Shally Verma]

HI Fiona, Anoob

>-----Original Message-----
>From: Anoob Joseph
>Sent: 17 January 2019 19:17
>To: Trahe, Fiona <fiona.trahe@intel.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
>Subject: RE: [PATCH] doc: announce ABI change for cryptodev config
>
>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.
[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.

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

[Anoob Joseph]

Hi Fiona,

Any more comments on this?

@ Akhil, Pablo

Can you review this change and share your thoughts?

Thanks,
Anoob

> -----Original Message-----
> From: Shally Verma
> Sent: 18 January 2019 12:29
> To: Anoob Joseph <anoobj@marvell.com>; Trahe, Fiona
> <fiona.trahe@intel.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>; dev@dpdk.org
> Subject: RE: [PATCH] doc: announce ABI change for cryptodev config
> 
> HI Fiona, Anoob
> 
> >-----Original Message-----
> >From: Anoob Joseph
> >Sent: 17 January 2019 19:17
> >To: Trahe, Fiona <fiona.trahe@intel.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
> >Subject: RE: [PATCH] doc: announce ABI change for cryptodev config
> >
> >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.
> [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.

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

[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 <anoobj@marvell.com>
>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

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

[Thomas Monjalon]

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 <anoobj@marvell.com>
> >
> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

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

[Trahe, Fiona]

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 <akhil.goyal@nxp.com>; Anoob Joseph <anoobj@marvell.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Narayana Prasad Raju Athreya <pathreya@marvell.com>; Shally Verma
> <shallyv@marvell.com>; Doherty, Declan <declan.doherty@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 <anoobj@marvell.com>
> > >
> > Acked-by: Akhil Goyal <akhil.goyal@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.

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

[Akhil Goyal]

Hi Anoob,

On 2/1/2019 5:19 PM, Trahe, Fiona wrote:
> 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 <akhil.goyal@nxp.com>; Anoob Joseph <anoobj@marvell.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>; Jerin Jacob Kollanukkaran
>> <jerinj@marvell.com>; Narayana Prasad Raju Athreya <pathreya@marvell.com>; Shally Verma
>> <shallyv@marvell.com>; Doherty, Declan <declan.doherty@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 <anoobj@marvell.com>
>>>>
>>> Acked-by: Akhil Goyal <akhil.goyal@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

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

[Anoob Joseph]

Hi Akhil,

I'll send a v2 incorporating Fiona's comments. 

Thanks,
Anoob

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Thursday, February 28, 2019 3:32 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; dev@dpdk.org
> Cc: Anoob Joseph <anoobj@marvell.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> <pathreya@marvell.com>; Shally Verma <shallyv@marvell.com>; Doherty,
> Declan <declan.doherty@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] doc: announce ABI change for cryptodev
> config
> 
> Hi Anoob,
> 
> On 2/1/2019 5:19 PM, Trahe, Fiona wrote:
> > 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 <akhil.goyal@nxp.com>; Anoob Joseph
> >> <anoobj@marvell.com>; De Lara Guarch, Pablo
> >> <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> >> <fiona.trahe@intel.com>; Jerin Jacob Kollanukkaran
> >> <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> >> <pathreya@marvell.com>; Shally Verma <shallyv@marvell.com>;
> Doherty,
> >> Declan <declan.doherty@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 <anoobj@marvell.com>
> >>>>
> >>> Acked-by: Akhil Goyal <akhil.goyal@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

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

[Anoob Joseph]

Add new field ff_disable 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_disable;
+   /**< Feature flags to be disabled. Only the following features are
+    * allowed to be disabled,
+    *  - RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO
+    *  - RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO
+    *  - RTE_CRYTPODEV_FF_SECURITY
+    */
};

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 <anoobj@marvell.com>
---
v2:
* Renamed 'ff_enable' to 'ff_disable'

 doc/guides/rel_notes/deprecation.rst | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1b4fcb7..d0a60f9 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -75,3 +75,17 @@ Deprecation Notices
 
 * crypto/aesni_mb: the minimum supported intel-ipsec-mb library version will be
   changed from 0.49.0 to 0.52.0.
+
+* cryptodev: New member in ``rte_cryptodev_config`` to allow applications to
+  disable features supported by the crypto device. Only the following features
+  would be allowed to be disabled this way,
+
+  - ``RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO``
+  - ``RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO``
+  - ``RTE_CRYPTODEV_FF_SECURITY``
+
+  Disabling unused features would facilitate efficient usage of HW/SW offload.
+
+  - Member ``uint64_t ff_disable`` in ``rte_cryptodev_config``
+
+  The field would be added in v19.08.
-- 
2.7.4

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

[Trahe, Fiona]

> -----Original Message-----
> From: Anoob Joseph [mailto:anoobj@marvell.com]
> Sent: Thursday, March 7, 2019 10:40 AM
> To: Akhil Goyal <akhil.goyal@nxp.com>; Trahe, Fiona <fiona.trahe@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@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>; Suheil Chandran
> <schandran@marvell.com>; dev@dpdk.org
> Subject: [PATCH v2] doc: announce ABI change for cryptodev config
> 
> Add new field ff_disable 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_disable;
> +   /**< Feature flags to be disabled. Only the following features are
> +    * allowed to be disabled,
> +    *  - RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO
> +    *  - RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO
> +    *  - RTE_CRYTPODEV_FF_SECURITY
> +    */
> };
> 
> 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 <anoobj@marvell.com>
Acked-by: Fiona Trahe <fiona.trahe@intel.com>

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

[Akhil Goyal]

On 3/7/2019 8:55 PM, Trahe, Fiona wrote:
>
>> -----Original Message-----
>> From: Anoob Joseph [mailto:anoobj@marvell.com]
>> Sent: Thursday, March 7, 2019 10:40 AM
>> To: Akhil Goyal <akhil.goyal@nxp.com>; Trahe, Fiona <fiona.trahe@intel.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@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>; Suheil Chandran
>> <schandran@marvell.com>; dev@dpdk.org
>> Subject: [PATCH v2] doc: announce ABI change for cryptodev config
>>
>> Add new field ff_disable 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_disable;
>> +   /**< Feature flags to be disabled. Only the following features are
>> +    * allowed to be disabled,
>> +    *  - RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO
>> +    *  - RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO
>> +    *  - RTE_CRYTPODEV_FF_SECURITY
>> +    */
>> };
>>
>> 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 <anoobj@marvell.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>
>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Applied to dpdk-next-crypto

Thanks.

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

[Akhil Goyal]

On 3/7/2019 8:55 PM, Trahe, Fiona wrote:
>
>> -----Original Message-----
>> From: Anoob Joseph [mailto:anoobj@marvell.com]
>> Sent: Thursday, March 7, 2019 10:40 AM
>> To: Akhil Goyal <akhil.goyal@nxp.com>; Trahe, Fiona <fiona.trahe@intel.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@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>; Suheil Chandran
>> <schandran@marvell.com>; dev@dpdk.org
>> Subject: [PATCH v2] doc: announce ABI change for cryptodev config
>>
>> Add new field ff_disable 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_disable;
>> +   /**< Feature flags to be disabled. Only the following features are
>> +    * allowed to be disabled,
>> +    *  - RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO
>> +    *  - RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO
>> +    *  - RTE_CRYTPODEV_FF_SECURITY
>> +    */
>> };
>>
>> 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 <anoobj@marvell.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>
>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Applied to dpdk-next-crypto

Thanks.