linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed()
@ 2023-08-02  9:00 Selvin Xavier
  2023-08-03 17:58 ` Leon Romanovsky
  2023-08-13  8:01 ` Leon Romanovsky
  0 siblings, 2 replies; 8+ messages in thread
From: Selvin Xavier @ 2023-08-02  9:00 UTC (permalink / raw)
  To: jgg, leon; +Cc: linux-rdma, andrew.gospodarek, Kalesh AP, Selvin Xavier

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

When the Ethernet driver does not provide the number of lanes
in the __ethtool_get_link_ksettings() response, the function
ib_get_width_and_speed() does not take consideration of 50G,
100G and 200G speeds while calculating the IB width and speed.
Update the width and speed for the above netdev speeds.

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
v1 - v2:
    - Rebased the patch based on the latest changes in ib_get_width_and_speed
    - removed the switch case and use the existing else if check
---
 drivers/infiniband/core/verbs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 25367bd..41ff559 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1899,9 +1899,18 @@ static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
 		} else if (netdev_speed <= SPEED_40000) {
 			*width = IB_WIDTH_4X;
 			*speed = IB_SPEED_FDR10;
-		} else {
+		} else if (netdev_speed <= SPEED_50000) {
+			*width = IB_WIDTH_2X;
+			*speed = IB_SPEED_EDR;
+		} else if (netdev_speed <= SPEED_100000) {
 			*width = IB_WIDTH_4X;
 			*speed = IB_SPEED_EDR;
+		} else if (netdev_speed <= SPEED_200000) {
+			*width = IB_WIDTH_4X;
+			*speed = IB_SPEED_HDR;
+		} else {
+			*width = IB_WIDTH_4X;
+			*speed = IB_SPEED_NDR;
 		}
 
 		return;
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed()
  2023-08-02  9:00 [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed() Selvin Xavier
@ 2023-08-03 17:58 ` Leon Romanovsky
  2023-08-04  4:13   ` Selvin Xavier
  2023-08-13  8:01 ` Leon Romanovsky
  1 sibling, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2023-08-03 17:58 UTC (permalink / raw)
  To: Selvin Xavier; +Cc: jgg, linux-rdma, andrew.gospodarek, Kalesh AP

On Wed, Aug 02, 2023 at 02:00:23AM -0700, Selvin Xavier wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> 
> When the Ethernet driver does not provide the number of lanes
> in the __ethtool_get_link_ksettings() response, the function
> ib_get_width_and_speed() does not take consideration of 50G,
> 100G and 200G speeds while calculating the IB width and speed.
> Update the width and speed for the above netdev speeds.
> 
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
> v1 - v2:
>     - Rebased the patch based on the latest changes in ib_get_width_and_speed
>     - removed the switch case and use the existing else if check
> ---
>  drivers/infiniband/core/verbs.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 25367bd..41ff559 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -1899,9 +1899,18 @@ static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
>  		} else if (netdev_speed <= SPEED_40000) {
>  			*width = IB_WIDTH_4X;
>  			*speed = IB_SPEED_FDR10;
> -		} else {
> +		} else if (netdev_speed <= SPEED_50000) {
> +			*width = IB_WIDTH_2X;
> +			*speed = IB_SPEED_EDR;
> +		} else if (netdev_speed <= SPEED_100000) {
>  			*width = IB_WIDTH_4X;
>  			*speed = IB_SPEED_EDR;
> +		} else if (netdev_speed <= SPEED_200000) {
> +			*width = IB_WIDTH_4X;
> +			*speed = IB_SPEED_HDR;


SPEED_50000, SPEED_100000 and SPEED_200000 depends on
ClassPortInfo:CapabilityMask2.is* values.

For example, SPEED_50000 can b IB_WIDTH_2X/IB_SPEED_EDR and IB_WIDTH_1X/IB_SPEED_HDR.

Thanks

> +		} else {
> +			*width = IB_WIDTH_4X;
> +			*speed = IB_SPEED_NDR;
>  		}
>  
>  		return;
> -- 
> 2.5.5
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed()
  2023-08-03 17:58 ` Leon Romanovsky
@ 2023-08-04  4:13   ` Selvin Xavier
  2023-08-07 13:52     ` Leon Romanovsky
  0 siblings, 1 reply; 8+ messages in thread
From: Selvin Xavier @ 2023-08-04  4:13 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: jgg, linux-rdma, andrew.gospodarek, Kalesh AP

[-- Attachment #1: Type: text/plain, Size: 2865 bytes --]

On Thu, Aug 3, 2023 at 11:28 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Wed, Aug 02, 2023 at 02:00:23AM -0700, Selvin Xavier wrote:
> > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> >
> > When the Ethernet driver does not provide the number of lanes
> > in the __ethtool_get_link_ksettings() response, the function
> > ib_get_width_and_speed() does not take consideration of 50G,
> > 100G and 200G speeds while calculating the IB width and speed.
> > Update the width and speed for the above netdev speeds.
> >
> > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > ---
> > v1 - v2:
> >     - Rebased the patch based on the latest changes in ib_get_width_and_speed
> >     - removed the switch case and use the existing else if check
> > ---
> >  drivers/infiniband/core/verbs.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> > index 25367bd..41ff559 100644
> > --- a/drivers/infiniband/core/verbs.c
> > +++ b/drivers/infiniband/core/verbs.c
> > @@ -1899,9 +1899,18 @@ static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
> >               } else if (netdev_speed <= SPEED_40000) {
> >                       *width = IB_WIDTH_4X;
> >                       *speed = IB_SPEED_FDR10;
> > -             } else {
> > +             } else if (netdev_speed <= SPEED_50000) {
> > +                     *width = IB_WIDTH_2X;
> > +                     *speed = IB_SPEED_EDR;
> > +             } else if (netdev_speed <= SPEED_100000) {
> >                       *width = IB_WIDTH_4X;
> >                       *speed = IB_SPEED_EDR;
> > +             } else if (netdev_speed <= SPEED_200000) {
> > +                     *width = IB_WIDTH_4X;
> > +                     *speed = IB_SPEED_HDR;
>
>
> SPEED_50000, SPEED_100000 and SPEED_200000 depends on
> ClassPortInfo:CapabilityMask2.is* values.
>
> For example, SPEED_50000 can b IB_WIDTH_2X/IB_SPEED_EDR and IB_WIDTH_1X/IB_SPEED_HDR.
Agree with that.
This reporting can be achieved by the existing code, but the L2 driver
needs to report non zero values for lanes in
ethtool_ops->get_link_ksettings.
Caller of this modified function gets the speed and number of lanes
from ethtool_ops->get_link_ksettings.

In this patch we are trying to handle the case where ethtool ops
doesn't provide the lanes.  We can  default to some value so that we
dont report wrong total link speed (width * speed) for any speed more
than 40G.
>
> Thanks
>
> > +             } else {
> > +                     *width = IB_WIDTH_4X;
> > +                     *speed = IB_SPEED_NDR;
> >               }
> >
> >               return;
> > --
> > 2.5.5
> >
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed()
  2023-08-04  4:13   ` Selvin Xavier
@ 2023-08-07 13:52     ` Leon Romanovsky
  2023-08-09  8:54       ` Selvin Xavier
  0 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2023-08-07 13:52 UTC (permalink / raw)
  To: Selvin Xavier; +Cc: jgg, linux-rdma, andrew.gospodarek, Kalesh AP

On Fri, Aug 04, 2023 at 09:43:28AM +0530, Selvin Xavier wrote:
> On Thu, Aug 3, 2023 at 11:28 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Wed, Aug 02, 2023 at 02:00:23AM -0700, Selvin Xavier wrote:
> > > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > >
> > > When the Ethernet driver does not provide the number of lanes
> > > in the __ethtool_get_link_ksettings() response, the function
> > > ib_get_width_and_speed() does not take consideration of 50G,
> > > 100G and 200G speeds while calculating the IB width and speed.
> > > Update the width and speed for the above netdev speeds.
> > >
> > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > > ---
> > > v1 - v2:
> > >     - Rebased the patch based on the latest changes in ib_get_width_and_speed
> > >     - removed the switch case and use the existing else if check
> > > ---
> > >  drivers/infiniband/core/verbs.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> > > index 25367bd..41ff559 100644
> > > --- a/drivers/infiniband/core/verbs.c
> > > +++ b/drivers/infiniband/core/verbs.c
> > > @@ -1899,9 +1899,18 @@ static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
> > >               } else if (netdev_speed <= SPEED_40000) {
> > >                       *width = IB_WIDTH_4X;
> > >                       *speed = IB_SPEED_FDR10;
> > > -             } else {
> > > +             } else if (netdev_speed <= SPEED_50000) {
> > > +                     *width = IB_WIDTH_2X;
> > > +                     *speed = IB_SPEED_EDR;
> > > +             } else if (netdev_speed <= SPEED_100000) {
> > >                       *width = IB_WIDTH_4X;
> > >                       *speed = IB_SPEED_EDR;
> > > +             } else if (netdev_speed <= SPEED_200000) {
> > > +                     *width = IB_WIDTH_4X;
> > > +                     *speed = IB_SPEED_HDR;
> >
> >
> > SPEED_50000, SPEED_100000 and SPEED_200000 depends on
> > ClassPortInfo:CapabilityMask2.is* values.
> >
> > For example, SPEED_50000 can b IB_WIDTH_2X/IB_SPEED_EDR and IB_WIDTH_1X/IB_SPEED_HDR.
> Agree with that.
> This reporting can be achieved by the existing code, but the L2 driver
> needs to report non zero values for lanes in
> ethtool_ops->get_link_ksettings.
> Caller of this modified function gets the speed and number of lanes
> from ethtool_ops->get_link_ksettings.
> 
> In this patch we are trying to handle the case where ethtool ops
> doesn't provide the lanes.  

Almost all drivers don't support lanes reporting.

> We can  default to some value so that we
> dont report wrong total link speed (width * speed) for any speed more
> than 40G.
> >
> > Thanks
> >
> > > +             } else {
> > > +                     *width = IB_WIDTH_4X;
> > > +                     *speed = IB_SPEED_NDR;
> > >               }
> > >
> > >               return;
> > > --
> > > 2.5.5
> > >
> >
> >



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed()
  2023-08-07 13:52     ` Leon Romanovsky
@ 2023-08-09  8:54       ` Selvin Xavier
  2023-08-09  9:06         ` Leon Romanovsky
  0 siblings, 1 reply; 8+ messages in thread
From: Selvin Xavier @ 2023-08-09  8:54 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: jgg, linux-rdma, andrew.gospodarek, Kalesh AP

[-- Attachment #1: Type: text/plain, Size: 3626 bytes --]

On Mon, Aug 7, 2023 at 7:22 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Fri, Aug 04, 2023 at 09:43:28AM +0530, Selvin Xavier wrote:
> > On Thu, Aug 3, 2023 at 11:28 PM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > On Wed, Aug 02, 2023 at 02:00:23AM -0700, Selvin Xavier wrote:
> > > > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > >
> > > > When the Ethernet driver does not provide the number of lanes
> > > > in the __ethtool_get_link_ksettings() response, the function
> > > > ib_get_width_and_speed() does not take consideration of 50G,
> > > > 100G and 200G speeds while calculating the IB width and speed.
> > > > Update the width and speed for the above netdev speeds.
> > > >
> > > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > > > ---
> > > > v1 - v2:
> > > >     - Rebased the patch based on the latest changes in ib_get_width_and_speed
> > > >     - removed the switch case and use the existing else if check
> > > > ---
> > > >  drivers/infiniband/core/verbs.c | 11 ++++++++++-
> > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> > > > index 25367bd..41ff559 100644
> > > > --- a/drivers/infiniband/core/verbs.c
> > > > +++ b/drivers/infiniband/core/verbs.c
> > > > @@ -1899,9 +1899,18 @@ static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
> > > >               } else if (netdev_speed <= SPEED_40000) {
> > > >                       *width = IB_WIDTH_4X;
> > > >                       *speed = IB_SPEED_FDR10;
> > > > -             } else {
> > > > +             } else if (netdev_speed <= SPEED_50000) {
> > > > +                     *width = IB_WIDTH_2X;
> > > > +                     *speed = IB_SPEED_EDR;
> > > > +             } else if (netdev_speed <= SPEED_100000) {
> > > >                       *width = IB_WIDTH_4X;
> > > >                       *speed = IB_SPEED_EDR;
> > > > +             } else if (netdev_speed <= SPEED_200000) {
> > > > +                     *width = IB_WIDTH_4X;
> > > > +                     *speed = IB_SPEED_HDR;
> > >
> > >
> > > SPEED_50000, SPEED_100000 and SPEED_200000 depends on
> > > ClassPortInfo:CapabilityMask2.is* values.
> > >
> > > For example, SPEED_50000 can b IB_WIDTH_2X/IB_SPEED_EDR and IB_WIDTH_1X/IB_SPEED_HDR.
> > Agree with that.
> > This reporting can be achieved by the existing code, but the L2 driver
> > needs to report non zero values for lanes in
> > ethtool_ops->get_link_ksettings.
> > Caller of this modified function gets the speed and number of lanes
> > from ethtool_ops->get_link_ksettings.
> >
> > In this patch we are trying to handle the case where ethtool ops
> > doesn't provide the lanes.
>
> Almost all drivers don't support lanes reporting.
Agreed. But this patch will at least correct the overall speed
reporting. So it's an improvement from the current behavior.
If you still feel that this is not needed, you can drop the patch. We
will have to modify the L2 drivers to report the lanes in that case.

>
> > We can  default to some value so that we
> > dont report wrong total link speed (width * speed) for any speed more
> > than 40G.
> > >
> > > Thanks
> > >
> > > > +             } else {
> > > > +                     *width = IB_WIDTH_4X;
> > > > +                     *speed = IB_SPEED_NDR;
> > > >               }
> > > >
> > > >               return;
> > > > --
> > > > 2.5.5
> > > >
> > >
> > >
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed()
  2023-08-09  8:54       ` Selvin Xavier
@ 2023-08-09  9:06         ` Leon Romanovsky
  2023-08-09 18:11           ` Jason Gunthorpe
  0 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2023-08-09  9:06 UTC (permalink / raw)
  To: Selvin Xavier; +Cc: jgg, linux-rdma, andrew.gospodarek, Kalesh AP

On Wed, Aug 09, 2023 at 02:24:22PM +0530, Selvin Xavier wrote:
> On Mon, Aug 7, 2023 at 7:22 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Fri, Aug 04, 2023 at 09:43:28AM +0530, Selvin Xavier wrote:
> > > On Thu, Aug 3, 2023 at 11:28 PM Leon Romanovsky <leon@kernel.org> wrote:
> > > >
> > > > On Wed, Aug 02, 2023 at 02:00:23AM -0700, Selvin Xavier wrote:
> > > > > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > > >
> > > > > When the Ethernet driver does not provide the number of lanes
> > > > > in the __ethtool_get_link_ksettings() response, the function
> > > > > ib_get_width_and_speed() does not take consideration of 50G,
> > > > > 100G and 200G speeds while calculating the IB width and speed.
> > > > > Update the width and speed for the above netdev speeds.
> > > > >
> > > > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > > > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > > > > ---
> > > > > v1 - v2:
> > > > >     - Rebased the patch based on the latest changes in ib_get_width_and_speed
> > > > >     - removed the switch case and use the existing else if check
> > > > > ---
> > > > >  drivers/infiniband/core/verbs.c | 11 ++++++++++-
> > > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> > > > > index 25367bd..41ff559 100644
> > > > > --- a/drivers/infiniband/core/verbs.c
> > > > > +++ b/drivers/infiniband/core/verbs.c
> > > > > @@ -1899,9 +1899,18 @@ static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
> > > > >               } else if (netdev_speed <= SPEED_40000) {
> > > > >                       *width = IB_WIDTH_4X;
> > > > >                       *speed = IB_SPEED_FDR10;
> > > > > -             } else {
> > > > > +             } else if (netdev_speed <= SPEED_50000) {
> > > > > +                     *width = IB_WIDTH_2X;
> > > > > +                     *speed = IB_SPEED_EDR;
> > > > > +             } else if (netdev_speed <= SPEED_100000) {
> > > > >                       *width = IB_WIDTH_4X;
> > > > >                       *speed = IB_SPEED_EDR;
> > > > > +             } else if (netdev_speed <= SPEED_200000) {
> > > > > +                     *width = IB_WIDTH_4X;
> > > > > +                     *speed = IB_SPEED_HDR;
> > > >
> > > >
> > > > SPEED_50000, SPEED_100000 and SPEED_200000 depends on
> > > > ClassPortInfo:CapabilityMask2.is* values.
> > > >
> > > > For example, SPEED_50000 can b IB_WIDTH_2X/IB_SPEED_EDR and IB_WIDTH_1X/IB_SPEED_HDR.
> > > Agree with that.
> > > This reporting can be achieved by the existing code, but the L2 driver
> > > needs to report non zero values for lanes in
> > > ethtool_ops->get_link_ksettings.
> > > Caller of this modified function gets the speed and number of lanes
> > > from ethtool_ops->get_link_ksettings.
> > >
> > > In this patch we are trying to handle the case where ethtool ops
> > > doesn't provide the lanes.
> >
> > Almost all drivers don't support lanes reporting.
> Agreed. But this patch will at least correct the overall speed
> reporting. So it's an improvement from the current behavior.
> If you still feel that this is not needed, you can drop the patch.

I'm ok with the implementation, just waiting to hear second opinion from Jason.

Thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed()
  2023-08-09  9:06         ` Leon Romanovsky
@ 2023-08-09 18:11           ` Jason Gunthorpe
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2023-08-09 18:11 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Selvin Xavier, linux-rdma, andrew.gospodarek, Kalesh AP

On Wed, Aug 09, 2023 at 12:06:01PM +0300, Leon Romanovsky wrote:
> On Wed, Aug 09, 2023 at 02:24:22PM +0530, Selvin Xavier wrote:
> > On Mon, Aug 7, 2023 at 7:22 PM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > On Fri, Aug 04, 2023 at 09:43:28AM +0530, Selvin Xavier wrote:
> > > > On Thu, Aug 3, 2023 at 11:28 PM Leon Romanovsky <leon@kernel.org> wrote:
> > > > >
> > > > > On Wed, Aug 02, 2023 at 02:00:23AM -0700, Selvin Xavier wrote:
> > > > > > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > > > >
> > > > > > When the Ethernet driver does not provide the number of lanes
> > > > > > in the __ethtool_get_link_ksettings() response, the function
> > > > > > ib_get_width_and_speed() does not take consideration of 50G,
> > > > > > 100G and 200G speeds while calculating the IB width and speed.
> > > > > > Update the width and speed for the above netdev speeds.
> > > > > >
> > > > > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > > > > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > > > > > ---
> > > > > > v1 - v2:
> > > > > >     - Rebased the patch based on the latest changes in ib_get_width_and_speed
> > > > > >     - removed the switch case and use the existing else if check
> > > > > > ---
> > > > > >  drivers/infiniband/core/verbs.c | 11 ++++++++++-
> > > > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> > > > > > index 25367bd..41ff559 100644
> > > > > > --- a/drivers/infiniband/core/verbs.c
> > > > > > +++ b/drivers/infiniband/core/verbs.c
> > > > > > @@ -1899,9 +1899,18 @@ static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
> > > > > >               } else if (netdev_speed <= SPEED_40000) {
> > > > > >                       *width = IB_WIDTH_4X;
> > > > > >                       *speed = IB_SPEED_FDR10;
> > > > > > -             } else {
> > > > > > +             } else if (netdev_speed <= SPEED_50000) {
> > > > > > +                     *width = IB_WIDTH_2X;
> > > > > > +                     *speed = IB_SPEED_EDR;
> > > > > > +             } else if (netdev_speed <= SPEED_100000) {
> > > > > >                       *width = IB_WIDTH_4X;
> > > > > >                       *speed = IB_SPEED_EDR;
> > > > > > +             } else if (netdev_speed <= SPEED_200000) {
> > > > > > +                     *width = IB_WIDTH_4X;
> > > > > > +                     *speed = IB_SPEED_HDR;
> > > > >
> > > > >
> > > > > SPEED_50000, SPEED_100000 and SPEED_200000 depends on
> > > > > ClassPortInfo:CapabilityMask2.is* values.
> > > > >
> > > > > For example, SPEED_50000 can b IB_WIDTH_2X/IB_SPEED_EDR and IB_WIDTH_1X/IB_SPEED_HDR.
> > > > Agree with that.
> > > > This reporting can be achieved by the existing code, but the L2 driver
> > > > needs to report non zero values for lanes in
> > > > ethtool_ops->get_link_ksettings.
> > > > Caller of this modified function gets the speed and number of lanes
> > > > from ethtool_ops->get_link_ksettings.
> > > >
> > > > In this patch we are trying to handle the case where ethtool ops
> > > > doesn't provide the lanes.
> > >
> > > Almost all drivers don't support lanes reporting.
> > Agreed. But this patch will at least correct the overall speed
> > reporting. So it's an improvement from the current behavior.
> > If you still feel that this is not needed, you can drop the patch.
> 
> I'm ok with the implementation, just waiting to hear second opinion from Jason.

It seems fine, I think 4x is a reasonable place to default to.

Jason

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed()
  2023-08-02  9:00 [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed() Selvin Xavier
  2023-08-03 17:58 ` Leon Romanovsky
@ 2023-08-13  8:01 ` Leon Romanovsky
  1 sibling, 0 replies; 8+ messages in thread
From: Leon Romanovsky @ 2023-08-13  8:01 UTC (permalink / raw)
  To: jgg, Selvin Xavier; +Cc: linux-rdma, andrew.gospodarek, Kalesh AP


On Wed, 02 Aug 2023 02:00:23 -0700, Selvin Xavier wrote:
> When the Ethernet driver does not provide the number of lanes
> in the __ethtool_get_link_ksettings() response, the function
> ib_get_width_and_speed() does not take consideration of 50G,
> 100G and 200G speeds while calculating the IB width and speed.
> Update the width and speed for the above netdev speeds.
> 
> 
> [...]

Applied, thanks!

[1/1] IB/core: Add more speed parsing in ib_get_width_and_speed()
      https://git.kernel.org/rdma/rdma/c/ca60fd116c7ee1

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-08-13  8:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-02  9:00 [PATCH for-next v2] IB/core: Add more speed parsing in ib_get_width_and_speed() Selvin Xavier
2023-08-03 17:58 ` Leon Romanovsky
2023-08-04  4:13   ` Selvin Xavier
2023-08-07 13:52     ` Leon Romanovsky
2023-08-09  8:54       ` Selvin Xavier
2023-08-09  9:06         ` Leon Romanovsky
2023-08-09 18:11           ` Jason Gunthorpe
2023-08-13  8:01 ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).