All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] doc: announce ABI change for cryptodev and ethdev
@ 2017-08-03 15:32 Akhil Goyal
  2017-08-04  5:26 ` Hemant Agrawal
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Akhil Goyal @ 2017-08-03 15:32 UTC (permalink / raw)
  To: dev, declan.doherty, thomas, radu.nicolau, aviadye, borisp,
	hemant.agrawal, pablo.de.lara.guarch
  Cc: Akhil Goyal

Support for security operations is planned to be added
in ethdev and cryptodev for the 17.11 release.

For this following changes are required.
- rte_cryptodev and rte_eth_dev structures need to be added
new parameter rte_security_ops which extend support for
security ops to the corresponding driver.
- rte_cryptodev_info and rte_ethd_dev_info need to be added
with rte_security_capabilities to identify the capabilities of
the corresponding driver.

Signed-off-by: Akhil Goyal <akhil.goyal@nxp.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 f6bd910..2393b4c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -69,3 +69,13 @@ Deprecation Notices
   be removed in 17.11:
 
   - ``rte_cryptodev_create_vdev``
+
+* cryptodev: new parameters - ``rte_security_capabilities`` and
+  ``rte_security_ops`` will be added to ``rte_cryptodev_info`` and
+  ``rte_cryptodev`` respectively to support security protocol offloaded
+  operations.
+
+* ethdev: new parameters - ``rte_security_capabilities`` and
+  ``rte_security_ops`` will be added to ``rte_eth_dev_info`` and
+  ``rte_eth_dev`` respectively  to support security operations like
+  ipsec inline.
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc: announce ABI change for cryptodev and ethdev
  2017-08-03 15:32 [PATCH] doc: announce ABI change for cryptodev and ethdev Akhil Goyal
@ 2017-08-04  5:26 ` Hemant Agrawal
  2017-08-07 17:41   ` Thomas Monjalon
  2017-08-04  9:28 ` De Lara Guarch, Pablo
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Hemant Agrawal @ 2017-08-04  5:26 UTC (permalink / raw)
  To: Akhil Goyal, dev, declan.doherty, thomas, radu.nicolau, aviadye,
	borisp, pablo.de.lara.guarch

On 8/3/2017 9:02 PM, Akhil Goyal wrote:
> Support for security operations is planned to be added
> in ethdev and cryptodev for the 17.11 release.
>
> For this following changes are required.
> - rte_cryptodev and rte_eth_dev structures need to be added
> new parameter rte_security_ops which extend support for
> security ops to the corresponding driver.
> - rte_cryptodev_info and rte_ethd_dev_info need to be added
> with rte_security_capabilities to identify the capabilities of
> the corresponding driver.
>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.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 f6bd910..2393b4c 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -69,3 +69,13 @@ Deprecation Notices
>    be removed in 17.11:
>
>    - ``rte_cryptodev_create_vdev``
> +
> +* cryptodev: new parameters - ``rte_security_capabilities`` and
> +  ``rte_security_ops`` will be added to ``rte_cryptodev_info`` and
> +  ``rte_cryptodev`` respectively to support security protocol offloaded
> +  operations.
> +
> +* ethdev: new parameters - ``rte_security_capabilities`` and
> +  ``rte_security_ops`` will be added to ``rte_eth_dev_info`` and
> +  ``rte_eth_dev`` respectively  to support security operations like
> +  ipsec inline.
>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc: announce ABI change for cryptodev and ethdev
  2017-08-03 15:32 [PATCH] doc: announce ABI change for cryptodev and ethdev Akhil Goyal
  2017-08-04  5:26 ` Hemant Agrawal
@ 2017-08-04  9:28 ` De Lara Guarch, Pablo
  2017-08-04 15:25 ` De Lara Guarch, Pablo
  2017-08-08  7:09 ` [PATCH v2] " Akhil Goyal
  3 siblings, 0 replies; 12+ messages in thread
From: De Lara Guarch, Pablo @ 2017-08-04  9:28 UTC (permalink / raw)
  To: Akhil Goyal, dev, Doherty, Declan, thomas, Nicolau, Radu,
	aviadye, borisp, hemant.agrawal



> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Thursday, August 3, 2017 4:32 PM
> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
> thomas@monjalon.net; Nicolau, Radu <radu.nicolau@intel.com>;
> aviadye@mellanox.com; borisp@mellanox.com;
> hemant.agrawal@nxp.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Cc: Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [PATCH] doc: announce ABI change for cryptodev and ethdev
> 
> Support for security operations is planned to be added in ethdev and
> cryptodev for the 17.11 release.
> 
> For this following changes are required.
> - rte_cryptodev and rte_eth_dev structures need to be added new
> parameter rte_security_ops which extend support for security ops to the
> corresponding driver.
> - rte_cryptodev_info and rte_ethd_dev_info need to be added with
> rte_security_capabilities to identify the capabilities of the corresponding
> driver.
> 
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>

Boris, Aviad, could you review this patch?

Thanks,
Pablo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc: announce ABI change for cryptodev and ethdev
  2017-08-03 15:32 [PATCH] doc: announce ABI change for cryptodev and ethdev Akhil Goyal
  2017-08-04  5:26 ` Hemant Agrawal
  2017-08-04  9:28 ` De Lara Guarch, Pablo
@ 2017-08-04 15:25 ` De Lara Guarch, Pablo
  2017-08-08  6:54   ` Akhil Goyal
  2017-08-08  7:09 ` [PATCH v2] " Akhil Goyal
  3 siblings, 1 reply; 12+ messages in thread
From: De Lara Guarch, Pablo @ 2017-08-04 15:25 UTC (permalink / raw)
  To: Akhil Goyal, dev, Doherty, Declan, thomas, Nicolau, Radu,
	aviadye, borisp, hemant.agrawal



> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Thursday, August 3, 2017 4:32 PM
> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
> thomas@monjalon.net; Nicolau, Radu <radu.nicolau@intel.com>;
> aviadye@mellanox.com; borisp@mellanox.com;
> hemant.agrawal@nxp.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Cc: Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [PATCH] doc: announce ABI change for cryptodev and ethdev
> 
> Support for security operations is planned to be added in ethdev and
> cryptodev for the 17.11 release.
> 
> For this following changes are required.
> - rte_cryptodev and rte_eth_dev structures need to be added new
> parameter rte_security_ops which extend support for security ops to the
> corresponding driver.
> - rte_cryptodev_info and rte_ethd_dev_info need to be added with
> rte_security_capabilities to identify the capabilities of the corresponding
> driver.
> 
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>

Not sure if this needed to be split into two patches, as this affects two libraries.
At least, from cryptodev side:

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc: announce ABI change for cryptodev and ethdev
  2017-08-04  5:26 ` Hemant Agrawal
@ 2017-08-07 17:41   ` Thomas Monjalon
  2017-08-07 18:07     ` Boris Pismenny
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Monjalon @ 2017-08-07 17:41 UTC (permalink / raw)
  To: dev
  Cc: Hemant Agrawal, Akhil Goyal, declan.doherty, radu.nicolau,
	aviadye, borisp, pablo.de.lara.guarch

04/08/2017 07:26, Hemant Agrawal:
> On 8/3/2017 9:02 PM, Akhil Goyal wrote:
> > Support for security operations is planned to be added
> > in ethdev and cryptodev for the 17.11 release.
> >
> > For this following changes are required.
> > - rte_cryptodev and rte_eth_dev structures need to be added
> > new parameter rte_security_ops which extend support for
> > security ops to the corresponding driver.
> > - rte_cryptodev_info and rte_ethd_dev_info need to be added
> > with rte_security_capabilities to identify the capabilities of
> > the corresponding driver.

It is not explained what is the fundamental difference between
rte_security and rte_crypto?
It looks to be just a technical workaround.

Why the ABI would be changed by rte_security additions?

> > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> >
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

No more opinions outside of NXP?
It seems there is not yet a consensus on how to manage IPsec offloading.
I heard there were some phone calls about these stuff but nothing clear
appears publicly on the mailing list.
Looks to be a community failure.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc: announce ABI change for cryptodev and ethdev
  2017-08-07 17:41   ` Thomas Monjalon
@ 2017-08-07 18:07     ` Boris Pismenny
  2017-08-08  5:03       ` Shahaf Shuler
  0 siblings, 1 reply; 12+ messages in thread
From: Boris Pismenny @ 2017-08-07 18:07 UTC (permalink / raw)
  To: Thomas Monjalon, dev
  Cc: Hemant Agrawal, Akhil Goyal, declan.doherty, radu.nicolau,
	Aviad Yehezkel, pablo.de.lara.guarch, Boris Pismenny

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> 04/08/2017 07:26, Hemant Agrawal:
> > On 8/3/2017 9:02 PM, Akhil Goyal wrote:
> > > Support for security operations is planned to be added in ethdev and
> > > cryptodev for the 17.11 release.
> > >
> > > For this following changes are required.
> > > - rte_cryptodev and rte_eth_dev structures need to be added new
> > > parameter rte_security_ops which extend support for security ops to
> > > the corresponding driver.
> > > - rte_cryptodev_info and rte_ethd_dev_info need to be added with
> > > rte_security_capabilities to identify the capabilities of the
> > > corresponding driver.
> 
> It is not explained what is the fundamental difference between rte_security
> and rte_crypto?
> It looks to be just a technical workaround.

rte_security is a layer between crypto and NIC.

Today crypto sessions are created exclusively against crypto devices, but they don't use network related fields, while the network namespace doesn't use crypto related fields. We expect this API to represent crypto sessions that combine network fields and allow to add/delete them for all devices.

For NICs we will use rte_flow with rte_security for inline/full crypto protocol offload such as ESP.

> 
> Why the ABI would be changed by rte_security additions?
> 
> > > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> > >
> > Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> 
> No more opinions outside of NXP?
> It seems there is not yet a consensus on how to manage IPsec offloading.
> I heard there were some phone calls about these stuff but nothing clear
> appears publicly on the mailing list.
> Looks to be a community failure.

We agreed to go ahead with this approach on one such phone call. I hope we could use the dpdk github for development. 

Acked-by: Boris Pismenny <borisp@mellanox.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc: announce ABI change for cryptodev and ethdev
  2017-08-07 18:07     ` Boris Pismenny
@ 2017-08-08  5:03       ` Shahaf Shuler
  2017-08-08 10:00         ` Thomas Monjalon
  0 siblings, 1 reply; 12+ messages in thread
From: Shahaf Shuler @ 2017-08-08  5:03 UTC (permalink / raw)
  To: Boris Pismenny, Thomas Monjalon, dev
  Cc: Hemant Agrawal, Akhil Goyal, declan.doherty, radu.nicolau,
	Aviad Yehezkel, pablo.de.lara.guarch, Boris Pismenny

Monday, August 7, 2017 9:07 PM, Boris Pismenny:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 04/08/2017 07:26, Hemant Agrawal:
> > > On 8/3/2017 9:02 PM, Akhil Goyal wrote:
> > > > Support for security operations is planned to be added in ethdev
> > > > and cryptodev for the 17.11 release.
> > > >
> > > > For this following changes are required.
> > > > - rte_cryptodev and rte_eth_dev structures need to be added new
> > > > parameter rte_security_ops which extend support for security ops
> > > > to the corresponding driver.
> > > > - rte_cryptodev_info and rte_ethd_dev_info need to be added with
> > > > rte_security_capabilities to identify the capabilities of the
> > > > corresponding driver.
> >
> > It is not explained what is the fundamental difference between
> > rte_security and rte_crypto?
> > It looks to be just a technical workaround.
> 
> rte_security is a layer between crypto and NIC.
> 
> Today crypto sessions are created exclusively against crypto devices, but
> they don't use network related fields, while the network namespace doesn't
> use crypto related fields. We expect this API to represent crypto sessions
> that combine network fields and allow to add/delete them for all devices.
> 
> For NICs we will use rte_flow with rte_security for inline/full crypto protocol
> offload such as ESP.
> 
> >
> > Why the ABI would be changed by rte_security additions?
> >
> > > > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> > > >
> > > Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >
> > No more opinions outside of NXP?
> > It seems there is not yet a consensus on how to manage IPsec offloading.
> > I heard there were some phone calls about these stuff but nothing
> > clear appears publicly on the mailing list.
> > Looks to be a community failure.
> 
> We agreed to go ahead with this approach on one such phone call. I hope we
> could use the dpdk github for development.
> 
> Acked-by: Boris Pismenny <borisp@mellanox.com>

Acked-by: Shahaf Shuler <shahafs@mellanox.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc: announce ABI change for cryptodev and ethdev
  2017-08-04 15:25 ` De Lara Guarch, Pablo
@ 2017-08-08  6:54   ` Akhil Goyal
  0 siblings, 0 replies; 12+ messages in thread
From: Akhil Goyal @ 2017-08-08  6:54 UTC (permalink / raw)
  To: De Lara Guarch, Pablo, Declan Doherty
  Cc: Nicolau, Radu, aviadye, borisp, hemant.agrawal, Thomas Monjalon, dev

Hi Pablo/Declan,

On 8/4/2017 8:55 PM, De Lara Guarch, Pablo wrote:
> 
> 
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Thursday, August 3, 2017 4:32 PM
>> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
>> thomas@monjalon.net; Nicolau, Radu <radu.nicolau@intel.com>;
>> aviadye@mellanox.com; borisp@mellanox.com;
>> hemant.agrawal@nxp.com; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>
>> Cc: Akhil Goyal <akhil.goyal@nxp.com>
>> Subject: [PATCH] doc: announce ABI change for cryptodev and ethdev
>>
>> Support for security operations is planned to be added in ethdev and
>> cryptodev for the 17.11 release.
>>
>> For this following changes are required.
>> - rte_cryptodev and rte_eth_dev structures need to be added new
>> parameter rte_security_ops which extend support for security ops to the
>> corresponding driver.
>> - rte_cryptodev_info and rte_ethd_dev_info need to be added with
>> rte_security_capabilities to identify the capabilities of the corresponding
>> driver.
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> 
> Not sure if this needed to be split into two patches, as this affects two libraries.
> At least, from cryptodev side:
> 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 

We would be needing one more ABI change, Can I send it now. I discovered 
it after I sent this patch.

In the struct rte_crypto_sym_op, we would need to add a pointer to a 
security session in the union of session and xform.

Also, Do I need to split this patch into two for crypto and eth?

Regards,
Akhil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] doc: announce ABI change for cryptodev and ethdev
  2017-08-03 15:32 [PATCH] doc: announce ABI change for cryptodev and ethdev Akhil Goyal
                   ` (2 preceding siblings ...)
  2017-08-04 15:25 ` De Lara Guarch, Pablo
@ 2017-08-08  7:09 ` Akhil Goyal
  2017-08-08  8:08   ` De Lara Guarch, Pablo
  3 siblings, 1 reply; 12+ messages in thread
From: Akhil Goyal @ 2017-08-08  7:09 UTC (permalink / raw)
  To: dev, declan.doherty, thomas, radu.nicolau, aviadye, borisp,
	hemant.agrawal, pablo.de.lara.guarch
  Cc: shahafs, Akhil Goyal

Support for security operations is planned to be added
in ethdev and cryptodev for the 17.11 release.

For this following changes are required.
- rte_cryptodev and rte_eth_dev structures need to be added
new parameter rte_security_ops which extend support for
security ops to the corresponding driver.
- rte_cryptodev_info and rte_eth_dev_info need to be added
with rte_security_capabilities to identify the capabilities of
the corresponding driver.
- rte_security_session needs to be added in the union inside
structure rte_crypto_sym_op to decide between various types of
sessions

Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Acked-by: Boris Pismenny <borisp@mellanox.com>
Acked-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
changes in v2:
Added one more ABI change wrt to security session,
This patch is not split into 3 patches as these all will be implemented
simultaneously with rte_security support.

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

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 72d1f35..e34a809 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -78,3 +78,18 @@ Deprecation Notices
   be removed in 17.11:
 
   - ``rte_cryptodev_create_vdev``
+
+* cryptodev: new parameters - ``rte_security_capabilities`` and
+  ``rte_security_ops`` will be added to ``rte_cryptodev_info`` and
+  ``rte_cryptodev`` respectively to support security protocol offloaded
+  operations.
+
+* cryptodev: new parameter ``rte_security_session`` will be added in the union
+  of the structure ``rte_crypto_sym_op``, so that the user can choose either to
+  use ``rte_cryptodev_sym_session`` or ``rte_crypto_sym_xform`` or
+  ``rte_security_session``.
+
+* ethdev: new parameters - ``rte_security_capabilities`` and
+  ``rte_security_ops`` will be added to ``rte_eth_dev_info`` and
+  ``rte_eth_dev`` respectively  to support security operations like
+  ipsec inline.
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: announce ABI change for cryptodev and ethdev
  2017-08-08  7:09 ` [PATCH v2] " Akhil Goyal
@ 2017-08-08  8:08   ` De Lara Guarch, Pablo
  2017-08-08  8:42     ` Akhil Goyal
  0 siblings, 1 reply; 12+ messages in thread
From: De Lara Guarch, Pablo @ 2017-08-08  8:08 UTC (permalink / raw)
  To: Akhil Goyal, dev, Doherty, Declan, thomas, Nicolau, Radu,
	aviadye, borisp, hemant.agrawal
  Cc: shahafs

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Tuesday, August 8, 2017 8:10 AM
> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
> thomas@monjalon.net; Nicolau, Radu <radu.nicolau@intel.com>;
> aviadye@mellanox.com; borisp@mellanox.com;
> hemant.agrawal@nxp.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Cc: shahafs@mellanox.com; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [PATCH v2] doc: announce ABI change for cryptodev and ethdev
> 
> Support for security operations is planned to be added in ethdev and
> cryptodev for the 17.11 release.
> 
> For this following changes are required.
> - rte_cryptodev and rte_eth_dev structures need to be added new
> parameter rte_security_ops which extend support for security ops to the
> corresponding driver.
> - rte_cryptodev_info and rte_eth_dev_info need to be added with
> rte_security_capabilities to identify the capabilities of the corresponding
> driver.
> - rte_security_session needs to be added in the union inside structure
> rte_crypto_sym_op to decide between various types of sessions
> 
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Acked-by: Boris Pismenny <borisp@mellanox.com>
> Acked-by: Shahaf Shuler <shahafs@mellanox.com>
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
> changes in v2:
> Added one more ABI change wrt to security session, This patch is not split
> into 3 patches as these all will be implemented simultaneously with
> rte_security support.
> 
>  doc/guides/rel_notes/deprecation.rst | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 72d1f35..e34a809 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,18 @@ Deprecation Notices
>    be removed in 17.11:
> 
>    - ``rte_cryptodev_create_vdev``
> +
> +* cryptodev: new parameters - ``rte_security_capabilities`` and
> +  ``rte_security_ops`` will be added to ``rte_cryptodev_info`` and
> +  ``rte_cryptodev`` respectively to support security protocol offloaded
> +  operations.
> +
> +* cryptodev: new parameter ``rte_security_session`` will be added in
> +the union
> +  of the structure ``rte_crypto_sym_op``, so that the user can choose
> +either to
> +  use ``rte_cryptodev_sym_session`` or ``rte_crypto_sym_xform`` or
> +  ``rte_security_session``.

I don't think it is required to have a deprecation notice for this, as it is adding
another element in a union, and it is not changing its size.
However, this would require an addition in rte_crypto_op_sess_type, which,
as long as it is added at the end, should not require a deprecation notice.

About splitting this into two patches, I would do it, as I believe deprecation notices
should target a specific library.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: announce ABI change for cryptodev and ethdev
  2017-08-08  8:08   ` De Lara Guarch, Pablo
@ 2017-08-08  8:42     ` Akhil Goyal
  0 siblings, 0 replies; 12+ messages in thread
From: Akhil Goyal @ 2017-08-08  8:42 UTC (permalink / raw)
  To: De Lara Guarch, Pablo, dev, Doherty, Declan, thomas, Nicolau,
	Radu, aviadye, borisp, hemant.agrawal
  Cc: shahafs

Hi Pablo,

On 8/8/2017 1:38 PM, De Lara Guarch, Pablo wrote:
> Hi Akhil,
> 
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Tuesday, August 8, 2017 8:10 AM
>> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
>> thomas@monjalon.net; Nicolau, Radu <radu.nicolau@intel.com>;
>> aviadye@mellanox.com; borisp@mellanox.com;
>> hemant.agrawal@nxp.com; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>
>> Cc: shahafs@mellanox.com; Akhil Goyal <akhil.goyal@nxp.com>
>> Subject: [PATCH v2] doc: announce ABI change for cryptodev and ethdev
>>
>> Support for security operations is planned to be added in ethdev and
>> cryptodev for the 17.11 release.
>>
>> For this following changes are required.
>> - rte_cryptodev and rte_eth_dev structures need to be added new
>> parameter rte_security_ops which extend support for security ops to the
>> corresponding driver.
>> - rte_cryptodev_info and rte_eth_dev_info need to be added with
>> rte_security_capabilities to identify the capabilities of the corresponding
>> driver.
>> - rte_security_session needs to be added in the union inside structure
>> rte_crypto_sym_op to decide between various types of sessions
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> Acked-by: Boris Pismenny <borisp@mellanox.com>
>> Acked-by: Shahaf Shuler <shahafs@mellanox.com>
>> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>> ---
>> changes in v2:
>> Added one more ABI change wrt to security session, This patch is not split
>> into 3 patches as these all will be implemented simultaneously with
>> rte_security support.
>>
>>   doc/guides/rel_notes/deprecation.rst | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst
>> b/doc/guides/rel_notes/deprecation.rst
>> index 72d1f35..e34a809 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -78,3 +78,18 @@ Deprecation Notices
>>     be removed in 17.11:
>>
>>     - ``rte_cryptodev_create_vdev``
>> +
>> +* cryptodev: new parameters - ``rte_security_capabilities`` and
>> +  ``rte_security_ops`` will be added to ``rte_cryptodev_info`` and
>> +  ``rte_cryptodev`` respectively to support security protocol offloaded
>> +  operations.
>> +
>> +* cryptodev: new parameter ``rte_security_session`` will be added in
>> +the union
>> +  of the structure ``rte_crypto_sym_op``, so that the user can choose
>> +either to
>> +  use ``rte_cryptodev_sym_session`` or ``rte_crypto_sym_xform`` or
>> +  ``rte_security_session``.
> 
> I don't think it is required to have a deprecation notice for this, as it is adding
> another element in a union, and it is not changing its size.
> However, this would require an addition in rte_crypto_op_sess_type, which,
> as long as it is added at the end, should not require a deprecation notice.
> 
> About splitting this into two patches, I would do it, as I believe deprecation notices
> should target a specific library.
> 
> 
> 

Thanks for the comments. I have marked the v2 as not applicable and 
marked v1 as in review in the patchwork.

-Akhil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc: announce ABI change for cryptodev and ethdev
  2017-08-08  5:03       ` Shahaf Shuler
@ 2017-08-08 10:00         ` Thomas Monjalon
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2017-08-08 10:00 UTC (permalink / raw)
  To: Akhil Goyal
  Cc: dev, Shahaf Shuler, Boris Pismenny, Hemant Agrawal,
	declan.doherty, radu.nicolau, Aviad Yehezkel,
	pablo.de.lara.guarch, techboard

08/08/2017 07:03, Shahaf Shuler:
> Monday, August 7, 2017 9:07 PM, Boris Pismenny:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 04/08/2017 07:26, Hemant Agrawal:
> > > > On 8/3/2017 9:02 PM, Akhil Goyal wrote:
> > > > > Support for security operations is planned to be added in ethdev
> > > > > and cryptodev for the 17.11 release.
> > > > >
> > > > > For this following changes are required.
> > > > > - rte_cryptodev and rte_eth_dev structures need to be added new
> > > > > parameter rte_security_ops which extend support for security ops
> > > > > to the corresponding driver.
> > > > > - rte_cryptodev_info and rte_ethd_dev_info need to be added with
> > > > > rte_security_capabilities to identify the capabilities of the
> > > > > corresponding driver.
> > >
> > > It is not explained what is the fundamental difference between
> > > rte_security and rte_crypto?
> > > It looks to be just a technical workaround.
> > 
> > rte_security is a layer between crypto and NIC.
> > 
> > Today crypto sessions are created exclusively against crypto devices, but
> > they don't use network related fields, while the network namespace doesn't
> > use crypto related fields. We expect this API to represent crypto sessions
> > that combine network fields and allow to add/delete them for all devices.
> > 
> > For NICs we will use rte_flow with rte_security for inline/full crypto protocol
> > offload such as ESP.
> > 
> > >
> > > Why the ABI would be changed by rte_security additions?
> > >
> > > > > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> > > > >
> > > > Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > >
> > > No more opinions outside of NXP?
> > > It seems there is not yet a consensus on how to manage IPsec offloading.
> > > I heard there were some phone calls about these stuff but nothing
> > > clear appears publicly on the mailing list.
> > > Looks to be a community failure.
> > 
> > We agreed to go ahead with this approach on one such phone call. I hope we
> > could use the dpdk github for development.
> > 
> > Acked-by: Boris Pismenny <borisp@mellanox.com>
> 
> Acked-by: Shahaf Shuler <shahafs@mellanox.com>

Applied
It means you have a chance to do this change in 17.11.
It does not mean you can be sure that the patches will be accepted.

This is introducing a new complexity.
It must be discussed with the technical board before approving
the final design in 17.11.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-08-08 10:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-03 15:32 [PATCH] doc: announce ABI change for cryptodev and ethdev Akhil Goyal
2017-08-04  5:26 ` Hemant Agrawal
2017-08-07 17:41   ` Thomas Monjalon
2017-08-07 18:07     ` Boris Pismenny
2017-08-08  5:03       ` Shahaf Shuler
2017-08-08 10:00         ` Thomas Monjalon
2017-08-04  9:28 ` De Lara Guarch, Pablo
2017-08-04 15:25 ` De Lara Guarch, Pablo
2017-08-08  6:54   ` Akhil Goyal
2017-08-08  7:09 ` [PATCH v2] " Akhil Goyal
2017-08-08  8:08   ` De Lara Guarch, Pablo
2017-08-08  8:42     ` Akhil Goyal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.