Re: reg. https://patches.dpdk.org/project/dpdk/list/?series=31200 - patch result

[Aaron Conole <aconole@redhat.com>]

"Power, Ciara" <ciara.power@intel.com> writes:

> Hi Aaron,
>
>> -----Original Message-----
>> From: Aaron Conole <aconole@redhat.com>
>> Sent: Wednesday, March 6, 2024 2:57 PM
>> To: Power, Ciara <ciara.power@intel.com>
>> Cc: Sivaramakrishnan, VenkatX <venkatx.sivaramakrishnan@intel.com>; Akhil
>> Goyal <gakhil@marvell.com>; Ji, Kai <kai.ji@intel.com>; probb@iol.unh.edu;
>> dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> Thomas Monjalon <thomas@monjalon.net>; David Marchand
>> <dmarchan@redhat.com>; Kevin Traynor <ktraynor@redhat.com>
>> Subject: Re: reg. https://patches.dpdk.org/project/dpdk/list/?series=31200 -
>> patch result
>> 
>> "Power, Ciara" <ciara.power@intel.com> writes:
>> 
>> > Hi Aaron,
>> >
>> >> -----Original Message-----
>> >> From: Aaron Conole <aconole@redhat.com>
>> >> Sent: Tuesday, March 5, 2024 6:37 PM
>> >> To: Power, Ciara <ciara.power@intel.com>
>> >> Cc: Sivaramakrishnan, VenkatX <venkatx.sivaramakrishnan@intel.com>;
>> >> Akhil Goyal <gakhil@marvell.com>; Ji, Kai <kai.ji@intel.com>;
>> >> probb@iol.unh.edu; dev@dpdk.org
>> >> Subject: Re: reg.
>> >> https://patches.dpdk.org/project/dpdk/list/?series=31200 - patch
>> >> result
>> >>
>> >> "Power, Ciara" <ciara.power@intel.com> writes:
>> >>
>> >> > + Patrick
>> >> >
>> >> >
>> >> >
>> >> > From: Power, Ciara
>> >> > Sent: Tuesday, March 5, 2024 10:05 AM
>> >> > To: Sivaramakrishnan, VenkatX <venkatx.sivaramakrishnan@intel.com>;
>> >> > Akhil Goyal <gakhil@marvell.com>
>> >> > Cc: Ji, Kai <kai.ji@intel.com>; Aaron Conole <aconole@redhat.com>
>> >> > Subject: RE: reg.
>> >> > https://patches.dpdk.org/project/dpdk/list/?series=31200 - patch
>> >> > result
>> >> >
>> >> >
>> >> >
>> >> > Hi folks,
>> >> >
>> >> >
>> >> >
>> >> > Had a quick look, I can also see this:
>> >> >
>> >> > crypto/ipsec_mb: IPSec_MB version >= 1.4.0 is required, found
>> >> > version
>> >> > 1.2.0
>> >>
>> >> This version of ipsec_mb is less than 1 year old.  Did this pass any
>> >> other CI testing?  I would be surprised if it did - I'm not sure any
>> >> downstream environments that would be using it already.
>> >
>> > We have been using 1.4 (and even 1.5 since it was released) for internal
>> regression testing and development.
>> > Other than that, the library would be tested by Intel-ipsec-mb team directly.
>> > 1.4 has been supported by the ipsec-mb SW PMDs since it was released,
>> > but now we would like to make it the required version, to remove the various
>> ifdef codepaths in PMD, and use the newer, more performant version of the
>> library.
>> 
>> While that is a good goal, this patch series would cause build issues on some
>> distributions (which is evident from the CI failures), and that there are new
>> requirements isn't as clearly documented.
>
> Yes, the CI failures need to be resolved, some questions/discussion on
> that in the other thread.
>
> Usually, for a version bump we would have documented in PMD
> documentation, and the release notes.
> We did similar before when moving from 0.53 to 1.0 as part of this
> work previously for 21.11:
> https://github.com/DPDK/dpdk/commit/c75542ae42000062b55cb03643575cd13b66aeaf
> https://github.com/DPDK/dpdk/commit/918fd2f1466b0e3b21a033df7012a77a83665582
>
>
>> 
>> AFAICT, it also shifts the requirements from either OpenSSL or IPSec-MB to
>> IPSec-MB.  Did I understand it correctly?
>
> No, bumping the ipsec-mb version from 1.1 to 1.4 for the ipsec-mb SW
> PMDs doesn't involve any Openssl dependency changes - these SW PMDs
> have only ever used the ipsec-mb library.
> This would affect snow3g, Kasumi, zuc, aesni_mb, aesni_gcm and
> chachapoly PMDs - they would need v1.4 to be compiled.

Ahh, okay - thanks that makes sense to me.  I guess one issue is that
the current github robot will be based on 22.04, which is only shipping
with ipsec-mb 1.2.  I know that 23.10 includes ipsec-mb 1.4, but this is
not an LTS so we likely wouldn't support it.  24.04 (the next LTS to
release) will use 1.5, so when we move it shouldn't be an issue any
longer.

Perhaps we can look for a PPA or even build from scratch during the
build the ipsec-mb to link against.  This will at least give us the
right coverage.  I did a quick glance, but didn't see that there is any
kind of 'jammy' (the 22.04 code name) ppa which covers this version, but
maybe the approach is to set one up and then use it until we move to
24.04 in the future.

WDYT?

>
>> 
>> >>
>> >> > I guess the installed PMD .so file isn’t created because they are
>> >> > not compiled in, due to the minimum version on environment not
>> >> > meeting the
>> >> new requirements.
>> >>
>> >> I don't see any such new requirements anywhere on the crypto tree.
>> >> The only change I know about was for QAT to try and default to
>> >> IPSec_MB 1.4, but it is supposed to fall back to OpenSSL if that is unavailable.
>> Did this change?
>> >
>> > This patchset introduces the requirement, it is not yet on the crypto tree.
>> > It is a SW PMD change only - currently they require 1.1 ipsec-mb, but we want
>> to bump that to 1.4.
>> > QAT dependencies are unchanged.
>> 
>> In that case I think there needs to be some additional communications /
>> announcements since it is changing a dependency.
>
> Sure, we can look to do that if needed.
> Apart from the release notes + PMD doc changes, what other comms/announcements
> do you suggest we do to bump the ipsec-mb PMD version from 1.0 to 1.4?
>
>  
> Thanks,
> Ciara