[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-29 Thread Thomas Monjalon 
2014-10-29 08:24, Zhang, Helin:
> 
> > -Original Message-
> > From: Zhang, Helin
> > Sent: Tuesday, October 28, 2014 8:01 PM
> > To: Thomas Monjalon
> > Cc: dev at dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes 
> > of
> > redirection table
> > 
> > Hi Thomas
> > 
> > > -Original Message-
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > Sent: Tuesday, October 28, 2014 6:10 PM
> > > To: Zhang, Helin
> > > Cc: dev at dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple
> > > sizes of redirection table
> > >
> > > 2014-10-28 00:33, Zhang, Helin:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2014-09-25 16:40, Helin Zhang:
> > > > > >  /* Definitions used for redirection table entry size */
> > > > > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > > > +#define ETH_RSS_RETA_SIZE_64  64 #define ETH_RSS_RETA_SIZE_128
> > > > > > +128 #define ETH_RSS_RETA_SIZE_512
> > > > > > +512
> > > > > > +
> > > > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > > > >
> > > > > Are these constants really needed?
> > > >
> > > > These constants were defined for the third input parameter of
> > > > rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query(). End
> > > > users need to give the correct reta size listed as above, as other
> > > > values is not valid. So it would be better to list the valid reta
> > > > sizes in macros
> > > here.
> > >
> > > OK, so you should explain that only these values are allowed.
> > > In general, it's something we explain in the comment of the function
> > It would be better to add comments for the functions.
> Now do not think explain what value is allowed for the functions in ethdev 
> layer,
> as it might be hardware specific.
> I think the best way is to comment out end users can get the reta size by
> 'dev_infos_get' on each port, rather than telling the values directly.

Yes, it's better.

Thanks
-- 
Thomas

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-29 Thread Zhang, Helin 


> -Original Message-
> From: Zhang, Helin
> Sent: Tuesday, October 28, 2014 8:01 PM
> To: Thomas Monjalon
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of
> redirection table
> 
> Hi Thomas
> 
> > -Original Message-
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > Sent: Tuesday, October 28, 2014 6:10 PM
> > To: Zhang, Helin
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple
> > sizes of redirection table
> >
> > 2014-10-28 00:33, Zhang, Helin:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2014-09-25 16:40, Helin Zhang:
> > > > >  /* Definitions used for redirection table entry size */
> > > > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > > +#define ETH_RSS_RETA_SIZE_64  64 #define ETH_RSS_RETA_SIZE_128
> > > > > +128 #define ETH_RSS_RETA_SIZE_512
> > > > > +512
> > > > > +
> > > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > > >
> > > > Are these constants really needed?
> > >
> > > These constants were defined for the third input parameter of
> > > rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query(). End
> > > users need to give the correct reta size listed as above, as other
> > > values is not valid. So it would be better to list the valid reta
> > > sizes in macros
> > here.
> >
> > OK, so you should explain that only these values are allowed.
> > In general, it's something we explain in the comment of the function
> It would be better to add comments for the functions.
Now do not think explain what value is allowed for the functions in ethdev 
layer,
as it might be hardware specific.
I think the best way is to comment out end users can get the reta size by
'dev_infos_get' on each port, rather than telling the values directly.

> 
> >
> > By the way, why only these values are allowed?
> It depends on hardware, 1G/10G hardware supports 128 reta size only, 40G
> hardware supports 512 or 128 depends on hardware configuration, 40G VF
> hardware supports 64. If more is introduced in the future, more values can be
> added later. It will return with errors if reta size is not supported for 
> specific
> hardware.
> 
> >
> > --
> > Thomas
> 
> Regards,
> Helin

Regards,
Helin

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-29 Thread Zhang, Helin 


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, October 28, 2014 10:23 PM
> To: Zhang, Helin
> Cc: Richardson, Bruce; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of
> redirection table
> 
> 2014-10-28 13:20, Zhang, Helin:
> > From: Richardson, Bruce
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > > > 2014-10-28 00:33, Zhang, Helin:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > 2014-09-25 16:40, Helin Zhang:
> > > > > > >  /* Definitions used for redirection table entry size */
> > > > > > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > > > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > > > > +#define ETH_RSS_RETA_SIZE_64  64 #define
> > > > > > > +ETH_RSS_RETA_SIZE_128 128 #define ETH_RSS_RETA_SIZE_512 512
> > > > > > > +
> > > > > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > > > > >
> > > > > > Are these constants really needed?
> > > > >
> > > > > These constants were defined for the third input parameter of
> > > > > rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query().
> > > > > End users need to give the correct reta size listed as above, as
> > > > > other values is not valid. So it would be better to list the
> > > > > valid reta sizes in macros here.
> > > >
> > > If only limited range of values are allowed, would an enum work
> > > better than a set of macros?
> >
> > Good idea! Any other comments for this from other guys?
> 
> I would prefer the API to be independent of the hardware capabilities.
Let keep it as it is, and add more possible comments somewhere.

> 
> --
> Thomas

Regards,
Helin

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Thomas Monjalon 
2014-10-28 13:20, Zhang, Helin:
> From: Richardson, Bruce
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > > 2014-10-28 00:33, Zhang, Helin:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2014-09-25 16:40, Helin Zhang:
> > > > > >  /* Definitions used for redirection table entry size */
> > > > > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > > > +#define ETH_RSS_RETA_SIZE_64  64
> > > > > > +#define ETH_RSS_RETA_SIZE_128 128
> > > > > > +#define ETH_RSS_RETA_SIZE_512 512
> > > > > > +
> > > > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > > > >
> > > > > Are these constants really needed?
> > > >
> > > > These constants were defined for the third input parameter of
> > > > rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query(). End
> > > > users need
> > > > to give the correct reta size listed as above, as other values is
> > > > not valid. So it would be
> > > > better to list the valid reta sizes in macros here.
> > >
> > If only limited range of values are allowed, would an enum work better than 
> > a set
> > of macros?
> 
> Good idea! Any other comments for this from other guys?

I would prefer the API to be independent of the hardware capabilities.

-- 
Thomas

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Zhang, Helin 
Hi Bruce

> -Original Message-
> From: Richardson, Bruce
> Sent: Tuesday, October 28, 2014 6:18 PM
> To: Thomas Monjalon; Zhang, Helin
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of
> redirection table
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Tuesday, October 28, 2014 10:10 AM
> > To: Zhang, Helin
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple
> > sizes of redirection table
> >
> > 2014-10-28 00:33, Zhang, Helin:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2014-09-25 16:40, Helin Zhang:
> > > > >  /* Definitions used for redirection table entry size */
> > > > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > > +#define ETH_RSS_RETA_SIZE_64  64 #define ETH_RSS_RETA_SIZE_128
> > > > > +128 #define ETH_RSS_RETA_SIZE_512 512
> > > > > +
> > > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > > >
> > > > Are these constants really needed?
> > >
> > > These constants were defined for the third input parameter of
> > > rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query(). End
> > > users
> > need
> > > to give the correct reta size listed as above, as other values is
> > > not valid. So it
> > would be
> > > better to list the valid reta sizes in macros here.
> >
> If only limited range of values are allowed, would an enum work better than a 
> set
> of macros?
Good idea! Any other comments for this from other guys?

> 
> > OK, so you should explain that only these values are allowed.
> > In general, it's something we explain in the comment of the function.
> >
> > By the way, why only these values are allowed?

Regards,
Helin

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Thomas Monjalon 
2014-10-28 12:00, Zhang, Helin:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2014-10-28 00:33, Zhang, Helin:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2014-09-25 16:40, Helin Zhang:
> > > > >  /* Definitions used for redirection table entry size */
> > > > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > > +#define ETH_RSS_RETA_SIZE_64  64
> > > > > +#define ETH_RSS_RETA_SIZE_128 128
> > > > > +#define ETH_RSS_RETA_SIZE_512 512
[...]
> > By the way, why only these values are allowed?
> 
> It depends on hardware, 1G/10G hardware supports 128 reta size only, 40G
> hardware supports 512 or 128 depends on hardware configuration, 40G VF
> hardware supports 64. If more is introduced in the future, more values can be
> added later. It will return with errors if reta size is not supported for 
> specific hardware.

So maybe it should be the responsibility of the driver to use
the right value. This kind of hardware differences should be abstracted
for the application.
If the application need to know the size, a "get" function could
be provided.

-- 
Thomas

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Zhang, Helin 
Hi Thomas

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, October 28, 2014 8:13 PM
> To: Zhang, Helin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of
> redirection table
> 
> 2014-10-28 12:00, Zhang, Helin:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2014-10-28 00:33, Zhang, Helin:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2014-09-25 16:40, Helin Zhang:
> > > > > >  /* Definitions used for redirection table entry size */
> > > > > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > > > +#define ETH_RSS_RETA_SIZE_64  64 #define
> > > > > > +ETH_RSS_RETA_SIZE_128 128 #define ETH_RSS_RETA_SIZE_512 512
> [...]
> > > By the way, why only these values are allowed?
> >
> > It depends on hardware, 1G/10G hardware supports 128 reta size only,
> > 40G hardware supports 512 or 128 depends on hardware configuration,
> > 40G VF hardware supports 64. If more is introduced in the future, more
> > values can be added later. It will return with errors if reta size is not 
> > supported
> for specific hardware.
> 
> So maybe it should be the responsibility of the driver to use the right 
> value. This
> kind of hardware differences should be abstracted for the application.
> If the application need to know the size, a "get" function could be provided.
Yes, the size can be gotten by ops of 'dev_infos_get'. But if end users know the
size of a specific port, he can just use these macros directly without getting 
it
from somewhere.

> 
> --
> Thomas

Regards,
Helin

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Zhang, Helin 
Hi Thomas

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, October 28, 2014 6:10 PM
> To: Zhang, Helin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of
> redirection table
> 
> 2014-10-28 00:33, Zhang, Helin:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2014-09-25 16:40, Helin Zhang:
> > > >  /* Definitions used for redirection table entry size */ -#define
> > > > ETH_RSS_RETA_NUM_ENTRIES 128
> > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > +#define ETH_RSS_RETA_SIZE_64  64
> > > > +#define ETH_RSS_RETA_SIZE_128 128 #define ETH_RSS_RETA_SIZE_512
> > > > +512
> > > > +
> > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > >
> > > Are these constants really needed?
> >
> > These constants were defined for the third input parameter of
> > rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query(). End
> > users need to give the correct reta size listed as above, as other
> > values is not valid. So it would be better to list the valid reta sizes in 
> > macros
> here.
> 
> OK, so you should explain that only these values are allowed.
> In general, it's something we explain in the comment of the function
It would be better to add comments for the functions.

> 
> By the way, why only these values are allowed?
It depends on hardware, 1G/10G hardware supports 128 reta size only, 40G
hardware supports 512 or 128 depends on hardware configuration, 40G VF
hardware supports 64. If more is introduced in the future, more values can be
added later. It will return with errors if reta size is not supported for 
specific hardware.

> 
> --
> Thomas

Regards,
Helin

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Thomas Monjalon 
2014-10-28 00:33, Zhang, Helin:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2014-09-25 16:40, Helin Zhang:
> > >  /* Definitions used for redirection table entry size */
> > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > +#define ETH_RSS_RETA_SIZE_64  64
> > > +#define ETH_RSS_RETA_SIZE_128 128
> > > +#define ETH_RSS_RETA_SIZE_512 512
> > > +
> > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > 
> > Are these constants really needed?
> 
> These constants were defined for the third input parameter of
> rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query(). End users need
> to give the correct reta size listed as above, as other values is not valid. 
> So it would be
> better to list the valid reta sizes in macros here.

OK, so you should explain that only these values are allowed.
In general, it's something we explain in the comment of the function.

By the way, why only these values are allowed?

-- 
Thomas

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Richardson, Bruce 
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, October 28, 2014 10:10 AM
> To: Zhang, Helin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of
> redirection table
> 
> 2014-10-28 00:33, Zhang, Helin:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2014-09-25 16:40, Helin Zhang:
> > > >  /* Definitions used for redirection table entry size */
> > > > -#define ETH_RSS_RETA_NUM_ENTRIES 128
> > > > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > > > +#define ETH_RSS_RETA_SIZE_64  64
> > > > +#define ETH_RSS_RETA_SIZE_128 128
> > > > +#define ETH_RSS_RETA_SIZE_512 512
> > > > +
> > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > >
> > > Are these constants really needed?
> >
> > These constants were defined for the third input parameter of
> > rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query(). End users
> need
> > to give the correct reta size listed as above, as other values is not 
> > valid. So it
> would be
> > better to list the valid reta sizes in macros here.
> 
If only limited range of values are allowed, would an enum work better than a 
set of macros? 

> OK, so you should explain that only these values are allowed.
> In general, it's something we explain in the comment of the function.
> 
> By the way, why only these values are allowed?

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Zhang, Helin 
Hi Thomas

Sorry, I should have answer your comments together with my reworking the latest 
patch set.
Please see my answers for your comments! I really appreciate your comments!

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, October 22, 2014 4:54 AM
> To: Zhang, Helin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of
> redirection table
> 
> 2014-09-25 16:40, Helin Zhang:
> > To support possible different sizes of redirection table, structures
> > and functions need to be redefined. In detail,
> > * 'struct rte_eth_rss_reta' has been redefined.
> > * 'uint16_t reta_size' has been added into
> >   'struct rte_eth_dev_info'.
> > * Updating/querying reta have been reimplemented with one
> >   more parameter of redirection table size.
> >
> > v2 changes:
> > * Put changes for supporting multiple sizes of reta in
> >   ethdev into a single patch.
> 
> In order to allow usage of git bisect, compilation must not be broken, even 
> inside
> a patchset.
> So when refactoring an existing API, you must adapt the dependent code in the
> same patch.
> To make things easy to review, please try to change API incrementally with 
> good
> explanation of why each change is needed.
OK. The patch set has been reworked to get all patches compiled well.

> 
> >  /* Definitions used for redirection table entry size */ -#define
> > ETH_RSS_RETA_NUM_ENTRIES 128
> > -#define ETH_RSS_RETA_MAX_QUEUE   16
> > +#define ETH_RSS_RETA_SIZE_64  64
> > +#define ETH_RSS_RETA_SIZE_128 128
> > +#define ETH_RSS_RETA_SIZE_512 512
> > +
> > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> 
> Are these constants really needed?
These constants were defined for the third input parameter of
rte_eth_dev_rss_reta_update() and rte_eth_dev_rss_reta_query(). End users need
to give the correct reta size listed as above, as other values is not valid. So 
it would be
better to list the valid reta sizes in macros here.

> 
> >  /**
> > - * A structure used to configure Redirection Table of  the Receive
> > Side
> > - * Scaling (RSS) feature of an Ethernet port.
> > + * A structure used to configure 64 entries of Redirection Table of
> > + the
> > + * Receive Side Scaling (RSS) feature of an Ethernet port. To
> > + configure
> > + * more than 64 entries supported by hardware, an array of this
> > + structure
> > + * is needed.
> >   */
> 
> Explaining the array of 64 entries could be useful in commit log.
> Please don't forget to answer the "why" question in commit logs.
OK. Rework for this has been done in the latest version of patch set.

> 
> Thanks
> --
> Thomas

Thanks,
Helin

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-10-21 Thread Thomas Monjalon 
2014-09-25 16:40, Helin Zhang:
> To support possible different sizes of redirection table,
> structures and functions need to be redefined. In detail,
> * 'struct rte_eth_rss_reta' has been redefined.
> * 'uint16_t reta_size' has been added into
>   'struct rte_eth_dev_info'.
> * Updating/querying reta have been reimplemented with one
>   more parameter of redirection table size.
> 
> v2 changes:
> * Put changes for supporting multiple sizes of reta in
>   ethdev into a single patch.

In order to allow usage of git bisect, compilation must not be broken,
even inside a patchset.
So when refactoring an existing API, you must adapt the dependent code
in the same patch.
To make things easy to review, please try to change API incrementally
with good explanation of why each change is needed.

>  /* Definitions used for redirection table entry size */
> -#define ETH_RSS_RETA_NUM_ENTRIES 128
> -#define ETH_RSS_RETA_MAX_QUEUE   16
> +#define ETH_RSS_RETA_SIZE_64  64
> +#define ETH_RSS_RETA_SIZE_128 128
> +#define ETH_RSS_RETA_SIZE_512 512
> +
> +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))

Are these constants really needed?

>  /**
> - * A structure used to configure Redirection Table of  the Receive Side
> - * Scaling (RSS) feature of an Ethernet port.
> + * A structure used to configure 64 entries of Redirection Table of the
> + * Receive Side Scaling (RSS) feature of an Ethernet port. To configure
> + * more than 64 entries supported by hardware, an array of this structure
> + * is needed.
>   */

Explaining the array of 64 entries could be useful in commit log.
Please don't forget to answer the "why" question in commit logs.

Thanks
-- 
Thomas

[dpdk-dev] [PATCH v2 04/13] ethdev: support of multiple sizes of redirection table

2014-09-25 Thread Helin Zhang 
To support possible different sizes of redirection table,
structures and functions need to be redefined. In detail,
* 'struct rte_eth_rss_reta' has been redefined.
* 'uint16_t reta_size' has been added into
  'struct rte_eth_dev_info'.
* Updating/querying reta have been reimplemented with one
  more parameter of redirection table size.

v2 changes:
* Put changes for supporting multiple sizes of reta in
  ethdev into a single patch.

Signed-off-by: Helin Zhang 
Reviewed-by: Jijiang Liu 
Reviewed-by: Cunming Liang 
Reviewed-by: Jingjing Wu 
---
 lib/librte_ether/rte_ethdev.c | 116 ++
 lib/librte_ether/rte_ethdev.h |  43 ++--
 2 files changed, 99 insertions(+), 60 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index b71b679..8c1cb25 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1917,78 +1917,104 @@ rte_eth_dev_priority_flow_ctrl_set(uint8_t port_id, 
struct rte_eth_pfc_conf *pfc
return (-ENOTSUP);
 }

-int
-rte_eth_dev_rss_reta_update(uint8_t port_id, struct rte_eth_rss_reta 
*reta_conf)
+static inline int
+rte_eth_check_reta_mask(struct rte_eth_rss_reta_entry64 *reta_conf,
+   uint16_t reta_size)
 {
-   struct rte_eth_dev *dev;
-   uint16_t max_rxq;
-   uint8_t i,j;
+   uint16_t i, num = reta_size / RTE_BIT_WIDTH_64;

-   if (port_id >= nb_ports) {
-   PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
-   return (-ENODEV);
-   }
+   if (!reta_conf)
+   return -EINVAL;

-   /* Invalid mask bit(s) setting */
-   if ((reta_conf->mask_lo == 0) && (reta_conf->mask_hi == 0)) {
-   PMD_DEBUG_TRACE("Invalid update mask bits for 
port=%d\n",port_id);
-   return (-EINVAL);
+   for (i = 0; i < num; i++) {
+   if (reta_conf[i].mask)
+   return 0;
}

-   dev = _eth_devices[port_id];
-   max_rxq = (dev->data->nb_rx_queues <= ETH_RSS_RETA_MAX_QUEUE) ?
-   dev->data->nb_rx_queues : ETH_RSS_RETA_MAX_QUEUE;
-   if (reta_conf->mask_lo != 0) {
-   for (i = 0; i < ETH_RSS_RETA_NUM_ENTRIES/2; i++) {
-   if ((reta_conf->mask_lo & (1ULL << i)) &&
-   (reta_conf->reta[i] >= max_rxq)) {
-   PMD_DEBUG_TRACE("RETA hash index output"
-   "configration for port=%d,invalid"
-   
"queue=%d\n",port_id,reta_conf->reta[i]);
+   return -EINVAL;
+}

-   return (-EINVAL);
-   }
+static inline int
+rte_eth_check_reta_entry(struct rte_eth_rss_reta_entry64 *reta_conf,
+uint16_t reta_size,
+uint8_t max_rxq)
+{
+   uint16_t i, idx, shift;
+
+   if (!reta_conf)
+   return -EINVAL;
+
+   if (max_rxq == 0) {
+   PMD_DEBUG_TRACE("No receive queue is available\n");
+   return -EINVAL;
+   }
+
+   for (i = 0; i < reta_size; i++) {
+   idx = i / RTE_BIT_WIDTH_64;
+   shift = i % RTE_BIT_WIDTH_64;
+   if ((reta_conf[idx].mask & (0x1 << shift)) &&
+   (reta_conf[idx].reta[shift] >= max_rxq)) {
+   PMD_DEBUG_TRACE("reta_conf[%u]->reta[%u]: %u exceeds "
+   "the maximum rxq index: %u\n", idx, shift,
+   reta_conf[idx].reta[shift], max_rxq);
+   return -EINVAL;
}
}

-   if (reta_conf->mask_hi != 0) {
-   for (i = 0; i< ETH_RSS_RETA_NUM_ENTRIES/2; i++) {
-   j = (uint8_t)(i + ETH_RSS_RETA_NUM_ENTRIES/2);
+   return 0;
+}

-   /* Check if the max entry >= 128 */
-   if ((reta_conf->mask_hi & (1ULL << i)) &&
-   (reta_conf->reta[j] >= max_rxq)) {
-   PMD_DEBUG_TRACE("RETA hash index output"
-   "configration for port=%d,invalid"
-   
"queue=%d\n",port_id,reta_conf->reta[j]);
+int
+rte_eth_dev_rss_reta_update(uint8_t port_id,
+   struct rte_eth_rss_reta_entry64 *reta_conf,
+   uint16_t reta_size)
+{
+   struct rte_eth_dev *dev;
+   int ret;

-   return (-EINVAL);
-   }
-   }
+   if (port_id >= nb_ports) {
+   PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+   return -ENODEV;
}

+   /* Check mask bits */
+   ret = rte_eth_check_reta_mask(reta_conf, reta_size);
+   if (ret < 0)
+   return ret;
+
+   dev = _eth_devices[port_id];
+
+   /* Check entry value */
+   ret =