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

Joseph, Anoob Anoob.Joseph at caviumnetworks.com
Fri Sep 28 13:14:20 CEST 2018


Hi Fiona,

Did you get a chance to look at this?

Thanks,
Anoob
On 24-09-2018 17:06, Joseph, Anoob wrote:
> 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
>>
>



More information about the dev mailing list