netdev - Re: [PATCH net-next 1/3] net: netdevsim: Add PHY support in netdevsim

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: <20250708161300.53e96d47@fedora>
Date: Tue, 8 Jul 2025 16:13:00 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: davem@...emloft.net, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com, Jakub Kicinski
 <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
 <pabeni@...hat.com>, Russell King <linux@...linux.org.uk>, Florian Fainelli
 <f.fainelli@...il.com>, Heiner Kallweit <hkallweit1@...il.com>, Vladimir
 Oltean <vladimir.oltean@....com>, Köry Maincent
 <kory.maincent@...tlin.com>, Oleksij Rempel <o.rempel@...gutronix.de>,
 Simon Horman <horms@...nel.org>, Shuah Khan <shuah@...nel.org>,
 linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next 1/3] net: netdevsim: Add PHY support in
 netdevsim

Hi Andrew,

On Tue, 8 Jul 2025 16:01:06 +0200
Andrew Lunn <andrew@...n.ch> wrote:

> > +static int nsim_phy_state_link_set(void *data, u64 val)
> > +{
> > +	struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
> > +
> > +	ns_phy->link = !!val;
> > +
> > +	phy_trigger_machine(ns_phy->phy);
> > +
> > +	return 0;
> > +}
> > +
> > +static int nsim_phy_state_link_get(void *data, u64 *val)
> > +{
> > +	struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
> > +
> > +	*val = ns_phy->link;
> > +
> > +	return 0;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(nsim_phy_state_link_fops, nsim_phy_state_link_get,
> > +			 nsim_phy_state_link_set, "%llu\n");
> > +
> > +static void nsim_phy_debugfs_create(struct nsim_dev_port *port,
> > +				    struct nsim_phy_device *ns_phy)
> > +{
> > +	char phy_dir_name[sizeof("phy") + 10];
> > +
> > +	sprintf(phy_dir_name, "phy%u", ns_phy->phy->phyindex);
> > +
> > +	/* create debugfs stuff */
> > +	ns_phy->phy_dir = debugfs_create_dir(phy_dir_name, port->ddir);
> > +
> > +	debugfs_create_file("link", 0600, ns_phy->phy_dir, ns_phy, &nsim_phy_state_link_fops);  
> 
> Maybe this can be converted into:
> 
> debugfs_create_bool("link", 0600, ns_phy->phy_dir, &ns_phy->link);
> 
> You loose the phy_trigger_machine(), but that might actually be
> good. PHYs are async to any operation you take on them. It can take up
> to 1 second due to the polling before any change is reported. So any
> user space tools which expect an immediate state change are broken. So
> leaving the PHY state machine to poll the PHY to notice the link has
> changed is a better simulation.

That's true indeed... Simpler and more accurate, I'll add that in V3.

Thanks for the feedback :)

Maxime