Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 12 Jul 2023 15:28:36 +0300
From:   Leon Romanovsky <leon@kernel.org>
To:     Junxian Huang <huangjunxian6@hisilicon.com>
Cc:     jgg@nvidia.com, linux-rdma@vger.kernel.org, linuxarm@huawei.com,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 for-next] RDMA/core: Get IB width and speed from netdev
Message-ID: <20230712122836.GZ41919@unreal>
References: <20230707105634.1921046-1-huangjunxian6@hisilicon.com>
 <20230712121534.GY41919@unreal>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230712121534.GY41919@unreal>
Precedence: bulk

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
> >