netdev - Re: [PATCH net-next v6 2/7] net: phy: Support speed selection for PHY loopback

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: <dad7188a-bdd9-4453-aa8d-1622fd8ea9d3@engleder-embedded.com>
Date: Wed, 12 Feb 2025 21:01:14 +0100
From: Gerhard Engleder <gerhard@...leder-embedded.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: hkallweit1@...il.com, linux@...linux.org.uk, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 netdev@...r.kernel.org
Subject: Re: [PATCH net-next v6 2/7] net: phy: Support speed selection for PHY
 loopback

On 12.02.25 02:48, Andrew Lunn wrote:
>> +int phy_loopback(struct phy_device *phydev, bool enable, int speed)
>> +{
>> +	bool link_up = false;
>> +	int ret = 0;
>> +
>> +	if (!phydev->drv)
>> +		return -EIO;
>> +
>> +	mutex_lock(&phydev->lock);
>> +
>> +	if (enable && phydev->loopback_enabled) {
>> +		ret = -EBUSY;
>> +		goto out;
>> +	}
>> +
>> +	if (!enable && !phydev->loopback_enabled) {
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	if (enable) {
>> +		/*
>> +		 * Link up is signaled with a defined speed. If speed changes,
>> +		 * then first link down and after that link up needs to be
>> +		 * signaled.
>> +		 */
>> +		if (phydev->link && phydev->state == PHY_RUNNING) {
>> +			/* link is up and signaled */
>> +			if (speed && phydev->speed != speed) {
>> +				/* signal link down and up for new speed */
>> +				phydev->link = false;
>> +				phydev->state = PHY_NOLINK;
>> +				phy_link_down(phydev);
> 
> If you set the link down here...
> 
>> +
>> +				link_up = true;
>> +			}
>> +		} else {
>> +			/* link is not signaled */
>> +			if (speed) {
>> +				/* signal link up for new speed */
>> +				link_up = true;
>> +			}
>> +		}
>> +	}
>> +
>> +	if (phydev->drv->set_loopback)
>> +		ret = phydev->drv->set_loopback(phydev, enable, speed);
>> +	else
>> +		ret = genphy_loopback(phydev, enable, speed);
>> +
>> +	if (ret)
>> +		goto out;
> 
> and this fails, you leave the link down. You should make an attempt to
> restore the link to the old state before returning the error.

I will try to do the same as for loopback disable to try to restore the
the link.

Gerhard