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 58616A0548; Wed, 1 Jun 2022 21:53:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C59E840689; Wed, 1 Jun 2022 21:53:10 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id DC1E44003F for ; Wed, 1 Jun 2022 21:53:08 +0200 (CEST) Received: from [192.168.1.126] (unknown [188.242.181.57]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 4B1F5B3; Wed, 1 Jun 2022 22:53:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 4B1F5B3 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1654113188; bh=CdlzDax01HU925ajUAvAK0RxyuWuFNo++UISAlVC+ho=; h=Date:Subject:To:References:From:Cc:In-Reply-To:From; b=pWTZxk/Nh4I5KD9B+iol73c9OqGQpANpffcORU7f6vB3VE9+IPAbJEmdNNokMjveo ZlRBFRRuRmm4WHjXetotjenkLRP6T/V1Mk83d1rMcQzBJxi3TebZID71XBXGQTUn/B lzh8+z/h3U5+6bBXTdPJPtp+jfysh2y5LfVR56ro= Message-ID: <108f8a9a-e3a4-d8b8-c29c-6dbdaf3340e8@oktetlabs.ru> Date: Wed, 1 Jun 2022 22:53:08 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v3 1/4] ethdev: introduce ethdev HW desc dump PI Content-Language: en-US To: "Min Hu (Connor)" , dev@dpdk.org References: <20220527023351.40577-1-humin29@huawei.com> <20220601074930.10313-1-humin29@huawei.com> <20220601074930.10313-2-humin29@huawei.com> From: Andrew Rybchenko Cc: Ferruh Yigit , Thomas Monjalon In-Reply-To: <20220601074930.10313-2-humin29@huawei.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 6/1/22 10:49, Min Hu (Connor) wrote: > Added the ethdev HW Rx desc dump API which provides functions for query > HW descriptor from device. HW descriptor info differs in different NICs. > The information demonstrates I/O process which is important for debug. > As the information is different between NICs, the new API is introduced. > > Signed-off-by: Min Hu (Connor) > --- > doc/guides/rel_notes/release_22_07.rst | 7 ++++ > lib/ethdev/ethdev_driver.h | 42 ++++++++++++++++++++++++ > lib/ethdev/rte_ethdev.c | 44 ++++++++++++++++++++++++++ > lib/ethdev/rte_ethdev.h | 44 ++++++++++++++++++++++++++ > lib/ethdev/version.map | 2 ++ > 5 files changed, 139 insertions(+) > > diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst > index 8932a1d478..56c675121a 100644 > --- a/doc/guides/rel_notes/release_22_07.rst > +++ b/doc/guides/rel_notes/release_22_07.rst > @@ -137,6 +137,13 @@ New Features > * ``RTE_EVENT_QUEUE_ATTR_WEIGHT`` > * ``RTE_EVENT_QUEUE_ATTR_AFFINITY`` > > +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.** > + > + Added the ethdev HW Rx desc dump API which provides functions for query > + HW descriptor from device. HW descriptor info differs in different NICs. > + The information demonstrates I/O process which is important for debug. > + As the information is different between NICs, the new API is introduced. > + > > Removed Items > ------------- > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > index 69d9dc21d8..9c1726eb2d 100644 > --- a/lib/ethdev/ethdev_driver.h > +++ b/lib/ethdev/ethdev_driver.h > @@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev, > */ > typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file); > > +/** > + * @internal > + * Dump ethdev Rx descriptor info to a file. ethdev is not required in the description above. It is an ethdev operation type. Is there any limitations on caller context? Should be it the same CPU core (since typically it is the case for per-queue API) which polls the queue? The answer must be in the API function description. > + * > + * @param file > + * A pointer to a file for output. > + * @param dev > + * Port (ethdev) handle. > + * @param queue_id > + * The selected queue. > + * @param desc_id > + * The selected descriptor. Is it an ID in the ring regardless of the current position or is it offset relative to current position in the ring? It should be clarified in any case. I'd say that it should be an offset to be consistent with descriptor status API. It would be useful to be able to dump many descriptor at once. > + * @return > + * Negative errno value on error, zero on success. > + */ > +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev, > + uint16_t queue_id, uint16_t desc_id); > + > +/** > + * @internal > + * Dump ethdev Tx descriptor info to a file. > + * > + * @param file > + * A pointer to a file for output. > + * @param dev > + * Port (ethdev) handle. > + * @param queue_id > + * The selected queue. > + * @param desc_id > + * The selected descriptor. > + * @return > + * Negative errno value on error, zero on success. > + */ > +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev, > + uint16_t queue_id, uint16_t desc_id); > + > /** > * @internal A structure containing the functions exported by an Ethernet driver. > */ > @@ -1283,6 +1319,12 @@ struct eth_dev_ops { > > /** Dump private info from device */ > eth_dev_priv_dump_t eth_dev_priv_dump; > + > + /** Dump ethdev Rx descriptor info */\\ It is an ethdev operations. So, 'ethdev' is not necessary in the description. > + eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump; > + > + /** Dump ethdev Tx descriptor info */ > + eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump; > }; > > /** > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > index 46c088dc88..bbd8439fa0 100644 > --- a/lib/ethdev/rte_ethdev.c > +++ b/lib/ethdev/rte_ethdev.c > @@ -5876,6 +5876,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file) > return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file)); > } > > +int > +rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id, > + uint16_t desc_id) > +{ > + struct rte_eth_dev *dev; > + int ret; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > + dev = &rte_eth_devices[port_id]; Shouldn't we check file vs null? > + > + if (queue_id >= dev->data->nb_rx_queues) { > + RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id); > + return -EINVAL; > + } Don't we need a check vs queue size? > + > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP); > + ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id, > + desc_id); > + > + return ret; > +}