[dpdk-dev] [PATCH] ethdev: reserve space in main structs for extension

[dpdk-dev] [PATCH] ethdev: reserve space in main structs for extension

[Thomas Monjalon]

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;
 
 /**
-- 
2.23.0

Re: [dpdk-dev] [PATCH] ethdev: reserve space in main structs for extension

[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>
> ---
>  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?

Re: [dpdk-dev] [PATCH] ethdev: reserve space in main structs for extension

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCH] ethdev: reserve space in main structs for extension

[Ferruh Yigit]

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>

Re: [dpdk-dev] [PATCH] ethdev: reserve space in main structs for extension

[Andrew Rybchenko]

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>

[dpdk-dev] [PATCH v2] ethdev: reserve space in main structs for extension

[Thomas Monjalon]

In order to allow smooth addition of features without breaking
ABI compatibility, some space is reserved in several core structs
of ethdev API.

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.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v2: padding more struct (config and get_info)

Note: previous acks are not kept in order to request an explicit
review of these new changes.
---
 lib/librte_ethdev/rte_ethdev.h      | 15 +++++++++++++++
 lib/librte_ethdev/rte_ethdev_core.h |  6 ++++++
 2 files changed, 21 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 44d77b332e..4ba01905c5 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -402,6 +402,9 @@ struct rte_eth_rxmode {
 	 * structure are allowed to be set.
 	 */
 	uint64_t offloads;
+
+	uint64_t reserved_64s[2]; /**< Reserved for future fields */
+	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
 /**
@@ -802,6 +805,9 @@ struct rte_eth_txmode {
 		/**< If set, reject sending out untagged pkts */
 		hw_vlan_insert_pvid : 1;
 		/**< If set, enable port based VLAN insertion */
+
+	uint64_t reserved_64s[2]; /**< Reserved for future fields */
+	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
 /**
@@ -818,6 +824,9 @@ struct rte_eth_rxconf {
 	 * fields on rte_eth_dev_info structure are allowed to be set.
 	 */
 	uint64_t offloads;
+
+	uint64_t reserved_64s[2]; /**< Reserved for future fields */
+	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
 /**
@@ -836,6 +845,9 @@ struct rte_eth_txconf {
 	 * fields on rte_eth_dev_info structure are allowed to be set.
 	 */
 	uint64_t offloads;
+
+	uint64_t reserved_64s[2]; /**< Reserved for future fields */
+	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
 /**
@@ -1260,6 +1272,9 @@ struct rte_eth_dev_info {
 	 * embedded managed interconnect/switch.
 	 */
 	struct rte_eth_switch_info switch_info;
+
+	uint64_t reserved_64s[2]; /**< Reserved for future fields */
+	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
 /**
diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index f215af7c94..4d52be6121 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -785,6 +785,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;
@@ -851,6 +854,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;
 
 /**
-- 
2.23.0

Re: [dpdk-dev] [PATCH v2] ethdev: reserve space in main structs for extension

[Ferruh Yigit]

On 11/11/2019 7:26 AM, Thomas Monjalon wrote:
> In order to allow smooth addition of features without breaking
> ABI compatibility, some space is reserved in several core structs
> of ethdev API.
> 
> 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.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2] ethdev: reserve space in main structs for extension

[Ferruh Yigit]

On 11/11/2019 10:54 AM, Ferruh Yigit wrote:
> On 11/11/2019 7:26 AM, Thomas Monjalon wrote:
>> In order to allow smooth addition of features without breaking
>> ABI compatibility, some space is reserved in several core structs
>> of ethdev API.
>>
>> 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.
>>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Applied to dpdk-next-net/master, thanks.