[v2,1/2] build: add in option for qat use intel ipsec-mb lib

Message ID 20220517141652.53769-1-kai.ji@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series [v2,1/2] build: add in option for qat use intel ipsec-mb lib |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ji, Kai May 17, 2022, 2:16 p.m. UTC
  Add in build option for QAT pmd use intel ipsec-mb lib
instead openssl for precompute partial hash & aes.

Signed-off-by: Kai Ji <kai.ji@intel.com>
---
 drivers/common/qat/meson.build | 12 ++++++++++++
 meson_options.txt              |  2 ++
 2 files changed, 14 insertions(+)
  

Comments

Fan Zhang May 17, 2022, 3 p.m. UTC | #1
> -----Original Message-----
> From: Kai Ji <kai.ji@intel.com>
> Sent: Tuesday, May 17, 2022 3:17 PM
> To: dev@dpdk.org
> Cc: Ji, Kai <kai.ji@intel.com>
> Subject: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-mb lib
> 
> Add in build option for QAT pmd use intel ipsec-mb lib
> instead openssl for precompute partial hash & aes.
> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> ---
Please don't forget to CC maintainer next time :-)

Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
  
Bruce Richardson May 18, 2022, 8:04 a.m. UTC | #2
On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
> Add in build option for QAT pmd use intel ipsec-mb lib
> instead openssl for precompute partial hash & aes.
> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> ---
>  drivers/common/qat/meson.build | 12 ++++++++++++
>  meson_options.txt              |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/common/qat/meson.build b/drivers/common/qat/meson.build
> index b7027f3164..d6eaff3e0e 100644
> --- a/drivers/common/qat/meson.build
> +++ b/drivers/common/qat/meson.build
> @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
>              'missing dependency, libcrypto')
>  endif
>  
> +if get_option('qat_libipsecmb')
> +    IMB_required_ver = '1.0.0'
> +    libipsecmb = cc.find_library('IPSec_MB', required: false)
> +    if not lib.found()
> +        build = false
> +        reason = 'missing dependency, "libIPSec_MB"'
> +    else
> +        ext_deps += libipsecmb
> +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
> +    endif
> +endif
> +

In general it's not a good idea for individual drivers to add top-level
meson.build options. If every driver does this for all its options we will
have an unmanageable set of hundreds of options. Unfortunately, though, I
don't see a really good way to replace this with something other than a
build option - I assume it's not just good enough to check which of the
dependencies is found/not-found? Is there some way that this can be a
generic option, that may be applicable to other drivers? Could the option
be instead to prioritize FIPs compatibility?

/Bruce
  
Ferruh Yigit May 18, 2022, 10:26 a.m. UTC | #3
On 5/18/2022 9:04 AM, Bruce Richardson wrote:
> On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
>> Add in build option for QAT pmd use intel ipsec-mb lib
>> instead openssl for precompute partial hash & aes.
>>
>> Signed-off-by: Kai Ji <kai.ji@intel.com>
>> ---
>>   drivers/common/qat/meson.build | 12 ++++++++++++
>>   meson_options.txt              |  2 ++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/common/qat/meson.build b/drivers/common/qat/meson.build
>> index b7027f3164..d6eaff3e0e 100644
>> --- a/drivers/common/qat/meson.build
>> +++ b/drivers/common/qat/meson.build
>> @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
>>               'missing dependency, libcrypto')
>>   endif
>>   
>> +if get_option('qat_libipsecmb')
>> +    IMB_required_ver = '1.0.0'
>> +    libipsecmb = cc.find_library('IPSec_MB', required: false)
>> +    if not lib.found()
>> +        build = false
>> +        reason = 'missing dependency, "libIPSec_MB"'
>> +    else
>> +        ext_deps += libipsecmb
>> +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
>> +    endif
>> +endif
>> +
> 
> In general it's not a good idea for individual drivers to add top-level
> meson.build options. If every driver does this for all its options we will
> have an unmanageable set of hundreds of options. Unfortunately, though, I
> don't see a really good way to replace this with something other than a
> build option - I assume it's not just good enough to check which of the
> dependencies is found/not-found? Is there some way that this can be a
> generic option, that may be applicable to other drivers? Could the option
> be instead to prioritize FIPs compatibility?
> 

+1 to not have top level meson option for a driver.

Will it work to give priority to intel-ipsec-mb lib, like when ipsec-mb 
library exists use it, else use openssl?
  
Bruce Richardson May 18, 2022, 10:33 a.m. UTC | #4
On Wed, May 18, 2022 at 11:26:27AM +0100, Ferruh Yigit wrote:
> On 5/18/2022 9:04 AM, Bruce Richardson wrote:
> > On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
> > > Add in build option for QAT pmd use intel ipsec-mb lib
> > > instead openssl for precompute partial hash & aes.
> > > 
> > > Signed-off-by: Kai Ji <kai.ji@intel.com>
> > > ---
> > >   drivers/common/qat/meson.build | 12 ++++++++++++
> > >   meson_options.txt              |  2 ++
> > >   2 files changed, 14 insertions(+)
> > > 
> > > diff --git a/drivers/common/qat/meson.build b/drivers/common/qat/meson.build
> > > index b7027f3164..d6eaff3e0e 100644
> > > --- a/drivers/common/qat/meson.build
> > > +++ b/drivers/common/qat/meson.build
> > > @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
> > >               'missing dependency, libcrypto')
> > >   endif
> > > +if get_option('qat_libipsecmb')
> > > +    IMB_required_ver = '1.0.0'
> > > +    libipsecmb = cc.find_library('IPSec_MB', required: false)
> > > +    if not lib.found()
> > > +        build = false
> > > +        reason = 'missing dependency, "libIPSec_MB"'
> > > +    else
> > > +        ext_deps += libipsecmb
> > > +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
> > > +    endif
> > > +endif
> > > +
> > 
> > In general it's not a good idea for individual drivers to add top-level
> > meson.build options. If every driver does this for all its options we will
> > have an unmanageable set of hundreds of options. Unfortunately, though, I
> > don't see a really good way to replace this with something other than a
> > build option - I assume it's not just good enough to check which of the
> > dependencies is found/not-found? Is there some way that this can be a
> > generic option, that may be applicable to other drivers? Could the option
> > be instead to prioritize FIPs compatibility?
> > 
> 
> +1 to not have top level meson option for a driver.
> 
> Will it work to give priority to intel-ipsec-mb lib, like when ipsec-mb
> library exists use it, else use openssl?

Looking at the patch a bit more, is there a reason why this cannot be made
a run-time option when both libraries are available?
  
Ji, Kai May 18, 2022, 1:35 p.m. UTC | #5
Hi Bruce,

We want user to have the ability to choice which lib they want to use, so the original design was to make the build option temporary, and it will get removed after ipsec-mb FIP certified.

Will look into the runtime option to avoid top level build option modification.

Regards

Kai 

> Looking at the patch a bit more, is there a reason why this cannot be made a run-
> time option when both libraries are available?
  
Fan Zhang May 19, 2022, 9:14 a.m. UTC | #6
> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Wednesday, May 18, 2022 11:33 AM
> To: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Cc: Ji, Kai <kai.ji@intel.com>; dev@dpdk.org; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: Re: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-mb
> lib
> 
> On Wed, May 18, 2022 at 11:26:27AM +0100, Ferruh Yigit wrote:
> > On 5/18/2022 9:04 AM, Bruce Richardson wrote:
> > > On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
> > > > Add in build option for QAT pmd use intel ipsec-mb lib
> > > > instead openssl for precompute partial hash & aes.
> > > >
> > > > Signed-off-by: Kai Ji <kai.ji@intel.com>
> > > > ---
> > > >   drivers/common/qat/meson.build | 12 ++++++++++++
> > > >   meson_options.txt              |  2 ++
> > > >   2 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/common/qat/meson.build
> b/drivers/common/qat/meson.build
> > > > index b7027f3164..d6eaff3e0e 100644
> > > > --- a/drivers/common/qat/meson.build
> > > > +++ b/drivers/common/qat/meson.build
> > > > @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
> > > >               'missing dependency, libcrypto')
> > > >   endif
> > > > +if get_option('qat_libipsecmb')
> > > > +    IMB_required_ver = '1.0.0'
> > > > +    libipsecmb = cc.find_library('IPSec_MB', required: false)
> > > > +    if not lib.found()
> > > > +        build = false
> > > > +        reason = 'missing dependency, "libIPSec_MB"'
> > > > +    else
> > > > +        ext_deps += libipsecmb
> > > > +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
> > > > +    endif
> > > > +endif
> > > > +
> > >
> > > In general it's not a good idea for individual drivers to add top-level
> > > meson.build options. If every driver does this for all its options we will
> > > have an unmanageable set of hundreds of options. Unfortunately,
> though, I
> > > don't see a really good way to replace this with something other than a
> > > build option - I assume it's not just good enough to check which of the
> > > dependencies is found/not-found? Is there some way that this can be a
> > > generic option, that may be applicable to other drivers? Could the option
> > > be instead to prioritize FIPs compatibility?
> > >
> >
> > +1 to not have top level meson option for a driver.
> >
> > Will it work to give priority to intel-ipsec-mb lib, like when ipsec-mb
> > library exists use it, else use openssl?
> 
> Looking at the patch a bit more, is there a reason why this cannot be made
> a run-time option when both libraries are available?

Hi Bruce,

Thanks for the review.
Are you suggesting adding a EAL option to the QAT PMD?

Regards,
Fan
  
Fan Zhang May 19, 2022, 9:22 a.m. UTC | #7
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Wednesday, May 18, 2022 11:26 AM
> To: Richardson, Bruce <bruce.richardson@intel.com>; Ji, Kai
> <kai.ji@intel.com>
> Cc: dev@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: Re: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-mb
> lib
> 
> On 5/18/2022 9:04 AM, Bruce Richardson wrote:
> > On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
> >> Add in build option for QAT pmd use intel ipsec-mb lib
> >> instead openssl for precompute partial hash & aes.
> >>
> >> Signed-off-by: Kai Ji <kai.ji@intel.com>
> >> ---
> >>   drivers/common/qat/meson.build | 12 ++++++++++++
> >>   meson_options.txt              |  2 ++
> >>   2 files changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/common/qat/meson.build
> b/drivers/common/qat/meson.build
> >> index b7027f3164..d6eaff3e0e 100644
> >> --- a/drivers/common/qat/meson.build
> >> +++ b/drivers/common/qat/meson.build
> >> @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
> >>               'missing dependency, libcrypto')
> >>   endif
> >>
> >> +if get_option('qat_libipsecmb')
> >> +    IMB_required_ver = '1.0.0'
> >> +    libipsecmb = cc.find_library('IPSec_MB', required: false)
> >> +    if not lib.found()
> >> +        build = false
> >> +        reason = 'missing dependency, "libIPSec_MB"'
> >> +    else
> >> +        ext_deps += libipsecmb
> >> +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
> >> +    endif
> >> +endif
> >> +
> >
> > In general it's not a good idea for individual drivers to add top-level
> > meson.build options. If every driver does this for all its options we will
> > have an unmanageable set of hundreds of options. Unfortunately, though,
> I
> > don't see a really good way to replace this with something other than a
> > build option - I assume it's not just good enough to check which of the
> > dependencies is found/not-found? Is there some way that this can be a
> > generic option, that may be applicable to other drivers? Could the option
> > be instead to prioritize FIPs compatibility?
> >
> 
> +1 to not have top level meson option for a driver.
> 
> Will it work to give priority to intel-ipsec-mb lib, like when ipsec-mb
> library exists use it, else use openssl?

Hi Ferruh,

If both ipsec-mb and openssl3.0 presents in the system.
- Choose openssl 3.0 will result in deprecated API warnings.
- Choose ipsec-mb will ruin the FIPs compliance of QAT PMD.
That's why we think it shall be up to user to select which way to do small cipher/hash op.

Regards,
fan
  
Bruce Richardson May 19, 2022, 10:22 a.m. UTC | #8
On Thu, May 19, 2022 at 10:14:14AM +0100, Zhang, Roy Fan wrote:
> > -----Original Message-----
> > From: Richardson, Bruce <bruce.richardson@intel.com>
> > Sent: Wednesday, May 18, 2022 11:33 AM
> > To: Ferruh Yigit <ferruh.yigit@xilinx.com>
> > Cc: Ji, Kai <kai.ji@intel.com>; dev@dpdk.org; Zhang, Roy Fan
> > <roy.fan.zhang@intel.com>
> > Subject: Re: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-mb
> > lib
> >
> > On Wed, May 18, 2022 at 11:26:27AM +0100, Ferruh Yigit wrote:
> > > On 5/18/2022 9:04 AM, Bruce Richardson wrote:
> > > > On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
> > > > > Add in build option for QAT pmd use intel ipsec-mb lib
> > > > > instead openssl for precompute partial hash & aes.
> > > > >
> > > > > Signed-off-by: Kai Ji <kai.ji@intel.com>
> > > > > ---
> > > > >   drivers/common/qat/meson.build | 12 ++++++++++++
> > > > >   meson_options.txt              |  2 ++
> > > > >   2 files changed, 14 insertions(+)
> > > > >
> > > > > diff --git a/drivers/common/qat/meson.build
> > b/drivers/common/qat/meson.build
> > > > > index b7027f3164..d6eaff3e0e 100644
> > > > > --- a/drivers/common/qat/meson.build
> > > > > +++ b/drivers/common/qat/meson.build
> > > > > @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
> > > > >               'missing dependency, libcrypto')
> > > > >   endif
> > > > > +if get_option('qat_libipsecmb')
> > > > > +    IMB_required_ver = '1.0.0'
> > > > > +    libipsecmb = cc.find_library('IPSec_MB', required: false)
> > > > > +    if not lib.found()
> > > > > +        build = false
> > > > > +        reason = 'missing dependency, "libIPSec_MB"'
> > > > > +    else
> > > > > +        ext_deps += libipsecmb
> > > > > +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
> > > > > +    endif
> > > > > +endif
> > > > > +
> > > >
> > > > In general it's not a good idea for individual drivers to add top-level
> > > > meson.build options. If every driver does this for all its options we will
> > > > have an unmanageable set of hundreds of options. Unfortunately,
> > though, I
> > > > don't see a really good way to replace this with something other than a
> > > > build option - I assume it's not just good enough to check which of the
> > > > dependencies is found/not-found? Is there some way that this can be a
> > > > generic option, that may be applicable to other drivers? Could the option
> > > > be instead to prioritize FIPs compatibility?
> > > >
> > >
> > > +1 to not have top level meson option for a driver.
> > >
> > > Will it work to give priority to intel-ipsec-mb lib, like when ipsec-mb
> > > library exists use it, else use openssl?
> >
> > Looking at the patch a bit more, is there a reason why this cannot be made
> > a run-time option when both libraries are available?
> 
> Hi Bruce,
> 
> Thanks for the review.
> Are you suggesting adding a EAL option to the QAT PMD?
>
I was thinking you can add a driver option, to allow switching from the
default if both openssl and ipsec-mb were available to be compiled in.
  
Ferruh Yigit May 19, 2022, 11:15 a.m. UTC | #9
On 5/19/2022 10:22 AM, Zhang, Roy Fan wrote:

> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
>> Sent: Wednesday, May 18, 2022 11:26 AM
>> To: Richardson, Bruce <bruce.richardson@intel.com>; Ji, Kai
>> <kai.ji@intel.com>
>> Cc: dev@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>
>> Subject: Re: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-mb
>> lib
>>
>> On 5/18/2022 9:04 AM, Bruce Richardson wrote:
>>> On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
>>>> Add in build option for QAT pmd use intel ipsec-mb lib
>>>> instead openssl for precompute partial hash & aes.
>>>>
>>>> Signed-off-by: Kai Ji <kai.ji@intel.com>
>>>> ---
>>>>    drivers/common/qat/meson.build | 12 ++++++++++++
>>>>    meson_options.txt              |  2 ++
>>>>    2 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/common/qat/meson.build
>> b/drivers/common/qat/meson.build
>>>> index b7027f3164..d6eaff3e0e 100644
>>>> --- a/drivers/common/qat/meson.build
>>>> +++ b/drivers/common/qat/meson.build
>>>> @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
>>>>                'missing dependency, libcrypto')
>>>>    endif
>>>>
>>>> +if get_option('qat_libipsecmb')
>>>> +    IMB_required_ver = '1.0.0'
>>>> +    libipsecmb = cc.find_library('IPSec_MB', required: false)
>>>> +    if not lib.found()
>>>> +        build = false
>>>> +        reason = 'missing dependency, "libIPSec_MB"'
>>>> +    else
>>>> +        ext_deps += libipsecmb
>>>> +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
>>>> +    endif
>>>> +endif
>>>> +
>>>
>>> In general it's not a good idea for individual drivers to add top-level
>>> meson.build options. If every driver does this for all its options we will
>>> have an unmanageable set of hundreds of options. Unfortunately, though,
>> I
>>> don't see a really good way to replace this with something other than a
>>> build option - I assume it's not just good enough to check which of the
>>> dependencies is found/not-found? Is there some way that this can be a
>>> generic option, that may be applicable to other drivers? Could the option
>>> be instead to prioritize FIPs compatibility?
>>>
>>
>> +1 to not have top level meson option for a driver.
>>
>> Will it work to give priority to intel-ipsec-mb lib, like when ipsec-mb
>> library exists use it, else use openssl?
> 
> Hi Ferruh,
> 
> If both ipsec-mb and openssl3.0 presents in the system.
> - Choose openssl 3.0 will result in deprecated API warnings.
> - Choose ipsec-mb will ruin the FIPs compliance of QAT PMD.
> That's why we think it shall be up to user to select which way to do small cipher/hash op.
> 

Hi Fan, got it. So the runtime devarg option Bruce mentioned can be a 
solution, if it works.
  
Fan Zhang May 19, 2022, 12:25 p.m. UTC | #10
Hi Bruce,

> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Thursday, May 19, 2022 11:23 AM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Cc: Ferruh Yigit <ferruh.yigit@xilinx.com>; Ji, Kai <kai.ji@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-mb
> lib
> 
> On Thu, May 19, 2022 at 10:14:14AM +0100, Zhang, Roy Fan wrote:
> > > -----Original Message-----
> > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > Sent: Wednesday, May 18, 2022 11:33 AM
> > > To: Ferruh Yigit <ferruh.yigit@xilinx.com>
> > > Cc: Ji, Kai <kai.ji@intel.com>; dev@dpdk.org; Zhang, Roy Fan
> > > <roy.fan.zhang@intel.com>
> > > Subject: Re: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-
> mb
> > > lib
> > >
> > > On Wed, May 18, 2022 at 11:26:27AM +0100, Ferruh Yigit wrote:
> > > > On 5/18/2022 9:04 AM, Bruce Richardson wrote:
> > > > > On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
> > > > > > Add in build option for QAT pmd use intel ipsec-mb lib
> > > > > > instead openssl for precompute partial hash & aes.
> > > > > >
> > > > > > Signed-off-by: Kai Ji <kai.ji@intel.com>
> > > > > > ---
> > > > > >   drivers/common/qat/meson.build | 12 ++++++++++++
> > > > > >   meson_options.txt              |  2 ++
> > > > > >   2 files changed, 14 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/common/qat/meson.build
> > > b/drivers/common/qat/meson.build
> > > > > > index b7027f3164..d6eaff3e0e 100644
> > > > > > --- a/drivers/common/qat/meson.build
> > > > > > +++ b/drivers/common/qat/meson.build
> > > > > > @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
> > > > > >               'missing dependency, libcrypto')
> > > > > >   endif
> > > > > > +if get_option('qat_libipsecmb')
> > > > > > +    IMB_required_ver = '1.0.0'
> > > > > > +    libipsecmb = cc.find_library('IPSec_MB', required: false)
> > > > > > +    if not lib.found()
> > > > > > +        build = false
> > > > > > +        reason = 'missing dependency, "libIPSec_MB"'
> > > > > > +    else
> > > > > > +        ext_deps += libipsecmb
> > > > > > +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
> > > > > > +    endif
> > > > > > +endif
> > > > > > +
> > > > >
> > > > > In general it's not a good idea for individual drivers to add top-level
> > > > > meson.build options. If every driver does this for all its options we will
> > > > > have an unmanageable set of hundreds of options. Unfortunately,
> > > though, I
> > > > > don't see a really good way to replace this with something other than
> a
> > > > > build option - I assume it's not just good enough to check which of the
> > > > > dependencies is found/not-found? Is there some way that this can be
> a
> > > > > generic option, that may be applicable to other drivers? Could the
> option
> > > > > be instead to prioritize FIPs compatibility?
> > > > >
> > > >
> > > > +1 to not have top level meson option for a driver.
> > > >
> > > > Will it work to give priority to intel-ipsec-mb lib, like when ipsec-mb
> > > > library exists use it, else use openssl?
> > >
> > > Looking at the patch a bit more, is there a reason why this cannot be
> made
> > > a run-time option when both libraries are available?
> >
> > Hi Bruce,
> >
> > Thanks for the review.
> > Are you suggesting adding a EAL option to the QAT PMD?
> >
> I was thinking you can add a driver option, to allow switching from the
> default if both openssl and ipsec-mb were available to be compiled in.

We may have to use macros to filter out openssl deprecated API calls
otherwise the warning may still be there. Also what kind of driver option
are we talking about - any reference you can point to us? Much appreciated!

Regards,
Fan
  
Bruce Richardson May 19, 2022, 2:39 p.m. UTC | #11
On Thu, May 19, 2022 at 01:25:25PM +0100, Zhang, Roy Fan wrote:
> Hi Bruce,
> 
> > -----Original Message-----
> > From: Richardson, Bruce <bruce.richardson@intel.com>
> > Sent: Thursday, May 19, 2022 11:23 AM
> > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Cc: Ferruh Yigit <ferruh.yigit@xilinx.com>; Ji, Kai <kai.ji@intel.com>;
> > dev@dpdk.org
> > Subject: Re: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-mb
> > lib
> >
> > On Thu, May 19, 2022 at 10:14:14AM +0100, Zhang, Roy Fan wrote:
> > > > -----Original Message-----
> > > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > Sent: Wednesday, May 18, 2022 11:33 AM
> > > > To: Ferruh Yigit <ferruh.yigit@xilinx.com>
> > > > Cc: Ji, Kai <kai.ji@intel.com>; dev@dpdk.org; Zhang, Roy Fan
> > > > <roy.fan.zhang@intel.com>
> > > > Subject: Re: [dpdk-dev v2 1/2] build: add in option for qat use intel ipsec-
> > mb
> > > > lib
> > > >
> > > > On Wed, May 18, 2022 at 11:26:27AM +0100, Ferruh Yigit wrote:
> > > > > On 5/18/2022 9:04 AM, Bruce Richardson wrote:
> > > > > > On Tue, May 17, 2022 at 10:16:51PM +0800, Kai Ji wrote:
> > > > > > > Add in build option for QAT pmd use intel ipsec-mb lib
> > > > > > > instead openssl for precompute partial hash & aes.
> > > > > > >
> > > > > > > Signed-off-by: Kai Ji <kai.ji@intel.com>
> > > > > > > ---
> > > > > > >   drivers/common/qat/meson.build | 12 ++++++++++++
> > > > > > >   meson_options.txt              |  2 ++
> > > > > > >   2 files changed, 14 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/common/qat/meson.build
> > > > b/drivers/common/qat/meson.build
> > > > > > > index b7027f3164..d6eaff3e0e 100644
> > > > > > > --- a/drivers/common/qat/meson.build
> > > > > > > +++ b/drivers/common/qat/meson.build
> > > > > > > @@ -35,6 +35,18 @@ if qat_crypto and not libcrypto.found()
> > > > > > >               'missing dependency, libcrypto')
> > > > > > >   endif
> > > > > > > +if get_option('qat_libipsecmb')
> > > > > > > +    IMB_required_ver = '1.0.0'
> > > > > > > +    libipsecmb = cc.find_library('IPSec_MB', required: false)
> > > > > > > +    if not lib.found()
> > > > > > > +        build = false
> > > > > > > +        reason = 'missing dependency, "libIPSec_MB"'
> > > > > > > +    else
> > > > > > > +        ext_deps += libipsecmb
> > > > > > > +        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
> > > > > > > +    endif
> > > > > > > +endif
> > > > > > > +
> > > > > >
> > > > > > In general it's not a good idea for individual drivers to add top-level
> > > > > > meson.build options. If every driver does this for all its options we will
> > > > > > have an unmanageable set of hundreds of options. Unfortunately,
> > > > though, I
> > > > > > don't see a really good way to replace this with something other than
> > a
> > > > > > build option - I assume it's not just good enough to check which of the
> > > > > > dependencies is found/not-found? Is there some way that this can be
> > a
> > > > > > generic option, that may be applicable to other drivers? Could the
> > option
> > > > > > be instead to prioritize FIPs compatibility?
> > > > > >
> > > > >
> > > > > +1 to not have top level meson option for a driver.
> > > > >
> > > > > Will it work to give priority to intel-ipsec-mb lib, like when ipsec-mb
> > > > > library exists use it, else use openssl?
> > > >
> > > > Looking at the patch a bit more, is there a reason why this cannot be
> > made
> > > > a run-time option when both libraries are available?
> > >
> > > Hi Bruce,
> > >
> > > Thanks for the review.
> > > Are you suggesting adding a EAL option to the QAT PMD?
> > >
> > I was thinking you can add a driver option, to allow switching from the
> > default if both openssl and ipsec-mb were available to be compiled in.
> 
> We may have to use macros to filter out openssl deprecated API calls
> otherwise the warning may still be there. Also what kind of driver option
> are we talking about - any reference you can point to us? Much appreciated!
> 

Many drivers support additional parameters, but the vdev's are the most
common case. For example, AF_XDP:

http://git.dpdk.org/dpdk/tree/drivers/net/af_xdp/rte_eth_af_xdp.c#n2056

or for a physical PCI devices, i40e:

http://git.dpdk.org/dpdk/tree/drivers/net/i40e/i40e_ethdev.c#n12133

/Bruce
  

Patch

diff --git a/drivers/common/qat/meson.build b/drivers/common/qat/meson.build
index b7027f3164..d6eaff3e0e 100644
--- a/drivers/common/qat/meson.build
+++ b/drivers/common/qat/meson.build
@@ -35,6 +35,18 @@  if qat_crypto and not libcrypto.found()
             'missing dependency, libcrypto')
 endif
 
+if get_option('qat_libipsecmb')
+    IMB_required_ver = '1.0.0'
+    libipsecmb = cc.find_library('IPSec_MB', required: false)
+    if not lib.found()
+        build = false
+        reason = 'missing dependency, "libIPSec_MB"'
+    else
+        ext_deps += libipsecmb
+        dpdk_conf.set('RTE_QAT_LIBIPSECMB', true)
+    endif
+endif
+
 # The driver should not build if both compression and crypto are disabled
 #FIXME common code depends on compression files so check only compress!
 if not qat_compress # and not qat_crypto
diff --git a/meson_options.txt b/meson_options.txt
index 7c220ad68d..e48e7dd966 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -42,6 +42,8 @@  option('mbuf_refcnt_atomic', type: 'boolean', value: true, description:
        'Atomically access the mbuf refcnt.')
 option('platform', type: 'string', value: 'native', description:
        'Platform to build, either "native", "generic" or a SoC. Please refer to the Linux build guide for more information.')
+option('qat_libipsecmb', type: 'boolean', value: false, description:
+       'use ipsec mb lib instead openssl for QAT pmd')
 option('enable_trace_fp', type: 'boolean', value: false, description:
        'enable fast path trace points.')
 option('tests', type: 'boolean', value: true, description: