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

Thomas Monjalon thomas at monjalon.net
Mon Dec 17 09:49:18 UTC 2018


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.





More information about the dev mailing list