[Linuxptp-devel] [PATCH 3/3] Extend clockcheck to check for changes in frequency.

[Miroslav L. <mli...@re...>]

Before setting the new frequency offset on a clock update, compare the
current frequency with the value saved from the previous update and
print a warning message if they are different.

This should detect misconfigurations where multiple processes are trying
to control the clock (e.g. another ptp4l/phc2sys instance or an NTP
client), even when they don't step the clock.

Signed-off-by: Miroslav Lichvar <mli...@re...>
---
 clock.c      | 10 ++++++++--
 clockcheck.c |  9 +++++++++
 clockcheck.h |  8 ++++++++
 phc2sys.c    |  6 +++++-
 ptp4l.8      |  4 +++-
 5 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/clock.c b/clock.c
index 46ac9c2..69e803b 100644
--- a/clock.c
+++ b/clock.c
@@ -1776,12 +1776,17 @@ static void clock_step_window(struct clock *c)
 
 static void clock_synchronize_locked(struct clock *c, double adj)
 {
+	if (c->sanity_check) {
+		clockcheck_freq(c->sanity_check,
+				clockadj_get_freq(c->clkid));
+	}
 	clockadj_set_freq(c->clkid, -adj);
 	if (c->clkid == CLOCK_REALTIME) {
 		sysclk_set_sync();
 	}
 	if (c->sanity_check) {
-		clockcheck_set_freq(c->sanity_check, -adj);
+		clockcheck_set_freq(c->sanity_check,
+				    clockadj_get_freq(c->clkid));
 	}
 }
 
@@ -1838,7 +1843,8 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t ingress, tmv_t origin)
 		clockadj_step(c->clkid, -tmv_to_nanoseconds(c->master_offset));
 		c->ingress_ts = tmv_zero();
 		if (c->sanity_check) {
-			clockcheck_set_freq(c->sanity_check, -adj);
+			clockcheck_set_freq(c->sanity_check,
+					    clockadj_get_freq(c->clkid));
 			clockcheck_step(c->sanity_check,
 					-tmv_to_nanoseconds(c->master_offset));
 		}
diff --git a/clockcheck.c b/clockcheck.c
index f0141be..c174e73 100644
--- a/clockcheck.c
+++ b/clockcheck.c
@@ -123,6 +123,15 @@ void clockcheck_set_freq(struct clockcheck *cc, int freq)
 	cc->freq_known = 1;
 }
 
+int clockcheck_freq(struct clockcheck *cc, int freq)
+{
+	if (cc->freq_known && cc->current_freq != freq) {
+		pr_warning("clockcheck: clock frequency changed unexpectedly!");
+		return 1;
+	}
+	return 0;
+}
+
 void clockcheck_step(struct clockcheck *cc, int64_t step)
 {
 	if (cc->last_ts)
diff --git a/clockcheck.h b/clockcheck.h
index 1ff86eb..4b09b98 100644
--- a/clockcheck.h
+++ b/clockcheck.h
@@ -54,6 +54,14 @@ int clockcheck_sample(struct clockcheck *cc, uint64_t ts);
  */
 void clockcheck_set_freq(struct clockcheck *cc, int freq);
 
+/**
+ * Check whether the frequency correction did not change unexpectedly.
+ * @param cc   Pointer to a clock check obtained via @ref clockcheck_create().
+ * @param freq Current reading of the frequency correction in ppb.
+ * @return Zero if the frequency did not change, non-zero otherwise.
+ */
+int clockcheck_freq(struct clockcheck *cc, int freq);
+
 /**
  * Inform clock check that the clock was stepped.
  * @param cc   Pointer to a clock check obtained via @ref clockcheck_create().
diff --git a/phc2sys.c b/phc2sys.c
index ebc43e5..afe33ec 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -561,11 +561,15 @@ static void update_clock(struct phc2sys_private *priv, struct clock *clock,
 		/* Fall through. */
 	case SERVO_LOCKED:
 	case SERVO_LOCKED_STABLE:
+		if (clock->sanity_check)
+			clockcheck_freq(clock->sanity_check,
+					clockadj_get_freq(clock->clkid));
 		clockadj_set_freq(clock->clkid, -ppb);
 		if (clock->clkid == CLOCK_REALTIME)
 			sysclk_set_sync();
 		if (clock->sanity_check)
-			clockcheck_set_freq(clock->sanity_check, -ppb);
+			clockcheck_set_freq(clock->sanity_check,
+					    clockadj_get_freq(clock->clkid));
 		break;
 	}
 
diff --git a/ptp4l.8 b/ptp4l.8
index 1268802..220d594 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -619,7 +619,9 @@ This option used to be called
 The maximum allowed frequency offset between uncorrected clock and the system
 monotonic clock in parts per billion (ppb). This is used as a sanity check of
 the synchronized clock. When a larger offset is measured, a warning message
-will be printed and the servo will be reset. When set to 0, the sanity check is
+will be printed and the servo will be reset. If the frequency correction set by
+ptp4l changes unexpectedly (e.g. due to another process trying to control the
+clock), a warning message will be printed. When set to 0, the sanity check is
 disabled. The default is 200000000 (20%).
 .TP
 .B initial_delay
-- 
2.37.3