[dpdk-dev] [PATCH] doc: plan splitting the ethdev ops struct

[Ananyev, Konstantin]

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.yigit at 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.yigit at intel.com>
> >>> ---
> >>> Cc: David Marchand <david.marchand at redhat.com>
> >>> Cc: Thomas Monjalon <thomas at monjalon.net>
> >>> Cc: Andrew Rybchenko <arybchenko at 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.
> >>