| Message ID | 20200213130701.11589-1-kamalheib1@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [PATH,for-next] RDMA/siw: Fix setting active_{speed, width} attributes | expand |
-----"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.
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? > > 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
-----"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
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://en.wikipedia.org/wiki/InfiniBand Thanks, Kamal
-----"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). Many thanks for the info, Bernard.
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. >
-----"Potnuri Bharat Teja" <bharat@chelsio.com> wrote: ----- >To: "Bernard Metzler" <BMT@zurich.ibm.com> >From: "Potnuri Bharat Teja" <bharat@chelsio.com> >Date: 02/17/2020 03:51PM >Cc: "Kamal Heib" <kamalheib1@gmail.com>, "linux-rdma@vger.kernel.org" ><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 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.or >g >> >>_wiki_InfiniBand&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZOg&r=2TaYXQ0T-r8ZO1P >P >> >>1alNwU_QJcRRLfmYTAgd3QCvqSc&m=Ay2tYZ7Z-SHKRw8UZDCk76kwlZzvkXhRMrO_0j >k >> >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. > This is a single port 40Gbs setup. All my fault: For iw_cxgb4, I looked at the reported values for the second port, which is not physically connected but reported PORT_DOWN and 4 x 25Gbs. The connected port 1 has 4 x 10Gbs for both siw and iw_cxgb4. Sorry about the confusion! So its all good. Thanks Bernard.
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,
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(-)