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 E3C4742B89; Wed, 24 May 2023 09:38:12 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7DA2C40ED8; Wed, 24 May 2023 09:38:12 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id 4BCFD4067E for ; Wed, 24 May 2023 09:38:10 +0200 (CEST) Received: from dggpeml500011.china.huawei.com (unknown [172.30.72.54]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4QR30Q2WNRzLmPq; Wed, 24 May 2023 15:36:38 +0800 (CST) Received: from [10.67.101.191] (10.67.101.191) by dggpeml500011.china.huawei.com (7.185.36.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Wed, 24 May 2023 15:38:06 +0800 Message-ID: Date: Wed, 24 May 2023 15:38:06 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v2 1/2] ethdev: add API to check queue ID validity Content-Language: en-US To: Andrew Rybchenko , CC: , , , , , , , , References: <20230516110021.1801443-1-huangdengdui@huawei.com> <20230522130947.345390-1-huangdengdui@huawei.com> <20230522130947.345390-2-huangdengdui@huawei.com> From: huangdengdui In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.101.191] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggpeml500011.china.huawei.com (7.185.36.84) 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/5/22 21:58, Andrew Rybchenko wrote: > On 5/22/23 16:09, Dengdui Huang wrote: >> The API rte_eth_dev_is_valid_rxq/txq checks >> the port ID validity and then the Rx/Tx queue ID is valid. > > What is valid Tx/Rx queue? It depends on on caller > expectations. Some functions are satisfied with just > check vs configured number of queues. Some require > the queue to be setup. May be some should require > the queue to be started. > > So, I suggest to avoid term "valid" and be more precise > here and API naming. > Hi Andrew, Thanks for your review, Accepted and fixed in v3. >> >> Signed-off-by: Dengdui Huang >> --- >>   doc/guides/rel_notes/release_23_07.rst |  5 ++++ >>   lib/ethdev/rte_ethdev.c                | 30 +++++++++++++++++++++ >>   lib/ethdev/rte_ethdev.h                | 36 ++++++++++++++++++++++++++ >>   lib/ethdev/version.map                 |  4 +++ >>   4 files changed, 75 insertions(+) >> >> diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst >> index a9b1293689..19e645156f 100644 >> --- a/doc/guides/rel_notes/release_23_07.rst >> +++ b/doc/guides/rel_notes/release_23_07.rst >> @@ -56,6 +56,11 @@ New Features >>        ======================================================= >>     +* **Added ethdev Rx/Tx queue id check API.** >> + >> +  Added ethdev Rx/Tx queue id check API which provides functions > > id -> ID > >> +  for check if Rx/Tx queue id is valid. > > id -> ID > >> + > > It should be two empty lines here and just one above. > Thanks, will fix in v3 >>   Removed Items >>   ------------- >>   diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >> index 4d03255683..3d85218127 100644 >> --- a/lib/ethdev/rte_ethdev.c >> +++ b/lib/ethdev/rte_ethdev.c >> @@ -407,6 +407,36 @@ rte_eth_dev_is_valid_port(uint16_t port_id) >>       return is_valid; >>   } >>   +int >> +rte_eth_dev_is_valid_rxq(uint16_t port_id, uint16_t queue_id) >> +{ >> +    struct rte_eth_dev *dev; >> + >> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> +    dev = &rte_eth_devices[port_id]; >> + >> +    if (queue_id >= dev->data->nb_rx_queues || >> +            dev->data->rx_queues[queue_id] == NULL) > > We already have internal eth_dev_validate_tx_queue(). Shouldn't > it be used here? > Thanks, will do in v3. > Also, some functions check that queues array is not NULL. > If the the is excessive after queue number check, it > should be consistent everywhere and corresponding check > of the array pointer vs NULL should be removed in a separate > cleanup patch. If the check is required in some corner cases > (I hope no), it should be here as well. > I cannot understand what you mean by that. >> +        return -EINVAL; >> + >> +    return 0; >> +} > > [snip] >