netdev - Re: [PATCH v3 1/3] net: phy: bcm54811: New link mode for BroadR-Reach

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: <1188b119-1191-4afa-8381-d022d447086c@lunn.ch>
Date: Thu, 23 May 2024 16:27:42 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Kamil Horák, 2N <kamilh@...s.com>
Cc: netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] net: phy: bcm54811: New link mode for BroadR-Reach

> > ethtool -s eth42 autoneg off linkmode 1BR10
> 
> This sounds perfect to me. The second (shorter) way is better because, at
> least with BCM54811, given the link mode, the duplex and speed are also
> determined. All BroadR-Reach link modes are full duplex, anyway.

Great.

> > You can probably add a new member to ethtool_link_ksettings to pass it
> > to phylib. From there, it probably needs putting into a phy_device
> > member, next to speed and duplex. The PHY driver can then use it to
> > configure the hardware.
> I did not dare to cut this deep so far, but as I see there is a demand,
> let's go for it!

It also seems quite a generic feature. e.g. to select between
1000BaseT_FULL and 1000BaseT1_FULL. So it should get reused for other
use cases.

> > 
> > 2) Invalid combinations of link modes when auto-neg is
> > enabled. Probably the first question to answer is, is this specific to
> > this PHY, or generic across all PHYs which support BR and IEEE
> > modes. If it is generic, we can add tests in
> > phy_ethtool_ksettings_set() to return EINVAL. If this is specific to
> > this PHY, it gets messy. When phylib call phy_start_aneg() to
> > configure the hardware, it does not expect it to return an error. We
> > might need to add an additional op to phy_driver to allow the PHY
> > driver to validate settings when phy_ethtool_ksettings_set() is
> > called. This would help solve a similar problem with a new mediatek
> > PHY which is broken with forced modes.
> Regarding the specificity, it definitely touches the BCM54811 and even more
> BCM54810, because the ...810 supports autoneg  in BroadR-Reach mode too.

That was what i did not know. Does 802.3 allow auto-neg for these
BroadR-Reach modes at the same time as 'normal' modes. And it seems
like the ..810 supports is, and i assume it conforms to 802.3? So we
cannot globally return an error in ethtool_ksetting_set() with a mix
or modes, it needs to be the driver which decides.

   Andrew