Re: [PATCH net-next 1/2] ethtool: add PHY Fast Link Down support

[Heiner Kallweit <hkallweit1@gmail.com>]

On 25.03.2019 18:49, Michal Kubecek wrote:
> On Sun, Mar 24, 2019 at 04:02:43PM +0100, Heiner Kallweit wrote:
>> This adds support for Fast Link Down as new PHY tunable.
>> Fast Link Down reduces the time until a link down event is reported
>> for 1000BaseT. According to the standard it's 750ms what is too long
>> for several use cases.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  include/uapi/linux/ethtool.h | 4 ++++
>>  net/core/ethtool.c           | 2 ++
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>> index 3652b239d..2c7136adc 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -252,9 +252,13 @@ struct ethtool_tunable {
>>  #define DOWNSHIFT_DEV_DEFAULT_COUNT	0xff
>>  #define DOWNSHIFT_DEV_DISABLE		0
>>  
>> +#define ETHTOOL_PHY_FAST_LINK_DOWN_ON	0
>> +#define ETHTOOL_PHY_FAST_LINK_DOWN_OFF	0xff
>> +
>>  enum phy_tunable_id {
>>  	ETHTOOL_PHY_ID_UNSPEC,
>>  	ETHTOOL_PHY_DOWNSHIFT,
>> +	ETHTOOL_PHY_FAST_LINK_DOWN,
>>  	/*
>>  	 * Add your fresh new phy tunable attribute above and remember to update
>>  	 * phy_tunable_strings[] in net/core/ethtool.c
> 
> It would be nice to have a short summary around here explaining how is
> the value interpreted. While it's obvious from the second patch, one
> shouldn't have to go into driver specific implementation to find out.
> 
OK

> I also wonder if the range 0-254 ms is sufficient. Would it be possible
> that there is some other hardware which would support e.g. 300 ms?
> 
The relevant use cases (according to the Marvell spec) require link down
recognition in <50ms. Something >200ms I think wouldn't be considered
as "fast" in general.

And for something completely different, all mails to John are bounced currently:
rejected because 209.85.128.65 is in a black list at 0spam.fusionzero.com Received: from mail-wm1-f65.google.com
I tried from another email address, but same result.

> Michal Kubecek
> 
Heiner

>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index b1eb32419..387d67eb7 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -136,6 +136,7 @@ static const char
>>  phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
>>  	[ETHTOOL_ID_UNSPEC]     = "Unspec",
>>  	[ETHTOOL_PHY_DOWNSHIFT]	= "phy-downshift",
>> +	[ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down",
>>  };
>>  
>>  static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
>> @@ -2432,6 +2433,7 @@ static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
>>  {
>>  	switch (tuna->id) {
>>  	case ETHTOOL_PHY_DOWNSHIFT:
>> +	case ETHTOOL_PHY_FAST_LINK_DOWN:
>>  		if (tuna->len != sizeof(u8) ||
>>  		    tuna->type_id != ETHTOOL_TUNABLE_U8)
>>  			return -EINVAL;
>> -- 
>> 2.21.0
>>
>>
>