On Thu, Sep 26, 2019 at 1:13 PM Andrew Rybchenko <arybche...@solarflare.com> wrote: > > On 9/24/19 7:50 PM, Ananyev, Konstantin wrote: > > Hi everyone, > > 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-...@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. > > My preference NOT to do #4. Reasons are: > - I have seen performance drop from 1.5% to 3.5% based on the arm64 > cores in use(Embedded vs Server cores) > - We need the correct approach to cater to cryptodev and eventdev as > well. If #4 is checked in, We will > take shotcut for cryptodev and eventdev > > My preference #1, do nothing, is probably ok and could live with #2, > adding padding, > and fix properly with #3 as when needed and use #3 scheme for crypto > dev and eventdev as well. > > > My preference would be #4 also. > If that's not an option, then I suppose #1 for 19.11 and #3 for next release > when ABI breakage would be allowed. > BTW, good point that we need similar thing for other dev types too. > Konstantin > > > My preference would be #4 or #1. > #2 and #3 are both tradeoffs and do not resolve ABI breaking completely. > #3 is really invasive, it requires changes of driverRx/Tx burst prototypes and > uninline descriptor status functions (may be it would be better to change > callback prototypes as well, but keep functions inline). > #4 is better since it is really a step to ABI stability and it still allow to > do many generic checks (dev->data dependent) on ethdev API level.
Did we ensure that external users have all the required api before hiding the rte_eth_dev struct? ovs still accesses rte_eth_devices[]. CC Ian and Ilya. -- David Marchand
