-----"Kamal Heib" <kamalheib1@xxxxxxxxx> wrote: -----
>To: linux-rdma@xxxxxxxxxxxxxxx
>From: "Kamal Heib" <kamalheib1@xxxxxxxxx>
>Date: 02/13/2020 02:07PM
>Cc: "Jason Gunthorpe" <jgg@xxxxxxxx>, "Doug Ledford"
><dledford@xxxxxxxxxx>, "Bernard Metzler" <bmt@xxxxxxxxxxxxxx>, "Kamal
>Heib" <kamalheib1@xxxxxxxxx>
>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@xxxxxxxxx>
>---
> 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.
