On Wed, Aug 30, 2023 at 4:46 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> Hello,
>
> Thanks for bringing a new capability.
>
> 26/08/2023 09:46, Jie Hai:
> > Currently, rte_eth_rss_conf supports configuring and querying
> > rss hash functions, rss key and it's length, but not rss hash
> > algorithm.
> >
> > The structure ``rte_eth_rss_conf`` is extended by adding a new
> > field "func". This represents the RSS algorithms to apply. The
> > following API is affected:
> >       - rte_eth_dev_configure
> >       - rte_eth_dev_rss_hash_update
> >       - rte_eth_dev_rss_hash_conf_get
>
> So far, the RSS algorithm was used only in flow RSS API.
>
> > --- a/doc/guides/rel_notes/release_23_11.rst
> > +++ b/doc/guides/rel_notes/release_23_11.rst
> > @@ -123,6 +123,8 @@ ABI Changes
> >     Also, make sure to start the actual text at the margin.
> >     =======================================================
> >
> > +   * ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash
> > +     algorithm.
>
> As written above, it should start at the margin.
>
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > +#include "rte_flow.h"
>
> It is strange to include rte_flow.h here.
> It would be better to move the enum.
>
> > + * The *func* field of the *rss_conf* structure indicates the hash algorithm
> > + * applied by the RSS hashing. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
> > + * the PMD to use its best-effort algorithm rather than a specific one.
> >   */
>
> I don't like commenting a field on top of the structure.
> By the way, the first sentence does not look helpful.
> RTE_ETH_HASH_FUNCTION_DEFAULT may be explained in the enum.
>
> >  struct rte_eth_rss_conf {
> >       uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
> >       uint8_t rss_key_len; /**< hash key length in bytes. */
> >       uint64_t rss_hf;     /**< Hash functions to apply - see below. */
> > +     enum rte_eth_hash_function func;        /**< Hash algorithm to apply. */
>
> You can drop "to apply" words.
>
> How the algorithms support combinations in rss_hf?
I will spend a little more time on this tomorrow.
Can you update testpmd also to display the info as a part of show rss.

>
>