netdev - Re: [PATCH net v1] net: dsa: mv88e6xxx: Make unsupported C45 reads return 0xffff

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] [day] [month] [year] [list]

Date: Wed, 24 Jan 2024 07:59:48 -0800
From: Tim Menninger <tmenninger@...estorage.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Vladimir Oltean <vladimir.oltean@....com>, netdev-maintainers <edumazet@...gle.com>, kuba@...nel.org, 
	pabeni@...hat.com, davem@...emloft.net, netdev <netdev@...r.kernel.org>, 
	stable@...r.kernel.org
Subject: Re: [PATCH net v1] net: dsa: mv88e6xxx: Make unsupported C45 reads
 return 0xffff

On Tue, Jan 23, 2024 at 2:59 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > Does that mean if there's a device there but it doesn't support C45 (no
> > phy_read_c45), it will now return ENODEV?
>
> Yes, mv88e6xxx_mdio_read_c45() will return -ENODEV if
> chip->info->ops->phy_read_c45 is NULL. That will cause the scan of
> that address to immediately skip to the next address. This is old
> behaviour for C22:
>
> commit 02a6efcab675fe32815d824837784c3f42a7d892
> Author: Alexandre Belloni <alexandre.belloni@...tlin.com>
> Date:   Tue Apr 24 18:09:04 2018 +0200
>
>     net: phy: allow scanning busses with missing phys
>
>     Some MDIO busses will error out when trying to read a phy address with no
>     phy present at that address. In that case, probing the bus will fail
>     because __mdiobus_register() is scanning the bus for all possible phys
>     addresses.
>
>     In case MII_PHYSID1 returns -EIO or -ENODEV, consider there is no phy at
>     this address and set the phy ID to 0xffffffff which is then properly
>     handled in get_phy_device().
>
> And there are a few MDIO bus drivers which make use of this, e.g.
>
> static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum)
> {
>         struct lan9303 *chip = ds->priv;
>         int phy_base = chip->phy_addr_base;
>
>         if (phy == phy_base)
>                 return lan9303_virt_phy_reg_read(chip, regnum);
>         if (phy > phy_base + 2)
>                 return -ENODEV;
>
>         return chip->ops->phy_read(chip, phy, regnum);
>
> This Ethernet switch supports only a number of PHY addresses, and
> returns -ENODEV for the rest.
>
> So its a legitimate way to say there is nothing here.
>
> You suggestion of allowing ENOPSUPP for C45 would of fixed the
> problem, but C22 and C45 would support different error codes, which i
> don't like. Its better to be uniform.
>
>         Andrew

Excellent, color me convinced.