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 C6B5D29D9 for ; Thu, 13 Jul 2017 11:17:53 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 4F805209FB; Thu, 13 Jul 2017 05:17:53 -0400 (EDT) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Thu, 13 Jul 2017 05:17:53 -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=CIx8h2ZYV3rfijk xNzKwrksqQ7Nc80xKLxJDb7xGICo=; b=rpn1NxxkgPcMLmXCm65xF+rGPZCeLKb i719xrwqg57CY9fBUMoMBCxZzd+jhmyAl/lXyHDON2GJYbjxVHUJUlM8fxfgeMgU O6ITBrahhQ5UGZBbbBv+tXn3wIcOdkidUi6MYQ6zoPX+USXuBTBkcPr8NVKe0BfN UvtKljsgk7kw= 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=CIx8h2ZYV3rfijkxNzKwrksqQ7Nc80xKLxJDb7xGICo=; b=dsOA0q7k Yax3RhuiiAz9HZF6E/JbW04jhn8WdNZQ5OwMFI6UNPkgVynreaq5gkXfypwJ81f8 cSL6myTY9PGkfZZy1yQaL4jUQjbOTI5oNQeSAXTtNCztol4EdcME3DVjWvkd1/tR NfhpgL8j9B0e/hrMMUHV6jFNP5FF70WFHbxRoOaYGux4TOE+kJqTEFLQOHVQXLOM q+TnDJ6TORbIsKONjjW5VFN7OW4qhboEHpC5vQB8fox92SEh9B1HBzw5xfT1Pl0J i/dFZMiJh1axO8DPtNcU/aN/NcgxU41UmTMkt/jPfBIyDHIO3gXsCUUvyNofTegt yAvui0XawQBTtw== X-ME-Sender: X-Sasl-enc: jmhTjIH+ytOz5wOPLgk7z57HEet7Ke3HDDMVX59GyIzB 1499937473 Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 0D50924604; Thu, 13 Jul 2017 05:17:53 -0400 (EDT) From: Thomas Monjalon To: =?ISO-8859-1?Q?Ga=EBtan?= Rivet Cc: dev@dpdk.org Date: Thu, 13 Jul 2017 11:17:52 +0200 Message-ID: <2174554.9RIM1Tt4jP@xps> In-Reply-To: <20170713081439.GH11154@bidouze.vm.6wind.com> References: <20170712182812.18404-1-thomas@monjalon.net> <1868638.SUpGZpUFpc@xps> <20170713081439.GH11154@bidouze.vm.6wind.com> 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: Thu, 13 Jul 2017 09:17:54 -0000 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, > > >=20 > > > Nice idea. A few remarks below: > > >=20 > > > 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; > > > > + > > >=20 > > > Superfluous line. > >=20 > > I don't think so :) It is isolating the "skip" block with its comment. > >=20 > > > > + /* skip plugged out devices */ > > > > + if (! first_init > > > > + && sdev->cmdline =3D=3D NULL > > > > + && strcmp(da->bus->name, "vdev") !=3D 0) { > > >=20 > > > 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. > >=20 > > Yes > >=20 > > > Indentation is wrong. > >=20 > > No, the coding style is to put 2 tabs for continuation lines. > >=20 > > > > + da->bus->scan(); > > > > + if (bus->find_device(NULL, cmp_dev_name, da->name) =3D=3D NULL) > > > > + continue; /* device not found */ > > >=20 > > > 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. > > >=20 > > > 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. > > >=20 > > > 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]. > > >=20 > > > The alternative fix is to propose an API for buses to transform device > > > names into their canonical form on demand... And it would certainly o= nly > > > be useful for the PCI bus. > > >=20 > > > The issue is discussed there: > > > [1]: http://dpdk.org/ml/archives/dev/2017-July/071155.html > >=20 > > OK, I was not aware of this exact issue. > > So I will wait above fix. >=20 > 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. >=20 > 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... No worries, it can be fixed in 17.11.