RE: [PATCH net-next 4/6] rtnetlink: Add support for SyncE recovered clock configuration

-----Original Message-----
From: Ido Schimmel <redacted>
Sent: Sunday, November 7, 2021 3:21 PM
To: Machnikowski, Maciej <redacted>
Subject: Re: [PATCH net-next 4/6] rtnetlink: Add support for SyncE recovered
clock configuration

On Fri, Nov 05, 2021 at 12:17:19PM +0000, Machnikowski, Maciej wrote:

quoted

quoted

-----Original Message-----
From: Roopa Prabhu <redacted>
Sent: Thursday, November 4, 2021 7:25 PM
To: Machnikowski, Maciej <redacted>;
netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
Cc: richardcochran@gmail.com; abyagowi@fb.com; Nguyen, Anthony L
[off-list ref]; davem@davemloft.net;

kuba@kernel.org;

quoted

quoted

linux-kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz;
saeed@kernel.org; michael.chan@broadcom.com
Subject: Re: [PATCH net-next 4/6] rtnetlink: Add support for SyncE

recovered

quoted

quoted

clock configuration


On 11/4/21 1:12 AM, Maciej Machnikowski wrote:

quoted

Add support for RTNL messages for reading/configuring SyncE

recovered

quoted

quoted

quoted

clocks.
The messages are:
RTM_GETRCLKRANGE: Reads the allowed pin index range for the

recovered

quoted

		  clock outputs. This can be aligned to PHY outputs or
		  to EEC inputs, whichever is better for a given
		  application

RTM_GETRCLKSTATE: Read the state of recovered pins that output

recovered

quoted

		  clock from a given port. The message will contain the
		  number of assigned clocks (IFLA_RCLK_STATE_COUNT) and
		  a N pin inexes in IFLA_RCLK_STATE_OUT_IDX

RTM_SETRCLKSTATE: Sets the redirection of the recovered clock for
		  a given pin

Signed-off-by: Maciej Machnikowski

[off-list ref]

quoted

quoted

quoted

---

Can't we just use a single RTM msg with nested attributes ?

With separate RTM msgtype for each syncE attribute we will end up
bloating the RTM msg namespace.

(these api's could also be in ethtool given its directly querying the
drivers)

I'm not a fan of merging those messages. The mergeable ones are
GETRCLKRANGE and GETCLKSTATE, but the get range function may be
result in a significantly longer call if the information about the underlying
HW require any HW calls.
They are currently grouped in 3 categories:
- reading the boundaries in GetRclkRange (we can later add more to it, like
	configurable frequency limits)
- Reading current configuration
- setting the required configuration

I don't plan adding any additional RTM msg types for those (and that's
the reason why I pulled them before EEC state which may have more
messages in the future)

We also discussed ethtool way in the past RFCs, but this concept
is applicable to any transport layer so I'd rather not limit it to ethernet
devices (i.e. SONET, infiniband and others).

I'm still not convinced that this doesn't belong in ethtool. I find it
very weird to have a message called "Get Ethernet Equipment Clock State"
in rtnetlink and not in ethtool. I also believe it was a mistake to add
DCB to rtnetlink (for example, why PAUSE is configured via ethtool, but
PFC via rtnetlink?)

We can use:
- SEC - Synchronous Equipment Clock
- EC - Equipment Clock

SyncE is a specific implementation of a more generic concept. I'd rather not limit
it to Ethernet only, as there are more network types that already use this concept,
like Sonet/SDH or PDH as well as GPON/EPON networks and given the recent
growth in timing applications - I expect more to follow.