Subject: Re: [RFC PATCH net-next 1/3] net: rtnetlink: Add link-down reason to RTNL messages

From: Andrew Lunn <[email-masked]>
Date: Sat, 16 Mar 2019 03:26:58 +0100

Hi Petr

> +enum rtnl_link_down_reason_major {
> +	RTNL_LDR_OTHER,

Does 'other' make any sense? Seem better to just not report anything
at all, or add a comment that more reasons should be added at the end
to reflect whatever the hardware or software can determine.

> +	RTNL_LDR_NO_CABLE,
> +	RTNL_LDR_UNSUPPORTED_CABLE,
> +	RTNL_LDR_AUTONEG_FAILURE,
> +	RTNL_LDR_NO_LINK_PARTNER,
> +	RTNL_LDR_LINK_TRAINING_FAILURE,
> +	RTNL_LDR_LOGICAL_MISMATCH,
> +	RTNL_LDR_REMOTE_FAULT,
> +	RTNL_LDR_BAD_SIGNAL_INTEGRITY,
> +	RTNL_LDR_CALIBRATION_FAILURE,
> +	RTNL_LDR_POWER_BUDGET_EXCEEDED,
> +};

What about SFP cage empty?, i.e. no SFP, SFP+ module in the cage?  An
SFP can also report LOS. That does not appear to be any of the above.
Or that the core SFP code has been unable to read the EEPROM? We have
people reporting this problem at the moment. We also have that the
SERDES has not yet obtained sync to its peer, which you know from
phylink_mac_change. That probably means the peer is using a different
bit rate.

I think it would be good if you handle the general case errors which
phylib and phylink can report, as well as the proprietary cases your
driver can report. We don't want this to be a Mellanox only API.

	Andrew