[Linuxptp-devel] [PATCH 0/2] Time scale usage clarification

[Libor Pechacek]

This is my proposal how to deal with the UTC/TAI confusion. The patch set adds
explanation of PTP time scale use in man page and disables the unsafe default
in phc2sys.

Libor Pechacek (2):
Document PTP time scale usage and provide examples
phc2sys: Require either -O or -w on command line

phc2sys.8 | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
phc2sys.c | 5 +++++
ptp4l.8 | 16 +++++++++++++++
3 files changed, 88 insertions(+)

[Libor Pechacek]

Signed-off-by: Libor Pechacek <***@suse.cz>
Cc: Miroslav Lichvar <***@redhat.com>
---
phc2sys.8 | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ptp4l.8 | 16 +++++++++++++++
2 files changed, 83 insertions(+)

diff --git a/phc2sys.8 b/phc2sys.8
index f5f72b4..d1e6e4c 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -157,5 +157,72 @@ Display a help message.
.B \-v
Prints the software version and exits.

+.SH TIME SCALE USAGE
+
+.B Ptp4l
+uses either PTP time scale or UTC (Coordinated Universal Time) time
+scale. PTP time scale is continuous and shifted against UTC by a few tens of
+seconds as PTP time scale does not apply leap seconds.
+
+In hardware time stamping mode,
+.B ptp4l
+announces use of PTP time scale and PHC
+is used for the stamps. That means PHC must follow PTP time scale while system
+clock follows UTC. Time offset between these two is maintained by
+.BR phc2sys .
+
+.B Phc2sys
+acquires the offset value either by reading it from ptp4l when
+.B \-w
+is in effect or from command line when
+.B \-O
+is supplied. Failure to maintain the correct offset can result in local system
+clock being off some seconds to domain master system clock when in slave mode,
+or incorect PTP time announced to the network in case the host is the domain
+master.
+
+.SH EXAMPLES
+
+The host is a domain master, PTP clock is synchronized to system clock and the
+time offset is obtained from
+.BR ptp4l .
+.B Phc2sys
+waits for
+.B ptp4l
+to get at least one port in master or slave mode before starting the
+synchronization.
+
+.RS
+\f(CWphc2sys \-c /dev/ptp0 \-s CLOCK_REALTIME \-w\fP
+.RE
+
+Same as above, time offset is provided on command line and
+.B phc2sys
+does not wait for
+.BR ptp4l .
+
+.RS
+\f(CWphc2sys \-c /dev/ptp0 \-s CLOCK_REALTIME \-O 35\fP
+.RE
+
+The host is in slave mode, system clock is synchronized from PTP clock,
+.B phc2sys
+waits for
+.B ptp4l
+and the offset is set automatically.
+
+.RS
+\f(CWphc2sys \-s /dev/ptp0 \-w\fP
+.RE
+
+Same as above, PTP clock id is read from the network interface, the offset is
+provided on command line
+.B phc2sys
+does not wait.
+
+.RS
+\f(CWphc2sys \-i eth0 \-O \-35\fP
+.RE
+
.SH SEE ALSO
.BR ptp4l (8)
diff --git a/ptp4l.8 b/ptp4l.8
index 11b1b6f..f3d2251 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -366,6 +366,22 @@ one-second offset slowly by changing the clock frequency (unless the
option is set to correct such offset by stepping).
Relevant only with software time stamping. The default is 1 (enabled).

+.SH TIME SCALE USAGE
+
+.B ptp4l
+as domain master either uses PTP or UTC time scale depending on time stamping
+mode. In software and legacy time stamping modes it announces Arbitrary time
+scale mode, which is effectively UTC here, in hardware time stamping mode it
+announces use of PTP time scale.
+
+When
+.B ptp4l
+is the domain master using hardware time stamping, it is up to
+.B phc2sys
+to maintain the correct offset between UTC and PTP times. See
+.BR phc2sys (8)
+manual page for more details.
+
.SH SEE ALSO
.BR pmc (8),
.BR phc2sys (8)

[Miroslav Lichvar]

Looks good. The example with the -i option will need an update when
the patch from Jacob removing the option is included.

Thanks,

[Libor Pechacek]

The default zero offset can lead to misalignment between system clocks or wrong
time to be broadcast to the domain. Therefore we require setting offset upon
invocation.

Signed-off-by: Libor Pechacek <***@suse.cz>
Cc: Miroslav Lichvar <***@redhat.com>
---
phc2sys.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/phc2sys.c b/phc2sys.c
index dbe2d9a..aedc0f6 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -648,6 +648,11 @@ int main(int argc, char *argv[])
}
}

+ if (!wait_sync && !forced_sync_offset) {
+ fprintf(stderr, "time offset must be specified using -w or -O\n");
+ return -1;
+ }
+
if (src == CLOCK_INVALID && ethdev) {
struct sk_ts_info ts_info;
char phc_device[16];

[Miroslav Lichvar]

Post by Libor Pechacek
The default zero offset can lead to misalignment between system clocks or wrong
time to be broadcast to the domain. Therefore we require setting offset upon
invocation.

I agree with the change, but please update also the description of the
-O option in the man page and the synopsis line, e.g. {-w | -O offset}
after the {-s | -i | -d } part. (it will conflict with the patch from
Jacob)

Thanks,

[Libor Pechacek]

Post by Miroslav Lichvar

Post by Libor Pechacek
The default zero offset can lead to misalignment between system clocks or wrong
time to be broadcast to the domain. Therefore we require setting offset upon
invocation.

I agree with the change, but please update also the description of the
-O option in the man page and the synopsis line, e.g. {-w | -O offset}

I'm working on the change, but I'm not sure I understand the part about
updating synopsis line.

Is that like -w and O should be closer to each other on the line
"
SYNOPSIS
phc2sys [ -d pps-device ] [ -s device ] [ -c device ] [ -P kp ] [ -I ki
] [ -R update-rate ] [ -N clock-readings ] *[ -O offset ] [ -w ]*
[ -u summary- updates ] [ -n domain-number ] [ -x ] [ -l print-level ]
[ -m ] [ -q ] [ -v ]
"
or something else?

Looking at the resulting text I'm tempted to compress the options without
arguments into one block like in gzip manual page and order the options from
mandatory to optional ones:
"
phc2sys [-wmqvx] [ -d pps-device ] [ -s device ] [ -c device ] [ -O
offset ] [ -n domain-number ] [ -P kp ] [ -I ki ] [ -R update-rate ]
[ -N clock-readings ] [ -u summary-updates ] [ -l print-level ]
"

Libor

[Miroslav Lichvar]

Post by Libor Pechacek
Is that like -w and O should be closer to each other on the line
"
SYNOPSIS
phc2sys [ -d pps-device ] [ -s device ] [ -c device ] [ -P kp ] [ -I ki
] [ -R update-rate ] [ -N clock-readings ] *[ -O offset ] [ -w ]*
[ -u summary- updates ] [ -n domain-number ] [ -x ] [ -l print-level ]
[ -m ] [ -q ] [ -v ]
"
or something else?

I was thinking to enclose them in curly braces to show that at least
one of them has to be used.

e.g.

phc2sys { -d pps-device | -s device } { -w | -O offset } [ -c device ] ...

Post by Libor Pechacek
Looking at the resulting text I'm tempted to compress the options without
arguments into one block like in gzip manual page and order the options from
"
phc2sys [-wmqvx] [ -d pps-device ] [ -s device ] [ -c device ] [ -O
offset ] [ -n domain-number ] [ -P kp ] [ -I ki ] [ -R update-rate ]
[ -N clock-readings ] [ -u summary-updates ] [ -l print-level ]
"

Yes, that would be nice.

Thanks,

[Libor Pechacek]

Post by Miroslav Lichvar

Post by Libor Pechacek
Is that like -w and O should be closer to each other on the line
"
SYNOPSIS
phc2sys [ -d pps-device ] [ -s device ] [ -c device ] [ -P kp ] [ -I ki
] [ -R update-rate ] [ -N clock-readings ] *[ -O offset ] [ -w ]*
[ -u summary- updates ] [ -n domain-number ] [ -x ] [ -l print-level ]
[ -m ] [ -q ] [ -v ]
"
or something else?

I was thinking to enclose them in curly braces to show that at least
one of them has to be used.
e.g.
phc2sys { -d pps-device | -s device } { -w | -O offset } [ -c device ] ...

Hmm, that's how I originally understood the request, but man-pages(7) does not
mention this convention and I was unagle to google anything better. Also the
vertical bar means "choice" while -w and -O can be both supplied on invocation.

I also looked at documentation of other tools and, for instance, gzip or tar
also do require arguments while their manual pages look very similar to the
above. All options are in brackets in synopsis.

I'd follow the crowd here and keep the synopsis as is for now.

Post by Miroslav Lichvar

Post by Libor Pechacek
Looking at the resulting text I'm tempted to compress the options without
arguments into one block like in gzip manual page and order the options from
"
phc2sys [-wmqvx] [ -d pps-device ] [ -s device ] [ -c device ] [ -O
offset ] [ -n domain-number ] [ -P kp ] [ -I ki ] [ -R update-rate ]
[ -N clock-readings ] [ -u summary-updates ] [ -l print-level ]
"

Yes, that would be nice.

Looking at ptp4l man page it will benefit even more from this kind of change.
I'll send a separate patch.

Libor

[Keller, Jacob E]

Libor,

-----Original Message-----
Sent: Friday, May 17, 2013 12:45 AM
To: Miroslav Lichvar
Subject: Re: [Linuxptp-devel] [PATCH 2/2] phc2sys: Require either -O or
-w on command line

Post by Miroslav Lichvar

Post by Libor Pechacek
Is that like -w and O should be closer to each other on the line
"
SYNOPSIS
phc2sys [ -d pps-device ] [ -s device ] [ -c device ] [ -P kp ] [ -I ki
] [ -R update-rate ] [ -N clock-readings ] *[ -O offset ] [ -w ]*
[ -u summary- updates ] [ -n domain-number ] [ -x ] [ -l print-

level ]

Post by Miroslav Lichvar

Post by Libor Pechacek
[ -m ] [ -q ] [ -v ]
"
or something else?

I was thinking to enclose them in curly braces to show that at least
one of them has to be used.
e.g.
phc2sys { -d pps-device | -s device } { -w | -O offset } [ -c device ] ...

Hmm, that's how I originally understood the request, but man-pages(7) does not
mention this convention and I was unagle to google anything better.
Also the
vertical bar means "choice" while -w and -O can be both supplied on invocation.
I also looked at documentation of other tools and, for instance, gzip or tar
also do require arguments while their manual pages look very similar to the
above. All options are in brackets in synopsis.
I'd follow the crowd here and keep the synopsis as is for now.

This is because standard GNU convention is that command line "options" should never be required... But I don't believe that really should be set in stone... Personally, I think | or || would work fine because logically, or can be true if either, or both is set. But there really is no standard.

I do agree with the other layout change you suggested. I also think that the sections below which describe the options can go into more detail.

- Jake

[Richard Cochran]

Miroslav, do you care to comment on these two patches?

Thanks,
Richard

[Richard Cochran]

Post by Libor Pechacek
This is my proposal how to deal with the UTC/TAI confusion. The patch set adds
explanation of PTP time scale use in man page and disables the unsafe default
in phc2sys.
Document PTP time scale usage and provide examples
phc2sys: Require either -O or -w on command line

Libor,

I have applied Jacob's two patches. Could you please respin these two
with consideration of Miroslav's comments?

Thanks,
Richard

[Libor Pechacek]

Post by Richard Cochran
I have applied Jacob's two patches. Could you please respin these two
with consideration of Miroslav's comments?

Done. Patch set based on current tip (2a89756) follows. Changes made in v2
are documented in patch headers. Please note the first patch also changes
manual page for both ptp4l and phc2sys.

Libor

Libor Pechacek (2):
Document PTP time scale usage and provide examples
phc2sys: Require either -O or -w on command line

phc2sys.8 | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
phc2sys.c | 7 ++++++
ptp4l.8 | 16 ++++++++++++++
3 files changed, 94 insertions(+), 4 deletions(-)

[Libor Pechacek]

The default zero offset can lead to misalignment between system clocks or wrong
time to be broadcast to the domain. Therefore we require setting offset upon
invocation.

Signed-off-by: Libor Pechacek <***@suse.cz>

---
changes in v2:
- documented the change in manual page
- possible changes in the synopsis line will be sent separately

phc2sys.8 | 8 ++++----
phc2sys.c | 7 +++++++
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/phc2sys.8 b/phc2sys.8
index 43e3abc..92081cf 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -109,10 +109,10 @@ minimize the error caused by random delays in scheduling and bus utilization.
The default is 5.
.TP
.BI \-O " offset"
-Specify the offset between the slave and master times in seconds. The default
-is set automatically with the
-.B \-w
-option, 0 otherwise.
+Specify the offset between the slave and master times in seconds. See
+.SM
+.B TIME SCALE USAGE
+below.
.TP
.BI \-u " summary-updates"
Specify the number of clock updates included in summary statistics. The
diff --git a/phc2sys.c b/phc2sys.c
index 1dbe2bc..309ee6c 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -684,6 +684,13 @@ int main(int argc, char *argv[])
return -1;
}

+ if (!wait_sync && !forced_sync_offset) {
+ fprintf(stderr,
+ "time offset must be specified using -w or -O\n");
+ usage(progname);
+ return -1;
+ }
+
if (dst_clock.stats_max_count > 0) {
dst_clock.offset_stats = stats_create();
dst_clock.freq_stats = stats_create();

[Libor Pechacek]

Signed-off-by: Libor Pechacek <***@suse.cz>

---
changes in v2:
- adapted and re-tested examples

phc2sys.8 | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ptp4l.8 | 16 +++++++++++++++
2 files changed, 83 insertions(+)

diff --git a/phc2sys.8 b/phc2sys.8
index ee93b80..43e3abc 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -156,5 +156,72 @@ Display a help message.
.B \-v
Prints the software version and exits.

+.SH TIME SCALE USAGE
+
+.B Ptp4l
+uses either PTP time scale or UTC (Coordinated Universal Time) time
+scale. PTP time scale is continuous and shifted against UTC by a few tens of
+seconds as PTP time scale does not apply leap seconds.
+
+In hardware time stamping mode,
+.B ptp4l
+announces use of PTP time scale and PHC
+is used for the stamps. That means PHC must follow PTP time scale while system
+clock follows UTC. Time offset between these two is maintained by
+.BR phc2sys .
+
+.B Phc2sys
+acquires the offset value either by reading it from ptp4l when
+.B \-w
+is in effect or from command line when
+.B \-O
+is supplied. Failure to maintain the correct offset can result in local system
+clock being off some seconds to domain master system clock when in slave mode,
+or incorect PTP time announced to the network in case the host is the domain
+master.
+
+.SH EXAMPLES
+
+The host is a domain master, PTP clock is synchronized to system clock and the
+time offset is obtained from
+.BR ptp4l .
+.B Phc2sys
+waits for
+.B ptp4l
+to get at least one port in master or slave mode before starting the
+synchronization.
+
+.RS
+\f(CWphc2sys \-c /dev/ptp0 \-s CLOCK_REALTIME \-w\fP
+.RE
+
+Same as above, time offset is provided on command line and
+.B phc2sys
+does not wait for
+.BR ptp4l .
+
+.RS
+\f(CWphc2sys \-c /dev/ptp0 \-s CLOCK_REALTIME \-O 35\fP
+.RE
+
+The host is in slave mode, system clock is synchronized from PTP clock,
+.B phc2sys
+waits for
+.B ptp4l
+and the offset is set automatically.
+
+.RS
+\f(CWphc2sys \-s /dev/ptp0 \-w\fP
+.RE
+
+Same as above, PTP clock id is read from the network interface, the offset is
+provided on command line
+.B phc2sys
+does not wait.
+
+.RS
+\f(CWphc2sys \-s eth0 \-O \-35\fP
+.RE
+
.SH SEE ALSO
.BR ptp4l (8)
diff --git a/ptp4l.8 b/ptp4l.8
index 11b1b6f..f3d2251 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -366,6 +366,22 @@ one-second offset slowly by changing the clock frequency (unless the
option is set to correct such offset by stepping).
Relevant only with software time stamping. The default is 1 (enabled).

+.SH TIME SCALE USAGE
+
+.B ptp4l
+as domain master either uses PTP or UTC time scale depending on time stamping
+mode. In software and legacy time stamping modes it announces Arbitrary time
+scale mode, which is effectively UTC here, in hardware time stamping mode it
+announces use of PTP time scale.
+
+When
+.B ptp4l
+is the domain master using hardware time stamping, it is up to
+.B phc2sys
+to maintain the correct offset between UTC and PTP times. See
+.BR phc2sys (8)
+manual page for more details.
+
.SH SEE ALSO
.BR pmc (8),
.BR phc2sys (8)

[Keller, Jacob E]

Libor,

-----Original Message-----
Sent: Friday, May 17, 2013 1:56 AM
Subject: [Linuxptp-devel] [PATCHv2 2/2] phc2sys: Require either -O or -
w on command line
The default zero offset can lead to misalignment between system clocks
or wrong
time to be broadcast to the domain. Therefore we require setting
offset upon
invocation.
---
- documented the change in manual page
- possible changes in the synopsis line will be sent separately
phc2sys.8 | 8 ++++----
phc2sys.c | 7 +++++++
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/phc2sys.8 b/phc2sys.8
index 43e3abc..92081cf 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -109,10 +109,10 @@ minimize the error caused by random delays in
scheduling and bus utilization.
The default is 5.
.TP
.BI \-O " offset"
-Specify the offset between the slave and master times in seconds. The
default
-is set automatically with the
-.B \-w
-option, 0 otherwise.
+Specify the offset between the slave and master times in seconds. See
+.SM
+.B TIME SCALE USAGE
+below.
.TP
.BI \-u " summary-updates"
Specify the number of clock updates included in summary statistics.
The
diff --git a/phc2sys.c b/phc2sys.c
index 1dbe2bc..309ee6c 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -684,6 +684,13 @@ int main(int argc, char *argv[])
return -1;
}
+ if (!wait_sync && !forced_sync_offset) {
+ fprintf(stderr,
+ "time offset must be specified using -w or -
O\n");
+ usage(progname);
+ return -1;
+ }
+
if (dst_clock.stats_max_count > 0) {
dst_clock.offset_stats = stats_create();
dst_clock.freq_stats = stats_create();
--
1.7.12.4

So.. why not just automatically enable waiting if you don't specify an -O? I'm not sure if you're just trying to prevent the odd case where you start adding a -O and then don't wait? It seems like this required thing could trip people up as well... What if we did this a little differently:

instead of "-w" for wait, and "-O number" for set offset, we have...

"-W number" for "set offset and wait",
"-O number" for "set offset and don't wait"

and the default is wait and set offset automatically?

That way we don't have a required option, but all the sets of options right? I'm not really sure, just trying to see if there's a good alternative to required options...

- Jake

[Libor Pechacek]

Jacob,

On Fri 17-05-13 18:46:09, Keller, Jacob E wrote:
[...]

Post by Keller, Jacob E

Post by Libor Pechacek
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -684,6 +684,13 @@ int main(int argc, char *argv[])
return -1;
}
+ if (!wait_sync && !forced_sync_offset) {
+ fprintf(stderr,
+ "time offset must be specified using -w or -
O\n");
+ usage(progname);
+ return -1;
+ }
+
if (dst_clock.stats_max_count > 0) {
dst_clock.offset_stats = stats_create();
dst_clock.freq_stats = stats_create();

So.. why not just automatically enable waiting if you don't specify an -O?
I'm not sure if you're just trying to prevent the odd case where you start
adding a -O and then don't wait? It seems like this required thing could trip
instead of "-w" for wait, and "-O number" for set offset, we have...
"-W number" for "set offset and wait",
"-O number" for "set offset and don't wait"
and the default is wait and set offset automatically?

Hmm, we've already explored this option:
http://sourceforge.net/mailarchive/message.php?msg_id=30821960

Default wait behavior would make phc2sys to depend on the ptp4l socket and/or
sync to appear. I don't like hiding the dependency behind default behavior.

Post by Keller, Jacob E
That way we don't have a required option, but all the sets of options right?
I'm not really sure, just trying to see if there's a good alternative to
required options...

I'd like the user make conscious decision whether to make phc2sys depend on
ptp4l and be safe with offset handling or make phc2sys independent with the
risk of getting the offset wrong.

Libor

[Keller, Jacob E]

Libor,

-----Original Message-----
Sent: Monday, May 20, 2013 5:46 AM
To: Keller, Jacob E
Subject: Re: [Linuxptp-devel] [PATCHv2 2/2] phc2sys: Require either -O
or -w on command line
Jacob,
[...]

Post by Keller, Jacob E

Post by Libor Pechacek
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -684,6 +684,13 @@ int main(int argc, char *argv[])
return -1;
}
+ if (!wait_sync && !forced_sync_offset) {
+ fprintf(stderr,
+ "time offset must be specified using -w or -
O\n");
+ usage(progname);
+ return -1;
+ }
+
if (dst_clock.stats_max_count > 0) {
dst_clock.offset_stats = stats_create();
dst_clock.freq_stats = stats_create();

So.. why not just automatically enable waiting if you don't specify an -

O?

Post by Keller, Jacob E
I'm not sure if you're just trying to prevent the odd case where you

start

Post by Keller, Jacob E
adding a -O and then don't wait? It seems like this required thing

could trip

Post by Keller, Jacob E
instead of "-w" for wait, and "-O number" for set offset, we have...
"-W number" for "set offset and wait",
"-O number" for "set offset and don't wait"
and the default is wait and set offset automatically?

http://sourceforge.net/mailarchive/message.php?msg_id=30821960
Default wait behavior would make phc2sys to depend on the ptp4l socket and/or
sync to appear. I don't like hiding the dependency behind default behavior.

Post by Keller, Jacob E
That way we don't have a required option, but all the sets of options

right?

Post by Keller, Jacob E
I'm not really sure, just trying to see if there's a good alternative to
required options...

I'd like the user make conscious decision whether to make phc2sys depend on
ptp4l and be safe with offset handling or make phc2sys independent with the
risk of getting the offset wrong.
Libor

I agree. I was just trying to see if there was an alternative. Generally "options" shouldn't be required.

But I do agree in this case there is no good default.

- Jake

[Richard Cochran]

Post by Libor Pechacek
---
- adapted and re-tested examples
phc2sys.8 | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ptp4l.8 | 16 +++++++++++++++
2 files changed, 83 insertions(+)

Applied.

Thanks,
Richard

[Richard Cochran]

Post by Libor Pechacek
The default zero offset can lead to misalignment between system clocks or wrong
time to be broadcast to the domain. Therefore we require setting offset upon
invocation.
---
- documented the change in manual page
- possible changes in the synopsis line will be sent separately
phc2sys.8 | 8 ++++----
phc2sys.c | 7 +++++++
2 files changed, 11 insertions(+), 4 deletions(-)

Applied.

Thanks,
Richard