On 2/21/20 1:40 PM, Ferruh Yigit wrote: > On 2/18/2020 6:01 AM, Stephen Hemminger wrote: >> On Mon, 17 Feb 2020 15:38:05 +0000 >> Ferruh Yigit <ferruh.yi...@intel.com> wrote: >> >>> For the ABI compatibility it is better to hide internal data structures >>> from the application as much as possible. But because of some inline >>> functions 'struct eth_dev_ops' can't be hidden completely. >>> >>> Plan is to split the 'struct eth_dev_ops' into two as ones used by >>> inline functions and ones not used, and hide the second part that not >>> used by inline functions completely to the application. >>> >>> Signed-off-by: Ferruh Yigit <ferruh.yi...@intel.com> >>> --- >>> Cc: David Marchand <david.march...@redhat.com> >>> Cc: Thomas Monjalon <tho...@monjalon.net> >>> Cc: Andrew Rybchenko <arybche...@solarflare.com> >>> --- >>> doc/guides/rel_notes/deprecation.rst | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/doc/guides/rel_notes/deprecation.rst >>> b/doc/guides/rel_notes/deprecation.rst >>> index dfcca87ab..2aa431028 100644 >>> --- a/doc/guides/rel_notes/deprecation.rst >>> +++ b/doc/guides/rel_notes/deprecation.rst >>> @@ -72,6 +72,12 @@ Deprecation Notices >>> In 19.11 PMDs will still update the field even when the offload is not >>> enabled. >>> >>> +* ethdev: Split the ``struct eth_dev_ops`` struct to hide it as much as >>> possible. >>> + Currently the ``struct eth_dev_ops`` struct is accessible by the >>> application >>> + because some inline functions, like ``rte_eth_tx_descriptor_status()``, >>> + access the struct directly. The struct will be separate in two, the ops >>> used >>> + by inline functions still will be accessible to user but rest will be >>> hidden. >>> + >>> * cryptodev: support for using IV with all sizes is added, J0 still can >>> be used but only when IV length in following structs >>> ``rte_crypto_auth_xform``, >>> ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or >>> equal >> >> Good luck, truely hiding internals is hard. The mbuf structure is already >> split but not really >> hidden at all (just look at dwarf output). It doesn't make sense to do it >> unless >> you can really hide it. > > I believe this can be done, only following [1] dev_ops are used by inline > functions, rest can be moved into separate struct and moved into ethdev driver > looking header. > > [1] > rx_queue_count > rx_descriptor_done > rx_descriptor_status > tx_descriptor_status
I think having 3 places (if I understand the intention correctly) with ethdev callbacks is too much. So, I think that these ops should be simply moved to nearby Tx/Rx burst and Tx prepare callbacks (e.g. move into inline_ops structure which is located at the beginning of rte_eth_dev in order to preserve 3 existing callback location). Also I'd consider to deprecate and remove rx_queue_count and rx_descriptor_done. >> >> I would attack the rte_device stuff first. Make rte_device opaque to the >> application >> that would help for future versions. Then work backwards to rte_tehtdev. >>
