* [dpdk-dev] [RFC] ethdev: support symmetric hash function @ 2019-07-04 4:46 simei 2019-07-04 9:06 ` Adrien Mazarguil 0 siblings, 1 reply; 4+ messages in thread From: simei @ 2019-07-04 4:46 UTC (permalink / raw) To: qi.z.zhang, jingjing.wu, beilei.xing, qiming.yang; +Cc: dev, simei.su From: Simei Su <simei.su@intel.com> Currently, there are DEFAULT,TOEPLITZ and SIMPLE_XOR hash funtion. To support symmetric hash by rte_flow RSS action, this RFC introduces SYMMETRIC_TOEPLITZ to rte_eth_hash_function. Signed-off-by: Simei Su <simei.su@intel.com> --- lib/librte_ethdev/rte_flow.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h index f3a8fb1..e3c4fe5 100644 --- a/lib/librte_ethdev/rte_flow.h +++ b/lib/librte_ethdev/rte_flow.h @@ -1744,6 +1744,7 @@ enum rte_eth_hash_function { RTE_ETH_HASH_FUNCTION_DEFAULT = 0, RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */ RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */ + RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ, /**< Symmetric TOEPLITZ */ RTE_ETH_HASH_FUNCTION_MAX, }; -- 1.8.3.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: support symmetric hash function 2019-07-04 4:46 [dpdk-dev] [RFC] ethdev: support symmetric hash function simei @ 2019-07-04 9:06 ` Adrien Mazarguil 2019-07-04 12:48 ` Zhang, Qi Z 0 siblings, 1 reply; 4+ messages in thread From: Adrien Mazarguil @ 2019-07-04 9:06 UTC (permalink / raw) To: simei Cc: qi.z.zhang, jingjing.wu, beilei.xing, qiming.yang, dev, Shahaf Shuler, Yongseok Koh On Thu, Jul 04, 2019 at 12:46:07PM +0800, simei wrote: > From: Simei Su <simei.su@intel.com> > > Currently, there are DEFAULT,TOEPLITZ and SIMPLE_XOR hash funtion. > To support symmetric hash by rte_flow RSS action, this RFC introduces > SYMMETRIC_TOEPLITZ to rte_eth_hash_function. > > Signed-off-by: Simei Su <simei.su@intel.com> > --- > lib/librte_ethdev/rte_flow.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h > index f3a8fb1..e3c4fe5 100644 > --- a/lib/librte_ethdev/rte_flow.h > +++ b/lib/librte_ethdev/rte_flow.h > @@ -1744,6 +1744,7 @@ enum rte_eth_hash_function { > RTE_ETH_HASH_FUNCTION_DEFAULT = 0, > RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */ > RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */ > + RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ, /**< Symmetric TOEPLITZ */ "Symmetric TOEPLITZ" => "Symmetric Toeplitz." > RTE_ETH_HASH_FUNCTION_MAX, > }; Other than that, no problem with this change (no ABI impact, no need for deprecation). Please update testpmd a part of the same patch: - Wherever "toeplitz" is mentioned in test-pmd/cmdline.c. - Ditto for flow command, i.e. add ACTION_RSS_FUNC_SYMMETRIC_TOEPLITZ to test-pmd/cmdline_flow.c. - Update "set_hash_global_config" documentation section in testpmd_app_ug/testpmd_funcs.rst. Note to Shahaf/Yongseok, since mlx5 supports both but defaults to symmetric Toeplitz on vanilla Linux and standard Toeplitz when using OFED, how about using this chance to make the algorithm configurable as well? Thanks. -- Adrien Mazarguil 6WIND ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: support symmetric hash function 2019-07-04 9:06 ` Adrien Mazarguil @ 2019-07-04 12:48 ` Zhang, Qi Z 2019-07-04 13:19 ` Adrien Mazarguil 0 siblings, 1 reply; 4+ messages in thread From: Zhang, Qi Z @ 2019-07-04 12:48 UTC (permalink / raw) To: Adrien Mazarguil, Su, Simei Cc: Wu, Jingjing, Xing, Beilei, Yang, Qiming, dev, Shahaf Shuler, Yongseok Koh > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > Sent: Thursday, July 4, 2019 5:07 PM > To: Su, Simei <simei.su@intel.com> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; > Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; > dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>; Yongseok Koh > <yskoh@mellanox.com> > Subject: Re: [dpdk-dev] [RFC] ethdev: support symmetric hash function > > On Thu, Jul 04, 2019 at 12:46:07PM +0800, simei wrote: > > From: Simei Su <simei.su@intel.com> > > > > Currently, there are DEFAULT,TOEPLITZ and SIMPLE_XOR hash funtion. > > To support symmetric hash by rte_flow RSS action, this RFC introduces > > SYMMETRIC_TOEPLITZ to rte_eth_hash_function. > > > > Signed-off-by: Simei Su <simei.su@intel.com> > > --- > > lib/librte_ethdev/rte_flow.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/lib/librte_ethdev/rte_flow.h > > b/lib/librte_ethdev/rte_flow.h index f3a8fb1..e3c4fe5 100644 > > --- a/lib/librte_ethdev/rte_flow.h > > +++ b/lib/librte_ethdev/rte_flow.h > > @@ -1744,6 +1744,7 @@ enum rte_eth_hash_function { > > RTE_ETH_HASH_FUNCTION_DEFAULT = 0, > > RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */ > > RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */ > > + RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ, /**< Symmetric > TOEPLITZ */ > > "Symmetric TOEPLITZ" => "Symmetric Toeplitz." > > > RTE_ETH_HASH_FUNCTION_MAX, > > }; > > Other than that, no problem with this change (no ABI impact, no need for > deprecation). Please update testpmd a part of the same patch: Is it still ABI break but just with little risk? RTE_ETH_HASH_FUNCTION_MAX's value is changed anyway. Should we just remove it, if no one use it? Regards Qi > > - Wherever "toeplitz" is mentioned in test-pmd/cmdline.c. > > - Ditto for flow command, i.e. add ACTION_RSS_FUNC_SYMMETRIC_TOEPLITZ > to > test-pmd/cmdline_flow.c. > > - Update "set_hash_global_config" documentation section in > testpmd_app_ug/testpmd_funcs.rst. > > Note to Shahaf/Yongseok, since mlx5 supports both but defaults to symmetric > Toeplitz on vanilla Linux and standard Toeplitz when using OFED, how about > using this chance to make the algorithm configurable as well? > > Thanks. > > -- > Adrien Mazarguil > 6WIND ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: support symmetric hash function 2019-07-04 12:48 ` Zhang, Qi Z @ 2019-07-04 13:19 ` Adrien Mazarguil 0 siblings, 0 replies; 4+ messages in thread From: Adrien Mazarguil @ 2019-07-04 13:19 UTC (permalink / raw) To: Zhang, Qi Z Cc: Su, Simei, Wu, Jingjing, Xing, Beilei, Yang, Qiming, dev, Shahaf Shuler, Yongseok Koh On Thu, Jul 04, 2019 at 12:48:25PM +0000, Zhang, Qi Z wrote: > > > > -----Original Message----- > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > > Sent: Thursday, July 4, 2019 5:07 PM > > To: Su, Simei <simei.su@intel.com> > > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; > > Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; > > dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>; Yongseok Koh > > <yskoh@mellanox.com> > > Subject: Re: [dpdk-dev] [RFC] ethdev: support symmetric hash function > > > > On Thu, Jul 04, 2019 at 12:46:07PM +0800, simei wrote: > > > From: Simei Su <simei.su@intel.com> > > > > > > Currently, there are DEFAULT,TOEPLITZ and SIMPLE_XOR hash funtion. > > > To support symmetric hash by rte_flow RSS action, this RFC introduces > > > SYMMETRIC_TOEPLITZ to rte_eth_hash_function. > > > > > > Signed-off-by: Simei Su <simei.su@intel.com> > > > --- > > > lib/librte_ethdev/rte_flow.h | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/lib/librte_ethdev/rte_flow.h > > > b/lib/librte_ethdev/rte_flow.h index f3a8fb1..e3c4fe5 100644 > > > --- a/lib/librte_ethdev/rte_flow.h > > > +++ b/lib/librte_ethdev/rte_flow.h > > > @@ -1744,6 +1744,7 @@ enum rte_eth_hash_function { > > > RTE_ETH_HASH_FUNCTION_DEFAULT = 0, > > > RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */ > > > RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */ > > > + RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ, /**< Symmetric > > TOEPLITZ */ > > > > "Symmetric TOEPLITZ" => "Symmetric Toeplitz." > > > > > RTE_ETH_HASH_FUNCTION_MAX, > > > }; > > > > Other than that, no problem with this change (no ABI impact, no need for > > deprecation). Please update testpmd a part of the same patch: > > Is it still ABI break but just with little risk? > RTE_ETH_HASH_FUNCTION_MAX's value is changed anyway. > Should we just remove it, if no one use it? Indeed, it will update RTE_ETH_HASH_FUNCTION_MAX so you're technically right, and the fact it's unused in DPDK is doesn't mean applications are not using it for something. However for this specific case, the intent behind RTE_ETH_HASH_FUNCTION_MAX is clearly to give out the number of enum entries, applications are not supposed to use it for anything other than determining if an arbitrary integer value corresponds to a valid hash function. And this is the reason we could say it's OK ABI-wise to increase it (not ideal but acceptable): a binary application has a fixed idea of RTE_ETH_HASH_FUNCTION_MAX, it doesn't know the entries you're about to add yet. To such an application, those will exceed RTE_ETH_HASH_FUNCTION_MAX and should be rejected accordingly. A more conservative approach would be to mark RTE_ETH_HASH_FUNCTION_MAX as deprecated (in a separate patch) and schedule it for removal while adding new entries after it. Its position in the enum could be recycled once removed. If you want to remove RTE_ETH_HASH_FUNCTION_MAX directly, do it in a separate RFC/patch as it will otherwise block the rest of your submission for something like 2 releases after deprecation. It's up to you. I'm fine with any of these approaches. -- Adrien Mazarguil 6WIND ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-04 13:19 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-04 4:46 [dpdk-dev] [RFC] ethdev: support symmetric hash function simei 2019-07-04 9:06 ` Adrien Mazarguil 2019-07-04 12:48 ` Zhang, Qi Z 2019-07-04 13:19 ` Adrien Mazarguil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).