[Linuxptp-devel] [PATCH RFC 0/3] Fix UTC offset issue

[Richard Cochran]

This patch series addresses the jumping UTC offset issue that occurs
when the default TAI-UTC offset is out of date, but a then GM master
appears providing the correct offset, only to disappear again.

Review and comments are most welcome.

Thanks,
Richard

Richard Cochran (3):
Simplify UTC tracking.
Latch the UTC offset.
Remove redundant test on the UTC flags.

clock.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)

[Richard Cochran]

When acting as a slave, we accept the foreign master's advertised
TAI-UTC offset for as long as that master is present. However, when
the master disappears, we fall back to the initial offset value. As a
result of this behavior, when starting with an out of date initial
offset, losing a foreign master causes a sudden UTC offset error (in
phc2sys for example) in the range of whole seconds.

This patch fixes the issue by remembering the UTC offset when assuming
the slave role.

Signed-off-by: Richard Cochran <***@gmail.com>
---
clock.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/clock.c b/clock.c
index aa1a5df..da1fcc4 100644
--- a/clock.c
+++ b/clock.c
@@ -658,6 +658,11 @@ static void clock_update_slave(struct clock *c)
if (c->tds.currentUtcOffset < c->utc_offset) {
pr_warning("running in a temporal vortex");
}
+ if ((c->tds.flags & UTC_OFF_VALID && c->tds.flags & TIME_TRACEABLE) ||
+ (c->tds.currentUtcOffset > c->utc_offset)) {
+ pr_info("updating UTC offset to %d", c->tds.currentUtcOffset);
+ c->utc_offset = c->tds.currentUtcOffset;
+ }
}

static int clock_utc_correct(struct clock *c, tmv_t ingress)

[Richard Cochran]

Now that we test the UTC flags in clock_update_slave(), the similar
code in clock_utc_correct() is redundant.

Signed-off-by: Richard Cochran <***@gmail.com>
---
clock.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/clock.c b/clock.c
index da1fcc4..9d224c9 100644
--- a/clock.c
+++ b/clock.c
@@ -674,13 +674,7 @@ static int clock_utc_correct(struct clock *c, tmv_t ingress)
if (!c->utc_timescale)
return 0;

- if (c->tds.flags & UTC_OFF_VALID && c->tds.flags & TIME_TRACEABLE) {
- utc_offset = c->tds.currentUtcOffset;
- } else if (c->tds.currentUtcOffset > c->utc_offset) {
- utc_offset = c->tds.currentUtcOffset;
- } else {
- utc_offset = c->utc_offset;
- }
+ utc_offset = c->utc_offset;

if (c->tds.flags & LEAP_61) {
leap = 1;

[Richard Cochran]

The clock code uses two different variables to store the TAI-UTC
offset. One variable represents the compile time, configuration file,
or command line initial UTC offset, while the other is used when
taking on the GM role and is settable at run time. However, making
this distinction makes no sense. This patch simplifies and clarifies
the code by using a single variable for the offset.

Signed-off-by: Richard Cochran <***@gmail.com>
---
clock.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/clock.c b/clock.c
index da15882..aa1a5df 100644
--- a/clock.c
+++ b/clock.c
@@ -101,8 +101,7 @@ struct clock {
int utc_offset_set;
int leap_set;
int kernel_leap;
- int utc_offset; /* grand master role */
- int current_utc_offset; /* UTC offset fallback */
+ int utc_offset;
int time_flags; /* grand master role */
int time_source; /* grand master role */
enum servo_state servo_state;
@@ -656,7 +655,7 @@ static void clock_update_slave(struct clock *c)
if (!(c->tds.flags & PTP_TIMESCALE)) {
pr_warning("foreign master not using PTP timescale");
}
- if (c->tds.currentUtcOffset < c->current_utc_offset) {
+ if (c->tds.currentUtcOffset < c->utc_offset) {
pr_warning("running in a temporal vortex");
}
}
@@ -672,10 +671,10 @@ static int clock_utc_correct(struct clock *c, tmv_t ingress)

if (c->tds.flags & UTC_OFF_VALID && c->tds.flags & TIME_TRACEABLE) {
utc_offset = c->tds.currentUtcOffset;
- } else if (c->tds.currentUtcOffset > c->current_utc_offset) {
+ } else if (c->tds.currentUtcOffset > c->utc_offset) {
utc_offset = c->tds.currentUtcOffset;
} else {
- utc_offset = c->current_utc_offset;
+ utc_offset = c->utc_offset;
}

if (c->tds.flags & LEAP_61) {
@@ -992,7 +991,7 @@ struct clock *clock_create(enum clock_type type, struct config *config,
c->freq_est_interval = config_get_int(config, NULL, "freq_est_interval");
c->grand_master_capable = config_get_int(config, NULL, "gmCapable");
c->kernel_leap = config_get_int(config, NULL, "kernel_leap");
- c->utc_offset = c->current_utc_offset = config_get_int(config, NULL, "utc_offset");
+ c->utc_offset = config_get_int(config, NULL, "utc_offset");
c->time_source = config_get_int(config, NULL, "timeSource");

if (c->free_running) {

[Keller, Jacob E]

-----Original Message-----
Sent: Monday, July 24, 2017 9:38 AM
Subject: [Linuxptp-devel] [PATCH RFC 0/3] Fix UTC offset issue
This patch series addresses the jumping UTC offset issue that occurs
when the default TAI-UTC offset is out of date, but a then GM master
appears providing the correct offset, only to disappear again.
Review and comments are most welcome.
Thanks,
Richard
Simplify UTC tracking.
Latch the UTC offset.
Remove redundant test on the UTC flags.

These all make sense to me, and I didn't spot any issues.

Thanks,
Jake

clock.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
--
2.11.0
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel