[dpdk-dev] Question: typedef of eventdev_info_get_t bug?

[dpdk-dev] Question: typedef of eventdev_info_get_t bug?

[Fredrik A Lindgren]

While debugging a driver I was looking at the code and realized that eventdev_info_get_t typedef is defined with void return value but documentation for it says it should return 0

lib/librte_eventdev/eventdev_pmd.h:

...
* @return
*   Returns 0 on success
*/
typedef void (*eventdev_info_get_t)(struct rte_eventdev *dev,
                                                          struct rte_event_dev_info *dev_info);
...
struct rte_eventdev_ops {
                             eventdev_info_get_t dev_infos_get;              /**< Get device info. */
...

While return value from it is used in lib/librte_ethdev/rte_ethdev.c:

int
rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
...
   diag = (*dev->dev_ops->dev_infos_get)(dev, dev_info);
   if (diag != 0) {
...


Driver I'm debugging doesn't have any return statement in it's dev_infos_get function which seems to cause it to have "result" of last operation done in that function as return value.
Though this behavior may be compiler specific but it should probably be clarified and updated (change prototype of stop using return value) to avoid issue with it.

Br,
Fredrik

Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?

[Van Haaren, Harry]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Fredrik A Lindgren
> Sent: Monday, February 1, 2021 9:22 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
> 
> While debugging a driver I was looking at the code and realized that
> eventdev_info_get_t typedef is defined with void return value but documentation
> for it says it should return 0

Hi Fredrik,

Be aware that your snippets of code below is mixing Event-dev and Eth-dev.
This is likely the cause of confusion.


> lib/librte_eventdev/eventdev_pmd.h:
> 
> ...
> * @return
> *   Returns 0 on success
> */
> typedef void (*eventdev_info_get_t)(struct rte_eventdev *dev,
>                                                           struct rte_event_dev_info *dev_info);
> ...
> struct rte_eventdev_ops {
>                              eventdev_info_get_t dev_infos_get;              /**< Get device info. */
> ...
> 
> While return value from it is used in lib/librte_ethdev/rte_ethdev.c:
> 
> int
> rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
> ...
>    diag = (*dev->dev_ops->dev_infos_get)(dev, dev_info);
>    if (diag != 0) {
> ...

At the Eventdev layer, info_get() can return int (particularly 0 on success, or -ERROR values).
At the Eventdev PMD layer, info_get() has a return void (the PMD cannot indicate failure on providing its info)

Hence, the info_get() typedef (for the PMD) is void, but the Eventdev layer itself checks some dev_id values
etc, and can return -EINVAL or -ENOTSUP in case the PMD doesn't support info_get().


> Driver I'm debugging doesn't have any return statement in it's dev_infos_get
> function which seems to cause it to have "result" of last operation done in that
> function as return value.
> Though this behavior may be compiler specific but it should probably be clarified and
> updated (change prototype of stop using return value) to avoid issue with it.

Perhaps provide some more detail on Eventdev/Ethdev that you're debugging a driver on,
provide some links to specific parts of the code? It seems like (from Eventdev anyway) its
meant to work this way.

> Br,
> Fredrik

Regards, -Harry

Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?

[Fredrik A Lindgren]

Hi Harry,

You are correct, multiple structs with dev_infos_get defined in them caused me to jump to wrong implementation.

Still a bit "wrong" that comment for eventdev_info_get_t for event setup states returns 0 on success.

Using older version of https://github.com/Xilinx/dma_ip_drivers/tree/master/QDMA/DPDK targeting dpdk-19.11 for a corporate internal project as such can't give much details.
That version had a issue with no return statements in the infos_get function which was causing me issues.

Br,
Fredrik

-----Original Message-----
From: Van Haaren, Harry <harry.van.haaren@intel.com> 
Sent: den 1 februari 2021 10:44
To: Fredrik A Lindgren <Fredrik.a.Lindgren@tieto.com>; dev@dpdk.org
Subject: RE: Question: typedef of eventdev_info_get_t bug?

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Fredrik A Lindgren
> Sent: Monday, February 1, 2021 9:22 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
> 
> While debugging a driver I was looking at the code and realized that 
> eventdev_info_get_t typedef is defined with void return value but 
> documentation for it says it should return 0

Hi Fredrik,

Be aware that your snippets of code below is mixing Event-dev and Eth-dev.
This is likely the cause of confusion.


> lib/librte_eventdev/eventdev_pmd.h:
> 
> ...
> * @return
> *   Returns 0 on success
> */
> typedef void (*eventdev_info_get_t)(struct rte_eventdev *dev,
>                                                           struct 
> rte_event_dev_info *dev_info); ...
> struct rte_eventdev_ops {
>                              eventdev_info_get_t dev_infos_get;              /**< Get device info. */
> ...
> 
> While return value from it is used in lib/librte_ethdev/rte_ethdev.c:
> 
> int
> rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info 
> *dev_info) ...
>    diag = (*dev->dev_ops->dev_infos_get)(dev, dev_info);
>    if (diag != 0) {
> ...

At the Eventdev layer, info_get() can return int (particularly 0 on success, or -ERROR values).
At the Eventdev PMD layer, info_get() has a return void (the PMD cannot indicate failure on providing its info)

Hence, the info_get() typedef (for the PMD) is void, but the Eventdev layer itself checks some dev_id values etc, and can return -EINVAL or -ENOTSUP in case the PMD doesn't support info_get().


> Driver I'm debugging doesn't have any return statement in it's 
> dev_infos_get function which seems to cause it to have "result" of 
> last operation done in that function as return value.
> Though this behavior may be compiler specific but it should probably 
> be clarified and updated (change prototype of stop using return value) to avoid issue with it.

Perhaps provide some more detail on Eventdev/Ethdev that you're debugging a driver on, provide some links to specific parts of the code? It seems like (from Eventdev anyway) its meant to work this way.

> Br,
> Fredrik

Regards, -Harry

Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?

[Van Haaren, Harry]

> -----Original Message-----
> From: Fredrik A Lindgren <Fredrik.a.Lindgren@tietoevry.com>
> Sent: Monday, February 1, 2021 10:02 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Subject: RE: Question: typedef of eventdev_info_get_t bug?
> 
> Hi Harry,
> 
> You are correct, multiple structs with dev_infos_get defined in them caused me to
> jump to wrong implementation.

No problem.

> Still a bit "wrong" that comment for eventdev_info_get_t for event setup states
> returns 0 on success.

Agreed, a patch would be welcome to remove the "@return" bit of the docs.
I can send it if you say you're not planning to, its a small/easy patch.

> Using older version of
> https://github.com/Xilinx/dma_ip_drivers/tree/master/QDMA/DPDK targeting dpdk-
> 19.11 for a corporate internal project as such can't give much details.
> That version had a issue with no return statements in the infos_get function which
> was causing me issues.

Sounds like you've root-caused and know of a solution.

> Br,
> Fredrik

Regards, -Harry

> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: den 1 februari 2021 10:44
> To: Fredrik A Lindgren <Fredrik.a.Lindgren@tieto.com>; dev@dpdk.org
> Subject: RE: Question: typedef of eventdev_info_get_t bug?
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Fredrik A Lindgren
> > Sent: Monday, February 1, 2021 9:22 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
> >
> > While debugging a driver I was looking at the code and realized that
> > eventdev_info_get_t typedef is defined with void return value but
> > documentation for it says it should return 0
> 
> Hi Fredrik,
> 
> Be aware that your snippets of code below is mixing Event-dev and Eth-dev.
> This is likely the cause of confusion.
> 
> 
> > lib/librte_eventdev/eventdev_pmd.h:
> >
> > ...
> > * @return
> > *   Returns 0 on success
> > */
> > typedef void (*eventdev_info_get_t)(struct rte_eventdev *dev,
> >                                                           struct
> > rte_event_dev_info *dev_info); ...
> > struct rte_eventdev_ops {
> >                              eventdev_info_get_t dev_infos_get;              /**< Get device info.
> */
> > ...
> >
> > While return value from it is used in lib/librte_ethdev/rte_ethdev.c:
> >
> > int
> > rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info
> > *dev_info) ...
> >    diag = (*dev->dev_ops->dev_infos_get)(dev, dev_info);
> >    if (diag != 0) {
> > ...
> 
> At the Eventdev layer, info_get() can return int (particularly 0 on success, or -ERROR
> values).
> At the Eventdev PMD layer, info_get() has a return void (the PMD cannot indicate
> failure on providing its info)
> 
> Hence, the info_get() typedef (for the PMD) is void, but the Eventdev layer itself
> checks some dev_id values etc, and can return -EINVAL or -ENOTSUP in case the
> PMD doesn't support info_get().
> 
> 
> > Driver I'm debugging doesn't have any return statement in it's
> > dev_infos_get function which seems to cause it to have "result" of
> > last operation done in that function as return value.
> > Though this behavior may be compiler specific but it should probably
> > be clarified and updated (change prototype of stop using return value) to avoid
> issue with it.
> 
> Perhaps provide some more detail on Eventdev/Ethdev that you're debugging a
> driver on, provide some links to specific parts of the code? It seems like (from
> Eventdev anyway) its meant to work this way.
> 
> > Br,
> > Fredrik
> 
> Regards, -Harry

Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?

[Fredrik A Lindgren]

I'm not 100% familiar with contribution guidelines of DPDK community and since I'm doing this on work hours Corporate praxis is to get legal approval to provide contributions to opensource
So if you have the time and already are familiar with contributing to DPDK feel free to submit such patch

Yes gotten around at least that issue, have other suspect behaviors when trying to use it though.
But those are most likely related to driver and as such probably for a discussion outside this mailing list with Xilinx

Br,
Fredrik

-----Original Message-----
From: Van Haaren, Harry <harry.van.haaren@intel.com> 
Sent: den 1 februari 2021 11:31
To: Fredrik A Lindgren <Fredrik.a.Lindgren@tieto.com>; dev@dpdk.org
Subject: RE: Question: typedef of eventdev_info_get_t bug?

> -----Original Message-----
> From: Fredrik A Lindgren <Fredrik.a.Lindgren@tietoevry.com>
> Sent: Monday, February 1, 2021 10:02 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Subject: RE: Question: typedef of eventdev_info_get_t bug?
> 
> Hi Harry,
> 
> You are correct, multiple structs with dev_infos_get defined in them 
> caused me to jump to wrong implementation.

No problem.

> Still a bit "wrong" that comment for eventdev_info_get_t for event 
> setup states returns 0 on success.

Agreed, a patch would be welcome to remove the "@return" bit of the docs.
I can send it if you say you're not planning to, its a small/easy patch.

> Using older version of
> https://github.com/Xilinx/dma_ip_drivers/tree/master/QDMA/DPDK 
> targeting dpdk-
> 19.11 for a corporate internal project as such can't give much details.
> That version had a issue with no return statements in the infos_get 
> function which was causing me issues.

Sounds like you've root-caused and know of a solution.

> Br,
> Fredrik

Regards, -Harry

> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: den 1 februari 2021 10:44
> To: Fredrik A Lindgren <Fredrik.a.Lindgren@tieto.com>; dev@dpdk.org
> Subject: RE: Question: typedef of eventdev_info_get_t bug?
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Fredrik A Lindgren
> > Sent: Monday, February 1, 2021 9:22 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
> >
> > While debugging a driver I was looking at the code and realized that 
> > eventdev_info_get_t typedef is defined with void return value but 
> > documentation for it says it should return 0
> 
> Hi Fredrik,
> 
> Be aware that your snippets of code below is mixing Event-dev and Eth-dev.
> This is likely the cause of confusion.
> 
> 
> > lib/librte_eventdev/eventdev_pmd.h:
> >
> > ...
> > * @return
> > *   Returns 0 on success
> > */
> > typedef void (*eventdev_info_get_t)(struct rte_eventdev *dev,
> >                                                           struct 
> > rte_event_dev_info *dev_info); ...
> > struct rte_eventdev_ops {
> >                              eventdev_info_get_t dev_infos_get;              /**< Get device info.
> */
> > ...
> >
> > While return value from it is used in lib/librte_ethdev/rte_ethdev.c:
> >
> > int
> > rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info
> > *dev_info) ...
> >    diag = (*dev->dev_ops->dev_infos_get)(dev, dev_info);
> >    if (diag != 0) {
> > ...
> 
> At the Eventdev layer, info_get() can return int (particularly 0 on 
> success, or -ERROR values).
> At the Eventdev PMD layer, info_get() has a return void (the PMD 
> cannot indicate failure on providing its info)
> 
> Hence, the info_get() typedef (for the PMD) is void, but the Eventdev 
> layer itself checks some dev_id values etc, and can return -EINVAL or 
> -ENOTSUP in case the PMD doesn't support info_get().
> 
> 
> > Driver I'm debugging doesn't have any return statement in it's 
> > dev_infos_get function which seems to cause it to have "result" of 
> > last operation done in that function as return value.
> > Though this behavior may be compiler specific but it should probably 
> > be clarified and updated (change prototype of stop using return 
> > value) to avoid
> issue with it.
> 
> Perhaps provide some more detail on Eventdev/Ethdev that you're 
> debugging a driver on, provide some links to specific parts of the 
> code? It seems like (from Eventdev anyway) its meant to work this way.
> 
> > Br,
> > Fredrik
> 
> Regards, -Harry

Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?

[Van Haaren, Harry]

> -----Original Message-----
> From: Fredrik A Lindgren <Fredrik.a.Lindgren@tietoevry.com>
> Sent: Monday, February 1, 2021 12:50 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Subject: RE: Question: typedef of eventdev_info_get_t bug?
> 
> I'm not 100% familiar with contribution guidelines of DPDK community and since I'm
> doing this on work hours Corporate praxis is to get legal approval to provide
> contributions to opensource
> So if you have the time and already are familiar with contributing to DPDK feel free
> to submit such patch

Sure, patch sent: http://patches.dpdk.org/patch/87588/ I've noted you reported this using
the Reported-by: tag, as per how DPDK project tracks contributions, thanks!

(https://doc.dpdk.org/guides/contributing/patches.html#tested-acked-and-reviewed-by)

> Yes gotten around at least that issue, have other suspect behaviors when trying to
> use it though.
> But those are most likely related to driver and as such probably for a discussion
> outside this mailing list with Xilinx
> 
> Br,
> Fredrik

Side note: on mailing lists (like this one) it is best to avoid "top posting", as it makes
discussions difficult to follow. Adding replies "inline" like this comment is best practice.

Regards, -Harry

> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: den 1 februari 2021 11:31
> To: Fredrik A Lindgren <Fredrik.a.Lindgren@tieto.com>; dev@dpdk.org
> Subject: RE: Question: typedef of eventdev_info_get_t bug?
> 
> > -----Original Message-----
> > From: Fredrik A Lindgren <Fredrik.a.Lindgren@tietoevry.com>
> > Sent: Monday, February 1, 2021 10:02 AM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> > Subject: RE: Question: typedef of eventdev_info_get_t bug?
> >
> > Hi Harry,
> >
> > You are correct, multiple structs with dev_infos_get defined in them
> > caused me to jump to wrong implementation.
> 
> No problem.
> 
> > Still a bit "wrong" that comment for eventdev_info_get_t for event
> > setup states returns 0 on success.
> 
> Agreed, a patch would be welcome to remove the "@return" bit of the docs.
> I can send it if you say you're not planning to, its a small/easy patch.
> 
> > Using older version of
> > https://github.com/Xilinx/dma_ip_drivers/tree/master/QDMA/DPDK
> > targeting dpdk-
> > 19.11 for a corporate internal project as such can't give much details.
> > That version had a issue with no return statements in the infos_get
> > function which was causing me issues.
> 
> Sounds like you've root-caused and know of a solution.
> 
> > Br,
> > Fredrik
> 
> Regards, -Harry
> 
> > -----Original Message-----
> > From: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Sent: den 1 februari 2021 10:44
> > To: Fredrik A Lindgren <Fredrik.a.Lindgren@tieto.com>; dev@dpdk.org
> > Subject: RE: Question: typedef of eventdev_info_get_t bug?
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Fredrik A Lindgren
> > > Sent: Monday, February 1, 2021 9:22 AM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
> > >
> > > While debugging a driver I was looking at the code and realized that
> > > eventdev_info_get_t typedef is defined with void return value but
> > > documentation for it says it should return 0
> >
> > Hi Fredrik,
> >
> > Be aware that your snippets of code below is mixing Event-dev and Eth-dev.
> > This is likely the cause of confusion.
> >
> >
> > > lib/librte_eventdev/eventdev_pmd.h:
> > >
> > > ...
> > > * @return
> > > *   Returns 0 on success
> > > */
> > > typedef void (*eventdev_info_get_t)(struct rte_eventdev *dev,
> > >                                                           struct
> > > rte_event_dev_info *dev_info); ...
> > > struct rte_eventdev_ops {
> > >                              eventdev_info_get_t dev_infos_get;              /**< Get device info.
> > */
> > > ...
> > >
> > > While return value from it is used in lib/librte_ethdev/rte_ethdev.c:
> > >
> > > int
> > > rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info
> > > *dev_info) ...
> > >    diag = (*dev->dev_ops->dev_infos_get)(dev, dev_info);
> > >    if (diag != 0) {
> > > ...
> >
> > At the Eventdev layer, info_get() can return int (particularly 0 on
> > success, or -ERROR values).
> > At the Eventdev PMD layer, info_get() has a return void (the PMD
> > cannot indicate failure on providing its info)
> >
> > Hence, the info_get() typedef (for the PMD) is void, but the Eventdev
> > layer itself checks some dev_id values etc, and can return -EINVAL or
> > -ENOTSUP in case the PMD doesn't support info_get().
> >
> >
> > > Driver I'm debugging doesn't have any return statement in it's
> > > dev_infos_get function which seems to cause it to have "result" of
> > > last operation done in that function as return value.
> > > Though this behavior may be compiler specific but it should probably
> > > be clarified and updated (change prototype of stop using return
> > > value) to avoid
> > issue with it.
> >
> > Perhaps provide some more detail on Eventdev/Ethdev that you're
> > debugging a driver on, provide some links to specific parts of the
> > code? It seems like (from Eventdev anyway) its meant to work this way.
> >
> > > Br,
> > > Fredrik
> >
> > Regards, -Harry