[PATCH] crypto/qat: fix build

[PATCH] crypto/qat: fix build

[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.

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

Re: [PATCH] crypto/qat: fix build

[Thomas Monjalon]

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>

RE: [EXT] Re: [PATCH] crypto/qat: fix build

[Akhil Goyal]

> 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.

RE: [EXT] Re: [PATCH] crypto/qat: fix build

[De Lara Guarch, Pablo]

> -----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.

[PATCH v2] crypto/qat: fix build

[Pablo de Lara]

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

RE: [PATCH v2] crypto/qat: fix build

[Ji, Kai]

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

RE: [PATCH v2] crypto/qat: fix build

[Akhil Goyal]

> 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.

[PATCH] crypto/qat: fix build

[Thomas Monjalon]

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

Re: [PATCH] crypto/qat: fix build

[Tyler Retzlaff]

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>

RE: [EXT] Re: [PATCH] crypto/qat: fix build

[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

Thanks.

Re: [EXT] Re: [PATCH] crypto/qat: fix build

[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
> 
> 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?

Re: [PATCH] crypto/qat: fix build

[Thomas Monjalon]

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?

RE: [PATCH] crypto/qat: fix build

[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
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?
> 
> 

Re: [PATCH] crypto/qat: fix build

[Thomas Monjalon]

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?

RE: [PATCH] crypto/qat: fix build

[De Lara Guarch, Pablo]

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

> 

Re: [PATCH] crypto/qat: fix build

[Thomas Monjalon]

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?

RE: [PATCH] crypto/qat: fix build

[De Lara Guarch, Pablo]

> -----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?

Re: [PATCH] crypto/qat: fix build

[Thomas Monjalon]

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.

Re: [PATCH] crypto/qat: fix build

[Tyler Retzlaff]

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.