From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ferruh.yigit@intel.com>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
by dpdk.org (Postfix) with ESMTP id B1456377E
for <dev@dpdk.org>; Tue, 18 Jul 2017 10:39:44 +0200 (CEST)
Received: from orsmga005.jf.intel.com ([10.7.209.41])
by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
18 Jul 2017 01:39:43 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.40,377,1496127600"; d="scan'208";a="126372200"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.255.134.81])
([10.255.134.81])
by orsmga005.jf.intel.com with ESMTP; 18 Jul 2017 01:39:40 -0700
To: Thomas Monjalon <thomas@monjalon.net>,
=?UTF-8?Q?Ga=c3=abtan_Rivet?= <gaetan.rivet@6wind.com>
Cc: dev@dpdk.org
References: <20170712182812.18404-1-thomas@monjalon.net>
<1868638.SUpGZpUFpc@xps> <20170713081439.GH11154@bidouze.vm.6wind.com>
<2174554.9RIM1Tt4jP@xps>
From: Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <c4130efe-fec9-2e83-33d5-279fd5bd9bfe@intel.com>
Date: Tue, 18 Jul 2017 09:39:37 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <2174554.9RIM1Tt4jP@xps>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 18 Jul 2017 08:39:46 -0000
On 7/13/2017 10:17 AM, Thomas Monjalon wrote:
> 13/07/2017 10:14, Gaëtan Rivet:
>> On Thu, Jul 13, 2017 at 08:52:33AM +0200, Thomas Monjalon wrote:
>>> 12/07/2017 22:39, Gaëtan 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 != DEV_PARSED)
>>>>> continue;
>>>>> da = &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 == NULL
>>>>> + && strcmp(da->bus->name, "vdev") != 0) {
>>>>
>>>> Use first_init == 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) == 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 device
>>>> 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...
>
> No worries, it can be fixed in 17.11.
>
What is the status of the patch, is this patch rejected for this
release, or will be merged into existing failsafe?