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

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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