On Wed, Sep 15, 2021 at 12:08:03PM +0200, Miroslav Lichvar wrote:
> When operating as a grandmaster, clear the programmed leap second and
> update the UTC offset when the first announce message after the leap
> second is sent. This guarantees the update happens in +/- 2 announce
> messages around the UTC midnight as required by the specification and
> removes the responsibility from the external process which programmed
> the leap second with the GRANDMASTER_SETTINGS_NP management message.
> 
> The code reads the clock directly instead of using a packet timestamp to
> not depend on (timing of) sync messages.
> 
> Signed-off-by: Miroslav Lichvar <mlich...@redhat.com>
> ---
>  clock.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/clock.c b/clock.c
> index c681cbf..c6646c3 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -1892,9 +1892,46 @@ void clock_sync_interval(struct clock *c, int n)
>       servo_sync_interval(c->servo, n < 0 ? 1.0 / (1 << -n) : 1 << n);
>  }
>  
> +static void clock_update_utc_offset(struct clock *c)
> +{
> +     struct timespec ts;
> +     int leap;
> +
> +     if (c->tds.flags & LEAP_61) {
> +             leap = 1;
> +     } else if (c->tds.flags & LEAP_59) {
> +             leap = -1;
> +     } else {
> +             leap = 0;
> +     }
> +
> +     /* Don't do anything if not a grandmaster with a leap flag set. */
> +     if (c->cur.stepsRemoved > 0 || leap == 0) {
> +             return;
> +     }
> +
> +     clock_gettime(c->clkid, &ts);
> +     if (c->time_flags & PTP_TIMESCALE) {
> +             ts.tv_sec -= c->utc_offset;
> +     }
> +
> +     /* Wait until the leap second has passed. */
> +     if (leap_second_status(tmv_to_nanoseconds(timespec_to_tmv(ts)),
> +                            leap, &leap, &c->utc_offset)) {
> +             return;
> +     }
> +
> +     c->time_flags &= ~(LEAP_61 | LEAP_59);
> +     clock_update_grandmaster(c);

This is not good.  The function, clock_update_grandmaster, implements
the data set update as specified in IEEE 1588 Clause 9.3.5.

> +}
> +
>  struct timePropertiesDS clock_time_properties(struct clock *c)
>  {
> -     struct timePropertiesDS tds = c->tds;
> +     struct timePropertiesDS tds;
> +
> +     clock_update_utc_offset(c);

The function, clock_time_properties, is a query that should not change
the program state.

With this change, one call chain will be:

port_tx_announce
 clock_time_properties
  clock_update_utc_offset
   clock_update_grandmaster

That clobbers the grandmasterIdentity with the local dds.clockIdentity
in case of BC.

Another bad situation is:

process_delay_req
 net_sync_resp_append
  clock_time_properties
   clock_update_utc_offset
    clock_update_grandmaster


Thanks,
Richard


_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to