netdev - Re: [PATCH net-next] vmxnet3: support higher link speeds from vmxnet3 v9

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240918072909.GT167971@kernel.org>
Date: Wed, 18 Sep 2024 08:29:09 +0100
From: Simon Horman <horms@...nel.org>
To: Ronak Doshi <ronak.doshi@...adcom.com>
Cc: netdev@...r.kernel.org,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] vmxnet3: support higher link speeds from
 vmxnet3 v9

On Tue, Sep 17, 2024 at 03:59:46PM -0700, Ronak Doshi wrote:
> Until now, vmxnet3 was default reporting 10Gbps as link speed.
> Vmxnet3 v9 adds support for user to configure higher link speeds.
> User can configure the link speed via VMs advanced parameters options
> in VCenter. This speed is reported in gbps by hypervisor.
> 
> This patch adds support for vmxnet3 to report higher link speeds and
> converts it to mbps as expected by Linux stack.
> 
> Signed-off-by: Ronak Doshi <ronak.doshi@...adcom.com>
> Acked-by: Guolin Yang <guolin.yang@...adcom.com>
> ---
>  drivers/net/vmxnet3/vmxnet3_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
> index b70654c7ad34..bb514b72c8b5 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -201,6 +201,8 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
>  
>  	adapter->link_speed = ret >> 16;
>  	if (ret & 1) { /* Link is up. */

Hi Ronak,

I think it would be nice to add a comment regarding the logic added below,
particularly the inequality.  It took me more than one reading to
understand it in the presence of the patch description. I expected may have
remained a mystery without some accompanying text.

> +		if (VMXNET3_VERSION_GE_9(adapter) && adapter->link_speed < 10000)

Please consider limiting Networking code to 80 columns wide where it
can trivially be achieved, as appears to be the case here.

checkpatch can be run with an option to flag this.

> +			adapter->link_speed = adapter->link_speed * 1000;

>  		netdev_info(adapter->netdev, "NIC Link is Up %d Mbps\n",
>  			    adapter->link_speed);
>  		netif_carrier_on(adapter->netdev);

net-next is currently closed for the v6.12 merge window.
Please repost this patch after it reopens, which will be after
v6.12-rc1 is released, most likely a little under two weeks from now.

-- 
pw-bot: changes-requested