* [PATCH] cryptodev: add missing algorithm strings
@ 2022-09-15 8:26 Volodymyr Fialko
2022-09-22 16:01 ` Akhil Goyal
2022-11-02 10:50 ` Kevin Traynor
0 siblings, 2 replies; 6+ messages in thread
From: Volodymyr Fialko @ 2022-09-15 8:26 UTC (permalink / raw)
To: dev, Akhil Goyal, Fan Zhang, Ravi Kumar; +Cc: jerinj, anoobj, Volodymyr Fialko
SHA3 family algorithms were missing in the array of algorithm strings.
Fixes: 1df800f89518 ("crypto/ccp: support SHA3 family")
Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
---
lib/cryptodev/rte_cryptodev.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index 42f3221052..35661f5347 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -130,6 +130,15 @@ rte_crypto_auth_algorithm_strings[] = {
[RTE_CRYPTO_AUTH_SHA512] = "sha2-512",
[RTE_CRYPTO_AUTH_SHA512_HMAC] = "sha2-512-hmac",
+ [RTE_CRYPTO_AUTH_SHA3_224] = "sha3-224",
+ [RTE_CRYPTO_AUTH_SHA3_224_HMAC] = "sha3-224-hmac",
+ [RTE_CRYPTO_AUTH_SHA3_256] = "sha3-256",
+ [RTE_CRYPTO_AUTH_SHA3_256_HMAC] = "sha3-256-hmac",
+ [RTE_CRYPTO_AUTH_SHA3_384] = "sha3-384",
+ [RTE_CRYPTO_AUTH_SHA3_384_HMAC] = "sha3-384-hmac",
+ [RTE_CRYPTO_AUTH_SHA3_512] = "sha3-512",
+ [RTE_CRYPTO_AUTH_SHA3_512_HMAC] = "sha3-512-hmac",
+
[RTE_CRYPTO_AUTH_KASUMI_F9] = "kasumi-f9",
[RTE_CRYPTO_AUTH_SNOW3G_UIA2] = "snow3g-uia2",
[RTE_CRYPTO_AUTH_ZUC_EIA3] = "zuc-eia3"
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] cryptodev: add missing algorithm strings
2022-09-15 8:26 [PATCH] cryptodev: add missing algorithm strings Volodymyr Fialko
@ 2022-09-22 16:01 ` Akhil Goyal
2022-11-02 10:50 ` Kevin Traynor
1 sibling, 0 replies; 6+ messages in thread
From: Akhil Goyal @ 2022-09-22 16:01 UTC (permalink / raw)
To: Volodymyr Fialko, dev, Fan Zhang, Ravi Kumar
Cc: Jerin Jacob Kollanukkaran, Anoob Joseph, Volodymyr Fialko
> Subject: [PATCH] cryptodev: add missing algorithm strings
>
> SHA3 family algorithms were missing in the array of algorithm strings.
>
> Fixes: 1df800f89518 ("crypto/ccp: support SHA3 family")
>
> Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> ---
Acked-by: Akhil Goyal <gakhil@marvell.com>
Applied to dpdk-next-crypto
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] cryptodev: add missing algorithm strings
2022-09-15 8:26 [PATCH] cryptodev: add missing algorithm strings Volodymyr Fialko
2022-09-22 16:01 ` Akhil Goyal
@ 2022-11-02 10:50 ` Kevin Traynor
2022-11-02 10:58 ` [EXT] " Akhil Goyal
1 sibling, 1 reply; 6+ messages in thread
From: Kevin Traynor @ 2022-11-02 10:50 UTC (permalink / raw)
To: Volodymyr Fialko, dev, Akhil Goyal, Fan Zhang, Ravi Kumar, Ali Alnubani
Cc: jerinj, anoobj
On 15/09/2022 09:26, Volodymyr Fialko wrote:
> SHA3 family algorithms were missing in the array of algorithm strings.
>
> Fixes: 1df800f89518 ("crypto/ccp: support SHA3 family")
>
> Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> ---
> lib/cryptodev/rte_cryptodev.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index 42f3221052..35661f5347 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -130,6 +130,15 @@ rte_crypto_auth_algorithm_strings[] = {
> [RTE_CRYPTO_AUTH_SHA512] = "sha2-512",
> [RTE_CRYPTO_AUTH_SHA512_HMAC] = "sha2-512-hmac",
>
> + [RTE_CRYPTO_AUTH_SHA3_224] = "sha3-224",
> + [RTE_CRYPTO_AUTH_SHA3_224_HMAC] = "sha3-224-hmac",
> + [RTE_CRYPTO_AUTH_SHA3_256] = "sha3-256",
> + [RTE_CRYPTO_AUTH_SHA3_256_HMAC] = "sha3-256-hmac",
> + [RTE_CRYPTO_AUTH_SHA3_384] = "sha3-384",
> + [RTE_CRYPTO_AUTH_SHA3_384_HMAC] = "sha3-384-hmac",
> + [RTE_CRYPTO_AUTH_SHA3_512] = "sha3-512",
> + [RTE_CRYPTO_AUTH_SHA3_512_HMAC] = "sha3-512-hmac",
> +
> [RTE_CRYPTO_AUTH_KASUMI_F9] = "kasumi-f9",
> [RTE_CRYPTO_AUTH_SNOW3G_UIA2] = "snow3g-uia2",
> [RTE_CRYPTO_AUTH_ZUC_EIA3] = "zuc-eia3"
This is being flagged as an ABI break for 21.11.3 [1]. I don't see it
mentioned in the commit message or discussed, is it ok for main branch?
Thanks to Ali for reporting. I will revert on 21.11 branch.
[1]
1 Changed variable:
[C] 'const char* rte_crypto_auth_algorithm_strings[]' was changed at
rte_crypto_sym.h:372:1:
size of symbol changed from 168 to 232
Error: ABI issue reported for 'abidiff --suppr
dpdk/devtools/libabigail.abignore --no-added-syms --headers-dir1
/opt/dpdklab/abi_references/v21.11/armgigabyteref/include --headers-dir2
build_install/include
/opt/dpdklab/abi_references/v21.11/armgigabyteref/dump/librte_cryptodev.dump
build_install/dump/librte_cryptodev.dump'
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [EXT] Re: [PATCH] cryptodev: add missing algorithm strings
2022-11-02 10:50 ` Kevin Traynor
@ 2022-11-02 10:58 ` Akhil Goyal
2022-11-02 12:13 ` David Marchand
0 siblings, 1 reply; 6+ messages in thread
From: Akhil Goyal @ 2022-11-02 10:58 UTC (permalink / raw)
To: Kevin Traynor, Volodymyr Fialko, dev, Fan Zhang, Ravi Kumar,
Ali Alnubani
Cc: Jerin Jacob Kollanukkaran, Anoob Joseph
> On 15/09/2022 09:26, Volodymyr Fialko wrote:
> > SHA3 family algorithms were missing in the array of algorithm strings.
> >
> > Fixes: 1df800f89518 ("crypto/ccp: support SHA3 family")
> >
> > Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> > ---
> > lib/cryptodev/rte_cryptodev.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> > index 42f3221052..35661f5347 100644
> > --- a/lib/cryptodev/rte_cryptodev.c
> > +++ b/lib/cryptodev/rte_cryptodev.c
> > @@ -130,6 +130,15 @@ rte_crypto_auth_algorithm_strings[] = {
> > [RTE_CRYPTO_AUTH_SHA512] = "sha2-512",
> > [RTE_CRYPTO_AUTH_SHA512_HMAC] = "sha2-512-hmac",
> >
> > + [RTE_CRYPTO_AUTH_SHA3_224] = "sha3-224",
> > + [RTE_CRYPTO_AUTH_SHA3_224_HMAC] = "sha3-224-hmac",
> > + [RTE_CRYPTO_AUTH_SHA3_256] = "sha3-256",
> > + [RTE_CRYPTO_AUTH_SHA3_256_HMAC] = "sha3-256-hmac",
> > + [RTE_CRYPTO_AUTH_SHA3_384] = "sha3-384",
> > + [RTE_CRYPTO_AUTH_SHA3_384_HMAC] = "sha3-384-hmac",
> > + [RTE_CRYPTO_AUTH_SHA3_512] = "sha3-512",
> > + [RTE_CRYPTO_AUTH_SHA3_512_HMAC] = "sha3-512-hmac",
> > +
> > [RTE_CRYPTO_AUTH_KASUMI_F9] = "kasumi-f9",
> > [RTE_CRYPTO_AUTH_SNOW3G_UIA2] = "snow3g-uia2",
> > [RTE_CRYPTO_AUTH_ZUC_EIA3] = "zuc-eia3"
>
> This is being flagged as an ABI break for 21.11.3 [1]. I don't see it
> mentioned in the commit message or discussed, is it ok for main branch?
Ok, we can keep it to main only.
But it will be an issue on 21.11.
>
> Thanks to Ali for reporting. I will revert on 21.11 branch.
>
> [1]
> 1 Changed variable:
>
> [C] 'const char* rte_crypto_auth_algorithm_strings[]' was changed at
> rte_crypto_sym.h:372:1:
> size of symbol changed from 168 to 232
>
> Error: ABI issue reported for 'abidiff --suppr
> dpdk/devtools/libabigail.abignore --no-added-syms --headers-dir1
> /opt/dpdklab/abi_references/v21.11/armgigabyteref/include --headers-dir2
> build_install/include
> /opt/dpdklab/abi_references/v21.11/armgigabyteref/dump/librte_cryptodev.du
> mp
> build_install/dump/librte_cryptodev.dump'
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [EXT] Re: [PATCH] cryptodev: add missing algorithm strings
2022-11-02 10:58 ` [EXT] " Akhil Goyal
@ 2022-11-02 12:13 ` David Marchand
2022-11-02 12:31 ` Akhil Goyal
0 siblings, 1 reply; 6+ messages in thread
From: David Marchand @ 2022-11-02 12:13 UTC (permalink / raw)
To: Akhil Goyal
Cc: Kevin Traynor, Volodymyr Fialko, dev, Ravi Kumar, Ali Alnubani,
Jerin Jacob Kollanukkaran, Anoob Joseph, Thomas Monjalon
On Wed, Nov 2, 2022 at 11:58 AM Akhil Goyal <gakhil@marvell.com> wrote:
> > This is being flagged as an ABI break for 21.11.3 [1]. I don't see it
> > mentioned in the commit message or discussed, is it ok for main branch?
>
> Ok, we can keep it to main only.
> But it will be an issue on 21.11.
>
> >
> > Thanks to Ali for reporting. I will revert on 21.11 branch.
> >
> > [1]
> > 1 Changed variable:
> >
> > [C] 'const char* rte_crypto_auth_algorithm_strings[]' was changed at
> > rte_crypto_sym.h:372:1:
> > size of symbol changed from 168 to 232
My two cents.
We have a algo "string to num" helper (rte_cryptodev_get_auth_algo_enum).
This code is not performance sensitive, is it?
If we add the, opposite, "num to string" helper, we can hide the
rte_crypto_auth_algorithm_strings symbol from the public ABI and avoid
this kind of issues in the future.
And looking at lib/crypto map, there are other arrays (*_strings
symbols) that are subject to similar "extending" issues.
We are late in the release for adding new API though such helpers
would be really simple.
Hiding such symbols is something to consider before entering ABI freeze.
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXT] Re: [PATCH] cryptodev: add missing algorithm strings
2022-11-02 12:13 ` David Marchand
@ 2022-11-02 12:31 ` Akhil Goyal
0 siblings, 0 replies; 6+ messages in thread
From: Akhil Goyal @ 2022-11-02 12:31 UTC (permalink / raw)
To: David Marchand
Cc: Kevin Traynor, Volodymyr Fialko, dev, Ravi Kumar, Ali Alnubani,
Jerin Jacob Kollanukkaran, Anoob Joseph, Thomas Monjalon
> Subject: Re: [EXT] Re: [PATCH] cryptodev: add missing algorithm strings
>
> On Wed, Nov 2, 2022 at 11:58 AM Akhil Goyal <gakhil@marvell.com> wrote:
> > > This is being flagged as an ABI break for 21.11.3 [1]. I don't see it
> > > mentioned in the commit message or discussed, is it ok for main branch?
> >
> > Ok, we can keep it to main only.
> > But it will be an issue on 21.11.
> >
> > >
> > > Thanks to Ali for reporting. I will revert on 21.11 branch.
> > >
> > > [1]
> > > 1 Changed variable:
> > >
> > > [C] 'const char* rte_crypto_auth_algorithm_strings[]' was changed at
> > > rte_crypto_sym.h:372:1:
> > > size of symbol changed from 168 to 232
>
> My two cents.
>
> We have a algo "string to num" helper (rte_cryptodev_get_auth_algo_enum).
>
> This code is not performance sensitive, is it?
> If we add the, opposite, "num to string" helper, we can hide the
> rte_crypto_auth_algorithm_strings symbol from the public ABI and avoid
> this kind of issues in the future.
>
> And looking at lib/crypto map, there are other arrays (*_strings
> symbols) that are subject to similar "extending" issues.
>
> We are late in the release for adding new API though such helpers
> would be really simple.
> Hiding such symbols is something to consider before entering ABI freeze.
>
Agreed, but it is quite late for this cycle.
We can plan these helper APIs for next release cycle and remove these symbols in 23.11.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-02 12:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15 8:26 [PATCH] cryptodev: add missing algorithm strings Volodymyr Fialko
2022-09-22 16:01 ` Akhil Goyal
2022-11-02 10:50 ` Kevin Traynor
2022-11-02 10:58 ` [EXT] " Akhil Goyal
2022-11-02 12:13 ` David Marchand
2022-11-02 12:31 ` Akhil Goyal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).