RE: [PATCH net-next 1/2] rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status

-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: Saturday, September 4, 2021 12:14 AM
Subject: Re: [PATCH net-next 1/2] rtnetlink: Add new RTM_GETEECSTATE
message to get SyncE status

On Fri,  3 Sep 2021 17:14:35 +0200 Maciej Machnikowski wrote:

quoted

This patch series introduces basic interface for reading the Ethernet
Equipment Clock (EEC) state on a SyncE capable device. This state gives
information about the state of EEC. This interface is required to
implement Synchronization Status Messaging on upper layers.

Initial implementation returns SyncE EEC state and flags attributes.
The only flag currently implemented is the EEC_SRC_PORT. When it's set
the EEC is synchronized to the recovered clock recovered from the
current port.

SyncE EEC state read needs to be implemented as a ndo_get_eec_state
function.

Signed-off-by: Maciej Machnikowski <redacted>

Since we're talking SyncE-only now my intuition would be to put this
op in ethtool. Was there a reason ethtool was not chosen? If not what
do others think / if yes can the reason be included in the commit
message?

Hmm. Main reason for netlink is that linuxptp already supports it,
and it was suggested by Richard.
Having an NDO would also make it easier to add a SyncE-related
files to the sysfs for easier operation (following the ideas from the ptp
pins subsystem).
But I'm open for suggestions. 

quoted

+#define EEC_SRC_PORT		(1 << 0) /* recovered clock from the

port is

quoted

+					  * currently the source for the EEC
+					  */

Why include it then? Just leave the value out and if the attr is not
present user space should assume the source is port.

This bit has a different meaning. If it's set the port in question
is a frequency source for the multiport device, if it's cleared - some other
source is used as a source. This is needed to prevent setting invalid 
configurations in the PHY (like setting the frequency source as a Master
in AN) or sending invalid messages. If the port is a frequency source
it must always send back QL-DNU messages to prevent synchronization
loops.

or don't check the ifindex at all and let dev_get_by.. fail.


Thanks for pushing this API forward!

Addressed all other comments - and thanks for giving a lot of helpful
suggestions!

Regards
Maciek