> -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ray Kinsella > Sent: Monday, September 23, 2019 6:19 PM > To: dpdk-dev > Cc: Richardson, Bruce; Jerin Jacob Kollanukkaran; Hemant Agrawal; > Thomas Monjalon; Stephen Hemminger; Yigit, Ferruh; Ananyev, Konstantin; > maxime.coque...@redhat.com; David Marchand; Marcin Zapolski > Subject: [dpdk-dev] RFC: hiding struct rte_eth_dev > > Hi folks, > > The ABI Stability proposals should be pretty well known at this point. > The latest rev is here ... > > http://inbox.dpdk.org/dev/1565864619-17206-1-git-send-email- > m...@ashroe.eu/ > > As has been discussed public data structure's are risky for ABI > stability, as any changes to a data structure can change the ABI. As a > general rule you want to expose as few as possible (ideally none), and > keep them as small as possible. > > One of the key data structures in DPDK is `struct rte_eth_dev`. In this > case, rte_eth_dev is exposed public-ally, as a side-effect of the > inlining of the [rx,tx]_burst functions. > > Marcin Zapolski has been looking at what to do about it, with no > current > consensus on a path forward. The options on our table is:- > > 1. Do nothing, live with the risk to DPDK v20 ABI stability. > > 2. Pad rte_eth_dev, add some extra bytes to the structure "in case" we > need to add a field during the v20 ABI (through to 20.11). > > 3. Break rte_eth_dev into public and private structs. > - See > http://inbox.dpdk.org/dev/20190906131813.1343-1- > marcinx.a.zapol...@intel.com/ > - This ends up quiet an invasive patch, late in the cycle, however it > does have no performance penalty. > > 4. Uninline [rx,tx]_burst functions > - See > http://inbox.dpdk.org/dev/20190730124950.1293-1- > marcinx.a.zapol...@intel.com/ > - This has a performance penalty of ~2% with testpmd, impact on a > "real > workload" is likely to be in the noise. > > We need to agree an approach for v19.11, and that may be we agree to do > nothing. My personal vote is 4. as the simplest with minimal impact. > > Thanks, > > Ray K
First of all, consider why an application would need to access the rte_eth_dev structure directly. I don't see why. So I assume that the only issue with hiding it behind an opaque handle is its use in the [rx,tx]_burst functions. Next, let's consider the impact of uninlining the [rx,tx]_burst functions. An actual application would have to be extremely simple for the impact to be anywhere near the 2 % seen with testpmd. Thus I vote for #4. #3 is even better than #4, assuming performance *of core functionality* trumps general readability and maintainability. And it probably doesn't affect readability and maintainability for DPDK users/consumers. So I also vote for #3. I think the decision about uninlining core functions should be a decision about the general principle, which should be discussed thoroughly, perhaps using this as a reference example. Furthermore, I would like to repeat my old request for a more realistic reference performance benchmark application than testpmd if performance benchmark results are used as arguments for or against suggestions like this. With testpmd, everything flies directly through the cache. I would be so bold as to claim that if you shuffle the mbuf fields between the first and second cache line, you probably wouldn't see any difference with testpmd; the packet goes directly to the tx step after the rx step, and the cache miss that would normally be seen in the tx step is instead taken in the rx step, so there is no performance difference. A reference performance benchmark application should have at least two or three separate steps, each processing a sufficiently large amount of packets, so the cache is only efficient within each step. It could be an ingress step for receiving and enqueueing the packets, a delay step holding enough packets for their information to disappear from the cache, and an egress step for dequeueing and transmitting the packets. Med venlig hilsen / kind regards - Morten Brørup
