[dpdk-dev] [PATCH] net/mlx5: use PCI BDF as the port name

[dpdk-dev] [PATCH] net/mlx5: use PCI BDF as the port name

[Yuanhan Liu]

It is suggested to use PCI BDF to identify a port for port addition
in OVS-DPDK. While mlx5 has its own naming style: name it by ib dev
name. This breaks the typical OVS DPDK use case and brings more puzzle
to the end users.

To fix it, this patch changes it to use PCI BDF as the name, too.
Also, a postfix " port %u" is added, just in case their might be more
than 1 port assoicated with a PCI device.

Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org>
---
 drivers/net/mlx5/mlx5.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 9d1de36..7cd9db7 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -610,6 +610,8 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 	INFO("%u port(s) detected", device_attr.orig_attr.phys_port_cnt);
 
 	for (i = 0; i < device_attr.orig_attr.phys_port_cnt; i++) {
+		char name[RTE_ETH_NAME_MAX_LEN];
+		int len;
 		uint32_t port = i + 1; /* ports are indexed from one */
 		uint32_t test = (1 << i);
 		struct ibv_context *ctx = NULL;
@@ -633,14 +635,15 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 			.inline_max_packet_sz = MLX5_ARG_UNSET,
 		};
 
+		len = snprintf(name, sizeof(name), PCI_PRI_FMT,
+			 pci_dev->addr.domain, pci_dev->addr.bus,
+			 pci_dev->addr.devid, pci_dev->addr.function);
+		if (device_attr.orig_attr.phys_port_cnt > 1)
+			snprintf(name + len, sizeof(name), " port %u", i);
+
 		mlx5_dev[idx].ports |= test;
 
 		if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
-			/* from rte_ethdev.c */
-			char name[RTE_ETH_NAME_MAX_LEN];
-
-			snprintf(name, sizeof(name), "%s port %u",
-				 ibv_get_device_name(ibv_dev), port);
 			eth_dev = rte_eth_dev_attach_secondary(name);
 			if (eth_dev == NULL) {
 				ERROR("can not attach rte ethdev");
@@ -834,14 +837,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		priv_get_mtu(priv, &priv->mtu);
 		DEBUG("port %u MTU is %u", priv->port, priv->mtu);
 
-		/* from rte_ethdev.c */
-		{
-			char name[RTE_ETH_NAME_MAX_LEN];
-
-			snprintf(name, sizeof(name), "%s port %u",
-				 ibv_get_device_name(ibv_dev), port);
-			eth_dev = rte_eth_dev_allocate(name);
-		}
+		eth_dev = rte_eth_dev_allocate(name);
 		if (eth_dev == NULL) {
 			ERROR("can not allocate rte ethdev");
 			err = ENOMEM;
-- 
2.7.4

Re: [dpdk-dev] [PATCH] net/mlx5: use PCI BDF as the port name

[Nélio Laranjeiro]

Hi Yuanhan,

On Mon, Jan 22, 2018 at 05:30:06PM +0800, Yuanhan Liu wrote:
> It is suggested to use PCI BDF to identify a port for port addition
> in OVS-DPDK. While mlx5 has its own naming style: name it by ib dev
> name. This breaks the typical OVS DPDK use case and brings more puzzle
> to the end users.
> 
> To fix it, this patch changes it to use PCI BDF as the name, too.
> Also, a postfix " port %u" is added, just in case their might be more
> than 1 port assoicated with a PCI device.

Seems only ixgbe, sfc, szedata2 use it whereas ark, bnxt, mlx4, mlx5,
mrvl, softnic don't.

Why not addressing all drivers at once?

> Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH] net/mlx5: use PCI BDF as the port name

[Shahaf Shuler]

Tuesday, January 23, 2018 10:24 AM, Nélio Laranjeiro:
> Hi Yuanhan,
> 
> On Mon, Jan 22, 2018 at 05:30:06PM +0800, Yuanhan Liu wrote:
> > It is suggested to use PCI BDF to identify a port for port addition in
> > OVS-DPDK. While mlx5 has its own naming style: name it by ib dev name.
> > This breaks the typical OVS DPDK use case and brings more puzzle to
> > the end users.
> >
> > To fix it, this patch changes it to use PCI BDF as the name, too.
> > Also, a postfix " port %u" is added, just in case their might be more
> > than 1 port assoicated with a PCI device.
> 
> Seems only ixgbe, sfc, szedata2 use it whereas ark, bnxt, mlx4, mlx5, mrvl,
> softnic don't.
> 
> Why not addressing all drivers at once?
> 
> > Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Applied to next-net-mlx, thanks.

> 
> --
> Nélio Laranjeiro
> 6WIND

Re: [dpdk-dev] [PATCH] net/mlx5: use PCI BDF as the port name

[Ferruh Yigit]

On 1/22/2018 9:30 AM, Yuanhan Liu wrote:
> It is suggested to use PCI BDF to identify a port for port addition
> in OVS-DPDK. While mlx5 has its own naming style: name it by ib dev
> name. This breaks the typical OVS DPDK use case and brings more puzzle
> to the end users.
> 
> To fix it, this patch changes it to use PCI BDF as the name, too.
> Also, a postfix " port %u" is added, just in case their might be more
> than 1 port assoicated with a PCI device.
> 
> Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org>

<...>

> @@ -633,14 +635,15 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  			.inline_max_packet_sz = MLX5_ARG_UNSET,
>  		};
>  
> +		len = snprintf(name, sizeof(name), PCI_PRI_FMT,
> +			 pci_dev->addr.domain, pci_dev->addr.bus,
> +			 pci_dev->addr.devid, pci_dev->addr.function);
> +		if (device_attr.orig_attr.phys_port_cnt > 1)
> +			snprintf(name + len, sizeof(name), " port %u", i);

Getting a build error [1] with icc [2].

Because commit 9a761de8ea14 ("net/mlx5: flow counter support") adds variable
"device_attr" into loop scope, but there is already a variable with same name in
function scope.
In this case the variable in the loop scope should be used and compiler error
looks correct, not sure why others compilers not complain about it.

[1]
.../dpdk/drivers/net/mlx5/mlx5.c(729): error #592: variable "device_attr" is
used before its value is set
                if (device_attr.orig_attr.phys_port_cnt > 1)

                    ^

[2]
icc (ICC) 18.0.1 20171018

[dpdk-dev] [PATCH] net/mlx5: fix ICC compilation

[Nelio Laranjeiro]

Remove the second declaration of device_attr [1] inside the loop as well as
the query_device_ex() which has already been done outside of the loop.

Fixes: 9a761de8ea14 ("net/mlx5: flow counter support")
Cc: orika@mellanox.com

Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

[1] https://dpdk.org/ml/archives/dev/2018-March/091744.html
---
 drivers/net/mlx5/mlx5.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index a2acac703..b8b362a20 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -723,7 +723,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		struct rte_eth_dev *eth_dev = NULL;
 		struct ibv_device_attr_ex device_attr_ex;
 		struct ether_addr mac;
-		struct ibv_device_attr_ex device_attr;
 		struct mlx5_dev_config config = {
 			.cqe_comp = cqe_comp,
 			.mps = mps,
@@ -780,7 +779,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			err = ENODEV;
 			goto port_error;
 		}
-		mlx5_glue->query_device_ex(ctx, NULL, &device_attr);
 		/* Check port status. */
 		err = mlx5_glue->query_port(ctx, port, &port_attr);
 		if (err) {
-- 
2.11.0

Re: [dpdk-dev] [PATCH] net/mlx5: fix ICC compilation

[Yongseok Koh]

> On Mar 19, 2018, at 3:42 AM, Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
> 
> Remove the second declaration of device_attr [1] inside the loop as well as
> the query_device_ex() which has already been done outside of the loop.
> 
> Fixes: 9a761de8ea14 ("net/mlx5: flow counter support")
> Cc: orika@mellanox.com
> 
> Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> [1] https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdpdk.org%2Fml%2Farchives%2Fdev%2F2018-March%2F091744.html&data=02%7C01%7Cyskoh%40mellanox.com%7C3b1029de8c1049ea04a608d58d8640e4%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636570530118173451&sdata=8olD%2BtcSHsXU8FNC82LlOdxkStJ8HFklyQWwVgewda4%3D&reserved=0
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

Re: [dpdk-dev] [PATCH] net/mlx5: fix ICC compilation

[Shahaf Shuler]

Wednesday, March 21, 2018 1:16 AM, Yongseok Koh:
> > On Mar 19, 2018, at 3:42 AM, Nelio Laranjeiro
> <nelio.laranjeiro@6wind.com> wrote:
> >
> > Remove the second declaration of device_attr [1] inside the loop as
> > well as the query_device_ex() which has already been done outside of the
> loop.
> >
> > Fixes: 9a761de8ea14 ("net/mlx5: flow counter support")
> > Cc: orika@mellanox.com
> >
> > Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Ferruh, can you confirm the compilation issue solved? 

> >
> > [1]
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdp
> d
> > k.org%2Fml%2Farchives%2Fdev%2F2018-
> March%2F091744.html&data=02%7C01%7C
> >
> yskoh%40mellanox.com%7C3b1029de8c1049ea04a608d58d8640e4%7Ca6529
> 71c7d2e
> >
> 4d9ba6a4d149256f461b%7C0%7C0%7C636570530118173451&sdata=8olD%2Bt
> cSHsXU
> > 8FNC82LlOdxkStJ8HFklyQWwVgewda4%3D&reserved=0
> > ---
> Acked-by: Yongseok Koh <yskoh@mellanox.com>
> 
> Thanks

Re: [dpdk-dev] [PATCH] net/mlx5: fix ICC compilation

[Ferruh Yigit]

On 3/19/2018 10:42 AM, Nelio Laranjeiro wrote:
> Remove the second declaration of device_attr [1] inside the loop as well as
> the query_device_ex() which has already been done outside of the loop.
> 
> Fixes: 9a761de8ea14 ("net/mlx5: flow counter support")
> Cc: orika@mellanox.com
> 
> Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH] net/mlx5: fix ICC compilation

[Shahaf Shuler]

Wednesday, March 21, 2018 11:53 AM, Ferruh Yigit:
> On 3/19/2018 10:42 AM, Nelio Laranjeiro wrote:
> > Remove the second declaration of device_attr [1] inside the loop as
> > well as the query_device_ex() which has already been done outside of the
> loop.
> >
> > Fixes: 9a761de8ea14 ("net/mlx5: flow counter support")
> > Cc: orika@mellanox.com
> >
> > Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to next-net-mlx, thanks.