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 ED3CF43268; Thu, 2 Nov 2023 04:41:00 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7F27340294; Thu, 2 Nov 2023 04:41:00 +0100 (CET) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 1EE3E4028C for ; Thu, 2 Nov 2023 04:40:58 +0100 (CET) Received: from kwepemi500020.china.huawei.com (unknown [172.30.72.55]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4SLV5c0M4XzVjTx; Thu, 2 Nov 2023 11:40:52 +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 11:40:54 +0800 Message-ID: Date: Thu, 2 Nov 2023 11:40:53 +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 02/10] lib/ethdev: check RSS key length To: Ferruh Yigit , , Thomas Monjalon , Andrew Rybchenko CC: , , References: <20230315110033.30143-1-liudongdong3@huawei.com> <20231101074039.3088716-1-haijie1@huawei.com> <20231101074039.3088716-3-haijie1@huawei.com> From: Jie Hai In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.175] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) 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:19, Ferruh Yigit wrote: > On 11/1/2023 7:40 AM, Jie Hai wrote: >> In rte_eth_dev_rss_hash_conf_get(), the "rss_key_len" should be >> greater than or equal to the "hash_key_size" which get from >> rte_eth_dev_info_get() API. And the "rss_key" should contain at >> least "hash_key_size" bytes. If these requirements are not met, >> the query unreliable. >> >> In rte_eth_dev_rss_hash_update() or rte_eth_dev_configure(), the >> "rss_key_len" indicates the length of the "rss_key" in bytes of >> the array pointed by "rss_key", it should be equal to the >> "hash_key_size" if "rss_key" is not NULL. >> >> This patch checks "rss_key_len" in ethdev level. >> > > Can you please squash this patch and previous one, previous one > clarifies the API and this one adds relevant checks, so they con be in > some patch. > > Can you also please update release notes, 'API Changes', explaining > 'rss_conf.rss_key_len' needs to be provided by user for the case > "conf.rss_key != NULL", it won't be taken as default 40 bytes anymore. > Thanks. Will update. > >> Signed-off-by: Jie Hai >> --- >> lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >> index af23ac0ad00f..07bb35833ba6 100644 >> --- a/lib/ethdev/rte_ethdev.c >> +++ b/lib/ethdev/rte_ethdev.c >> @@ -1500,6 +1500,16 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, >> goto rollback; >> } >> >> + if (dev_conf->rx_adv_conf.rss_conf.rss_key != NULL && >> + dev_conf->rx_adv_conf.rss_conf.rss_key_len < dev_info.hash_key_size) { >> > > Why check is "rss_key_len < dev_info.hash_key_size", is it allowed to > have "rss_key_len > dev_info.hash_key_size"? > > Shouldn't it enforce that "rss_key_len == dev_info.hash_key_size"? > > Yes, should be equal, will correct. >> + RTE_ETHDEV_LOG(ERR, >> + "Ethdev port_id=%u invalid RSS key len: %u, valid value: %u\n", >> + port_id, dev_conf->rx_adv_conf.rss_conf.rss_key_len, >> + dev_info.hash_key_size); >> + ret = -EINVAL; >> + goto rollback; >> + } >> + >> /* >> * Setup new number of Rx/Tx queues and reconfigure device. >> */ >> @@ -4698,6 +4708,14 @@ rte_eth_dev_rss_hash_update(uint16_t port_id, >> return -ENOTSUP; >> } >> >> + if (rss_conf->rss_key != NULL && >> + rss_conf->rss_key_len != dev_info.hash_key_size) { >> + RTE_ETHDEV_LOG(ERR, >> + "Ethdev port_id=%u invalid RSS key len: %u, valid value: %u\n", >> + port_id, rss_conf->rss_key_len, dev_info.hash_key_size); >> + return -EINVAL; >> + } >> + >> if (*dev->dev_ops->rss_hash_update == NULL) >> return -ENOTSUP; >> ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev, >> @@ -4712,6 +4730,7 @@ int >> rte_eth_dev_rss_hash_conf_get(uint16_t port_id, >> struct rte_eth_rss_conf *rss_conf) >> { >> + struct rte_eth_dev_info dev_info = { 0 }; >> struct rte_eth_dev *dev; >> int ret; >> >> @@ -4725,6 +4744,18 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id, >> return -EINVAL; >> } >> >> + ret = rte_eth_dev_info_get(port_id, &dev_info); >> + if (ret != 0) >> + return ret; >> + >> + if (rss_conf->rss_key != NULL && >> + rss_conf->rss_key_len < dev_info.hash_key_size) { >> + RTE_ETHDEV_LOG(ERR, >> + "Ethdev port_id=%u invalid RSS key len: %u, should not be less than: %u\n", >> + port_id, rss_conf->rss_key_len, dev_info.hash_key_size); >> + return -EINVAL; >> + } >> + >> if (*dev->dev_ops->rss_hash_conf_get == NULL) >> return -ENOTSUP; >> ret = eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev, > > .