[v3,for-next] RDMA/core: Get IB width and speed from netdev

Message ID 20230707105634.1921046-1-huangjunxian6@hisilicon.com
State New
Headers
Series [v3,for-next] RDMA/core: Get IB width and speed from netdev |

Commit Message

Junxian Huang July 7, 2023, 10:56 a.m. UTC
  From: Haoyue Xu <xuhaoyue1@hisilicon.com>

Previously, there was no way to query the number of lanes for a network
card, so the same netdev_speed would result in a fixed pair of width and
speed. As network card specifications become more diverse, such fixed
mode is no longer suitable, so a method is needed to obtain the correct
width and speed based on the number of lanes.

This patch retrieves netdev lanes and speed from net_device and
translates them to IB width and speed. Also, add a generic function
to translating netdev speed to IB speed.

Signed-off-by: Haoyue Xu <xuhaoyue1@hisilicon.com>
Signed-off-by: Luoyouming <luoyouming@huawei.com>
Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
---
 drivers/infiniband/core/verbs.c | 17 +++++++++++++++--
 include/rdma/ib_verbs.h         | 26 ++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)
  

Comments

Leon Romanovsky July 12, 2023, 12:15 p.m. UTC | #1
On Fri, Jul 07, 2023 at 06:56:34PM +0800, Junxian Huang wrote:
> From: Haoyue Xu <xuhaoyue1@hisilicon.com>
> 
> Previously, there was no way to query the number of lanes for a network
> card, so the same netdev_speed would result in a fixed pair of width and
> speed. As network card specifications become more diverse, such fixed
> mode is no longer suitable, so a method is needed to obtain the correct
> width and speed based on the number of lanes.
> 
> This patch retrieves netdev lanes and speed from net_device and
> translates them to IB width and speed. Also, add a generic function
> to translating netdev speed to IB speed.
> 
> Signed-off-by: Haoyue Xu <xuhaoyue1@hisilicon.com>
> Signed-off-by: Luoyouming <luoyouming@huawei.com>
> Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
> ---
>  drivers/infiniband/core/verbs.c | 17 +++++++++++++++--
>  include/rdma/ib_verbs.h         | 26 ++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index b99b3cc283b6..55a3ac9d01e2 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -1880,6 +1880,13 @@ int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
>  }
>  EXPORT_SYMBOL(ib_modify_qp_with_udata);
>  
> +static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
> +				   u16 *speed, u8 *width)
> +{
> +	*width = ib_int_to_ib_width(lanes);
> +	*speed = ib_eth_to_ib_speed(netdev_speed / lanes);
> +}
> +
>  int ib_get_eth_speed(struct ib_device *dev, u32 port_num, u16 *speed, u8 *width)
>  {
>  	int rc;
> @@ -1902,10 +1909,16 @@ int ib_get_eth_speed(struct ib_device *dev, u32 port_num, u16 *speed, u8 *width)
>  
>  	if (!rc && lksettings.base.speed != (u32)SPEED_UNKNOWN) {
>  		netdev_speed = lksettings.base.speed;
> +		if (lksettings.lanes) {
> +			ib_get_width_and_speed(netdev_speed, lksettings.lanes,
> +					       speed, width);
> +			return 0;
> +		}
>  	} else {
>  		netdev_speed = SPEED_1000;
> -		pr_warn("%s speed is unknown, defaulting to %u\n", netdev->name,
> -			netdev_speed);
> +		if (rc)

This if (rc) is not needed as we will take this else leaf for two
reasons: rc != 0 or base_speed is SPEED_UNKNOWN.

Fixed it locally and applied.

> +			pr_warn("%s speed is unknown, defaulting to %u\n",
> +				netdev->name, netdev_speed);
>  	}
>  
>  	if (netdev_speed <= SPEED_1000) {
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 1e7774ac808f..7dc926ec7fee 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -552,6 +552,18 @@ static inline int ib_width_enum_to_int(enum ib_port_width width)
>  	}
>  }
>  
> +static inline int ib_int_to_ib_width(u32 lanes)
> +{
> +	switch (lanes) {
> +	case 1: return IB_WIDTH_1X;
> +	case 2: return IB_WIDTH_2X;
> +	case 4: return IB_WIDTH_4X;
> +	case 8: return IB_WIDTH_8X;
> +	case 12: return IB_WIDTH_12X;
> +	default: return IB_WIDTH_1X;
> +	}
> +}
> +
>  enum ib_port_speed {
>  	IB_SPEED_SDR	= 1,
>  	IB_SPEED_DDR	= 2,
> @@ -563,6 +575,20 @@ enum ib_port_speed {
>  	IB_SPEED_NDR	= 128,
>  };
>  
> +static inline int ib_eth_to_ib_speed(u32 speed)
> +{
> +	switch (speed) {
> +	case SPEED_2500: return IB_SPEED_SDR;
> +	case SPEED_5000: return IB_SPEED_DDR;
> +	case SPEED_10000: return IB_SPEED_FDR10;
> +	case SPEED_14000: return IB_SPEED_FDR;
> +	case SPEED_25000: return IB_SPEED_EDR;
> +	case SPEED_50000: return IB_SPEED_HDR;
> +	case SPEED_100000: return IB_SPEED_NDR;
> +	default: return IB_SPEED_SDR;
> +	}
> +}
> +
>  enum ib_stat_flag {
>  	IB_STAT_FLAG_OPTIONAL = 1 << 0,
>  };
> -- 
> 2.30.0
>
  
Leon Romanovsky July 12, 2023, 12:28 p.m. UTC | #2
On Wed, Jul 12, 2023 at 03:15:34PM +0300, Leon Romanovsky wrote:
> On Fri, Jul 07, 2023 at 06:56:34PM +0800, Junxian Huang wrote:
> > From: Haoyue Xu <xuhaoyue1@hisilicon.com>
> > 
> > Previously, there was no way to query the number of lanes for a network
> > card, so the same netdev_speed would result in a fixed pair of width and
> > speed. As network card specifications become more diverse, such fixed
> > mode is no longer suitable, so a method is needed to obtain the correct
> > width and speed based on the number of lanes.
> > 
> > This patch retrieves netdev lanes and speed from net_device and
> > translates them to IB width and speed. Also, add a generic function
> > to translating netdev speed to IB speed.
> > 
> > Signed-off-by: Haoyue Xu <xuhaoyue1@hisilicon.com>
> > Signed-off-by: Luoyouming <luoyouming@huawei.com>
> > Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
> > ---
> >  drivers/infiniband/core/verbs.c | 17 +++++++++++++++--
> >  include/rdma/ib_verbs.h         | 26 ++++++++++++++++++++++++++
> >  2 files changed, 41 insertions(+), 2 deletions(-)

<...>

> >  
> >  	if (!rc && lksettings.base.speed != (u32)SPEED_UNKNOWN) {
> >  		netdev_speed = lksettings.base.speed;
> > +		if (lksettings.lanes) {
> > +			ib_get_width_and_speed(netdev_speed, lksettings.lanes,
> > +					       speed, width);
> > +			return 0;
> > +		}
> >  	} else {
> >  		netdev_speed = SPEED_1000;
> > -		pr_warn("%s speed is unknown, defaulting to %u\n", netdev->name,
> > -			netdev_speed);
> > +		if (rc)
> 
> This if (rc) is not needed as we will take this else leaf for two
> reasons: rc != 0 or base_speed is SPEED_UNKNOWN.
> 
> Fixed it locally and applied.

Actually not, this patch still needs some changes before it can be applied.

Thanks

> 
> > +			pr_warn("%s speed is unknown, defaulting to %u\n",
> > +				netdev->name, netdev_speed);
> >  	}
> >  
> >  	if (netdev_speed <= SPEED_1000) {
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index 1e7774ac808f..7dc926ec7fee 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -552,6 +552,18 @@ static inline int ib_width_enum_to_int(enum ib_port_width width)
> >  	}
> >  }
> >  
> > +static inline int ib_int_to_ib_width(u32 lanes)
> > +{
> > +	switch (lanes) {
> > +	case 1: return IB_WIDTH_1X;
> > +	case 2: return IB_WIDTH_2X;
> > +	case 4: return IB_WIDTH_4X;
> > +	case 8: return IB_WIDTH_8X;
> > +	case 12: return IB_WIDTH_12X;
> > +	default: return IB_WIDTH_1X;
> > +	}
> > +}
> > +
> >  enum ib_port_speed {
> >  	IB_SPEED_SDR	= 1,
> >  	IB_SPEED_DDR	= 2,
> > @@ -563,6 +575,20 @@ enum ib_port_speed {
> >  	IB_SPEED_NDR	= 128,
> >  };
> >  
> > +static inline int ib_eth_to_ib_speed(u32 speed)
> > +{
> > +	switch (speed) {
> > +	case SPEED_2500: return IB_SPEED_SDR;
> > +	case SPEED_5000: return IB_SPEED_DDR;
> > +	case SPEED_10000: return IB_SPEED_FDR10;
> > +	case SPEED_14000: return IB_SPEED_FDR;
> > +	case SPEED_25000: return IB_SPEED_EDR;
> > +	case SPEED_50000: return IB_SPEED_HDR;
> > +	case SPEED_100000: return IB_SPEED_NDR;
> > +	default: return IB_SPEED_SDR;
> > +	}
> > +}
> > +
> >  enum ib_stat_flag {
> >  	IB_STAT_FLAG_OPTIONAL = 1 << 0,
> >  };
> > -- 
> > 2.30.0
> >
  
Leon Romanovsky July 12, 2023, 12:35 p.m. UTC | #3
On Fri, Jul 07, 2023 at 06:56:34PM +0800, Junxian Huang wrote:
> From: Haoyue Xu <xuhaoyue1@hisilicon.com>
> 
> Previously, there was no way to query the number of lanes for a network
> card, so the same netdev_speed would result in a fixed pair of width and
> speed. As network card specifications become more diverse, such fixed
> mode is no longer suitable, so a method is needed to obtain the correct
> width and speed based on the number of lanes.
> 
> This patch retrieves netdev lanes and speed from net_device and
> translates them to IB width and speed. Also, add a generic function
> to translating netdev speed to IB speed.
> 
> Signed-off-by: Haoyue Xu <xuhaoyue1@hisilicon.com>
> Signed-off-by: Luoyouming <luoyouming@huawei.com>
> Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
> ---
>  drivers/infiniband/core/verbs.c | 17 +++++++++++++++--
>  include/rdma/ib_verbs.h         | 26 ++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index b99b3cc283b6..55a3ac9d01e2 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -1880,6 +1880,13 @@ int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
>  }
>  EXPORT_SYMBOL(ib_modify_qp_with_udata);
>  
> +static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
> +				   u16 *speed, u8 *width)
> +{
> +	*width = ib_int_to_ib_width(lanes);
> +	*speed = ib_eth_to_ib_speed(netdev_speed / lanes);
> +}
> +
>  int ib_get_eth_speed(struct ib_device *dev, u32 port_num, u16 *speed, u8 *width)
>  {
>  	int rc;
> @@ -1902,10 +1909,16 @@ int ib_get_eth_speed(struct ib_device *dev, u32 port_num, u16 *speed, u8 *width)
>  
>  	if (!rc && lksettings.base.speed != (u32)SPEED_UNKNOWN) {
>  		netdev_speed = lksettings.base.speed;
> +		if (lksettings.lanes) {
> +			ib_get_width_and_speed(netdev_speed, lksettings.lanes,
> +					       speed, width);

Please move this function to be after "if {} else {}" section and combine
with *width/*speed calculations at the bottom of ib_get_eth_speed()
function.

> +			return 0;
> +		}
>  	} else {
>  		netdev_speed = SPEED_1000;
> -		pr_warn("%s speed is unknown, defaulting to %u\n", netdev->name,
> -			netdev_speed);
> +		if (rc)

No need to remove this if ().

> +			pr_warn("%s speed is unknown, defaulting to %u\n",
> +				netdev->name, netdev_speed);
>  	}
>  
>  	if (netdev_speed <= SPEED_1000) {
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 1e7774ac808f..7dc926ec7fee 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -552,6 +552,18 @@ static inline int ib_width_enum_to_int(enum ib_port_width width)
>  	}
>  }
>  
> +static inline int ib_int_to_ib_width(u32 lanes)
> +{

<...>

> +static inline int ib_eth_to_ib_speed(u32 speed)
> +{

These two functions shouldn't be part in global include file.

Please embed them into ib_get_width_and_speed().

Thanks
  

Patch

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index b99b3cc283b6..55a3ac9d01e2 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1880,6 +1880,13 @@  int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
 }
 EXPORT_SYMBOL(ib_modify_qp_with_udata);
 
+static void ib_get_width_and_speed(u32 netdev_speed, u32 lanes,
+				   u16 *speed, u8 *width)
+{
+	*width = ib_int_to_ib_width(lanes);
+	*speed = ib_eth_to_ib_speed(netdev_speed / lanes);
+}
+
 int ib_get_eth_speed(struct ib_device *dev, u32 port_num, u16 *speed, u8 *width)
 {
 	int rc;
@@ -1902,10 +1909,16 @@  int ib_get_eth_speed(struct ib_device *dev, u32 port_num, u16 *speed, u8 *width)
 
 	if (!rc && lksettings.base.speed != (u32)SPEED_UNKNOWN) {
 		netdev_speed = lksettings.base.speed;
+		if (lksettings.lanes) {
+			ib_get_width_and_speed(netdev_speed, lksettings.lanes,
+					       speed, width);
+			return 0;
+		}
 	} else {
 		netdev_speed = SPEED_1000;
-		pr_warn("%s speed is unknown, defaulting to %u\n", netdev->name,
-			netdev_speed);
+		if (rc)
+			pr_warn("%s speed is unknown, defaulting to %u\n",
+				netdev->name, netdev_speed);
 	}
 
 	if (netdev_speed <= SPEED_1000) {
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 1e7774ac808f..7dc926ec7fee 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -552,6 +552,18 @@  static inline int ib_width_enum_to_int(enum ib_port_width width)
 	}
 }
 
+static inline int ib_int_to_ib_width(u32 lanes)
+{
+	switch (lanes) {
+	case 1: return IB_WIDTH_1X;
+	case 2: return IB_WIDTH_2X;
+	case 4: return IB_WIDTH_4X;
+	case 8: return IB_WIDTH_8X;
+	case 12: return IB_WIDTH_12X;
+	default: return IB_WIDTH_1X;
+	}
+}
+
 enum ib_port_speed {
 	IB_SPEED_SDR	= 1,
 	IB_SPEED_DDR	= 2,
@@ -563,6 +575,20 @@  enum ib_port_speed {
 	IB_SPEED_NDR	= 128,
 };
 
+static inline int ib_eth_to_ib_speed(u32 speed)
+{
+	switch (speed) {
+	case SPEED_2500: return IB_SPEED_SDR;
+	case SPEED_5000: return IB_SPEED_DDR;
+	case SPEED_10000: return IB_SPEED_FDR10;
+	case SPEED_14000: return IB_SPEED_FDR;
+	case SPEED_25000: return IB_SPEED_EDR;
+	case SPEED_50000: return IB_SPEED_HDR;
+	case SPEED_100000: return IB_SPEED_NDR;
+	default: return IB_SPEED_SDR;
+	}
+}
+
 enum ib_stat_flag {
 	IB_STAT_FLAG_OPTIONAL = 1 << 0,
 };