netdev - RE: [PATCH net-next 1/1] net: stmmac: add check for advertising linkmode request for set-eee

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: <DM6PR11MB3306A3162F6A6086A4CBA049B9A0A@DM6PR11MB3306.namprd11.prod.outlook.com>
Date: Tue, 31 Oct 2023 08:44:23 +0000
From: "Gan, Yi Fang" <yi.fang.gan@...el.com>
To: Russell King <linux@...linux.org.uk>
CC: Alexandre Torgue <alexandre.torgue@...s.st.com>, Jose Abreu
	<joabreu@...opsys.com>, "David S . Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Maxime Coquelin <mcoquelin.stm32@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-stm32@...md-mailman.stormreply.com"
	<linux-stm32@...md-mailman.stormreply.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Looi, Hong Aun" <hong.aun.looi@...el.com>,
	"Voon, Weifeng" <weifeng.voon@...el.com>, "Song, Yoong Siang"
	<yoong.siang.song@...el.com>, "Ahmad Tarmizi, Noor Azura"
	<noor.azura.ahmad.tarmizi@...el.com>
Subject: RE: [PATCH net-next 1/1] net: stmmac: add check for advertising
 linkmode request for set-eee

Hi Russell King,

Why should this functionality be specific to stmmac?
This functionality is not specific to stmmac but other drivers can have their
 own implementation. 
(e.g. https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/qlogic/qede/qede_ethtool.c#L1855)

Why do we need this?
Current implementation will not take any effect if user enters unsupported value but user might
not aware. With this, an error will be prompted if unsupported value is given. 

What is wrong with the checking and masking that phylib is doing?
Nothing wrong with the phylib but there is no error return back to ethtool commands 
if unsupported value is given.

Why should we trust the value in edata->supported provided by the user?
The edata->supported is getting from the current setting and the value is set upon bootup.
Users are not allowed to change it.

Sorry, but no. I see no reason that this should be done, especially not in the stmmac driver.
I understand your reasoning. From your point of view, is this kind of error message/ error handling 
not needed?


Best Regards,
Fang

> -----Original Message-----
> From: Russell King <linux@...linux.org.uk>
> Sent: Friday, October 27, 2023 3:39 PM
> To: Gan, Yi Fang <yi.fang.gan@...el.com>
> Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>; Jose Abreu
> <joabreu@...opsys.com>; David S . Miller <davem@...emloft.net>; Eric
> Dumazet <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>; Paolo
> Abeni <pabeni@...hat.com>; Maxime Coquelin
> <mcoquelin.stm32@...il.com>; netdev@...r.kernel.org; linux-stm32@st-
> md-mailman.stormreply.com; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org; Looi, Hong Aun <hong.aun.looi@...el.com>; Voon,
> Weifeng <weifeng.voon@...el.com>; Song, Yoong Siang
> <yoong.siang.song@...el.com>; Ahmad Tarmizi, Noor Azura
> <noor.azura.ahmad.tarmizi@...el.com>
> Subject: Re: [PATCH net-next 1/1] net: stmmac: add check for advertising
> linkmode request for set-eee
> 
> On Fri, Oct 27, 2023 at 02:50:54PM +0800, Gan Yi Fang wrote:
> > From: Noor Azura Ahmad Tarmizi <noor.azura.ahmad.tarmizi@...el.com>
> >
> > Add check for advertising linkmode set request with what is currently
> > being supported by PHY before configuring the EEE. Unsupported setting
> > will be rejected and a message will be prompted. No checking is
> > required while setting the EEE to off.
> 
> Why should this functionality be specific to stmmac?
> 
> Why do we need this?
> 
> What is wrong with the checking and masking that phylib is doing?
> 
> Why should we trust the value in edata->supported provided by the user?
> 
> Sorry, but no. I see no reason that this should be done, especially not in the
> stmmac driver.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!