Re: [dpdk-dev] [PATCH] test/crypto: change cipher offset for esn vector

[Anoob Joseph <anoobj@marvell.com>]

Hi Akhil, 

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Wednesday, July 8, 2020 12:51 AM
> To: Ankur Dwivedi <adwivedi@marvell.com>; dev@dpdk.org
> Cc: declan.doherty@intel.com; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Vakul Garg <vakul.garg@nxp.com>; Anoob
> Joseph <anoobj@marvell.com>
> Subject: [EXT] RE: [PATCH] test/crypto: change cipher offset for esn vector
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> > >> >Why do we need this change?
> > >> >The existing test case is to demonstrate a generic case where we
> > >> >can have an auth only trailer as well. It is similar to a case of
> > >> >IPSEC ESN but not exactly IPSec. Cipher offset can be anything as
> > >> >per the app
> > >requirement.
> > >> >I don't think there is anything wrong in the vector. It should
> > >> >pass in every hardware without any issue.
> > >> [Ankur] It's a limitation in OCTEON TX PMDs that the (encr offset -
> > >> auth offset) should be 8 bytes aligned.
> > >> In the IPSEC ESN scenario generally the offsets will be such.
> > >> But in the above IPSEC ESN test vector, this requirement is not met
> > >> and hence the associated test cases fails on the PMD.
> > >
> > >In that case, I think it is better to have a separate test vector and
> > >both should be executed. With the previous one as not supported in
> > >your case and this one will be supported.
> > [Ankur] The offsets values are present per crypto operation. So to
> > make these tests as unsupported the pmd datapath needs to be changed.
> > Is there an alternative to make these tests unsupported?
> 
> I believe this is a data path error and a limitation in your PMD.
> You can not stop the application writer from using unaligned cipher
> offsets(non-8 byte aligned)

[Anoob] Yes. But the typical case with IPsec is what is supported in the PMD. 

> 
> This is just a test application, which may hide your PMD limitation by
> accepting this patch But in actual the scenario will fail when some user
> configures a 12B cipher offset.

[Anoob] Agreed. But autotest having a failure is not an ideal situation to be it. Especially when it's not the typical usage. Can I propose to add a field like "Know Issues:" in the summary field? We can add a check for OCTEONTX PMDs in the test case and mark it as a known case. I do understand that the vision was to remove all driver specific tests and have generic tests for all PMDs, but here we are left with no other option. Chances are, other PMDs also could have similar limitations when moving to generic framework.

If you have suggestions to skip this test in any other way, that would also work for us.
 
> IMO, you should add a new test vector instead of replacing this one and it
> should Be OK to have the existing one fail in your case.
>