crypto/qat: fix build

  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(+)
  

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>
  

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

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?
  

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?
  

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

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?
  

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

>
  

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?
  

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

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.
  

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.
  

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