From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id 976B62949 for ; Wed, 23 Mar 2016 09:51:51 +0100 (CET) Received: by mail-wm0-f49.google.com with SMTP id l68so13877684wml.1 for ; Wed, 23 Mar 2016 01:51:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-transfer-encoding; bh=mFadP/HybJOGEDDsz81G3fproGoamL7d3o8OUy27umY=; b=w0iPHtsPuEL6XMVwYu0pOscwYCh9v2wDYaoSNlPeRkv+vYeBK9Xb5tt/eDOM1uQjoz Xa0eebSQDIFrQss90jy0WaYtkRWIsZenaDpLNpl5AvErsERRJyIj3W2ncmx3uS+wo0qJ DeR64U27HlIOJG/TZqLXR8nuTc5eu1ExjF2GedZ+EAFx+uQLsyTXTsAAKkXAzQVrP2OU 5vS7FbrJbIjTN22EEZnWkEnqgaYWNITqWN13/OEQH4FljbBJkYp95gBb0th1AJR55qxc AE8KQfhXcuPJ5i19R1sNiMOROa1FAIig+8r/kWDj00MsFz8eKC18PbHb1ceogc3VHA6C UZXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=mFadP/HybJOGEDDsz81G3fproGoamL7d3o8OUy27umY=; b=XLQ3NujWDLXbPauXYa1QczpInqIYrwblARyS57XREY/dzAFRVRmteivVG70Ci0j2xr pYGm+LMq2juAIGDsACKCQvrEuvPb2RFP8+Z0jScKZxscg4U7AwualPzZHeteQRtJUhA/ U52ts7jMsCQJj92d/kh+3LMFj/RTC1XrJ3Lu/DsMDCax6UThuNNGt/EHvvD3pDp+b7qW JxvEk4ZtmFBU0d5G9wUXEufFc/O0tzr7B0E3LSmpxIr33igNCm4meigDMCMyxSWRyiLO pkOo+VS+tLc20XJSgs0N7UCv4hCpN/pXqa2Y9tMy/c8AiU72ZVPaZDsqaMw7grGPdgRh 43yg== X-Gm-Message-State: AD7BkJKbIDmorEA6IsANiFHPPNEBdYVSBQelWTnrx3peqaPSS9p+qG194a26swgtP7KtJsbO X-Received: by 10.194.103.227 with SMTP id fz3mr1943178wjb.43.1458723111487; Wed, 23 Mar 2016 01:51:51 -0700 (PDT) Received: from [192.168.0.10] (was59-1-82-226-113-214.fbx.proxad.net. [82.226.113.214]) by smtp.gmail.com with ESMTPSA id y3sm21095178wmy.17.2016.03.23.01.51.50 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 23 Mar 2016 01:51:50 -0700 (PDT) To: Stephen Hemminger , dev@dpdk.org References: <1458684557-15378-1-git-send-email-stephen@networkplumber.org> From: Olivier Matz X-Enigmail-Draft-Status: N1110 Message-ID: <56F25925.60405@6wind.com> Date: Wed, 23 Mar 2016 09:51:49 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: <1458684557-15378-1-git-send-email-stephen@networkplumber.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 08:51:51 -0000 Hi Stephen, 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. > > Bug was introduced by commit d4fef8b0d5e5 > ("ethdev: expose generic and driver specific stats in xstats") > > Signed-off-by: Stephen Hemminger > --- > lib/librte_ether/rte_ethdev.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index db35102..8721a6b 100644 > --- 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. Thanks, Olivier