Re: [Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and clock
Richard,
Thanks for the feedback. As I mentioned to Erez, my goal is to provide access
to the statistics behind this log message without resorting to log scraping:
ptp4l[5024110.731]: rms9 max 17 freq -17594 +/- 14 delay 220 +/- 1
I wasn’t aware of the SUBSCRIBE_EVENTS_NP…that partially solves my use case for
the first two numbers - rms and max offset.
./pmc -d 24 -u 'SET SUBSCRIBE_EVENTS_NP duration 10 NOTIFY_PORT_STATE on
NOTIFY_TIME_SYNC on'
…
0c42a1.fffe.d1d1bd-0 seq 67 RESPONSE MANAGEMENT TIME_STATUS_NP
master_offset -13
ingress_time 1622134808076003202
cumulativeScaledRateOffset +0.0
scaledLastGmPhaseChange0
gmTimeBaseIndicator0
lastGmPhaseChange 0x'.
gmPresent true
…
I could use the ‘master_offset’ notifications to calculate statistics on the
offset in an external program, but in order to calculate statistics on the
frequency (rate) that external program would need to make assumptions on the
servo tracking loop internal state variables and loop coefficients, i.e. the
‘adj’ variable (which is the rate) is also required:
static void clock_stats_update(struct clock *c,
double offset, double freq)
…
adj = servo_sample(c->servo, offset, tmv_to_nanoseconds(ingress),
weight, );
…
if (c->stats.max_count > 1) {
clock_stats_update(c, tmv_dbl(c->master_offset), adj);
Using only the SUBSCRIBE_EVENTS_NP would also leave the delay statistic
unavailable.
Thanks,
Tim
From: Richard Cochran
Date: Thursday, May 27, 2021 at 6:18 AM
To: Tim Martin
Cc: linuxptp-devel@lists.sourceforge.net
Subject: Re: [Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and
clock
External email: Use caution opening links or attachments
On Tue, May 25, 2021 at 08:28:34PM +, Tim Martin wrote:
> Currently there is no way to programmatically access statistics about
> the clock frequency offset, time delay, or time offset (collectively,
> the "clock_stats" metrics), except for parsing the ptp4l logs. One
> option for time offset would be to poll TLV_TIME_STATUS_NP in regular
> intervals from a custom client, but that has the disadvantage of either
> very high poll rates or missed updates which would affect the statistics.
Not if you use the push method.
commit 6d7c090706e76af334185ffcec9cc56d0570e215
Author: Juergen Werner
Date: Wed Jan 20 20:11:34 2021 +0100
Implement push notification for TIME_STATUS_NP
Subscribers to NOTIFY_TIME_SYNC will be notified on every clock
synchronization.
Thanks,
Richard
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and clock
Thanks Miroslav,
Thanks for the feedback. Sure, I can make an update to send
network(big)-endian fixed point instead of platform-endian floating point if
the patch will otherwise be accepted, but need to resolve the feedback from
Erez and Richard first.
Tim
From: Miroslav Lichvar
Date: Wednesday, May 26, 2021 at 4:45 AM
To: Tim Martin
Cc: linuxptp-devel@lists.sourceforge.net
Subject: Re: [Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and
clock
External email: Use caution opening links or attachments
On Tue, May 25, 2021 at 08:28:34PM +, Tim Martin wrote:
> +struct clock_stats_np_tlv {
> + struct stats_result offset_stats;
> + struct stats_result freq_stats;
> + struct stats_result delay_stats;
> + uint8_t offset_freq_stats_valid;
> + uint8_t delay_stats_valid;
> +} PACKED;
Floating-point formats are not generally portable. In a network
protocol they need to be converted to something else. Some of the
implemented TLVs use fixed-point formats shifting by 32 or 41 bits. In
this case I think it might be ok to just use integer nanoseconds and
parts per billion.
--
Miroslav Lichvar
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and clock
Thanks for the feedback Erez,
My goal of this initial patch was to give an API to extract the statistics that
are already calculated/existing in the clock module and already printed to the
logs, i.e. the statistics that generate the numbers in this log message:
/* Path delay stats are updated separately, they may be empty. */
if (c->delay_stats_valid) {
pr_info("rms %4.0f max %4.0f "
"freq %+6.0f +/- %3.0f "
"delay %5.0f +/- %3.0f",
c->offset_stats.rms, c->offset_stats.max_abs,
c->freq_stats.mean, c->freq_stats.stddev,
c->delay_stats.mean, c->delay_stats.stddev);
} else {
pr_info("rms %4.0f max %4.0f "
"freq %+6.0f +/- %3.0f",
c->offset_stats.rms, c->offset_stats.max_abs,
c->freq_stats.mean, c->freq_stats.stddev);
The log message looks like this:
ptp4l[5024110.731]: rms9 max 17 freq -17594 +/- 14 delay 220 +/- 1
One method of course would be to scrape the logs for these 6 numbers, but when
I looked into the implementation I found the other statistics so I decided to
include all of them in the CLOCK_STATS_NP TLV for thoroughness:
struct stats_result {
double min;
double max;
double max_abs;
double mean;
double rms;
double stddev;
};
As far as I can tell, the PARENT_DATA_SET TLV isn’t actually implemented?
/* Initialize the parentDS. */
clock_update_grandmaster(c);
c->dad.pds.parentStats = 0;
c->dad.pds.observedParentOffsetScaledLogVariance = 0x;
c->dad.pds.observedParentClockPhaseChangeRate= 0x7fff;
Doing a pmc get confirms this, at least for my setup:
sending: GET PARENT_DATA_SET
0c42a1.fffe.d1d1bd-0 seq 0 RESPONSE MANAGEMENT PARENT_DATA_SET
…
parentStats 0
observedParentOffsetScaledLogVariance 0x
observedParentClockPhaseChangeRate0x7fff
…
After reading the relevant 1588-2019 paragraphs, I’m not 100% sure the existing
calculation of freq_stats and offset_stats are transformable to
“observedParentClockPhaseChangeRate” and
“observedParentOffsetScaledLogVariance”, respectively. Modification of these
existing stats could be future work after a more careful review of 1588-2019
(i.e. section 7.6.3, etc).
Thanks,
Tim
From: Geva, Erez
Date: Wednesday, May 26, 2021 at 4:09 AM
To: Tim Martin
Cc: linuxptp-devel@lists.sourceforge.net
Subject: RE: [Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and
clock
External email: Use caution opening links or attachments
In IEEE 1588-2019
I see PARENT_DATA_SET with
- observedParentOffsetScaledLogVariance
- observedParentClockPhaseChangeRate
You use a natural statistics syntax.
Perhaps you may use a more coherent phrasing that match the standard.
May be something that match the performanceMonitoringDS, like
- averageMasterSlaveDelay
- minMasterSlaveDelay
...
Erez
-Original Message-
From: Tim Martin
Sent: Tuesday, 25 May 2021 22:29
To: linuxptp-devel@lists.sourceforge.net
Subject: [Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and
clock
Currently there is no way to programmatically access statistics about the clock
frequency offset, time delay, or time offset (collectively, the "clock_stats"
metrics), except for parsing the ptp4l logs. One option for time offset would
be to poll TLV_TIME_STATUS_NP in regular intervals from a custom client, but
that has the disadvantage of either very high poll rates or missed updates
which would affect the statistics.
To address these issues, introduce a management mesage, CLOCK_STATS_NP, which
reports the statistics that would be displayed in the ptp4l log file. This
fixes the above problem by allowing a custom client to poll CLOCK_STATS_NP at
longer intervals without missing any updates.
Follow-up patches could then introduc knobs to configure the time over which
CLOCK_STATS_NP integrates statistics, separate from any other tracking loop
updates, which would allow even longer polling intervals.
Signed-off-by: Tim Martin
---
clock.c | 50 ++
pmc.c| 48
pmc_common.c | 1 +
tlv.h| 10 ++
4 files changed, 93 insertions(+), 16 deletions(-)
diff --git a/clock.c b/clock.c
index e545a9b..550d222 100644
--- a/clock.c
+++ b/clock.c
@@ -129,6 +129,9 @@ struct clock {
double nrr;
struct clock_description desc;
struct clock_stats stats;
+ struct stats_result offset_stats, freq_stats, delay_stats;
+ int offset_freq_stats_valid;
+ int
[Linuxptp-devel] [PATCH] Add new CLOCK_STATS_NP TLV GET to pmc and clock
Currently there is no way to programmatically access statistics about
the clock frequency offset, time delay, or time offset (collectively,
the "clock_stats" metrics), except for parsing the ptp4l logs. One
option for time offset would be to poll TLV_TIME_STATUS_NP in regular
intervals from a custom client, but that has the disadvantage of either
very high poll rates or missed updates which would affect the statistics.
To address these issues, introduce a management mesage,
CLOCK_STATS_NP, which reports the statistics that would be displayed
in the ptp4l log file. This fixes the above problem by allowing
a custom client to poll CLOCK_STATS_NP at longer intervals without
missing any updates.
Follow-up patches could then introduc knobs to configure the time
over which CLOCK_STATS_NP integrates statistics, separate from any
other tracking loop updates, which would allow even longer polling
intervals.
Signed-off-by: Tim Martin
---
clock.c | 50 ++
pmc.c| 48
pmc_common.c | 1 +
tlv.h| 10 ++
4 files changed, 93 insertions(+), 16 deletions(-)
diff --git a/clock.c b/clock.c
index e545a9b..550d222 100644
--- a/clock.c
+++ b/clock.c
@@ -129,6 +129,9 @@ struct clock {
double nrr;
struct clock_description desc;
struct clock_stats stats;
+ struct stats_result offset_stats, freq_stats, delay_stats;
+ int offset_freq_stats_valid;
+ int delay_stats_valid;
int stats_interval;
struct clockcheck *sanity_check;
struct interface *uds_rw_if;
@@ -144,7 +147,7 @@ struct clock the_clock;
static void handle_state_decision_event(struct clock *c);
static int clock_resize_pollfd(struct clock *c, int new_nports);
static void clock_remove_port(struct clock *c, struct port *p);
-static void clock_stats_display(struct clock_stats *s);
+static void clock_stats_display(struct clock *c);
static void remove_subscriber(struct clock_subscriber *s)
{
@@ -351,6 +354,7 @@ static int clock_management_fill_response(struct clock *c,
struct port *p,
struct tlv_extra *extra;
struct PTPText *text;
uint16_t duration;
+ struct clock_stats_np_tlv *stats;
int datalen = 0;
extra = tlv_extra_alloc();
@@ -462,6 +466,15 @@ static int clock_management_fill_response(struct clock *c,
struct port *p,
mtd->val = c->local_sync_uncertain;
datalen = sizeof(*mtd);
break;
+ case TLV_CLOCK_STATS_NP:
+ stats = (struct clock_stats_np_tlv *) tlv->data;
+ stats->offset_stats = c->offset_stats;
+ stats->freq_stats = c->freq_stats;
+ stats->delay_stats = c->delay_stats;
+ stats->offset_freq_stats_valid = c->offset_freq_stats_valid;
+ stats->delay_stats_valid = c->delay_stats_valid;
+ datalen = sizeof(*stats);
+ break;
default:
/* The caller should *not* respond to this message. */
tlv_extra_recycle(extra);
@@ -544,7 +557,7 @@ static int clock_management_set(struct clock *c, struct
port *p,
/* Display stats on change of local_sync_uncertain */
if (c->local_sync_uncertain != mtd->val
&& stats_get_num_values(c->stats.offset))
- clock_stats_display(>stats);
+ clock_stats_display(c);
c->local_sync_uncertain = mtd->val;
respond = 1;
break;
@@ -556,38 +569,41 @@ static int clock_management_set(struct clock *c, struct
port *p,
return respond ? 1 : 0;
}
-static void clock_stats_update(struct clock_stats *s,
+static void clock_stats_update(struct clock *c,
double offset, double freq)
{
+ struct clock_stats *s = >stats;
stats_add_value(s->offset, offset);
stats_add_value(s->freq, freq);
if (stats_get_num_values(s->offset) < s->max_count)
return;
- clock_stats_display(s);
+ clock_stats_display(c);
}
-static void clock_stats_display(struct clock_stats *s)
+static void clock_stats_display(struct clock *c)
{
- struct stats_result offset_stats, freq_stats, delay_stats;
+ struct clock_stats *s = >stats;
- stats_get_result(s->offset, _stats);
- stats_get_result(s->freq, _stats);
+ stats_get_result(s->offset, >offset_stats);
+ stats_get_result(s->freq, >freq_stats);
+ c->offset_freq_stats_valid = 1;
+ c->delay_stats_valid = !stats_get_result(s->delay, >delay_stats);
/* Path delay stats are updated separately, they may be empty. */
-
