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 C7B0DA00C2; Thu, 6 Oct 2022 10:26:29 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6BEF342B98; Thu, 6 Oct 2022 10:26:29 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id 91BE142B88 for ; Thu, 6 Oct 2022 10:26:27 +0200 (CEST) Received: from kwepemi500017.china.huawei.com (unknown [172.30.72.54]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4MjkxC5z26zJn1d; Thu, 6 Oct 2022 16:23:59 +0800 (CST) Received: from [10.67.103.235] (10.67.103.235) by kwepemi500017.china.huawei.com (7.221.188.110) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Thu, 6 Oct 2022 16:26:24 +0800 Subject: Re: [PATCH v4 1/3] ethdev: introduce ethdev desc dump API To: Ferruh Yigit , , , , , , References: <20220527023351.40577-1-humin29@huawei.com> <20220923074316.25077-1-liudongdong3@huawei.com> <20220923074316.25077-2-liudongdong3@huawei.com> <32e4a2fe-e75f-e832-278c-651d5081324a@amd.com> CC: "Min Hu (Connor)" From: Dongdong Liu Message-ID: <4c4cd3ea-ef32-02fd-1070-7c17fdc4d0c0@huawei.com> Date: Thu, 6 Oct 2022 16:26:24 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <32e4a2fe-e75f-e832-278c-651d5081324a@amd.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.103.235] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To kwepemi500017.china.huawei.com (7.221.188.110) 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 Ferruh Many thanks for your review. On 2022/10/4 6:40, Ferruh Yigit wrote: > On 9/23/2022 8:43 AM, Dongdong Liu wrote: > >> >> From: "Min Hu (Connor)" >> >> Added the ethdev Rx/Tx desc dump API which provides functions for query >> 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 > > Overall OK to have these new APIs, please find comments below. > > Do you think does it worth to list this as one of the PMD future in > future list, in 'doc/guides/nics/features.rst' ? As Andrew said It does not deserve entry in features. It is a deep debugging using vendor-specific information. I agree with him. > > between NICs, the new API is introduced. >> >> Signed-off-by: Min Hu (Connor) >> Signed-off-by: Dongdong Liu >> Acked-by: Ray Kinsella > > <...> > >> int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file); >> >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change, or be removed, without prior >> notice >> + * >> + * Dump ethdev Rx descriptor info to a file. >> + * >> + * This API is used for debugging, not a dataplane API. >> + * >> + * @param file >> + * A pointer to a file for output. >> + * @param dev >> + * Port (ethdev) handle. >> + * @param queue_id >> + * The selected queue. >> + * @param num >> + * The number of the descriptors to dump. >> + * @return >> + * - On success, zero. >> + * - On failure, a negative value. >> + */ >> +__rte_experimental >> +int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t >> queue_id, >> + uint16_t num); > > There are other HW desc related APIs, like > 'rte_eth_rx_descriptor_status()'. > Should this APIs follow same naming convention: > 'rte_eth_rx_descriptor_dump()' > 'rte_eth_tx_descriptor_dump()' Looks good, will do. > >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change, or be removed, without prior >> notice >> + * >> + * Dump ethdev Tx descriptor info to a file. >> + * >> + * This API is used for debugging, not a dataplane API. >> + * >> + * @param file >> + * A pointer to a file for output. >> + * @param dev >> + * Port (ethdev) handle. >> + * @param queue_id >> + * The selected queue. >> + * @param num >> + * The number of the descriptors to dump. >> + * @return >> + * - On success, zero. >> + * - On failure, a negative value. >> + */ >> +__rte_experimental >> +int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t >> queue_id, >> + uint16_t num); > > 'num' is provided, does it assume it starts from offset 0, what do you > think to provide 'offset' as parameter? > It may be a use case to start from where tail/head pointer is. It will be ok to provide 'offset' as parameter. will change as below. /** * @warning * @b EXPERIMENTAL: this API may change, or be removed, without prior notice * * Dump ethdev Rx descriptor info to a file. * * This API is used for debugging, not a dataplane API. * * @param port_id * The port identifier of the Ethernet device. * @param queue_id * A Rx queue identifier on this port. * @param offset * The offset of the descriptor starting from tail. (0 is the next * packet to be received by the driver). * @param num * The number of the descriptors to dump. * @param file * A pointer to a file for output. * @return * - On success, zero. * - On failure, a negative value. */ __rte_experimental int rte_eth_rx_descriptor_dump(uint16_t port_id, uint16_t queue_id, uint16_t offset, uint16_t num, FILE *file); > >> + >> + >> #include >> >> /** >> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map >> index 03f52fee91..3c7c75b582 100644 >> --- a/lib/ethdev/version.map >> +++ b/lib/ethdev/version.map >> @@ -285,6 +285,8 @@ EXPERIMENTAL { >> rte_mtr_color_in_protocol_priority_get; >> rte_mtr_color_in_protocol_set; >> rte_mtr_meter_vlan_table_update; >> + rte_eth_rx_hw_desc_dump; >> + rte_eth_tx_hw_desc_dump; > > These new APIs should go after "# added in 22.11" comment, if you rebase > on top of latest HEAD, comment is already there. Will rebase on top of latest HEAD. Thanks, Dongdong > > . >