From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Shreyansh Jain <shreyansh.jain@nxp.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v7 5/6] bus: add helper to find a bus from a device name
Date: Wed, 5 Jul 2017 15:45:21 +0200 [thread overview]
Message-ID: <20170705134521.GI11154@bidouze.vm.6wind.com> (raw)
In-Reply-To: <decf56f4-b3cf-4f81-6a56-1b3ff7882642@nxp.com>
Hi Shreyansh,
On Wed, Jul 05, 2017 at 07:05:40PM +0530, Shreyansh Jain wrote:
> 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 <gaetan.rivet@6wind.com>
> >---
> > 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.
>
I agree that this is not clear. First thing however: this is a limit on
the device name length, not bus.
This problem is fixed in [1], as a single common device name location is
defined. The problem is that it is in another patchset (even if both are
closely linked and were only separated to ease integration).
This limit was currently taken from the arbitrary limit of device in
name in the original rte_devargs for virtual devices. I think it could
be exported as a define by rte_dev.h, used there and reused in [1] to
define the new rte_devargs structure.
Would that be ok with you?
[1]: http://dpdk.org/ml/archives/dev/2017-July/070225.html
> >+ 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.
>
For this release cycle the discussion was mostly about forbidding (or
not forbidding) characters in bus names. This character is one
limitation previously defined for device names, to separate the device
name from its kvargs.
I am thus following the current rte_devargs API.
Not sure if that's documented. There are examples in doc, but maybe this
device name limitation is not explicitly said.
> >+ 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.)
>
No problem, thanks for reviewing so quickly this patchset.
> -
> Shreyansh
--
Gaëtan Rivet
6WIND
next prev parent reply other threads:[~2017-07-05 13:45 UTC|newest]
Thread overview: 123+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-24 15:12 [dpdk-dev] [PATCH 0/9] rte_bus parse API Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 1/9] bus: fix bus name registration Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 2/9] bus: verify bus name on registration Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 3/9] bus: introduce parsing functionality Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 4/9] vdev: implement parse bus operation Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 5/9] pci: " Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 6/9] bus: add helper to find bus from a name Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 7/9] bus: add helper to find a bus from a device name Gaetan Rivet
2017-06-07 17:28 ` Jan Blunck
2017-06-07 20:03 ` Gaëtan Rivet
2017-06-08 10:45 ` Jan Blunck
2017-06-08 11:36 ` Gaëtan Rivet
2017-06-08 11:40 ` Jan Blunck
2017-06-08 12:51 ` Gaëtan Rivet
2017-06-10 8:50 ` Jan Blunck
2017-06-12 14:21 ` Gaëtan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 8/9] vdev: expose bus name Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 9/9] devargs: parse bus info Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 0/9] rte_bus parse API Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 1/9] bus: fix bus name registration Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 2/9] bus: verify bus name on registration Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 3/9] bus: introduce parsing functionality Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 4/9] vdev: implement parse bus operation Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 5/9] pci: " Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 6/9] bus: add helper to find bus from a name Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 7/9] bus: add helper to find a bus from a device name Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 8/9] vdev: expose bus name Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 9/9] devargs: parse bus info Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 0/9] rte_bus parse API Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 1/9] bus: fix bus name registration Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 2/9] bus: verify bus name on registration Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 3/9] bus: introduce parsing functionality Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 4/9] vdev: implement parse bus operation Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 5/9] pci: " Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 6/9] bus: add helper to find bus from a name Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 7/9] bus: add helper to find a bus from a device name Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 8/9] vdev: expose bus name Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 9/9] devargs: parse bus info Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 0/9] rte_bus parse API Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 1/9] bus: fix bus name registration Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 2/9] bus: verify bus name on registration Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 3/9] bus: introduce parsing functionality Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 4/9] vdev: implement parse bus operation Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 5/9] pci: " Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 6/9] bus: add helper to find a bus from a bus name Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 7/9] bus: add helper to find a bus from a device name Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 8/9] vdev: expose bus name Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 9/9] devargs: parse bus info Gaetan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 0/7] rte_bus parse API Gaetan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 1/7] bus: fix bus name registration Gaetan Rivet
2017-06-27 15:53 ` Bruce Richardson
2017-06-27 19:19 ` Jan Blunck
2017-07-04 1:05 ` Gaëtan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 2/7] bus: introduce parsing functionality Gaetan Rivet
2017-06-27 15:54 ` Bruce Richardson
2017-06-27 19:26 ` Jan Blunck
2017-07-04 1:35 ` Gaëtan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 3/7] vdev: implement parse bus operation Gaetan Rivet
2017-06-27 15:59 ` Bruce Richardson
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 4/7] pci: " Gaetan Rivet
2017-06-27 16:18 ` Bruce Richardson
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 5/7] bus: add helper to find a bus from a device name Gaetan Rivet
2017-06-27 16:26 ` Bruce Richardson
2017-06-27 18:55 ` Jan Blunck
2017-06-28 17:03 ` Thomas Monjalon
2017-06-29 7:56 ` Jan Blunck
2017-06-29 8:21 ` Thomas Monjalon
2017-06-29 10:23 ` Gaëtan Rivet
2017-06-29 10:30 ` Richardson, Bruce
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 6/7] vdev: expose bus name Gaetan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 7/7] devargs: parse bus info Gaetan Rivet
2017-06-27 16:01 ` [dpdk-dev] [PATCH v5 0/7] rte_bus parse API Bruce Richardson
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 0/6] " Gaetan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 1/6] bus: fix bus name registration Gaetan Rivet
2017-07-04 21:43 ` [dpdk-dev] [PATCH] bus: fix driver registration Thomas Monjalon
2017-07-05 5:47 ` Shreyansh Jain
2017-07-05 6:01 ` Shreyansh Jain
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 2/6] bus: introduce parsing functionality Gaetan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 3/6] vdev: implement parse bus operation Gaetan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 4/6] pci: " Gaetan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 5/6] bus: add helper to find a bus from a device name Gaetan Rivet
2017-07-04 12:28 ` Gaëtan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 6/6] devargs: parse bus info Gaetan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 0/6] rte_bus parse API Gaetan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 1/6] bus: fix driver registration Gaetan Rivet
2017-07-05 13:03 ` Shreyansh Jain
2017-07-06 6:05 ` santosh
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 2/6] bus: introduce parsing functionality Gaetan Rivet
2017-07-05 13:04 ` Shreyansh Jain
2017-07-06 9:19 ` santosh
2017-07-06 12:30 ` Gaëtan Rivet
2017-07-06 13:12 ` santosh
2017-07-06 13:30 ` Gaëtan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 3/6] vdev: implement parse bus operation Gaetan Rivet
2017-07-05 13:16 ` Shreyansh Jain
2017-07-06 9:35 ` santosh
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 4/6] pci: " Gaetan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 5/6] bus: add helper to find a bus from a device name Gaetan Rivet
2017-07-05 13:35 ` Shreyansh Jain
2017-07-05 13:45 ` Gaëtan Rivet [this message]
2017-07-05 14:12 ` Shreyansh Jain
2017-07-06 10:10 ` Thomas Monjalon
2017-07-06 11:37 ` Gaëtan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 6/6] devargs: parse bus info Gaetan Rivet
2017-07-05 18:03 ` Stephen Hemminger
2017-07-06 9:07 ` Shreyansh Jain
2017-07-06 10:00 ` Gaëtan Rivet
2017-07-06 13:17 ` Shreyansh Jain
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 0/6] rte_bus parse API Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 1/6] bus: fix driver registration Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 2/6] bus: introduce parsing functionality Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 3/6] vdev: implement parse bus operation Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 4/6] pci: " Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 5/6] bus: add helper to find a bus from a device name Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 6/6] devargs: parse bus info Gaetan Rivet
2017-07-07 0:45 ` Stephen Hemminger
2017-07-07 8:31 ` Gaëtan Rivet
2017-07-08 20:31 ` Thomas Monjalon
2017-07-08 20:30 ` [dpdk-dev] [PATCH v8 0/6] rte_bus parse API Thomas Monjalon
2017-06-07 17:22 ` [dpdk-dev] [PATCH 0/9] " Jan Blunck
2017-06-07 19:55 ` Gaëtan Rivet
2017-06-08 11:38 ` Jan Blunck
2017-06-08 13:04 ` Gaëtan Rivet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170705134521.GI11154@bidouze.vm.6wind.com \
--to=gaetan.rivet@6wind.com \
--cc=dev@dpdk.org \
--cc=shreyansh.jain@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).