[dpdk-dev] eventdev: add dev id checks to config functions

Message ID 1500900500-144237-1-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Van Haaren, Harry July 24, 2017, 12:48 p.m. UTC
  This commit adds checks to verify the device ID is valid
to the following functions. Given that they are non-datapath,
these checks are always performed.

This commit also updates the event/octeontx test-cases to
have the correct signed-ness, as the API has changes this
change is required in order to compile.

Suggested-by: Jesse Bruni <jesse.bruni@intel.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---
 lib/librte_eventdev/rte_eventdev.c | 36 +++++++++++++++++++++++++-----------
 lib/librte_eventdev/rte_eventdev.h | 36 ++++++++++++++++++------------------
 test/test/test_eventdev_octeontx.c | 32 +++++++++++++++++++-------------
 3 files changed, 62 insertions(+), 42 deletions(-)
  

Comments

Jerin Jacob Sept. 4, 2017, 5:20 a.m. UTC | #1
-----Original Message-----
> Date: Mon, 24 Jul 2017 13:48:20 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH] eventdev: add dev id checks to config functions
> X-Mailer: git-send-email 2.7.4
> 
> This commit adds checks to verify the device ID is valid
> to the following functions. Given that they are non-datapath,
> these checks are always performed.

Makes sense.

> 
> This commit also updates the event/octeontx test-cases to
> have the correct signed-ness, as the API has changes this
> change is required in order to compile.
> 
> Suggested-by: Jesse Bruni <jesse.bruni@intel.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> @@ -1288,9 +1293,10 @@ worker_ordered_flow_producer(void *arg)
>  static inline int
>  test_producer_consumer_ingress_order_test(int (*fn)(void *))
>  {
> -	uint8_t nr_ports;
> +	int16_t nr_ports;
>  
> -	nr_ports = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1);
> +	nr_ports = RTE_MIN(rte_event_port_count(evdev),
> +			(int)rte_lcore_count() - 1);

While I agree on the problem statement, I am trying to see
1/ an API symmetrical to ethdev APIs. Similar problem solved in a differently in
ethdev. see rte_eth_dev_adjust_nb_rx_tx_desc().
Just want to make sure, all the APIs across ethdev, eventdev looks same

2/ How to get rid of above typecasting

Considering above two points and following the
rte_eth_dev_adjust_nb_rx_tx_desc() pattern. How about,

Removing,
rte_event_port_dequeue_depth()
rte_event_port_enqueue_depth()
rte_event_port_count()

rte_event_queue_count()
rte_event_queue_priority()

and change to,

int rte_event_port_attr_get(uint8_t dev_id, uint8_t port_id,
uint8_t *enqueue_depth /*out */, uint8_t *dequeue_depth /* out*/, uin8_t *count /*out*/);

int rte_event_queue_attr_get(uint8_t dev_id, uint8_t port_id,
uin8_t *prio /* out */, uint8_t *count /*out */);

or something similar.
  
Van Haaren, Harry Sept. 6, 2017, 2:45 p.m. UTC | #2
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, September 4, 2017 6:21 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] eventdev: add dev id checks to config functions
> 
> -----Original Message-----
> > Date: Mon, 24 Jul 2017 13:48:20 +0100
> > From: Harry van Haaren <harry.van.haaren@intel.com>
> > To: dev@dpdk.org
> > CC: jerin.jacob@caviumnetworks.com, Harry van Haaren
> >  <harry.van.haaren@intel.com>
> > Subject: [PATCH] eventdev: add dev id checks to config functions
> > X-Mailer: git-send-email 2.7.4
> >
> > This commit adds checks to verify the device ID is valid
> > to the following functions. Given that they are non-datapath,
> > these checks are always performed.
> 
> Makes sense.

Great - lets discuss implementation ;)


> > This commit also updates the event/octeontx test-cases to
> > have the correct signed-ness, as the API has changes this
> > change is required in order to compile.
> >
> > Suggested-by: Jesse Bruni <jesse.bruni@intel.com>
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > ---
> > @@ -1288,9 +1293,10 @@ worker_ordered_flow_producer(void *arg)
> >  static inline int
> >  test_producer_consumer_ingress_order_test(int (*fn)(void *))
> >  {
> > -	uint8_t nr_ports;
> > +	int16_t nr_ports;
> >
> > -	nr_ports = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1);
> > +	nr_ports = RTE_MIN(rte_event_port_count(evdev),
> > +			(int)rte_lcore_count() - 1);
> 
> While I agree on the problem statement, I am trying to see
> 1/ an API symmetrical to ethdev APIs. Similar problem solved in a differently in
> ethdev. see rte_eth_dev_adjust_nb_rx_tx_desc().
> Just want to make sure, all the APIs across ethdev, eventdev looks same
> 
> 2/ How to get rid of above typecasting
> 
> Considering above two points and following the
> rte_eth_dev_adjust_nb_rx_tx_desc() pattern. How about,
> 
> Removing,
> rte_event_port_dequeue_depth()
> rte_event_port_enqueue_depth()
> rte_event_port_count()
> 
> rte_event_queue_count()
> rte_event_queue_priority()
> 
> and change to,
> 
> int rte_event_port_attr_get(uint8_t dev_id, uint8_t port_id,
> uint8_t *enqueue_depth /*out */, uint8_t *dequeue_depth /* out*/, uin8_t *count /*out*/);
> 
> int rte_event_queue_attr_get(uint8_t dev_id, uint8_t port_id,
> uin8_t *prio /* out */, uint8_t *count /*out */);
> 
> or something similar.

Hmm, I don't like that we'd have to break ABI every time we want to add an item to attr_get().. so adding a parameter "attr_id" would allow adding events in future. This solution feels a bit like a re-implementation of the xstats API..

Thoughts? -H


enum {
  PORT_COUNT,
  PORT_DEQUEUE_DEPTH,
  PORT_ENQUEUE_DEPTH,
}

/* retrieve value of port 
int rte_event_port_attr_get(uint8_t dev_id, uint8_t port_id, uint32_t attr_id, uint32_t *attr_value /* out */);
  
Jerin Jacob Sept. 6, 2017, 2:57 p.m. UTC | #3
-----Original Message-----
> Date: Wed, 6 Sep 2017 14:45:29 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>
> Subject: RE: [PATCH] eventdev: add dev id checks to config functions
> 
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, September 4, 2017 6:21 AM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH] eventdev: add dev id checks to config functions
> > 
> > -----Original Message-----
> > > Date: Mon, 24 Jul 2017 13:48:20 +0100
> > > From: Harry van Haaren <harry.van.haaren@intel.com>
> > > To: dev@dpdk.org
> > > CC: jerin.jacob@caviumnetworks.com, Harry van Haaren
> > >  <harry.van.haaren@intel.com>
> > > Subject: [PATCH] eventdev: add dev id checks to config functions
> > > X-Mailer: git-send-email 2.7.4
> > >
> > > This commit adds checks to verify the device ID is valid
> > > to the following functions. Given that they are non-datapath,
> > > these checks are always performed.
> > 
> > Makes sense.
> 
> Great - lets discuss implementation ;)
> 
> 
> > > This commit also updates the event/octeontx test-cases to
> > > have the correct signed-ness, as the API has changes this
> > > change is required in order to compile.
> > >
> > > Suggested-by: Jesse Bruni <jesse.bruni@intel.com>
> > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > >
> > > ---
> > > @@ -1288,9 +1293,10 @@ worker_ordered_flow_producer(void *arg)
> > >  static inline int
> > >  test_producer_consumer_ingress_order_test(int (*fn)(void *))
> > >  {
> > > -	uint8_t nr_ports;
> > > +	int16_t nr_ports;
> > >
> > > -	nr_ports = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1);
> > > +	nr_ports = RTE_MIN(rte_event_port_count(evdev),
> > > +			(int)rte_lcore_count() - 1);
> > 
> > While I agree on the problem statement, I am trying to see
> > 1/ an API symmetrical to ethdev APIs. Similar problem solved in a differently in
> > ethdev. see rte_eth_dev_adjust_nb_rx_tx_desc().
> > Just want to make sure, all the APIs across ethdev, eventdev looks same
> > 
> > 2/ How to get rid of above typecasting
> > 
> > Considering above two points and following the
> > rte_eth_dev_adjust_nb_rx_tx_desc() pattern. How about,
> > 
> > Removing,
> > rte_event_port_dequeue_depth()
> > rte_event_port_enqueue_depth()
> > rte_event_port_count()
> > 
> > rte_event_queue_count()
> > rte_event_queue_priority()
> > 
> > and change to,
> > 
> > int rte_event_port_attr_get(uint8_t dev_id, uint8_t port_id,
> > uint8_t *enqueue_depth /*out */, uint8_t *dequeue_depth /* out*/, uin8_t *count /*out*/);
> > 
> > int rte_event_queue_attr_get(uint8_t dev_id, uint8_t port_id,
> > uin8_t *prio /* out */, uint8_t *count /*out */);
> > 
> > or something similar.
> 
> Hmm, I don't like that we'd have to break ABI every time we want to add an item to attr_get().. so adding a parameter "attr_id" would allow adding events in future. This solution feels a bit like a re-implementation of the xstats API..
> 
> Thoughts? -H
> 
> 
> enum {
>   PORT_COUNT,
>   PORT_DEQUEUE_DEPTH,
>   PORT_ENQUEUE_DEPTH,
> }
> 
> /* retrieve value of port 
> int rte_event_port_attr_get(uint8_t dev_id, uint8_t port_id, uint32_t attr_id, uint32_t *attr_value /* out */);

Looks good to me.

> 
>
  
Van Haaren, Harry Sept. 8, 2017, 3:18 p.m. UTC | #4
This patchset refactors the eventdev API to be more flexible
and capable. In particular, the API is capable of returning an
error value if an invalid device, port or attribute ID is passed
in, which was not possible with the previous APIs.

The implementation of this patchset is based on a v1 patch[1],
and after some discussion this API was seen as the best solution.

In terms of flexibility, the attribute id allows addition of new
common eventdev layer attributes without breaking ABI or adding
new functions. Note that these attributes are not data-path, and
that PMDs should continue to use the xstats API for reporting any
unique PMD statistics that are available.

Regarding API/ABI compatibility, I have removed the functions from
the .map files - please review the .map file changes for ABI issues
carefully.

The last patch of this series adds a started attribute to the device,
allowing the application to query if a device is currently running.

-Harry

[1] http://dpdk.org/dev/patchwork/patch/27152/


Harry van Haaren (3):
  eventdev: add port attribute function
  eventdev: add dev attribute get function
  eventdev: add queue attribute function

 lib/librte_eventdev/rte_eventdev.c           |  88 +++++++++++------
 lib/librte_eventdev/rte_eventdev.h           | 115 ++++++++++-----------
 lib/librte_eventdev/rte_eventdev_version.map |  12 ++-
 test/test/test_eventdev.c                    | 132 +++++++++++++++++++------
 test/test/test_eventdev_octeontx.c           | 143 ++++++++++++++++++++-------
 5 files changed, 334 insertions(+), 156 deletions(-)
  
Van Haaren, Harry Sept. 8, 2017, 3:36 p.m. UTC | #5
This patchset refactors the eventdev API to be more flexible
and capable. In particular, the API is capable of returning an
error value if an invalid device, port or attribute ID is passed
in, which was not possible with the previous APIs.

The implementation of this patchset is based on a v1 patch[1],
and after some discussion this API was seen as the best solution.

In terms of flexibility, the attribute id allows addition of new
common eventdev layer attributes without breaking ABI or adding
new functions. Note that these attributes are not data-path, and
that PMDs should continue to use the xstats API for reporting any
unique PMD statistics that are available.

Regarding API/ABI compatibility, I have removed the functions from
the .map files - please review the .map file changes for ABI issues
carefully.

The last patch of this series adds a started attribute to the device,
allowing the application to query if a device is currently running.

-Harry

[1] http://dpdk.org/dev/patchwork/patch/27152/

---

v3:
- Fix checkpatch issues... somehow I broke my checkpatch script :/

v2:
- New APIs design based on discussion of initial patch.



Harry van Haaren (4):
  eventdev: add port attribute function
  eventdev: add dev attribute get function
  eventdev: add queue attribute function
  eventdev: add device started attribute

 lib/librte_eventdev/rte_eventdev.c           |  91 +++++++++++------
 lib/librte_eventdev/rte_eventdev.h           | 118 +++++++++++-----------
 lib/librte_eventdev/rte_eventdev_version.map |  12 ++-
 test/test/test_eventdev.c                    | 132 +++++++++++++++++++------
 test/test/test_eventdev_octeontx.c           | 143 ++++++++++++++++++++-------
 5 files changed, 340 insertions(+), 156 deletions(-)
  
Jerin Jacob Sept. 11, 2017, 4:16 p.m. UTC | #6
-----Original Message-----
> Date: Fri, 8 Sep 2017 16:36:51 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH v3 0/4] eventdev: add attribute based get APIs
> X-Mailer: git-send-email 2.7.4
> 
> This patchset refactors the eventdev API to be more flexible
> and capable. In particular, the API is capable of returning an
> error value if an invalid device, port or attribute ID is passed
> in, which was not possible with the previous APIs.
> 
> The implementation of this patchset is based on a v1 patch[1],
> and after some discussion this API was seen as the best solution.
> 
> In terms of flexibility, the attribute id allows addition of new
> common eventdev layer attributes without breaking ABI or adding
> new functions. Note that these attributes are not data-path, and
> that PMDs should continue to use the xstats API for reporting any
> unique PMD statistics that are available.
> 
> Regarding API/ABI compatibility, I have removed the functions from
> the .map files - please review the .map file changes for ABI issues
> carefully.

Please update the LIBABIVER in the Makefile.

Reference:

http://dpdk.org/browse/dpdk/commit/?id=945081a76ab0bb481f1d62125aa5b547fcc000bd
  
Van Haaren, Harry Sept. 14, 2017, 4:08 p.m. UTC | #7
This patchset refactors the eventdev API to be more flexible
and capable. In particular, the API is capable of returning an
error value if an invalid device, port or attribute ID is passed
in, which was not possible with the previous APIs.

The implementation of this patchset is based on a v1 patch[1],
and after some discussion this API was seen as the best solution.

In terms of flexibility, the attribute id allows addition of new
common eventdev layer attributes without breaking ABI or adding
new functions. Note that these attributes are not data-path, and
that PMDs should continue to use the xstats API for reporting any
unique PMD statistics that are available.

Regarding API/ABI compatibility, I have removed the functions from
the .map files - please review the .map file changes for ABI issues
carefully.

The last patch of this series adds a started attribute to the device,
allowing the application to query if a device is currently running.

-Harry

[1] http://dpdk.org/dev/patchwork/patch/27152/

---

v4:
- Rework based on review by Jerin
  - default: cases into switches
  - Remove old functions from .map file
  - Remove /* out */ parameters
  - Rework header file definitions to match logical order
- Rework patch split
  - Cleaner removal of queue_count() function

v3:
- Fix checkpatch issues... somehow I broke my checkpatch script :/

v2:
- New APIs design based on discussion of initial patch.



Harry van Haaren (4):
  eventdev: add port attribute function
  eventdev: add dev attribute get function
  eventdev: add queue attribute function
  eventdev: add device started attribute

 lib/librte_eventdev/rte_eventdev.c           |  97 ++++++++++++------
 lib/librte_eventdev/rte_eventdev.h           | 115 +++++++++++----------
 lib/librte_eventdev/rte_eventdev_version.map |  14 ++-
 test/test/test_eventdev.c                    | 132 +++++++++++++++++++------
 test/test/test_eventdev_octeontx.c           | 143 ++++++++++++++++++++-------
 5 files changed, 345 insertions(+), 156 deletions(-)
  
Nipun Gupta Sept. 15, 2017, 6:14 a.m. UTC | #8
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harry van Haaren
> Sent: Thursday, September 14, 2017 21:39
> To: dev@dpdk.org
> Cc: jerin.jacob@caviumnetworks.com; Harry van Haaren
> <harry.van.haaren@intel.com>
> Subject: [dpdk-dev] [PATCH v4 0/4] eventdev: add attribute based get APIs
> 
> This patchset refactors the eventdev API to be more flexible
> and capable. In particular, the API is capable of returning an
> error value if an invalid device, port or attribute ID is passed
> in, which was not possible with the previous APIs.
> 
> The implementation of this patchset is based on a v1 patch[1],
> and after some discussion this API was seen as the best solution.
> 
> In terms of flexibility, the attribute id allows addition of new
> common eventdev layer attributes without breaking ABI or adding
> new functions. Note that these attributes are not data-path, and
> that PMDs should continue to use the xstats API for reporting any
> unique PMD statistics that are available.
> 
> Regarding API/ABI compatibility, I have removed the functions from
> the .map files - please review the .map file changes for ABI issues
> carefully.
> 
> The last patch of this series adds a started attribute to the device,
> allowing the application to query if a device is currently running.
> 
> -Harry
> 
> [1] http://dpdk.org/dev/patchwork/patch/27152/
> 
> ---
> 
> v4:
> - Rework based on review by Jerin
>   - default: cases into switches
>   - Remove old functions from .map file
>   - Remove /* out */ parameters
>   - Rework header file definitions to match logical order
> - Rework patch split
>   - Cleaner removal of queue_count() function
> 
> v3:
> - Fix checkpatch issues... somehow I broke my checkpatch script :/
> 
> v2:
> - New APIs design based on discussion of initial patch.
> 
> 
> 
> Harry van Haaren (4):
>   eventdev: add port attribute function
>   eventdev: add dev attribute get function
>   eventdev: add queue attribute function
>   eventdev: add device started attribute
> 
>  lib/librte_eventdev/rte_eventdev.c           |  97 ++++++++++++------
>  lib/librte_eventdev/rte_eventdev.h           | 115 +++++++++++----------
>  lib/librte_eventdev/rte_eventdev_version.map |  14 ++-
>  test/test/test_eventdev.c                    | 132 +++++++++++++++++++------
>  test/test/test_eventdev_octeontx.c           | 143 ++++++++++++++++++++-------
>  5 files changed, 345 insertions(+), 156 deletions(-)
> 
> --
> 2.7.4

Changes look good.
Series Acked-by: Nipun Gupta <Nipun.gupta@nxp.com>
  
Jerin Jacob Sept. 15, 2017, 12:33 p.m. UTC | #9
-----Original Message-----
> Date: Thu, 14 Sep 2017 17:08:59 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH v4 0/4] eventdev: add attribute based get APIs
> X-Mailer: git-send-email 2.7.4
> 
> This patchset refactors the eventdev API to be more flexible
> and capable. In particular, the API is capable of returning an
> error value if an invalid device, port or attribute ID is passed
> in, which was not possible with the previous APIs.
> 
> The implementation of this patchset is based on a v1 patch[1],
> and after some discussion this API was seen as the best solution.
> 
> In terms of flexibility, the attribute id allows addition of new
> common eventdev layer attributes without breaking ABI or adding
> new functions. Note that these attributes are not data-path, and
> that PMDs should continue to use the xstats API for reporting any
> unique PMD statistics that are available.
> 
> Regarding API/ABI compatibility, I have removed the functions from
> the .map files - please review the .map file changes for ABI issues
> carefully.
> 
> The last patch of this series adds a started attribute to the device,
> allowing the application to query if a device is currently running.
> 
> -Harry
> 
> [1] http://dpdk.org/dev/patchwork/patch/27152/
> 
> ---
> 
> v4:
> - Rework based on review by Jerin
>   - default: cases into switches
>   - Remove old functions from .map file
>   - Remove /* out */ parameters
>   - Rework header file definitions to match logical order
> - Rework patch split
>   - Cleaner removal of queue_count() function

The changes looks good.

Since the functions are removed, we need to update .so version.
Please update LIBABIVER(file: lib/librte_eventdev/Makefile) and  update
the release notes

reference to the similar change:
http://dpdk.org/browse/dpdk/commit/?id=945081a76ab0bb481f1d62125aa5b547fcc000bd
  
Van Haaren, Harry Sept. 20, 2017, 1:35 p.m. UTC | #10
This patchset refactors the eventdev API to be more flexible
and capable. In particular, the API is capable of returning an
error value if an invalid device, port or attribute ID is passed
in, which was not possible with the previous APIs.

The implementation of this patchset is based on a v1 patch[1],
and after some discussion this API was seen as the best solution.

In terms of flexibility, the attribute id allows addition of new
common eventdev layer attributes without breaking ABI or adding
new functions. Note that these attributes are not data-path, and
that PMDs should continue to use the xstats API for reporting any
unique PMD statistics that are available.

Regarding API/ABI compatibility, I have removed the functions from
the .map files - please review the .map file changes for ABI issues
carefully.

The last patch of this series adds a started attribute to the device,
allowing the application to query if a device is currently running.

-Harry

[1] http://dpdk.org/dev/patchwork/patch/27152/

---

v5:
- Bump library version of Eventdev (Jerin)
  - http://dpdk.org/ml/archives/dev/2017-September/075551.html

v4:
- Rework based on review by Jerin
  - default: cases into switches
  - Remove old functions from .map file
  - Remove /* out */ parameters
  - Rework header file definitions to match logical order
- Rework patch split
  - Cleaner removal of queue_count() function

v3:
- Fix checkpatch issues... somehow I broke my checkpatch script :/

v2:
- New APIs design based on discussion of initial patch.


Harry van Haaren (5):
  eventdev: add port attribute function
  eventdev: add dev attribute get function
  eventdev: add queue attribute function
  eventdev: add device started attribute
  eventdev: bump library version

 doc/guides/rel_notes/release_17_11.rst       |   2 +-
 lib/librte_eventdev/Makefile                 |   2 +-
 lib/librte_eventdev/rte_eventdev.c           |  97 ++++++++++++------
 lib/librte_eventdev/rte_eventdev.h           | 115 +++++++++++----------
 lib/librte_eventdev/rte_eventdev_version.map |  14 ++-
 test/test/test_eventdev.c                    | 132 +++++++++++++++++++------
 test/test/test_eventdev_octeontx.c           | 143 ++++++++++++++++++++-------
 7 files changed, 347 insertions(+), 158 deletions(-)
  
Jerin Jacob Sept. 21, 2017, 9:57 a.m. UTC | #11
-----Original Message-----
> Date: Fri, 15 Sep 2017 06:14:26 +0000
> From: Nipun Gupta <nipun.gupta@nxp.com>
> To: Harry van Haaren <harry.van.haaren@intel.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> CC: "jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>
> Subject: RE: [dpdk-dev] [PATCH v4 0/4] eventdev: add attribute based get
>  APIs
> 
> 
> 
> > --
> > 2.7.4
> 
> Changes look good.
> Series Acked-by: Nipun Gupta <Nipun.gupta@nxp.com>

Applied the v5 series to dpdk-next-eventdev/master. Thanks.

>
  

Patch

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index bbb3805..e71f20c 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -609,21 +609,26 @@  rte_event_queue_setup(uint8_t dev_id, uint8_t queue_id,
 	return (*dev->dev_ops->queue_setup)(dev, queue_id, queue_conf);
 }
 
-uint8_t
+int16_t
 rte_event_queue_count(uint8_t dev_id)
 {
 	struct rte_eventdev *dev;
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 
 	dev = &rte_eventdevs[dev_id];
 	return dev->data->nb_queues;
 }
 
-uint8_t
+int16_t
 rte_event_queue_priority(uint8_t dev_id, uint8_t queue_id)
 {
-	struct rte_eventdev *dev;
+	struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	if (!is_valid_queue(dev, queue_id)) {
+		RTE_EDEV_LOG_ERR("Invalid queue_id=%" PRIu8, queue_id);
+		return -EINVAL;
+	}
 
-	dev = &rte_eventdevs[dev_id];
 	if (dev->data->event_dev_cap & RTE_EVENT_DEV_CAP_QUEUE_QOS)
 		return dev->data->queues_prio[queue_id];
 	else
@@ -743,28 +748,37 @@  rte_event_port_setup(uint8_t dev_id, uint8_t port_id,
 	return 0;
 }
 
-uint8_t
+int16_t
 rte_event_port_dequeue_depth(uint8_t dev_id, uint8_t port_id)
 {
-	struct rte_eventdev *dev;
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+	if (!is_valid_port(dev, port_id)) {
+		RTE_EDEV_LOG_ERR("Invalid port_id=%" PRIu8, port_id);
+		return -EINVAL;
+	}
 
-	dev = &rte_eventdevs[dev_id];
 	return dev->data->ports_dequeue_depth[port_id];
 }
 
-uint8_t
+int16_t
 rte_event_port_enqueue_depth(uint8_t dev_id, uint8_t port_id)
 {
-	struct rte_eventdev *dev;
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+	struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+	if (!is_valid_port(dev, port_id)) {
+		RTE_EDEV_LOG_ERR("Invalid port_id=%" PRIu8, port_id);
+		return -EINVAL;
+	}
 
-	dev = &rte_eventdevs[dev_id];
 	return dev->data->ports_enqueue_depth[port_id];
 }
 
-uint8_t
+int16_t
 rte_event_port_count(uint8_t dev_id)
 {
 	struct rte_eventdev *dev;
+	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 
 	dev = &rte_eventdevs[dev_id];
 	return dev->data->nb_ports;
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index 128bc52..204ff82 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -611,10 +611,10 @@  rte_event_queue_setup(uint8_t dev_id, uint8_t queue_id,
  *
  * @param dev_id
  *   Event device identifier.
- * @return
- *   - The number of configured event queues
+ * @retval Positive The number of configured event queues
+ * @retval -EINVAL Invalid device id
  */
-uint8_t
+int16_t
 rte_event_queue_count(uint8_t dev_id);
 
 /**
@@ -624,13 +624,13 @@  rte_event_queue_count(uint8_t dev_id);
  *   Event device identifier.
  * @param queue_id
  *   Event queue identifier.
- * @return
- *   - If the device has RTE_EVENT_DEV_CAP_QUEUE_QOS capability then the
- *    configured priority of the event queue in
- *    [RTE_EVENT_DEV_PRIORITY_HIGHEST, RTE_EVENT_DEV_PRIORITY_LOWEST] range
- *    else the value RTE_EVENT_DEV_PRIORITY_NORMAL
+ * @retval Positive If the device has RTE_EVENT_DEV_CAP_QUEUE_QOS capability
+ *         then the configured priority of the event queue in
+ *         [RTE_EVENT_DEV_PRIORITY_HIGHEST, RTE_EVENT_DEV_PRIORITY_LOWEST]
+ *         range else the value RTE_EVENT_DEV_PRIORITY_NORMAL
+ * @retval -EINVAL Invalid device id or queue id
  */
-uint8_t
+int16_t
 rte_event_queue_priority(uint8_t dev_id, uint8_t queue_id);
 
 /* Event port specific APIs */
@@ -722,12 +722,12 @@  rte_event_port_setup(uint8_t dev_id, uint8_t port_id,
  *   Event device identifier.
  * @param port_id
  *   Event port identifier.
- * @return
- *   - The number of configured dequeue queue depth
+ * @retval Positive The dequeue queue depth
+ * @retval -EINVAL Invalid device ID or port ID
  *
  * @see rte_event_dequeue_burst()
  */
-uint8_t
+int16_t
 rte_event_port_dequeue_depth(uint8_t dev_id, uint8_t port_id);
 
 /**
@@ -738,12 +738,12 @@  rte_event_port_dequeue_depth(uint8_t dev_id, uint8_t port_id);
  *   Event device identifier.
  * @param port_id
  *   Event port identifier.
- * @return
- *   - The number of configured enqueue queue depth
+ * @retval Positive The enqueue queue depth
+ * @retval -EINVAL Invalid device ID or port ID
  *
  * @see rte_event_enqueue_burst()
  */
-uint8_t
+int16_t
 rte_event_port_enqueue_depth(uint8_t dev_id, uint8_t port_id);
 
 /**
@@ -751,10 +751,10 @@  rte_event_port_enqueue_depth(uint8_t dev_id, uint8_t port_id);
  *
  * @param dev_id
  *   Event device identifier.
- * @return
- *   - The number of configured ports
+ * @retval Positive The number of configured ports
+ * @retval -EINVAL Invalid device id
  */
-uint8_t
+int16_t
 rte_event_port_count(uint8_t dev_id);
 
 /**
diff --git a/test/test/test_eventdev_octeontx.c b/test/test/test_eventdev_octeontx.c
index 774d030..6794f1f 100644
--- a/test/test/test_eventdev_octeontx.c
+++ b/test/test/test_eventdev_octeontx.c
@@ -591,7 +591,7 @@  wait_workers_to_join(int lcore, const rte_atomic32_t *count)
 static inline int
 launch_workers_and_wait(int (*master_worker)(void *),
 			int (*slave_workers)(void *), uint32_t total_events,
-			uint8_t nb_workers, uint8_t sched_type)
+			int16_t nb_workers, uint8_t sched_type)
 {
 	uint8_t port = 0;
 	int w_lcore;
@@ -651,14 +651,15 @@  static int
 test_multi_queue_enq_multi_port_deq(void)
 {
 	const unsigned int total_events = MAX_EVENTS;
-	uint8_t nr_ports;
+	int16_t nr_ports;
 	int ret;
 
 	ret = generate_random_events(total_events);
 	if (ret)
 		return TEST_FAILED;
 
-	nr_ports = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1);
+	nr_ports = RTE_MIN(rte_event_port_count(evdev),
+			(int)rte_lcore_count() - 1);
 
 	if (!nr_ports) {
 		printf("%s: Not enough ports=%d or workers=%d\n", __func__,
@@ -698,7 +699,7 @@  test_queue_to_port_single_link(void)
 	}
 
 	nr_links = RTE_MIN(rte_event_port_count(evdev),
-				rte_event_queue_count(evdev));
+				(int)rte_event_queue_count(evdev));
 	const unsigned int total_events = MAX_EVENTS / nr_links;
 
 	/* Link queue x to port x and inject events to queue x through port x */
@@ -750,7 +751,8 @@  static int
 test_queue_to_port_multi_link(void)
 {
 	int ret, port0_events = 0, port1_events = 0;
-	uint8_t nr_queues, nr_ports, queue, port;
+	int16_t nr_queues, nr_ports, port;
+	uint8_t queue;
 
 	nr_queues = rte_event_queue_count(evdev);
 	nr_ports = rte_event_port_count(evdev);
@@ -854,10 +856,11 @@  test_multiport_flow_sched_type_test(uint8_t in_sched_type,
 			uint8_t out_sched_type)
 {
 	const unsigned int total_events = MAX_EVENTS;
-	uint8_t nr_ports;
+	int16_t nr_ports;
 	int ret;
 
-	nr_ports = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1);
+	nr_ports = RTE_MIN(rte_event_port_count(evdev),
+			(int)rte_lcore_count() - 1);
 
 	if (!nr_ports) {
 		printf("%s: Not enough ports=%d or workers=%d\n", __func__,
@@ -1007,10 +1010,11 @@  test_multiport_queue_sched_type_test(uint8_t in_sched_type,
 			uint8_t out_sched_type)
 {
 	const unsigned int total_events = MAX_EVENTS;
-	uint8_t nr_ports;
+	int16_t nr_ports;
 	int ret;
 
-	nr_ports = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1);
+	nr_ports = RTE_MIN(rte_event_port_count(evdev),
+			(int)rte_lcore_count() - 1);
 
 	if (rte_event_queue_count(evdev) < 2 ||  !nr_ports) {
 		printf("%s: Not enough queues=%d ports=%d or workers=%d\n",
@@ -1142,10 +1146,11 @@  worker_flow_based_pipeline_max_stages_rand_sched_type(void *arg)
 static int
 launch_multi_port_max_stages_random_sched_type(int (*fn)(void *))
 {
-	uint8_t nr_ports;
+	int16_t nr_ports;
 	int ret;
 
-	nr_ports = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1);
+	nr_ports = RTE_MIN(rte_event_port_count(evdev),
+			(int)rte_lcore_count() - 1);
 
 	if (!nr_ports) {
 		printf("%s: Not enough ports=%d or workers=%d\n", __func__,
@@ -1288,9 +1293,10 @@  worker_ordered_flow_producer(void *arg)
 static inline int
 test_producer_consumer_ingress_order_test(int (*fn)(void *))
 {
-	uint8_t nr_ports;
+	int16_t nr_ports;
 
-	nr_ports = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1);
+	nr_ports = RTE_MIN(rte_event_port_count(evdev),
+			(int)rte_lcore_count() - 1);
 
 	if (rte_lcore_count() < 3 || nr_ports < 2) {
 		printf("### Not enough cores for %s test.\n", __func__);