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 9535C424EC; Mon, 4 Sep 2023 09:10:56 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 702E0402AE; Mon, 4 Sep 2023 09:10:56 +0200 (CEST) Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by mails.dpdk.org (Postfix) with ESMTP id CB453402A9 for ; Mon, 4 Sep 2023 09:10:54 +0200 (CEST) Received: from kwepemi500020.china.huawei.com (unknown [172.30.72.53]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4RfKW739RMz1M96W; Mon, 4 Sep 2023 15:09:07 +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; Mon, 4 Sep 2023 15:10:51 +0800 Message-ID: <9abeba8a-4596-6e69-ded1-59d7a518b3d7@huawei.com> Date: Mon, 4 Sep 2023 15:10:51 +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 v2 1/5] ethdev: support setting and querying RSS algorithm To: Thomas Monjalon CC: , Ferruh Yigit , Andrew Rybchenko , , , , References: <20230315110033.30143-1-liudongdong3@huawei.com> <20230826074607.16771-1-haijie1@huawei.com> <20230826074607.16771-2-haijie1@huawei.com> <1939253.PYKUYFuaPT@thomas> From: Jie Hai In-Reply-To: <1939253.PYKUYFuaPT@thomas> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.175] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) 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 Hi, Thomas Thanks for your review. On 2023/8/30 19:46, Thomas Monjalon wrote: > Hello, > > Thanks for bringing a new capability. > > 26/08/2023 09:46, Jie Hai: >> 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 is affected: >> - rte_eth_dev_configure >> - rte_eth_dev_rss_hash_update >> - rte_eth_dev_rss_hash_conf_get > > So far, the RSS algorithm was used only in flow RSS API. > Fixed in V3, these API will be affected. >> --- a/doc/guides/rel_notes/release_23_11.rst >> +++ b/doc/guides/rel_notes/release_23_11.rst >> @@ -123,6 +123,8 @@ ABI Changes >> Also, make sure to start the actual text at the margin. >> ======================================================= >> >> + * ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash >> + algorithm. > > As written above, it should start at the margin. > Fixed in V3. >> --- a/lib/ethdev/rte_ethdev.h >> +++ b/lib/ethdev/rte_ethdev.h >> +#include "rte_flow.h" > > It is strange to include rte_flow.h here. > It would be better to move the enum. > Fixed in V3, the definations are removed to rte_ethdev.h. >> + * 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. >> 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. > > . Thanks, Jie Hai