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.jos...@cavium.com] > Sent: Tuesday, October 16, 2018 10:41 PM > To: Thomas Monjalon <tho...@monjalon.net>; Trahe, Fiona > <fiona.tr...@intel.com> > Cc: dev@dpdk.org; Akhil Goyal <akhil.go...@nxp.com>; De Lara Guarch, Pablo > <pablo.de.lara.gua...@intel.com>; Murthy, Nidadavolu > <nidadavolu.mur...@cavium.com>; Jacob, Jerin > <jerin.jacobkollanukka...@cavium.com>; Athreya, Narayana Prasad > <narayanaprasad.athr...@cavium.com>; Dwivedi, Ankur > <ankur.dwiv...@cavium.com>; Dabilpuram, > Nithin <nithin.dabilpu...@cavium.com>; Jayaraman, Ragothaman > <ragothaman.jayara...@cavium.com>; Srinivasan, Srisivasubramanian > <srisivasubramanian.sriniva...@cavium.com>; Tejasree, Kondoj > <kondoj.tejas...@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 <tho...@monjalon.net>; Trahe, Fiona > > <fiona.tr...@intel.com> > > Cc: dev@dpdk.org; Akhil Goyal <akhil.go...@nxp.com>; Joseph, Anoob > > <anoob.jos...@cavium.com>; De Lara Guarch, Pablo > > <pablo.de.lara.gua...@intel.com>; Murthy, Nidadavolu > > <nidadavolu.mur...@cavium.com>; Jacob, Jerin > > <jerin.jacobkollanukka...@cavium.com>; Athreya, Narayana Prasad > > <narayanaprasad.athr...@cavium.com>; Dwivedi, Ankur > > <ankur.dwiv...@cavium.com>; Dabilpuram, Nithin > > <nithin.dabilpu...@cavium.com>; Jayaraman, Ragothaman > > <ragothaman.jayara...@cavium.com>; Srinivasan, Srisivasubramanian > > <srisivasubramanian.sriniva...@cavium.com>; Tejasree, Kondoj > > <kondoj.tejas...@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:tho...@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. > > > > > > > > >