[v2] doc: plan splitting the ethdev ops struct
Checks
Commit Message
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(+)
Comments
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.
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
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.
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.
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.
@@ -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