Hi Fiona, Reminder!!
Thanks, Anoob > -----Original Message----- > From: Joseph, Anoob > Sent: 10 October 2018 11:10 > To: Thomas Monjalon <[email protected]>; Trahe, Fiona > <[email protected]> > Cc: [email protected]; Akhil Goyal <[email protected]>; Joseph, Anoob > <[email protected]>; De Lara Guarch, Pablo > <[email protected]>; Murthy, Nidadavolu > <[email protected]>; Jacob, Jerin > <[email protected]>; Athreya, Narayana Prasad > <[email protected]>; Dwivedi, Ankur > <[email protected]>; Dabilpuram, Nithin > <[email protected]>; Jayaraman, Ragothaman > <[email protected]>; Srinivasan, Srisivasubramanian > <[email protected]>; Tejasree, Kondoj > <[email protected]> > 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:[email protected]] > >>> 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. > > > > > >
