DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] crypto/qat: fix build
@ 2022-12-30 21:07 Thomas Monjalon
  2022-12-30 21:38 ` Tyler Retzlaff
  0 siblings, 1 reply; 15+ 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] 15+ messages in thread

* Re: [PATCH] crypto/qat: fix build
  2022-12-30 21:07 [PATCH] crypto/qat: fix build Thomas Monjalon
@ 2022-12-30 21:38 ` Tyler Retzlaff
  2023-01-04 11:56   ` [EXT] " Akhil Goyal
  0 siblings, 1 reply; 15+ 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] 15+ messages in thread

* RE: [EXT] Re: [PATCH] crypto/qat: fix build
  2022-12-30 21:38 ` Tyler Retzlaff
@ 2023-01-04 11:56   ` Akhil Goyal
  2023-01-11  9:03     ` Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Akhil Goyal @ 2023-01-04 11:56 UTC (permalink / raw)
  To: Tyler Retzlaff, 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>

Applied to dpdk-next-crypto

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [EXT] Re: [PATCH] crypto/qat: fix build
  2023-01-04 11:56   ` [EXT] " Akhil Goyal
@ 2023-01-11  9:03     ` Thomas Monjalon
  2023-01-11 23:20       ` Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2023-01-11  9:03 UTC (permalink / raw)
  To: Kai Ji, pablo.de.lara.guarch, Akhil Goyal
  Cc: Tyler Retzlaff, dev, dev, David Marchand

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?



^ permalink raw reply	[flat|nested] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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
  2023-01-12 16:56                     ` [EXT] " Akhil Goyal
  0 siblings, 1 reply; 15+ 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] 15+ 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; 15+ 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] 15+ messages in thread

* RE: [EXT] Re: [PATCH] crypto/qat: fix build
  2023-01-12 16:28                   ` Thomas Monjalon
@ 2023-01-12 16:56                     ` Akhil Goyal
  0 siblings, 0 replies; 15+ messages in thread
From: Akhil Goyal @ 2023-01-12 16:56 UTC (permalink / raw)
  To: Thomas Monjalon, 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.
> 
Done.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] crypto/qat: fix build
  2023-01-12 19:30 Pablo de Lara
@ 2023-01-12 20:39 ` Thomas Monjalon
  0 siblings, 0 replies; 15+ 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] 15+ messages in thread

* [PATCH] crypto/qat: fix build
@ 2023-01-12 19:30 Pablo de Lara
  2023-01-12 20:39 ` Thomas Monjalon
  0 siblings, 1 reply; 15+ 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] 15+ messages in thread

end of thread, other threads:[~2023-01-12 20:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-30 21:07 [PATCH] crypto/qat: fix build 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:56                     ` [EXT] " Akhil Goyal
2023-01-12 16:34               ` Tyler Retzlaff
2023-01-12 19:30 Pablo de Lara
2023-01-12 20:39 ` Thomas Monjalon

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