* [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-17 22:59 [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state Thomas Monjalon
@ 2019-04-17 22:59 ` Thomas Monjalon
2019-04-17 22:59 ` [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable Thomas Monjalon
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2019-04-17 22:59 UTC (permalink / raw)
To: Shahaf Shuler, Yongseok Koh, Ferruh Yigit, Andrew Rybchenko; +Cc: dev
Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
instead of using the iterators based on rte_eth_find_next().
A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
because there should be no need of iterating over all devices in the API.
The public iterators have additional filters for ownership, parent device
or sibling ports.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
drivers/net/mlx5/mlx5.c | 9 ++-------
lib/librte_ethdev/rte_ethdev.c | 25 ++++++++++++-------------
2 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 9ff50dfbe..4deaada5c 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1964,14 +1964,9 @@ static int
mlx5_pci_remove(struct rte_pci_device *pci_dev)
{
uint16_t port_id;
- struct rte_eth_dev *port;
- for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
- port = &rte_eth_devices[port_id];
- if (port->state != RTE_ETH_DEV_UNUSED &&
- port->device == &pci_dev->device)
- rte_eth_dev_close(port_id);
- }
+ RTE_ETH_FOREACH_DEV_OF(port_id, &pci_dev->device)
+ rte_eth_dev_close(port_id);
return 0;
}
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 243beb4dd..cca15efca 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -339,6 +339,11 @@ rte_eth_find_next(uint16_t port_id)
return port_id;
}
+#define RTE_ETH_FOREACH_VALID_DEV(port_id) \
+ for (port_id = rte_eth_find_next(0); \
+ port_id < RTE_MAX_ETHPORTS; \
+ port_id = rte_eth_find_next(port_id + 1))
+
uint16_t
rte_eth_find_next_of(uint16_t port_id, const struct rte_device *parent)
{
@@ -584,13 +589,10 @@ rte_eth_is_valid_owner_id(uint64_t owner_id)
uint64_t
rte_eth_find_next_owned_by(uint16_t port_id, const uint64_t owner_id)
{
+ port_id = rte_eth_find_next(port_id);
while (port_id < RTE_MAX_ETHPORTS &&
- (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED ||
- rte_eth_devices[port_id].data->owner.id != owner_id))
- port_id++;
-
- if (port_id >= RTE_MAX_ETHPORTS)
- return RTE_MAX_ETHPORTS;
+ rte_eth_devices[port_id].data->owner.id != owner_id)
+ port_id = rte_eth_find_next(port_id + 1);
return port_id;
}
@@ -768,9 +770,8 @@ rte_eth_dev_count_total(void)
{
uint16_t port, count = 0;
- for (port = 0; port < RTE_MAX_ETHPORTS; port++)
- if (rte_eth_devices[port].state != RTE_ETH_DEV_UNUSED)
- count++;
+ RTE_ETH_FOREACH_VALID_DEV(port)
+ count++;
return count;
}
@@ -804,13 +805,11 @@ rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id)
return -EINVAL;
}
- for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
- if (rte_eth_devices[pid].state != RTE_ETH_DEV_UNUSED &&
- !strcmp(name, rte_eth_dev_shared_data->data[pid].name)) {
+ RTE_ETH_FOREACH_VALID_DEV(pid)
+ if (!strcmp(name, rte_eth_dev_shared_data->data[pid].name)) {
*port_id = pid;
return 0;
}
- }
return -ENODEV;
}
--
2.21.0
^ permalink raw reply [flat|nested] 16+ messages in thread* [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable
2019-04-17 22:59 [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state Thomas Monjalon
2019-04-17 22:59 ` Thomas Monjalon
@ 2019-04-17 22:59 ` Thomas Monjalon
2019-04-17 22:59 ` Thomas Monjalon
2019-04-18 11:51 ` Ferruh Yigit
2019-04-18 11:50 ` [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state Ferruh Yigit
2019-04-18 18:37 ` Ferruh Yigit
3 siblings, 2 replies; 16+ messages in thread
From: Thomas Monjalon @ 2019-04-17 22:59 UTC (permalink / raw)
To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev
The function rte_eth_dev_count_total() was introduced one year ago
in the release 18.05. It can be declared non experimental in 19.05.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
lib/librte_ethdev/rte_ethdev.c | 2 +-
lib/librte_ethdev/rte_ethdev.h | 2 +-
lib/librte_ethdev/rte_ethdev_version.map | 8 +++++++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index cca15efca..4d99186e8 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -765,7 +765,7 @@ rte_eth_dev_count_avail(void)
return count;
}
-uint16_t __rte_experimental
+uint16_t
rte_eth_dev_count_total(void)
{
uint16_t port, count = 0;
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 40a068fe8..aafc503db 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1560,7 +1560,7 @@ uint16_t rte_eth_dev_count_avail(void);
* @return
* The total count of Ethernet devices.
*/
-uint16_t __rte_experimental rte_eth_dev_count_total(void);
+uint16_t rte_eth_dev_count_total(void);
/**
* Convert a numerical speed in Mbps to a bitmap flag that can be used in
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index b37a4167d..afcd25599 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -229,11 +229,17 @@ DPDK_18.11 {
} DPDK_18.08;
+DPDK_19.05 {
+ global:
+
+ rte_eth_dev_count_total;
+
+} DPDK_18.11;
+
EXPERIMENTAL {
global:
rte_eth_devargs_parse;
- rte_eth_dev_count_total;
rte_eth_dev_create;
rte_eth_dev_destroy;
rte_eth_dev_get_module_eeprom;
--
2.21.0
^ permalink raw reply [flat|nested] 16+ messages in thread* [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable
2019-04-17 22:59 ` [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable Thomas Monjalon
@ 2019-04-17 22:59 ` Thomas Monjalon
2019-04-18 11:51 ` Ferruh Yigit
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2019-04-17 22:59 UTC (permalink / raw)
To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev
The function rte_eth_dev_count_total() was introduced one year ago
in the release 18.05. It can be declared non experimental in 19.05.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
lib/librte_ethdev/rte_ethdev.c | 2 +-
lib/librte_ethdev/rte_ethdev.h | 2 +-
lib/librte_ethdev/rte_ethdev_version.map | 8 +++++++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index cca15efca..4d99186e8 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -765,7 +765,7 @@ rte_eth_dev_count_avail(void)
return count;
}
-uint16_t __rte_experimental
+uint16_t
rte_eth_dev_count_total(void)
{
uint16_t port, count = 0;
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 40a068fe8..aafc503db 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1560,7 +1560,7 @@ uint16_t rte_eth_dev_count_avail(void);
* @return
* The total count of Ethernet devices.
*/
-uint16_t __rte_experimental rte_eth_dev_count_total(void);
+uint16_t rte_eth_dev_count_total(void);
/**
* Convert a numerical speed in Mbps to a bitmap flag that can be used in
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index b37a4167d..afcd25599 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -229,11 +229,17 @@ DPDK_18.11 {
} DPDK_18.08;
+DPDK_19.05 {
+ global:
+
+ rte_eth_dev_count_total;
+
+} DPDK_18.11;
+
EXPERIMENTAL {
global:
rte_eth_devargs_parse;
- rte_eth_dev_count_total;
rte_eth_dev_create;
rte_eth_dev_destroy;
rte_eth_dev_get_module_eeprom;
--
2.21.0
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable
2019-04-17 22:59 ` [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable Thomas Monjalon
2019-04-17 22:59 ` Thomas Monjalon
@ 2019-04-18 11:51 ` Ferruh Yigit
2019-04-18 11:51 ` Ferruh Yigit
2019-04-18 12:34 ` Andrew Rybchenko
1 sibling, 2 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-18 11:51 UTC (permalink / raw)
To: Thomas Monjalon, Andrew Rybchenko; +Cc: dev
On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> The function rte_eth_dev_count_total() was introduced one year ago
> in the release 18.05. It can be declared non experimental in 19.05.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable
2019-04-18 11:51 ` Ferruh Yigit
@ 2019-04-18 11:51 ` Ferruh Yigit
2019-04-18 12:34 ` Andrew Rybchenko
1 sibling, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-18 11:51 UTC (permalink / raw)
To: Thomas Monjalon, Andrew Rybchenko; +Cc: dev
On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> The function rte_eth_dev_count_total() was introduced one year ago
> in the release 18.05. It can be declared non experimental in 19.05.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable
2019-04-18 11:51 ` Ferruh Yigit
2019-04-18 11:51 ` Ferruh Yigit
@ 2019-04-18 12:34 ` Andrew Rybchenko
2019-04-18 12:34 ` Andrew Rybchenko
1 sibling, 1 reply; 16+ messages in thread
From: Andrew Rybchenko @ 2019-04-18 12:34 UTC (permalink / raw)
To: Ferruh Yigit, Thomas Monjalon; +Cc: dev
On 4/18/19 2:51 PM, Ferruh Yigit wrote:
> On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
>> The function rte_eth_dev_count_total() was introduced one year ago
>> in the release 18.05. It can be declared non experimental in 19.05.
>>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable
2019-04-18 12:34 ` Andrew Rybchenko
@ 2019-04-18 12:34 ` Andrew Rybchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Rybchenko @ 2019-04-18 12:34 UTC (permalink / raw)
To: Ferruh Yigit, Thomas Monjalon; +Cc: dev
On 4/18/19 2:51 PM, Ferruh Yigit wrote:
> On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
>> The function rte_eth_dev_count_total() was introduced one year ago
>> in the release 18.05. It can be declared non experimental in 19.05.
>>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-17 22:59 [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state Thomas Monjalon
2019-04-17 22:59 ` Thomas Monjalon
2019-04-17 22:59 ` [dpdk-dev] [PATCH 2/2] ethdev: promote function for port count as stable Thomas Monjalon
@ 2019-04-18 11:50 ` Ferruh Yigit
2019-04-18 11:50 ` Ferruh Yigit
2019-04-18 12:47 ` Thomas Monjalon
2019-04-18 18:37 ` Ferruh Yigit
3 siblings, 2 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-18 11:50 UTC (permalink / raw)
To: Thomas Monjalon, Shahaf Shuler, Yongseok Koh, Andrew Rybchenko; +Cc: dev
On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
> instead of using the iterators based on rte_eth_find_next().
>
> A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
> because there should be no need of iterating over all devices in the API.
> The public iterators have additional filters for ownership, parent device
> or sibling ports.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> drivers/net/mlx5/mlx5.c | 9 ++-------
> lib/librte_ethdev/rte_ethdev.c | 25 ++++++++++++-------------
No strong opinion but should we separate patch for driver and the library,
logically both changes RTE_ETH_DEV_UNUSED check with macros, but there is no
dependency, I mean they are individual changes, driver patch will be valid on
its own.
> 2 files changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 9ff50dfbe..4deaada5c 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -1964,14 +1964,9 @@ static int
> mlx5_pci_remove(struct rte_pci_device *pci_dev)
> {
> uint16_t port_id;
> - struct rte_eth_dev *port;
>
> - for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
> - port = &rte_eth_devices[port_id];
> - if (port->state != RTE_ETH_DEV_UNUSED &&
> - port->device == &pci_dev->device)
> - rte_eth_dev_close(port_id);
> - }
> + RTE_ETH_FOREACH_DEV_OF(port_id, &pci_dev->device)
> + rte_eth_dev_close(port_id);
> return 0;
> }
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 243beb4dd..cca15efca 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -339,6 +339,11 @@ rte_eth_find_next(uint16_t port_id)
> return port_id;
> }
>
> +#define RTE_ETH_FOREACH_VALID_DEV(port_id) \
> + for (port_id = rte_eth_find_next(0); \
> + port_id < RTE_MAX_ETHPORTS; \
> + port_id = rte_eth_find_next(port_id + 1))
> +
What do you think adding some documentation to the new macro, specially I think
documenting the difference between "RTE_ETH_FOREACH_DEV" and this one can be
good otherwise it may confuse people that "RTE_ETH_FOREACH_DEV" iterates on
invalid devices too?
> uint16_t
> rte_eth_find_next_of(uint16_t port_id, const struct rte_device *parent)
> {
> @@ -584,13 +589,10 @@ rte_eth_is_valid_owner_id(uint64_t owner_id)
> uint64_t
> rte_eth_find_next_owned_by(uint16_t port_id, const uint64_t owner_id)
> {
> + port_id = rte_eth_find_next(port_id);
> while (port_id < RTE_MAX_ETHPORTS &&
> - (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED ||
> - rte_eth_devices[port_id].data->owner.id != owner_id))
> - port_id++;
> -
> - if (port_id >= RTE_MAX_ETHPORTS)
> - return RTE_MAX_ETHPORTS;
> + rte_eth_devices[port_id].data->owner.id != owner_id)
> + port_id = rte_eth_find_next(port_id + 1);
>
> return port_id;
> }
> @@ -768,9 +770,8 @@ rte_eth_dev_count_total(void)
> {
> uint16_t port, count = 0;
>
> - for (port = 0; port < RTE_MAX_ETHPORTS; port++)
> - if (rte_eth_devices[port].state != RTE_ETH_DEV_UNUSED)
> - count++;
> + RTE_ETH_FOREACH_VALID_DEV(port)
> + count++;
>
> return count;
> }
> @@ -804,13 +805,11 @@ rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id)
> return -EINVAL;
> }
>
> - for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
> - if (rte_eth_devices[pid].state != RTE_ETH_DEV_UNUSED &&
> - !strcmp(name, rte_eth_dev_shared_data->data[pid].name)) {
> + RTE_ETH_FOREACH_VALID_DEV(pid)
> + if (!strcmp(name, rte_eth_dev_shared_data->data[pid].name)) {
> *port_id = pid;
> return 0;
> }
> - }
>
> return -ENODEV;
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-18 11:50 ` [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state Ferruh Yigit
@ 2019-04-18 11:50 ` Ferruh Yigit
2019-04-18 12:47 ` Thomas Monjalon
1 sibling, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-18 11:50 UTC (permalink / raw)
To: Thomas Monjalon, Shahaf Shuler, Yongseok Koh, Andrew Rybchenko; +Cc: dev
On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
> instead of using the iterators based on rte_eth_find_next().
>
> A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
> because there should be no need of iterating over all devices in the API.
> The public iterators have additional filters for ownership, parent device
> or sibling ports.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> drivers/net/mlx5/mlx5.c | 9 ++-------
> lib/librte_ethdev/rte_ethdev.c | 25 ++++++++++++-------------
No strong opinion but should we separate patch for driver and the library,
logically both changes RTE_ETH_DEV_UNUSED check with macros, but there is no
dependency, I mean they are individual changes, driver patch will be valid on
its own.
> 2 files changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 9ff50dfbe..4deaada5c 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -1964,14 +1964,9 @@ static int
> mlx5_pci_remove(struct rte_pci_device *pci_dev)
> {
> uint16_t port_id;
> - struct rte_eth_dev *port;
>
> - for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
> - port = &rte_eth_devices[port_id];
> - if (port->state != RTE_ETH_DEV_UNUSED &&
> - port->device == &pci_dev->device)
> - rte_eth_dev_close(port_id);
> - }
> + RTE_ETH_FOREACH_DEV_OF(port_id, &pci_dev->device)
> + rte_eth_dev_close(port_id);
> return 0;
> }
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 243beb4dd..cca15efca 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -339,6 +339,11 @@ rte_eth_find_next(uint16_t port_id)
> return port_id;
> }
>
> +#define RTE_ETH_FOREACH_VALID_DEV(port_id) \
> + for (port_id = rte_eth_find_next(0); \
> + port_id < RTE_MAX_ETHPORTS; \
> + port_id = rte_eth_find_next(port_id + 1))
> +
What do you think adding some documentation to the new macro, specially I think
documenting the difference between "RTE_ETH_FOREACH_DEV" and this one can be
good otherwise it may confuse people that "RTE_ETH_FOREACH_DEV" iterates on
invalid devices too?
> uint16_t
> rte_eth_find_next_of(uint16_t port_id, const struct rte_device *parent)
> {
> @@ -584,13 +589,10 @@ rte_eth_is_valid_owner_id(uint64_t owner_id)
> uint64_t
> rte_eth_find_next_owned_by(uint16_t port_id, const uint64_t owner_id)
> {
> + port_id = rte_eth_find_next(port_id);
> while (port_id < RTE_MAX_ETHPORTS &&
> - (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED ||
> - rte_eth_devices[port_id].data->owner.id != owner_id))
> - port_id++;
> -
> - if (port_id >= RTE_MAX_ETHPORTS)
> - return RTE_MAX_ETHPORTS;
> + rte_eth_devices[port_id].data->owner.id != owner_id)
> + port_id = rte_eth_find_next(port_id + 1);
>
> return port_id;
> }
> @@ -768,9 +770,8 @@ rte_eth_dev_count_total(void)
> {
> uint16_t port, count = 0;
>
> - for (port = 0; port < RTE_MAX_ETHPORTS; port++)
> - if (rte_eth_devices[port].state != RTE_ETH_DEV_UNUSED)
> - count++;
> + RTE_ETH_FOREACH_VALID_DEV(port)
> + count++;
>
> return count;
> }
> @@ -804,13 +805,11 @@ rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id)
> return -EINVAL;
> }
>
> - for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
> - if (rte_eth_devices[pid].state != RTE_ETH_DEV_UNUSED &&
> - !strcmp(name, rte_eth_dev_shared_data->data[pid].name)) {
> + RTE_ETH_FOREACH_VALID_DEV(pid)
> + if (!strcmp(name, rte_eth_dev_shared_data->data[pid].name)) {
> *port_id = pid;
> return 0;
> }
> - }
>
> return -ENODEV;
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-18 11:50 ` [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state Ferruh Yigit
2019-04-18 11:50 ` Ferruh Yigit
@ 2019-04-18 12:47 ` Thomas Monjalon
2019-04-18 12:47 ` Thomas Monjalon
2019-04-18 17:38 ` Thomas Monjalon
1 sibling, 2 replies; 16+ messages in thread
From: Thomas Monjalon @ 2019-04-18 12:47 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Shahaf Shuler, Yongseok Koh, Andrew Rybchenko, dev
18/04/2019 13:50, Ferruh Yigit:
> On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> > Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
> > instead of using the iterators based on rte_eth_find_next().
> >
> > A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
> > because there should be no need of iterating over all devices in the API.
> > The public iterators have additional filters for ownership, parent device
> > or sibling ports.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > drivers/net/mlx5/mlx5.c | 9 ++-------
> > lib/librte_ethdev/rte_ethdev.c | 25 ++++++++++++-------------
>
> No strong opinion but should we separate patch for driver and the library,
> logically both changes RTE_ETH_DEV_UNUSED check with macros, but there is no
> dependency, I mean they are individual changes, driver patch will be valid on
> its own.
This is the same change. I removed usage of RTE_ETH_DEV_UNUSED.
By chance, it was used only in ethdev and mlx5.
I don't feel the need to split because there are usages in different files.
> > +#define RTE_ETH_FOREACH_VALID_DEV(port_id) \
> > + for (port_id = rte_eth_find_next(0); \
> > + port_id < RTE_MAX_ETHPORTS; \
> > + port_id = rte_eth_find_next(port_id + 1))
> > +
>
> What do you think adding some documentation to the new macro, specially I think
> documenting the difference between "RTE_ETH_FOREACH_DEV" and this one can be
> good otherwise it may confuse people that "RTE_ETH_FOREACH_DEV" iterates on
> invalid devices too?
This one is not part of the API.
I am not sure what I can document more than "iterating all valid ports"?
About RTE_ETH_FOREACH_DEV, it is already documented:
"Macro to iterate over all enabled and ownerless ethdev ports."
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-18 12:47 ` Thomas Monjalon
@ 2019-04-18 12:47 ` Thomas Monjalon
2019-04-18 17:38 ` Thomas Monjalon
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2019-04-18 12:47 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Shahaf Shuler, Yongseok Koh, Andrew Rybchenko, dev
18/04/2019 13:50, Ferruh Yigit:
> On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> > Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
> > instead of using the iterators based on rte_eth_find_next().
> >
> > A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
> > because there should be no need of iterating over all devices in the API.
> > The public iterators have additional filters for ownership, parent device
> > or sibling ports.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > drivers/net/mlx5/mlx5.c | 9 ++-------
> > lib/librte_ethdev/rte_ethdev.c | 25 ++++++++++++-------------
>
> No strong opinion but should we separate patch for driver and the library,
> logically both changes RTE_ETH_DEV_UNUSED check with macros, but there is no
> dependency, I mean they are individual changes, driver patch will be valid on
> its own.
This is the same change. I removed usage of RTE_ETH_DEV_UNUSED.
By chance, it was used only in ethdev and mlx5.
I don't feel the need to split because there are usages in different files.
> > +#define RTE_ETH_FOREACH_VALID_DEV(port_id) \
> > + for (port_id = rte_eth_find_next(0); \
> > + port_id < RTE_MAX_ETHPORTS; \
> > + port_id = rte_eth_find_next(port_id + 1))
> > +
>
> What do you think adding some documentation to the new macro, specially I think
> documenting the difference between "RTE_ETH_FOREACH_DEV" and this one can be
> good otherwise it may confuse people that "RTE_ETH_FOREACH_DEV" iterates on
> invalid devices too?
This one is not part of the API.
I am not sure what I can document more than "iterating all valid ports"?
About RTE_ETH_FOREACH_DEV, it is already documented:
"Macro to iterate over all enabled and ownerless ethdev ports."
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-18 12:47 ` Thomas Monjalon
2019-04-18 12:47 ` Thomas Monjalon
@ 2019-04-18 17:38 ` Thomas Monjalon
2019-04-18 17:38 ` Thomas Monjalon
1 sibling, 1 reply; 16+ messages in thread
From: Thomas Monjalon @ 2019-04-18 17:38 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Shahaf Shuler, Yongseok Koh, Andrew Rybchenko
18/04/2019 14:47, Thomas Monjalon:
> 18/04/2019 13:50, Ferruh Yigit:
> > On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> > > Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
> > > instead of using the iterators based on rte_eth_find_next().
> > >
> > > A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
> > > because there should be no need of iterating over all devices in the API.
> > > The public iterators have additional filters for ownership, parent device
> > > or sibling ports.
> > >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > > +#define RTE_ETH_FOREACH_VALID_DEV(port_id) \
> > > + for (port_id = rte_eth_find_next(0); \
> > > + port_id < RTE_MAX_ETHPORTS; \
> > > + port_id = rte_eth_find_next(port_id + 1))
> > > +
> >
> > What do you think adding some documentation to the new macro, specially I think
> > documenting the difference between "RTE_ETH_FOREACH_DEV" and this one can be
> > good otherwise it may confuse people that "RTE_ETH_FOREACH_DEV" iterates on
> > invalid devices too?
>
> This one is not part of the API.
> I am not sure what I can document more than "iterating all valid ports"?
> About RTE_ETH_FOREACH_DEV, it is already documented:
> "Macro to iterate over all enabled and ownerless ethdev ports."
OK, let's add a comment to explain the difference:
/*
* Macro to iterate over all valid ports for internal usage.
* Note: RTE_ETH_FOREACH_DEV is different because filtering owned ports.
*/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-18 17:38 ` Thomas Monjalon
@ 2019-04-18 17:38 ` Thomas Monjalon
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2019-04-18 17:38 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Shahaf Shuler, Yongseok Koh, Andrew Rybchenko
18/04/2019 14:47, Thomas Monjalon:
> 18/04/2019 13:50, Ferruh Yigit:
> > On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> > > Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
> > > instead of using the iterators based on rte_eth_find_next().
> > >
> > > A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
> > > because there should be no need of iterating over all devices in the API.
> > > The public iterators have additional filters for ownership, parent device
> > > or sibling ports.
> > >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > > +#define RTE_ETH_FOREACH_VALID_DEV(port_id) \
> > > + for (port_id = rte_eth_find_next(0); \
> > > + port_id < RTE_MAX_ETHPORTS; \
> > > + port_id = rte_eth_find_next(port_id + 1))
> > > +
> >
> > What do you think adding some documentation to the new macro, specially I think
> > documenting the difference between "RTE_ETH_FOREACH_DEV" and this one can be
> > good otherwise it may confuse people that "RTE_ETH_FOREACH_DEV" iterates on
> > invalid devices too?
>
> This one is not part of the API.
> I am not sure what I can document more than "iterating all valid ports"?
> About RTE_ETH_FOREACH_DEV, it is already documented:
> "Macro to iterate over all enabled and ownerless ethdev ports."
OK, let's add a comment to explain the difference:
/*
* Macro to iterate over all valid ports for internal usage.
* Note: RTE_ETH_FOREACH_DEV is different because filtering owned ports.
*/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-17 22:59 [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state Thomas Monjalon
` (2 preceding siblings ...)
2019-04-18 11:50 ` [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state Ferruh Yigit
@ 2019-04-18 18:37 ` Ferruh Yigit
2019-04-18 18:37 ` Ferruh Yigit
3 siblings, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-18 18:37 UTC (permalink / raw)
To: Thomas Monjalon, Shahaf Shuler, Yongseok Koh, Andrew Rybchenko; +Cc: dev
On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
> instead of using the iterators based on rte_eth_find_next().
>
> A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
> because there should be no need of iterating over all devices in the API.
> The public iterators have additional filters for ownership, parent device
> or sibling ports.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Series applied to dpdk-next-net/master, thanks.
(Note for macro added while merging)
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH 1/2] ethdev: avoid explicit check of valid port state
2019-04-18 18:37 ` Ferruh Yigit
@ 2019-04-18 18:37 ` Ferruh Yigit
0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-18 18:37 UTC (permalink / raw)
To: Thomas Monjalon, Shahaf Shuler, Yongseok Koh, Andrew Rybchenko; +Cc: dev
On 4/17/2019 11:59 PM, Thomas Monjalon wrote:
> Some port iterations are manually checking against RTE_ETH_DEV_UNUSED
> instead of using the iterators based on rte_eth_find_next().
>
> A new macro RTE_ETH_FOREACH_VALID_DEV() is introduced, but kept private
> because there should be no need of iterating over all devices in the API.
> The public iterators have additional filters for ownership, parent device
> or sibling ports.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Series applied to dpdk-next-net/master, thanks.
(Note for macro added while merging)
^ permalink raw reply [flat|nested] 16+ messages in thread