From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 0306047CE for ; Tue, 18 Jul 2017 22:49:59 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id A32F02071D; Tue, 18 Jul 2017 16:49:59 -0400 (EDT) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Tue, 18 Jul 2017 16:49:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=t3Dq5YTvtS2KZKO L5DX0EEN+X5jJx2bU+jfV9BzqncA=; b=q/P8b1lUbKLxvUGB32fVz2EzcCJcGMp 8QBfRUWaP8dBzRDfiPNlMkstGOL4hrb9GCi8rCgNZJsFgQns49xZ5U1WVoupmUYF vDB5Q/k31y83S4Ch5dVWG03zqQ/ssmQK/2p0RzKjy7WyAMJ8NlDjAf0LJKxJDarH lHbsRs9fauWE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc:x-sasl-enc; s= fm1; bh=t3Dq5YTvtS2KZKOL5DX0EEN+X5jJx2bU+jfV9BzqncA=; b=r3jq4kBb M5XSXu4RFrCz37red2/hhJ41QPyz0pa9y/iH92573w3kOi3Jk0LZIJhdz4TEag3s 6vxT4Hn4/XDchyJWE3Cbkkg5ooa/L2WYW4kSokXumoDrTcUsJGmAGsYOdF20Fx64 v6wO9kNyRJ79hYgH3M1ar6u1iV5myEUa93Z1PR+2p2ztmhuET03IcJaw5uw5HDiD WsYfqjmEf7sTCnjKYhlFg8Xn/UMBxV3+/prgumLatUfNeHgsFoQT9VIO2NV+NMNm hlvLPBueR7NKPYQN3NKZf7Sls/wbsbRY/h2sTQ97BmdKCjWcKhHZ+1lD9V7b9USA g2OqtDVDjLlhAw== X-ME-Sender: X-Sasl-enc: UsEzmCnEdHXX7m6IQk8jiaDHizK4ymkCTkumAVcdz2+4 1500410998 Received: from xps.localnet (unknown [37.165.12.175]) by mail.messagingengine.com (Postfix) with ESMTPA id F1AAD2418A; Tue, 18 Jul 2017 16:49:58 -0400 (EDT) From: Thomas Monjalon To: Ferruh Yigit Cc: dev@dpdk.org, =?ISO-8859-1?Q?Ga=EBtan?= Rivet Date: Tue, 18 Jul 2017 23:49:55 +0300 Message-ID: <2216873.ENK3EkxbcF@xps> In-Reply-To: References: <20170712182812.18404-1-thomas@monjalon.net> <2174554.9RIM1Tt4jP@xps> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" Subject: Re: [dpdk-dev] [PATCH] net/failsafe: do not probe device if plugged out 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: Tue, 18 Jul 2017 20:50:00 -0000 18/07/2017 11:39, Ferruh Yigit: > On 7/13/2017 10:17 AM, Thomas Monjalon wrote: > > 13/07/2017 10:14, Ga=EBtan Rivet: > >> On Thu, Jul 13, 2017 at 08:52:33AM +0200, Thomas Monjalon wrote: > >>> 12/07/2017 22:39, Ga=EBtan Rivet: > >>>> Hi Thomas, > >>>> > >>>> Nice idea. A few remarks below: > >>>> > >>>> On Wed, Jul 12, 2017 at 08:28:12PM +0200, Thomas Monjalon wrote: > >>>>> FOREACH_SUBDEV(sdev, i, dev) { > >>>>> if (sdev->state !=3D DEV_PARSED) > >>>>> continue; > >>>>> da =3D &sdev->devargs; > >>>>> + > >>>> > >>>> Superfluous line. > >>> > >>> I don't think so :) It is isolating the "skip" block with its comment. > >>> > >>>>> + /* skip plugged out devices */ > >>>>> + if (! first_init > >>>>> + && sdev->cmdline =3D=3D NULL > >>>>> + && strcmp(da->bus->name, "vdev") !=3D 0) { > >>>> > >>>> Use first_init =3D=3D false instead of negation. > >>>> && should be at the end of the line instead of the start of the next > >>>> one. > >>> > >>> Yes > >>> > >>>> Indentation is wrong. > >>> > >>> No, the coding style is to put 2 tabs for continuation lines. > >>> > >>>>> + da->bus->scan(); > >>>>> + if (bus->find_device(NULL, cmp_dev_name, da->name) =3D=3D NULL) > >>>>> + continue; /* device not found */ > >>>> > >>>> da->bus->find_device instead of bus->find_device. > >>>> This function cannot find the device back currently on the PCI bus, > >>>> blocking the plugging of VF. > >>>> > >>>> The PCI bus will scan the VF while no rte_devargs exists to > >>>> describe it within the global list. If the device exists, it will > >>>> detect it, allocate it and then set its name. > >>>> Without any rte_devargs, the name of a PCI device falls back to its > >>>> canonical name (DomBDF instead of BDF). The name comparison with > >>>> da->name can only succeed if the slave was declared using the DomBDF > >>>> format. > >>>> > >>>> The fix is to do a deep copy of the rte_devargs (the API has been > >>>> sent previously with the rte_devargs rework but I have since removed > >>>> it) and insert it using rte_eal_devargs_insert(). This is essentially > >>>> the solution I used for the rte_eal_hotplug_add() fix[1]. > >>>> > >>>> The alternative fix is to propose an API for buses to transform devi= ce > >>>> names into their canonical form on demand... And it would certainly = only > >>>> be useful for the PCI bus. > >>>> > >>>> The issue is discussed there: > >>>> [1]: http://dpdk.org/ml/archives/dev/2017-July/071155.html > >>> > >>> OK, I was not aware of this exact issue. > >>> So I will wait above fix. > >> > >> The fix above will not solve this issue. > >> If you are waiting for a proper, general fix, I don't think it will > >> arrive anytime soon. > >> > >> I have the rte_eal_devargs_clone and a working version that I can send > >> in-reply-to if you want. But that's an API extension while in RC2 so... > >=20 > > No worries, it can be fixed in 17.11. > >=20 >=20 > What is the status of the patch, is this patch rejected for this > release, or will be merged into existing failsafe? I think this patch is rejected.