xilinx-cns / sfptpd Goto Github PK

The clustering feature which gives a score to remote clock sources based on their time offsets currently has one implementation, the 'discriminator' mode. The 'discriminator' mode filters out sources which are significantly off from a source which is trusted to provide a good time, even if it is not provided to a very high accuracy, e.g. NTP.

Where there are three or more high quality (e.g. PTP or PPS) sources available then a preferable form of clustering may be a 'voting' algorithm allowing the sources which diverge significantly from the majority of sources to be filtered out.

The task here is to add a 'voting' type of clustering which can be configured to this end. As with the discriminator, which is binary, there is likely to be some quantization needed in this determination so that the other selection criteria continue to be used effectively.

Implementation of this feature would improve the 'Enterprise Profile' offering in sfptpd.

The clustering calculation is currently performed on behalf of the sync instances by a callback in sfptpd_engine.c and defined in the global configuration sfptpd_general_config.c. It is possible that a future change will simplify these mechanics to avoid the callback and determine the score solely in the engine but this should not affect the implementation of this proposed enhancement.

32-bit builds fail with errors like:

    src/sfptpd_statistics.c: In function ‘stats_range_history_write_data’:
    src/sfptpd_statistics.c:518:62: error: passing argument 4 of ‘sfptpd_local_strftime’ from incompatible pointer type [-Werror=incompatible-pointer-types]
      518 |                                               "%Y-%m-%d %X", &entry->min_time.sec);
          |                                                              ^~~~~~~~~~~~~~~~~~~~
          |                                                              |
          |                                                              int64_t * {aka long long int *}
    In file included from src/include/sfptpd_logging.h:16,
                     from src/sfptpd_statistics.c:20:
    src/include/sfptpd_misc.h:144:83: note: expected ‘const time_t *’ {aka ‘const long int *’} but argument is of type ‘int64_t *’ {aka ‘long long int *’}
      144 | void sfptpd_local_strftime(char *s, size_t max, const char *format, const time_t *timep);
          |                                                                     ~~~~~~~~~~~~~~^~~~~

On Raspberry Pi 5, the Ethernet interface is correctly enumerated and identified as having an associated PHC device but a corresponding 'clock' object does not get created.

The effect of this is confusion and an inability to use the NIC clock.

This was not previously a problem in testing with a CM4 but it is unconfirmed whether this is a regression or not.

Implement the 'Path trace' optional feature of IEEE1588-2019 section 16.2. This feature has two parts, sending and receiving. Either could be implemented independently but it would be desirable to present the two parts together to aid testability and review.

Option to append PATH_TRACE TLV to outgoing Announce messages

Implement the 'Path trace' optional feature of IEEE1588-2019 section 16.2 by appending PATH_TRACE TLVs to outgoing Announce messages, controlled via an instance-scope configuration option to the PTP sync module.

The PORT_COMMUNICATION_AVAILABILITY TLV provides an example of how to add an optional TLV to outgoing announce messages:

• Identifying the TLV:

  sfptpd/src/ptp/ptpd2/constants.h

  Line 335 in 3a4113b

  PTPD_TLV_PATH_TRACE=0x0008,

• Defining the TLV: https://github.com/Xilinx-CNS/sfptpd/blob/master/src/ptp/ptpd2/def/optional/port_communication_capabilities.def
• Packing functions for for TLV:

  sfptpd/src/ptp/ptpd2/dep/msg.c

  Lines 2463 to 2493 in 3a4113b

  	appendPortCommunicationCapabilitiesTLV(PortCommunicationCapabilities *data, Octet *buf, size_t space)
  	{
  	ssize_t result = PACK_INIT;
  	int tlv_start = getHeaderLength(buf);
  	int offset = tlv_start;
  	int base;
  	TLV tlv = { .tlvType = PTPD_TLV_PORT_COMMUNICATION_CAPABILITIES };
  	TLV_BOUNDARY_CHECK(offset, space);
  	result = msgPackTLVHeader(buf + tlv_start, space - tlv_start, &tlv);
  	if (!PACK_OK(result)) goto finish;
  	offset += result;
  	base = offset;
  	#define OPERATE( name, size, type ) STANDARD_PACKING_OPERATION(name, size, type)
  	#include "../def/optional/port_communication_capabilities.def"
  	PAD_TO_EVEN_LENGTH(buf, offset, space, result, finish);
  	/* Set TLV length */
  	setHeaderLength(buf + tlv_start, offset - base);
  	/* Set message length */
  	setHeaderLength(buf, offset);
  	result = PACK_SIZE(offset);
  	finish:
  	/* return length */
  	return result;
  	}

• sfptpd/src/ptp/ptpd2/protocol.c

  Lines 3235 to 3241 in 3a4113b

  	/* @task71885: append multicast/unicast capability information */
  	if (rtOpts->comm_caps_tlv_enabled) {
  	pack_result = appendPortCommunicationCapabilitiesTLV(&rtOpts->comm_caps,
  	ptpClock->msgObuf,
  	sizeof ptpClock->msgObuf);
  	assert(PACK_OK(pack_result));
  	}

Handle received PATH_TRACE TLVs for current parent.

Example for port communication capabilities:

• Handling the TLV:

  sfptpd/src/ptp/ptpd2/protocol.c

  Lines 192 to 198 in 3a4113b

  	{
  	.tlv_type = PTPD_TLV_PORT_COMMUNICATION_CAPABILITIES,
  	.name = "PORT_COMMUNICATION_CAPABILITIES",
  	.permitted_message_types_mask = 1 << PTPD_MSG_ANNOUNCE,
  	.required_acl_types_mask = 0,
  	.pass1_handler_fn = port_communication_capabilities_handler,
  	.pass2_handler_fn = NULL,

• Processing the TLV:

  sfptpd/src/ptp/ptpd2/protocol.c

  Lines 3184 to 3219 in 3a4113b

  	static enum ptpd_tlv_result
  	port_communication_capabilities_handler(const MsgHeader *header, ssize_t length,
  	TimeInternal *time, Boolean timestampValid, Boolean isFromSelf,
  	RunTimeOpts *rtOpts, PtpClock *ptpClock,
  	TLV *tlv, size_t tlv_offset)
  	{
  	ssize_t result;
  	DBGV("PORT_COMMUNICATION_CAPABILITIES received : \n");
  	if (isFromSelf) {
  	/* Ignore */
  	return PTPD_TLV_RESULT_CONTINUE;
  	}
  	if (rtOpts->ptp_version_minor < 1) {
  	DBG2("ignore COMMUNICATION_CAPABILITIES TLV in version %d.%d mode\n",
  	PTPD_PROTOCOL_VERSION,
  	rtOpts->ptp_version_minor);
  	ptpClock->counters.discardedMessages++;
  	ptpClock->counters.versionMismatchErrors++;
  	return PTPD_TLV_RESULT_CONTINUE;
  	}
  	/* Save a pointer for the announce handler to use */
  	result = unpackPortCommunicationCapabilities(tlv->valueField,
  	tlv->lengthField,
  	&ptpClock->transient_packet_state.port_comm_caps,
  	ptpClock);
  	if (!UNPACK_OK(result))
  	return PTPD_TLV_RESULT_ERROR;
  	ptpClock->transient_packet_state.port_comm_caps_provided = true;
  	return PTPD_TLV_RESULT_CONTINUE;
  	}

• Saving the traced clock list:

  sfptpd/src/ptp/ptpd2/datatypes.h

  Lines 83 to 84 in 3a4113b

  	/* The announce multicast/unicast capabilities */
  	PortCommunicationCapabilities comm_caps;

  - these are the foreign master records. It is a trade off between space for storing for all foreign masters and accessing later via FMR for selected master or a more complex solution. I suggest doing the former.
• Expand the topology where the clock list for parent nodes is available:

  sfptpd/src/ptp/sfptpd_ptp_module.c

  Lines 2762 to 2789 in 3a4113b

  	sfptpd_log_topology_write_field(stream, true, "grandmaster");
  	if (boundary) {
  	sfptpd_log_topology_write_field(stream, true,
  	SFPTPD_FORMAT_EUI64,
  	gm[0], gm[1], gm[2], gm[3],
  	gm[4], gm[5], gm[6], gm[7]);
  	sfptpd_log_topology_write_1to1_connector(stream, false, true,
  	steps_format, steps_removed);
  	sfptpd_log_topology_write_field(stream, true, "parent");
  	}
  	sfptpd_log_topology_write_field(stream, true,
  	SFPTPD_FORMAT_EUI64 "/%d",
  	p[0], p[1], p[2], p[3],
  	p[4], p[5], p[6], p[7],
  	instance->ptpd_port_snapshot.parent.port_num);
  	if (instance->ptpd_port_snapshot.current.transparent_clock) {
  	sfptpd_log_topology_write_1to1_connector(stream, false, true, NULL);
  	sfptpd_log_topology_write_field(stream, true, "transparent");
  	sfptpd_log_topology_write_field(stream, true, "clock");
  	}
  	if (state == PTPD_PASSIVE) {
  	sfptpd_log_topology_write_1to1_connector(stream, false, true, "zzz");
  	} else {
  	sfptpd_log_topology_write_1to1_connector(stream, false, true,
  	SFPTPD_FORMAT_TOPOLOGY_FLOAT,
  	ofm_ns);
  	}
  	break;

  Other than the configuration, this is the only part of the solution which is outside of the ptpd2 fork. The coding standard/convention is slightly different here. The native sfptpd and vendored ptpd2 parts of the application are broadly linked via the ptpd_lib.c and ptpd_lib.h files. The configuration and state objects are incorporated into the sfptpd_ptp_module.c instance state but preferably accessed via accessors in ptpd_lib.

This is a packaging issue; sorry if this isn't the correct place. For many years and many versions of sfptpd, I have "debianized" the RPM distribution through alien command.

I understand Debian is unsupported, but the result here indicatives a minor RPM package issue.

$ fakeroot alien --to-deb --scripts sfptpd-3.6.0.1015-1.x86_64.rpm 
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.6.0.1015-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
sfptpd_3.6.0.1015-2_amd64.deb generated

$ dpkg -I sfptpd_3.6.0.1015-2_amd64.deb

However, with v3.7.0 currently on the release website here, I get the following:

$ fakeroot alien --to-deb --scripts sfptpd-3.7.0.1006-1.x86_64.rpm
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
warning: sfptpd-3.7.0.1006-1.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID ca969f38: NOKEY
Package build failed. Here's the log:
dh binary
   dh_update_autotools_config
   dh_autoreconf
   create-stamp debian/debhelper-build-stamp
   dh_testroot
   dh_prep
   debian/rules override_dh_auto_install
make[1]: Entering directory '/home/nm/evan/packages/solarflare/sfptpd-3.7.0.1006-1.x86_64.rpm/sfptpd-3.7.0.1006'
mkdir -p debian/sfptpd
# Copy the packages's files.
find . -maxdepth 1 -mindepth 1 -not -name debian -print0 | \
	sed -e s#'./'##g | \
	xargs -0 -r -i cp -a ./{} debian/sfptpd/{}
make[1]: Leaving directory '/home/nm/evan/packages/solarflare/sfptpd-3.7.0.1006-1.x86_64.rpm/sfptpd-3.7.0.1006'
   dh_installdocs
   dh_installchangelogs
   dh_installman
   dh_perl
   debian/rules override_dh_usrlocal
make[1]: Entering directory '/home/nm/evan/packages/solarflare/sfptpd-3.7.0.1006-1.x86_64.rpm/sfptpd-3.7.0.1006'
dh_usrlocal || printf "Your package seems to be installing files into /usr/local/, which could be buggy. Will continue anyway.\n"
make[1]: Leaving directory '/home/nm/evan/packages/solarflare/sfptpd-3.7.0.1006-1.x86_64.rpm/sfptpd-3.7.0.1006'
   dh_link
   dh_strip_nondeterminism
   dh_compress
   dh_missing
   debian/rules override_dh_strip
make[1]: Entering directory '/home/nm/evan/packages/solarflare/sfptpd-3.7.0.1006-1.x86_64.rpm/sfptpd-3.7.0.1006'
#	dh_strip
make[1]: Leaving directory '/home/nm/evan/packages/solarflare/sfptpd-3.7.0.1006-1.x86_64.rpm/sfptpd-3.7.0.1006'
   dh_makeshlibs
   debian/rules override_dh_shlibdeps
make[1]: Entering directory '/home/nm/evan/packages/solarflare/sfptpd-3.7.0.1006-1.x86_64.rpm/sfptpd-3.7.0.1006'
dh_shlibdeps
dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/sfptpd/usr/sbin/sfptpd debian/sfptpd/usr/sbin/sfptpdctl were not linked against libpthread.so.0 (they use none of the library's symbols)
dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/sfptpd/usr/sbin/sfptpd debian/sfptpd/usr/sbin/sfptpdctl were not linked against librt.so.1 (they use none of the library's symbols)
make[1]: Leaving directory '/home/nm/evan/packages/solarflare/sfptpd-3.7.0.1006-1.x86_64.rpm/sfptpd-3.7.0.1006'
   dh_installdeb
   dh_gencontrol
   dh_md5sums
   dh_builddeb
dpkg-deb: error: conffile '/var/lib/sfptpd/freq-correction-system' does not appear in package
dh_builddeb: error: dpkg-deb --build debian/sfptpd .. returned exit code 2
dh_builddeb: error: Aborting due to earlier error
make: *** [debian/rules:7: binary] Error 25

Most of it is probably warnings? But the notable part might be this line:

dpkg-deb: error: conffile '/var/lib/sfptpd/freq-correction-system' does not appear in package

I cracked open the RPM and it seems to be missing that file. I'm not sure if that would fix everything, but it is something.