From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id 9E80C29B6 for ; Wed, 23 Mar 2016 11:20:54 +0100 (CET) Received: by mail-wm0-f54.google.com with SMTP id l68so188699742wml.0 for ; Wed, 23 Mar 2016 03:20:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:organization:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=yrKapUunZK7x41uMQhUhcjMhdRi41awJeinPsUosKbA=; b=s5Md1rkPZxf7UIxaRhg43ZegPiHFnoXFxT/GtD8BkWO35enarCkueh26B2a/bTJ9c3 +BRsvC3xXsdiFT8TumIcydrj+rGwBzQeWatWfHU4O4yfNNc9eAWWrUhsIFvjZvij6xOX NycY+aiTCnpv4/SYfDQTj823F7FxH4IiAZPfX1Cvhe94gxA5KFsL+/UXqJii0YnYj2Je 5CrX2WRHXt/iDZ/7QQTwQTeWxMUj++2co2sRTuo1pM61UEE/GKzHEYKGuuf21tKhjOfN 0qvg0Arobi2zi5WuEw5iTF1DjpkCiRlLNLx2kZNEz6t5bP4iQn+hIDfYHi4adSQRNrhj 1eqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding; bh=yrKapUunZK7x41uMQhUhcjMhdRi41awJeinPsUosKbA=; b=dVjD7eQar5ZPtEBUnalNLKjTmlSvyHGFwg+oko8iW/guTEwOw87GVZizXQ6/kidqkc oZ/JOKbJGaTxmHNb++xIgT7wpzWPBYvtr668CGmDKKx/rlbcbYs/OHiP+dlMd38uOeUB SX71qiWJOmfP5W+xp6YC8Ty/0qF7bQanK3Rp8m45WI/E47RhLFDVbGnhSdknkxXuaoZS cSfoIeG8iQbPa08PMnEZpKZ1DitpCbTFQUA3/RVU1euxyxsOnNdoLxNAyeWlUyj2bpPX LcjdB8shIO1nmzKKmFnKME/2OfsfSQOuqEJyNimECd+YYcTq5UuB5aZVou0CvW3GFK62 IfGQ== X-Gm-Message-State: AD7BkJIXXWN9YLqaH6WyHsYkluYru0D1DsDnKQhZBtJA7Wjhx7c8SSErxlhz7VCtXTxYi9j1 X-Received: by 10.194.187.240 with SMTP id fv16mr2762165wjc.53.1458728454504; Wed, 23 Mar 2016 03:20:54 -0700 (PDT) Received: from xps13.localnet (91.111.75.86.rev.sfr.net. [86.75.111.91]) by smtp.gmail.com with ESMTPSA id m6sm1822932wje.21.2016.03.23.03.20.53 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 23 Mar 2016 03:20:53 -0700 (PDT) From: Thomas Monjalon To: Stephen Hemminger Cc: dev@dpdk.org, Olivier Matz Date: Wed, 23 Mar 2016 11:19:12 +0100 Message-ID: <2833753.Klr9nq0nAo@xps13> Organization: 6WIND User-Agent: KMail/4.14.10 (Linux/4.1.6-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <56F25925.60405@6wind.com> References: <1458684557-15378-1-git-send-email-stephen@networkplumber.org> <56F25925.60405@6wind.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] ethdev: fix rte_eth_dev_xstats_get with NULL X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Mar 2016 10:20:54 -0000 2016-03-23 09:51, Olivier Matz: > On 03/22/2016 11:09 PM, Stephen Hemminger wrote: > > Normal usage of rte_eth_dev_xstats_get is to call twice. The > > first time the function is called with portid, xstats = NULL > > and n = 0; this returns the number of entries in the statistics > > table that need to be allocated. > > > > The problem is that the routine adds a count value to NULL (0) > > and assumes that this is a valid pointer (it isn't). Device drivers > > all have a check for NULL, and this no longer matches. This check for NULL should be done after the check (n == 0). > > Bug was introduced by commit d4fef8b0d5e5 > > ("ethdev: expose generic and driver specific stats in xstats") > > > > Signed-off-by: Stephen Hemminger [...] > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -1495,8 +1495,9 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats, > > /* Retrieve the xstats from the driver at the end of the > > * xstats struct. > > */ > > - xcount = (*dev->dev_ops->xstats_get)(dev, &xstats[count], > > - (n > count) ? n - count : 0); > > + xcount = (*dev->dev_ops->xstats_get)(dev, > > + xstats ? xstats + count : NULL, > > + (n > count) ? n - count : 0); > > > > if (xcount < 0) > > return xcount; > > Acked-by: Olivier Matz > > Just one comment: I think the driver should not rely on the pointer > value, but on the count. From the documentation of rte_eth_xstats_get(), > the function returns a: > > "positive value higher than n: 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." > > So maybe it should be also fixed in the driver you are talking about. In all known drivers, n is checked before xstats pointer. So there is no issue but the patch is still valid for unknown drivers. Applied, thanks