From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7F087A0531; Tue, 4 Feb 2020 10:49:12 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E5D1C1C000; Tue, 4 Feb 2020 10:49:11 +0100 (CET) Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by dpdk.org (Postfix) with ESMTP id A8D471BFAD for ; Tue, 4 Feb 2020 10:49:10 +0100 (CET) X-Originating-IP: 37.58.153.206 Received: from [10.0.3.185] (bny206.haproxy.com [37.58.153.206]) (Authenticated sender: grive@u256.net) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 87AA01C0013 for ; Tue, 4 Feb 2020 09:49:10 +0000 (UTC) To: dev@dpdk.org References: <20200204091548.2861-1-somnath.kotur@broadcom.com> From: Gaetan Rivet Message-ID: Date: Tue, 4 Feb 2020 10:49:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.2 MIME-Version: 1.0 In-Reply-To: <20200204091548.2861-1-somnath.kotur@broadcom.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] eal: fix to set the rte_device ptr's device args before hotplug 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 04/02/2020 10:15, Somnath Kotur wrote: > As per the comments in this code section, "since there is a matching device, > it is now its responsibility to manage the devargs we've just inserted." > But the matching device ptr's devargs is still uninitialized or not pointing > to the newest dev_args that were passed as a parameter to local_dev_probe(). > This is needed particularly in the case when *probe is called again* on an > already probed device(the parent device for the representor) as part of adding > a representor port to an OVS switch(OVS-DPDK) like so: > ovs-vsctl add-port ovsbr0 vfrep1 -- set Interface vfrep1 type=dpdk \ > options:dpdk-devargs=0000:06:02.0,representor=[1] > > Fixes: 7e8b26650146 ("eal: fix hotplug add / remove") > > Signed-off-by: Somnath Kotur > --- > lib/librte_eal/common/eal_common_dev.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c > index 9e4f09d..311eef5 100644 > --- a/lib/librte_eal/common/eal_common_dev.c > +++ b/lib/librte_eal/common/eal_common_dev.c > @@ -171,6 +171,7 @@ static int cmp_dev_name(const struct rte_device *dev, const void *_name) > * those devargs shouldn't be removed manually anymore. > */ > > + dev->devargs = da; > ret = dev->bus->plug(dev); > if (ret > 0) > ret = -ENOTSUP; > Hello Somnath, On a surface level, the fix does not seem correct. The comment "Since there is a matching device, it is now its responsibility to manage the devargs we've just inserted. From this point, those devargs should'nt be removed manually anymore." means that the err_devarg label is not correct, on further error in the function, returning the error code and cleaning up the device is sufficient. Setting the devargs for a device is the responsibility of the bus scan function. In the PCI bus for example, this is done in pci_name_set(), called once a device name is fully qualified after scanning the system and thus being able to match a devargs to the device name. Can you please give more information about the device bus, and maybe trace the path taken by the line "ret = da->bus->scan();" a few lines above your edit? If your dev->devargs is not set afterward, it seems the bug would be there. It could be the bus that is not properly implemented, the devargs name not fully qualified, a special path taken by a vdev (I'm not sure what the ovsbr0 device is for example), or a specific representor pluging function, difficult to pinpoint exactly right now.