mbox series

[RFC,v5,net-next,0/5] Add RTNL interface for SyncE

Message ID 20211026173146.1031412-1-maciej.machnikowski@intel.com (mailing list archive)
Headers show
Series Add RTNL interface for SyncE | expand

Message

Machnikowski, Maciej Oct. 26, 2021, 5:31 p.m. UTC
Synchronous Ethernet networks use a physical layer clock to syntonize
the frequency across different network elements.

Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet
Equipment Clock (EEC) and have the ability to recover synchronization
from the synchronization inputs - either traffic interfaces or external
frequency sources.
The EEC can synchronize its frequency (syntonize) to any of those sources.
It is also able to select synchronization source through priority tables
and synchronization status messaging. It also provides neccessary
filtering and holdover capabilities

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 source of the syntonization signal (ether my port,
or any external one) and the state of EEC. This interface is required\
to implement Synchronization Status Messaging on upper layers.

v2:
- removed whitespace changes
- fix issues reported by test robot
v3:
- Changed naming from SyncE to EEC
- Clarify cover letter and commit message for patch 1
v4:
- Removed sync_source and pin_idx info
- Changed one structure to attributes
- Added EEC_SRC_PORT flag to indicate that the EEC is synchronized
  to the recovered clock of a port that returns the state
v5:
- add EEC source as an optiona attribute
- implement support for recovered clocks
- align states returned by EEC to ITU-T G.781

Maciej Machnikowski (5):
  ice: add support detecting features based on netlist
  rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status
  ice: add support for reading SyncE DPLL state
  rtnetlink: Add support for SyncE recovered clock configuration
  ice: add support for SyncE recovered clocks

 drivers/net/ethernet/intel/ice/ice.h          |   7 +
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  94 ++++++-
 drivers/net/ethernet/intel/ice/ice_common.c   | 175 ++++++++++++
 drivers/net/ethernet/intel/ice/ice_common.h   |  17 +-
 drivers/net/ethernet/intel/ice/ice_devids.h   |   3 +
 drivers/net/ethernet/intel/ice/ice_lib.c      |   6 +-
 drivers/net/ethernet/intel/ice/ice_main.c     | 138 ++++++++++
 drivers/net/ethernet/intel/ice/ice_ptp.c      |  34 +++
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c   |  94 +++++++
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h   |  25 ++
 drivers/net/ethernet/intel/ice/ice_type.h     |   1 +
 include/linux/netdevice.h                     |  18 ++
 include/uapi/linux/if_link.h                  |  53 ++++
 include/uapi/linux/rtnetlink.h                |  10 +
 net/core/rtnetlink.c                          | 253 ++++++++++++++++++
 security/selinux/nlmsgtab.c                   |   6 +-
 16 files changed, 930 insertions(+), 4 deletions(-)

Comments

Ido Schimmel Oct. 27, 2021, 6:50 a.m. UTC | #1
On Tue, Oct 26, 2021 at 07:31:41PM +0200, Maciej Machnikowski wrote:
> Synchronous Ethernet networks use a physical layer clock to syntonize
> the frequency across different network elements.
> 
> Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet
> Equipment Clock (EEC) and have the ability to recover synchronization
> from the synchronization inputs - either traffic interfaces or external
> frequency sources.
> The EEC can synchronize its frequency (syntonize) to any of those sources.
> It is also able to select synchronization source through priority tables
> and synchronization status messaging. It also provides neccessary
> filtering and holdover capabilities
> 
> 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 source of the syntonization signal (ether my port,
> or any external one) and the state of EEC. This interface is required\
> to implement Synchronization Status Messaging on upper layers.
> 
> v2:
> - removed whitespace changes
> - fix issues reported by test robot
> v3:
> - Changed naming from SyncE to EEC
> - Clarify cover letter and commit message for patch 1
> v4:
> - Removed sync_source and pin_idx info
> - Changed one structure to attributes
> - Added EEC_SRC_PORT flag to indicate that the EEC is synchronized
>   to the recovered clock of a port that returns the state
> v5:
> - add EEC source as an optiona attribute
> - implement support for recovered clocks
> - align states returned by EEC to ITU-T G.781

Hi,

Thanks for continuing to work on this.

I was under the impression (might be wrong) that the consensus last time
was to add a new ethtool message to query the mapping between the port
and the EEC clock (similar to TSINFO_GET) and then use a new generic
netlink family to perform operations on the clock itself.

At least in the case of RTM_GETEECSTATE and a multi-port adapter, you
would actually query the same state via each netdev, but without
realizing it's the same clock.

I think another reason to move to ethtool was that this stuff is
completely specific to Ethernet and not applicable to all logical
netdevs.
Machnikowski, Maciej Oct. 27, 2021, 1:21 p.m. UTC | #2
> -----Original Message-----
> From: Ido Schimmel <idosch@idosch.org>
> Sent: Wednesday, October 27, 2021 8:50 AM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org;
> richardcochran@gmail.com; abyagowi@fb.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net; kuba@kernel.org;
> linux-kselftest@vger.kernel.org; mkubecek@suse.cz; saeed@kernel.org;
> michael.chan@broadcom.com
> Subject: Re: [RFC v5 net-next 0/5] Add RTNL interface for SyncE
> 
> On Tue, Oct 26, 2021 at 07:31:41PM +0200, Maciej Machnikowski wrote:
> > Synchronous Ethernet networks use a physical layer clock to syntonize
> > the frequency across different network elements.
> >
> > Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet
> > Equipment Clock (EEC) and have the ability to recover synchronization
> > from the synchronization inputs - either traffic interfaces or external
> > frequency sources.
> > The EEC can synchronize its frequency (syntonize) to any of those sources.
> > It is also able to select synchronization source through priority tables
> > and synchronization status messaging. It also provides neccessary
> > filtering and holdover capabilities
> >
> > 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 source of the syntonization signal (ether my port,
> > or any external one) and the state of EEC. This interface is required\
> > to implement Synchronization Status Messaging on upper layers.
> >
> > v2:
> > - removed whitespace changes
> > - fix issues reported by test robot
> > v3:
> > - Changed naming from SyncE to EEC
> > - Clarify cover letter and commit message for patch 1
> > v4:
> > - Removed sync_source and pin_idx info
> > - Changed one structure to attributes
> > - Added EEC_SRC_PORT flag to indicate that the EEC is synchronized
> >   to the recovered clock of a port that returns the state
> > v5:
> > - add EEC source as an optiona attribute
> > - implement support for recovered clocks
> > - align states returned by EEC to ITU-T G.781
> 
> Hi,
> 
> Thanks for continuing to work on this.
> 
> I was under the impression (might be wrong) that the consensus last time
> was to add a new ethtool message to query the mapping between the port
> and the EEC clock (similar to TSINFO_GET) and then use a new generic
> netlink family to perform operations on the clock itself.

Hi!

I believe we finally agreed to continue with this implementations (for a
simplified devices) and when the DPLL subsystem is ready, plug it into this
API as well using the discovery mechanism. As there may be some simplified
solutions that would not use the controllable DPLL and only provide the
status (i.e. using physical signals)

> At least in the case of RTM_GETEECSTATE and a multi-port adapter, you
> would actually query the same state via each netdev, but without
> realizing it's the same clock.

True, yet for a given port we need info whether we are locked or not,
so the interdependency wouldn't break anything.
 
> I think another reason to move to ethtool was that this stuff is
> completely specific to Ethernet and not applicable to all logical
> netdevs.

That was an open in previous discussion. Wanted to first show the
full API to discuss where it fits. I believe all other networks (like SONET)
may also benefit from having it in the netdev, but am open for discussion.

Regards
Maciek
Ido Schimmel Oct. 27, 2021, 3:05 p.m. UTC | #3
On Wed, Oct 27, 2021 at 01:21:58PM +0000, Machnikowski, Maciej wrote:
> 
> 
> > -----Original Message-----
> > From: Ido Schimmel <idosch@idosch.org>
> > Sent: Wednesday, October 27, 2021 8:50 AM
> > To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> > Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org;
> > richardcochran@gmail.com; abyagowi@fb.com; Nguyen, Anthony L
> > <anthony.l.nguyen@intel.com>; davem@davemloft.net; kuba@kernel.org;
> > linux-kselftest@vger.kernel.org; mkubecek@suse.cz; saeed@kernel.org;
> > michael.chan@broadcom.com
> > Subject: Re: [RFC v5 net-next 0/5] Add RTNL interface for SyncE
> > 
> > On Tue, Oct 26, 2021 at 07:31:41PM +0200, Maciej Machnikowski wrote:
> > > Synchronous Ethernet networks use a physical layer clock to syntonize
> > > the frequency across different network elements.
> > >
> > > Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet
> > > Equipment Clock (EEC) and have the ability to recover synchronization
> > > from the synchronization inputs - either traffic interfaces or external
> > > frequency sources.
> > > The EEC can synchronize its frequency (syntonize) to any of those sources.
> > > It is also able to select synchronization source through priority tables
> > > and synchronization status messaging. It also provides neccessary
> > > filtering and holdover capabilities
> > >
> > > 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 source of the syntonization signal (ether my port,
> > > or any external one) and the state of EEC. This interface is required\
> > > to implement Synchronization Status Messaging on upper layers.
> > >
> > > v2:
> > > - removed whitespace changes
> > > - fix issues reported by test robot
> > > v3:
> > > - Changed naming from SyncE to EEC
> > > - Clarify cover letter and commit message for patch 1
> > > v4:
> > > - Removed sync_source and pin_idx info
> > > - Changed one structure to attributes
> > > - Added EEC_SRC_PORT flag to indicate that the EEC is synchronized
> > >   to the recovered clock of a port that returns the state
> > > v5:
> > > - add EEC source as an optiona attribute
> > > - implement support for recovered clocks
> > > - align states returned by EEC to ITU-T G.781
> > 
> > Hi,
> > 
> > Thanks for continuing to work on this.
> > 
> > I was under the impression (might be wrong) that the consensus last time
> > was to add a new ethtool message to query the mapping between the port
> > and the EEC clock (similar to TSINFO_GET) and then use a new generic
> > netlink family to perform operations on the clock itself.
> 
> Hi!
> 
> I believe we finally agreed to continue with this implementations (for a
> simplified devices) and when the DPLL subsystem is ready, plug it into this
> API as well using the discovery mechanism. As there may be some simplified
> solutions that would not use the controllable DPLL and only provide the
> status (i.e. using physical signals)

By "simplified solutions" you are referring to simple NEs that only
synchronize their frequency according to what they extract from the
physical layer as opposed to an external source such as in the PRC case?

> 
> > At least in the case of RTM_GETEECSTATE and a multi-port adapter, you
> > would actually query the same state via each netdev, but without
> > realizing it's the same clock.
> 
> True, yet for a given port we need info whether we are locked or not,
> so the interdependency wouldn't break anything.

But if two ports are using the same EEC, then it's not possible for them
to report a different EEC state, right? The only difference is that one
port can be the source and the other isn't, but this is also an
attribute of the EEC.

Basically, I'm saying that it seems that we report attributes of a
single object (the EEC) via multiple objects using it (the netdevs)
without making the relation clear to user space.

I'm not strictly against it, but rather wondering why.

>  
> > I think another reason to move to ethtool was that this stuff is
> > completely specific to Ethernet and not applicable to all logical
> > netdevs.
> 
> That was an open in previous discussion. Wanted to first show the
> full API to discuss where it fits. I believe all other networks (like SONET)
> may also benefit from having it in the netdev, but am open for discussion.

OK, didn't think about SONET. There is include/uapi/linux/sonet.h with a
few SONET specific ioctls and a couple of drivers implementing them. The
whole thing looks quite dead...

ethtool still seems like a better option for something that has
"Ethernet" in its name :)
Machnikowski, Maciej Oct. 28, 2021, 6:32 a.m. UTC | #4
> -----Original Message-----
> From: Ido Schimmel <idosch@idosch.org>
> Sent: Wednesday, October 27, 2021 5:05 PM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Subject: Re: [RFC v5 net-next 0/5] Add RTNL interface for SyncE
> 
> On Wed, Oct 27, 2021 at 01:21:58PM +0000, Machnikowski, Maciej wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ido Schimmel <idosch@idosch.org>
> > > Sent: Wednesday, October 27, 2021 8:50 AM
> > > To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> > > Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org;
> > > richardcochran@gmail.com; abyagowi@fb.com; Nguyen, Anthony L
> > > <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> kuba@kernel.org;
> > > linux-kselftest@vger.kernel.org; mkubecek@suse.cz; saeed@kernel.org;
> > > michael.chan@broadcom.com
> > > Subject: Re: [RFC v5 net-next 0/5] Add RTNL interface for SyncE
> > >
> > > On Tue, Oct 26, 2021 at 07:31:41PM +0200, Maciej Machnikowski wrote:
> > > > Synchronous Ethernet networks use a physical layer clock to syntonize
> > > > the frequency across different network elements.
> > > >
> > > > Basic SyncE node defined in the ITU-T G.8264 consist of an Ethernet
> > > > Equipment Clock (EEC) and have the ability to recover synchronization
> > > > from the synchronization inputs - either traffic interfaces or external
> > > > frequency sources.
> > > > The EEC can synchronize its frequency (syntonize) to any of those
> sources.
> > > > It is also able to select synchronization source through priority tables
> > > > and synchronization status messaging. It also provides neccessary
> > > > filtering and holdover capabilities
> > > >
> > > > 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 source of the syntonization signal (ether my
> port,
> > > > or any external one) and the state of EEC. This interface is required\
> > > > to implement Synchronization Status Messaging on upper layers.
> > > >
> > > > v2:
> > > > - removed whitespace changes
> > > > - fix issues reported by test robot
> > > > v3:
> > > > - Changed naming from SyncE to EEC
> > > > - Clarify cover letter and commit message for patch 1
> > > > v4:
> > > > - Removed sync_source and pin_idx info
> > > > - Changed one structure to attributes
> > > > - Added EEC_SRC_PORT flag to indicate that the EEC is synchronized
> > > >   to the recovered clock of a port that returns the state
> > > > v5:
> > > > - add EEC source as an optiona attribute
> > > > - implement support for recovered clocks
> > > > - align states returned by EEC to ITU-T G.781
> > >
> > > Hi,
> > >
> > > Thanks for continuing to work on this.
> > >
> > > I was under the impression (might be wrong) that the consensus last time
> > > was to add a new ethtool message to query the mapping between the
> port
> > > and the EEC clock (similar to TSINFO_GET) and then use a new generic
> > > netlink family to perform operations on the clock itself.
> >
> > Hi!
> >
> > I believe we finally agreed to continue with this implementations (for a
> > simplified devices) and when the DPLL subsystem is ready, plug it into this
> > API as well using the discovery mechanism. As there may be some
> simplified
> > solutions that would not use the controllable DPLL and only provide the
> > status (i.e. using physical signals)
> 
> By "simplified solutions" you are referring to simple NEs that only
> synchronize their frequency according to what they extract from the
> physical layer as opposed to an external source such as in the PRC case?
> 
> >
> > > At least in the case of RTM_GETEECSTATE and a multi-port adapter, you
> > > would actually query the same state via each netdev, but without
> > > realizing it's the same clock.
> >
> > True, yet for a given port we need info whether we are locked or not,
> > so the interdependency wouldn't break anything.
> 
> But if two ports are using the same EEC, then it's not possible for them
> to report a different EEC state, right? The only difference is that one
> port can be the source and the other isn't, but this is also an
> attribute of the EEC.
> 
> Basically, I'm saying that it seems that we report attributes of a
> single object (the EEC) via multiple objects using it (the netdevs)
> without making the relation clear to user space.
> 
> I'm not strictly against it, but rather wondering why.

Agreed. The reason to start with this approach is that it will be applicable
to some  devices in the future and we can easily fix that mapping later by 
adding EEC/DPLL ID to the attributes.

> >
> > > I think another reason to move to ethtool was that this stuff is
> > > completely specific to Ethernet and not applicable to all logical
> > > netdevs.
> >
> > That was an open in previous discussion. Wanted to first show the
> > full API to discuss where it fits. I believe all other networks (like SONET)
> > may also benefit from having it in the netdev, but am open for discussion.
> 
> OK, didn't think about SONET. There is include/uapi/linux/sonet.h with a
> few SONET specific ioctls and a couple of drivers implementing them. The
> whole thing looks quite dead...
> 
> ethtool still seems like a better option for something that has
> "Ethernet" in its name :)

For now we add it for SyncE, which indeed has the Ethernet in its name,
but I think the concept of recovering clock from the data stream is
common and applicable to other transport layers. I believe the same
API can be used in infiniband, fiber channel, xPON if they implement
PHYs supporting this capability. Hence if it's not a strong NO for putting
it in netdevs I'd rather keep it there to avoid challenges we found with
timing cards where we had to stretch the concept of Ethernet device
to use the APIs defined in the ethtool :)
Also the ITU-T recommendations G8261 and G.8262 are more generic
than just SyncE and they are applicable to any packet network.

Regards
Maciek