On Thu, Feb 13, 2020 at 01:59:30PM +0000, Bernard Metzler wrote:
> -----"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,
Hi Bernard,
> 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.
>
Sure :)
> 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)?
Could you please share the output of ethtool <if_name> for the underlying
net device that used for both iw_cxgb4 and siw?
>
> 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.
Sure, I'll fix it in v2.
>
> Many thanks!
> Bernard.
>
Thanks,
Kamal
