[v2,3/5] cryptodev: remove crypto list end enumerators

Message ID 20200930173226.770-4-arkadiuszx.kusztal@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series cryptodev: remove list end enumerators |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Arkadiusz Kusztal Sept. 30, 2020, 5:32 p.m. UTC
  This patch removes enumerators RTE_CRYPTO_CIPHER_LIST_END,
RTE_CRYPTO_AUTH_LIST_END, RTE_CRYPTO_AEAD_LIST_END to prevent
some problems that may arise when adding new crypto algorithms.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/librte_cryptodev/rte_crypto_sym.h | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)
  

Comments

Akhil Goyal Oct. 8, 2020, 7:58 p.m. UTC | #1
> diff --git a/lib/librte_cryptodev/rte_crypto_sym.h
> b/lib/librte_cryptodev/rte_crypto_sym.h
> index f29c98051..7a2556a9e 100644
> --- a/lib/librte_cryptodev/rte_crypto_sym.h
> +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> @@ -132,15 +132,12 @@ enum rte_crypto_cipher_algorithm {
>  	 * for m_src and m_dst in the rte_crypto_sym_op must be NULL.
>  	 */
> 
> -	RTE_CRYPTO_CIPHER_DES_DOCSISBPI,
> +	RTE_CRYPTO_CIPHER_DES_DOCSISBPI
>  	/**< DES algorithm using modes required by
>  	 * DOCSIS Baseline Privacy Plus Spec.
>  	 * Chained mbufs are not supported in this mode, i.e. rte_mbuf.next
>  	 * for m_src and m_dst in the rte_crypto_sym_op must be NULL.
>  	 */
> -
> -	RTE_CRYPTO_CIPHER_LIST_END
> -
>  };

Probably we should add a comment for each of the enums that we change,
that the user can define its own LIST_END = last item in the enum +1.
LIST_END is not added to avoid ABI breakage across releases when new algos
are added.

> 
>  /** Cipher algorithm name strings */
> @@ -312,10 +309,8 @@ enum rte_crypto_auth_algorithm {
>  	/**< HMAC using 384 bit SHA3 algorithm. */
>  	RTE_CRYPTO_AUTH_SHA3_512,
>  	/**< 512 bit SHA3 algorithm. */
> -	RTE_CRYPTO_AUTH_SHA3_512_HMAC,
> +	RTE_CRYPTO_AUTH_SHA3_512_HMAC
>  	/**< HMAC using 512 bit SHA3 algorithm. */
> -
> -	RTE_CRYPTO_AUTH_LIST_END
>  };
> 
>  /** Authentication algorithm name strings */
> @@ -412,9 +407,8 @@ enum rte_crypto_aead_algorithm {
>  	/**< AES algorithm in CCM mode. */
>  	RTE_CRYPTO_AEAD_AES_GCM,
>  	/**< AES algorithm in GCM mode. */
> -	RTE_CRYPTO_AEAD_CHACHA20_POLY1305,
> +	RTE_CRYPTO_AEAD_CHACHA20_POLY1305
>  	/**< Chacha20 cipher with poly1305 authenticator */
> -	RTE_CRYPTO_AEAD_LIST_END
>  };
> 
>  /** AEAD algorithm name strings */
> --
> 2.17.1
  
Arkadiusz Kusztal Oct. 12, 2020, 5:15 a.m. UTC | #2
Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: czwartek, 8 października 2020 21:58
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; ruifeng.wang@arm.com;
> michaelsh@marvell.com
> Subject: RE: [PATCH v2 3/5] cryptodev: remove crypto list end enumerators
> 
> > diff --git a/lib/librte_cryptodev/rte_crypto_sym.h
> > b/lib/librte_cryptodev/rte_crypto_sym.h
> > index f29c98051..7a2556a9e 100644
> > --- a/lib/librte_cryptodev/rte_crypto_sym.h
> > +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> > @@ -132,15 +132,12 @@ enum rte_crypto_cipher_algorithm {
> >  	 * for m_src and m_dst in the rte_crypto_sym_op must be NULL.
> >  	 */
> >
> > -	RTE_CRYPTO_CIPHER_DES_DOCSISBPI,
> > +	RTE_CRYPTO_CIPHER_DES_DOCSISBPI
> >  	/**< DES algorithm using modes required by
> >  	 * DOCSIS Baseline Privacy Plus Spec.
> >  	 * Chained mbufs are not supported in this mode, i.e. rte_mbuf.next
> >  	 * for m_src and m_dst in the rte_crypto_sym_op must be NULL.
> >  	 */
> > -
> > -	RTE_CRYPTO_CIPHER_LIST_END
> > -
> >  };
> 
> Probably we should add a comment for each of the enums that we change, that
> the user can define its own LIST_END = last item in the enum +1.
> LIST_END is not added to avoid ABI breakage across releases when new algos
> are added.
[Arek] - I do not now if necessary, should it be some kind of guarantee that order and number of enumerators will not change across the releases?
> 
> >
> >  /** Cipher algorithm name strings */
> > @@ -312,10 +309,8 @@ enum rte_crypto_auth_algorithm {
> >  	/**< HMAC using 384 bit SHA3 algorithm. */
> >  	RTE_CRYPTO_AUTH_SHA3_512,
> >  	/**< 512 bit SHA3 algorithm. */
> > -	RTE_CRYPTO_AUTH_SHA3_512_HMAC,
> > +	RTE_CRYPTO_AUTH_SHA3_512_HMAC
> >  	/**< HMAC using 512 bit SHA3 algorithm. */
> > -
> > -	RTE_CRYPTO_AUTH_LIST_END
> >  };
> >
> >  /** Authentication algorithm name strings */ @@ -412,9 +407,8 @@ enum
> > rte_crypto_aead_algorithm {
> >  	/**< AES algorithm in CCM mode. */
> >  	RTE_CRYPTO_AEAD_AES_GCM,
> >  	/**< AES algorithm in GCM mode. */
> > -	RTE_CRYPTO_AEAD_CHACHA20_POLY1305,
> > +	RTE_CRYPTO_AEAD_CHACHA20_POLY1305
> >  	/**< Chacha20 cipher with poly1305 authenticator */
> > -	RTE_CRYPTO_AEAD_LIST_END
> >  };
> >
> >  /** AEAD algorithm name strings */
> > --
> > 2.17.1
  
Akhil Goyal Oct. 12, 2020, 11:46 a.m. UTC | #3
Hi Arek,
> Hi Akhil,
> 
> > -----Original Message-----
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> > Sent: czwartek, 8 października 2020 21:58
> > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > Cc: Trahe, Fiona <fiona.trahe@intel.com>; ruifeng.wang@arm.com;
> > michaelsh@marvell.com
> > Subject: RE: [PATCH v2 3/5] cryptodev: remove crypto list end enumerators
> >
> > > diff --git a/lib/librte_cryptodev/rte_crypto_sym.h
> > > b/lib/librte_cryptodev/rte_crypto_sym.h
> > > index f29c98051..7a2556a9e 100644
> > > --- a/lib/librte_cryptodev/rte_crypto_sym.h
> > > +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> > > @@ -132,15 +132,12 @@ enum rte_crypto_cipher_algorithm {
> > >  	 * for m_src and m_dst in the rte_crypto_sym_op must be NULL.
> > >  	 */
> > >
> > > -	RTE_CRYPTO_CIPHER_DES_DOCSISBPI,
> > > +	RTE_CRYPTO_CIPHER_DES_DOCSISBPI
> > >  	/**< DES algorithm using modes required by
> > >  	 * DOCSIS Baseline Privacy Plus Spec.
> > >  	 * Chained mbufs are not supported in this mode, i.e. rte_mbuf.next
> > >  	 * for m_src and m_dst in the rte_crypto_sym_op must be NULL.
> > >  	 */
> > > -
> > > -	RTE_CRYPTO_CIPHER_LIST_END
> > > -
> > >  };
> >
> > Probably we should add a comment for each of the enums that we change,
> that
> > the user can define its own LIST_END = last item in the enum +1.
> > LIST_END is not added to avoid ABI breakage across releases when new algos
> > are added.
> [Arek] - I do not now if necessary, should it be some kind of guarantee that
> order and number of enumerators will not change across the releases?

Yes we should make sure in the future that the order of enums are not changed
In future. This can be mentioned in the comments. We will be adding the new algos
In the end only. Please add a comment.
  

Patch

diff --git a/lib/librte_cryptodev/rte_crypto_sym.h b/lib/librte_cryptodev/rte_crypto_sym.h
index f29c98051..7a2556a9e 100644
--- a/lib/librte_cryptodev/rte_crypto_sym.h
+++ b/lib/librte_cryptodev/rte_crypto_sym.h
@@ -132,15 +132,12 @@  enum rte_crypto_cipher_algorithm {
 	 * for m_src and m_dst in the rte_crypto_sym_op must be NULL.
 	 */
 
-	RTE_CRYPTO_CIPHER_DES_DOCSISBPI,
+	RTE_CRYPTO_CIPHER_DES_DOCSISBPI
 	/**< DES algorithm using modes required by
 	 * DOCSIS Baseline Privacy Plus Spec.
 	 * Chained mbufs are not supported in this mode, i.e. rte_mbuf.next
 	 * for m_src and m_dst in the rte_crypto_sym_op must be NULL.
 	 */
-
-	RTE_CRYPTO_CIPHER_LIST_END
-
 };
 
 /** Cipher algorithm name strings */
@@ -312,10 +309,8 @@  enum rte_crypto_auth_algorithm {
 	/**< HMAC using 384 bit SHA3 algorithm. */
 	RTE_CRYPTO_AUTH_SHA3_512,
 	/**< 512 bit SHA3 algorithm. */
-	RTE_CRYPTO_AUTH_SHA3_512_HMAC,
+	RTE_CRYPTO_AUTH_SHA3_512_HMAC
 	/**< HMAC using 512 bit SHA3 algorithm. */
-
-	RTE_CRYPTO_AUTH_LIST_END
 };
 
 /** Authentication algorithm name strings */
@@ -412,9 +407,8 @@  enum rte_crypto_aead_algorithm {
 	/**< AES algorithm in CCM mode. */
 	RTE_CRYPTO_AEAD_AES_GCM,
 	/**< AES algorithm in GCM mode. */
-	RTE_CRYPTO_AEAD_CHACHA20_POLY1305,
+	RTE_CRYPTO_AEAD_CHACHA20_POLY1305
 	/**< Chacha20 cipher with poly1305 authenticator */
-	RTE_CRYPTO_AEAD_LIST_END
 };
 
 /** AEAD algorithm name strings */