* [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
@ 2021-02-01 9:21 Fredrik A Lindgren
2021-02-01 9:44 ` Van Haaren, Harry
0 siblings, 1 reply; 6+ messages in thread
From: Fredrik A Lindgren @ 2021-02-01 9:21 UTC (permalink / raw)
To: dev
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
2021-02-01 9:21 [dpdk-dev] Question: typedef of eventdev_info_get_t bug? Fredrik A Lindgren
@ 2021-02-01 9:44 ` Van Haaren, Harry
2021-02-01 10:02 ` Fredrik A Lindgren
0 siblings, 1 reply; 6+ messages in thread
From: Van Haaren, Harry @ 2021-02-01 9:44 UTC (permalink / raw)
To: Fredrik A Lindgren, dev
> -----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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
2021-02-01 9:44 ` Van Haaren, Harry
@ 2021-02-01 10:02 ` Fredrik A Lindgren
2021-02-01 10:31 ` Van Haaren, Harry
0 siblings, 1 reply; 6+ messages in thread
From: Fredrik A Lindgren @ 2021-02-01 10:02 UTC (permalink / raw)
To: Van Haaren, Harry, dev
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
2021-02-01 10:02 ` Fredrik A Lindgren
@ 2021-02-01 10:31 ` Van Haaren, Harry
2021-02-01 12:49 ` Fredrik A Lindgren
0 siblings, 1 reply; 6+ messages in thread
From: Van Haaren, Harry @ 2021-02-01 10:31 UTC (permalink / raw)
To: Fredrik A Lindgren, dev
> -----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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
2021-02-01 10:31 ` Van Haaren, Harry
@ 2021-02-01 12:49 ` Fredrik A Lindgren
2021-02-01 13:23 ` Van Haaren, Harry
0 siblings, 1 reply; 6+ messages in thread
From: Fredrik A Lindgren @ 2021-02-01 12:49 UTC (permalink / raw)
To: Van Haaren, Harry, dev
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] Question: typedef of eventdev_info_get_t bug?
2021-02-01 12:49 ` Fredrik A Lindgren
@ 2021-02-01 13:23 ` Van Haaren, Harry
0 siblings, 0 replies; 6+ messages in thread
From: Van Haaren, Harry @ 2021-02-01 13:23 UTC (permalink / raw)
To: Fredrik A Lindgren, dev
> -----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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-01 13:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01 9:21 [dpdk-dev] Question: typedef of eventdev_info_get_t bug? Fredrik A Lindgren
2021-02-01 9:44 ` Van Haaren, Harry
2021-02-01 10:02 ` Fredrik A Lindgren
2021-02-01 10:31 ` Van Haaren, Harry
2021-02-01 12:49 ` Fredrik A Lindgren
2021-02-01 13:23 ` Van Haaren, Harry
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).