diff mbox series

[v3,3/4] net/mlx5: use port sibling iterators

Message ID 20190401022700.1570-4-thomas@monjalon.net (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev iterators for multi-ports device |

Checks

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

Commit Message

Thomas Monjalon April 1, 2019, 2:26 a.m. UTC 
  Iterating over siblings was done with RTE_ETH_FOREACH_DEV()
which skips the owned ports.
The new iterators RTE_ETH_FOREACH_DEV_SIBLING()
and RTE_ETH_FOREACH_DEV_OF() are more appropriate and more correct.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/mlx5/mlx5.c        | 34 +++++++++++++---------------------
 drivers/net/mlx5/mlx5_ethdev.c |  6 +-----
 2 files changed, 14 insertions(+), 26 deletions(-)

Comments

Ferruh Yigit April 3, 2019, 2:19 p.m. UTC | #1 
On 4/1/2019 3:26 AM, Thomas Monjalon wrote:
> Iterating over siblings was done with RTE_ETH_FOREACH_DEV()
> which skips the owned ports.
> The new iterators RTE_ETH_FOREACH_DEV_SIBLING()
> and RTE_ETH_FOREACH_DEV_OF() are more appropriate and more correct.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Hi Shahaf, Yongseok,

Any comment on this patch?
Slava Ovsiienko April 3, 2019, 3:04 p.m. UTC | #2 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon
> Sent: Monday, April 1, 2019 5:27
> To: gaetan.rivet@6wind.com; Shahaf Shuler <shahafs@mellanox.com>;
> Yongseok Koh <yskoh@mellanox.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 3/4] net/mlx5: use port sibling iterators
> 
> Iterating over siblings was done with RTE_ETH_FOREACH_DEV() which skips
> the owned ports.
> The new iterators RTE_ETH_FOREACH_DEV_SIBLING() and
> RTE_ETH_FOREACH_DEV_OF() are more appropriate and more correct.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Tested-by: Viacheslav Ovsiienko <mellanox.com>

> ---
>  drivers/net/mlx5/mlx5.c        | 34 +++++++++++++---------------------
>  drivers/net/mlx5/mlx5_ethdev.c |  6 +-----
>  2 files changed, 14 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 1d7ca615b..3287a3d78 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -493,17 +493,15 @@ mlx5_dev_close(struct rte_eth_dev *dev)
>  			dev->data->port_id);
>  	if (priv->domain_id != RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID)
> {
>  		unsigned int c = 0;
> -		unsigned int i = mlx5_dev_to_port_id(dev->device, NULL, 0);
> -		uint16_t port_id[i];
> +		uint16_t port_id;
> 
> -		i = RTE_MIN(mlx5_dev_to_port_id(dev->device, port_id, i), i);
> -		while (i--) {
> +		RTE_ETH_FOREACH_DEV_OF(port_id, dev->device) {
>  			struct mlx5_priv *opriv =
> -				rte_eth_devices[port_id[i]].data-
> >dev_private;
> +				rte_eth_devices[port_id].data->dev_private;
> 
>  			if (!opriv ||
>  			    opriv->domain_id != priv->domain_id ||
> -			    &rte_eth_devices[port_id[i]] == dev)
> +			    &rte_eth_devices[port_id] == dev)
>  				continue;
>  			++c;
>  		}
> @@ -1147,22 +1145,16 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>  	 * Look for sibling devices in order to reuse their switch domain
>  	 * if any, otherwise allocate one.
>  	 */
> -	i = mlx5_dev_to_port_id(dpdk_dev, NULL, 0);
> -	if (i > 0) {
> -		uint16_t port_id[i];
> +	RTE_ETH_FOREACH_DEV_OF(port_id, dpdk_dev) {
> +		const struct mlx5_priv *opriv =
> +			rte_eth_devices[port_id].data->dev_private;
> 
> -		i = RTE_MIN(mlx5_dev_to_port_id(dpdk_dev, port_id, i), i);
> -		while (i--) {
> -			const struct mlx5_priv *opriv =
> -				rte_eth_devices[port_id[i]].data-
> >dev_private;
> -
> -			if (!opriv ||
> -			    opriv->domain_id ==
> -			    RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID)
> -				continue;
> -			priv->domain_id = opriv->domain_id;
> -			break;
> -		}
> +		if (!opriv ||
> +			opriv->domain_id ==
> +			RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID)
> +			continue;
> +		priv->domain_id = opriv->domain_id;
> +		break;
>  	}
>  	if (priv->domain_id ==
> RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID) {
>  		err = rte_eth_switch_domain_alloc(&priv->domain_id);
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> b/drivers/net/mlx5/mlx5_ethdev.c index 7273bd940..e01b698fc 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1398,11 +1398,7 @@ mlx5_dev_to_port_id(const struct rte_device
> *dev, uint16_t *port_list,
>  	uint16_t id;
>  	unsigned int n = 0;
> 
> -	RTE_ETH_FOREACH_DEV(id) {
> -		struct rte_eth_dev *ldev = &rte_eth_devices[id];
> -
> -		if (ldev->device != dev)
> -			continue;
> +	RTE_ETH_FOREACH_DEV_OF(id, dev) {
>  		if (n < port_list_n)
>  			port_list[n] = id;
>  		n++;
> --
> 2.21.0
Yongseok Koh April 3, 2019, 6:07 p.m. UTC | #3 
> On Apr 3, 2019, at 7:19 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> On 4/1/2019 3:26 AM, Thomas Monjalon wrote:
>> Iterating over siblings was done with RTE_ETH_FOREACH_DEV()
>> which skips the owned ports.
>> The new iterators RTE_ETH_FOREACH_DEV_SIBLING()
>> and RTE_ETH_FOREACH_DEV_OF() are more appropriate and more correct.
>> 
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Hi Shahaf, Yongseok,
> 
> Any comment on this patch?

Sorry for late reply.
I know it has been merged but it looks okay to me.

Thanks,
Yongseok
Ferruh Yigit April 4, 2019, 11:33 a.m. UTC | #4 
On 4/3/2019 7:07 PM, Yongseok Koh wrote:
> 
>> On Apr 3, 2019, at 7:19 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 4/1/2019 3:26 AM, Thomas Monjalon wrote:
>>> Iterating over siblings was done with RTE_ETH_FOREACH_DEV()
>>> which skips the owned ports.
>>> The new iterators RTE_ETH_FOREACH_DEV_SIBLING()
>>> and RTE_ETH_FOREACH_DEV_OF() are more appropriate and more correct.
>>>
>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>
>> Hi Shahaf, Yongseok,
>>
>> Any comment on this patch?
> 
> Sorry for late reply.
> I know it has been merged but it looks okay to me.

Thanks, I am appending explicit ack to the patch:

Acked-by: Yongseok Koh <yskoh@mellanox.com>
diff mbox series

Patch

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 1d7ca615b..3287a3d78 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -493,17 +493,15 @@  mlx5_dev_close(struct rte_eth_dev *dev)
 			dev->data->port_id);
 	if (priv->domain_id != RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID) {
 		unsigned int c = 0;
-		unsigned int i = mlx5_dev_to_port_id(dev->device, NULL, 0);
-		uint16_t port_id[i];
+		uint16_t port_id;
 
-		i = RTE_MIN(mlx5_dev_to_port_id(dev->device, port_id, i), i);
-		while (i--) {
+		RTE_ETH_FOREACH_DEV_OF(port_id, dev->device) {
 			struct mlx5_priv *opriv =
-				rte_eth_devices[port_id[i]].data->dev_private;
+				rte_eth_devices[port_id].data->dev_private;
 
 			if (!opriv ||
 			    opriv->domain_id != priv->domain_id ||
-			    &rte_eth_devices[port_id[i]] == dev)
+			    &rte_eth_devices[port_id] == dev)
 				continue;
 			++c;
 		}
@@ -1147,22 +1145,16 @@  mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	 * Look for sibling devices in order to reuse their switch domain
 	 * if any, otherwise allocate one.
 	 */
-	i = mlx5_dev_to_port_id(dpdk_dev, NULL, 0);
-	if (i > 0) {
-		uint16_t port_id[i];
+	RTE_ETH_FOREACH_DEV_OF(port_id, dpdk_dev) {
+		const struct mlx5_priv *opriv =
+			rte_eth_devices[port_id].data->dev_private;
 
-		i = RTE_MIN(mlx5_dev_to_port_id(dpdk_dev, port_id, i), i);
-		while (i--) {
-			const struct mlx5_priv *opriv =
-				rte_eth_devices[port_id[i]].data->dev_private;
-
-			if (!opriv ||
-			    opriv->domain_id ==
-			    RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID)
-				continue;
-			priv->domain_id = opriv->domain_id;
-			break;
-		}
+		if (!opriv ||
+			opriv->domain_id ==
+			RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID)
+			continue;
+		priv->domain_id = opriv->domain_id;
+		break;
 	}
 	if (priv->domain_id == RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID) {
 		err = rte_eth_switch_domain_alloc(&priv->domain_id);
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 7273bd940..e01b698fc 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1398,11 +1398,7 @@  mlx5_dev_to_port_id(const struct rte_device *dev, uint16_t *port_list,
 	uint16_t id;
 	unsigned int n = 0;
 
-	RTE_ETH_FOREACH_DEV(id) {
-		struct rte_eth_dev *ldev = &rte_eth_devices[id];
-
-		if (ldev->device != dev)
-			continue;
+	RTE_ETH_FOREACH_DEV_OF(id, dev) {
 		if (n < port_list_n)
 			port_list[n] = id;
 		n++;