| Message ID | 20200607145945.30559-5-amitc@mellanox.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add extended state | expand |
> +/** > + * enum ethtool_ext_substate_cable_issue - more information in > + * addition to ETHTOOL_EXT_STATE_CABLE_ISSUE. > + */ > +enum ethtool_ext_substate_cable_issue { > + ETHTOOL_EXT_SUBSTATE_UNSUPPORTED_CABLE = 1, > + ETHTOOL_EXT_SUBSTATE_SHORTED_CABLE, > +}; I'm not too happy about shorted cable. I can see this getting extended to open cable, shorted to another pair, etc. It then becomes a duplicate of the PHY cable testing infrastructure. A more generic > + ETHTOOL_EXT_SUBSTATE_CABLE_TEST_FAILURE, would be better, and then the user can use then use the cable testing infrastructure to get the full details. Andrew
> @@ -108,6 +131,12 @@ static int linkstate_reply_size(const struct ethnl_req_info *req_base, > if (data->sqi_max != -EOPNOTSUPP) > len += nla_total_size(sizeof(u32)); > > + if (data->ext_state_provided) > + len += sizeof(u8); /* LINKSTATE_EXT_STATE */ > + > + if (data->ethtool_ext_state_info.__ext_substate) > + len += sizeof(u8); /* LINKSTATE_EXT_SUBSTATE */ > + This looks wrong. A u8 attribute takes up a lot more space than sizeof(u8) because of the TLV overheads. That is what the nla_total_size() is for. Andrew
> +static void linkstate_get_ext_state(struct net_device *dev, > + struct linkstate_reply_data *data) > +{ > + int err; > + > + if (!dev->ethtool_ops->get_ext_state) > + return; > + > + err = dev->ethtool_ops->get_ext_state(dev, &data->ethtool_ext_state_info); > + if (err) { > + data->ext_state_provided = false; > + return; > + } > + > + data->ext_state_provided = true; > } A void function is rather odd for this sort of thing. It is much more normal to return an error code, -EOPNOTSUPP if the op is not available, or 0 if it all went well. Andrew
On Sun, Jun 07, 2020 at 05:59:39PM +0300, Amit Cohen wrote: > Currently, drivers can only tell whether the link is up/down using > LINKSTATE_GET, but no additional information is given. > > Add attributes to LINKSTATE_GET command in order to allow drivers > to expose the user more information in addition to link state to ease > the debug process, for example, reason for link down state. > > Extended state consists of two attributes - ext_state and ext_substate. > The idea is to avoid 'vendor specific' states in order to prevent > drivers to use specific ext_state that can be in the future common > ext_state. > > The substates allows drivers to add more information to the common > ext_state. For example, vendor can expose 'Autoneg failure' as > ext_state and add 'No partner detected during force mode' as > ext_substate. > > If a driver cannot pinpoint the extended state with the substate > accuracy, it is free to expose only the extended state and omit the > substate attribute. Maybe it is hiding somewhere, but shoudn't there be a check to see if the interface is administratively up? I don't think the information returned here makes much sense if the interface is configured down. Andrew
On 6/7/2020 7:59 AM, Amit Cohen wrote: > Currently, drivers can only tell whether the link is up/down using > LINKSTATE_GET, but no additional information is given. > > Add attributes to LINKSTATE_GET command in order to allow drivers > to expose the user more information in addition to link state to ease > the debug process, for example, reason for link down state. > > Extended state consists of two attributes - ext_state and ext_substate. > The idea is to avoid 'vendor specific' states in order to prevent > drivers to use specific ext_state that can be in the future common > ext_state. > > The substates allows drivers to add more information to the common > ext_state. For example, vendor can expose 'Autoneg failure' as > ext_state and add 'No partner detected during force mode' as > ext_substate. > > If a driver cannot pinpoint the extended state with the substate > accuracy, it is free to expose only the extended state and omit the > substate attribute. > > Signed-off-by: Amit Cohen <amitc@mellanox.com> > Reviewed-by: Petr Machata <petrm@mellanox.com> > Reviewed-by: Jiri Pirko <jiri@mellanox.com> > --- > include/linux/ethtool.h | 22 +++++++++ > include/uapi/linux/ethtool.h | 70 ++++++++++++++++++++++++++++ > include/uapi/linux/ethtool_netlink.h | 2 + > net/ethtool/linkstate.c | 40 ++++++++++++++++ > 4 files changed, 134 insertions(+) > > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h > index a23b26eab479..48ec542f4504 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -86,6 +86,22 @@ struct net_device; > u32 ethtool_op_get_link(struct net_device *dev); > int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *eti); > > + > +/** > + * struct ethtool_ext_state_info - link extended state and substate. > + */ > +struct ethtool_ext_state_info { > + enum ethtool_ext_state ext_state; > + union { > + enum ethtool_ext_substate_autoneg autoneg; > + enum ethtool_ext_substate_link_training link_training; > + enum ethtool_ext_substate_link_logical_mismatch link_logical_mismatch; > + enum ethtool_ext_substate_bad_signal_integrity bad_signal_integrity; > + enum ethtool_ext_substate_cable_issue cable_issue; > + int __ext_substate; > + }; > +}; > + > /** > * ethtool_rxfh_indir_default - get default value for RX flow hash indirection > * @index: Index in RX flow hash indirection table > @@ -245,6 +261,10 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32, > * @get_link: Report whether physical link is up. Will only be called if > * the netdev is up. Should usually be set to ethtool_op_get_link(), > * which uses netif_carrier_ok(). > + * @get_ext_state: Report link extended state. Should set ext_state and > + * ext_substate (ext_substate of 0 means ext_substate is unknown, > + * do not attach ext_substate attribute to netlink message). If not > + * implemented, ext_state and ext_substate will not be sent to userspace. For consistency with the other link-related operations, I would name this get_link_ext_state. > * @get_eeprom: Read data from the device EEPROM. > * Should fill in the magic field. Don't need to check len for zero > * or wraparound. Fill in the data argument with the eeprom values > @@ -384,6 +404,8 @@ struct ethtool_ops { > void (*set_msglevel)(struct net_device *, u32); > int (*nway_reset)(struct net_device *); > u32 (*get_link)(struct net_device *); > + int (*get_ext_state)(struct net_device *, > + struct ethtool_ext_state_info *); > int (*get_eeprom_len)(struct net_device *); > int (*get_eeprom)(struct net_device *, > struct ethtool_eeprom *, u8 *); > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index f4662b3a9e1e..830fa0d6aebe 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -579,6 +579,76 @@ struct ethtool_pauseparam { > __u32 tx_pause; > }; > > +/** > + * enum ethtool_ext_state - link extended state > + */ > +enum ethtool_ext_state { > + ETHTOOL_EXT_STATE_AUTONEG_FAILURE, > + ETHTOOL_EXT_STATE_LINK_TRAINING_FAILURE, > + ETHTOOL_EXT_STATE_LINK_LOGICAL_MISMATCH, > + ETHTOOL_EXT_STATE_BAD_SIGNAL_INTEGRITY, > + ETHTOOL_EXT_STATE_NO_CABLE, > + ETHTOOL_EXT_STATE_CABLE_ISSUE, > + ETHTOOL_EXT_STATE_EEPROM_ISSUE, Does the EEPROM issue would indicate for instance that it was not possile for the firmware/kernel to determine what transceiver capabilities are supported from e.g.: a SFP or SFF EEPROM, and therefore the link state could be down because of that. Is this the idea? > + ETHTOOL_EXT_STATE_CALIBRATION_FAILURE, > + ETHTOOL_EXT_STATE_POWER_BUDGET_EXCEEDED, > + ETHTOOL_EXT_STATE_OVERHEAT, > +}; > + > +/** > + * enum ethtool_ext_substate_autoneg - more information in addition to > + * ETHTOOL_EXT_STATE_AUTONEG_FAILURE. > + */ > +enum ethtool_ext_substate_autoneg { > + ETHTOOL_EXT_SUBSTATE_AN_NO_PARTNER_DETECTED = 1, > + ETHTOOL_EXT_SUBSTATE_AN_ACK_NOT_RECEIVED, > + ETHTOOL_EXT_SUBSTATE_AN_NEXT_PAGE_EXCHANGE_FAILED, > + ETHTOOL_EXT_SUBSTATE_AN_NO_PARTNER_DETECTED_FORCE_MODE, > + ETHTOOL_EXT_SUBSTATE_AN_FEC_MISMATCH_DURING_OVERRIDE, > + ETHTOOL_EXT_SUBSTATE_AN_NO_HCD, > +}; > + > +/** > + * enum ethtool_ext_substate_link_training - more information in addition to > + * ETHTOOL_EXT_STATE_LINK_TRAINING_FAILURE. > + */ > +enum ethtool_ext_substate_link_training { > + ETHTOOL_EXT_SUBSTATE_LT_KR_FRAME_LOCK_NOT_ACQUIRED = 1, > + ETHTOOL_EXT_SUBSTATE_LT_KR_LINK_INHIBIT_TIMEOUT, > + ETHTOOL_EXT_SUBSTATE_LT_KR_LINK_PARTNER_DID_NOT_SET_RECEIVER_READY, > + ETHTOOL_EXT_SUBSTATE_LT_REMOTE_FAULT, > +}; OK, so we leave it to the driver to report link sub-state information that is relevnt to the supported/avertised link modes, such that for instance, reporting LT_KR_FRAME_LOCK_NOT_ACQUIRED would not happen if we were only advertising 1000baseT for instance. That sounds fair.
Andrew Lunn <andrew@lunn.ch> writes: >> +/** >> + * enum ethtool_ext_substate_cable_issue - more information in >> + * addition to ETHTOOL_EXT_STATE_CABLE_ISSUE. >> + */ >> +enum ethtool_ext_substate_cable_issue { >> + ETHTOOL_EXT_SUBSTATE_UNSUPPORTED_CABLE = 1, >> + ETHTOOL_EXT_SUBSTATE_SHORTED_CABLE, >> +}; > >I'm not too happy about shorted cable. I can see this getting extended to open cable, shorted to another pair, etc. It then becomes a duplicate of the PHY cable testing infrastructure. A more generic > >> + ETHTOOL_EXT_SUBSTATE_CABLE_TEST_FAILURE, > >would be better, and then the user can use then use the cable testing infrastructure to get the full details. > Ok, makes sense. > Andrew
On 07-Jun-20 21:17, Florian Fainelli wrote: > > > On 6/7/2020 7:59 AM, Amit Cohen wrote: >> Currently, drivers can only tell whether the link is up/down using >> LINKSTATE_GET, but no additional information is given. >> >> Add attributes to LINKSTATE_GET command in order to allow drivers >> to expose the user more information in addition to link state to ease >> the debug process, for example, reason for link down state. >> >> Extended state consists of two attributes - ext_state and ext_substate. >> The idea is to avoid 'vendor specific' states in order to prevent >> drivers to use specific ext_state that can be in the future common >> ext_state. >> >> The substates allows drivers to add more information to the common >> ext_state. For example, vendor can expose 'Autoneg failure' as >> ext_state and add 'No partner detected during force mode' as >> ext_substate. >> >> If a driver cannot pinpoint the extended state with the substate >> accuracy, it is free to expose only the extended state and omit the >> substate attribute. >> >> Signed-off-by: Amit Cohen <amitc@mellanox.com> >> Reviewed-by: Petr Machata <petrm@mellanox.com> >> Reviewed-by: Jiri Pirko <jiri@mellanox.com> >> --- >> include/linux/ethtool.h | 22 +++++++++ >> include/uapi/linux/ethtool.h | 70 ++++++++++++++++++++++++++++ >> include/uapi/linux/ethtool_netlink.h | 2 + >> net/ethtool/linkstate.c | 40 ++++++++++++++++ >> 4 files changed, 134 insertions(+) >> >> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h >> index a23b26eab479..48ec542f4504 100644 >> --- a/include/linux/ethtool.h >> +++ b/include/linux/ethtool.h >> @@ -86,6 +86,22 @@ struct net_device; >> u32 ethtool_op_get_link(struct net_device *dev); >> int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *eti); >> >> + >> +/** >> + * struct ethtool_ext_state_info - link extended state and substate. >> + */ >> +struct ethtool_ext_state_info { >> + enum ethtool_ext_state ext_state; >> + union { >> + enum ethtool_ext_substate_autoneg autoneg; >> + enum ethtool_ext_substate_link_training link_training; >> + enum ethtool_ext_substate_link_logical_mismatch link_logical_mismatch; >> + enum ethtool_ext_substate_bad_signal_integrity bad_signal_integrity; >> + enum ethtool_ext_substate_cable_issue cable_issue; >> + int __ext_substate; >> + }; >> +}; >> + >> /** >> * ethtool_rxfh_indir_default - get default value for RX flow hash indirection >> * @index: Index in RX flow hash indirection table >> @@ -245,6 +261,10 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32, >> * @get_link: Report whether physical link is up. Will only be called if >> * the netdev is up. Should usually be set to ethtool_op_get_link(), >> * which uses netif_carrier_ok(). >> + * @get_ext_state: Report link extended state. Should set ext_state and >> + * ext_substate (ext_substate of 0 means ext_substate is unknown, >> + * do not attach ext_substate attribute to netlink message). If not >> + * implemented, ext_state and ext_substate will not be sent to userspace. > > For consistency with the other link-related operations, I would name > this get_link_ext_state. > ok. >> * @get_eeprom: Read data from the device EEPROM. >> * Should fill in the magic field. Don't need to check len for zero >> * or wraparound. Fill in the data argument with the eeprom values >> @@ -384,6 +404,8 @@ struct ethtool_ops { >> void (*set_msglevel)(struct net_device *, u32); >> int (*nway_reset)(struct net_device *); >> u32 (*get_link)(struct net_device *); >> + int (*get_ext_state)(struct net_device *, >> + struct ethtool_ext_state_info *); >> int (*get_eeprom_len)(struct net_device *); >> int (*get_eeprom)(struct net_device *, >> struct ethtool_eeprom *, u8 *); >> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h >> index f4662b3a9e1e..830fa0d6aebe 100644 >> --- a/include/uapi/linux/ethtool.h >> +++ b/include/uapi/linux/ethtool.h >> @@ -579,6 +579,76 @@ struct ethtool_pauseparam { >> __u32 tx_pause; >> }; >> >> +/** >> + * enum ethtool_ext_state - link extended state >> + */ >> +enum ethtool_ext_state { >> + ETHTOOL_EXT_STATE_AUTONEG_FAILURE, >> + ETHTOOL_EXT_STATE_LINK_TRAINING_FAILURE, >> + ETHTOOL_EXT_STATE_LINK_LOGICAL_MISMATCH, >> + ETHTOOL_EXT_STATE_BAD_SIGNAL_INTEGRITY, >> + ETHTOOL_EXT_STATE_NO_CABLE, >> + ETHTOOL_EXT_STATE_CABLE_ISSUE, >> + ETHTOOL_EXT_STATE_EEPROM_ISSUE, > > Does the EEPROM issue would indicate for instance that it was not > possile for the firmware/kernel to determine what transceiver > capabilities are supported from e.g.: a SFP or SFF EEPROM, and therefore > the link state could be down because of that. Is this the idea? > We get this reason from firmware if the cable identifier is not spec compliant, missing or was not able to be read on time (I2C reading issue). >> + ETHTOOL_EXT_STATE_CALIBRATION_FAILURE, >> + ETHTOOL_EXT_STATE_POWER_BUDGET_EXCEEDED, >> + ETHTOOL_EXT_STATE_OVERHEAT, >> +}; >> + >> +/** >> + * enum ethtool_ext_substate_autoneg - more information in addition to >> + * ETHTOOL_EXT_STATE_AUTONEG_FAILURE. >> + */ >> +enum ethtool_ext_substate_autoneg { >> + ETHTOOL_EXT_SUBSTATE_AN_NO_PARTNER_DETECTED = 1, >> + ETHTOOL_EXT_SUBSTATE_AN_ACK_NOT_RECEIVED, >> + ETHTOOL_EXT_SUBSTATE_AN_NEXT_PAGE_EXCHANGE_FAILED, >> + ETHTOOL_EXT_SUBSTATE_AN_NO_PARTNER_DETECTED_FORCE_MODE, >> + ETHTOOL_EXT_SUBSTATE_AN_FEC_MISMATCH_DURING_OVERRIDE, >> + ETHTOOL_EXT_SUBSTATE_AN_NO_HCD, >> +}; >> + >> +/** >> + * enum ethtool_ext_substate_link_training - more information in addition to >> + * ETHTOOL_EXT_STATE_LINK_TRAINING_FAILURE. >> + */ >> +enum ethtool_ext_substate_link_training { >> + ETHTOOL_EXT_SUBSTATE_LT_KR_FRAME_LOCK_NOT_ACQUIRED = 1, >> + ETHTOOL_EXT_SUBSTATE_LT_KR_LINK_INHIBIT_TIMEOUT, >> + ETHTOOL_EXT_SUBSTATE_LT_KR_LINK_PARTNER_DID_NOT_SET_RECEIVER_READY, >> + ETHTOOL_EXT_SUBSTATE_LT_REMOTE_FAULT, >> +}; > > OK, so we leave it to the driver to report link sub-state information > that is relevnt to the supported/avertised link modes, such that for > instance, reporting LT_KR_FRAME_LOCK_NOT_ACQUIRED would not happen if we > were only advertising 1000baseT for instance. That sounds fair. >
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index a23b26eab479..48ec542f4504 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -86,6 +86,22 @@ struct net_device; u32 ethtool_op_get_link(struct net_device *dev); int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *eti); + +/** + * struct ethtool_ext_state_info - link extended state and substate. + */ +struct ethtool_ext_state_info { + enum ethtool_ext_state ext_state; + union { + enum ethtool_ext_substate_autoneg autoneg; + enum ethtool_ext_substate_link_training link_training; + enum ethtool_ext_substate_link_logical_mismatch link_logical_mismatch; + enum ethtool_ext_substate_bad_signal_integrity bad_signal_integrity; + enum ethtool_ext_substate_cable_issue cable_issue; + int __ext_substate; + }; +}; + /** * ethtool_rxfh_indir_default - get default value for RX flow hash indirection * @index: Index in RX flow hash indirection table @@ -245,6 +261,10 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32, * @get_link: Report whether physical link is up. Will only be called if * the netdev is up. Should usually be set to ethtool_op_get_link(), * which uses netif_carrier_ok(). + * @get_ext_state: Report link extended state. Should set ext_state and + * ext_substate (ext_substate of 0 means ext_substate is unknown, + * do not attach ext_substate attribute to netlink message). If not + * implemented, ext_state and ext_substate will not be sent to userspace. * @get_eeprom: Read data from the device EEPROM. * Should fill in the magic field. Don't need to check len for zero * or wraparound. Fill in the data argument with the eeprom values @@ -384,6 +404,8 @@ struct ethtool_ops { void (*set_msglevel)(struct net_device *, u32); int (*nway_reset)(struct net_device *); u32 (*get_link)(struct net_device *); + int (*get_ext_state)(struct net_device *, + struct ethtool_ext_state_info *); int (*get_eeprom_len)(struct net_device *); int (*get_eeprom)(struct net_device *, struct ethtool_eeprom *, u8 *); diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index f4662b3a9e1e..830fa0d6aebe 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -579,6 +579,76 @@ struct ethtool_pauseparam { __u32 tx_pause; }; +/** + * enum ethtool_ext_state - link extended state + */ +enum ethtool_ext_state { + ETHTOOL_EXT_STATE_AUTONEG_FAILURE, + ETHTOOL_EXT_STATE_LINK_TRAINING_FAILURE, + ETHTOOL_EXT_STATE_LINK_LOGICAL_MISMATCH, + ETHTOOL_EXT_STATE_BAD_SIGNAL_INTEGRITY, + ETHTOOL_EXT_STATE_NO_CABLE, + ETHTOOL_EXT_STATE_CABLE_ISSUE, + ETHTOOL_EXT_STATE_EEPROM_ISSUE, + ETHTOOL_EXT_STATE_CALIBRATION_FAILURE, + ETHTOOL_EXT_STATE_POWER_BUDGET_EXCEEDED, + ETHTOOL_EXT_STATE_OVERHEAT, +}; + +/** + * enum ethtool_ext_substate_autoneg - more information in addition to + * ETHTOOL_EXT_STATE_AUTONEG_FAILURE. + */ +enum ethtool_ext_substate_autoneg { + ETHTOOL_EXT_SUBSTATE_AN_NO_PARTNER_DETECTED = 1, + ETHTOOL_EXT_SUBSTATE_AN_ACK_NOT_RECEIVED, + ETHTOOL_EXT_SUBSTATE_AN_NEXT_PAGE_EXCHANGE_FAILED, + ETHTOOL_EXT_SUBSTATE_AN_NO_PARTNER_DETECTED_FORCE_MODE, + ETHTOOL_EXT_SUBSTATE_AN_FEC_MISMATCH_DURING_OVERRIDE, + ETHTOOL_EXT_SUBSTATE_AN_NO_HCD, +}; + +/** + * enum ethtool_ext_substate_link_training - more information in addition to + * ETHTOOL_EXT_STATE_LINK_TRAINING_FAILURE. + */ +enum ethtool_ext_substate_link_training { + ETHTOOL_EXT_SUBSTATE_LT_KR_FRAME_LOCK_NOT_ACQUIRED = 1, + ETHTOOL_EXT_SUBSTATE_LT_KR_LINK_INHIBIT_TIMEOUT, + ETHTOOL_EXT_SUBSTATE_LT_KR_LINK_PARTNER_DID_NOT_SET_RECEIVER_READY, + ETHTOOL_EXT_SUBSTATE_LT_REMOTE_FAULT, +}; + +/** + * enum ethtool_ext_substate_logical_mismatch - more information in addition + * to ETHTOOL_EXT_STATE_LINK_LOGICAL_MISMATCH. + */ +enum ethtool_ext_substate_link_logical_mismatch { + ETHTOOL_EXT_SUBSTATE_LLM_PCS_DID_NOT_ACQUIRE_BLOCK_LOCK = 1, + ETHTOOL_EXT_SUBSTATE_LLM_PCS_DID_NOT_ACQUIRE_AM_LOCK, + ETHTOOL_EXT_SUBSTATE_LLM_PCS_DID_NOT_GET_ALIGN_STATUS, + ETHTOOL_EXT_SUBSTATE_LLM_FC_FEC_IS_NOT_LOCKED, + ETHTOOL_EXT_SUBSTATE_LLM_RS_FEC_IS_NOT_LOCKED, +}; + +/** + * enum ethtool_ext_substate_bad_signal_integrity - more information in + * addition to ETHTOOL_EXT_STATE_BAD_SIGNAL_INTEGRITY. + */ +enum ethtool_ext_substate_bad_signal_integrity { + ETHTOOL_EXT_SUBSTATE_BSI_LARGE_NUMBER_OF_PHYSICAL_ERRORS = 1, + ETHTOOL_EXT_SUBSTATE_BSI_UNSUPPORTED_RATE, +}; + +/** + * enum ethtool_ext_substate_cable_issue - more information in + * addition to ETHTOOL_EXT_STATE_CABLE_ISSUE. + */ +enum ethtool_ext_substate_cable_issue { + ETHTOOL_EXT_SUBSTATE_UNSUPPORTED_CABLE = 1, + ETHTOOL_EXT_SUBSTATE_SHORTED_CABLE, +}; + #define ETH_GSTRING_LEN 32 /** diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h index e6f109b76c9a..1de8b77bedff 100644 --- a/include/uapi/linux/ethtool_netlink.h +++ b/include/uapi/linux/ethtool_netlink.h @@ -234,6 +234,8 @@ enum { ETHTOOL_A_LINKSTATE_LINK, /* u8 */ ETHTOOL_A_LINKSTATE_SQI, /* u32 */ ETHTOOL_A_LINKSTATE_SQI_MAX, /* u32 */ + ETHTOOL_A_LINKSTATE_EXT_STATE, /* u8 */ + ETHTOOL_A_LINKSTATE_EXT_SUBSTATE, /* u8 */ /* add new constants above here */ __ETHTOOL_A_LINKSTATE_CNT, diff --git a/net/ethtool/linkstate.c b/net/ethtool/linkstate.c index 7f47ba89054e..0f3a15fe9919 100644 --- a/net/ethtool/linkstate.c +++ b/net/ethtool/linkstate.c @@ -13,6 +13,8 @@ struct linkstate_reply_data { int link; int sqi; int sqi_max; + bool ext_state_provided; + struct ethtool_ext_state_info ethtool_ext_state_info; }; #define LINKSTATE_REPDATA(__reply_base) \ @@ -25,6 +27,8 @@ linkstate_get_policy[ETHTOOL_A_LINKSTATE_MAX + 1] = { [ETHTOOL_A_LINKSTATE_LINK] = { .type = NLA_REJECT }, [ETHTOOL_A_LINKSTATE_SQI] = { .type = NLA_REJECT }, [ETHTOOL_A_LINKSTATE_SQI_MAX] = { .type = NLA_REJECT }, + [ETHTOOL_A_LINKSTATE_EXT_STATE] = { .type = NLA_REJECT }, + [ETHTOOL_A_LINKSTATE_EXT_SUBSTATE] = { .type = NLA_REJECT }, }; static int linkstate_get_sqi(struct net_device *dev) @@ -61,6 +65,23 @@ static int linkstate_get_sqi_max(struct net_device *dev) mutex_unlock(&phydev->lock); return ret; +}; + +static void linkstate_get_ext_state(struct net_device *dev, + struct linkstate_reply_data *data) +{ + int err; + + if (!dev->ethtool_ops->get_ext_state) + return; + + err = dev->ethtool_ops->get_ext_state(dev, &data->ethtool_ext_state_info); + if (err) { + data->ext_state_provided = false; + return; + } + + data->ext_state_provided = true; } static int linkstate_prepare_data(const struct ethnl_req_info *req_base, @@ -88,6 +109,8 @@ static int linkstate_prepare_data(const struct ethnl_req_info *req_base, data->sqi_max = ret; + linkstate_get_ext_state(dev, data); + ethnl_ops_complete(dev); return 0; @@ -108,6 +131,12 @@ static int linkstate_reply_size(const struct ethnl_req_info *req_base, if (data->sqi_max != -EOPNOTSUPP) len += nla_total_size(sizeof(u32)); + if (data->ext_state_provided) + len += sizeof(u8); /* LINKSTATE_EXT_STATE */ + + if (data->ethtool_ext_state_info.__ext_substate) + len += sizeof(u8); /* LINKSTATE_EXT_SUBSTATE */ + return len; } @@ -129,6 +158,17 @@ static int linkstate_fill_reply(struct sk_buff *skb, nla_put_u32(skb, ETHTOOL_A_LINKSTATE_SQI_MAX, data->sqi_max)) return -EMSGSIZE; + if (data->ext_state_provided) { + if (nla_put_u8(skb, ETHTOOL_A_LINKSTATE_EXT_STATE, + data->ethtool_ext_state_info.ext_state)) + return -EMSGSIZE; + + if (data->ethtool_ext_state_info.__ext_substate && + nla_put_u8(skb, ETHTOOL_A_LINKSTATE_EXT_SUBSTATE, + data->ethtool_ext_state_info.__ext_substate)) + return -EMSGSIZE; + } + return 0; }