[dpdk-dev] [PATCH v4 2/9] lib/librte_power: add extra msg type for policies

[Hunt, David]

On 5/10/2017 10:21 AM, santosh wrote:
> Hi David,
>
>
> On Thursday 05 October 2017 02:08 PM, Hunt, David wrote:
>> Hi Santosh,
>>
>> On 4/10/2017 4:36 PM, santosh wrote:
>>> Hi David,
>>>
>>>
>>> On Wednesday 04 October 2017 02:45 PM, David Hunt wrote:
>>>> Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic at intel.com>
>>>> Signed-off-by: Rory Sexton <rory.sexton at intel.com>
>>>> Signed-off-by: David Hunt <david.hunt at intel.com>
>>>> ---
>>> my 2cent:
>>> General comment on implementation approach:
>>> IMO, we should avoid PMD details in common lib area.
>>> example: file channel_commons.h has ifdef clutter referencing
>>> i40e pmds all over.
>>>
>>> Perhaps we should introduce opaque handle example void * or introduce pmd
>>> specific callback/handle which points to PMD specific metadata in power library.
>>>
>>> Example:
>>> struct channel_packet {
>>>     void *pmd_specific_metadata;
>>> }
>>>
>>> Or someway via callback (I'm not sure at the moment)
>>> so that we could hide PMD details in common area.
>>>
>>> Thanks.
>> I would agree that PMD specific details are good left to the PMDs, however I think that the initial
>> example should be OK as is, and as new PMDs are added, we can find commonality between them
>> which stays in the example, and any really specific stuff can be pushed back behind an opaque.
>>
>> What about the v5 I submitted (without the #ifdef's)? Are you OK with that for this release, and we can
>> fine tune as other PMDS are added in future releases?
>>
> Yes. But in future releases, we should do more code clean up in power lib and example area..
> meaning; current example implementation uses names like _vsi.. specific to intel NICs,
> we should remove such naming and their dependency code from example area.
>
> Thanks.

I agree. I plan to clean up the API in the next release of DPDK. For 
exmaple, there are private header files that are called rte_*.h that 
expose private functions to the documentation. These need to be renamed, 
as well as moving some structures around. I can also look at re-naming 
some of the vsi vars to something more generic.
Thanks,
Dave.