Hi Andew, > 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).
If you are going to change ABI anyway, would it be worth to consider moving rx/tx burst/prepare functions to be per queue, instead of per device? > > 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. > >>
