netdev - Re: What is the purpose of the first phylink_validate() call from phylink

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]

Message-ID: <20231005104456.sytvw32f2r2qnpuk@skbuf>
Date: Thu, 5 Oct 2023 13:44:56 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: netdev@...r.kernel.org
Subject: Re: What is the purpose of the first phylink_validate() call from
 phylink_create()?

On Thu, Oct 05, 2023 at 10:16:48AM +0100, Russell King (Oracle) wrote:
> You've found the exact reason for it - so that we report something that
> seems at least reasonable to userspace, rather than reporting absolutely
> nothing which may cause issues.

Thanks for confirming. I don't need to change the user-observable behavior,
I think I can work my way around it. I just wanted to know what to look for,
and I deleted the phylink_validate() call just to exaggerate the effect.

> The original code in mvneta would've done this:
> 
> int mvneta_ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> {
>         struct mvneta_port *pp = netdev_priv(dev);
> 
>         if (!pp->phy_dev)
>                 return -ENODEV;
> 
>         return phy_ethtool_gset(pp->phy_dev, cmd);
> }
> 
> Thus making the call fail if the device wasn't up - and that may be
> an alternative if we're expecting a PHY but we have none.

Ok, but I admit I don't know how to make phylink_ethtool_ksettings_get()
return -ENODEV just for this case. For example, I'm thinking of the
situation of a copper SFP module with an inaccessible PHY, using SGMII.
My understanding is that phylink_expects_phy() would return true, so
that helper couldn't be used to discern this kind of SFP from a PHY
which is accessible but phylink_bringup_phy() wasn't yet called on it.

In any case, please consider the question answered for now with no other
actionable item.

Also, I now see that patchwork thinks this question is a patch
(https://patchwork.kernel.org/project/netdevbpf/patch/20231004222523.p5t2cqaot6irstwq@skbuf/),
so:

pw-bot: not-applicable