* [PATCH] crypto/qat: fix build
@ 2023-01-12 19:30 Pablo de Lara
2023-01-12 20:39 ` Thomas Monjalon
2023-01-18 11:05 ` [PATCH v2] " Pablo de Lara
0 siblings, 2 replies; 17+ messages in thread
From: Pablo de Lara @ 2023-01-12 19:30 UTC (permalink / raw)
To: kai.ji, akhil.goyal; +Cc: dev, Pablo de Lara, stable
Instead of undefining AES_BLOCK_SIZE, define NO_COMPAT_IMB_API_053,
which prevents legacy macros like AES_BLOCK_SIZE from being defined
from IPSec MB library.
Fixes: 3227bc7138f5 ("crypto/qat: use intel-ipsec-mb for partial hash and AES")
Cc: stable@dpdk.org
Cc: kai.ji@intel.com
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
drivers/crypto/qat/qat_sym_session.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c
index 978ee49dce..7f18e9875d 100644
--- a/drivers/crypto/qat/qat_sym_session.c
+++ b/drivers/crypto/qat/qat_sym_session.c
@@ -10,6 +10,7 @@
#include <openssl/evp.h> /* Needed for bpi runt block processing */
#ifdef RTE_QAT_LIBIPSECMB
+#define NO_COMPAT_IMB_API_053
#if defined(RTE_ARCH_ARM)
#include <ipsec-mb.h>
#else
--
2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] crypto/qat: fix build
2023-01-12 19:30 [PATCH] crypto/qat: fix build Pablo de Lara
@ 2023-01-12 20:39 ` Thomas Monjalon
2023-01-17 8:22 ` [EXT] " Akhil Goyal
2023-01-18 11:05 ` [PATCH v2] " Pablo de Lara
1 sibling, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2023-01-12 20:39 UTC (permalink / raw)
To: Pablo de Lara; +Cc: kai.ji, akhil.goyal, dev, stable
12/01/2023 20:30, Pablo de Lara:
> Instead of undefining AES_BLOCK_SIZE, define NO_COMPAT_IMB_API_053,
> which prevents legacy macros like AES_BLOCK_SIZE from being defined
> from IPSec MB library.
Given we are discarding my first workaround,
your first sentence doesn't really make sense.
Please give more context, explain the conflict with OpenSSL,
and copy the error message.
Thanks
> Fixes: 3227bc7138f5 ("crypto/qat: use intel-ipsec-mb for partial hash and AES")
> Cc: stable@dpdk.org
> Cc: kai.ji@intel.com
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [EXT] Re: [PATCH] crypto/qat: fix build
2023-01-12 20:39 ` Thomas Monjalon
@ 2023-01-17 8:22 ` Akhil Goyal
2023-01-18 9:51 ` De Lara Guarch, Pablo
0 siblings, 1 reply; 17+ messages in thread
From: Akhil Goyal @ 2023-01-17 8:22 UTC (permalink / raw)
To: Thomas Monjalon, Pablo de Lara; +Cc: kai.ji, akhil.goyal, dev, stable
> 12/01/2023 20:30, Pablo de Lara:
> > Instead of undefining AES_BLOCK_SIZE, define NO_COMPAT_IMB_API_053,
> > which prevents legacy macros like AES_BLOCK_SIZE from being defined
> > from IPSec MB library.
>
> Given we are discarding my first workaround,
> your first sentence doesn't really make sense.
> Please give more context, explain the conflict with OpenSSL,
> and copy the error message.
>
Hi Pablo,
Can you fix the description as suggested by Thomas?
I would like to take all the qat build fixes together.
Thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [EXT] Re: [PATCH] crypto/qat: fix build
2023-01-17 8:22 ` [EXT] " Akhil Goyal
@ 2023-01-18 9:51 ` De Lara Guarch, Pablo
0 siblings, 0 replies; 17+ messages in thread
From: De Lara Guarch, Pablo @ 2023-01-18 9:51 UTC (permalink / raw)
To: Akhil Goyal, Thomas Monjalon; +Cc: Ji, Kai, akhil.goyal, dev, stable
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, January 17, 2023 8:23 AM
> To: Thomas Monjalon <thomas@monjalon.net>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Cc: Ji, Kai <kai.ji@intel.com>; akhil.goyal@marvell.com; dev@dpdk.org;
> stable@dpdk.org
> Subject: RE: [EXT] Re: [PATCH] crypto/qat: fix build
>
> > 12/01/2023 20:30, Pablo de Lara:
> > > Instead of undefining AES_BLOCK_SIZE, define
> NO_COMPAT_IMB_API_053,
> > > which prevents legacy macros like AES_BLOCK_SIZE from being defined
> > > from IPSec MB library.
> >
> > Given we are discarding my first workaround, your first sentence
> > doesn't really make sense.
> > Please give more context, explain the conflict with OpenSSL, and copy
> > the error message.
> >
> Hi Pablo,
>
> Can you fix the description as suggested by Thomas?
> I would like to take all the qat build fixes together.
Sure, will do.
Pablo
>
> Thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] crypto/qat: fix build
2023-01-12 19:30 [PATCH] crypto/qat: fix build Pablo de Lara
2023-01-12 20:39 ` Thomas Monjalon
@ 2023-01-18 11:05 ` Pablo de Lara
2023-01-18 14:17 ` Ji, Kai
1 sibling, 1 reply; 17+ messages in thread
From: Pablo de Lara @ 2023-01-18 11:05 UTC (permalink / raw)
To: gakhil; +Cc: dev, Pablo de Lara, stable, kai.ji
When building QAT PMD, the following issue comes up:
intel-ipsec-mb.h:333: error: "AES_BLOCK_SIZE" redefined
333 | #define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
In file included from drivers/crypto/qat/qat_sym_session.c:8:
/usr/include/openssl/aes.h:26: previous definition
26 | # define AES_BLOCK_SIZE 16
By defining NO_COMPAT_IMB_API_053, it prevents legacy macros
like AES_BLOCK_SIZE from being defined from IPSec MB library,
avoiding the redefinition.
Fixes: 3227bc7138f5 ("crypto/qat: use intel-ipsec-mb for partial hash and AES")
Cc: stable@dpdk.org
Cc: kai.ji@intel.com
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
v2: reworded the commit message
---
drivers/crypto/qat/qat_sym_session.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c
index 978ee49dce..7f18e9875d 100644
--- a/drivers/crypto/qat/qat_sym_session.c
+++ b/drivers/crypto/qat/qat_sym_session.c
@@ -10,6 +10,7 @@
#include <openssl/evp.h> /* Needed for bpi runt block processing */
#ifdef RTE_QAT_LIBIPSECMB
+#define NO_COMPAT_IMB_API_053
#if defined(RTE_ARCH_ARM)
#include <ipsec-mb.h>
#else
--
2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v2] crypto/qat: fix build
2023-01-18 11:05 ` [PATCH v2] " Pablo de Lara
@ 2023-01-18 14:17 ` Ji, Kai
2023-01-19 9:35 ` Akhil Goyal
0 siblings, 1 reply; 17+ messages in thread
From: Ji, Kai @ 2023-01-18 14:17 UTC (permalink / raw)
To: De Lara Guarch, Pablo, gakhil; +Cc: dev, stable
Acked-by: Kai Ji <kai.ji@intel.com>
> -----Original Message-----
> From: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Sent: Wednesday, January 18, 2023 11:06 AM
> To: gakhil@marvell.com
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> stable@dpdk.org; Ji, Kai <kai.ji@intel.com>
> Subject: [PATCH v2] crypto/qat: fix build
>
> When building QAT PMD, the following issue comes up:
>
> intel-ipsec-mb.h:333: error: "AES_BLOCK_SIZE" redefined
> 333 | #define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
> In file included from drivers/crypto/qat/qat_sym_session.c:8:
> /usr/include/openssl/aes.h:26: previous definition
> 26 | # define AES_BLOCK_SIZE 16
>
> By defining NO_COMPAT_IMB_API_053, it prevents legacy macros like
> AES_BLOCK_SIZE from being defined from IPSec MB library, avoiding the
> redefinition.
>
> Fixes: 3227bc7138f5 ("crypto/qat: use intel-ipsec-mb for partial hash
> and AES")
> Cc: stable@dpdk.org
> Cc: kai.ji@intel.com
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
> v2: reworded the commit message
> ---
>
> drivers/crypto/qat/qat_sym_session.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/crypto/qat/qat_sym_session.c
> b/drivers/crypto/qat/qat_sym_session.c
> index 978ee49dce..7f18e9875d 100644
> --- a/drivers/crypto/qat/qat_sym_session.c
> +++ b/drivers/crypto/qat/qat_sym_session.c
> @@ -10,6 +10,7 @@
> #include <openssl/evp.h> /* Needed for bpi runt block processing */
>
> #ifdef RTE_QAT_LIBIPSECMB
> +#define NO_COMPAT_IMB_API_053
> #if defined(RTE_ARCH_ARM)
> #include <ipsec-mb.h>
> #else
> --
> 2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v2] crypto/qat: fix build
2023-01-18 14:17 ` Ji, Kai
@ 2023-01-19 9:35 ` Akhil Goyal
0 siblings, 0 replies; 17+ messages in thread
From: Akhil Goyal @ 2023-01-19 9:35 UTC (permalink / raw)
To: Ji, Kai, De Lara Guarch, Pablo; +Cc: dev, stable
> Acked-by: Kai Ji <kai.ji@intel.com>
>
> > Subject: [PATCH v2] crypto/qat: fix build
> >
> > When building QAT PMD, the following issue comes up:
> >
> > intel-ipsec-mb.h:333: error: "AES_BLOCK_SIZE" redefined
> > 333 | #define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
> > In file included from drivers/crypto/qat/qat_sym_session.c:8:
> > /usr/include/openssl/aes.h:26: previous definition
> > 26 | # define AES_BLOCK_SIZE 16
> >
> > By defining NO_COMPAT_IMB_API_053, it prevents legacy macros like
> > AES_BLOCK_SIZE from being defined from IPSec MB library, avoiding the
> > redefinition.
> >
> > Fixes: 3227bc7138f5 ("crypto/qat: use intel-ipsec-mb for partial hash
> > and AES")
> > Cc: stable@dpdk.org
> > Cc: kai.ji@intel.com
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Applied to dpdk-next-crypto
Thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] crypto/qat: fix build
2023-01-12 13:22 ` De Lara Guarch, Pablo
2023-01-12 15:00 ` Thomas Monjalon
@ 2023-01-12 16:34 ` Tyler Retzlaff
1 sibling, 0 replies; 17+ messages in thread
From: Tyler Retzlaff @ 2023-01-12 16:34 UTC (permalink / raw)
To: De Lara Guarch, Pablo
Cc: Thomas Monjalon, Akhil Goyal, Ji, Kai, dev, David Marchand,
Dooley, Brian, Power, Ciara, Mcnamara, John
On Thu, Jan 12, 2023 at 01:22:09PM +0000, De Lara Guarch, Pablo wrote:
> Hi Thomas,
>
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, January 12, 2023 10:41 AM
> > To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Akhil Goyal
> > <gakhil@marvell.com>; Ji, Kai <kai.ji@intel.com>
> > Cc: dev@dpdk.org; Tyler Retzlaff <roretzla@linux.microsoft.com>;
> > dev@dpdk.org; David Marchand <david.marchand@redhat.com>; Dooley,
> > Brian <brian.dooley@intel.com>; Power, Ciara <ciara.power@intel.com>;
> > Mcnamara, John <john.mcnamara@intel.com>
> > Subject: Re: [PATCH] crypto/qat: fix build
> >
> > 12/01/2023 11:32, Ji, Kai:
> > > Ok, a long story short, this issue should only occurred when
> > RTE_QAT_LIBIPSECMB is enabled.
> > > It was intend to remove Openssl lib dependency in QAT replaced with
> > > ipsec_mb lib, but the work was partially done due to limitation of
> > > ipsec_mb by the time (FIPS certification)
> > >
> > > I'm happy with current fix and please cc: stable@dpdk.org
> >
> > I'm not happy with this fix. It is a dirty workaround.
> > It would be better to have an #ifdef in ipsec_mb.
> >
> > Also I would like an answer to the question below. What triggered this error?
> > Is it a new thing in the lib ipsec_mb?
> > Why defining AES_BLOCK_SIZE while IMB_AES_BLOCK_SIZE can be used and
> > have a proper prefix?
>
> Apologies for the late response.
>
> This macro was renamed to IMB_AES_BLOCK_SIZE, as you already know.
> The problem is that, for compatibility reasons, we had to keep the old macro as well.
doesn't this mean the compat could have been retained with a simple
check?
#ifndef AES_BLOCK_SIZE
#define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
#endif
anyway, you can ignore this comment if you already worked out a solution
on the mail thread.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] crypto/qat: fix build
2023-01-12 16:16 ` De Lara Guarch, Pablo
@ 2023-01-12 16:28 ` Thomas Monjalon
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2023-01-12 16:28 UTC (permalink / raw)
To: Akhil Goyal, Ji, Kai, De Lara Guarch, Pablo
Cc: dev, Tyler Retzlaff, dev, David Marchand, Dooley, Brian, Power,
Ciara, Mcnamara, John
12/01/2023 17:16, De Lara Guarch, Pablo:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 12/01/2023 14:22, De Lara Guarch, Pablo:
> > > Hi Thomas,
> > >
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 12/01/2023 11:32, Ji, Kai:
> > > > > Ok, a long story short, this issue should only occurred when
> > > > RTE_QAT_LIBIPSECMB is enabled.
> > > > > It was intend to remove Openssl lib dependency in QAT replaced
> > > > > with ipsec_mb lib, but the work was partially done due to
> > > > > limitation of ipsec_mb by the time (FIPS certification)
> > > > >
> > > > > I'm happy with current fix and please cc: stable@dpdk.org
> > > >
> > > > I'm not happy with this fix. It is a dirty workaround.
> > > > It would be better to have an #ifdef in ipsec_mb.
> > > >
> > > > Also I would like an answer to the question below. What triggered this
> > error?
> > > > Is it a new thing in the lib ipsec_mb?
> > > > Why defining AES_BLOCK_SIZE while IMB_AES_BLOCK_SIZE can be used
> > and
> > > > have a proper prefix?
> > >
> > > Apologies for the late response.
> > >
> > > This macro was renamed to IMB_AES_BLOCK_SIZE, as you already know.
> > > The problem is that, for compatibility reasons, we had to keep the old
> > macro as well.
> > > However, we added a compile time flag to remove these legacy macros,
> > > for exactly this reason (NO_COMPAT_IMB_API_053).
> > >
> > > I think a solution could be to use this flag in QAT, so the legacy macros are
> > not defined.
> > >
> > > I will send a patch to fix this.
> >
> > OK good, so we can reject this patch?
> >
>
> Well, this patch is merged already, but mine will revert it and add the new flag
> (pointing at the other commit to be fixed), so that should be OK, right?
The patch was merged in the crypto tree but we can discard it.
Akhil, please remove this patch from your tree, thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] crypto/qat: fix build
2023-01-12 15:00 ` Thomas Monjalon
@ 2023-01-12 16:16 ` De Lara Guarch, Pablo
2023-01-12 16:28 ` Thomas Monjalon
0 siblings, 1 reply; 17+ messages in thread
From: De Lara Guarch, Pablo @ 2023-01-12 16:16 UTC (permalink / raw)
To: Thomas Monjalon, Akhil Goyal, Ji, Kai
Cc: dev, Tyler Retzlaff, dev, David Marchand, Dooley, Brian, Power,
Ciara, Mcnamara, John
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, January 12, 2023 3:00 PM
> To: Akhil Goyal <gakhil@marvell.com>; Ji, Kai <kai.ji@intel.com>; De Lara
> Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; Tyler Retzlaff <roretzla@linux.microsoft.com>;
> dev@dpdk.org; David Marchand <david.marchand@redhat.com>; Dooley,
> Brian <brian.dooley@intel.com>; Power, Ciara <ciara.power@intel.com>;
> Mcnamara, John <john.mcnamara@intel.com>
> Subject: Re: [PATCH] crypto/qat: fix build
>
> 12/01/2023 14:22, De Lara Guarch, Pablo:
> > Hi Thomas,
> >
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 12/01/2023 11:32, Ji, Kai:
> > > > Ok, a long story short, this issue should only occurred when
> > > RTE_QAT_LIBIPSECMB is enabled.
> > > > It was intend to remove Openssl lib dependency in QAT replaced
> > > > with ipsec_mb lib, but the work was partially done due to
> > > > limitation of ipsec_mb by the time (FIPS certification)
> > > >
> > > > I'm happy with current fix and please cc: stable@dpdk.org
> > >
> > > I'm not happy with this fix. It is a dirty workaround.
> > > It would be better to have an #ifdef in ipsec_mb.
> > >
> > > Also I would like an answer to the question below. What triggered this
> error?
> > > Is it a new thing in the lib ipsec_mb?
> > > Why defining AES_BLOCK_SIZE while IMB_AES_BLOCK_SIZE can be used
> and
> > > have a proper prefix?
> >
> > Apologies for the late response.
> >
> > This macro was renamed to IMB_AES_BLOCK_SIZE, as you already know.
> > The problem is that, for compatibility reasons, we had to keep the old
> macro as well.
> > However, we added a compile time flag to remove these legacy macros,
> > for exactly this reason (NO_COMPAT_IMB_API_053).
> >
> > I think a solution could be to use this flag in QAT, so the legacy macros are
> not defined.
> >
> > I will send a patch to fix this.
>
> OK good, so we can reject this patch?
>
Well, this patch is merged already, but mine will revert it and add the new flag
(pointing at the other commit to be fixed), so that should be OK, right?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] crypto/qat: fix build
2023-01-12 13:22 ` De Lara Guarch, Pablo
@ 2023-01-12 15:00 ` Thomas Monjalon
2023-01-12 16:16 ` De Lara Guarch, Pablo
2023-01-12 16:34 ` Tyler Retzlaff
1 sibling, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2023-01-12 15:00 UTC (permalink / raw)
To: Akhil Goyal, Ji, Kai, De Lara Guarch, Pablo
Cc: dev, Tyler Retzlaff, dev, David Marchand, Dooley, Brian, Power,
Ciara, Mcnamara, John
12/01/2023 14:22, De Lara Guarch, Pablo:
> Hi Thomas,
>
> From: Thomas Monjalon <thomas@monjalon.net>
> > 12/01/2023 11:32, Ji, Kai:
> > > Ok, a long story short, this issue should only occurred when
> > RTE_QAT_LIBIPSECMB is enabled.
> > > It was intend to remove Openssl lib dependency in QAT replaced with
> > > ipsec_mb lib, but the work was partially done due to limitation of
> > > ipsec_mb by the time (FIPS certification)
> > >
> > > I'm happy with current fix and please cc: stable@dpdk.org
> >
> > I'm not happy with this fix. It is a dirty workaround.
> > It would be better to have an #ifdef in ipsec_mb.
> >
> > Also I would like an answer to the question below. What triggered this error?
> > Is it a new thing in the lib ipsec_mb?
> > Why defining AES_BLOCK_SIZE while IMB_AES_BLOCK_SIZE can be used and
> > have a proper prefix?
>
> Apologies for the late response.
>
> This macro was renamed to IMB_AES_BLOCK_SIZE, as you already know.
> The problem is that, for compatibility reasons, we had to keep the old macro as well.
> However, we added a compile time flag to remove these legacy macros, for exactly this reason
> (NO_COMPAT_IMB_API_053).
>
> I think a solution could be to use this flag in QAT, so the legacy macros are not defined.
>
> I will send a patch to fix this.
OK good, so we can reject this patch?
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] crypto/qat: fix build
2023-01-12 10:40 ` Thomas Monjalon
@ 2023-01-12 13:22 ` De Lara Guarch, Pablo
2023-01-12 15:00 ` Thomas Monjalon
2023-01-12 16:34 ` Tyler Retzlaff
0 siblings, 2 replies; 17+ messages in thread
From: De Lara Guarch, Pablo @ 2023-01-12 13:22 UTC (permalink / raw)
To: Thomas Monjalon, Akhil Goyal, Ji, Kai
Cc: dev, Tyler Retzlaff, dev, David Marchand, Dooley, Brian, Power,
Ciara, Mcnamara, John
Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, January 12, 2023 10:41 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Akhil Goyal
> <gakhil@marvell.com>; Ji, Kai <kai.ji@intel.com>
> Cc: dev@dpdk.org; Tyler Retzlaff <roretzla@linux.microsoft.com>;
> dev@dpdk.org; David Marchand <david.marchand@redhat.com>; Dooley,
> Brian <brian.dooley@intel.com>; Power, Ciara <ciara.power@intel.com>;
> Mcnamara, John <john.mcnamara@intel.com>
> Subject: Re: [PATCH] crypto/qat: fix build
>
> 12/01/2023 11:32, Ji, Kai:
> > Ok, a long story short, this issue should only occurred when
> RTE_QAT_LIBIPSECMB is enabled.
> > It was intend to remove Openssl lib dependency in QAT replaced with
> > ipsec_mb lib, but the work was partially done due to limitation of
> > ipsec_mb by the time (FIPS certification)
> >
> > I'm happy with current fix and please cc: stable@dpdk.org
>
> I'm not happy with this fix. It is a dirty workaround.
> It would be better to have an #ifdef in ipsec_mb.
>
> Also I would like an answer to the question below. What triggered this error?
> Is it a new thing in the lib ipsec_mb?
> Why defining AES_BLOCK_SIZE while IMB_AES_BLOCK_SIZE can be used and
> have a proper prefix?
Apologies for the late response.
This macro was renamed to IMB_AES_BLOCK_SIZE, as you already know.
The problem is that, for compatibility reasons, we had to keep the old macro as well.
However, we added a compile time flag to remove these legacy macros, for exactly this reason
(NO_COMPAT_IMB_API_053).
I think a solution could be to use this flag in QAT, so the legacy macros are not defined.
I will send a patch to fix this.
Thanks,
Pablo
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] crypto/qat: fix build
2023-01-12 10:32 ` Ji, Kai
@ 2023-01-12 10:40 ` Thomas Monjalon
2023-01-12 13:22 ` De Lara Guarch, Pablo
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2023-01-12 10:40 UTC (permalink / raw)
To: De Lara Guarch, Pablo, Akhil Goyal, Ji, Kai
Cc: dev, Tyler Retzlaff, dev, David Marchand, Dooley, Brian, Power,
Ciara, Mcnamara, John
12/01/2023 11:32, Ji, Kai:
> Ok, a long story short, this issue should only occurred when RTE_QAT_LIBIPSECMB is enabled.
> It was intend to remove Openssl lib dependency in QAT replaced with ipsec_mb lib, but the work was partially done due to limitation of ipsec_mb by the time (FIPS certification)
>
> I'm happy with current fix and please cc: stable@dpdk.org
I'm not happy with this fix. It is a dirty workaround.
It would be better to have an #ifdef in ipsec_mb.
Also I would like an answer to the question below. What triggered this error?
Is it a new thing in the lib ipsec_mb?
Why defining AES_BLOCK_SIZE while IMB_AES_BLOCK_SIZE can be used and have a proper prefix?
> The fully removal of Openssl dependency is already ongoing, I will take a note to fix this properly
>
> Regards
>
> Kai
>
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Subject: Re: [PATCH] crypto/qat: fix build
> >
> > Waiting for an answer here.
> > The commit log is not supposed to stay like this with questions.
> >
> >
> > 11/01/2023 10:03, Thomas Monjalon:
> > > 04/01/2023 12:56, Akhil Goyal:
> > > > > On Fri, Dec 30, 2022 at 10:07:28PM +0100, Thomas Monjalon wrote:
> > > > > > When trying to compile on a fresh system, I hit this error:
> > > > > >
> > > > > > intel-ipsec-mb.h:333: error: "AES_BLOCK_SIZE" redefined
> > > > > > 333 | #define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
> > > > > > In file included from drivers/crypto/qat/qat_sym_session.c:8:
> > > > > > /usr/include/openssl/aes.h:26: previous definition
> > > > > > 26 | # define AES_BLOCK_SIZE 16
> > > > > >
> > > > > > I don't know why it was not seen before.
> > > > > > Is it because of a change in intel-ipsec-mb.h or in OpenSSL?
> > > > > >
> > > > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > > > > ---
> > > > >
> > > > > owners of intel-ipsec-mb.h should guard against the namespace
> > > > > conflict...
> > > > >
> > > > > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > >
> > > > Applied to dpdk-next-crypto
> >
> > If there is no better fix, we should at least add Cc: stable@dpdk.org
> > assuming it could be reproduced with an older DPDK.
> >
> > > >
> > > > Thanks.
> > >
> > > I'm concerned to have no answer from Pablo and Kai.
> > > It is real design problem. Is there any plan to have a protected
> > namespace?
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] crypto/qat: fix build
2023-01-11 23:20 ` Thomas Monjalon
@ 2023-01-12 10:32 ` Ji, Kai
2023-01-12 10:40 ` Thomas Monjalon
0 siblings, 1 reply; 17+ messages in thread
From: Ji, Kai @ 2023-01-12 10:32 UTC (permalink / raw)
To: Thomas Monjalon, De Lara Guarch, Pablo, Akhil Goyal
Cc: dev, Tyler Retzlaff, dev, David Marchand, Dooley, Brian, Power,
Ciara, Mcnamara, John
Ok, a long story short, this issue should only occurred when RTE_QAT_LIBIPSECMB is enabled.
It was intend to remove Openssl lib dependency in QAT replaced with ipsec_mb lib, but the work was partially done due to limitation of ipsec_mb by the time (FIPS certification)
I'm happy with current fix and please cc: stable@dpdk.org
The fully removal of Openssl dependency is already ongoing, I will take a note to fix this properly
Regards
Kai
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, January 11, 2023 11:21 PM
> To: Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Akhil Goyal <gakhil@marvell.com>
> Cc: dev@dpdk.org; Tyler Retzlaff <roretzla@linux.microsoft.com>;
> dev@dpdk.org; David Marchand <david.marchand@redhat.com>; Dooley, Brian
> <brian.dooley@intel.com>; Power, Ciara <ciara.power@intel.com>; Mcnamara,
> John <john.mcnamara@intel.com>
> Subject: Re: [PATCH] crypto/qat: fix build
>
> Waiting for an answer here.
> The commit log is not supposed to stay like this with questions.
>
>
> 11/01/2023 10:03, Thomas Monjalon:
> > 04/01/2023 12:56, Akhil Goyal:
> > > > On Fri, Dec 30, 2022 at 10:07:28PM +0100, Thomas Monjalon wrote:
> > > > > When trying to compile on a fresh system, I hit this error:
> > > > >
> > > > > intel-ipsec-mb.h:333: error: "AES_BLOCK_SIZE" redefined
> > > > > 333 | #define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
> > > > > In file included from drivers/crypto/qat/qat_sym_session.c:8:
> > > > > /usr/include/openssl/aes.h:26: previous definition
> > > > > 26 | # define AES_BLOCK_SIZE 16
> > > > >
> > > > > I don't know why it was not seen before.
> > > > > Is it because of a change in intel-ipsec-mb.h or in OpenSSL?
> > > > >
> > > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > > > ---
> > > >
> > > > owners of intel-ipsec-mb.h should guard against the namespace
> > > > conflict...
> > > >
> > > > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > >
> > > Applied to dpdk-next-crypto
>
> If there is no better fix, we should at least add Cc: stable@dpdk.org
> assuming it could be reproduced with an older DPDK.
>
> > >
> > > Thanks.
> >
> > I'm concerned to have no answer from Pablo and Kai.
> > It is real design problem. Is there any plan to have a protected
> namespace?
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] crypto/qat: fix build
2023-01-11 9:03 ` Thomas Monjalon
@ 2023-01-11 23:20 ` Thomas Monjalon
2023-01-12 10:32 ` Ji, Kai
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2023-01-11 23:20 UTC (permalink / raw)
To: Kai Ji, pablo.de.lara.guarch, Akhil Goyal
Cc: dev, Tyler Retzlaff, dev, David Marchand, Brian Dooley,
Ciara Power, John McNamara
Waiting for an answer here.
The commit log is not supposed to stay like this with questions.
11/01/2023 10:03, Thomas Monjalon:
> 04/01/2023 12:56, Akhil Goyal:
> > > On Fri, Dec 30, 2022 at 10:07:28PM +0100, Thomas Monjalon wrote:
> > > > When trying to compile on a fresh system, I hit this error:
> > > >
> > > > intel-ipsec-mb.h:333: error: "AES_BLOCK_SIZE" redefined
> > > > 333 | #define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
> > > > In file included from drivers/crypto/qat/qat_sym_session.c:8:
> > > > /usr/include/openssl/aes.h:26: previous definition
> > > > 26 | # define AES_BLOCK_SIZE 16
> > > >
> > > > I don't know why it was not seen before.
> > > > Is it because of a change in intel-ipsec-mb.h or in OpenSSL?
> > > >
> > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > > ---
> > >
> > > owners of intel-ipsec-mb.h should guard against the namespace
> > > conflict...
> > >
> > > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >
> > Applied to dpdk-next-crypto
If there is no better fix, we should at least add Cc: stable@dpdk.org
assuming it could be reproduced with an older DPDK.
> >
> > Thanks.
>
> I'm concerned to have no answer from Pablo and Kai.
> It is real design problem. Is there any plan to have a protected namespace?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] crypto/qat: fix build
2022-12-30 21:07 [PATCH] " Thomas Monjalon
@ 2022-12-30 21:38 ` Tyler Retzlaff
2023-01-04 11:56 ` [EXT] " Akhil Goyal
1 sibling, 0 replies; 17+ messages in thread
From: Tyler Retzlaff @ 2022-12-30 21:38 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Kai Ji, dev, pablo.de.lara.guarch, David Marchand
On Fri, Dec 30, 2022 at 10:07:28PM +0100, Thomas Monjalon wrote:
> When trying to compile on a fresh system, I hit this error:
>
> intel-ipsec-mb.h:333: error: "AES_BLOCK_SIZE" redefined
> 333 | #define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
> In file included from drivers/crypto/qat/qat_sym_session.c:8:
> /usr/include/openssl/aes.h:26: previous definition
> 26 | # define AES_BLOCK_SIZE 16
>
> I don't know why it was not seen before.
> Is it because of a change in intel-ipsec-mb.h or in OpenSSL?
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
owners of intel-ipsec-mb.h should guard against the namespace
conflict...
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] crypto/qat: fix build
@ 2022-12-30 21:07 Thomas Monjalon
2022-12-30 21:38 ` Tyler Retzlaff
2023-01-04 11:56 ` [EXT] " Akhil Goyal
0 siblings, 2 replies; 17+ messages in thread
From: Thomas Monjalon @ 2022-12-30 21:07 UTC (permalink / raw)
To: Kai Ji; +Cc: dev, pablo.de.lara.guarch, David Marchand
When trying to compile on a fresh system, I hit this error:
intel-ipsec-mb.h:333: error: "AES_BLOCK_SIZE" redefined
333 | #define AES_BLOCK_SIZE IMB_AES_BLOCK_SIZE
In file included from drivers/crypto/qat/qat_sym_session.c:8:
/usr/include/openssl/aes.h:26: previous definition
26 | # define AES_BLOCK_SIZE 16
I don't know why it was not seen before.
Is it because of a change in intel-ipsec-mb.h or in OpenSSL?
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
drivers/crypto/qat/qat_sym_session.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c
index 0ebc66f89e..37bde297c0 100644
--- a/drivers/crypto/qat/qat_sym_session.c
+++ b/drivers/crypto/qat/qat_sym_session.c
@@ -10,6 +10,7 @@
#include <openssl/evp.h> /* Needed for bpi runt block processing */
#ifdef RTE_QAT_LIBIPSECMB
+#undef AES_BLOCK_SIZE
#if defined(RTE_ARCH_ARM)
#include <ipsec-mb.h>
#else
--
2.39.0
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-01-19 9:35 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-12 19:30 [PATCH] crypto/qat: fix build Pablo de Lara
2023-01-12 20:39 ` Thomas Monjalon
2023-01-17 8:22 ` [EXT] " Akhil Goyal
2023-01-18 9:51 ` De Lara Guarch, Pablo
2023-01-18 11:05 ` [PATCH v2] " Pablo de Lara
2023-01-18 14:17 ` Ji, Kai
2023-01-19 9:35 ` Akhil Goyal
-- strict thread matches above, loose matches on Subject: below --
2022-12-30 21:07 [PATCH] " Thomas Monjalon
2022-12-30 21:38 ` Tyler Retzlaff
2023-01-04 11:56 ` [EXT] " Akhil Goyal
2023-01-11 9:03 ` Thomas Monjalon
2023-01-11 23:20 ` Thomas Monjalon
2023-01-12 10:32 ` Ji, Kai
2023-01-12 10:40 ` Thomas Monjalon
2023-01-12 13:22 ` De Lara Guarch, Pablo
2023-01-12 15:00 ` Thomas Monjalon
2023-01-12 16:16 ` De Lara Guarch, Pablo
2023-01-12 16:28 ` Thomas Monjalon
2023-01-12 16:34 ` Tyler Retzlaff
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).