[dpdk-dev,v7,2/6] bus: introduce parsing functionality
Checks
Commit Message
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
On Wednesday 05 July 2017 05:25 AM, 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: Shreyansh Jain <shreyansh.jain@nxp.com>
Hi Gaetan,
On Wednesday 05 July 2017 05:25 AM, 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>
> ---
> 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 773b0d7..aebf57e 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,
> 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.
> + *
r / is not a error/ is valid?
> + * @return
> + * 0 if parsing was successful.
> + * !0 for any error.
> + */
> +typedef int (*rte_bus_parse_t)(const char *name, void *addr);
> +
_parse handler in _common_vdev or common_pci file return boolean value
i.e..0 for success and 1 for error, right? if so then
!0 for any error would be like '1' for error case.. make sense?
On Thu, Jul 06, 2017 at 02:49:41PM +0530, santosh wrote:
> Hi Gaetan,
>
> On Wednesday 05 July 2017 05:25 AM, 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>
> > ---
> > 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 773b0d7..aebf57e 100644
> > --- a/lib/librte_eal/common/include/rte_bus.h
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,
> > 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.
> > + *
>
> r / is not a error/ is valid?
>
I'm partial to "is not an error" here, but it doesn't matter that much
and I can change it if you prefer.
> > + * @return
> > + * 0 if parsing was successful.
> > + * !0 for any error.
> > + */
> > +typedef int (*rte_bus_parse_t)(const char *name, void *addr);
> > +
>
> _parse handler in _common_vdev or common_pci file return boolean value
> i.e..0 for success and 1 for error, right? if so then
> !0 for any error would be like '1' for error case.. make sense?
>
I thought of that yes, and actually your suggestion was the first
version I used.
Ultimately however, this function is not only saying "can parse": it is
not merely a test of being able to process the input, but also the
process itself. The test value is then a byproduct.
As such, I decided to settle on the standard "0 means nothing of note
happened, carry on".
On Thursday 06 July 2017 06:00 PM, Gaëtan Rivet wrote:
> On Thu, Jul 06, 2017 at 02:49:41PM +0530, santosh wrote:
>> Hi Gaetan,
>>
>> On Wednesday 05 July 2017 05:25 AM, 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>
>>> ---
>>> 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 773b0d7..aebf57e 100644
>>> --- a/lib/librte_eal/common/include/rte_bus.h
>>> +++ b/lib/librte_eal/common/include/rte_bus.h
>>> @@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,
>>> 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.
>>> + *
>> r / is not a error/ is valid?
>>
> I'm partial to "is not an error" here, but it doesn't matter that much
> and I can change it if you prefer.
>
>>> + * @return
>>> + * 0 if parsing was successful.
>>> + * !0 for any error.
>>> + */
>>> +typedef int (*rte_bus_parse_t)(const char *name, void *addr);
>>> +
>> _parse handler in _common_vdev or common_pci file return boolean value
>> i.e..0 for success and 1 for error, right? if so then
>> !0 for any error would be like '1' for error case.. make sense?
>>
> I thought of that yes, and actually your suggestion was the first
> version I used.
>
> Ultimately however, this function is not only saying "can parse": it is
> not merely a test of being able to process the input, but also the
> process itself. The test value is then a byproduct.
>
> As such, I decided to settle on the standard "0 means nothing of note
> happened, carry on".
I'm not aware of past work history, catching up with stuff so no
strong opinion but In my opinion: if we're sure about return values then better
mention them explicitly.
Thanks.
On Thu, Jul 06, 2017 at 06:42:23PM +0530, santosh wrote:
> On Thursday 06 July 2017 06:00 PM, Gaëtan Rivet wrote:
>
> > On Thu, Jul 06, 2017 at 02:49:41PM +0530, santosh wrote:
> >> Hi Gaetan,
> >>
> >> On Wednesday 05 July 2017 05:25 AM, 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>
> >>> ---
> >>> 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 773b0d7..aebf57e 100644
> >>> --- a/lib/librte_eal/common/include/rte_bus.h
> >>> +++ b/lib/librte_eal/common/include/rte_bus.h
> >>> @@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,
> >>> 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.
> >>> + *
> >> r / is not a error/ is valid?
> >>
> > I'm partial to "is not an error" here, but it doesn't matter that much
> > and I can change it if you prefer.
> >
> >>> + * @return
> >>> + * 0 if parsing was successful.
> >>> + * !0 for any error.
> >>> + */
> >>> +typedef int (*rte_bus_parse_t)(const char *name, void *addr);
> >>> +
> >> _parse handler in _common_vdev or common_pci file return boolean value
> >> i.e..0 for success and 1 for error, right? if so then
> >> !0 for any error would be like '1' for error case.. make sense?
> >>
> > I thought of that yes, and actually your suggestion was the first
> > version I used.
> >
> > Ultimately however, this function is not only saying "can parse": it is
> > not merely a test of being able to process the input, but also the
> > process itself. The test value is then a byproduct.
> >
> > As such, I decided to settle on the standard "0 means nothing of note
> > happened, carry on".
>
> I'm not aware of past work history, catching up with stuff so no
> strong opinion but In my opinion: if we're sure about return values then better
> mention them explicitly.
>
That's the way it has been implemented in vdev and pci, but other buses
might want to use internal helpers in their version, that might return
something else than 1 on error.
As such, I think it's better to push the weakest specification
sufficient to match our needs and not force unnecessary hoops on
developers downstream.
> Thanks.
>
>
@@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,
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 {
@@ -148,6 +168,7 @@ struct rte_bus {
rte_bus_find_device_t find_device; /**< Find a device on the 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 */
};
/**