Re: [PATH for-next] RDMA/siw: Fix setting active_{speed, width} attributes

["Bernard Metzler" <BMT@zurich.ibm.com>]

-----"Kamal Heib" <kamalheib1@gmail.com> wrote: -----

>To: linux-rdma@vger.kernel.org
>From: "Kamal Heib" <kamalheib1@gmail.com>
>Date: 02/13/2020 02:07PM
>Cc: "Jason Gunthorpe" <jgg@ziepe.ca>, "Doug Ledford"
><dledford@redhat.com>, "Bernard Metzler" <bmt@zurich.ibm.com>, "Kamal
>Heib" <kamalheib1@gmail.com>
>Subject: [EXTERNAL] [PATH for-next] RDMA/siw: Fix setting
>active_{speed, width} attributes
>
>Make sure to set the active_{speed, width} attributes to avoid
>reporting
>the same values regardless of the underlying device.
>
>Fixes: 303ae1cdfdf7 ("rdma/siw: application interface")
>Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
>---
> drivers/infiniband/sw/siw/siw_verbs.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/infiniband/sw/siw/siw_verbs.c
>b/drivers/infiniband/sw/siw/siw_verbs.c
>index 73485d0da907..b1aaec912edb 100644
>--- a/drivers/infiniband/sw/siw/siw_verbs.c
>+++ b/drivers/infiniband/sw/siw/siw_verbs.c
>@@ -165,11 +165,12 @@ int siw_query_port(struct ib_device *base_dev,
>u8 port,
> 		   struct ib_port_attr *attr)
> {
> 	struct siw_device *sdev = to_siw_dev(base_dev);
>+	int rc;
> 
> 	memset(attr, 0, sizeof(*attr));
> 
>-	attr->active_speed = 2;
>-	attr->active_width = 2;
>+	rc = ib_get_eth_speed(base_dev, port, &attr->active_speed,
>+			 &attr->active_width);
> 	attr->gid_tbl_len = 1;
> 	attr->max_msg_sz = -1;
> 	attr->max_mtu = ib_mtu_int_to_enum(sdev->netdev->mtu);
>@@ -192,7 +193,7 @@ int siw_query_port(struct ib_device *base_dev, u8
>port,
> 	 * attr->subnet_timeout = 0;
> 	 * attr->init_type_repy = 0;
> 	 */
>-	return 0;
>+	return rc;
> }
> 
> int siw_get_port_immutable(struct ib_device *base_dev, u8 port,
>-- 
>2.21.1
>
>
Hi Kamal, 
Many thanks for looking after this! So there definitely seem to 
be applications which are taking care of those values. So, good
to get my obvious laziness fixed.

I tried your patch on a 40Gbs Ethernet link (Chelsio cxgb4 driver).
Works in principle, but reported numbers are off. I am not saying
I would get right numbers when using Chelsio HW iWarp (iw_cxgb4),
but it's closer to reality (using ibv_devinfo <ibname> -vv)

iw_cxgb4 driver:
...
   active_width:           4X (2)
   active_speed:           25.0 Gbps (32)

siw driver with your patch:
...
   active_width:           4X (2)
   active_speed:           10.0 Gbps (8)

Any idea how we can improve that, maybe coming even
close to reality (40Gbs)?

Another remark: It has been siw folklore to name
integer return values 'rv', and not 'rc'. I never
liked 'return code'. It's a value in principle,
sometimes it's interpreted as a code though, as in
your case.

Many thanks!
Bernard.