[dpdk-dev,v5,2/7] bus: introduce parsing functionality

Message ID ffdae5a5fb2760d3916a794fe73dbbc9a190b974.1497999826.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Gaëtan Rivet June 20, 2017, 11:30 p.m. UTC
  This operation can be used either to validate that a device
representation can be understood by a bus, as well as store the resulting
specialized device representation in any format determined by the bus.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
  

Comments

Bruce Richardson June 27, 2017, 3:54 p.m. UTC | #1
On Wed, Jun 21, 2017 at 01:30:31AM +0200, Gaetan Rivet wrote:
> This operation can be used either to validate that a device
> representation can be understood by a bus, as well as store the resulting
> specialized device representation in any format determined by the bus.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  
Jan Blunck June 27, 2017, 7:26 p.m. UTC | #2
On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> This operation can be used either to validate that a device
> representation can be understood by a bus, as well as store the resulting
> specialized device representation in any format determined by the bus.
>

Again, I don't think this makes sense to have. Also there is no user
for this as far as I can see. The bus specific device representation
should come from the scan function instead of this.


> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index b220299..05503ea 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -117,6 +117,26 @@ typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);
>  typedef int (*rte_bus_unplug_t)(struct rte_device *dev);
>
>  /**
> + * Bus specific parsing function.
> + * Validates the syntax used in the textual representation of a device,
> + * If the syntax is valid and ``addr`` is not NULL, writes the bus-specific
> + * device representation to ``addr``.
> + *
> + * @param[in] name
> + *     device textual description
> + *
> + * @param[out] addr
> + *     device information location address, into which parsed info
> + *     should be written. If NULL, nothing should be written, which
> + *     is not an error.
> + *
> + * @return
> + *     0 if parsing was successful.
> + *     !0 for any error.
> + */
> +typedef int (*rte_bus_parse_t)(const char *name, void *addr);
> +
> +/**
>   * A structure describing a generic bus.
>   */
>  struct rte_bus {
> @@ -127,6 +147,7 @@ struct rte_bus {
>         rte_bus_find_device_t find_device; /**< Find device on bus */
>         rte_bus_plug_t plug;         /**< Probe single device for drivers */
>         rte_bus_unplug_t unplug;     /**< Remove single device from driver */
> +       rte_bus_parse_t parse;       /**< Parse a device name */
>  };
>
>  /**
> --
> 2.1.4
>
  
Gaëtan Rivet July 4, 2017, 1:35 a.m. UTC | #3
On Tue, Jun 27, 2017 at 09:26:20PM +0200, Jan Blunck wrote:
> On Wed, Jun 21, 2017 at 1:30 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > This operation can be used either to validate that a device
> > representation can be understood by a bus, as well as store the resulting
> > specialized device representation in any format determined by the bus.
> >
> 
> Again, I don't think this makes sense to have. Also there is no user
> for this as far as I can see. The bus specific device representation
> should come from the scan function instead of this.
> 
> 

I think it makes sense that scanning would store this representation
yes. The issue is that it requires another rte_bus_scan API evolution,
which is beyond this patchset.

Keep in mind that I was bound when writing this to follow the existing
API. I agree that some cleanup could come up in the EAL, but it cannot
all happen at once. These first steps (generic devargs / vdev & PCI move
to drivers/bus) serve to trim the EAL before these changes.

> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >  lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > index b220299..05503ea 100644
> > --- a/lib/librte_eal/common/include/rte_bus.h
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -117,6 +117,26 @@ typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);
> >  typedef int (*rte_bus_unplug_t)(struct rte_device *dev);
> >
> >  /**
> > + * Bus specific parsing function.
> > + * Validates the syntax used in the textual representation of a device,
> > + * If the syntax is valid and ``addr`` is not NULL, writes the bus-specific
> > + * device representation to ``addr``.
> > + *
> > + * @param[in] name
> > + *     device textual description
> > + *
> > + * @param[out] addr
> > + *     device information location address, into which parsed info
> > + *     should be written. If NULL, nothing should be written, which
> > + *     is not an error.
> > + *
> > + * @return
> > + *     0 if parsing was successful.
> > + *     !0 for any error.
> > + */
> > +typedef int (*rte_bus_parse_t)(const char *name, void *addr);
> > +
> > +/**
> >   * A structure describing a generic bus.
> >   */
> >  struct rte_bus {
> > @@ -127,6 +147,7 @@ struct rte_bus {
> >         rte_bus_find_device_t find_device; /**< Find device on bus */
> >         rte_bus_plug_t plug;         /**< Probe single device for drivers */
> >         rte_bus_unplug_t unplug;     /**< Remove single device from driver */
> > +       rte_bus_parse_t parse;       /**< Parse a device name */
> >  };
> >
> >  /**
> > --
> > 2.1.4
> >
  

Patch

diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index b220299..05503ea 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -117,6 +117,26 @@  typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);
 typedef int (*rte_bus_unplug_t)(struct rte_device *dev);
 
 /**
+ * Bus specific parsing function.
+ * Validates the syntax used in the textual representation of a device,
+ * If the syntax is valid and ``addr`` is not NULL, writes the bus-specific
+ * device representation to ``addr``.
+ *
+ * @param[in] name
+ *	device textual description
+ *
+ * @param[out] addr
+ *	device information location address, into which parsed info
+ *	should be written. If NULL, nothing should be written, which
+ *	is not an error.
+ *
+ * @return
+ *	0 if parsing was successful.
+ *	!0 for any error.
+ */
+typedef int (*rte_bus_parse_t)(const char *name, void *addr);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -127,6 +147,7 @@  struct rte_bus {
 	rte_bus_find_device_t find_device; /**< Find device on bus */
 	rte_bus_plug_t plug;         /**< Probe single device for drivers */
 	rte_bus_unplug_t unplug;     /**< Remove single device from driver */
+	rte_bus_parse_t parse;       /**< Parse a device name */
 };
 
 /**