netdev - Re: [PATCH net-next v3 07/47] net: phy: Add support for rate adaptation

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: <6f8e17cb-3f6b-9e89-51a4-9452d562204b@seco.com>
Date:   Sat, 16 Jul 2022 17:55:38 -0400
From:   Sean Anderson <sean.anderson@...o.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Madalin Bucur <madalin.bucur@....com>, netdev@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        linux-arm-kernel@...ts.infradead.org,
        Russell King <linux@...linux.org.uk>,
        linux-kernel@...r.kernel.org,
        Alexandru Marginean <alexandru.marginean@....com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Subject: Re: [PATCH net-next v3 07/47] net: phy: Add support for rate
 adaptation

On 7/16/22 3:39 PM, Andrew Lunn wrote:
>>   drivers/net/phy/phy.c | 21 +++++++++++++++++++++
>>   include/linux/phy.h   | 38 ++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 8d3ee3a6495b..cf4a8b055a42 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -114,6 +114,27 @@ void phy_print_status(struct phy_device *phydev)
>>   }
>>   EXPORT_SYMBOL(phy_print_status);
>>   
>> +/**
>> + * phy_get_rate_adaptation - determine if rate adaptation is supported
>> + * @phydev: The phy device to return rate adaptation for
>> + * @iface: The interface mode to use
>> + *
>> + * This determines the type of rate adaptation (if any) that @phy supports
>> + * using @iface. @iface may be %PHY_INTERFACE_MODE_NA to determine if any
>> + * interface supports rate adaptation.
>> + *
>> + * Return: The type of rate adaptation @phy supports for @iface, or
>> + *         %RATE_ADAPT_NONE.
>> + */
>> +enum rate_adaptation phy_get_rate_adaptation(struct phy_device *phydev,
>> +					     phy_interface_t iface)
>> +{
>> +	if (phydev->drv->get_rate_adaptation)
>> +		return phydev->drv->get_rate_adaptation(phydev, iface);
> 
> It is normal that any call into the driver is performed with the
> phydev->lock held.

Ah, so like phy_ethtool_get_strings.

>>   #define PHY_INIT_TIMEOUT	100000
>>   #define PHY_FORCE_TIMEOUT	10
>> @@ -570,6 +588,7 @@ struct macsec_ops;
>>    * @lp_advertising: Current link partner advertised linkmodes
>>    * @eee_broken_modes: Energy efficient ethernet modes which should be prohibited
>>    * @autoneg: Flag autoneg being used
>> + * @rate_adaptation: Current rate adaptation mode
>>    * @link: Current link state
>>    * @autoneg_complete: Flag auto negotiation of the link has completed
>>    * @mdix: Current crossover
>> @@ -637,6 +656,8 @@ struct phy_device {
>>   	unsigned irq_suspended:1;
>>   	unsigned irq_rerun:1;
>>   
>> +	enum rate_adaptation rate_adaptation;
> 
> It is not clear what the locking is on this member. Is it only safe to
> access it during the adjust_link callback, when it is guaranteed that
> the phydev->lock is held, so the value is consistent? Or is the MAC
> allowed to access this at other times?

The former. My intention is that this has the same access as link/interface/speed/duplex.

--Sean