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, 5 Jul 2023 10:12:05 +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 v2 for-next] RDMA/core: Get IB width and speed from netdev
Message-ID: <20230705071205.GH6455@unreal>
References: <20230603063833.541682-1-huangjunxian6@hisilicon.com>
 <20230611174605.GG12152@unreal>
 <1b11b9e9-a729-0d61-52e3-6bcf132ca356@hisilicon.com>
 <20230628050019.GC23952@unreal>
 <53e8025d-803d-f6c5-b853-8352163d9a2e@hisilicon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <53e8025d-803d-f6c5-b853-8352163d9a2e@hisilicon.com>
Precedence: bulk

On Wed, Jul 05, 2023 at 11:05:50AM +0800, Junxian Huang wrote:
> 
> 
> On 2023/6/28 13:00, Leon Romanovsky wrote:
> > On Mon, Jun 19, 2023 at 02:20:54PM +0800, Junxian Huang wrote:
> >>
> >>
> >> On 2023/6/12 1:46, Leon Romanovsky wrote:
> >>> On Sat, Jun 03, 2023 at 02:38:33PM +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.
> >>>
> >>> I'm sorry but I didn't understand the problem statement. Can you please
> >>> provide an example of configuration that will give different results 
> >>> before this patch and after?
> >>>
> >>
> >> I'll give examples with 20G and 200G netdevs respectively.
> >>
> >> 20G:
> >> Before this patch, regardless of the actual number of lanes, the width and
> >> speed displayed in ibv_devinfo would be always fixed:
> >> 	active_width: 4X
> >> 	active_speed: 5 Gbps
> >> After this patch, there will be different combinations of width and speed
> >> according to the number of lanes. For example, for a 20G netdev whose number
> >> of lanes is 2, the width and speed displayed in ibv_devinfo will be:
> >> 	active_width: 2X
> >> 	active_speed: 10 Gbps
> >>
> >> 200G:
> >> Before this patch, netdevs with netdev_speed more than 40G cannot get a right
> >> width and speed in ibv_devinfo. Only the default result would be displayed:
> >> 	active_width: 4X
> >> 	active_speed: 25 Gbps
> >> After this patch, taking an example with 4 lanes, the displayed results will be:
> >> 	active_width: 4X
> >> 	active_speed: 50 Gbps
> >>
> > 
> > <...>
> > 
> >>>> +	cap_link_lanes_supported = netdev->ethtool_ops->cap_link_lanes_supported;
> >>>>  	rtnl_unlock();
> >>>>  
> >>>>  	dev_put(netdev);
> >>>>  
> >>>>  	if (!rc && lksettings.base.speed != (u32)SPEED_UNKNOWN) {
> >>>>  		netdev_speed = lksettings.base.speed;
> >>>> +		if (cap_link_lanes_supported && lksettings.lanes) {
> >>>
> >>> According to the documentation cap_link_lanes_supported defines if
> >>> number of lanes can be supplied by user and I would expect from
> >>> __ethtool_get_link_ksettings() to get right numbers after it was
> >>> changed.
> > 
> > No, I'm saying that cap_link_lanes_supported is variable which only says
> > if number of lanes can be changed and __ethtool_get_link_ksettings()
> > will return right number of lanes every time it is called without need
> > to call to ib_get_width_and_speed() again.
> > 
> > Thanks
> > 
> 
> These two functions have different purposes.
> 
> The number of lanes is indeed obtained from __ethtool_get_link_ksettings(),
> and ib_get_width_and_speed() converts the number of lanes into ib_width and
> ib_speed, which are the output of ib_get_eth_speed().

Great, so why do you need to rely on cap_link_lanes_supported in ib_get_width_and_speed()?

Thanks