netdev - Re: [RFC PATCH net-next 1/3] ethtool: Add link down reason callback

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: <20170626132812.GA2623@lunn.ch>
Date:   Mon, 26 Jun 2017 15:28:12 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Gal Pressman <galp@...lanox.com>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        "John W. Linville" <linville@...driver.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Vidya Sagar Ravipati <vidya@...ulusnetworks.com>,
        Jiri Pirko <jiri@...lanox.com>,
        David Decotigny <decot@...glers.com>, kernel-team@...com
Subject: Re: [RFC PATCH net-next 1/3] ethtool: Add link down reason callback

On Wed, Jun 21, 2017 at 04:04:44PM +0300, Gal Pressman wrote:
> Currently, drivers can only tell whether the link is up/down through
> ETHTOOL_GLINK callback, but no additional information is given in case
> of link down.
> This patch provides an infrastructure that allows drivers to hint
> the user with the reason why the link is down, in order to ease the
> debug process.
> 
> Reasons are separated to two types, generic and vendor specific.
> Drivers can reply with a generic reason using the enums provided in this
> patch (and the ones that will be added in the future), which will be
> translated to strings by the userspace ethtool.
> In case of a vendor specific reason (not suitable for most vendors),
> drivers can reply with ETHTOOL_VENDOR_SPECIFIC reason, in this case the
> vendor_reason field should be filled with a vendor specific status code
> which will be parsed by the vendor specific userspace parser if one is
> available.
> 
> This kind of information can save system administrators precious time
> which will not be wasted trying to understand why the link won't go
> up.
> 
> For example, when the cable is unplugged:
> $ ethtool ethXX
> ...
> Link detected: no (Cable unplugged)
> 
> Signed-off-by: Gal Pressman <galp@...lanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
> ---
>  include/linux/ethtool.h      |  2 ++
>  include/uapi/linux/ethtool.h | 33 +++++++++++++++++++++++++++++++++
>  net/core/ethtool.c           | 24 ++++++++++++++++++++++++
>  3 files changed, 59 insertions(+)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 83cc986..d472047 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -374,5 +374,7 @@ struct ethtool_ops {
>  				      struct ethtool_link_ksettings *);
>  	int	(*set_link_ksettings)(struct net_device *,
>  				      const struct ethtool_link_ksettings *);
> +	int	(*get_link_down_reason)(struct net_device *,
> +					struct ethtool_link_down_reason *);
>  };
>  #endif /* _LINUX_ETHTOOL_H */
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 7d4a594..8cf9d2c 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -550,6 +550,13 @@ struct ethtool_pauseparam {
>  
>  #define ETH_GSTRING_LEN		32
>  
> +struct ethtool_link_down_reason {
> +	__u32	cmd;
> +	__u32	reason;
> +	__u32	vendor_reason;
> +	__u32	reserved[4];
> +};

Shouldn't this be a list? The device is over its power budget,
overheating and does not have a cable plugged in. There can be
multiple reasons it is down.

	 Andrew