netdev - Re: [PATCH net-next] bnxt: convert EEE handling to use linkmode bitmaps

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: <4cafbe42-5050-4e98-aaab-fc05f76a32b4@lunn.ch>
Date: Sun, 4 Feb 2024 05:46:01 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Michael Chan <michael.chan@...adcom.com>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	David Miller <davem@...emloft.net>,
	Russell King - ARM Linux <linux@...linux.org.uk>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] bnxt: convert EEE handling to use linkmode
 bitmaps

On Sat, Feb 03, 2024 at 04:16:51PM -0800, Michael Chan wrote:
> On Sat, Feb 3, 2024 at 1:59 PM Andrew Lunn <andrew@...n.ch> wrote:
> >
> > > -     if (!edata->advertised_u32) {
> > > -             edata->advertised_u32 = advertising & eee->supported_u32;
> > > -     } else if (edata->advertised_u32 & ~advertising) {
> > > -             netdev_warn(dev, "EEE advertised %x must be a subset of autoneg advertised speeds %x\n",
> > > -                         edata->advertised_u32, advertising);
> >
> > That warning text looks wrong. I think it should be
> >
> > EEE advertised %x must be a subset of autoneg supported speeds %x
> >
> > and it should print eee->supported, not advertising.
> >

> I think it is correct.  EEE advertised must be a subset of the
> advertised speed.

I don't think that is true. At least, i've not seen any other MAC/PHY
driver change EEE advertising when they change the general
advertising. What i expect is that the PHY and link partner first
resolve the general link speed. They then look at what is being
advertised in terms of EEE and decide if EEE is being advertised by
both ends at the resolved speed. So it does not matter if the link
speed is resolved at 1G, but both ends advertise both 40G and 1G for
EEE. The 40G is simply ignored because they have resolved to 1G.

What does however matter is that EEE supported lists only 1G, but user
space ask for both 1G and 40G to be advertised. I would expect the
driver to mask the requested advertised with support, see that
unsupported link modes are being asked for and return -EINVAL.

	Andrew