From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id 6A62F1B2CC for ; Tue, 16 Jan 2018 23:31:18 +0100 (CET) Received: by mail-wm0-f54.google.com with SMTP id 143so11373425wma.5 for ; Tue, 16 Jan 2018 14:31:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=Aa/6t6kV3W1ggdYljVN+WrOZ/cS3MmoqXutLhn1JFJk=; b=yu7mtxXnnDQn+ijL2nBP/eCQ08u4M/DQWxXE3E8uakXPPO4lj42VIt2X+tYA92qpjM bkPSg3gnMYfqpMKCKC4dGwVcjEUpkG5I3RD3nvEVJYo8ocAU83A7GDoTijNb9E72+9H3 KFnMfnILxEd6atcznGty00A2x/vFxG2b4zbowX7sfVfhC/DF6MNQ3Q+WqtcUUsbshJEy HGCmUGtIPYpic48IYGWExSlvZDbM3i3nLkde4A+6n0ZD3nG8rw40SFN8CzXkTlqewO8+ UMSp8s5tAzsl7YanDps76I6fXurFOkEqQqS20/kIYZLsDfQTOc3uDTpF4TMJpR3VONQ/ RM6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=Aa/6t6kV3W1ggdYljVN+WrOZ/cS3MmoqXutLhn1JFJk=; b=GJOlcxZ1sAHvoV6K42vjVmFzWMNK4+zNh/IxeUpaJSM5NzsHxe21FF8CHwQ8uqFLQt ItK6VQHkvK0qEr/YTtEhg+Ye179ieos8cecuFQi051MgRBprbvPaE+MzyqrE1t2ayccT 0haW0zA5K0kpJGaeRMSt8CPGB6jHmzdjZRnScpoQIrDzWj24n1DmwWSiP2HUKykADk2v KeIKSGQd1kfeMVDeE7ulRI1KPaD/BcBQEuP41iVJuQz8NBDDxSePEgFN7cQCvHwXZsKG ZQy5e6pgjdKNh5xNoTqPyiBtiZEH8gJO4BuxEmt6hLz2yTUNLel7jwmfyVb5c4RDMv5Y X+Ng== X-Gm-Message-State: AKwxytcLproLSYfIXz/B2W8g6Ro0a6Iw3vRmpsD/WLNpRa8zyXSECLV8 5pxASZIMASGb3slztTvSubrjIwIM X-Google-Smtp-Source: ACJfBou+WLRwKferinusyvSKQBB08dWHDNOhW6U2O+xt+VDuDGkhA6jeoiKBVyljn5qExkDMq4eKBw== X-Received: by 10.80.240.86 with SMTP id u22mr26205135edl.276.1516141877999; Tue, 16 Jan 2018 14:31:17 -0800 (PST) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id c45sm2115283edb.17.2018.01.16.14.31.16 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 16 Jan 2018 14:31:17 -0800 (PST) Date: Tue, 16 Jan 2018 23:31:04 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Matan Azrad Cc: Ferruh Yigit , Thomas Monjalon , "dev@dpdk.org" , "stephen@networkplumber.org" Message-ID: <20180116223104.y2iesdbxqkf775xp@bidouze.vm.6wind.com> References: <20171222173846.20731-1-adrien.mazarguil@6wind.com> <1515509253-17834-1-git-send-email-matan@mellanox.com> <1515509253-17834-4-git-send-email-matan@mellanox.com> <20180116110920.vqp3bqjroudsdjm4@bidouze.vm.6wind.com> <20180116144050.ho4k2dp24lgzhtdr@bidouze.vm.6wind.com> <20180116165409.a463ukxbsmtdoc2v@bidouze.vm.6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v3 3/8] net/failsafe: support probed sub-devices getting 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, 16 Jan 2018 22:31:18 -0000 Hi Matan, On Tue, Jan 16, 2018 at 05:20:27PM +0000, Matan Azrad wrote: > Hi Gaetan > > > > > In 18.05, or 18.08 there should be an EAL function that would be > > > > able to identify a device given a specific ID string (very close to an > > rte_devargs). > > > > Currently, this API does not exist. > > > > > > > > You can hack your way around this for the moment, IF you really, > > > > really > > > > want: parse your devargs, get the bus, use the bus->parse() function > > > > to get a binary device representation, and compare bytes per bytes > > > > the binary representation given by your devargs and by the device- > > >name. > > > > > > > > But this is a hack, and a pretty ugly one at that: you have no way > > > > of knowing the size taken by this binary representation, so you can > > > > restrict yourself to the vdev and PCI bus for the moment and take > > > > the larger of an rte_vdev_driver pointer and an rte_pci_addr.... > > > > > > > > { > > > > union { > > > > rte_vdev_driver *drv; > > > > struct rte_pci_addr pci_addr; > > > > } bindev1, bindev2; > > > > memset(&bindev1, 0, sizeof(bindev1)); > > > > memset(&bindev2, 0, sizeof(bindev2)); > > > > rte_eal_devargs_parse(device->name, da1); > > > > rte_eal_devargs_parse(your_devstr, da2); > > > > RTE_ASSERT(da1->bus == rte_bus_find_by_name("pci") || > > > > da1->bus == rte_bus_find_by_name("vdev")); > > > > RTE_ASSERT(da2->bus == rte_bus_find_by_name("pci") || > > > > da2->bus == rte_bus_find_by_name("vdev")); > > > > da1->bus->parse(da1->name, &bindev1); > > > > da1->bus->parse(da2->name, &bindev2); > > > > if (memcmp(&bindev1, &bindev2, sizeof(bindev1)) == 0) { > > > > /* found the device */ > > > > } else { > > > > /* not found */ > > > > } > > > > } > > > > > > > > So, really, really ugly. Anyway. > > > > > > > Yes, ugly :) Thanks for this update! > > > Will keep the comparison by device->name. > > > > > > > Well as explained, above, the comparison by device->name only works with > > whitelisted devices. > > > > > So either implement something broken right now that you will need to > > update in 18.05, or implement it properly in 18.05 from the get go. > > > For the current needs it is enough. > We can also say that it is the user responsibility to pass to failsafe the same names and same args as he passes for EAL(or default EAL names). > I think I emphasized it in documentation. > Okay, as you wish. Just be aware of this limitation. I think this functionality is good and useful, but it needs to be made clean. The proper function should be available soon, then this implementaion should be cleaned up. > > > > > > > > > > > > > > > + /* Take control of device probed by EAL > > options. */ > > > > > > > + DEBUG("Taking control of a probed sub > > device" > > > > > > > + " %d named %s", i, da->name); > > > > > > > > > > > > In this case, the devargs of the probed device must be copied > > > > > > within the sub- device definition and removed from the EAL using > > > > > > the proper rte_devargs API. > > > > > > > > > > > > Note that there is no rte_devargs copy function. You can use > > > > > > rte_devargs_parse instead, "parsing" again the original devargs > > > > > > into the sub- device one. It is necessary for complying with > > > > > > internal rte_devargs requirements (da->args being malloc-ed, at > > > > > > the moment, > > > > but may evolve). > > > > > > > > > > > > The rte_eal_devargs_parse function is not easy enough to use > > > > > > right now, you will have to build a devargs string (using snprintf) and > > submit it. > > > > > > I proposed a change this release for it but it will not make it > > > > > > for 18.02, that would have simplified your implementation. > > > > > > > > > > > > > > > > Got you. You right we need to remove the created devargs in > > > > > fail-safe > > > > parse level. > > > > > What do you think about checking it in the parse level and avoid > > > > > the new > > > > devargs creation? > > > > > Also to do the copy in parse level(same method as we are doing in > > > > > probe > > > > level)? > > > > > > > > > > > > > Not sure I follow here, but the new rte_devargs is part of the > > > > sub-device (it is not a pointer, but allocated alongside the sub_device). > > > > > > > > So keep everything here, it is the right place to deal with these things. > > > > > > > But it will prevent the double parsing and also saves the method: > > > If the device already parsed - copy its devargs and continue. > > > If the device already probed - copy the device pointer and continue. > > > > > > I think this is the right dealing, no? > > > Why to deal with parse level in probe level? Just keep all the parse work to > > parse level and the probe work to probe level. > > > > After re-reading, I think we misunderstood each other. > > You cannot remove the rte_devargs created during parsing: it is allocated > > alongside the sub_device structure. > > > > You must only remove the rte_devargs allocated by the EAL (using > > rte_eal_devargs_remove()). > > > > Sure. > > > Before removing it, you must copy its content in the local sub_device > > rte_devargs structure. I only proposed a way to do this copy that would not > > deal with rte_devargs internals, as it is bound to evolve rather soon. > > > Yes. > > > Otherwise, no, I do not want to complicate the parsing operations, they are > > already too complicated and too criticals. Better to keep it all here. > > I think fs_parse_device function is not complicated and it is the natural place for devargs games. > For me this is the right place for the copy & remove devargs. > Are you insisting to put all in fs_bus_init? You would have to put fs_ethdev_portid_find in failsafe_args, which is mixing layers. Sorry but yes, please keep all these changes in this file. Thanks, -- Gaëtan Rivet 6WIND