From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 87012A034F; Fri, 9 Apr 2021 01:49:50 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 39E124068E; Fri, 9 Apr 2021 01:49:50 +0200 (CEST) Received: from new3-smtp.messagingengine.com (new3-smtp.messagingengine.com [66.111.4.229]) by mails.dpdk.org (Postfix) with ESMTP id 45CA34014D for ; Fri, 9 Apr 2021 01:49:48 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailnew.nyi.internal (Postfix) with ESMTP id 962745805A7; Thu, 8 Apr 2021 19:49:47 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Thu, 08 Apr 2021 19:49:47 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm3; bh= XGS8G5ZL7sT3tk4DCO9YKU1Y7zmDbzJEf8EkylR0dn0=; b=t2hrobzOqv2e6bNF yJUBghTwdtBjM1yWcBuoz/8l9ieaRNM7rilJhak14wVVQ28ky7U+UIDMJK0p1DcK KYGyICYIghEKIfNBPZWDDE443eiDU3sc84+nHpP1nHb16HWRVao/5ziP1rmhRadr 1LJItXIQyLMsfEvEi08mfF10jUFoLHlJInYEsxLsSisgJbycWyH5Q8HaALAgvMv5 g9sME06CUTFo0HFVT9zOP8ZTFf96r1++1347GDmU9A+9j/hAqOMbEjZamOY9jZRX iweC80rC3ufjRflaRXfzRaMUHDjqeFaANmW8lcvASVC/MRVaF0eWDebXeshslxWZ 8MIz5Q== 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-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=XGS8G5ZL7sT3tk4DCO9YKU1Y7zmDbzJEf8EkylR0d n0=; b=AUvM7zepaNthbRWtxZeEoAsKyOhXdj6Mu0HHnd3Ym3IQfCPUW9p4F7/YR wRyyKMtsiuji+el22LB14huXOU3108xxR8Y5FgaAUUh7yupcOm8eyNILh8nkm1od KlKzTsTBRPe50L/wV4l4IFD/8q7q4k0ZO3KI6gbsrDeZfkhn4ZPfWIxz90aNFBWX ESC1qX24Hazn9NlvOaVqEYWCWNfvC4lJ6UhTez8Yo/1JK8Q1TmvSwFVWX+66s91T UrVGpRZB2vkj5GH93T+uWwBhyK32IUGlsv4tIqsjbiHGaU/ji7r1B3YASqbsVdre +zfG+BE3gt/U7n9qSBxjC1iUKefnw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrudektddgvdegucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecukfhppeejjedrudefgedrvddtfedrudekgeenucevlhhushhtvghruf hiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghl ohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 65D55240054; Thu, 8 Apr 2021 19:49:45 -0400 (EDT) From: Thomas Monjalon To: "Xueming(Steven) Li" Cc: dev@dpdk.org, Gaetan Rivet , "dev@dpdk.org" , Asaf Penso , "david.marchand@redhat.com" , "ferruh.yigit@intel.com" , "andrew.rybchenko@oktetlabs.ru" , "hemant.agrawal@nxp.com" , "stephen@networkplumber.org" , "rosen.xu@intel.com" , "ajit.khaparde@broadcom.com" , "jerinj@marvell.com" Date: Fri, 09 Apr 2021 01:49:43 +0200 Message-ID: <4292859.GXhFi8a5gR@thomas> In-Reply-To: References: <1617106521-25931-1-git-send-email-xuemingl@nvidia.com> <5906599.tKcfDlsr6y@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v3 4/5] bus: add device arguments name parsing API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" 01/04/2021 17:13, Xueming(Steven) Li: >From: Thomas Monjalon > >30/03/2021 14:15, Xueming Li: > >> To use Global Device Syntax as devargs, name is required for device > >> management. > > > >Context is missing. > >You mean the argument "name" for the vdev bus? > > Devargs.name, it is used by probe and device iterator. I think we could avoid having a name with the new syntax. In my understanding, this is for compatibility with the legacy syntax. > To locate a device from a devargs, devargs.name is compared > agains name of probed devices. > This not an issue for legacy syntax, > since the string after "bus:" is saved as name. It would be interesting to explain where the name is parsed for the legacy syntax: rte_devargs_parse and for the new syntax: rte_devargs_layers_parse called in rte_devargs_parse in the next patch. > >> In legacy parsing API, devargs name was extracted after bus name: > >> bus:name,kv_params,,, > >> > >> To parse new Global Device Syntax, this patch introduces new bus API > >> to parse devargs and update name, different bus driver might choose > >> different keys from parameters with unified formating, example: > >> -a bus=pci,addr=83:00.0/class=eth/driver=mlx5,... > >> name: 0000:03:00.0 > >> -a bus=vdev,name=pcap0/class=eth/driver=pcap,... > >> name:pcap0 > > > >Only PCI and vdev buses are implemented. > >What can be the plan for others? > >We should track the progress somewhere, maybe with TODO comments. > > Like legacy parser, how about using "name" as default name key, the new syntax parser can resolve it by default. > Then PCI bus overrides by using "addr" key in new bus API, > vdev and other bus drivers simply use default implementation, i.e. using "name" as key.. Yes, you mean if devargs_parse is not implemented by the bus driver, the default is to parse the name property, while the PCI implementation fills the devargs name with the addr property. > >[...] > >> +int > >> +rte_pci_devargs_parse(struct rte_devargs *da) { > >> + struct rte_kvargs *kvargs; > >> + const char *addr_str; > >> + struct rte_pci_addr addr; > >> + int ret; > >> + > >> + if (da == NULL) > >> + return 0; > >> + RTE_ASSERT(da->bus_str != NULL); > >> + > >> + kvargs = rte_kvargs_parse(da->bus_str, NULL); > >> + if (kvargs == NULL) { > >> + RTE_LOG(ERR, EAL, "cannot parse argument list: %s\n", > >> + da->bus_str); > >> + ret = -ENODEV; > >> + goto out; > >> + } > >> + > >> + addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]); > >> + if (addr_str == NULL) { > >> + RTE_LOG(ERR, EAL, "No PCI address specified using '%s=' in: %s\n", > >> + pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str); > >> + ret = -ENODEV; > >> + goto out; > >> + } > >> + > >> + ret = rte_pci_addr_parse(addr_str, &addr); > >> + if (ret != 0) { > >> + RTE_LOG(ERR, EAL, "PCI address invalid: %s\n", da->bus_str); > >> + ret = -EINVAL; > >> + goto out; > >> + } > >> + > >> + rte_pci_device_name(&addr, da->name, sizeof(da->name)); > >> + > >> + /* TODO: class parse -> driver parse */ > > > >Please could you give a longer explanation of what is missing? > > Just an option to give class driver and device driver to override name parsing. > But this should happen in new devargs parser, not here if needed. > > I'll remove this line, and mention it in commit log. What would be the benefit of overriding devargs name parsing? I feel it would be better to not rely on the devargs name with the new syntax if we have such need.