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]

Message-ID: <51ee86d8-5baa-4419-9419-bcf737229868@lunn.ch>
Date: Fri, 25 Aug 2023 15:47:20 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Pawel Chmielewski <pawel.chmielewski@...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

> 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.

> The third step in case with those NICs with FW-managed PHYs should be
> done manually in the MAC driver somewhere. Like "I am qede and I don't
> support mode XX at 50Gbps, but support the rest, so I clear that one bit".

I don't think that will work. New bits keep getting added, more speeds
added. So 'support the rest' is not well defined. You need an explicit
list of link modes the driver needs. We already have code to convert
an array of link mode bits into an actual mask, e.g:

        linkmode_set_bit_array(phy_basic_t1_features_array,
                               ARRAY_SIZE(phy_basic_t1_features_array),
                               phy_basic_t1_features);

	Andrew