netdev - Re: [Intel-wired-lan] [PATCH iwl-next v2 2/9] ethtool: Add forced speed to supported link modes maps

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]

Date: Thu, 31 Aug 2023 15:08:52 +0200
From: Pawel Chmielewski <pawel.chmielewski@...el.com>
To: Andrew Lunn <andrew@...n.ch>
CC: Alexander Lobakin <aleksander.lobakin@...el.com>, "Greenwalt, Paul"
	<paul.greenwalt@...el.com>, <aelior@...vell.com>,
	<intel-wired-lan@...ts.osuosl.org>, <manishc@...vell.com>,
	<netdev@...r.kernel.org>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 2/9] ethtool: Add forced
 speed to supported link modes maps

On Fri, Aug 25, 2023 at 03:47:20PM +0200, Andrew Lunn wrote:
> > Let me think how we could do that.
> > Andrew's idea is good. But most high-speed NICs, which have a standalone
> > management firmware for PHY, don't use phylib/phylink.
> > So in order to be able to unify all that, they should have ->supported
> > bitmap somewhere else. Not sure struct net_device is the best place...
> 
> I would probably keep it in the driver priv structure, and just pass
> it as needed. So long as you only need one or two values, i don't see
> the need for a shared structure.
> 
> > If I recall Phylink logics correctly (it's been a while since I last
> > time was working with my embedded project),
> > 
> > 1) in the NIC (MAC) driver, you initialize ->supported with *speeds* and
> >    stuff like duplex, no link modes;
> > 2) Phylink core sets the corresponding link mode bits;
> > 3) phylib core then clears the bits unsupported by the PHY IIRC
> 
> No, not really.
> 
> All i think you need is a low level helper. So don't worry too much
> about how phylink works, just implement that low level helper passing
> in values as needed, not phylib or phylink structure.
> 
> What i don't want is a second infrastructure to be built for those MAC
> drivers which don't use Linux to control the PHY. Either share a few
> helpers, or swap to phylink.
> 

Let me check if I understand correctly- is that what was sent with the
v3 [1] , with the initialization helper (ethtool_forced_speed_maps_init)
and the structure map in the ethtool code? Or do you have another helper
in mind?

[1] https://lore.kernel.org/netdev/20230823180633.2450617-5-pawel.chmielewski@intel.com/T/#m208153896dfd623da278427285d3bda25a74ef95