From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH v7 5/6] bus: add helper to find a bus from a device name Date: Wed, 5 Jul 2017 19:05:40 +0530 Message-ID: References: <3cedddd79f0f518e412431526c9e2b3058d3220b.1499211317.git.gaetan.rivet@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: To: Gaetan Rivet Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0076.outbound.protection.outlook.com [104.47.34.76]) by dpdk.org (Postfix) with ESMTP id 114EB2C48 for ; Wed, 5 Jul 2017 15:26:35 +0200 (CEST) In-Reply-To: <3cedddd79f0f518e412431526c9e2b3058d3220b.1499211317.git.gaetan.rivet@6wind.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hello Gaetan, Some comments inline: On Wednesday 05 July 2017 05:25 AM, Gaetan Rivet wrote: > Find which bus should be able to parse this device name into an internal > device representation. > > Signed-off-by: Gaetan Rivet > --- > lib/librte_eal/common/eal_common_bus.c | 21 +++++++++++++++++++++ > lib/librte_eal/common/eal_private.h | 12 ++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c > index 87b0c6e..34fcfa1 100644 > --- a/lib/librte_eal/common/eal_common_bus.c > +++ b/lib/librte_eal/common/eal_common_bus.c > @@ -204,3 +204,24 @@ rte_bus_find_by_name(const char *busname) > { > return rte_bus_find(NULL, cmp_bus_name, (const void *)busname); > } > + > +static int > +bus_can_parse(const struct rte_bus *bus, const void *_name) > +{ > + const char *name = _name; > + > + return !(bus->parse && bus->parse(name, NULL) == 0); > +} > + > +struct rte_bus * > +rte_bus_find_by_device_name(const char *str) > +{ > + char name[32]; It is possible to use a constant here? Basically, I am not sure why '32' has been chosen - or maybe, it might remind a reader in future the reason for this value. Just to clarify: is there any documented limit on bus name? Until this point, the name (and length) of bus was responsibility of bus driver implementation. eal_common_bus.c doesn't codify any limit - so, this may have to be advertised, even if just within the code. > + char *c; > + > + snprintf(name, sizeof(name), "%s", str); > + c = strchr(name, ','); I saw a lot of discussion on the naming assumptions/presumptions. Though, I am not sure I have an understood conclusion from that. So, this might be incorrect/ill-informed: Is it assumed that ',' is not present in device name? Do you think it would be better if this is documented that ',' in a device name is reserved (as per devargs)? (or, is this already known?) For example, in case of DPAA2 devices, I didn't consider this as a case while generating names while scanning the bus (though, I didn't have a ',' in the name). But, if a well known fact, bus drivers can normalize their device names before creating device names. > + if (c != NULL) > + c[0] = '\0'; > + return rte_bus_find(NULL, bus_can_parse, name); > +} > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h > index 6cacce0..0836339 100644 > --- a/lib/librte_eal/common/eal_private.h > +++ b/lib/librte_eal/common/eal_private.h > @@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void); > */ > bool rte_eal_using_phys_addrs(void); > > +/** > + * Find a bus capable of identifying a device. > + * > + * @param str > + * A device identifier (PCI address, virtual PMD name, ...). > + * > + * @return > + * A valid bus handle if found. > + * NULL if no bus is able to parse this device. > + */ > +struct rte_bus *rte_bus_find_by_device_name(const char *str); > + > #endif /* _EAL_PRIVATE_H_ */ > (Apologies for commenting so late in series - feel free to ignore if this disrupts your cycle or sounds out-of-context.) - Shreyansh