Hi Konstantin, On 2/25/20 2:07 PM, Ananyev, Konstantin wrote: > 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?
I'm thinking about it from time to time. In general I like the idea. The only question I have if there is a demand for queues with different offloads which could result in different Rx/Tx callbacks. Also I'm not sure that it is doable without performance degradation at all, but hopefully it will be tiny. >> 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. >>>>
