DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	Ilya Maximets <i.maximets@samsung.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"ovs-dev@openvswitch.org" <ovs-dev@openvswitch.org>,
	"Stokes, Ian" <ian.stokes@intel.com>,
	Kevin Traynor <ktraynor@redhat.com>,
	Ophir Munk <ophirmu@mellanox.com>,
	Shahaf Shuler <shahafs@mellanox.com>,
	Eelco Chaudron <echaudro@redhat.com>,
	"arybchenko@solarflare.com" <arybchenko@solarflare.com>
Subject: Re: [dpdk-dev] Direct using of 'rte_eth_devices' in DPDK apps.
Date: Fri, 16 Nov 2018 09:51:15 +0000	[thread overview]
Message-ID: <2601191342CEEE43887BDE71AB977258010CEB8C73@IRSMSX106.ger.corp.intel.com> (raw)
In-Reply-To: <11426330.8DnILqFDIn@xps>

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

> 
> > 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?
> 
> 

  reply	other threads:[~2018-11-16  9:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20181116084233eucas1p2ae806fd36b2fa1ea77d1a450facb0922@eucas1p2.samsung.com>
2018-11-16  8:42 ` Ilya Maximets
2018-11-16  9:29   ` Thomas Monjalon
2018-11-16  9:51     ` Ananyev, Konstantin [this message]
2018-11-16 14:16       ` Wiles, Keith
2018-11-16 14:19       ` Wiles, Keith
2018-11-16  9:30   ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2601191342CEEE43887BDE71AB977258010CEB8C73@IRSMSX106.ger.corp.intel.com \
    --to=konstantin.ananyev@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=echaudro@redhat.com \
    --cc=ferruh.yigit@intel.com \
    --cc=i.maximets@samsung.com \
    --cc=ian.stokes@intel.com \
    --cc=ktraynor@redhat.com \
    --cc=ophirmu@mellanox.com \
    --cc=ovs-dev@openvswitch.org \
    --cc=shahafs@mellanox.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).