[dpdk-dev] [PATCH v2 09/33] crypto/octeontx: adds symmetric capabilities

Joseph, Anoob Anoob.Joseph at cavium.com
Mon Oct 22 05:49:01 CEST 2018


Hi Fiona,

I do agree that your solution seems to be a neat way for organizing capabilities. But Akhil & Thomas were against that idea and we had to come up with one array with all capabilities. This would not scale well when we start supporting devices with varying capabilities.

If your plan is to follow the same approach for asym support, maybe we will also follow suit and submit the required patches.

@Akhil, Thomas, thoughts?

Thanks,
Anoob

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe at intel.com>
> Sent: 20 October 2018 02:40
> To: Joseph, Anoob <Anoob.Joseph at cavium.com>; Thomas Monjalon
> <thomas at monjalon.net>
> Cc: dev at dpdk.org; Akhil Goyal <akhil.goyal at nxp.com>; De Lara Guarch,
> Pablo <pablo.de.lara.guarch at intel.com>; Murthy, Nidadavolu
> <Nidadavolu.Murthy at cavium.com>; Jacob, Jerin
> <Jerin.JacobKollanukkaran at cavium.com>; Athreya, Narayana Prasad
> <NarayanaPrasad.Athreya at cavium.com>; Dwivedi, Ankur
> <Ankur.Dwivedi at cavium.com>; Dabilpuram, Nithin
> <Nithin.Dabilpuram at cavium.com>; Jayaraman, Ragothaman
> <Ragothaman.Jayaraman at cavium.com>; Srinivasan, Srisivasubramanian
> <Srisivasubramanian.Srinivasan at cavium.com>; Tejasree, Kondoj
> <Kondoj.Tejasree at cavium.com>; Trahe, Fiona <fiona.trahe at intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 09/33] crypto/octeontx: adds symmetric
> capabilities
> 
> External Email
> 
> Hi Anoob,
> 
> Sorry for the delay, I've been travelling a lot lately.
> We don't have an alternative solution - will have to explore options when we
> get to that stage of the asym PMD development.
> I think the macro works well for this specific case, however we'll look for an
> alternative. At the moment we don't have bandwidth to investigate further.
> If you come up with a neat solution we'll be happy to follow it.
> 
> Fiona
> 
> > -----Original Message-----
> > From: Joseph, Anoob [mailto:Anoob.Joseph at cavium.com]
> > Sent: Tuesday, October 16, 2018 10:41 PM
> > To: Thomas Monjalon <thomas at monjalon.net>; Trahe, Fiona
> > <fiona.trahe at intel.com>
> > Cc: dev at dpdk.org; Akhil Goyal <akhil.goyal at nxp.com>; De Lara Guarch,
> > Pablo <pablo.de.lara.guarch at intel.com>; Murthy, Nidadavolu
> > <Nidadavolu.Murthy at cavium.com>; Jacob, Jerin
> > <Jerin.JacobKollanukkaran at cavium.com>; Athreya, Narayana Prasad
> > <NarayanaPrasad.Athreya at cavium.com>; Dwivedi, Ankur
> > <Ankur.Dwivedi at cavium.com>; Dabilpuram, Nithin
> > <Nithin.Dabilpuram at cavium.com>; Jayaraman, Ragothaman
> > <Ragothaman.Jayaraman at cavium.com>; Srinivasan, Srisivasubramanian
> > <Srisivasubramanian.Srinivasan at cavium.com>; Tejasree, Kondoj
> > <Kondoj.Tejasree at cavium.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 09/33] crypto/octeontx: adds
> > symmetric capabilities
> >
> > Hi Fiona,
> >
> > Reminder!!
> >
> > Thanks,
> > Anoob
> >
> > > -----Original Message-----
> > > From: Joseph, Anoob
> > > Sent: 10 October 2018 11:10
> > > To: Thomas Monjalon <thomas at monjalon.net>; Trahe, Fiona
> > > <fiona.trahe at intel.com>
> > > Cc: dev at dpdk.org; Akhil Goyal <akhil.goyal at nxp.com>; Joseph, Anoob
> > > <Anoob.Joseph at cavium.com>; De Lara Guarch, Pablo
> > > <pablo.de.lara.guarch at intel.com>; Murthy, Nidadavolu
> > > <Nidadavolu.Murthy at cavium.com>; Jacob, Jerin
> > > <Jerin.JacobKollanukkaran at cavium.com>; Athreya, Narayana Prasad
> > > <NarayanaPrasad.Athreya at cavium.com>; Dwivedi, Ankur
> > > <Ankur.Dwivedi at cavium.com>; Dabilpuram, Nithin
> > > <Nithin.Dabilpuram at cavium.com>; Jayaraman, Ragothaman
> > > <Ragothaman.Jayaraman at cavium.com>; Srinivasan, Srisivasubramanian
> > > <Srisivasubramanian.Srinivasan at cavium.com>; Tejasree, Kondoj
> > > <Kondoj.Tejasree at cavium.com>
> > > Subject: Re: [dpdk-dev] [PATCH v2 09/33] crypto/octeontx: adds
> > > symmetric capabilities
> > >
> > > Hi Fiona,
> > >
> > > We were following the QAT approach for defining the capabilities.
> > > OCTEON TX crypto PMD has similar number of capabilities and QAT was
> > > the close model that we could follow. I can see the advantages of
> > > the macro approach, but that would give a checkpatch warning. Also,
> > > Thomas didn't really like the idea of having long macros. So we have fixed
> it in the upstream code.
> > >
> > > I would like to understand what would be your approach when you add
> > > asymmetric support. We are also adding asymmetric support and would
> > > like to understand how you would be adding, while supporting devices
> > > with varying capability.
> > >
> > > Thanks,
> > > Anoob
> > > On 09-10-2018 01:57, Thomas Monjalon wrote:
> > > > External Email
> > > >
> > > > 08/10/2018 17:59, Trahe, Fiona:
> > > >> Hi Akhil, Joseph, Thomas,
> > > >> Just spotted this now.
> > > >> See below.
> > > >>
> > > >> From: Thomas Monjalon [mailto:thomas at monjalon.net]
> > > >>> 24/09/2018 13:36, Joseph, Anoob:
> > > >>>> Hi Fiona,
> > > >>>>
> > > >>>> Can you please comment on this?
> > > >>>>
> > > >>>> We are adding all capabilities of octeontx-crypto PMD as a
> > > >>>> macro in otx_cryptodev_capabilites.h file and then we are using
> > > >>>> it from otx_cryptodev_ops.c. This is the approach followed by
> > > >>>> QAT crypto PMD. As per my understanding, this is to ensure that
> > > >>>> cryptodev_ops file remains simple. For other PMDs with fewer
> > > >>>> number of capabilities, the structure can be populated in the
> > > >>>> .c file itself without the size of the file coming into the picture.
> > > >>>>
> > > >>>> But this would cause checkpatch to report error. Akhil's
> > > >>>> suggestion is to move the entire definition to a header and
> > > >>>> include it from the .c file. I believe, the QAT approach was to
> > > >>>> avoid variable definition in the header. What do you think
> > > >>>> would be a better approach
> > > here?
> > > >>> I think we should avoid adding some code in a .h file.
> > > >>> And it is even worst when using macros.
> > > >>>
> > > >>> I suggest defining the capabilities in a .c file.
> > > >>> If you don't want to bloat the main .c file, you can create a
> > > >>> function defined in another .c file.
> > > >>>
> > > >> I can't remember all the variations we tried, but there were a few.
> > > >> I think the macro works well in this case.
> > > >> What is the issue we need to solve?
> > > > It is a discussion about best practice.
> > > > My answer is: avoid long macros and avoid instructions in .h file.
> > > >
> > > >
> > > >



More information about the dev mailing list