Re: [dpdk-dev] Direct using of 'rte_eth_devices' in DPDK apps.

["Wiles, Keith" <keith.wiles@intel.com>]

> On Nov 16, 2018, at 3:51 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> 
> Hi everyone,
> 
>> 
>> Hi,
>> 
>> 16/11/2018 09:42, Ilya Maximets:
>>> Hi,
>>> While discussing the ways to enable DPDK 18.11 new features in OVS
>>> there was suggestions to use 'rte_eth_devices[]' array directly.
>>> But this array is marked as '@internal' and also it located in
>>> the internal header 'lib/librte_ethdev/rte_ethdev_core.h' with the
>>> following disclaimer:
>>> 
>>> /**
>>> * @file
>>> *
>>> * RTE Ethernet Device internal header.
>>> *
>>> * This header contains internal data types. But they are still part of the
>>> * public API because they are used by inline functions in the published API.
>>> *
>>> * Applications should not use these directly.
>>> *
>>> */
>>> 
>>> From the other hand, test-pmd and some example apps in DPDK source
>>> tree are using this array for various reasons.
>>> 
>>> So, is it OK to use this array directly or not?
>> 
>> Good question :)
>> Thanks for bringing this discussion.
>> 
>> As you said, it is public because of inline functions using it directly
>> for performance purpose. The DPDK API is bad for separating public and
>> internal stuff. And over time, there is not a lot of attention on trying
>> to not use internal symbols in applications.
>> 
>>> In general we need to change the API, i.e. make 'rte_eth_devices' part
>>> of a public API. Or change the test-pmd and example apps to stop
>>> using it.
>> 
>> I agree we need to decide an option and make it clear.
>> 
>> We can try to make this variable private and add more public functions
>> to use it (I'm thinking at more iterators like sibling ones).
>> It would clarify the API.
>> It can be evaluated what is the real cost after compiler optimization
>> for Rx/Tx functions. It can also be evaluated to uninline functions.
>> 
>> On the other hand, we can wonder what is the real benefit of trying to
>> hide access to internal resources. Should we make all public?
> 
> In that case every change in any of such structures will be an ABI breakage.
> Even now any change in rte_eth_dev is quite problematic because of that.
> I think we better keep them private as much as possible and cleanup
> our examples and testpmd code.
> Konstantin

I Agree here, I have noticed a few places we allow direct access to internal data structures, which we need to restrict access by making them private with getter/setter functions or just getter/setter functions even if we can not make them private. At least with moving members and adding members we can state that it is not a ABI breakage as long as everyone uses the getter/setter functions. These functions could not be inline functions correct as that would still break API?

> 
>> 
>>> One more related question: Is it OK to access internal device
>>> stuff using 'device' pointer obtained by 'rte_eth_dev_info'?
>>> This looks really dangerous. It's unclear why pointers like this
>>> exposed to user.
>> 
>> It's a lot easier to expose pointers than doing a good API for all uses.
>> We need to question what is really dangerous and what we want to avoid?

Regards,
Keith