[dpdk-dev] [PATCH] cryptodev: mark experimental state

[dpdk-dev] [PATCH] cryptodev: mark experimental state

[Thomas Monjalon]

The crypto API is in an early state.
It requires more discussions and experiments to declare it stable,
as discussed in http://dpdk.org/ml/archives/dev/2015-November/028634.html

A documentation section will be required in the guides.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 MAINTAINERS                            |  2 +-
 config/common_bsdapp                   |  1 +
 config/common_linuxapp                 |  1 +
 doc/guides/contributing/versioning.rst |  1 +
 doc/guides/rel_notes/release_2_2.rst   | 10 ++++++++++
 lib/librte_cryptodev/rte_cryptodev.h   |  3 +++
 6 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7f3d967..460245b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -219,7 +219,7 @@ M: Thomas Monjalon <thomas.monjalon@6wind.com>
 F: lib/librte_ether/
 F: scripts/test-null.sh
 
-Crypto API
+Crypto API - EXPERIMENTAL
 M: Declan Doherty <declan.doherty@intel.com>
 F: lib/librte_cryptodev/
 F: app/test/test_cryptodev*
diff --git a/config/common_bsdapp b/config/common_bsdapp
index 56020b6..3286481 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -310,6 +310,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
 
 #
 # Compile generic crypto device library
+# EXPERIMENTAL: API may change without prior notice
 #
 CONFIG_RTE_LIBRTE_CRYPTODEV=y
 CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
diff --git a/config/common_linuxapp b/config/common_linuxapp
index eaad8d6..2866986 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -319,6 +319,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
 
 #
 # Compile generic crypto device library
+# EXPERIMENTAL: API may change without prior notice
 #
 CONFIG_RTE_LIBRTE_CRYPTODEV=y
 CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
index 8a739dd..653c7d0 100644
--- a/doc/guides/contributing/versioning.rst
+++ b/doc/guides/contributing/versioning.rst
@@ -13,6 +13,7 @@ General Guidelines
 ------------------
 
 #. Whenever possible, ABI should be preserved
+#. The libraries marked in experimental state may change without constraint.
 #. The addition of symbols is generally not problematic
 #. The modification of symbols can generally be managed with versioning
 #. The removal of symbols generally is an ABI break and requires bumping of the
diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
index 8c77768..d0a9955 100644
--- a/doc/guides/rel_notes/release_2_2.rst
+++ b/doc/guides/rel_notes/release_2_2.rst
@@ -17,6 +17,16 @@ New Features
 
 * **Added keepalive support to EAL and example application.**
 
+* **Added experimental cryptodev API**
+
+  The cryptographic processing of packet is provided as a preview
+  with two drivers for:
+
+  * Intel QuickAssist devices
+  * Intel AES-NI multi-buffer library
+
+  Due to its experimental state, the API may change without prior notice.
+
 * **Added ethdev API to support IEEE1588.**
 
   Added functions to read, write and adjust system time in the NIC.
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index 04bade7..aa9f785 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -39,6 +39,9 @@
  *
  * Defines RTE Crypto Device APIs for the provisioning of cipher and
  * authentication operations.
+ *
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
  */
 
 #ifdef __cplusplus
-- 
2.5.2

Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state

[Thomas Monjalon]

2015-11-25 18:38, Thomas Monjalon:
> The crypto API is in an early state.
> It requires more discussions and experiments to declare it stable,
> as discussed in http://dpdk.org/ml/archives/dev/2015-November/028634.html
> 
> A documentation section will be required in the guides.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied

Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state

[Panu Matilainen]

On 11/25/2015 07:38 PM, Thomas Monjalon wrote:
> The crypto API is in an early state.
> It requires more discussions and experiments to declare it stable,
> as discussed in http://dpdk.org/ml/archives/dev/2015-November/028634.html
>
> A documentation section will be required in the guides.
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
[...]
> --- a/config/common_bsdapp
> +++ b/config/common_bsdapp
> @@ -310,6 +310,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
>
>   #
>   # Compile generic crypto device library
> +# EXPERIMENTAL: API may change without prior notice
>   #
>   CONFIG_RTE_LIBRTE_CRYPTODEV=y
>   CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
> diff --git a/config/common_linuxapp b/config/common_linuxapp
> index eaad8d6..2866986 100644
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -319,6 +319,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
>
>   #
>   # Compile generic crypto device library
> +# EXPERIMENTAL: API may change without prior notice
>   #
>   CONFIG_RTE_LIBRTE_CRYPTODEV=y
>   CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
[...]

I think an experimental library which declares itself exempt from the 
ABI policy should not be compiled by default. That way anybody wanting 
to try it out will be forced to notice the experimental status.

More generally / longer term, perhaps there should be a 
CONFIG_RTE_EXPERIMENTAL which wraps all experimental features and 
defaults to off.

	- Panu -

Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state

[Panu Matilainen]

On 11/26/2015 09:39 AM, Panu Matilainen wrote:
> On 11/25/2015 07:38 PM, Thomas Monjalon wrote:
>> The crypto API is in an early state.
>> It requires more discussions and experiments to declare it stable,
>> as discussed in http://dpdk.org/ml/archives/dev/2015-November/028634.html
>>
>> A documentation section will be required in the guides.
>>
>> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
>> ---
> [...]
>> --- a/config/common_bsdapp
>> +++ b/config/common_bsdapp
>> @@ -310,6 +310,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
>>
>>   #
>>   # Compile generic crypto device library
>> +# EXPERIMENTAL: API may change without prior notice
>>   #
>>   CONFIG_RTE_LIBRTE_CRYPTODEV=y
>>   CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
>> diff --git a/config/common_linuxapp b/config/common_linuxapp
>> index eaad8d6..2866986 100644
>> --- a/config/common_linuxapp
>> +++ b/config/common_linuxapp
>> @@ -319,6 +319,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
>>
>>   #
>>   # Compile generic crypto device library
>> +# EXPERIMENTAL: API may change without prior notice
>>   #
>>   CONFIG_RTE_LIBRTE_CRYPTODEV=y
>>   CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
> [...]
>
> I think an experimental library which declares itself exempt from the
> ABI policy should not be compiled by default. That way anybody wanting
> to try it out will be forced to notice the experimental status.
>
> More generally / longer term, perhaps there should be a
> CONFIG_RTE_EXPERIMENTAL which wraps all experimental features and
> defaults to off.

On a related note, librte_mbuf_offload cannot be built if 
CONFIG_RTE_LIBRTE_CRYPTODEV is disabled. Which seems to suggest its (at 
least currently) so tightly couple to cryptodev that perhaps it too 
should be marked experimental and default to off.

	- Panu -

Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state

[Thomas Monjalon]

2015-11-26 10:00, Panu Matilainen:
> On 11/26/2015 09:39 AM, Panu Matilainen wrote:
> > On 11/25/2015 07:38 PM, Thomas Monjalon wrote:
> >> --- a/config/common_linuxapp
> >> +++ b/config/common_linuxapp
> >> @@ -319,6 +319,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
> >>
> >>   #
> >>   # Compile generic crypto device library
> >> +# EXPERIMENTAL: API may change without prior notice
> >>   #
> >>   CONFIG_RTE_LIBRTE_CRYPTODEV=y
> >>   CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
> > [...]
> >
> > I think an experimental library which declares itself exempt from the
> > ABI policy should not be compiled by default. That way anybody wanting
> > to try it out will be forced to notice the experimental status.
> >
> > More generally / longer term, perhaps there should be a
> > CONFIG_RTE_EXPERIMENTAL which wraps all experimental features and
> > defaults to off.
> 
> On a related note, librte_mbuf_offload cannot be built if 
> CONFIG_RTE_LIBRTE_CRYPTODEV is disabled. Which seems to suggest its (at 
> least currently) so tightly couple to cryptodev that perhaps it too 
> should be marked experimental and default to off.

I think you are right.
Declan, what is your opinion?

Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state

[Doherty, Declan]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, November 26, 2015 10:09 AM
> To: Panu Matilainen; Doherty, Declan
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state
> 
> 2015-11-26 10:00, Panu Matilainen:
> > On 11/26/2015 09:39 AM, Panu Matilainen wrote:
> > > On 11/25/2015 07:38 PM, Thomas Monjalon wrote:
> > >> --- a/config/common_linuxapp
> > >> +++ b/config/common_linuxapp
> > >> @@ -319,6 +319,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
> > >>
> > >>   #
> > >>   # Compile generic crypto device library
> > >> +# EXPERIMENTAL: API may change without prior notice
> > >>   #
> > >>   CONFIG_RTE_LIBRTE_CRYPTODEV=y
> > >>   CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
> > > [...]
> > >
> > > I think an experimental library which declares itself exempt from the
> > > ABI policy should not be compiled by default. That way anybody wanting
> > > to try it out will be forced to notice the experimental status.
> > >
> > > More generally / longer term, perhaps there should be a
> > > CONFIG_RTE_EXPERIMENTAL which wraps all experimental features and
> > > defaults to off.
> >
> > On a related note, librte_mbuf_offload cannot be built if
> > CONFIG_RTE_LIBRTE_CRYPTODEV is disabled. Which seems to suggest its (at
> > least currently) so tightly couple to cryptodev that perhaps it too
> > should be marked experimental and default to off.
> 
> I think you are right.
> Declan, what is your opinion?


Hey Thomas, yes librte_mbuf_offload should also be set as experimental, it's 
probably one of the areas which will most likely change in the future.
 
On the issue of turning off experimental libraries in the build by default, my
preference would be not to turn them off unless the library has external
dependencies, otherwise the possibility of patches being submitted which 
could break an experimental library will be much higher. In my opinion the
fewer build configurations developers have to test against the better.

Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state

[Panu Matilainen]

On 11/26/2015 03:51 PM, Doherty, Declan wrote:
>> -----Original Message-----
>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>> Sent: Thursday, November 26, 2015 10:09 AM
>> To: Panu Matilainen; Doherty, Declan
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state
>>
>> 2015-11-26 10:00, Panu Matilainen:
>>> On 11/26/2015 09:39 AM, Panu Matilainen wrote:
>>>> On 11/25/2015 07:38 PM, Thomas Monjalon wrote:
>>>>> --- a/config/common_linuxapp
>>>>> +++ b/config/common_linuxapp
>>>>> @@ -319,6 +319,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
>>>>>
>>>>>    #
>>>>>    # Compile generic crypto device library
>>>>> +# EXPERIMENTAL: API may change without prior notice
>>>>>    #
>>>>>    CONFIG_RTE_LIBRTE_CRYPTODEV=y
>>>>>    CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n
>>>> [...]
>>>>
>>>> I think an experimental library which declares itself exempt from the
>>>> ABI policy should not be compiled by default. That way anybody wanting
>>>> to try it out will be forced to notice the experimental status.
>>>>
>>>> More generally / longer term, perhaps there should be a
>>>> CONFIG_RTE_EXPERIMENTAL which wraps all experimental features and
>>>> defaults to off.
>>>
>>> On a related note, librte_mbuf_offload cannot be built if
>>> CONFIG_RTE_LIBRTE_CRYPTODEV is disabled. Which seems to suggest its (at
>>> least currently) so tightly couple to cryptodev that perhaps it too
>>> should be marked experimental and default to off.
>>
>> I think you are right.
>> Declan, what is your opinion?
>
>
> Hey Thomas, yes librte_mbuf_offload should also be set as experimental, it's
> probably one of the areas which will most likely change in the future.
>
> On the issue of turning off experimental libraries in the build by default, my
> preference would be not to turn them off unless the library has external
> dependencies, otherwise the possibility of patches being submitted which
> could break an experimental library will be much higher. In my opinion the
> fewer build configurations developers have to test against the better.

What I'm more worried about is users and developers starting to rely on 
it while still in experimental state, a single comment in the header is 
really easy to miss.

So I'd like to see *some* mechanism which forces users and developers to 
acknowledge the fact that they're dealing with experimental work. 
Defaulting to off is one possibility, another one would be wrapping 
experimental APIs behind a define which you have to set to be able to 
use the API, eg:

#if defined(I_KNOW_THIS_IS_EXPERIMENTAL_AND_MAY_EAT_BABIES)
[...]
#endif

	- Panu -

Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state

[Thomas Monjalon]

2015-11-27 15:09, Panu Matilainen:
> On 11/26/2015 03:51 PM, Doherty, Declan wrote:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> >> 2015-11-26 10:00, Panu Matilainen:
> >>> On 11/26/2015 09:39 AM, Panu Matilainen wrote:
> >>>> I think an experimental library which declares itself exempt from the
> >>>> ABI policy should not be compiled by default. That way anybody wanting
> >>>> to try it out will be forced to notice the experimental status.
> >>>>
> >>>> More generally / longer term, perhaps there should be a
> >>>> CONFIG_RTE_EXPERIMENTAL which wraps all experimental features and
> >>>> defaults to off.
> >>>
> >>> On a related note, librte_mbuf_offload cannot be built if
> >>> CONFIG_RTE_LIBRTE_CRYPTODEV is disabled. Which seems to suggest its (at
> >>> least currently) so tightly couple to cryptodev that perhaps it too
> >>> should be marked experimental and default to off.
> >>
> >> I think you are right.
> >> Declan, what is your opinion?
> >
> >
> > Hey Thomas, yes librte_mbuf_offload should also be set as experimental, it's
> > probably one of the areas which will most likely change in the future.
> >
> > On the issue of turning off experimental libraries in the build by default, my
> > preference would be not to turn them off unless the library has external
> > dependencies, otherwise the possibility of patches being submitted which
> > could break an experimental library will be much higher. In my opinion the
> > fewer build configurations developers have to test against the better.
> 
> What I'm more worried about is users and developers starting to rely on 
> it while still in experimental state, a single comment in the header is 
> really easy to miss.

There are some comments in the config, the header file, doxygen and the release notes.
When using a feature, you have to read the header or the doc.
So would it be better advertised by adding a comment in the doxygen section of
some of the mandatory functions or structures?

> So I'd like to see *some* mechanism which forces users and developers to 
> acknowledge the fact that they're dealing with experimental work. 
> Defaulting to off is one possibility, another one would be wrapping 
> experimental APIs behind a define which you have to set to be able to 
> use the API, eg:
> 
> #if defined(I_KNOW_THIS_IS_EXPERIMENTAL_AND_MAY_EAT_BABIES)
> [...]
> #endif

Are you sure about the babies? ;)

[dpdk-dev] [PATCH] mbuf_offload: mark experimental state

[Thomas Monjalon]

Cryptodev was marked experimental and mbuf_offload depends on it.
The mbuf_offload library is one of the crypto area which requires
some discussions before having a stable API.

The experimental mark is also added to rte_cryptodev_configure()
to be sure one cannot miss it.

Fixes: 66874e55f550 ("cryptodev: mark experimental state")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 MAINTAINERS                                | 2 +-
 config/common_bsdapp                       | 1 +
 config/common_linuxapp                     | 1 +
 lib/librte_cryptodev/rte_cryptodev.h       | 2 ++
 lib/librte_mbuf_offload/rte_mbuf_offload.h | 5 +++++
 5 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a3ff3f2..3292e84 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -217,7 +217,7 @@ F: lib/librte_mbuf/
 F: doc/guides/prog_guide/mbuf_lib.rst
 F: app/test/test_mbuf.c
 
-Packet buffer offload
+Packet buffer offload - EXPERIMENTAL
 M: Declan Doherty <declan.doherty@intel.com>
 F: lib/librte_mbuf_offload/
 
diff --git a/config/common_bsdapp b/config/common_bsdapp
index 07328d9..8a4d448 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -362,6 +362,7 @@ CONFIG_RTE_PKTMBUF_HEADROOM=128
 
 #
 # Compile librte_mbuf_offload
+# EXPERIMENTAL: API may change without prior notice
 #
 CONFIG_RTE_LIBRTE_MBUF_OFFLOAD=y
 CONFIG_RTE_LIBRTE_MBUF_OFFLOAD_DEBUG=n
diff --git a/config/common_linuxapp b/config/common_linuxapp
index ba9e55d..dda309c 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -379,6 +379,7 @@ CONFIG_RTE_PKTMBUF_HEADROOM=128
 
 #
 # Compile librte_mbuf_offload
+# EXPERIMENTAL: API may change without prior notice
 #
 CONFIG_RTE_LIBRTE_MBUF_OFFLOAD=y
 CONFIG_RTE_LIBRTE_MBUF_OFFLOAD_DEBUG=n
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index aa9f785..892375d 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -243,6 +243,8 @@ struct rte_cryptodev_config {
 /**
  * Configure a device.
  *
+ * EXPERIMENTAL: this API file may change without prior notice
+ *
  * This function must be invoked first before any other function in the
  * API. This function can also be re-invoked when a device is in the
  * stopped state.
diff --git a/lib/librte_mbuf_offload/rte_mbuf_offload.h b/lib/librte_mbuf_offload/rte_mbuf_offload.h
index f52f163..4345f06 100644
--- a/lib/librte_mbuf_offload/rte_mbuf_offload.h
+++ b/lib/librte_mbuf_offload/rte_mbuf_offload.h
@@ -51,6 +51,9 @@
  * operations, with supporting allocate and free functions. It also provides
  * APIs for attaching an offload to a mbuf, as well as an API to retrieve a
  * specified offload type from an mbuf offload chain.
+ *
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
  */
 
 #include <rte_mbuf.h>
@@ -92,6 +95,8 @@ struct rte_pktmbuf_offload_pool_private {
 /**
  * Creates a mempool of rte_mbuf_offload objects
  *
+ * EXPERIMENTAL: this API file may change without prior notice
+ *
  * @param	name		mempool name
  * @param	size		number of objects in mempool
  * @param	cache_size	cache size of objects for each core
-- 
2.5.2

Re: [dpdk-dev] [PATCH] mbuf_offload: mark experimental state

[Thomas Monjalon]

2015-12-12 00:34, Thomas Monjalon:
> Cryptodev was marked experimental and mbuf_offload depends on it.
> The mbuf_offload library is one of the crypto area which requires
> some discussions before having a stable API.
> 
> The experimental mark is also added to rte_cryptodev_configure()
> to be sure one cannot miss it.
> 
> Fixes: 66874e55f550 ("cryptodev: mark experimental state")
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied shortly