Re: [dpdk-dev] [PATCH v4 7/8] ethdev: support of multiple sizes of redirection table

["Zhang, Helin" <helin.zhang@intel.com>]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, November 6, 2014 4:33 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 7/8] ethdev: support of multiple sizes of
> redirection table
> 
> 2014-11-06 01:02, Zhang, Helin:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-31 17:03, Helin Zhang:
> > > >  #define ETH_RSS_RETA_SIZE_64  64
> > > >  #define ETH_RSS_RETA_SIZE_128 128  #define ETH_RSS_RETA_SIZE_512
> > > > 512
> > >
> > > Are these values still needed?
> >
> > It was widely used in igb/ixgbe/i40e code, and app/testpmd. It is good
> > to be kept there, though we can define them separately in each
> > component. This would be more convenient for PMDs and user applications.
> 
> If it should be used by applications, it must stay in ethdev.
Good to get it aligned with us.

> 
> > > Why 256 is forbidden?
> >
> > 256 is not a valid table size of current supported NICs, for
> > other/future NIC which supports this size, it can be added later as needed.
> 
> The problem is that we don't know which value is supported for each driver.
> You should add a comment like this:
> /**@{
>  * Some RSS RETA sizes may be not supported by some drivers.
>  * Check in the PMD documentation.
>  */
> #define ETH_RSS_RETA_SIZE_64  64
> #define ETH_RSS_RETA_SIZE_128 128
> #define ETH_RSS_RETA_SIZE_512 512
> /**@}*/
In rte_ethdev.h, there is comments for rte_eth_dev_rss_reta_update() and
rte_eth_dev_rss_reta_query() that the reta table size can be queried by
rte_eth_dev_info_get().
So the end users could know the reta size of each NIC by reading its datasheet,
or call that function to query the size directly.
The macros defined here let the reta size more straightforward, and easy to use.
OK, it is good to add some annotations here. Thanks!

> 
> And then add some comments in the PMD to describe the supported sizes.
> 
> > > Maybe that some comments are needed here.
> >
> > Comments might not be needed, as their names tell us what they are
> > clearly. Did you mean any other annotations to be added for these
> > macros? I am open for that to add any good annotations for them.
> 
> We just have to keep in mind that the API reference for users is in doxygen.
> Some details are obvious for you but not clear for the user, especially if he
> doesn't care about i40e.
> 
> Thanks
> --
> Thomas

Regards,
Helin