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

[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?

Thanks,
Anoob
On 17-09-2018 18:05, Joseph, Anoob wrote:
> Hi Akhil,
>
> On 17-09-2018 17:31, Akhil Goyal wrote:
>> External Email
>>
>>> diff --git a/drivers/crypto/octeontx/otx_cryptodev_ops.c 
>>> b/drivers/crypto/octeontx/otx_cryptodev_ops.c
>>> index d25f9c1..cc0030e 100644
>>> --- a/drivers/crypto/octeontx/otx_cryptodev_ops.c
>>> +++ b/drivers/crypto/octeontx/otx_cryptodev_ops.c
>>> @@ -10,9 +10,15 @@
>>>   #include "cpt_pmd_logs.h"
>>>
>>>   #include "otx_cryptodev.h"
>>> +#include "otx_cryptodev_capabilities.h"
>>>   #include "otx_cryptodev_hw_access.h"
>>>   #include "otx_cryptodev_ops.h"
>>>
>>> +static const struct rte_cryptodev_capabilities otx_capabilities[] = {
>>> +     OTX_SYM_CAPABILITIES,
>>> +     RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
>>> +};
>>> +
>>
>> better to have otx_capabilities structure defined in the 
>> otx_cryptodev_capabilities.h
>>
>> I don't see any value addition of creating a macro in one file using 
>> in a separate structure in another file
>>
>> which doesn't have anything new in that structure. It would also give 
>> checkpatch error.
>>
>> You can directly have a capability structure without the #define.
> This was the convention followed in qat driver.
>
> https://git.dpdk.org/dpdk/tree/drivers/crypto/qat/qat_sym_capabilities.h
>
> I guess it was to avoid variable definition in header. May be Pablo 
> too can comment on this. I'll make the change accordingly.
>
> Thanks,
> Anoob
>