RE: [PATCH] ethdev: add new symmetric hash function

["Xueming(Steven) Li" <xuemingl@nvidia.com>]

> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: 8/8/2023 9:43
> To: Ivan Malov <ivan.malov@arknetworks.am>; Xueming(Steven) Li
> <xuemingl@nvidia.com>
> Cc: Ori Kam <orika@nvidia.com>; dev@dpdk.org
> Subject: Re: [PATCH] ethdev: add new symmetric hash function
> 
> On 2023/8/8 6:32, Ivan Malov wrote:
> > Hi,
> >
> > Please see my notes below.
> >
> > On Mon, 7 Aug 2023, Xueming Li wrote:
> >
> >> The new symmetric hash function swap src/dst L3 address and
> >> L4 ports automatically by sorting.
> >>
> >> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> >> ---
> >> lib/ethdev/rte_flow.h | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> >> 86ed98c562..ec6dd170b5 100644
> >> --- a/lib/ethdev/rte_flow.h
> >> +++ b/lib/ethdev/rte_flow.h
> >> @@ -3204,6 +3204,11 @@ enum rte_eth_hash_function {
> >>      * src or dst address will xor with zero pair.
> >>      */
> >>     RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ,
> >> +    /**
> >> +     * Symmetric Toeplitz: src, dst will be swapped
> >> +     * automatically by sorting.
> >
> > This is very vague. Consider:
> >
> > For symmetric Toeplitz, four inputs are prepared as follows:
> > - src_addr | dst_addr
> > - src_addr ^ dst_addr
> > - src_port | dst_port
> > - src_port ^ dst_port
> > and then passed to the regular Toeplitz function.
> >
> > It is important to be as specific as possible so that readers don't
> > have to guess.
> 
> +1 for this, I try to understand and google it, but can't find useful info.
> 
> Also, how this new algo with src/dst only ?
> 

Thanks for taking care of this. 
When set the L3 and the L4 fields are sorted prior to the hash function.
  If src_ip > dst_ip, swap src_ip and dst_ip.
  If src_port > dst_port, swap src_port and dst_port.

> >
> > Thank you.
> >
> >> +     */
> >> +    RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ_SORT,
> >>     RTE_ETH_HASH_FUNCTION_MAX,
> 
> The new value will break the definition of MAX (maybe ABI compatible).
> but I found only hns3 drivers use RTE_ETH_HASH_FUNCTION_MAX, not sure
> the application will use it.
> 
> >> };
> >>
> >> --
> >> 2.25.1
> >>
> >>
> >
> > .