From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2EAD443269; Thu, 2 Nov 2023 07:58:52 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D66614029E; Thu, 2 Nov 2023 07:58:51 +0100 (CET) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id 2642740282 for ; Thu, 2 Nov 2023 07:58:50 +0100 (CET) Received: from kwepemi500020.china.huawei.com (unknown [172.30.72.53]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4SLZNx0K5hzMlqJ; Thu, 2 Nov 2023 14:54:25 +0800 (CST) Received: from [10.67.121.175] (10.67.121.175) by kwepemi500020.china.huawei.com (7.221.188.8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Thu, 2 Nov 2023 14:58:46 +0800 Message-ID: <458c2e2d-2c62-c381-d51b-78fe98c2b7fe@huawei.com> Date: Thu, 2 Nov 2023 14:58:45 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH v8 03/10] ethdev: support setting and querying RSS algorithm To: Ferruh Yigit , , Thomas Monjalon , Andrew Rybchenko , Ori Kam CC: , , References: <20230315110033.30143-1-liudongdong3@huawei.com> <20231101074039.3088716-1-haijie1@huawei.com> <20231101074039.3088716-4-haijie1@huawei.com> <508c5f6d-7afa-4c78-a6d8-9e9e66917e77@amd.com> From: Jie Hai In-Reply-To: <508c5f6d-7afa-4c78-a6d8-9e9e66917e77@amd.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.175] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemi500020.china.huawei.com (7.221.188.8) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2023/11/1 21:36, Ferruh Yigit wrote: > On 11/1/2023 7:40 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_dev_info`` is extended by adding a new >> field "rss_algo_capa". Drivers are responsible for reporting this >> capa and configurations of RSS hash algorithm can be verified based >> on the capability. The default value of "rss_algo_capa" is >> RTE_ETH_HASH_ALGO_CAPA_MASK(DEFAULT) if drivers do not report it. >> >> The structure ``rte_eth_rss_conf`` is extended by adding a new >> field "algorithm". This represents the RSS algorithms to apply. >> If the value of "algorithm" used for configuration is a gibberish >> value, drivers should report the error. >> >> To check whether the drivers report valid "algorithm", it is set >> to default value before querying in rte_eth_dev_rss_hash_conf_get(). >> >> Signed-off-by: Jie Hai >> Signed-off-by: Dongdong Liu >> Acked-by: Huisong Li >> --- >> doc/guides/rel_notes/release_23_11.rst | 5 +++++ >> lib/ethdev/rte_ethdev.c | 26 +++++++++++++++++++++++ >> lib/ethdev/rte_ethdev.h | 29 ++++++++++++++++++++++++++ >> lib/ethdev/rte_flow.c | 1 - >> lib/ethdev/rte_flow.h | 26 ++--------------------- >> 5 files changed, 62 insertions(+), 25 deletions(-) >> >> diff --git a/doc/guides/rel_notes/release_23_11.rst b/doc/guides/rel_notes/release_23_11.rst >> index 95db98d098d8..e207786044f9 100644 >> --- a/doc/guides/rel_notes/release_23_11.rst >> +++ b/doc/guides/rel_notes/release_23_11.rst >> @@ -372,6 +372,11 @@ ABI Changes >> * security: struct ``rte_security_ipsec_sa_options`` was updated >> due to inline out-of-place feature addition. >> >> +* ethdev: Added "rss_algo_capa" field to ``rte_eth_dev_info`` structure for >> +* reporting RSS hash algorithm capability. >> + >> +* ethdev: Added "algorithm" field to ``rte_eth_rss_conf`` structure for RSS >> + hash algorithm. >> > > As well as ABI change, can you also update the "New Features", to > document getting hash algorithm capability and setting hash algorithm > support added? > > Also please add an empty line here. thanks，will add. > >> Known Issues >> ------------ >> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >> index 07bb35833ba6..f9bd99d07eb1 100644 >> --- a/lib/ethdev/rte_ethdev.c >> +++ b/lib/ethdev/rte_ethdev.c >> @@ -1269,6 +1269,7 @@ int >> rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, >> const struct rte_eth_conf *dev_conf) >> { >> + enum rte_eth_hash_function algorithm; >> struct rte_eth_dev *dev; >> struct rte_eth_dev_info dev_info; >> struct rte_eth_conf orig_conf; >> @@ -1510,6 +1511,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, >> goto rollback; >> } >> >> + algorithm = dev_conf->rx_adv_conf.rss_conf.algorithm; >> + if (RTE_ETH_HASH_ALGO_TO_CAPA(algorithm) == 0 || >> > > "RTE_ETH_HASH_ALGO_TO_CAPA(algorithm)" can't be zero for valid "enum > rte_eth_hash_function" values, I assume above check is for the case > algorith > 31, as it will result zero. > My concern is, this is undefined behaviour (shift left >= 32) and some > compiler can complain about it, instead of relying this can you please > add explicit "0 <= algorithm < 32" check? yes, how about associate with "rss_algo_capa"? + if (algorithm >= CHAR_BIT * sizeof(dev_info.rss_algo_capa) || + (dev_info.rss_algo_capa & RTE_ETH_HASH_ALGO_TO_CAPA(algorithm)) == 0) { > > > > .