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

[Van Haaren, Harry]

> -----Original Message-----
> From: Fredrik A Lindgren <Fredrik.a.Lindgren at tietoevry.com>
> Sent: Monday, February 1, 2021 12:50 PM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>; dev at 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 at intel.com>
> Sent: den 1 februari 2021 11:31
> To: Fredrik A Lindgren <Fredrik.a.Lindgren at tieto.com>; dev at dpdk.org
> Subject: RE: Question: typedef of eventdev_info_get_t bug?
> 
> > -----Original Message-----
> > From: Fredrik A Lindgren <Fredrik.a.Lindgren at tietoevry.com>
> > Sent: Monday, February 1, 2021 10:02 AM
> > To: Van Haaren, Harry <harry.van.haaren at intel.com>; dev at 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 at intel.com>
> > Sent: den 1 februari 2021 10:44
> > To: Fredrik A Lindgren <Fredrik.a.Lindgren at tieto.com>; dev at dpdk.org
> > Subject: RE: Question: typedef of eventdev_info_get_t bug?
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces at dpdk.org> On Behalf Of Fredrik A Lindgren
> > > Sent: Monday, February 1, 2021 9:22 AM
> > > To: dev at 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