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

[Potnuri Bharat Teja <bharat@chelsio.com>]

On Monday, February 02/17/20, 2020 at 19:57:54 +0530, Bernard Metzler wrote:
> -----"Kamal Heib" <kamalheib1@gmail.com> wrote: -----
> 
> >To: "Bernard Metzler" <BMT@zurich.ibm.com>
> >From: "Kamal Heib" <kamalheib1@gmail.com>
> >Date: 02/17/2020 03:12PM
> >Cc: linux-rdma@vger.kernel.org, "Jason Gunthorpe" <jgg@ziepe.ca>,
> >"Doug Ledford" <dledford@redhat.com>
> >Subject: [EXTERNAL] Re: [PATH for-next] RDMA/siw: Fix setting
> >active_{speed, width} attributes
> >
> >On Mon, Feb 17, 2020 at 10:13:21AM +0000, Bernard Metzler wrote:
> >> -----"Kamal Heib" <kamalheib1@gmail.com> wrote: -----
> >> 
> >> >To: "Bernard Metzler" <BMT@zurich.ibm.com>
> >> >From: "Kamal Heib" <kamalheib1@gmail.com>
> >> >Date: 02/16/2020 02:43PM
> >> >Cc: linux-rdma@vger.kernel.org, "Jason Gunthorpe" <jgg@ziepe.ca>,
> >> >"Doug Ledford" <dledford@redhat.com>
> >> >Subject: [EXTERNAL] Re: [PATH for-next] RDMA/siw: Fix setting
> >> >active_{speed, width} attributes
> >> >
> >> >On Thu, Feb 13, 2020 at 01:59:30PM +0000, Bernard Metzler wrote:
> >> >> -----"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, 
> >> >
> >> >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?
> >> >
> >> H Kamal,
> >> 
> >> Sure! Speed looks correct, and its also what I get
> >> at maximum:
> >> 
> >> [bmt@spoke ~]$ ethtool enp1s0f4 
> >> Settings for enp1s0f4:
> >>         Supported ports: [ FIBRE ]
> >>         Supported link modes:   40000baseSR4/Full 
> >>         Supported pause frame use: Symmetric
> >>         Supports auto-negotiation: Yes
> >>         Supported FEC modes: None
> >>         Advertised link modes:  40000baseSR4/Full 
> >>         Advertised pause frame use: Symmetric
> >>         Advertised auto-negotiation: Yes
> >>         Advertised FEC modes: None
> >>         Link partner advertised link modes:  40000baseSR4/Full 
> >>         Link partner advertised pause frame use: Symmetric
> >>         Link partner advertised auto-negotiation: Yes
> >>         Link partner advertised FEC modes: None
> >>         Speed: 40000Mb/s
> >>         Duplex: Full
> >>         Port: Direct Attach Copper
> >>         PHYAD: 255
> >>         Transceiver: internal
> >>         Auto-negotiation: on
> >> Cannot get wake-on-lan settings: Operation not permitted
> >>         Current message level: 0x000000ff (255)
> >>                                drv probe link timer ifdown ifup
> >rx_err tx_err
> >>         Link detected: yes
> >>
> >
> >Hi Bernard,
> >
> >Well, this is the expected value for 40GbE, Because the reported
> >value
> >is 4X aggregation of FDR10. For more info please the table of speeds
> >under [1].
> >
> >[1] -
> >https://urldefense.proofpoint.com/v2/url?u=https-3A__en.wikipedia.org
> >_wiki_InfiniBand&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZOg&r=2TaYXQ0T-r8ZO1PP
> >1alNwU_QJcRRLfmYTAgd3QCvqSc&m=Ay2tYZ7Z-SHKRw8UZDCk76kwlZzvkXhRMrO_0jk
> >YjcY&s=D4Z0CAH5UVO95WHNnUozLzrqjxRQgVe-2lc8_jwVlhw&e= 
> >
> >Thanks,
> >Kamal
> >
> Hi Kamal, so I have to do the math! 4 x 10Gbs = 40Gbs.
> So it is all correct as reported by ibv_devinfio
> (and somehow strange what the iw_cxgb4 makes out of it).
Hi Bernard,
Are you running siw and iw_cxgb4 on the same port or different ports?
if not same, Is the port running iw_cxgb4 connected with 100G cable?
If 40G cable is conencted to iw_cxgb4 as well then its reporting wrong.

Thanks.

> 
> Many thanks for the info,
> Bernard.
>