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

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

[Ferruh Yigit]

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

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

[Jerin Jacob]

On Mon, Feb 17, 2020 at 9:08 PM Ferruh Yigit <ferruh.yigit@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.

It is a good improvement.  IMO, If anything used in fast-path it
should be in ``struct rte_eth_dev``
and rest can completely be moved to internal. In this case, if
`rte_eth_tx_descriptor_status`
not used on fastpath, Maybe we don't need to maintain the inline
status and move completely
to .c file.

Those may be specifics of the work. In general, this change looks good to me.

Acked-by: Jerin Jacob <jerinj@marvell.com>





>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: David Marchand <david.marchand@redhat.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Andrew Rybchenko <arybchenko@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
> --
> 2.24.1
>

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

[Stephen Hemminger]

On Mon, 17 Feb 2020 15:38:05 +0000
Ferruh Yigit <ferruh.yigit@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@intel.com>
> ---
> Cc: David Marchand <david.marchand@redhat.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Andrew Rybchenko <arybchenko@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 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.

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

[Ferruh Yigit]

On 2/18/2020 6:01 AM, Stephen Hemminger wrote:
> On Mon, 17 Feb 2020 15:38:05 +0000
> Ferruh Yigit <ferruh.yigit@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@intel.com>
>> ---
>> Cc: David Marchand <david.marchand@redhat.com>
>> Cc: Thomas Monjalon <thomas@monjalon.net>
>> Cc: Andrew Rybchenko <arybchenko@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 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.
>

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

[Andrew Rybchenko]

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

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

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

[Andrew Rybchenko]

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

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

[Ferruh Yigit]

On 2/25/2020 10:35 AM, Andrew Rybchenko wrote:
> 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@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@intel.com>
>>>> ---
>>>> Cc: David Marchand <david.marchand@redhat.com>
>>>> Cc: Thomas Monjalon <thomas@monjalon.net>
>>>> Cc: Andrew Rybchenko <arybchenko@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. 

Yes, this is the intention.

> 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).

I think this is reasonable, but this can be done only in 20.11 with ABI break.

What do you think doing the initial hide in 20.05 with three places and do
proper splitting in 20.11 as suggested above.

Or it is possible to drop the interim work and do the all changes in 20.11, not
sure.


> 
> Also I'd consider to deprecate and remove rx_queue_count
> and rx_descriptor_done.

+1 to deprecate the 'rx_descriptor_done', we have already 'rx_descriptor_status'
& 'tx_descriptor_status' to replace the functionality.

@Thomas, @Ray, can you please remind how deprecation was done, is it marking
API, 'rte_eth_rx_descriptor_done', as deprecated in 20.11 and removing it in
21.11? If so I guess deprecation notice should be send before 20.11? (And make
sure all PMDs did the switch before 20.11)


Not sure about 'rx_queue_count', if any application may need it or not.

> 
>>>
>>> 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.
>>>
> 

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

[Ferruh Yigit]

On 2/18/2020 5:07 AM, Jerin Jacob wrote:
> On Mon, Feb 17, 2020 at 9:08 PM Ferruh Yigit <ferruh.yigit@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.
> 
> It is a good improvement.  IMO, If anything used in fast-path it
> should be in ``struct rte_eth_dev``
> and rest can completely be moved to internal. In this case, if
> `rte_eth_tx_descriptor_status`
> not used on fastpath, Maybe we don't need to maintain the inline
> status and move completely
> to .c file.

Moving fast-past dev_ops to ``struct rte_eth_dev`` makes sense, also suggested
by Andrew, and will hide the rest.

Those inline dev_ops are datapath related so may be better to keep them as
inline, but not really sure about their users and how much they care about minor
performance impact if we chose to convert them into regular functions.

> 
> Those may be specifics of the work. In general, this change looks good to me.
> 
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> 
> 
> 
> 
> 
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: David Marchand <david.marchand@redhat.com>
>> Cc: Thomas Monjalon <thomas@monjalon.net>
>> Cc: Andrew Rybchenko <arybchenko@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
>> --
>> 2.24.1
>>

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

[Ferruh Yigit]

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@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
Cc: David Marchand <david.marchand@redhat.com>
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>

v2:
* Add target date for the work
* Give more detail on what will be done
---
 doc/guides/rel_notes/deprecation.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 99d81564a..ff612a615 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -86,6 +86,17 @@ 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.
+  Initial split will be done in 20.05 with adding reserved fields for the struct
+  used by inline functions, and by putting new struct reference into public one
+  to not increase the size of ``struct rte_eth_dev``, proper split will be done
+  in 20.11 by moving inline dev_ops function to next to Rx/Tx burst functions and
+  hiding rest.
+
 * 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
-- 
2.24.1

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

[Andrew Rybchenko]

On 2/25/20 3:44 PM, Ferruh Yigit 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@intel.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> ---
> Cc: David Marchand <david.marchand@redhat.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
>
> v2:
> * Add target date for the work
> * Give more detail on what will be done
> ---
>  doc/guides/rel_notes/deprecation.rst | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 99d81564a..ff612a615 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -86,6 +86,17 @@ 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.
> +  Initial split will be done in 20.05 with adding reserved fields for the struct
> +  used by inline functions, and by putting new struct reference into public one
> +  to not increase the size of ``struct rte_eth_dev``, proper split will be done
> +  in 20.11 by moving inline dev_ops function to next to Rx/Tx burst functions and
> +  hiding rest.
> +
>  * 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

I'd like to understand why do we need an intermediate solution first.
Also rereading above few times I've failed to fully understand what
will be done and why it does not break API/ABI.

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

[Ferruh Yigit]

On 2/25/2020 3:51 PM, Andrew Rybchenko wrote:
> On 2/25/20 3:44 PM, Ferruh Yigit 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@intel.com>
>> Acked-by: Jerin Jacob <jerinj@marvell.com>
>> ---
>> Cc: David Marchand <david.marchand@redhat.com>
>> Cc: Thomas Monjalon <thomas@monjalon.net>
>> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
>>
>> v2:
>> * Add target date for the work
>> * Give more detail on what will be done
>> ---
>>  doc/guides/rel_notes/deprecation.rst | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>> index 99d81564a..ff612a615 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -86,6 +86,17 @@ 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.
>> +  Initial split will be done in 20.05 with adding reserved fields for the struct
>> +  used by inline functions, and by putting new struct reference into public one
>> +  to not increase the size of ``struct rte_eth_dev``, proper split will be done
>> +  in 20.11 by moving inline dev_ops function to next to Rx/Tx burst functions and
>> +  hiding rest.
>> +
>>  * 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
> 
> I'd like to understand why do we need an intermediate solution first.

We don't need it really, we may prefer to postpone the update to 20.11.

> Also rereading above few times I've failed to fully understand what
> will be done and why it does not break API/ABI.
> 

Agree it wasn't very clear, but I mean something like below, I hope it clarifies:

Previously:

struct rte_eth_dev

+--------+
|        |    *dev_ops    struct eth_dev_ops
|        |
|        +-------------->+--------+
|        |               |        |
|        |               |        |
|        |               |        |
|        |               |        |
+--------+               |        |
                         |        |
                         |        |
                         |        |
                         +--------+


Proposed:

struct rte_eth_dev

+--------+
|        |                struct eth_dev_ops
|        |
|*dev_ops+-------------->+--------+
|        |               |        |
|        |               | Reserv |
|        |               |        |
|        |               |        |
+--------+               |        |
                         +--------+
                         | inline |
                         | de^_ops|               struct eth_dev_ops
                         +--------+
                         | *priv  +------------->+--------+
                         +--------+              |        |
                                                 |        |
                                                 |        |
                                                 |        |
                                                 |        |
                                                 |        |
                                                 |        |
                                                 |        |
                                                 +--------+

This is only to keep ABI compatibility [1] while separating the struct.

[1]
- The offset of some functions in the dev_ops struct should be same
- The size of the "struct rte_eth_dev" should be same

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

[Andrew Rybchenko]

On 2/25/20 7:13 PM, Ferruh Yigit wrote:
> On 2/25/2020 3:51 PM, Andrew Rybchenko wrote:
>> On 2/25/20 3:44 PM, Ferruh Yigit 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@intel.com>
>>> Acked-by: Jerin Jacob <jerinj@marvell.com>
>>> ---
>>> Cc: David Marchand <david.marchand@redhat.com>
>>> Cc: Thomas Monjalon <thomas@monjalon.net>
>>> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
>>>
>>> v2:
>>> * Add target date for the work
>>> * Give more detail on what will be done
>>> ---
>>>  doc/guides/rel_notes/deprecation.rst | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>>> index 99d81564a..ff612a615 100644
>>> --- a/doc/guides/rel_notes/deprecation.rst
>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>> @@ -86,6 +86,17 @@ 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.
>>> +  Initial split will be done in 20.05 with adding reserved fields for the struct
>>> +  used by inline functions, and by putting new struct reference into public one
>>> +  to not increase the size of ``struct rte_eth_dev``, proper split will be done
>>> +  in 20.11 by moving inline dev_ops function to next to Rx/Tx burst functions and
>>> +  hiding rest.
>>> +
>>>  * 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
>>
>> I'd like to understand why do we need an intermediate solution first.
> 
> We don't need it really, we may prefer to postpone the update to 20.11.
> 
>> Also rereading above few times I've failed to fully understand what
>> will be done and why it does not break API/ABI.
>>
> 
> Agree it wasn't very clear, but I mean something like below, I hope it clarifies:
> 
> Previously:
> 
> struct rte_eth_dev
> 
> +--------+
> |        |    *dev_ops    struct eth_dev_ops
> |        |
> |        +-------------->+--------+
> |        |               |        |
> |        |               |        |
> |        |               |        |
> |        |               |        |
> +--------+               |        |
>                          |        |
>                          |        |
>                          |        |
>                          +--------+
> 
> 
> Proposed:
> 
> struct rte_eth_dev
> 
> +--------+
> |        |                struct eth_dev_ops
> |        |
> |*dev_ops+-------------->+--------+
> |        |               |        |
> |        |               | Reserv |
> |        |               |        |
> |        |               |        |
> +--------+               |        |
>                          +--------+
>                          | inline |
>                          | de^_ops|               struct eth_dev_ops
>                          +--------+
>                          | *priv  +------------->+--------+
>                          +--------+              |        |
>                                                  |        |
>                                                  |        |
>                                                  |        |
>                                                  |        |
>                                                  |        |
>                                                  |        |
>                                                  |        |
>                                                  +--------+
> 
> This is only to keep ABI compatibility [1] while separating the struct.
> 
> [1]
> - The offset of some functions in the dev_ops struct should be same
> - The size of the "struct rte_eth_dev" should be same
> 

OK, got it. Many thanks for explanations.

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

[David Marchand]

On Tue, Feb 25, 2020 at 1:44 PM Ferruh Yigit <ferruh.yigit@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@intel.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>

Sorry, this patch is missing the necessary acks for merging.
This change can still be announced for 20.11 in the next releases and
the details can be discussed again since a conclusion was not reached.


Thanks.

-- 
David Marchand

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

[Ferruh Yigit]

On 2/25/2020 6:13 PM, David Marchand wrote:
> On Tue, Feb 25, 2020 at 1:44 PM Ferruh Yigit <ferruh.yigit@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@intel.com>
>> Acked-by: Jerin Jacob <jerinj@marvell.com>
> 
> Sorry, this patch is missing the necessary acks for merging.
> This change can still be announced for 20.11 in the next releases and
> the details can be discussed again since a conclusion was not reached.
> 
> 

That is OK, we can drop for 20.02, I can send a new version to target the 20.11
change only removing interim change.

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

[Ferruh Yigit]

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.

Because of ABI break the work will be done in 20.11

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: David Marchand <david.marchand@redhat.com>
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>

v2:
* Add target date for the work
* Give more detail on what will be done

v3:
* Drop the interim implementation, and target 20.11 with ABI break.
---
 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1339f54f5..0bb252f71 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -93,6 +93,15 @@ 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
+  will be done in 20.11.
+  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 will be moved
+  next to Rx/Tx burst functions, rest of the ``struct eth_dev_ops`` struct will be
+  moved to header file for drivers to hide it from applications.
+
 * 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
-- 
2.24.1

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

[Thomas Monjalon]

04/03/2020 10:57, Ferruh Yigit:
> 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.
> 
> Because of ABI break the work will be done in 20.11
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> +* ethdev: Split the ``struct eth_dev_ops`` struct to hide it as much as possible
> +  will be done in 20.11.
> +  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 will be moved
> +  next to Rx/Tx burst functions, rest of the ``struct eth_dev_ops`` struct will be
> +  moved to header file for drivers to hide it from applications.

Acked-by: Thomas Monjalon <thomas@monjalon.net>

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

[Andrew Rybchenko]

On 5/25/20 2:18 AM, Thomas Monjalon wrote:
> 04/03/2020 10:57, Ferruh Yigit:
>> 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.
>>
>> Because of ABI break the work will be done in 20.11
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> +* ethdev: Split the ``struct eth_dev_ops`` struct to hide it as much as possible
>> +  will be done in 20.11.
>> +  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 will be moved
>> +  next to Rx/Tx burst functions, rest of the ``struct eth_dev_ops`` struct will be
>> +  moved to header file for drivers to hide it from applications.
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

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

[David Marchand]

On Wed, Mar 4, 2020 at 10:57 AM Ferruh Yigit <ferruh.yigit@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.
>
> Because of ABI break the work will be done in 20.11
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Acked-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

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

[Thomas Monjalon]

18/02/2020 06:07, Jerin Jacob:
> On Mon, Feb 17, 2020 at 9:08 PM Ferruh Yigit <ferruh.yigit@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.
> 
> It is a good improvement.  IMO, If anything used in fast-path it
> should be in ``struct rte_eth_dev``
> and rest can completely be moved to internal. In this case, if
> `rte_eth_tx_descriptor_status`
> not used on fastpath, Maybe we don't need to maintain the inline
> status and move completely
> to .c file.
> 
> Those may be specifics of the work. In general, this change looks good to me.
> 
> Acked-by: Jerin Jacob <jerinj@marvell.com>

This ack is missing from v3.
Jerin, please could you confirm on v3?

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

[Thomas Monjalon]

25/05/2020 11:11, Andrew Rybchenko:
> On 5/25/20 2:18 AM, Thomas Monjalon wrote:
> > 04/03/2020 10:57, Ferruh Yigit:
> >> 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.
> >>
> >> Because of ABI break the work will be done in 20.11
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >> +* ethdev: Split the ``struct eth_dev_ops`` struct to hide it as much as possible
> >> +  will be done in 20.11.
> >> +  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 will be moved
> >> +  next to Rx/Tx burst functions, rest of the ``struct eth_dev_ops`` struct will be
> >> +  moved to header file for drivers to hide it from applications.
> > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Acked-by: David Marchand <david.marchand@redhat.com>

Applied, thanks