netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Gal Pressman <galp@mellanox.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	"John W. Linville" <linville@tuxdriver.com>,
	Saeed Mahameed <saeedm@mellanox.com>,
	Vidya Sagar Ravipati <vidya@cumulusnetworks.com>,
	Jiri Pirko <jiri@mellanox.com>,
	David Decotigny <decot@googlers.com>,
	kernel-team@fb.com
Subject: Re: [RFC PATCH net-next 1/3] ethtool: Add link down reason callback
Date: Sun, 25 Jun 2017 17:19:53 +0200	[thread overview]
Message-ID: <20170625151953.GA7799@lunn.ch> (raw)
In-Reply-To: <f216ad74-772b-0f6c-1303-ecb0c12afc96@mellanox.com>

On Sun, Jun 25, 2017 at 02:59:24PM +0300, Gal Pressman wrote:
> 
> > On Thu, Jun 22, 2017 at 11:09:04AM +0300, Gal Pressman wrote:
> >>>> +enum {
> >>>> +	ETHTOOL_LINK_VENDOR_SPECIFIC = -1, /* Vendor specific issue provided in vendor_reason */
> >>>> +	ETHTOOL_LINK_NO_ISSUE, /* No issue observed with link */
> >>>> +	ETHTOOL_LINK_REASON_UNKNOWN, /* Unknown reason */
> >>> I think OTHER would be better that UNKNOWN. 
> >> Fine with me.
> >>>> +	ETHTOOL_LINK_NETDEV_CARRIER_DOWN, /* Netdev carrier is down */
> >>>> +	ETHTOOL_LINK_ADMIN_DOWN, /* Admin down */
> >>> These two are interesting. We have that information already. Why do we
> >>> want it again?
> >> My goal is to gather all link issue reasons in one place.
> > I'm actually wondering if this is a user space problem. Nearly
> > everything you list is already available. Some you get from ip link,
> > others from ethtool or ethtool --module-info, including I2C bus
> > error, since you would expect EIO or ETIMEOUT.
> >
> > If you were to write a user space tool using the information what is
> > currently available, what would be missing?
> >
> > 	  Andrew
> 
> I think most of the reasons in this list would be missing.
> Auto negotiation failure,

You can probably get that from the PHY layer. You get both the local
and remote AN capabilities.

> unplugged, over temperature, power budget exceeded..

Don't you get over temperature from the SFF data? Also power budget?

And what does cable unplugged actually mean? Do you have a micro
switch inside the socket? So that is maybe a gpio-key?

Another thing to remember is that your device is the exception to the
rule. You have some firmware doing a lot of the work bringing this all
together. But nearly every other Ethernet interface has a discrete MAC
and PHY, I2C bus driver, EEPROM driver, generic SFF decoder, HWMON
temperature sensor, etc. How does your call work in this normal
situation? How do you make calls into all these subsystems to get the
information? You want a generic solution which can be made to work for
everybody.

	Andrew




> 
> Keep in mind that this is just an initial list, not to mention the vendor reasons which are not part of any existing API.
> I don't see how a user space tool that expects ETIMEOUT/EIO is better than this suggestion.

  reply	other threads:[~2017-06-25 15:19 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-21 13:04 [RFC PATCH net-next 0/3] ethtool: Add link down reason reporting Gal Pressman
2017-06-21 13:04 ` [RFC PATCH net-next 1/3] ethtool: Add link down reason callback Gal Pressman
2017-06-21 13:58   ` Andrew Lunn
2017-06-22  8:09     ` Gal Pressman
2017-06-22 13:34       ` Andrew Lunn
2017-06-23  8:23         ` Gal Pressman
2017-06-23 13:52           ` Andrew Lunn
2017-06-24 19:04       ` Andrew Lunn
2017-06-25 11:59         ` Gal Pressman
2017-06-25 15:19           ` Andrew Lunn [this message]
2017-06-26 11:52             ` Gal Pressman
2017-06-26 13:34               ` Andrew Lunn
2017-06-27 19:40                 ` David Miller
2017-06-21 15:43   ` Stephen Hemminger
2017-06-22 10:33     ` Gal Pressman
2017-06-26 13:28   ` Andrew Lunn
2017-06-26 15:48     ` Gal Pressman
2017-06-21 13:04 ` [RFC PATCH net-next 2/3] net/mlx5: Add PDDR register infrastructure Gal Pressman
2017-06-21 13:04 ` [RFC PATCH net-next 3/3] net/mlx5e: Expose link down reason to ethtool Gal Pressman
2017-06-21 14:03   ` Andrew Lunn
2017-06-22  8:17     ` Gal Pressman
2017-06-22  4:33   ` Jakub Kicinski
2017-06-22  8:33     ` Gal Pressman
2017-06-22 22:38       ` Jakub Kicinski
2017-06-23  8:52         ` Gal Pressman
2017-06-21 15:44 ` [RFC PATCH net-next 0/3] ethtool: Add link down reason reporting Stephen Hemminger
2017-06-22 11:13   ` Gal Pressman
2017-06-22  4:14 ` Jakub Kicinski
2017-06-22 11:37   ` Gal Pressman
2017-06-22 21:53     ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170625151953.GA7799@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=decot@googlers.com \
    --cc=galp@mellanox.com \
    --cc=jiri@mellanox.com \
    --cc=kernel-team@fb.com \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    --cc=saeedm@mellanox.com \
    --cc=vidya@cumulusnetworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).