[ovs-dev] [ovs-dev, dpdk-hwol, v1] netdev-dpdk: support port representors

[Ilya Maximets]

On 17.12.2018 12:49, Thomas Monjalon wrote:
> 16/12/2018 13:02, Ophir Munk:
>> Hi Ilya,
>> Please find comments inline.
>>
>> From: Ilya Maximets <i.maximets at samsung.com>
>>>
>>> Not a full review. Just one comment inline.
>>>
>>> Best regards, Ilya Maximets.
>>>
>>> On 12.12.2018 2:34, Ophir Munk wrote:
>>>> Dpdk port representors were introduced in dpdk 18.xx.
>>>
>> .....
>>>>
>>>> -    rte_eth_dev_close(port_id);
>>>> +    rte_dev = rte_eth_devices[port_id].device;
>>>
>>>
>>> We should not use 'rte_eth_devices' directly because it's internal to DPDK.
>>> See the discussion here:
>>>  http://mails.dpdk.org/archives/dev/2018-November/119198.html
>>
>> The discussions mentioned in the link raised pros and cons for using a direct access to rte_eth_devices[] but was not conclusive.
>> I am in favor of keeping the direct access to rte_eth_devices array for the following reasons:
>> 1. Using rte_eth_dev_info_get() API uses the same pointer to the internal rte_eth_devices[] array. So actually it is as dangerous as accessing the array directly.
>> 2. Theoretically there is logic in using the API as it may have a safer implementation in the future. However, I talked with Thomans Monjalon - the maintainer of this API - and he recommended using the direct array access. Thomas mentioned that this API will be dropped. 
>>
>> Thomas - can you please share your view on this topic?
> 
> I agree with the idea of avoiding direct access to this structure.
> That's why I wrote a new function specifically for this use case:
> 	https://patches.dpdk.org/patch/48429/
> 
> About workarounding the direct access with rte_eth_dev_info_get(),
> I am not sure it is a great idea.
> In my opinion, it is best to keep the direct access to the array
> and add a comment about the need to replace it with a proper API.

OK. Thank you, Thomas. That sounds reasonable.
Ophir, please, add a "FIXME" comment that describes the situation.

Best regards, Ilya Maximets.