diff mbox series

[v4,2/2] event/skeleton: set driver name string

Message ID 20231018122558.126686-2-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series [v4,1/2] event/*: set device pointer for vdev-based eventdevs |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

Bruce Richardson Oct. 18, 2023, 12:25 p.m. UTC 
  When calling rte_eventdev_info_get() the driver name string field should
be populated.

Fixes: bbbb929da5e6 ("event/skeleton: add skeleton eventdev driver")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/event/skeleton/skeleton_eventdev.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

David Marchand Oct. 18, 2023, 12:44 p.m. UTC | #1 
On Wed, Oct 18, 2023 at 2:26 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> When calling rte_eventdev_info_get() the driver name string field should
> be populated.
>
> Fixes: bbbb929da5e6 ("event/skeleton: add skeleton eventdev driver")
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

event/dpaa2 seems affected too.

Can rte_eventdev_info_get() fill this based on the driver attached to
the device, like ethdev does?

Something like untested:

diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 95373bbaad..37ccc0dc77 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -104,6 +104,7 @@ rte_event_dev_info_get(uint8_t dev_id, struct
rte_event_dev_info *dev_info)
        dev_info->dequeue_timeout_ns = dev->data->dev_conf.dequeue_timeout_ns;

        dev_info->dev = dev->dev;
+       dev_info->driver_name = dev->dev->driver->name;

        rte_eventdev_trace_info_get(dev_id, dev_info, dev_info->dev);
Bruce Richardson Oct. 18, 2023, 12:52 p.m. UTC | #2 
On Wed, Oct 18, 2023 at 02:44:11PM +0200, David Marchand wrote:
> On Wed, Oct 18, 2023 at 2:26 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > When calling rte_eventdev_info_get() the driver name string field should
> > be populated.
> >
> > Fixes: bbbb929da5e6 ("event/skeleton: add skeleton eventdev driver")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> event/dpaa2 seems affected too.
> 

Ok, hadn't noticed that. I tested all the SW drivers I could and didn't
find any of them affected.

> Can rte_eventdev_info_get() fill this based on the driver attached to
> the device, like ethdev does?
> 
> Something like untested:
> 
> diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
> index 95373bbaad..37ccc0dc77 100644
> --- a/lib/eventdev/rte_eventdev.c
> +++ b/lib/eventdev/rte_eventdev.c
> @@ -104,6 +104,7 @@ rte_event_dev_info_get(uint8_t dev_id, struct
> rte_event_dev_info *dev_info)
>         dev_info->dequeue_timeout_ns = dev->data->dev_conf.dequeue_timeout_ns;
> 
>         dev_info->dev = dev->dev;
> +       dev_info->driver_name = dev->dev->driver->name;
> 
>         rte_eventdev_trace_info_get(dev_id, dev_info, dev_info->dev);
> 
Ok, let me do up and test a patch.

/Bruce
Bruce Richardson Oct. 18, 2023, 1:42 p.m. UTC | #3 
On Wed, Oct 18, 2023 at 01:52:08PM +0100, Bruce Richardson wrote:
> On Wed, Oct 18, 2023 at 02:44:11PM +0200, David Marchand wrote:
> > On Wed, Oct 18, 2023 at 2:26 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > When calling rte_eventdev_info_get() the driver name string field should
> > > be populated.
> > >
> > > Fixes: bbbb929da5e6 ("event/skeleton: add skeleton eventdev driver")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > 
> > event/dpaa2 seems affected too.
> > 
> 
> Ok, hadn't noticed that. I tested all the SW drivers I could and didn't
> find any of them affected.
> 
> > Can rte_eventdev_info_get() fill this based on the driver attached to
> > the device, like ethdev does?
> > 
> > Something like untested:
> > 
> > diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
> > index 95373bbaad..37ccc0dc77 100644
> > --- a/lib/eventdev/rte_eventdev.c
> > +++ b/lib/eventdev/rte_eventdev.c
> > @@ -104,6 +104,7 @@ rte_event_dev_info_get(uint8_t dev_id, struct
> > rte_event_dev_info *dev_info)
> >         dev_info->dequeue_timeout_ns = dev->data->dev_conf.dequeue_timeout_ns;
> > 
> >         dev_info->dev = dev->dev;
> > +       dev_info->driver_name = dev->dev->driver->name;
> > 
> >         rte_eventdev_trace_info_get(dev_id, dev_info, dev_info->dev);
> > 
> Ok, let me do up and test a patch.
> 
V5 sent with this fix instead. I just added some additional conditionals
around it, just in case we end up with some Null pointers in that chain
(hopefully not after the previous patch, but better to check).

Testing with skeleton, sw, dsw and opdl drivers showed no issues with
reporting the driver name.

/Bruce
diff mbox series

Patch

diff --git a/drivers/event/skeleton/skeleton_eventdev.c b/drivers/event/skeleton/skeleton_eventdev.c
index 7db1efaf14..6afb5de824 100644
--- a/drivers/event/skeleton/skeleton_eventdev.c
+++ b/drivers/event/skeleton/skeleton_eventdev.c
@@ -24,6 +24,7 @@ 
 
 #define EVENTDEV_NAME_SKELETON_PMD event_skeleton
 /**< Skeleton event device PMD name */
+#define EVENTDEV_NAME_STRING RTE_STR(EVENTDEV_NAME_SKELETON_PMD)
 
 static uint16_t
 skeleton_eventdev_enqueue(void *port, const struct rte_event *ev)
@@ -88,6 +89,7 @@  skeleton_eventdev_info_get(struct rte_eventdev *dev,
 
 	RTE_SET_USED(skel);
 
+	dev_info->driver_name = EVENTDEV_NAME_STRING;
 	dev_info->min_dequeue_timeout_ns = 1;
 	dev_info->max_dequeue_timeout_ns = 10000;
 	dev_info->dequeue_timeout_ns = 25;