On Fri, Sep 8, 2023 at 1:44 AM Jie Hai <haijie1@huawei.com> wrote:
>
> Hi, Thomas
> Thanks for your review.
>
> On 2023/9/4 15:45, Thomas Monjalon wrote:
> > 04/09/2023 09:10, Jie Hai:
> >> On 2023/8/30 19:46, Thomas Monjalon wrote:
> >>> 26/08/2023 09:46, Jie Hai:
> >   >> + * 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.
> >>>
> >> Other fields  above the structure 'rte_eth_rss_conf' have comments.
> >> If the new fields 'func' do not have comments, it may be misleading.
> >> Unless all the comments above are removed. I'm not sure whether to
> >> delete them or not.
> >
> > You should explain RTE_ETH_HASH_FUNCTION_DEFAULT in its enum.
> > The rest of the explanation can be on the struct field.
> > I'm OK to have another patch moving old explanations from the struct
> > to the fields.
> Fixed in V4, please check it.
> >
> >>>>    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.
> >> Fixed in V3.
> >>>
> >>> How the algorithms support combinations in rss_hf?
> >>>
> >> The rss_hf defines the input of the RSS hash algorithms.
> >> For example, rss_hf = RTE_ETH_RSS_NONFRAG_IPV4_TCP | RTE_ETH_RSS_IPV4,
> >> these two kinds of packets can be dispatched to different queues
> >> according to their tuple field value.
> >> For ipv4-tcp packets, src-ip, dst-ip, src-port, dst-port are used as
> >> parameters of RSS hash algorithms to compute hash value.
> >> For ipv4 packets src-ip, dst-ip are used.
> >>
> >> If rss_hf = RTE_ETH_RSS_NONFRAG_IPV4_TCP | RTE_ETH_RSS_L4_SRC_ONLY, for
> >> ipv4-tcp packets, src-port is used as parameters of RSS hash algorithms
> >> to compute hash value.
> >
> > I know what is rss_hf.
> > My question is about the algorithms.
> > Do they all support any combination in rss_hf?
> >
> >
> I don't know about all vendors' hardware implementations,
> so here's just my simple opinion.
>
> Theoretically, I think that all algorithms should support all
> combinations in rss_hf, The reasons are as follows.

Leave it to the driver and hardware to decide what combinations can be
supported.

>
> 1. The rss_hf and algorithms are independent. For different algorithms
> and the same packets and hash key, the input parameters for different
> algorithms should be the same, which depends on implemetation of hardware.
>
> 2. As long as hardware and driver support, all packet types defined by
> rss_hf could generate the the input parameters for the algorithm to compute.
>
> Thanks,
> Jie Hai
> > .