netdev - Re: [PATCH net 4/4] net/mlx5e: Keep netdev when leave switchdev for devlink set legacy only

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <65ef1684-b2d5-40f6-b20a-71ccb9ee302d@intel.com>
Date: Fri, 20 Dec 2024 16:10:54 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jakub Kicinski <kuba@...nel.org>, Alexander Lobakin
	<aleksander.lobakin@...el.com>
CC: Tariq Toukan <tariqt@...dia.com>, Jianbo Liu <jianbol@...dia.com>,
	<netdev@...r.kernel.org>, Saeed Mahameed <saeedm@...dia.com>, Gal Pressman
	<gal@...dia.com>, Leon Romanovsky <leonro@...dia.com>, Mark Bloch
	<mbloch@...dia.com>, "David S. Miller" <davem@...emloft.net>, Paolo Abeni
	<pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, ITP Upstream
	<nxne.cnse.osdt.itp.upstreaming@...el.com>
Subject: Re: [PATCH net 4/4] net/mlx5e: Keep netdev when leave switchdev for
 devlink set legacy only

On 12/20/24 15:46, Jakub Kicinski wrote:
> On Fri, 20 Dec 2024 09:48:11 +0100 Przemek Kitszel wrote:
>>>    	mlx5_core_uplink_netdev_set(mdev, NULL);
>>>    	mlx5e_dcbnl_delete_app(priv);
>>> -	unregister_netdev(priv->netdev);
>>> -	_mlx5e_suspend(adev, false);
>>> +	/* When unload driver, the netdev is in registered state
>>
>> /*
>>    * Netdev dropped the special comment allowance rule,
>>    * now you have to put one line almost blank at the front.
>>    */
> 
> Incorrect, we still prefer the old comment style, we just give a pass
> now to people who have a strong preference the opposite way.

good to know, I will pass it down to my folks

> 
>>> +	 * if it's from legacy mode. If from switchdev mode, it
>>> +	 * is already unregistered before changing to NIC profile.
>>> +	 */
>>> +	if (priv->netdev->reg_state == NETREG_REGISTERED) {
>>> +		unregister_netdev(priv->netdev);
>>> +		_mlx5e_suspend(adev, false);
>>> +	} else {
>>> +		struct mlx5_core_dev *pos;
>>> +		int i;
>>> +
>>> +		if (test_bit(MLX5E_STATE_DESTROYING, &priv->state))
>>
>> you have more than one statement/expression inside the if,
>> so you must wrap with braces
> 
> I'm not aware of that as a hard rule either.

[1] says
"Also, use braces when a loop contains more than a single simple statement:"

And I see that here we have and if() instead of a loop, but I believe
that the intent of "the rule" (not sure how hard) was to pet the brace
in such cases.
To be clear, I don't want to stop an otherwise good PR just for this!

[1] 
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces