From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bert van Leeuwen Subject: Re: [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS Date: Fri, 11 Nov 2016 11:48:15 +0200 Message-ID: <4D79411C-0FB4-45DF-9CDB-06570DDB2818@netronome.com> References: <1478786449-44745-1-git-send-email-alejandro.lucero@netronome.com> <1630834.YGmmObtqnA@xps13> Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Cc: Alejandro Lucero , dev@dpdk.org To: Thomas Monjalon Return-path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id 614002B8C for ; Fri, 11 Nov 2016 10:48:20 +0100 (CET) Received: by mail-wm0-f44.google.com with SMTP id a197so415763876wmd.0 for ; Fri, 11 Nov 2016 01:48:20 -0800 (PST) In-Reply-To: <1630834.YGmmObtqnA@xps13> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > On 11 Nov 2016, at 11:29 AM, Thomas Monjalon = wrote: >=20 > 2016-11-11 09:16, Alejandro Lucero: >> Thomas, >>=20 >> We are wondering if you realize this patch fixes a bug with current = ethdev >> code as a device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS. >>=20 >> Maybe the commit message is giving the wrong impression and as you >> commented, it should just focus on the bug it fixes and to leave for >> another email thread the discussion of how to solve the >> RTE_ETHDEV_QUEUE_STAT_CNTRS >> problem. >>=20 >> Should we remove this from patchwork and to send another patch that = way? >=20 > Yes please. It was my first comment, we don't understand the exact = issue > you are fixing. In a nutshell, the rte_eth_xstats_get function was reading memory beyond = what was stored in the internal arrays (and thus returning junk values). = The patch simply prevents it from going out of bounds, it does not = address the issue that the rest of the counters are lost though. This = issue is not specific to our device, in fact we found it while using a = multiqueue virtio device (32 queues), and traced the issue into this = core ethdev functionality. > And I have a bad feeling it could break something else (really just a = feeling). > It is not the kind of patch we can apply the last day of a release. > That's why I think it should wait 17.02. >=20 > Of course you can try to convince me and others to apply it as a last = minute > patch. But why are you sending a patch on the generic API in the last = days? >=20 > Last argument: it is not fixing a regression of 16.11, so it is not so = urgent. Yes, it looks like this bug was present in DPDK 2.2 even, and possibly = before that too.=