From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id E9E461E2F for ; Wed, 15 Aug 2018 11:50:59 +0200 (CEST) Received: by mail-wm0-f65.google.com with SMTP id y2-v6so869555wma.1 for ; Wed, 15 Aug 2018 02:50:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:mime-version; bh=OgXlrXffJ8TcflRctB77AxgAezvzhWeUoz3oQ58h1MA=; b=XKbtpmtWFPWgk7GWUFE4W4HxtSM1wz3rZF0DIPCLKs++EpPN3Awz7WnL8Cwu4JGNK0 Cz4T5HJihKMtaKkkH9tCwBFayeEYQRJQMn6rb0Wuny6eZX8zXMhjHMHJO8hfD5ERad+a wcZF0fqSRhnORjkCromA1CRZAu7jcvN7fVw7xUmkTJA7ooy2mvp7BX+sUo6koRjNL5Gq GgSM5nzkr3yxa/wOlEDl20cNpjRGnwZvUM3Iqu+4YZIC6uE1COW42+362Q24QrvBIotq 7LCsWJRDHNJf/b3AtwAldYJ1A7xJP4aPNTIEkQU8yBt6Va908DidCOrfEnOEx6Jl675I D0dg== X-Gm-Message-State: AOUpUlFcoqRCBfsqpgUGu4K4ycXoohCk88W2OX4sqxrUrj9LRvw14RDE CHuryh5Pz2j1vc/Bdo29Oz4= X-Google-Smtp-Source: AA+uWPxZD3utC80QegPHWiiRm4rFfC5+zreLIn6C02mqVzf+Vw/OURvXOYXyMOpO5Xstl98gAUv1VA== X-Received: by 2002:a1c:5e48:: with SMTP id s69-v6mr12698218wmb.19.1534326659521; Wed, 15 Aug 2018 02:50:59 -0700 (PDT) Received: from localhost ([2001:1be0:110d:fcfe:41aa:5bfa:6cf3:7531]) by smtp.gmail.com with ESMTPSA id p5-v6sm16729221wre.32.2018.08.15.02.50.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 15 Aug 2018 02:50:58 -0700 (PDT) Message-ID: <1534326657.5764.11.camel@debian.org> From: Luca Boccassi To: Tiwei Bie Cc: dev@dpdk.org, maxime.coquelin@redhat.com, zhihong.wang@intel.com, bruce.richardson@intel.com, Brian Russell Date: Wed, 15 Aug 2018 10:50:57 +0100 In-Reply-To: <20180815031144.GA7324@debian> References: <20180814143035.19640-1-bluca@debian.org> <20180814143035.19640-2-bluca@debian.org> <20180815031144.GA7324@debian> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.22.6-1+deb9u1 Mime-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 2/2] virtio: fix PCI config err handling X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Aug 2018 09:51:00 -0000 On Wed, 2018-08-15 at 11:11 +0800, Tiwei Bie wrote: > On Tue, Aug 14, 2018 at 03:30:35PM +0100, Luca Boccassi wrote: > > From: Brian Russell > >=20 > > In virtio_read_caps, rte_pci_read_config returns the number of > > bytes > > read from PCI config or < 0 on error. > > If less than the expected number of bytes are read then log the > > failure and return rather than carrying on with garbage. >=20 > Is this a fix or an improvement? > Or did you see anything broken without this patch? > If so, we may need a fixes line and Cc stable. It is a fix, as it was creating problems in production due to the constant flux of errors in the logs. But given patch 1/2 is effectively doing a small change in the BSD bus API, and it's a requirement for 2/2, I don't think we can include it in the stable releases unfortunately. > >=20 > > Signed-off-by: Brian Russell > > --- > >=20 > > Follow-up from: > > http://mails.dpdk.org/archives/dev/2017-June/067278.html > > https://patches.dpdk.org/patch/25056/ > >=20 > > =C2=A0drivers/net/virtio/virtio_pci.c | 12 +++++++----- > > =C2=A01 file changed, 7 insertions(+), 5 deletions(-) > >=20 > > diff --git a/drivers/net/virtio/virtio_pci.c > > b/drivers/net/virtio/virtio_pci.c > > index 6bd22e54a6..a10698aed8 100644 > > --- a/drivers/net/virtio/virtio_pci.c > > +++ b/drivers/net/virtio/virtio_pci.c > > @@ -567,16 +567,18 @@ virtio_read_caps(struct rte_pci_device *dev, > > struct virtio_hw *hw) > > =C2=A0 } > > =C2=A0 > > =C2=A0 ret =3D rte_pci_read_config(dev, &pos, 1, > > PCI_CAPABILITY_LIST); > > - if (ret < 0) { > > - PMD_INIT_LOG(DEBUG, "failed to read pci capability > > list"); > > + if (ret !=3D 1) { > > + PMD_INIT_LOG(DEBUG, > > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0"failed to read pci capability list, > > ret %d", ret); > > =C2=A0 return -1; > > =C2=A0 } > > =C2=A0 > > =C2=A0 while (pos) { > > =C2=A0 ret =3D rte_pci_read_config(dev, &cap, sizeof(cap), > > pos); > > - if (ret < 0) { > > - PMD_INIT_LOG(ERR, > > - "failed to read pci cap at pos: > > %x", pos); > > + if (ret !=3D sizeof(cap)) { > > + PMD_INIT_LOG(DEBUG, >=20 > Why change the log level to DEBUG? >=20 > Thanks Beforehand reading less than the required amount of bytes caused problems in the following code, so it warranted printing errors - but now it will not go ahead without the right amount of data, so it's not critical anymore to inform the user. Main issue is, log will get very spammy with errors, and paying customers don't like that :-) --=20 Kind regards, Luca Boccassi