Hi Cliff,
On Tue, 17 Dec 2019 at 02:34, Cliff Spradlin via Linuxptp-devel
<linuxptp-devel@lists.sourceforge.net> wrote:
>
> > + ts += offset;
>
> I think there is a sign error here. This line of code is attempting to
> reconstruct the 'tp' variable in sysoff_estimate(). In other words, ts
> should become the ethernet device's PHC timestamp at the time of the
> best sample.
>
> sysoff_measure writes the approximate CLOCK_REALTIME timestamp ((t1 +
> t2) / 2) into the 'ts' variable. However, as written, the line code
> quoted above will adjust that timestamp in the wrong direction.
>
> An example:
>
> Inside sysoff_estimate() with SYSOFF_BASIC:
> t1=1576539849164524663
> t2=1576539849164526165
> tp=1576539849919759249
>
> sysoff_measure() writes:
> offset=-755233835
> delay=1502
> ts=1576539849164525414 (halfway between t1 and t2)
>
> so ts += offset is meant to recover tp, 1576539849919759249.
> Instead, the code is doing 1576539849164525414 + -755233835 =
> 1576539848409291579.
>
> Flipping the sign of the offset produces the correct result:
> 1576539849164525414 + 755233835 = 1576539849919759249
>
> I discovered this because the clock servo would sometimes get
> unrecoverably out-of-sync if the offset suddenly dramatically changed,
> and I think this is the root cause.
>
> Could you please check my work?
>
> Thanks,
> -cliff
>
> On Mon, Nov 12, 2018 at 8:30 AM Miroslav Lichvar <mlich...@redhat.com> wrote:
> >
> > If synchronizing a PHC to the system clock, use one of the
> > PTP_SYS_OFFSET ioctls (if supported) to measure the offset between the
> > two clocks. Negate the offset and switch the timestamp before passing
> > them to the servo.
> >
> > This makes the synchronization between PHC and system clock symmetric.
> >
> > Signed-off-by: Miroslav Lichvar <mlich...@redhat.com>
> > ---
> > phc2sys.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/phc2sys.c b/phc2sys.c
> > index 2cd477a..b8f1ea0 100644
> > --- a/phc2sys.c
> > +++ b/phc2sys.c
> > @@ -790,6 +790,16 @@ static int do_loop(struct node *node, int
> > subscriptions)
> > node->phc_readings,
> > &offset, &ts, &delay) <
> > 0)
> > return -1;
> > + } else if (node->master->clkid == CLOCK_REALTIME &&
> > + clock->sysoff_method >= 0) {
> > + /* use reversed sysoff */
> > + if
> > (sysoff_measure(CLOCKID_TO_FD(clock->clkid),
> > + clock->sysoff_method,
> > + node->phc_readings,
> > + &offset, &ts, &delay) <
> > 0)
> > + return -1;
> > + ts += offset;
Empyrically I can confirm you are right. Changing this to "ts -=
offset" indeed fixes clock stepping for me on the sja1105 DSA driver
when CLOCK_REALTIME is in 2019 and the PHC is in 1970.
Before:
phc2sys[1382]: [239784.751] swp5 sys offset 89167954881846384 s0 freq
+32000000 delay 6285274194481588160
phc2sys[1382]: [239785.752] clockcheck: clock jumped forward or
running faster than expected!
phc2sys[1382]: [239785.752] swp5 sys offset -822381257309914952 s0
freq +32000000 delay 6463362488160962496
phc2sys[1382]: [239786.753] clockcheck: clock jumped backward or
running slower than expected!
phc2sys[1382]: [239786.753] swp5 sys offset 1577295143851923077 s0
freq +32000000
phc2sys[1382]: [239787.753] clockcheck: clock jumped backward or
running slower than expected!
phc2sys[1382]: [239787.753] swp5 sys offset 4072813798994638600 s0
freq +32000000 delay 1809398123186300864
phc2sys[1382]: [239788.754] clockcheck: clock jumped forward or
running faster than expected!
phc2sys[1382]: [239788.754] swp5 sys offset -456572393231267008 s0
freq +32000000
phc2sys[1382]: [239789.754] clockcheck: clock jumped backward or
running slower than expected!
phc2sys[1382]: [239789.754] swp5 sys offset 3445959673445359880 s0
freq +32000000 delay 4558090348213536704
After:
phc2sys[1473]: [240000.866] swp5 sys offset -44288878234 s0 freq
+32000000 delay 46720
phc2sys[1473]: [240001.867] swp5 sys offset -44320915420 s1 freq
-20822 delay 44960
phc2sys[1473]: [240002.868] swp5 sys offset -6101 s2 freq -26923
delay 44800
phc2sys[1473]: [240003.868] swp5 sys offset -183 s2 freq -22835
delay 44880
phc2sys[1473]: [240004.869] swp5 sys offset 1495 s2 freq -21212
delay 44800
phc2sys[1473]: [240005.869] swp5 sys offset 1678 s2 freq -20580
delay 44799
phc2sys[1473]: [240006.870] swp5 sys offset 1076 s2 freq -20679
delay 45120
phc2sys[1473]: [240007.871] swp5 sys offset 834 s2 freq -20598
delay 44800
phc2sys[1473]: [240008.871] swp5 sys offset 248 s2 freq -20934
delay 44960
phc2sys[1473]: [240009.872] swp5 sys offset 1630 s2 freq -19477
delay 43120
phc2sys[1473]: [240010.872] swp5 sys offset -1387 s2 freq -22005
delay 44800
phc2sys[1473]: [240011.873] swp5 sys offset -477 s2 freq -21512
delay 45200
phc2sys[1473]: [240012.874] swp5 sys offset 81 s2 freq -21097
delay 45120
phc2sys[1473]: [240013.874] swp5 sys offset 119 s2 freq -21034
delay 44880
phc2sys[1473]: [240014.875] swp5 sys offset -202 s2 freq -21320
delay 45200
phc2sys[1473]: [240015.875] swp5 sys offset 883 s2 freq -20295
delay 44320
phc2sys[1473]: [240016.876] swp5 sys offset -422 s2 freq -21335
delay 44960
phc2sys[1473]: [240017.877] swp5 sys offset 560 s2 freq -20480
delay 44640
phc2sys[1473]: [240018.877] swp5 sys offset -658 s2 freq -21530
delay 44960
phc2sys[1473]: [240019.878] swp5 sys offset -420 s2 freq -21489
delay 44960
phc2sys[1473]: [240020.878] swp5 sys offset 59 s2 freq -21136
delay 44879
Good job spotting it!
If you send a patch, you can feel free to add:
Tested-by: Vladimir Oltean <olte...@gmail.com>
> > + offset = -offset;
> > } else {
> > /* use phc */
> > if (!read_phc(node->master->clkid,
> > clock->clkid,
> > --
> > 2.17.2
> >
> >
> >
> > _______________________________________________
> > Linuxptp-devel mailing list
> > Linuxptp-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Linuxptp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Thanks,
-Vladimir
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
