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 056C1A0C51 for ; Fri, 23 Jul 2021 20:47:42 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E63AC40E6E; Fri, 23 Jul 2021 20:47:41 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 0AF1B406A3; Fri, 23 Jul 2021 20:47:40 +0200 (CEST) Received: from [192.168.100.116] (unknown [37.139.99.76]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id EA4717F4FD; Fri, 23 Jul 2021 21:47:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru EA4717F4FD DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1627066059; bh=wutDVLHVQCB+3z1xAmbeaqiis2KNFQA/KZHlGJXmJkA=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=FihgjEhfZXzTttNzU37kSYwN+9nDYiXADsZClqQRke6H56c3QpCJZk8P8s14CyWxo mObfHlbhMZG87lWZAKpX42N5cCLSULCLXUdKbSmlgMpDAp2cOOPpUYLLplLTs7gbaJ GGGYLdNugpgknPgq4RT+feyp3gpc6we3kWkO5SIc= To: Ferruh Yigit , dev@dpdk.org Cc: Ivan Ilchenko , stable@dpdk.org, Andy Moreton , Thomas Monjalon , Kuba Kozak References: <20210604144225.287678-1-andrew.rybchenko@oktetlabs.ru> <20210604144225.287678-5-andrew.rybchenko@oktetlabs.ru> <16f738a7-28b7-bd41-cfd2-60f35f868c32@intel.com> <7b25c75c-6a4a-fd51-09e9-af9ecac1a7cf@oktetlabs.ru> <2601474d-1cc7-c819-e950-43bcbbf548b7@intel.com> From: Andrew Rybchenko Message-ID: <4c0a892b-ed33-4f90-7ecc-6af5ed754e76@oktetlabs.ru> Date: Fri, 23 Jul 2021 21:47:37 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <2601474d-1cc7-c819-e950-43bcbbf548b7@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-stable] [PATCH 04/11] ethdev: fix docs of drivers callbacks getting xstats by IDs X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 7/23/21 5:31 PM, Ferruh Yigit wrote: > On 7/22/2021 10:33 AM, Andrew Rybchenko wrote: >> On 7/20/21 7:51 PM, Ferruh Yigit wrote: >>> On 6/4/2021 3:42 PM, Andrew Rybchenko wrote: >>>> From: Ivan Ilchenko >>>> >>>> Update xstats by IDs callbacks documentation in accordance with >>>> ethdev usage of these callbacks. Document valid combinations of >>>> input arguments to make driver implementation simpler. >>>> >>>> Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID") >>>> Cc: stable@dpdk.org >>>> >>>> Signed-off-by: Ivan Ilchenko >>>> Signed-off-by: Andrew Rybchenko >>>> Reviewed-by: Andy Moreton [snip] >>>> + * @param dev >>>> + *   ethdev handle of port. >>>> + * @param xstats_names >>>> + *   An rte_eth_xstat_name array of at least *size* elements to >>>> + *   be filled. Can be NULL together with @p ids to retrieve number of >>>> + *   available statistics. >>> >>> As far as I understand both _by_id APIs and devops behave same, so argument >>> descriptions/behavior should be same. >> >> In fact no, it is slightly different. For example, devops is never >> called with NULL ids and not NULL names or non-zero size. It allows to >> check less in drivers. >> > > I am not sure if this difference is intentional. What we're trying to do here is to document existing usage of these callbacks to more strictly define valid input parameters to allow drivers to support less combinations (to make implementation simpler). So, intentional. If you're trying to say that API functions could handle less variants, the answer is simple - it could be discussed, but out of scope of the patch series since it will be API change. > 'eth_dev_get_xstats_count()' is ethdev internal function, what do you think > removing 'xstats_get_names_by_id()' call from it. And make both _by_id() dev_ops > doesn't accept NULL ids and NULL values/names, so simplifies PMD implementations. > In fact the function eth_dev_get_xstats_count() looks buggy since it does not use xstats_get_names_by_id positive result if xstats_get_names is not NULL. Really confusing. Again, it will be slight change on PMD requirements. May be it is OK to do, but I'd conduct it separately since it is logically a separate thing. Current patches just clarify documentation.