[dpdk-dev] [PATCH] cryptodev: clarify wireless inputs in digest-encrypted cases

[dpdk-dev] [PATCH] cryptodev: clarify wireless inputs in digest-encrypted cases

[Fiona Trahe]

Clarify constraints on fields specified in bits for wireless
algorithms in digest-encrypted case.

Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
 lib/librte_cryptodev/rte_crypto_sym.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/lib/librte_cryptodev/rte_crypto_sym.h b/lib/librte_cryptodev/rte_crypto_sym.h
index bc8da24..0204d37 100644
--- a/lib/librte_cryptodev/rte_crypto_sym.h
+++ b/lib/librte_cryptodev/rte_crypto_sym.h
@@ -703,6 +703,13 @@ struct rte_crypto_sym_op {
 					 * auth.data.length and is typically
 					 * equal to auth.data.offset +
 					 * auth.data.length + digest_length.
+					 * - for wireless algorithms, i.e.
+					 * SNOW 3G, KASUMI and ZUC, as the
+					 * cipher.data.length,
+					 * cipher.data.offset,
+					 * auth.data.length and
+					 * auth.data.offset are in bits, they
+					 * must be 8-bit multiples.
 					 *
 					 * Note, that for security reasons, it
 					 * is PMDs' responsibility to not
-- 
1.7.0.7

Re: [dpdk-dev] [PATCH] cryptodev: clarify wireless inputs in digest-encrypted cases

[Akhil Goyal]

Hi Fiona,

> 
> Clarify constraints on fields specified in bits for wireless
> algorithms in digest-encrypted case.
> 
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto_sym.h |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto_sym.h
> b/lib/librte_cryptodev/rte_crypto_sym.h
> index bc8da24..0204d37 100644
> --- a/lib/librte_cryptodev/rte_crypto_sym.h
> +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> @@ -703,6 +703,13 @@ struct rte_crypto_sym_op {
>  					 * auth.data.length and is typically
>  					 * equal to auth.data.offset +
>  					 * auth.data.length + digest_length.
> +					 * - for wireless algorithms, i.e.
> +					 * SNOW 3G, KASUMI and ZUC, as the
> +					 * cipher.data.length,
> +					 * cipher.data.offset,
> +					 * auth.data.length and
> +					 * auth.data.offset are in bits, they
> +					 * must be 8-bit multiples.

What is the meaning of 'they' here? 
cipher.data.length, cipher.data.offset, auth.data.length and auth.data.offset are in bits
and may or may not be multiple of 8bits. This is documented in their comments.


>  					 *
>  					 * Note, that for security reasons, it
>  					 * is PMDs' responsibility to not
> --
> 1.7.0.7

Re: [dpdk-dev] [PATCH] cryptodev: clarify wireless inputs in digest-encrypted cases

[Trahe, Fiona]

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Tuesday, October 22, 2019 12:49 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org
> Cc: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> Subject: RE: [PATCH] cryptodev: clarify wireless inputs in digest-encrypted cases
> 
> Hi Fiona,
> 
> >
> > Clarify constraints on fields specified in bits for wireless
> > algorithms in digest-encrypted case.
> >
> > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > ---
> >  lib/librte_cryptodev/rte_crypto_sym.h |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/lib/librte_cryptodev/rte_crypto_sym.h
> > b/lib/librte_cryptodev/rte_crypto_sym.h
> > index bc8da24..0204d37 100644
> > --- a/lib/librte_cryptodev/rte_crypto_sym.h
> > +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> > @@ -703,6 +703,13 @@ struct rte_crypto_sym_op {
> >  					 * auth.data.length and is typically
> >  					 * equal to auth.data.offset +
> >  					 * auth.data.length + digest_length.
> > +					 * - for wireless algorithms, i.e.
> > +					 * SNOW 3G, KASUMI and ZUC, as the
> > +					 * cipher.data.length,
> > +					 * cipher.data.offset,
> > +					 * auth.data.length and
> > +					 * auth.data.offset are in bits, they
> > +					 * must be 8-bit multiples.
> 
> What is the meaning of 'they' here?
> cipher.data.length, cipher.data.offset, auth.data.length and auth.data.offset are in bits
> and may or may not be multiple of 8bits. This is documented in their comments.
> 
[Fiona] This added note is an extra bullet under the explanation for Digest-encrypted cases.
In these cases only those fields listed MUST be 8-bit multiples.
Can you suggest a change that would make that clearer?

 
> 
> >  					 *
> >  					 * Note, that for security reasons, it
> >  					 * is PMDs' responsibility to not
> > --
> > 1.7.0.7

Re: [dpdk-dev] [PATCH] cryptodev: clarify wireless inputs in digest-encrypted cases

[Akhil Goyal]

> 
> Hi Akhil,
> 
> >
> > Hi Fiona,
> >
> > >
> > > Clarify constraints on fields specified in bits for wireless
> > > algorithms in digest-encrypted case.
> > >
> > > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > > ---
> > >  lib/librte_cryptodev/rte_crypto_sym.h |    7 +++++++
> > >  1 files changed, 7 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/lib/librte_cryptodev/rte_crypto_sym.h
> > > b/lib/librte_cryptodev/rte_crypto_sym.h
> > > index bc8da24..0204d37 100644
> > > --- a/lib/librte_cryptodev/rte_crypto_sym.h
> > > +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> > > @@ -703,6 +703,13 @@ struct rte_crypto_sym_op {
> > >  					 * auth.data.length and is typically
> > >  					 * equal to auth.data.offset +
> > >  					 * auth.data.length + digest_length.
> > > +					 * - for wireless algorithms, i.e.
> > > +					 * SNOW 3G, KASUMI and ZUC, as the
> > > +					 * cipher.data.length,
> > > +					 * cipher.data.offset,
> > > +					 * auth.data.length and
> > > +					 * auth.data.offset are in bits, they
> > > +					 * must be 8-bit multiples.
> >
> > What is the meaning of 'they' here?
> > cipher.data.length, cipher.data.offset, auth.data.length and auth.data.offset
> are in bits
> > and may or may not be multiple of 8bits. This is documented in their comments.
> >
> [Fiona] This added note is an extra bullet under the explanation for Digest-
> encrypted cases.
> In these cases only those fields listed MUST be 8-bit multiples.
> Can you suggest a change that would make that clearer?

@@ -590,7 +590,9 @@ struct rte_crypto_sym_op {
                                          * For SNOW 3G @ RTE_CRYPTO_CIPHER_SNOW3G_UEA2,
                                          * KASUMI @ RTE_CRYPTO_CIPHER_KASUMI_F8
                                          * and ZUC @ RTE_CRYPTO_CIPHER_ZUC_EEA3,
-                                         * this field should be in bits.
+                                         * this field should be in bits and must be
+                                         * 8-bit multiples in cases of encrypted
+                                         * digest.
                                          */

Same change may be done in other 3 fields.

> 
> 
> >
> > >  					 *
> > >  					 * Note, that for security reasons, it
> > >  					 * is PMDs' responsibility to not
> > > --
> > > 1.7.0.7

Re: [dpdk-dev] [PATCH] cryptodev: clarify wireless inputs in digest-encrypted cases

[Trahe, Fiona]

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Tuesday, October 22, 2019 1:08 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org
> Cc: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> Subject: RE: [PATCH] cryptodev: clarify wireless inputs in digest-encrypted cases
> 
> 
> 

> @@ -590,7 +590,9 @@ struct rte_crypto_sym_op {
>                                           * For SNOW 3G @ RTE_CRYPTO_CIPHER_SNOW3G_UEA2,
>                                           * KASUMI @ RTE_CRYPTO_CIPHER_KASUMI_F8
>                                           * and ZUC @ RTE_CRYPTO_CIPHER_ZUC_EEA3,
> -                                         * this field should be in bits.
> +                                         * this field should be in bits and must be
> +                                         * 8-bit multiples in cases of encrypted
> +                                         * digest.
>                                           */
> 
> Same change may be done in other 3 fields.
Ok, thanks, I'll send v2

[dpdk-dev] [PATCH v2] cryptodev: clarify wireless inputs in digest-encrypted cases

[Fiona Trahe]

Clarify constraints on fields specified in bits for wireless
algorithms in digest-encrypted case.

v2 changes:
 - added note on each of the affected parameter fields

Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
 lib/librte_cryptodev/rte_crypto_sym.h |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/lib/librte_cryptodev/rte_crypto_sym.h b/lib/librte_cryptodev/rte_crypto_sym.h
index bc8da24..ffa038d 100644
--- a/lib/librte_cryptodev/rte_crypto_sym.h
+++ b/lib/librte_cryptodev/rte_crypto_sym.h
@@ -590,7 +590,9 @@ struct rte_crypto_sym_op {
 					  * For SNOW 3G @ RTE_CRYPTO_CIPHER_SNOW3G_UEA2,
 					  * KASUMI @ RTE_CRYPTO_CIPHER_KASUMI_F8
 					  * and ZUC @ RTE_CRYPTO_CIPHER_ZUC_EEA3,
-					  * this field should be in bits.
+					  * this field should be in bits. For
+					  * digest-encrypted cases this must be
+					  * an 8-bit multiple.
 					  */
 					uint32_t length;
 					 /**< The message length, in bytes, of the
@@ -604,7 +606,9 @@ struct rte_crypto_sym_op {
 					  * For SNOW 3G @ RTE_CRYPTO_AUTH_SNOW3G_UEA2,
 					  * KASUMI @ RTE_CRYPTO_CIPHER_KASUMI_F8
 					  * and ZUC @ RTE_CRYPTO_CIPHER_ZUC_EEA3,
-					  * this field should be in bits.
+					  * this field should be in bits. For
+					  * digest-encrypted cases this must be
+					  * an 8-bit multiple.
 					  */
 				} data; /**< Data offsets and length for ciphering */
 			} cipher;
@@ -620,7 +624,9 @@ struct rte_crypto_sym_op {
 					  * For SNOW 3G @ RTE_CRYPTO_AUTH_SNOW3G_UIA2,
 					  * KASUMI @ RTE_CRYPTO_AUTH_KASUMI_F9
 					  * and ZUC @ RTE_CRYPTO_AUTH_ZUC_EIA3,
-					  * this field should be in bits.
+					  * this field should be in bits. For
+					  * digest-encrypted cases this must be
+					  * an 8-bit multiple.
 					  *
 					  * @note
 					  * For KASUMI @ RTE_CRYPTO_AUTH_KASUMI_F9,
@@ -635,7 +641,9 @@ struct rte_crypto_sym_op {
 					  * For SNOW 3G @ RTE_CRYPTO_AUTH_SNOW3G_UIA2,
 					  * KASUMI @ RTE_CRYPTO_AUTH_KASUMI_F9
 					  * and ZUC @ RTE_CRYPTO_AUTH_ZUC_EIA3,
-					  * this field should be in bits.
+					  * this field should be in bits. For
+					  * digest-encrypted cases this must be
+					  * an 8-bit multiple.
 					  *
 					  * @note
 					  * For KASUMI @ RTE_CRYPTO_AUTH_KASUMI_F9,
@@ -703,6 +711,13 @@ struct rte_crypto_sym_op {
 					 * auth.data.length and is typically
 					 * equal to auth.data.offset +
 					 * auth.data.length + digest_length.
+					 * - for wireless algorithms, i.e.
+					 * SNOW 3G, KASUMI and ZUC, as the
+					 * cipher.data.length,
+					 * cipher.data.offset,
+					 * auth.data.length and
+					 * auth.data.offset are in bits, they
+					 * must be 8-bit multiples.
 					 *
 					 * Note, that for security reasons, it
 					 * is PMDs' responsibility to not
-- 
1.7.0.7

Re: [dpdk-dev] [PATCH v2] cryptodev: clarify wireless inputs in digest-encrypted cases

[Akhil Goyal]

> 
> Clarify constraints on fields specified in bits for wireless
> algorithms in digest-encrypted case.
> 
> v2 changes:
>  - added note on each of the affected parameter fields
> 
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto_sym.h |   23 +++++++++++++++++++----
>  1 files changed, 19 insertions(+), 4 deletions(-)
> 
//snip//

> @@ -703,6 +711,13 @@ struct rte_crypto_sym_op {
>  					 * auth.data.length and is typically
>  					 * equal to auth.data.offset +
>  					 * auth.data.length + digest_length.
> +					 * - for wireless algorithms, i.e.
> +					 * SNOW 3G, KASUMI and ZUC, as the
> +					 * cipher.data.length,
> +					 * cipher.data.offset,
> +					 * auth.data.length and
> +					 * auth.data.offset are in bits, they
> +					 * must be 8-bit multiples.

Why do you need this change? This is redundant information.
>  					 *
>  					 * Note, that for security reasons, it
>  					 * is PMDs' responsibility to not
> --
> 1.7.0.7

Re: [dpdk-dev] [PATCH v2] cryptodev: clarify wireless inputs in digest-encrypted cases

[Trahe, Fiona]

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Tuesday, October 22, 2019 2:24 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org
> Cc: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> Subject: RE: [PATCH v2] cryptodev: clarify wireless inputs in digest-encrypted cases
> 
> 
> 
> >
> > Clarify constraints on fields specified in bits for wireless
> > algorithms in digest-encrypted case.
> >
> > v2 changes:
> >  - added note on each of the affected parameter fields
> >
> > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > ---
> >  lib/librte_cryptodev/rte_crypto_sym.h |   23 +++++++++++++++++++----
> >  1 files changed, 19 insertions(+), 4 deletions(-)
> >
> //snip//
> 
> > @@ -703,6 +711,13 @@ struct rte_crypto_sym_op {
> >  					 * auth.data.length and is typically
> >  					 * equal to auth.data.offset +
> >  					 * auth.data.length + digest_length.
> > +					 * - for wireless algorithms, i.e.
> > +					 * SNOW 3G, KASUMI and ZUC, as the
> > +					 * cipher.data.length,
> > +					 * cipher.data.offset,
> > +					 * auth.data.length and
> > +					 * auth.data.offset are in bits, they
> > +					 * must be 8-bit multiples.
> 
> Why do you need this change? This is redundant information.
[Fiona] I think it's useful that it's under the digest-encrypted case notes.
So ok as is and is good to have info in both places people may look.
However if we want to avoid duplication, then I'd prefer to keep here and add a note
under the specific params to direct to check here. Like:
                                                                          * and ZUC @ RTE_CRYPTO_CIPHER_ZUC_EEA3,
-					  * this field should be in bits.
+					  * this field should be in bits. See also
+					  * notes about digest-encrypted case.
 					  */

Re: [dpdk-dev] [PATCH v2] cryptodev: clarify wireless inputs in digest-encrypted cases

[Akhil Goyal]

> >
> >
> > >
> > > Clarify constraints on fields specified in bits for wireless
> > > algorithms in digest-encrypted case.
> > >
> > > v2 changes:
> > >  - added note on each of the affected parameter fields
> > >
> > > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > > ---
> > >  lib/librte_cryptodev/rte_crypto_sym.h |   23 +++++++++++++++++++----
> > >  1 files changed, 19 insertions(+), 4 deletions(-)
> > >
> > //snip//
> >
> > > @@ -703,6 +711,13 @@ struct rte_crypto_sym_op {
> > >  					 * auth.data.length and is typically
> > >  					 * equal to auth.data.offset +
> > >  					 * auth.data.length + digest_length.
> > > +					 * - for wireless algorithms, i.e.
> > > +					 * SNOW 3G, KASUMI and ZUC, as the
> > > +					 * cipher.data.length,
> > > +					 * cipher.data.offset,
> > > +					 * auth.data.length and
> > > +					 * auth.data.offset are in bits, they
> > > +					 * must be 8-bit multiples.
> >
> > Why do you need this change? This is redundant information.
> [Fiona] I think it's useful that it's under the digest-encrypted case notes.
> So ok as is and is good to have info in both places people may look.
> However if we want to avoid duplication, then I'd prefer to keep here and add a
> note
> under the specific params to direct to check here. Like:
>                                                                           * and ZUC @
> RTE_CRYPTO_CIPHER_ZUC_EEA3,
> -					  * this field should be in bits.
> +					  * this field should be in bits. See also
> +					  * notes about digest-encrypted case.
>  					  */


OK
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Applied to dpdk-next-crypto

Thanks.