[dpdk-dev,1/6] ethdev: add an field for querying hash key size

Message ID 1433379638-32715-2-git-send-email-helin.zhang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Zhang, Helin June 4, 2015, 1 a.m. UTC
  To support querying hash key size per port, an new field of
'hash_key_size' was added in 'struct rte_eth_dev_info' for storing
hash key size in bytes.

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 lib/librte_ether/rte_ethdev.h | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Neil Horman June 4, 2015, 1:05 p.m. UTC | #1
On Thu, Jun 04, 2015 at 09:00:33AM +0800, Helin Zhang wrote:
> To support querying hash key size per port, an new field of
> 'hash_key_size' was added in 'struct rte_eth_dev_info' for storing
> hash key size in bytes.
> 
> Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 16dbe00..004b05a 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -916,6 +916,7 @@ struct rte_eth_dev_info {
>  	uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
>  	uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
>  	uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
> +	uint8_t hash_key_size; /**< Hash key size in bytes */
>  	uint16_t reta_size;
>  	/**< Device redirection table size, the total number of entries. */
>  	/** Bit mask of RSS offloads, the bit offset also means flow type */
> -- 
> 1.9.3
> 
> 

You'll need to at least move this to the end of the structure to avoid ABI
breakage, but even then, since the examples statically allocate this struct on
the stack, you need to worry about previously compiled applications not having
enough space allocated.  Is there a hole in the struct that this can fit into to
avoid changing the other member offsets?
Neil
  
Zhang, Helin June 5, 2015, 6:21 a.m. UTC | #2
Hi Neil

Yes, thank you very much for the comments!
I realized the ABI issue after I sent out the patch. I think even I put the new one the end of this structure, it may also have issue.
I'd like to have this change announced and then get it merged. That means I'd like to make this change and follow the policy and process.

Regards,
Helin

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Thursday, June 4, 2015 9:05 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/6] ethdev: add an field for querying hash key
> size
> 
> On Thu, Jun 04, 2015 at 09:00:33AM +0800, Helin Zhang wrote:
> > To support querying hash key size per port, an new field of
> > 'hash_key_size' was added in 'struct rte_eth_dev_info' for storing
> > hash key size in bytes.
> >
> > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > ---
> >  lib/librte_ether/rte_ethdev.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.h
> > b/lib/librte_ether/rte_ethdev.h index 16dbe00..004b05a 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -916,6 +916,7 @@ struct rte_eth_dev_info {
> >  	uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
> >  	uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
> >  	uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
> > +	uint8_t hash_key_size; /**< Hash key size in bytes */
> >  	uint16_t reta_size;
> >  	/**< Device redirection table size, the total number of entries. */
> >  	/** Bit mask of RSS offloads, the bit offset also means flow type */
> > --
> > 1.9.3
> >
> >
> 
> You'll need to at least move this to the end of the structure to avoid ABI breakage,
> but even then, since the examples statically allocate this struct on the stack, you
> need to worry about previously compiled applications not having enough space
> allocated.  Is there a hole in the struct that this can fit into to avoid changing the
> other member offsets?
> Neil
  
Neil Horman June 5, 2015, 10:30 a.m. UTC | #3
On Fri, Jun 05, 2015 at 06:21:52AM +0000, Zhang, Helin wrote:
> Hi Neil
> 
> Yes, thank you very much for the comments!
> I realized the ABI issue after I sent out the patch. I think even I put the new one the end of this structure, it may also have issue.
> I'd like to have this change announced and then get it merged. That means I'd like to make this change and follow the policy and process.
> 
> Regards,
> Helin
> 

Ok, sounds good.

Thanks!
Neil

> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, June 4, 2015 9:05 PM
> > To: Zhang, Helin
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 1/6] ethdev: add an field for querying hash key
> > size
> > 
> > On Thu, Jun 04, 2015 at 09:00:33AM +0800, Helin Zhang wrote:
> > > To support querying hash key size per port, an new field of
> > > 'hash_key_size' was added in 'struct rte_eth_dev_info' for storing
> > > hash key size in bytes.
> > >
> > > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > > ---
> > >  lib/librte_ether/rte_ethdev.h | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/lib/librte_ether/rte_ethdev.h
> > > b/lib/librte_ether/rte_ethdev.h index 16dbe00..004b05a 100644
> > > --- a/lib/librte_ether/rte_ethdev.h
> > > +++ b/lib/librte_ether/rte_ethdev.h
> > > @@ -916,6 +916,7 @@ struct rte_eth_dev_info {
> > >  	uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
> > >  	uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
> > >  	uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
> > > +	uint8_t hash_key_size; /**< Hash key size in bytes */
> > >  	uint16_t reta_size;
> > >  	/**< Device redirection table size, the total number of entries. */
> > >  	/** Bit mask of RSS offloads, the bit offset also means flow type */
> > > --
> > > 1.9.3
> > >
> > >
> > 
> > You'll need to at least move this to the end of the structure to avoid ABI breakage,
> > but even then, since the examples statically allocate this struct on the stack, you
> > need to worry about previously compiled applications not having enough space
> > allocated.  Is there a hole in the struct that this can fit into to avoid changing the
> > other member offsets?
> > Neil
> 
>
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 16dbe00..004b05a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -916,6 +916,7 @@  struct rte_eth_dev_info {
 	uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
 	uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
 	uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
+	uint8_t hash_key_size; /**< Hash key size in bytes */
 	uint16_t reta_size;
 	/**< Device redirection table size, the total number of entries. */
 	/** Bit mask of RSS offloads, the bit offset also means flow type */