ethdev: reserve space in main structs for extension

Message ID 20191107221524.23205-1-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: reserve space in main structs for extension |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance fail Performance Testing issues
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Thomas Monjalon Nov. 7, 2019, 10:15 p.m. UTC
  The struct rte_eth_dev and rte_eth_dev_data are supposed
to be used internally only, but there is a chance that
increasing their size would break ABI for some applications.
In order to allow smooth addition of features without breaking
ABI compatibility, some space is reserved.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_ethdev/rte_ethdev_core.h | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Stephen Hemminger Nov. 8, 2019, 3:41 a.m. UTC | #1
On Thu,  7 Nov 2019 23:15:24 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> The struct rte_eth_dev and rte_eth_dev_data are supposed
> to be used internally only, but there is a chance that
> increasing their size would break ABI for some applications.
> In order to allow smooth addition of features without breaking
> ABI compatibility, some space is reserved.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/librte_ethdev/rte_ethdev_core.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
> index 392aea8e6b..ea8dd1d9ba 100644
> --- a/lib/librte_ethdev/rte_ethdev_core.h
> +++ b/lib/librte_ethdev/rte_ethdev_core.h
> @@ -698,6 +698,9 @@ struct rte_eth_dev {
>  	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
>  	enum rte_eth_dev_state state; /**< Flag indicating the port state */
>  	void *security_ctx; /**< Context for security ops */
> +
> +	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> +	void *reserved_ptrs[4]; /**< Reserved for future fields */
>  } __rte_cache_aligned;
>  
>  struct rte_eth_dev_sriov;
> @@ -764,6 +767,9 @@ struct rte_eth_dev_data {
>  			/**< Switch-specific identifier.
>  			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
>  			 */
> +
> +	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> +	void *reserved_ptrs[4]; /**< Reserved for future fields */
>  } __rte_cache_aligned;
>  
>  /**

Void * is 32 bits on 32 bit architectures is that helpful or not?
  
Thomas Monjalon Nov. 8, 2019, 9:40 a.m. UTC | #2
08/11/2019 04:41, Stephen Hemminger:
> On Thu,  7 Nov 2019 23:15:24 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > The struct rte_eth_dev and rte_eth_dev_data are supposed
> > to be used internally only, but there is a chance that
> > increasing their size would break ABI for some applications.
> > In order to allow smooth addition of features without breaking
> > ABI compatibility, some space is reserved.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > @@ -764,6 +767,9 @@ struct rte_eth_dev_data {
> > +
> > +	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> > +	void *reserved_ptrs[4]; /**< Reserved for future fields */
> >  } __rte_cache_aligned;
> 
> Void * is 32 bits on 32 bit architectures is that helpful or not?

That's why I reserved separately uint and pointers.
If we need to add a pointer, we decrease the size of the pointer array
to keep the same struct size on all archs.
  
Ferruh Yigit Nov. 8, 2019, 9:57 a.m. UTC | #3
On 11/7/2019 10:15 PM, Thomas Monjalon wrote:
> The struct rte_eth_dev and rte_eth_dev_data are supposed
> to be used internally only, but there is a chance that
> increasing their size would break ABI for some applications.
> In order to allow smooth addition of features without breaking
> ABI compatibility, some space is reserved.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
  
Andrew Rybchenko Nov. 8, 2019, 10:52 a.m. UTC | #4
On 11/8/19 12:57 PM, Ferruh Yigit wrote:
> On 11/7/2019 10:15 PM, Thomas Monjalon wrote:
>> The struct rte_eth_dev and rte_eth_dev_data are supposed
>> to be used internally only, but there is a chance that
>> increasing their size would break ABI for some applications.
>> In order to allow smooth addition of features without breaking
>> ABI compatibility, some space is reserved.
>>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

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

Patch

diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 392aea8e6b..ea8dd1d9ba 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -698,6 +698,9 @@  struct rte_eth_dev {
 	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
 	enum rte_eth_dev_state state; /**< Flag indicating the port state */
 	void *security_ctx; /**< Context for security ops */
+
+	uint64_t reserved_64s[4]; /**< Reserved for future fields */
+	void *reserved_ptrs[4]; /**< Reserved for future fields */
 } __rte_cache_aligned;
 
 struct rte_eth_dev_sriov;
@@ -764,6 +767,9 @@  struct rte_eth_dev_data {
 			/**< Switch-specific identifier.
 			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
 			 */
+
+	uint64_t reserved_64s[4]; /**< Reserved for future fields */
+	void *reserved_ptrs[4]; /**< Reserved for future fields */
 } __rte_cache_aligned;
 
 /**