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

2017-06-27 Thread David Miller
From: Andrew Lunn Date: Mon, 26 Jun 2017 15:34:39 +0200 > I still fear this is going to be an ethtool call with only one user. That is my fear as well. We are also in a sort-of moratorium for adding new major ethtool features until the conversion of ethtool over to netlink

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

2017-06-26 Thread Gal Pressman
> 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

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

2017-06-26 Thread Andrew Lunn
> The driver has a good intimate information of his device > implementation, and hence an analysis done by the device vendor is > favorable. For a firmware based NIC, maybe. For a discrete NIC, making use of all the Linux subsystems, this is going to be hard from within the kernel. > The driver

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

2017-06-26 Thread Andrew Lunn
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

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

2017-06-26 Thread Gal Pressman
> 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,

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

2017-06-25 Thread Andrew Lunn
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,

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

2017-06-25 Thread Gal Pressman
> 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

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

2017-06-24 Thread Andrew Lunn
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

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

2017-06-23 Thread Andrew Lunn
On Fri, Jun 23, 2017 at 11:23:17AM +0300, Gal Pressman wrote: > >> The I2C bus that's connected to this module (interface). > >> We can add another reason for MDIO BUS errors or merge to one BUS error > >> reason. > +ETHTOOL_LINK_UNSUPP_EEPROM, /* Unsupported EEPROM */ > >>> Which

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

2017-06-23 Thread Gal Pressman
>> The I2C bus that's connected to this module (interface). >> We can add another reason for MDIO BUS errors or merge to one BUS error >> reason. + ETHTOOL_LINK_UNSUPP_EEPROM, /* Unsupported EEPROM */ >>> Which EEPROM? >> Module EEPROM. > Which module? This is all very vague. Some of the

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

2017-06-22 Thread Andrew Lunn
> The I2C bus that's connected to this module (interface). > We can add another reason for MDIO BUS errors or merge to one BUS error > reason. > >> + ETHTOOL_LINK_UNSUPP_EEPROM, /* Unsupported EEPROM */ > > Which EEPROM? > > Module EEPROM. Which module? This is all very vague. Some of the

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

2017-06-22 Thread Gal Pressman
>> +ETHTOOL_LINK_PWR_BUDGET_EXC, /* Power budget exceeded */ >> +ETHTOOL_LINK_MODULE_ADMIN_DOWN, /* Module admin down */ >> + >> +ETHTOOL_LINK_REASONS_COUNT >> +}; > Any enumerated list is going to get changed too often. > Could the API just return a string? The motivation for the

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

2017-06-22 Thread Gal Pressman
>> +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.

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

2017-06-21 Thread Stephen Hemminger
On Wed, 21 Jun 2017 16:04:44 +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, /*

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

2017-06-21 Thread Andrew Lunn
> +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. > +

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

2017-06-21 Thread Gal Pressman
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