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 1EFB1A0C47; Mon, 26 Jul 2021 12:13:04 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 603AB410EA; Mon, 26 Jul 2021 12:13:03 +0200 (CEST) Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) by mails.dpdk.org (Postfix) with ESMTP id E91DA40F35 for ; Mon, 26 Jul 2021 12:13:01 +0200 (CEST) Received: by mail-wr1-f52.google.com with SMTP id h14so2439433wrx.10 for ; Mon, 26 Jul 2021 03:13:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=E76eJiAEm70wsSwKSByK03EkQ7RtnG6753GrkU6mFgU=; b=IpDDGP/UPnsUm2tt/8EKnU11gQw3s5AubNTrSaROCmHvSBiJnWOk2BV0yE7ppM3N5I 4u3dWFqeeAnYi2xRZW2v84xLJcr2K1uUnJFuL0pCqWKJtkX6VdglhJllBuYgFwAZrD4/ 0KBfDi/CFdv9cD5cSkr7YcAiU0z39i0gJYjrMy/OJiPPHsT3ro4HJw/bqldH0/hP9opC e+1/b0BiLQEOtnaPInOwzhnpTMmimh8V+h/OJsyp2Y1DmytDazC3oAuntV4tvEwVyNv4 PkuyLOT1ehBmLl3HhG9QjtNsyljwEjrAlXEi+UB3Kwy6CLwqw89fspP+xV3LVwYAdb73 tPfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=E76eJiAEm70wsSwKSByK03EkQ7RtnG6753GrkU6mFgU=; b=N0h07knwXJLh+7EnGOmos6QyYHEdOPEtIGPiJQzLK/qY5kJPU6L7QBJsxJGl4893ho RAfS5qXnJ1DbVd/ThgMNg+dhk87PfmYb+Q+wB6/nVRBhFM8qPFwjDO+6gpvg83ndUnB0 kv2geYnsqwFikAb+hJH0+rsZNy0IWLnKA9b1r5gkMGZUnncgaRSLdZlvNUSQUE241N2a zv3dggv5+jwNVUuIQWSWwyIKp+Q7+5kJaLtz1pAZa95IU53ZRq4Nc+2H1oV8XunKfMB0 tYhsTPgl6tQca5ymiAoavXqGtOUkCMl2xlYP23rqJRzJJEV7Lo197hpnLFXJVTpL87HV c3iA== X-Gm-Message-State: AOAM531iNZ1bU+v6rRAMINAEkOvCmos3EU6i3/anKlkyYx5SB+Q5nOCs Tg1xPAcrEM8lbtScoELc52e71w== X-Google-Smtp-Source: ABdhPJyfqUHhKGAScaifsGjYDKA998LqO7M5T7cehzTwcMqv94yLR/XIK8PndC1bT12RmhmNsGA+HA== X-Received: by 2002:a5d:4b42:: with SMTP id w2mr18143114wrs.47.1627294381651; Mon, 26 Jul 2021 03:13:01 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id f17sm23879624wrr.81.2021.07.26.03.13.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Jul 2021 03:13:01 -0700 (PDT) Date: Mon, 26 Jul 2021 12:13:00 +0200 From: Olivier Matz To: Andrew Rybchenko Cc: dev@dpdk.org, Thomas Monjalon , Ferruh Yigit , Kuba Kozak , Ivan Ilchenko , stable@dpdk.org, Andy Moreton Message-ID: References: <20210604144225.287678-1-andrew.rybchenko@oktetlabs.ru> <20210724123314.2970537-1-andrew.rybchenko@oktetlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210724123314.2970537-1-andrew.rybchenko@oktetlabs.ru> Subject: Re: [dpdk-dev] [PATCH v4 1/2] ethdev: fix docs of functions getting xstats by IDs 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 Sender: "dev" Hi Andrew, Some comments below. On Sat, Jul 24, 2021 at 03:33:13PM +0300, Andrew Rybchenko wrote: > From: Ivan Ilchenko > > Document valid combinations of input arguments in accordance with > current implementation in ethdev. > > 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 > --- > lib/ethdev/rte_ethdev.h | 32 +++++++++++++++++++------------- > 1 file changed, 19 insertions(+), 13 deletions(-) > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > index d2b27c351f..b14067fe7e 100644 > --- a/lib/ethdev/rte_ethdev.h > +++ b/lib/ethdev/rte_ethdev.h > @@ -2872,13 +2872,17 @@ int rte_eth_xstats_get(uint16_t port_id, struct rte_eth_xstat *xstats, > * @param port_id > * The port identifier of the Ethernet device. > * @param xstats_names > - * An rte_eth_xstat_name array of at least *size* elements to > - * be filled. If set to NULL, the function returns the required number > - * of elements. > + * Array to be filled in with names of requested device statistics. > + * Must not be NULL if @p ids are specified (not NULL). > * @param ids > - * IDs array given by app to retrieve specific statistics > + * IDs array given by app to retrieve specific statistics. May be NULL to > + * retrieve names of all available statistics or, if @p xstats_names is > + * NULL as well, just a number of available statistics. double spaces before "just" "a number" -> "the number"? > * @param size > - * The size of the xstats_names array (number of elements). > + * If @p ids is not NULL, number of elements in the array with requested IDs > + * and number of elements in @p xstats_names to put names in. If @p ids is > + * NULL, number of elements in @p xstats_names to put all available statistics > + * names in. Just a suggestion here, I feel the following description would be clearer: Number of elements in @p xstats_names array (if not NULL) and in @p ids array (if not NULL). Shouldn't we say that it has to be 0 if both arrays are NULL? Also, the order of arguments is not the same in comment and in the function. I think it can make sense to align the comment to the prototype. > * @return > * - A positive value lower or equal to size: success. The return value > * is the number of entries filled in the stats table. Not seen in the patch, but right after this line, there is: * - A positive value higher than size: error, the given statistics table * is too small. The return value corresponds to the size that should * be given to succeed. The entries in the table are not valid and * shall not be used by the caller. I wonder if it shouldn't be slighly reworded to remove 'error'. After all, passing NULL arrays (and size == 0) is a valid, so the return is not an error. > @@ -2886,7 +2890,7 @@ int rte_eth_xstats_get(uint16_t port_id, struct rte_eth_xstat *xstats, > * is too small. The return value corresponds to the size that should > * be given to succeed. The entries in the table are not valid and > * shall not be used by the caller. > - * - A negative value on error (invalid port id). > + * - A negative value on error. > */ > int > rte_eth_xstats_get_names_by_id(uint16_t port_id, > @@ -2899,14 +2903,16 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id, > * @param port_id > * The port identifier of the Ethernet device. > * @param ids > - * A pointer to an ids array passed by application. This tells which > - * statistics values function should retrieve. This parameter > - * can be set to NULL if size is 0. In this case function will retrieve > - * all available statistics. > + * IDs array given by app to retrieve specific statistics. May be NULL to > + * retrieve all available statistics or, if @p values is NULL as well, > + * just a number of available statistics. > * @param values > - * A pointer to a table to be filled with device statistics values. > + * Array to be filled in with requested device statistics. > + * Must not be NULL if ids are specified (not NULL). > * @param size > - * The size of the ids array (number of elements). > + * If @p ids is not NULL, number of elements in the array with requested IDs > + * and number of elements in @p values to put statistics in. If @p ids is NULL, > + * number of elements in @p values to put all available statistics in. > * @return > * - A positive value lower or equal to size: success. The return value > * is the number of entries filled in the stats table. > @@ -2914,7 +2920,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id, > * is too small. The return value corresponds to the size that should > * be given to succeed. The entries in the table are not valid and > * shall not be used by the caller. > - * - A negative value on error (invalid port id). > + * - A negative value on error. > */ Some of the comments above also apply here. > int rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, > uint64_t *values, unsigned int size); > -- > 2.30.2 >