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

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: <20190316022658.GC29548@lunn.ch>
Date:   Sat, 16 Mar 2019 03:26:58 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Petr Machata <petrm@...lanox.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Tariq Toukan <tariqt@...lanox.com>,
        "jakub.kicinski@...ronome.com" <jakub.kicinski@...ronome.com>,
        "stephen@...workplumber.org" <stephen@...workplumber.org>
Subject: Re: [RFC PATCH net-next 1/3] net: rtnetlink: Add link-down reason to
 RTNL messages

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