> -----Original Message----- > From: Jakub Kicinski <kuba@xxxxxxxxxx> > 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: > > 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 <maciej.machnikowski@xxxxxxxxx> > > 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. > > > > +#define EEC_SRC_PORT (1 << 0) /* recovered clock from the > port is > > + * 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
