On Wed, Sep 20, 2023 at 9:39 AM Ferruh Yigit <ferruh.yi...@amd.com> wrote: > > On 9/8/2023 9:00 AM, Jie Hai wrote: > > 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 will be affected: > > - rte_eth_dev_configure > > - rte_eth_dev_rss_hash_update > > - rte_eth_dev_rss_hash_conf_get > > > > The problem with adding new configuration fields is, none of the drivers > are using it. > > I can see you are updating hns3 driver in next patch, but what about > others, application will set this field and almost all drivers will > ignore it for now. > And in near future, some will be supporting it and some won't, still > frustrating for the application perspective. > > In the past I was gathering list of these items and follow > implementation of them with drivers, but as the number of drivers > increased this became very hard to manage. > > Another way to manage this is go and update all drivers, and if the > configuration requests anything other than > 'RTE_ETH_HASH_FUNCTION_DEFAULT', return error. So that application can > know this is not supported by this driver. I think it is a good option. And then a driver can modify it when it adds support for other algorithms.
> Do you have better solution for this?
>
:::: snip ::::
>
> > +};
> > +
> > /**
> > * A structure used to configure the Receive Side Scaling (RSS) feature
> > * of an Ethernet port.
> > @@ -464,6 +484,7 @@ struct rte_eth_rss_conf {
> > * Supplying an *rss_hf* equal to zero disables the RSS feature.
> > */
> > uint64_t rss_hf;
> > + enum rte_eth_hash_function func; /**< Hash algorithm. */
>
> Can we use 'algorithm' as the field name?
>
> I know it won't exactly match type name (rte_eth_hash_function) but I
> think this will be more accurate also less confusing for "rss_hf (rss
> hash function)" and "func (function)"?
+1
>
> > };
> >
> > /*
> > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > index 271d854f7807..d3f2d466d841 100644
> > --- a/lib/ethdev/rte_flow.c
> > +++ b/lib/ethdev/rte_flow.c
> > @@ -12,7 +12,6 @@
> > #include <rte_branch_prediction.h>
> > #include <rte_string_fns.h>
> > #include <rte_mbuf_dyn.h>
> > -#include "rte_ethdev.h"
> > #include "rte_flow_driver.h"
> > #include "rte_flow.h"
> >
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index 3bd0dc64fbe2..a7578b1d2eff 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -40,6 +40,8 @@
> > #include <rte_macsec.h>
> > #include <rte_ib.h>
> >
> > +#include "rte_ethdev.h"
> > +
> > #ifdef __cplusplus
> > extern "C" {
> > #endif
> > @@ -3183,26 +3185,6 @@ struct rte_flow_query_count {
> > uint64_t bytes; /**< Number of bytes through this rule [out]. */
> > };
> >
> > -/**
> > - * Hash function types.
> > - */
> > -enum rte_eth_hash_function {
> > - /**
> > - * DEFAULT means that conformance to a specific hash algorithm is
> > - * uncared to the caller. The driver can pick the one it deems
> > optimal.
> > - */
> > - RTE_ETH_HASH_FUNCTION_DEFAULT = 0,
> > - RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */
> > - RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */
> > - /**
> > - * Symmetric Toeplitz: src, dst will be replaced by
> > - * xor(src, dst). For the case with src/dst only,
> > - * src or dst address will xor with zero pair.
> > - */
> > - RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ,
> > - RTE_ETH_HASH_FUNCTION_MAX,
> > -};
> > -
> > /**
> > * RTE_FLOW_ACTION_TYPE_RSS
> > *
>
smime.p7s
Description: S/MIME Cryptographic Signature
