[Linuxptp-devel] [PATCHv2 1/6] Convert and correct time stamps early.

[Miroslav Lichvar]

Convert time stamps to tmv_t and apply all corrections before passing
them to clock/port functions to reduce the number of parameters.

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
clock.c | 56 +++++++++++---------------------------------------------
clock.h | 19 ++++++++-----------
port.c | 46 +++++++++++++++++++++++++++-------------------
3 files changed, 46 insertions(+), 75 deletions(-)

diff --git a/clock.c b/clock.c
index b841e81..f5349b9 100644
--- a/clock.c
+++ b/clock.c
@@ -106,8 +106,6 @@ struct clock {
struct freq_estimator fest;
struct time_status_np status;
double nrr;
- tmv_t c1;
- tmv_t c2;
tmv_t t1;
tmv_t t2;
struct clock_description desc;
@@ -549,16 +547,9 @@ static enum servo_state clock_no_adjust(struct clock *c)
{
double fui;
double ratio, freq;
- tmv_t origin2;
struct freq_estimator *f = &c->fest;
enum servo_state state = SERVO_UNLOCKED;
/*
- * We have clock.t1 as the origin time stamp, and clock.t2 as
- * the ingress. According to the master's clock, the time at
- * which the sync arrived is:
- *
- * origin = origin_ts + path_delay + correction
- *
* The ratio of the local clock freqency to the master clock
* is estimated by:
*
@@ -571,7 +562,7 @@ static enum servo_state clock_no_adjust(struct clock *c)
*/
if (!f->ingress1) {
f->ingress1 = c->t2;
- f->origin1 = tmv_add(c->t1, tmv_add(c->c1, c->c2));
+ f->origin1 = c->t1;
return state;
}
f->count++;
@@ -582,12 +573,8 @@ static enum servo_state clock_no_adjust(struct clock *c)
pr_warning("bad timestamps in rate ratio calculation");
return state;
}
- /*
- * origin2 = c->t1 (+c->path_delay) + c->c1 + c->c2;
- */
- origin2 = tmv_add(c->t1, tmv_add(c->c1, c->c2));

- ratio = tmv_dbl(tmv_sub(origin2, f->origin1)) /
+ ratio = tmv_dbl(tmv_sub(c->t1, f->origin1)) /
tmv_dbl(tmv_sub(c->t2, f->ingress1));
freq = (1.0 - ratio) * 1e9;

@@ -611,7 +598,7 @@ static enum servo_state clock_no_adjust(struct clock *c)
pr_debug("diff %+.9f", ratio - (fui + c->nrr - 1.0));

f->ingress1 = c->t2;
- f->origin1 = origin2;
+ f->origin1 = c->t1;
f->count = 0;

return state;
@@ -1289,27 +1276,22 @@ int clock_poll(struct clock *c)
return 0;
}

-void clock_path_delay(struct clock *c, struct timespec req, struct timestamp rx,
- Integer64 correction)
+void clock_path_delay(struct clock *c, tmv_t req, tmv_t rx)
{
- tmv_t c1, c2, c3, pd, t1, t2, t3, t4;
+ tmv_t pd, t1, t2, t3, t4;
double rr;

if (tmv_is_zero(c->t1))
return;

- c1 = c->c1;
- c2 = c->c2;
- c3 = correction_to_tmv(correction);
t1 = c->t1;
t2 = c->t2;
- t3 = timespec_to_tmv(req);
- t4 = timestamp_to_tmv(rx);
+ t3 = req;
+ t4 = rx;
rr = clock_rate_ratio(c);

/*
* c->path_delay = (t2 - t3) * rr + (t4 - t1);
- * c->path_delay -= c_sync + c_fup + c_delay_resp;
* c->path_delay /= 2.0;
*/

@@ -1317,18 +1299,14 @@ void clock_path_delay(struct clock *c, struct timespec req, struct timestamp rx,
if (rr != 1.0)
pd = dbl_tmv(tmv_dbl(pd) * rr);
pd = tmv_add(pd, tmv_sub(t4, t1));
- pd = tmv_sub(pd, tmv_add(c1, tmv_add(c2, c3)));
pd = tmv_div(pd, 2);

if (pd < 0) {
pr_debug("negative path delay %10" PRId64, pd);
- pr_debug("path_delay = (t2 - t3) * rr + (t4 - t1) - (c1 + c2 + c3)");
+ pr_debug("path_delay = (t2 - t3) * rr + (t4 - t1)");
pr_debug("t2 - t3 = %+10" PRId64, t2 - t3);
pr_debug("t4 - t1 = %+10" PRId64, t4 - t1);
pr_debug("rr = %.9f", rr);
- pr_debug("c1 %10" PRId64, c1);
- pr_debug("c2 %10" PRId64, c2);
- pr_debug("c3 %10" PRId64, c3);
}

c->path_delay = filter_sample(c->delay_filter, pd);
@@ -1395,30 +1373,18 @@ int clock_switch_phc(struct clock *c, int phc_index)
return 0;
}

-enum servo_state clock_synchronize(struct clock *c,
- struct timespec ingress_ts,
- struct timestamp origin_ts,
- Integer64 correction1,
- Integer64 correction2)
+enum servo_state clock_synchronize(struct clock *c, tmv_t ingress, tmv_t origin)
{
double adj;
- tmv_t ingress, origin;
enum servo_state state = SERVO_UNLOCKED;

- ingress = timespec_to_tmv(ingress_ts);
- origin = timestamp_to_tmv(origin_ts);
-
c->t1 = origin;
c->t2 = ingress;

- c->c1 = correction_to_tmv(correction1);
- c->c2 = correction_to_tmv(correction2);
-
/*
- * c->master_offset = ingress - origin - c->path_delay - c->c1 - c->c2;
+ * c->master_offset = ingress - origin - c->path_delay;
*/
- c->master_offset = tmv_sub(ingress,
- tmv_add(origin, tmv_add(c->path_delay, tmv_add(c->c1, c->c2))));
+ c->master_offset = tmv_sub(ingress, tmv_add(origin, c->path_delay));

if (!c->path_delay)
return state;
diff --git a/clock.h b/clock.h
index 4834464..b6df7a9 100644
--- a/clock.h
+++ b/clock.h
@@ -169,11 +169,10 @@ struct PortIdentity clock_parent_identity(struct clock *c);
* @param c The clock instance.
* @param req The transmission time of the delay request message.
* @param rx The reception time of the delay request message,
- * as reported in the delay response message.
- * @param correction The correction field from the delay response message.
+ * as reported in the delay response message, including
+ * correction.
*/
-void clock_path_delay(struct clock *c, struct timespec req, struct timestamp rx,
- Integer64 correction);
+void clock_path_delay(struct clock *c, tmv_t req, tmv_t rx);

/**
* Provide the estimated peer delay from a slave port.
@@ -215,18 +214,16 @@ int clock_switch_phc(struct clock *c, int phc_index);
/**
* Provide a data point to synchronize the clock.
* @param c The clock instance to synchronize.
- * @param ingress_ts The ingress time stamp on the sync message.
- * @param origin_ts The reported transmission time of the sync message.
+ * @param ingress The ingress time stamp on the sync message.
+ * @param origin The reported transmission time of the sync message,
+ including any corrections.
* @param correction1 The correction field of the sync message.
* @param correction2 The correction field of the follow up message.
* Pass zero in the case of one step operation.
* @return The state of the clock's servo.
*/
-enum servo_state clock_synchronize(struct clock *c,
- struct timespec ingress_ts,
- struct timestamp origin_ts,
- Integer64 correction1,
- Integer64 correction2);
+enum servo_state clock_synchronize(struct clock *c, tmv_t ingress,
+ tmv_t origin);

/**
* Inform a slaved clock about the master's sync interval.
diff --git a/port.c b/port.c
index 18a956d..77421aa 100644
--- a/port.c
+++ b/port.c
@@ -887,9 +887,8 @@ static int port_management_set(struct port *target,
return respond ? 1 : 0;
}

-static void port_nrate_calculate(struct port *p, tmv_t t3, tmv_t t4, tmv_t c)
+static void port_nrate_calculate(struct port *p, tmv_t origin, tmv_t ingress)
{
- tmv_t origin2;
struct nrate_estimator *n = &p->nrate;

/*
@@ -899,24 +898,23 @@ static void port_nrate_calculate(struct port *p, tmv_t t3, tmv_t t4, tmv_t c)
p->pdr_missing = 0;

if (!n->ingress1) {
- n->ingress1 = t4;
- n->origin1 = tmv_add(t3, c);
+ n->ingress1 = origin;
+ n->origin1 = ingress;
return;
}
n->count++;
if (n->count < n->max_count) {
return;
}
- origin2 = tmv_add(t3, c);
- if (tmv_eq(t4, n->ingress1)) {
+ if (tmv_eq(ingress, n->ingress1)) {
pr_warning("bad timestamps in nrate calculation");
return;
}
n->ratio =
- tmv_dbl(tmv_sub(origin2, n->origin1)) /
- tmv_dbl(tmv_sub(t4, n->ingress1));
- n->ingress1 = t4;
- n->origin1 = origin2;
+ tmv_dbl(tmv_sub(origin, n->origin1)) /
+ tmv_dbl(tmv_sub(ingress, n->ingress1));
+ n->ingress1 = ingress;
+ n->origin1 = origin;
n->count = 0;
n->ratio_valid = 1;
}
@@ -1012,11 +1010,17 @@ static void port_synchronize(struct port *p,
Integer64 correction1, Integer64 correction2)
{
enum servo_state state;
+ tmv_t t1, t1c, t2, c1, c2;

port_set_sync_rx_tmo(p);

- state = clock_synchronize(p->clock, ingress_ts, origin_ts,
- correction1, correction2);
+ t1 = timestamp_to_tmv(origin_ts);
+ t2 = timespec_to_tmv(ingress_ts);
+ c1 = correction_to_tmv(correction1);
+ c2 = correction_to_tmv(correction2);
+ t1c = tmv_add(t1, tmv_add(c1, c2));
+
+ state = clock_synchronize(p->clock, t2, t1c);
switch (state) {
case SERVO_UNLOCKED:
port_dispatch(p, EV_SYNCHRONIZATION_FAULT, 0);
@@ -1623,6 +1627,7 @@ static void process_delay_resp(struct port *p, struct ptp_message *m)
struct delay_req_msg *req;
struct delay_resp_msg *rsp = &m->delay_resp;
struct PortIdentity master;
+ tmv_t c3, t3, t4, t4c;

if (!p->delay_req)
return;
@@ -1639,8 +1644,12 @@ static void process_delay_resp(struct port *p, struct ptp_message *m)
if (!pid_eq(&master, &m->header.sourcePortIdentity))
return;

- clock_path_delay(p->clock, p->delay_req->hwts.ts, m->ts.pdu,
- m->header.correction);
+ c3 = correction_to_tmv(m->header.correction);
+ t3 = timespec_to_tmv(p->delay_req->hwts.ts);
+ t4 = timestamp_to_tmv(m->ts.pdu);
+ t4c = tmv_sub(t4, c3);
+
+ clock_path_delay(p->clock, t3, t4c);

if (p->logMinDelayReqInterval != rsp->hdr.logMessageInterval) {
// TODO - validate the input.
@@ -1789,7 +1798,7 @@ out:

static void port_peer_delay(struct port *p)
{
- tmv_t c1, c2, t1, t2, t3, t4, pd;
+ tmv_t c1, c2, t1, t2, t3, t3c, t4, pd;
struct ptp_message *req = p->peer_delay_req;
struct ptp_message *rsp = p->peer_delay_resp;
struct ptp_message *fup = p->peer_delay_fup;
@@ -1837,11 +1846,10 @@ static void port_peer_delay(struct port *p)
t3 = timestamp_to_tmv(fup->ts.pdu);
c2 = correction_to_tmv(fup->header.correction);
calc:
+ t3c = tmv_add(t3, tmv_add(c1, c2));
adj_t41 = p->nrate.ratio * clock_rate_ratio(p->clock) *
tmv_dbl(tmv_sub(t4, t1));
- pd = tmv_sub(dbl_tmv(adj_t41), tmv_sub(t3, t2));
- pd = tmv_sub(pd, c1);
- pd = tmv_sub(pd, c2);
+ pd = tmv_sub(dbl_tmv(adj_t41), tmv_sub(t3c, t2));
pd = tmv_div(pd, 2);

p->peer_delay = filter_sample(p->delay_filter, pd);
@@ -1851,7 +1859,7 @@ calc:
pr_debug("pdelay %hu %10" PRId64 "%10" PRId64, portnum(p), p->peer_delay, pd);

if (p->pod.follow_up_info)
- port_nrate_calculate(p, t3, t4, tmv_add(c1, c2));
+ port_nrate_calculate(p, t3c, t4);

if (p->state == PS_UNCALIBRATED || p->state == PS_SLAVE) {
clock_peer_delay(p->clock, p->peer_delay, p->nrate.ratio);

[Miroslav Lichvar]

Add new time stamp processing modes to return raw delay and offset based
on the raw delay instead of the long-term filtered delay, and to return
also a weight of the sample. The weight is set to the ratio between the
two delays. This gives smaller weight to samples where the sync and/or
delay messages were delayed significantly in the network and possibly
include a large error.

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
clock.c | 5 +++--
config.c | 24 ++++++++++++++++++++++++
config.h | 1 +
default.cfg | 1 +
ds.h | 2 ++
gPTP.cfg | 1 +
port.c | 3 ++-
ptp4l.8 | 8 ++++++++
ptp4l.c | 1 +
tsproc.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
tsproc.h | 17 +++++++++++++++--
11 files changed, 104 insertions(+), 10 deletions(-)

diff --git a/clock.c b/clock.c
index 9735fbd..71fda50 100644
--- a/clock.c
+++ b/clock.c
@@ -852,7 +852,8 @@ struct clock *clock_create(int phc_index, struct interfaces_head *ifaces,
}
c->servo_state = SERVO_UNLOCKED;
c->servo_type = servo;
- c->tsproc = tsproc_create(dds->delay_filter, dds->delay_filter_length);
+ c->tsproc = tsproc_create(dds->tsproc_mode, dds->delay_filter,
+ dds->delay_filter_length);
if (!c->tsproc) {
pr_err("Failed to create time stamp processor");
return NULL;
@@ -1357,7 +1358,7 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t ingress, tmv_t origin)

tsproc_down_ts(c->tsproc, origin, ingress);

- if (tsproc_update_offset(c->tsproc, &c->master_offset))
+ if (tsproc_update_offset(c->tsproc, &c->master_offset, NULL))
return state;

if (clock_utc_correct(c, ingress))
diff --git a/config.c b/config.c
index ec8c538..ee0f302 100644
--- a/config.c
+++ b/config.c
@@ -203,6 +203,18 @@ static enum parser_result parse_port_setting(const char *option,
else
return BAD_VALUE;

+ } else if (!strcmp(option, "tsproc_mode")) {
+ if (!strcasecmp("filter", value))
+ iface->tsproc_mode = TSPROC_FILTER;
+ else if (!strcasecmp("raw", value))
+ iface->tsproc_mode = TSPROC_RAW;
+ else if (!strcasecmp("filter_weight", value))
+ iface->tsproc_mode = TSPROC_FILTER_WEIGHT;
+ else if (!strcasecmp("raw_weight", value))
+ iface->tsproc_mode = TSPROC_RAW_WEIGHT;
+ else
+ return BAD_VALUE;
+
} else if (!strcmp(option, "delay_filter")) {
if (!strcasecmp("moving_average", value))
iface->delay_filter = FILTER_MOVING_AVERAGE;
@@ -566,6 +578,18 @@ static enum parser_result parse_global_setting(const char *option,
return r;
cfg->dds.time_source = val;

+ } else if (!strcmp(option, "tsproc_mode")) {
+ if (!strcasecmp("filter", value))
+ cfg->dds.tsproc_mode = TSPROC_FILTER;
+ else if (!strcasecmp("raw", value))
+ cfg->dds.tsproc_mode = TSPROC_RAW;
+ else if (!strcasecmp("filter_weight", value))
+ cfg->dds.tsproc_mode = TSPROC_FILTER_WEIGHT;
+ else if (!strcasecmp("raw_weight", value))
+ cfg->dds.tsproc_mode = TSPROC_RAW_WEIGHT;
+ else
+ return BAD_VALUE;
+
} else if (!strcmp(option, "delay_filter")) {
if (!strcasecmp("moving_average", value))
cfg->dds.delay_filter = FILTER_MOVING_AVERAGE;
diff --git a/config.h b/config.h
index c870e42..7bff11f 100644
--- a/config.h
+++ b/config.h
@@ -39,6 +39,7 @@ struct interface {
enum transport_type transport;
struct port_defaults pod;
struct sk_ts_info ts_info;
+ enum tsproc_mode tsproc_mode;
enum filter_type delay_filter;
int delay_filter_length;
int boundary_clock_jbod;
diff --git a/default.cfg b/default.cfg
index ec2ce58..b46c0f6 100644
--- a/default.cfg
+++ b/default.cfg
@@ -68,6 +68,7 @@ uds_address /var/run/ptp4l
network_transport UDPv4
delay_mechanism E2E
time_stamping hardware
+tsproc_mode filter
delay_filter moving_median
delay_filter_length 10
egressLatency 0
diff --git a/ds.h b/ds.h
index 8f44c3b..162687a 100644
--- a/ds.h
+++ b/ds.h
@@ -23,6 +23,7 @@
#include "ddt.h"
#include "fault.h"
#include "filter.h"
+#include "tsproc.h"

/* clock data sets */

@@ -59,6 +60,7 @@ struct default_ds {
int sanity_freq_limit;
int time_source;
struct clock_description clock_desc;
+ enum tsproc_mode tsproc_mode;
enum filter_type delay_filter;
int delay_filter_length;
int boundary_clock_jbod;
diff --git a/gPTP.cfg b/gPTP.cfg
index d917bd7..34fa238 100644
--- a/gPTP.cfg
+++ b/gPTP.cfg
@@ -67,6 +67,7 @@ uds_address /var/run/ptp4l
network_transport L2
delay_mechanism P2P
time_stamping hardware
+tsproc_mode filter
delay_filter moving_median
delay_filter_length 10
egressLatency 0
diff --git a/port.c b/port.c
index c1100d0..0a7b5a0 100644
--- a/port.c
+++ b/port.c
@@ -2534,7 +2534,8 @@ struct port *port_open(int phc_index,
p->delayMechanism = interface->dm;
p->versionNumber = PTP_VERSION;

- p->tsproc = tsproc_create(interface->delay_filter,
+ p->tsproc = tsproc_create(interface->tsproc_mode,
+ interface->delay_filter,
interface->delay_filter_length);
if (!p->tsproc) {
pr_err("Failed to create time stamp processor");
diff --git a/ptp4l.8 b/ptp4l.8
index 0d01f29..fd00ea3 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -197,6 +197,14 @@ greater than this value the port is marked as not 802.1AS capable.
Lower limit for peer delay in nanoseconds. If the estimated peer delay is
smaller than this value the port is marked as not 802.1AS capable.
.TP
+.B tsproc_mode
+Select the time stamp processing mode used to calculate offset and delay.
+Possible values are filter, raw, filter_weight, raw_weight. Raw modes perform
+well when the rate of sync messages (logSyncInterval) is similar to the rate of
+delay messages (logMinDelayReqInterval or logMinPdelayReqInterval). Weighting
+is useful with larger network jitters (e.g. software time stamping).
+The default is filter.
+.TP
.B delay_filter
Select the algorithm used to filter the measured delay and peer delay. Possible
values are moving_average and moving_median.
diff --git a/ptp4l.c b/ptp4l.c
index 1294d88..61c5854 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -73,6 +73,7 @@ static struct config cfg_settings = {
.userDescription = { .max_symbols = 128 },
.manufacturerIdentity = { 0, 0, 0 },
},
+ .tsproc_mode = TSPROC_FILTER,
.delay_filter = FILTER_MOVING_MEDIAN,
.delay_filter_length = 10,
.boundary_clock_jbod = 0,
diff --git a/tsproc.c b/tsproc.c
index 3be9f0e..53588ef 100644
--- a/tsproc.c
+++ b/tsproc.c
@@ -25,6 +25,10 @@
#include "print.h"

struct tsproc {
+ /* Processing options */
+ int raw_mode;
+ int weighting;
+
/* Current ratio between remote and local clock frequency */
double clock_rate_ratio;

@@ -40,7 +44,8 @@ struct tsproc {
struct filter *delay_filter;
};

-struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length)
+struct tsproc *tsproc_create(enum tsproc_mode mode,
+ enum filter_type delay_filter, int filter_length)
{
struct tsproc *tsp;

@@ -48,6 +53,28 @@ struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length)
if (!tsp)
return NULL;

+ switch (mode) {
+ case TSPROC_FILTER:
+ tsp->raw_mode = 0;
+ tsp->weighting = 0;
+ break;
+ case TSPROC_RAW:
+ tsp->raw_mode = 1;
+ tsp->weighting = 0;
+ break;
+ case TSPROC_FILTER_WEIGHT:
+ tsp->raw_mode = 0;
+ tsp->weighting = 1;
+ break;
+ case TSPROC_RAW_WEIGHT:
+ tsp->raw_mode = 1;
+ tsp->weighting = 1;
+ break;
+ default:
+ free(tsp);
+ return NULL;
+ }
+
tsp->delay_filter = filter_create(delay_filter, filter_length);
if (!tsp->delay_filter) {
free(tsp);
@@ -124,14 +151,14 @@ int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay)
tsp->filtered_delay, raw_delay);

if (delay)
- *delay = tsp->filtered_delay;
+ *delay = tsp->raw_mode ? raw_delay : tsp->filtered_delay;

return 0;
}

-int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset)
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset, double *weight)
{
- tmv_t delay;
+ tmv_t delay, raw_delay = 0;

if (!tsp->t1 || !tsp->t2 || !tsp->t3 || !tsp->t4)
return -1;
@@ -139,11 +166,25 @@ int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset)
if (!offset)
return 0;

- delay = tsp->filtered_delay;
+ if (tsp->raw_mode || tsp->weighting)
+ raw_delay = get_raw_delay(tsp);
+
+ delay = tsp->raw_mode ? raw_delay : tsp->filtered_delay;

/* offset = t2 - t1 - delay */
*offset = tmv_sub(tmv_sub(tsp->t2, tsp->t1), delay);

+ if (!weight)
+ return 0;
+
+ if (tsp->weighting && tsp->filtered_delay > 0 && raw_delay > 0) {
+ *weight = (double)tsp->filtered_delay / raw_delay;
+ if (*weight > 1.0)
+ *weight = 1.0;
+ } else {
+ *weight = 1.0;
+ }
+
return 0;
}

diff --git a/tsproc.h b/tsproc.h
index b8b2ffd..247dc29 100644
--- a/tsproc.h
+++ b/tsproc.h
@@ -26,12 +26,24 @@
struct tsproc;

/**
+ * Defines the available modes.
+ */
+enum tsproc_mode {
+ TSPROC_FILTER,
+ TSPROC_RAW,
+ TSPROC_FILTER_WEIGHT,
+ TSPROC_RAW_WEIGHT,
+};
+
+/**
* Create a new instance of the time stamp processor.
+ * @param mode Time stamp processing mode.
* @param delay_filter Type of the filter that will be applied to delay.
* @param filter_length Length of the filter.
* @return A pointer to a new tsproc on success, NULL otherwise.
*/
-struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length);
+struct tsproc *tsproc_create(enum tsproc_mode mode,
+ enum filter_type delay_filter, int filter_length);

/**
* Destroy a time stamp processor.
@@ -82,9 +94,10 @@ int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay);
* Update offset in a time stamp processor using new measurements.
* @param tsp Pointer obtained via @ref tsproc_create().
* @param offset A pointer to store the new offset, may be NULL.
+ * @param weight A pointer to store the weight of the sample, may be NULL.
* @return 0 on success, -1 when missing a measurement.
*/
-int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset);
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset, double *weight);

/**
* Reset a time stamp processor.

[Richard Cochran]

Post by Miroslav Lichvar
Add new time stamp processing modes to return raw delay and offset based
on the raw delay instead of the long-term filtered delay, and to return
also a weight of the sample. The weight is set to the ratio between the
two delays. This gives smaller weight to samples where the sync and/or
delay messages were delayed significantly in the network and possibly
include a large error.
---

Patches 3 to 6 all look good to me.

Thanks,
Richard

[Miroslav Lichvar]

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
pi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pi.c b/pi.c
index 9c7b148..e0116fe 100644
--- a/pi.c
+++ b/pi.c
@@ -137,8 +137,8 @@ static double pi_sample(struct servo *servo,
break;
}

- ki_term = s->ki * offset;
- ppb = s->kp * offset + s->drift + ki_term;
+ ki_term = s->ki * offset * weight;
+ ppb = s->kp * offset * weight + s->drift + ki_term;
if (ppb < -servo->max_frequency) {
ppb = -servo->max_frequency;
} else if (ppb > servo->max_frequency) {

[Miroslav Lichvar]

Add weight parameter to the sample function. Samples with smaller weight
are less reliable, they can be ignored by the servo or the adjustments
of the clock can be smaller.

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
clock.c | 6 +++---
linreg.c | 1 +
ntpshm.c | 1 +
phc2sys.c | 2 +-
pi.c | 1 +
servo.c | 3 ++-
servo.h | 3 +++
servo_private.h | 2 +-
8 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/clock.c b/clock.c
index 71fda50..3350b3d 100644
--- a/clock.c
+++ b/clock.c
@@ -1351,14 +1351,14 @@ int clock_switch_phc(struct clock *c, int phc_index)

enum servo_state clock_synchronize(struct clock *c, tmv_t ingress, tmv_t origin)
{
- double adj;
+ double adj, weight;
enum servo_state state = SERVO_UNLOCKED;

c->ingress_ts = ingress;

tsproc_down_ts(c->tsproc, origin, ingress);

- if (tsproc_update_offset(c->tsproc, &c->master_offset, NULL))
+ if (tsproc_update_offset(c->tsproc, &c->master_offset, &weight))
return state;

if (clock_utc_correct(c, ingress))
@@ -1370,7 +1370,7 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t ingress, tmv_t origin)
return clock_no_adjust(c, ingress, origin);

adj = servo_sample(c->servo, tmv_to_nanoseconds(c->master_offset),
- tmv_to_nanoseconds(ingress), &state);
+ tmv_to_nanoseconds(ingress), weight, &state);
c->servo_state = state;

if (c->stats.max_count > 1) {
diff --git a/linreg.c b/linreg.c
index fde604d..3f7fe9a 100644
--- a/linreg.c
+++ b/linreg.c
@@ -209,6 +209,7 @@ static int get_best_size(struct linreg_servo *s)
static double linreg_sample(struct servo *servo,
int64_t offset,
uint64_t local_ts,
+ double weight,
enum servo_state *state)
{
struct linreg_servo *s = container_of(servo, struct linreg_servo, servo);
diff --git a/ntpshm.c b/ntpshm.c
index 21a11cf..8b18e2d 100644
--- a/ntpshm.c
+++ b/ntpshm.c
@@ -80,6 +80,7 @@ static void ntpshm_destroy(struct servo *servo)
static double ntpshm_sample(struct servo *servo,
int64_t offset,
uint64_t local_ts,
+ double weight,
enum servo_state *state)
{
struct ntpshm_servo *s = container_of(servo, struct ntpshm_servo, servo);
diff --git a/phc2sys.c b/phc2sys.c
index 23993ac..9ff5bf9 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -469,7 +469,7 @@ static void update_clock(struct node *node, struct clock *clock,
if (clock->sanity_check && clockcheck_sample(clock->sanity_check, ts))
servo_reset(clock->servo);

- ppb = servo_sample(clock->servo, offset, ts, &state);
+ ppb = servo_sample(clock->servo, offset, ts, 1.0, &state);
clock->servo_state = state;

switch (state) {
diff --git a/pi.c b/pi.c
index c8b8587..9c7b148 100644
--- a/pi.c
+++ b/pi.c
@@ -64,6 +64,7 @@ static void pi_destroy(struct servo *servo)
static double pi_sample(struct servo *servo,
int64_t offset,
uint64_t local_ts,
+ double weight,
enum servo_state *state)
{
struct pi_servo *s = container_of(servo, struct pi_servo, servo);
diff --git a/servo.c b/servo.c
index f200f75..2739ebd 100644
--- a/servo.c
+++ b/servo.c
@@ -78,11 +78,12 @@ void servo_destroy(struct servo *servo)
double servo_sample(struct servo *servo,
int64_t offset,
uint64_t local_ts,
+ double weight,
enum servo_state *state)
{
double r;

- r = servo->sample(servo, offset, local_ts, state);
+ r = servo->sample(servo, offset, local_ts, weight, state);

if (*state != SERVO_UNLOCKED)
servo->first_update = 0;
diff --git a/servo.h b/servo.h
index e054501..4be8c23 100644
--- a/servo.h
+++ b/servo.h
@@ -104,12 +104,15 @@ void servo_destroy(struct servo *servo);
* @param servo Pointer to a servo obtained via @ref servo_create().
* @param offset The estimated clock offset in nanoseconds.
* @param local_ts The local time stamp of the sample in nanoseconds.
+ * @param weight The weight of the sample, larger if more reliable,
+ * 1.0 is the maximum value.
* @param state Returns the servo's state.
* @return The clock adjustment in parts per billion.
*/
double servo_sample(struct servo *servo,
int64_t offset,
uint64_t local_ts,
+ double weight,
enum servo_state *state);

/**
diff --git a/servo_private.h b/servo_private.h
index 9a1a459..b8c3c98 100644
--- a/servo_private.h
+++ b/servo_private.h
@@ -30,7 +30,7 @@ struct servo {
void (*destroy)(struct servo *servo);

double (*sample)(struct servo *servo,
- int64_t offset, uint64_t local_ts,
+ int64_t offset, uint64_t local_ts, double weight,
enum servo_state *state);

void (*sync_interval)(struct servo *servo, double interval);

[Miroslav Lichvar]

Introduce a time stamp processor for offset/delay calculations and use
it in the clock and port modules.

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
clock.c | 100 ++++++++++++++-------------------------
clock.h | 5 +-
makefile | 2 +-
port.c | 39 ++++++++--------
tsproc.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tsproc.h | 97 ++++++++++++++++++++++++++++++++++++++
6 files changed, 318 insertions(+), 86 deletions(-)
create mode 100644 tsproc.c
create mode 100644 tsproc.h

diff --git a/clock.c b/clock.c
index f5349b9..9735fbd 100644
--- a/clock.c
+++ b/clock.c
@@ -38,6 +38,7 @@
#include "stats.h"
#include "print.h"
#include "tlv.h"
+#include "tsproc.h"
#include "uds.h"
#include "util.h"

@@ -102,12 +103,11 @@ struct clock {
enum servo_state servo_state;
tmv_t master_offset;
tmv_t path_delay;
- struct filter *delay_filter;
+ tmv_t ingress_ts;
+ struct tsproc *tsproc;
struct freq_estimator fest;
struct time_status_np status;
double nrr;
- tmv_t t1;
- tmv_t t2;
struct clock_description desc;
struct clock_stats stats;
int stats_interval;
@@ -271,7 +271,7 @@ void clock_destroy(struct clock *c)
phc_close(c->clkid);
}
servo_destroy(c->servo);
- filter_destroy(c->delay_filter);
+ tsproc_destroy(c->tsproc);
stats_destroy(c->stats.offset);
stats_destroy(c->stats.freq);
stats_destroy(c->stats.delay);
@@ -413,7 +413,7 @@ static int clock_management_fill_response(struct clock *c, struct port *p,
case TLV_TIME_STATUS_NP:
tsn = (struct time_status_np *) tlv->data;
tsn->master_offset = c->master_offset;
- tsn->ingress_time = tmv_to_nanoseconds(c->t2);
+ tsn->ingress_time = tmv_to_nanoseconds(c->ingress_ts);
tsn->cumulativeScaledRateOffset =
(Integer32) (c->status.cumulativeScaledRateOffset +
c->nrr * POW2_41 - POW2_41);
@@ -543,7 +543,8 @@ static void clock_stats_update(struct clock_stats *s,
stats_reset(s->delay);
}

-static enum servo_state clock_no_adjust(struct clock *c)
+static enum servo_state clock_no_adjust(struct clock *c, tmv_t ingress,
+ tmv_t origin)
{
double fui;
double ratio, freq;
@@ -561,21 +562,21 @@ static enum servo_state clock_no_adjust(struct clock *c)
* error caused by our imperfect path delay measurement.
*/
if (!f->ingress1) {
- f->ingress1 = c->t2;
- f->origin1 = c->t1;
+ f->ingress1 = ingress;
+ f->origin1 = origin;
return state;
}
f->count++;
if (f->count < f->max_count) {
return state;
}
- if (tmv_eq(c->t2, f->ingress1)) {
+ if (tmv_eq(ingress, f->ingress1)) {
pr_warning("bad timestamps in rate ratio calculation");
return state;
}

- ratio = tmv_dbl(tmv_sub(c->t1, f->origin1)) /
- tmv_dbl(tmv_sub(c->t2, f->ingress1));
+ ratio = tmv_dbl(tmv_sub(origin, f->origin1)) /
+ tmv_dbl(tmv_sub(ingress, f->ingress1));
freq = (1.0 - ratio) * 1e9;

if (c->stats.max_count > 1) {
@@ -597,8 +598,8 @@ static enum servo_state clock_no_adjust(struct clock *c)
pr_debug("master/local %.9f", ratio);
pr_debug("diff %+.9f", ratio - (fui + c->nrr - 1.0));

- f->ingress1 = c->t2;
- f->origin1 = c->t1;
+ f->ingress1 = ingress;
+ f->origin1 = origin;
f->count = 0;

return state;
@@ -851,10 +852,9 @@ struct clock *clock_create(int phc_index, struct interfaces_head *ifaces,
}
c->servo_state = SERVO_UNLOCKED;
c->servo_type = servo;
- c->delay_filter = filter_create(dds->delay_filter,
- dds->delay_filter_length);
- if (!c->delay_filter) {
- pr_err("Failed to create delay filter");
+ c->tsproc = tsproc_create(dds->delay_filter, dds->delay_filter_length);
+ if (!c->tsproc) {
+ pr_err("Failed to create time stamp processor");
return NULL;
}
c->nrr = 1.0;
@@ -1278,52 +1278,27 @@ int clock_poll(struct clock *c)

void clock_path_delay(struct clock *c, tmv_t req, tmv_t rx)
{
- tmv_t pd, t1, t2, t3, t4;
- double rr;
+ tsproc_up_ts(c->tsproc, req, rx);

- if (tmv_is_zero(c->t1))
+ if (tsproc_update_delay(c->tsproc, &c->path_delay))
return;

- t1 = c->t1;
- t2 = c->t2;
- t3 = req;
- t4 = rx;
- rr = clock_rate_ratio(c);
-
- /*
- * c->path_delay = (t2 - t3) * rr + (t4 - t1);
- * c->path_delay /= 2.0;
- */
-
- pd = tmv_sub(t2, t3);
- if (rr != 1.0)
- pd = dbl_tmv(tmv_dbl(pd) * rr);
- pd = tmv_add(pd, tmv_sub(t4, t1));
- pd = tmv_div(pd, 2);
-
- if (pd < 0) {
- pr_debug("negative path delay %10" PRId64, pd);
- pr_debug("path_delay = (t2 - t3) * rr + (t4 - t1)");
- pr_debug("t2 - t3 = %+10" PRId64, t2 - t3);
- pr_debug("t4 - t1 = %+10" PRId64, t4 - t1);
- pr_debug("rr = %.9f", rr);
- }
-
- c->path_delay = filter_sample(c->delay_filter, pd);
-
c->cur.meanPathDelay = tmv_to_TimeInterval(c->path_delay);

- pr_debug("path delay %10" PRId64 " %10" PRId64, c->path_delay, pd);
-
if (c->stats.delay)
- stats_add_value(c->stats.delay, tmv_to_nanoseconds(pd));
+ stats_add_value(c->stats.delay,
+ tmv_to_nanoseconds(c->path_delay));
}

-void clock_peer_delay(struct clock *c, tmv_t ppd, double nrr)
+void clock_peer_delay(struct clock *c, tmv_t ppd, tmv_t req, tmv_t rx,
+ double nrr)
{
c->path_delay = ppd;
c->nrr = nrr;

+ tsproc_set_delay(c->tsproc, ppd);
+ tsproc_up_ts(c->tsproc, req, rx);
+
if (c->stats.delay)
stats_add_value(c->stats.delay, tmv_to_nanoseconds(ppd));
}
@@ -1378,15 +1353,11 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t ingress, tmv_t origin)
double adj;
enum servo_state state = SERVO_UNLOCKED;

- c->t1 = origin;
- c->t2 = ingress;
+ c->ingress_ts = ingress;

- /*
- * c->master_offset = ingress - origin - c->path_delay;
- */
- c->master_offset = tmv_sub(ingress, tmv_add(origin, c->path_delay));
+ tsproc_down_ts(c->tsproc, origin, ingress);

- if (!c->path_delay)
+ if (tsproc_update_offset(c->tsproc, &c->master_offset))
return state;

if (clock_utc_correct(c, ingress))
@@ -1395,7 +1366,7 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t ingress, tmv_t origin)
c->cur.offsetFromMaster = tmv_to_TimeInterval(c->master_offset);

if (c->free_running)
- return clock_no_adjust(c);
+ return clock_no_adjust(c, ingress, origin);

adj = servo_sample(c->servo, tmv_to_nanoseconds(c->master_offset),
tmv_to_nanoseconds(ingress), &state);
@@ -1411,19 +1382,21 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t ingress, tmv_t origin)
tmv_to_nanoseconds(c->path_delay));
}

+ tsproc_set_clock_rate_ratio(c->tsproc, clock_rate_ratio(c));
+
switch (state) {
case SERVO_UNLOCKED:
break;
case SERVO_JUMP:
clockadj_set_freq(c->clkid, -adj);
clockadj_step(c->clkid, -tmv_to_nanoseconds(c->master_offset));
- c->t1 = tmv_zero();
- c->t2 = tmv_zero();
+ c->ingress_ts = tmv_zero();
if (c->sanity_check) {
clockcheck_set_freq(c->sanity_check, -adj);
clockcheck_step(c->sanity_check,
-tmv_to_nanoseconds(c->master_offset));
}
+ tsproc_reset(c->tsproc, 0);
break;
case SERVO_LOCKED:
clockadj_set_freq(c->clkid, -adj);
@@ -1497,9 +1470,8 @@ static void handle_state_decision_event(struct clock *c)

if (!cid_eq(&best_id, &c->best_id)) {
clock_freq_est_reset(c);
- filter_reset(c->delay_filter);
- c->t1 = tmv_zero();
- c->t2 = tmv_zero();
+ tsproc_reset(c->tsproc, 1);
+ c->ingress_ts = tmv_zero();
c->path_delay = 0;
c->nrr = 1.0;
fresh_best = 1;
diff --git a/clock.h b/clock.h
index b6df7a9..a8286dd 100644
--- a/clock.h
+++ b/clock.h
@@ -178,9 +178,12 @@ void clock_path_delay(struct clock *c, tmv_t req, tmv_t rx);
* Provide the estimated peer delay from a slave port.
* @param c The clock instance.
* @param ppd The peer delay as measured on a slave port.
+ * @param req The transmission time of the pdelay request message.
+ * @param rx The reception time of the pdelay request message.
* @param nrr The neighbor rate ratio as measured on a slave port.
*/
-void clock_peer_delay(struct clock *c, tmv_t ppd, double nrr);
+void clock_peer_delay(struct clock *c, tmv_t ppd, tmv_t req, tmv_t rx,
+ double nrr);

/**
* Poll for events and dispatch them.
diff --git a/makefile b/makefile
index 5469301..ca82f5e 100644
--- a/makefile
+++ b/makefile
@@ -26,7 +26,7 @@ PRG = ptp4l pmc phc2sys hwstamp_ctl phc_ctl timemaster
OBJ = bmc.o clock.o clockadj.o clockcheck.o config.o fault.o \
filter.o fsm.o linreg.o mave.o mmedian.o msg.o ntpshm.o phc.o \
pi.o port.o print.o ptp4l.o raw.o servo.o sk.o stats.o tlv.o \
- transport.o udp.o udp6.o uds.o util.o version.o
+ transport.o tsproc.o udp.o udp6.o uds.o util.o version.o

OBJECTS = $(OBJ) hwstamp_ctl.o phc2sys.o phc_ctl.o pmc.o pmc_common.o \
sysoff.o timemaster.o
diff --git a/port.c b/port.c
index 77421aa..c1100d0 100644
--- a/port.c
+++ b/port.c
@@ -35,6 +35,7 @@
#include "sk.h"
#include "tlv.h"
#include "tmv.h"
+#include "tsproc.h"
#include "util.h"

#define ALLOWED_LOST_RESPONSES 3
@@ -86,7 +87,7 @@ struct port {
UInteger16 sync;
} seqnum;
tmv_t peer_delay;
- struct filter *delay_filter;
+ struct tsproc *tsproc;
int log_sync_interval;
struct nrate_estimator nrate;
unsigned int pdr_missing;
@@ -1798,11 +1799,10 @@ out:

static void port_peer_delay(struct port *p)
{
- tmv_t c1, c2, t1, t2, t3, t3c, t4, pd;
+ tmv_t c1, c2, t1, t2, t3, t3c, t4;
struct ptp_message *req = p->peer_delay_req;
struct ptp_message *rsp = p->peer_delay_resp;
struct ptp_message *fup = p->peer_delay_fup;
- double adj_t41;

/* Check for response, validate port and sequence number. */

@@ -1847,22 +1847,21 @@ static void port_peer_delay(struct port *p)
c2 = correction_to_tmv(fup->header.correction);
calc:
t3c = tmv_add(t3, tmv_add(c1, c2));
- adj_t41 = p->nrate.ratio * clock_rate_ratio(p->clock) *
- tmv_dbl(tmv_sub(t4, t1));
- pd = tmv_sub(dbl_tmv(adj_t41), tmv_sub(t3c, t2));
- pd = tmv_div(pd, 2);
-
- p->peer_delay = filter_sample(p->delay_filter, pd);
+ tsproc_set_clock_rate_ratio(p->tsproc, p->nrate.ratio *
+ clock_rate_ratio(p->clock));
+ tsproc_up_ts(p->tsproc, t1, t2);
+ tsproc_down_ts(p->tsproc, t3c, t4);
+ if (tsproc_update_delay(p->tsproc, &p->peer_delay))
+ return;

p->peerMeanPathDelay = tmv_to_TimeInterval(p->peer_delay);

- pr_debug("pdelay %hu %10" PRId64 "%10" PRId64, portnum(p), p->peer_delay, pd);
-
if (p->pod.follow_up_info)
port_nrate_calculate(p, t3c, t4);

if (p->state == PS_UNCALIBRATED || p->state == PS_SLAVE) {
- clock_peer_delay(p->clock, p->peer_delay, p->nrate.ratio);
+ clock_peer_delay(p->clock, p->peer_delay, t1, t2,
+ p->nrate.ratio);
}

msg_put(p->peer_delay_req);
@@ -1981,7 +1980,7 @@ void port_close(struct port *p)
port_disable(p);
}
transport_destroy(p->trp);
- filter_destroy(p->delay_filter);
+ tsproc_destroy(p->tsproc);
if (p->fault_fd >= 0)
close(p->fault_fd);
free(p);
@@ -2535,10 +2534,10 @@ struct port *port_open(int phc_index,
p->delayMechanism = interface->dm;
p->versionNumber = PTP_VERSION;

- p->delay_filter = filter_create(interface->delay_filter,
- interface->delay_filter_length);
- if (!p->delay_filter) {
- pr_err("Failed to create delay filter");
+ p->tsproc = tsproc_create(interface->delay_filter,
+ interface->delay_filter_length);
+ if (!p->tsproc) {
+ pr_err("Failed to create time stamp processor");
goto err_transport;
}
p->nrate.ratio = 1.0;
@@ -2549,13 +2548,13 @@ struct port *port_open(int phc_index,
p->fault_fd = timerfd_create(CLOCK_MONOTONIC, 0);
if (p->fault_fd < 0) {
pr_err("timerfd_create failed: %m");
- goto err_filter;
+ goto err_tsproc;
}
}
return p;

-err_filter:
- filter_destroy(p->delay_filter);
+err_tsproc:
+ tsproc_destroy(p->tsproc);
err_transport:
transport_destroy(p->trp);
err_port:
diff --git a/tsproc.c b/tsproc.c
new file mode 100644
index 0000000..3be9f0e
--- /dev/null
+++ b/tsproc.c
@@ -0,0 +1,161 @@
+/**
+ * @file tsproc.c
+ * @note Copyright (C) 2015 Miroslav Lichvar <***@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <stdlib.h>
+#include <inttypes.h>
+
+#include "tsproc.h"
+#include "filter.h"
+#include "print.h"
+
+struct tsproc {
+ /* Current ratio between remote and local clock frequency */
+ double clock_rate_ratio;
+
+ /* Latest down measurement */
+ tmv_t t1, t2;
+ /* Latest up measurement */
+ tmv_t t3, t4;
+
+ /* Current filtered delay */
+ tmv_t filtered_delay;
+
+ /* Delay filter */
+ struct filter *delay_filter;
+};
+
+struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length)
+{
+ struct tsproc *tsp;
+
+ tsp = calloc(1, sizeof(*tsp));
+ if (!tsp)
+ return NULL;
+
+ tsp->delay_filter = filter_create(delay_filter, filter_length);
+ if (!tsp->delay_filter) {
+ free(tsp);
+ return NULL;
+ }
+
+ tsp->clock_rate_ratio = 1.0;
+
+ return tsp;
+}
+
+void tsproc_destroy(struct tsproc *tsp)
+{
+ filter_destroy(tsp->delay_filter);
+ free(tsp);
+}
+
+void tsproc_down_ts(struct tsproc *tsp, tmv_t remote_ts, tmv_t local_ts)
+{
+ tsp->t1 = remote_ts;
+ tsp->t2 = local_ts;
+}
+
+void tsproc_up_ts(struct tsproc *tsp, tmv_t local_ts, tmv_t remote_ts)
+{
+ tsp->t3 = local_ts;
+ tsp->t4 = remote_ts;
+}
+
+void tsproc_set_clock_rate_ratio(struct tsproc *tsp, double clock_rate_ratio)
+{
+ tsp->clock_rate_ratio = clock_rate_ratio;
+}
+
+void tsproc_set_delay(struct tsproc *tsp, tmv_t delay)
+{
+ tsp->filtered_delay = delay;
+}
+
+tmv_t get_raw_delay(struct tsproc *tsp)
+{
+ tmv_t t23, t41, delay;
+
+ /* delay = ((t2 - t3) * rr + (t4 - t1)) / 2 */
+
+ t23 = tmv_sub(tsp->t2, tsp->t3);
+ if (tsp->clock_rate_ratio != 1.0)
+ t23 = dbl_tmv(tmv_dbl(t23) * tsp->clock_rate_ratio);
+ t41 = tmv_sub(tsp->t4, tsp->t1);
+ delay = tmv_div(tmv_add(t23, t41), 2);
+
+ if (delay < 0) {
+ pr_debug("negative delay %10" PRId64, delay);
+ pr_debug("delay = (t2 - t3) * rr + (t4 - t1)");
+ pr_debug("t2 - t3 = %+10" PRId64, t23);
+ pr_debug("t4 - t1 = %+10" PRId64, t41);
+ pr_debug("rr = %.9f", tsp->clock_rate_ratio);
+ }
+
+ return delay;
+}
+
+int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay)
+{
+ tmv_t raw_delay;
+
+ if (!tsp->t1 || !tsp->t2 || !tsp->t3 || !tsp->t4)
+ return -1;
+
+ raw_delay = get_raw_delay(tsp);
+ tsp->filtered_delay = filter_sample(tsp->delay_filter, raw_delay);
+
+ pr_debug("delay filtered %10" PRId64 " raw %10" PRId64,
+ tsp->filtered_delay, raw_delay);
+
+ if (delay)
+ *delay = tsp->filtered_delay;
+
+ return 0;
+}
+
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset)
+{
+ tmv_t delay;
+
+ if (!tsp->t1 || !tsp->t2 || !tsp->t3 || !tsp->t4)
+ return -1;
+
+ if (!offset)
+ return 0;
+
+ delay = tsp->filtered_delay;
+
+ /* offset = t2 - t1 - delay */
+ *offset = tmv_sub(tmv_sub(tsp->t2, tsp->t1), delay);
+
+ return 0;
+}
+
+void tsproc_reset(struct tsproc *tsp, int full)
+{
+ tsp->t1 = tmv_zero();
+ tsp->t2 = tmv_zero();
+ tsp->t3 = tmv_zero();
+ tsp->t4 = tmv_zero();
+
+ if (full) {
+ tsp->clock_rate_ratio = 1.0;
+ filter_reset(tsp->delay_filter);
+ }
+}
diff --git a/tsproc.h b/tsproc.h
new file mode 100644
index 0000000..b8b2ffd
--- /dev/null
+++ b/tsproc.h
@@ -0,0 +1,97 @@
+/**
+ * @file tsproc.h
+ * @brief Time stamp processor.
+ * @note Copyright (C) 2015 Miroslav Lichvar <***@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#ifndef HAVE_TSPROC_H
+#define HAVE_TSPROC_H
+
+#include "filter.h"
+
+/** Opaque type */
+struct tsproc;
+
+/**
+ * Create a new instance of the time stamp processor.
+ * @param delay_filter Type of the filter that will be applied to delay.
+ * @param filter_length Length of the filter.
+ * @return A pointer to a new tsproc on success, NULL otherwise.
+ */
+struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length);
+
+/**
+ * Destroy a time stamp processor.
+ * @param tsp Pointer obtained via @ref tsproc_create().
+ */
+void tsproc_destroy(struct tsproc *tsp);
+
+/**
+ * Feed a downstream measurement into a time stamp processor.
+ * @param tsp Pointer obtained via @ref tsproc_create().
+ * @param remote_ts The remote transmission time.
+ * @param local_ts The local reception time.
+ */
+void tsproc_down_ts(struct tsproc *tsp, tmv_t remote_ts, tmv_t local_ts);
+
+/**
+ * Feed an upstream measurement into a time stamp processor.
+ * @param tsp Pointer obtained via @ref tsproc_create().
+ * @param local_ts The local transmission time.
+ * @param remote_ts The remote reception time.
+ */
+void tsproc_up_ts(struct tsproc *tsp, tmv_t local_ts, tmv_t remote_ts);
+
+/**
+ * Set ratio between remote and local clock frequencies.
+ * @param tsp Pointer obtained via @ref tsproc_create().
+ * @param clock_rate_ratio The ratio between frequencies.
+ */
+void tsproc_set_clock_rate_ratio(struct tsproc *tsp, double clock_rate_ratio);
+
+/**
+ * Set delay in a time stamp processor. This can be used to override the last
+ * calculated value.
+ * @param tsp Pointer obtained via @ref tsproc_create().
+ * @param delay The new delay.
+ */
+void tsproc_set_delay(struct tsproc *tsp, tmv_t delay);
+
+/**
+ * Update delay in a time stamp processor using new measurements.
+ * @param tsp Pointer obtained via @ref tsproc_create().
+ * @param delay A pointer to store the new delay, may be NULL.
+ * @return 0 on success, -1 when missing a measurement.
+ */
+int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay);
+
+/**
+ * Update offset in a time stamp processor using new measurements.
+ * @param tsp Pointer obtained via @ref tsproc_create().
+ * @param offset A pointer to store the new offset, may be NULL.
+ * @return 0 on success, -1 when missing a measurement.
+ */
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset);
+
+/**
+ * Reset a time stamp processor.
+ * @param tsp Pointer obtained via @ref tsproc_create().
+ * @param full 0 to reset stored measurements (e.g. after clock was stepped),
+ * 1 to reset everything (e.g. when remote clock changed).
+ */
+void tsproc_reset(struct tsproc *tsp, int full);
+
+#endif

[Richard Cochran]

Few gripes, and a question...

Post by Miroslav Lichvar
+struct tsproc {
+ /* Current ratio between remote and local clock frequency */
+ double clock_rate_ratio;
+
+ /* Latest down measurement */
+ tmv_t t1, t2;
+ /* Latest up measurement */
+ tmv_t t3, t4;

One field per line, please.

Post by Miroslav Lichvar
+
+ /* Current filtered delay */
+ tmv_t filtered_delay;
+
+ /* Delay filter */
+ struct filter *delay_filter;
+};
+int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay)
+{
+ tmv_t raw_delay;
+
+ if (!tsp->t1 || !tsp->t2 || !tsp->t3 || !tsp->t4)
+ return -1;

Use tmv_zero() here ...

Post by Miroslav Lichvar
+ raw_delay = get_raw_delay(tsp);
+ tsp->filtered_delay = filter_sample(tsp->delay_filter, raw_delay);
+
+ pr_debug("delay filtered %10" PRId64 " raw %10" PRId64,
+ tsp->filtered_delay, raw_delay);
+
+ if (delay)
+ *delay = tsp->filtered_delay;
+
+ return 0;
+}
+
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset)
+{
+ tmv_t delay;
+
+ if (!tsp->t1 || !tsp->t2 || !tsp->t3 || !tsp->t4)
+ return -1;

and here.

Post by Miroslav Lichvar
+ if (!offset)
+ return 0;

What is point of allowing 'offset' to be a null pointer?

Post by Miroslav Lichvar
+ delay = tsp->filtered_delay;
+
+ /* offset = t2 - t1 - delay */
+ *offset = tmv_sub(tmv_sub(tsp->t2, tsp->t1), delay);
+
+ return 0;
+}

Thanks,
Richard

[Richard Cochran]

Post by Richard Cochran

Post by Miroslav Lichvar
+int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay)
+{
+ tmv_t raw_delay;
+
+ if (!tsp->t1 || !tsp->t2 || !tsp->t3 || !tsp->t4)
+ return -1;

Use tmv_zero() here ...

Rather, tmv_is_zero().

Thanks,
Richard

[Miroslav Lichvar]

Post by Richard Cochran

Post by Miroslav Lichvar
+int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay)
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset)
+ if (!offset)
+ return 0;

What is point of allowing 'offset' to be a null pointer?

That was for consistency with the tsproc_update_delay function, there
is one call with NULL delay. For offset that probably won't be needed
and it doesn't make much sense anyway.

[Miroslav Lichvar]

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
linreg.c | 48 ++++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/linreg.c b/linreg.c
index 3f7fe9a..8f354f4 100644
--- a/linreg.c
+++ b/linreg.c
@@ -42,6 +42,7 @@
struct point {
uint64_t x;
uint64_t y;
+ double w;
};

struct result {
@@ -70,6 +71,8 @@ struct linreg_servo {
uint64_t last_update;
/* Regression results for all sizes */
struct result results[MAX_SIZE - MIN_SIZE + 1];
+ /* Selected size */
+ unsigned int size;
/* Current frequency offset of the clock */
double clock_freq;
/* Expected interval between updates */
@@ -120,12 +123,13 @@ static void update_reference(struct linreg_servo *s, uint64_t local_ts)
s->last_update = local_ts;
}

-static void add_sample(struct linreg_servo *s, int64_t offset)
+static void add_sample(struct linreg_servo *s, int64_t offset, double weight)
{
s->last_point = (s->last_point + 1) % MAX_POINTS;

s->points[s->last_point].x = s->reference.x;
s->points[s->last_point].y = s->reference.y - offset;
+ s->points[s->last_point].w = weight;

if (s->num_points < MAX_POINTS)
s->num_points++;
@@ -133,11 +137,11 @@ static void add_sample(struct linreg_servo *s, int64_t offset)

static void regress(struct linreg_servo *s)
{
- double x, y, y0, e, x_sum, y_sum, xy_sum, x2_sum;
+ double x, y, y0, e, x_sum, y_sum, xy_sum, x2_sum, w, w_sum;
unsigned int i, l, n, size;
struct result *res;

- x_sum = 0.0, y_sum = 0.0, xy_sum = 0.0, x2_sum = 0.0;
+ x_sum = 0.0, y_sum = 0.0, xy_sum = 0.0, x2_sum = 0.0; w_sum = 0.0;
i = 0;

y0 = (int64_t)(s->points[s->last_point].y - s->reference.y);
@@ -169,27 +173,30 @@ static void regress(struct linreg_servo *s)

x = (int64_t)(s->points[l].x - s->reference.x);
y = (int64_t)(s->points[l].y - s->reference.y);
+ w = s->points[l].w;

- x_sum += x;
- y_sum += y;
- xy_sum += x * y;
- x2_sum += x * x;
+ x_sum += x * w;
+ y_sum += y * w;
+ xy_sum += x * y * w;
+ x2_sum += x * x * w;
+ w_sum += w;
}

/* Get new intercept and slope */
- res->slope = (xy_sum - x_sum * y_sum / n) /
- (x2_sum - x_sum * x_sum / n);
- res->intercept = (y_sum - res->slope * x_sum) / n;
+ res->slope = (xy_sum - x_sum * y_sum / w_sum) /
+ (x2_sum - x_sum * x_sum / w_sum);
+ res->intercept = (y_sum - res->slope * x_sum) / w_sum;
}
}

-/* Return largest size with smallest prediction error */
-static int get_best_size(struct linreg_servo *s)
+static void update_size(struct linreg_servo *s)
{
struct result *res;
double best_err;
int size, best_size;

+ /* Find largest size with smallest prediction error */
+
best_size = 0;
best_err = 0.0;

@@ -203,7 +210,7 @@ static int get_best_size(struct linreg_servo *s)
}
}

- return best_size;
+ s->size = best_size;
}

static double linreg_sample(struct servo *servo,
@@ -214,7 +221,7 @@ static double linreg_sample(struct servo *servo,
{
struct linreg_servo *s = container_of(servo, struct linreg_servo, servo);
struct result *res;
- int size, corr_interval;
+ int corr_interval;

/*
* The current time and the time when will be the frequency of the
@@ -225,21 +232,21 @@ static double linreg_sample(struct servo *servo,
*/

update_reference(s, local_ts);
- add_sample(s, offset);
+ add_sample(s, offset, weight);
regress(s);

- size = get_best_size(s);
+ update_size(s);

- if (size < MIN_SIZE) {
+ if (s->size < MIN_SIZE) {
/* Not enough points, wait for more */
*state = SERVO_UNLOCKED;
return -s->clock_freq;
}

- res = &s->results[size - MIN_SIZE];
+ res = &s->results[s->size - MIN_SIZE];

pr_debug("linreg: points %d slope %.9f intercept %.0f err %.0f",
- 1 << size, res->slope, res->intercept, res->err);
+ 1 << s->size, res->slope, res->intercept, res->err);

if ((servo->first_update &&
servo->first_step_threshold &&
@@ -265,7 +272,7 @@ static double linreg_sample(struct servo *servo,
* correction slowing down the clock will result in an overshoot. With
* the system clock's maximum adjustment of 10% that's acceptable.
*/
- corr_interval = size <= 4 ? 1 : size / 2;
+ corr_interval = s->size <= 4 ? 1 : s->size / 2;
s->clock_freq += res->intercept / s->update_interval / corr_interval;

/* Clamp the frequency to the allowed maximum */
@@ -293,6 +300,7 @@ static void linreg_reset(struct servo *servo)

s->num_points = 0;
s->last_update = 0;
+ s->size = 0;
s->frequency_ratio = 1.0;

for (i = MIN_SIZE; i <= MAX_SIZE; i++) {

[Richard Cochran]

Post by Miroslav Lichvar
diff --git a/port.c b/port.c
index 18a956d..77421aa 100644
--- a/port.c
+++ b/port.c
@@ -887,9 +887,8 @@ static int port_management_set(struct port *target,
return respond ? 1 : 0;
}
-static void port_nrate_calculate(struct port *p, tmv_t t3, tmv_t t4, tmv_t c)
+static void port_nrate_calculate(struct port *p, tmv_t origin, tmv_t ingress)
{
- tmv_t origin2;
struct nrate_estimator *n = &p->nrate;
/*
@@ -899,24 +898,23 @@ static void port_nrate_calculate(struct port *p, tmv_t t3, tmv_t t4, tmv_t c)
p->pdr_missing = 0;
if (!n->ingress1) {
- n->ingress1 = t4;
- n->origin1 = tmv_add(t3, c);
+ n->ingress1 = origin;
+ n->origin1 = ingress;

Aren't these two inverted?

Thanks,
Richard

Post by Miroslav Lichvar
return;
}
n->count++;
if (n->count < n->max_count) {
return;
}
- origin2 = tmv_add(t3, c);
- if (tmv_eq(t4, n->ingress1)) {
+ if (tmv_eq(ingress, n->ingress1)) {
pr_warning("bad timestamps in nrate calculation");
return;
}
n->ratio =
- tmv_dbl(tmv_sub(origin2, n->origin1)) /
- tmv_dbl(tmv_sub(t4, n->ingress1));
- n->ingress1 = t4;
- n->origin1 = origin2;
+ tmv_dbl(tmv_sub(origin, n->origin1)) /
+ tmv_dbl(tmv_sub(ingress, n->ingress1));
+ n->ingress1 = ingress;
+ n->origin1 = origin;
n->count = 0;
n->ratio_valid = 1;
}
@@ -1012,11 +1010,17 @@ static void port_synchronize(struct port *p,
Integer64 correction1, Integer64 correction2)
{
enum servo_state state;
+ tmv_t t1, t1c, t2, c1, c2;
port_set_sync_rx_tmo(p);
- state = clock_synchronize(p->clock, ingress_ts, origin_ts,
- correction1, correction2);
+ t1 = timestamp_to_tmv(origin_ts);
+ t2 = timespec_to_tmv(ingress_ts);
+ c1 = correction_to_tmv(correction1);
+ c2 = correction_to_tmv(correction2);
+ t1c = tmv_add(t1, tmv_add(c1, c2));
+
+ state = clock_synchronize(p->clock, t2, t1c);
switch (state) {
port_dispatch(p, EV_SYNCHRONIZATION_FAULT, 0);
@@ -1623,6 +1627,7 @@ static void process_delay_resp(struct port *p, struct ptp_message *m)
struct delay_req_msg *req;
struct delay_resp_msg *rsp = &m->delay_resp;
struct PortIdentity master;
+ tmv_t c3, t3, t4, t4c;
if (!p->delay_req)
return;
@@ -1639,8 +1644,12 @@ static void process_delay_resp(struct port *p, struct ptp_message *m)
if (!pid_eq(&master, &m->header.sourcePortIdentity))
return;
- clock_path_delay(p->clock, p->delay_req->hwts.ts, m->ts.pdu,
- m->header.correction);
+ c3 = correction_to_tmv(m->header.correction);
+ t3 = timespec_to_tmv(p->delay_req->hwts.ts);
+ t4 = timestamp_to_tmv(m->ts.pdu);
+ t4c = tmv_sub(t4, c3);
+
+ clock_path_delay(p->clock, t3, t4c);
if (p->logMinDelayReqInterval != rsp->hdr.logMessageInterval) {
// TODO - validate the input.
static void port_peer_delay(struct port *p)
{
- tmv_t c1, c2, t1, t2, t3, t4, pd;
+ tmv_t c1, c2, t1, t2, t3, t3c, t4, pd;
struct ptp_message *req = p->peer_delay_req;
struct ptp_message *rsp = p->peer_delay_resp;
struct ptp_message *fup = p->peer_delay_fup;
@@ -1837,11 +1846,10 @@ static void port_peer_delay(struct port *p)
t3 = timestamp_to_tmv(fup->ts.pdu);
c2 = correction_to_tmv(fup->header.correction);
+ t3c = tmv_add(t3, tmv_add(c1, c2));
adj_t41 = p->nrate.ratio * clock_rate_ratio(p->clock) *
tmv_dbl(tmv_sub(t4, t1));
- pd = tmv_sub(dbl_tmv(adj_t41), tmv_sub(t3, t2));
- pd = tmv_sub(pd, c1);
- pd = tmv_sub(pd, c2);
+ pd = tmv_sub(dbl_tmv(adj_t41), tmv_sub(t3c, t2));
pd = tmv_div(pd, 2);
p->peer_delay = filter_sample(p->delay_filter, pd);
pr_debug("pdelay %hu %10" PRId64 "%10" PRId64, portnum(p), p->peer_delay, pd);
if (p->pod.follow_up_info)
- port_nrate_calculate(p, t3, t4, tmv_add(c1, c2));
+ port_nrate_calculate(p, t3c, t4);
if (p->state == PS_UNCALIBRATED || p->state == PS_SLAVE) {
clock_peer_delay(p->clock, p->peer_delay, p->nrate.ratio);
--
2.1.0
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Miroslav Lichvar]

Post by Richard Cochran

Post by Miroslav Lichvar
if (!n->ingress1) {
- n->ingress1 = t4;
- n->origin1 = tmv_add(t3, c);
+ n->ingress1 = origin;
+ n->origin1 = ingress;

Aren't these two inverted?

Hm, yes, they are. I'm not sure how that happened.

Thanks,

[Richard Cochran]

Post by Miroslav Lichvar
@@ -1623,6 +1627,7 @@ static void process_delay_resp(struct port *p, struct ptp_message *m)
struct delay_req_msg *req;
struct delay_resp_msg *rsp = &m->delay_resp;
struct PortIdentity master;
+ tmv_t c3, t3, t4, t4c;
if (!p->delay_req)
return;
@@ -1639,8 +1644,12 @@ static void process_delay_resp(struct port *p, struct ptp_message *m)
if (!pid_eq(&master, &m->header.sourcePortIdentity))
return;
- clock_path_delay(p->clock, p->delay_req->hwts.ts, m->ts.pdu,
- m->header.correction);
+ c3 = correction_to_tmv(m->header.correction);
+ t3 = timespec_to_tmv(p->delay_req->hwts.ts);
+ t4 = timestamp_to_tmv(m->ts.pdu);
+ t4c = tmv_sub(t4, c3);
+
+ clock_path_delay(p->clock, t3, t4c);

Gripe: t4c doesn't deserve to be a separate own variable. Just having
t4 = tmv_sub(t4, c3) is clear enough.

Thanks,
Richard

[Richard Cochran]

Post by Richard Cochran
Gripe: t4c doesn't deserve to be a separate own variable. Just having
t4 = tmv_sub(t4, c3) is clear enough.

Same goes for 't1c' in port_synchronize().

Thanks,
Richard

[Miroslav Lichvar]

Post by Richard Cochran

Post by Richard Cochran
Gripe: t4c doesn't deserve to be a separate own variable. Just having
t4 = tmv_sub(t4, c3) is clear enough.

Same goes for 't1c' in port_synchronize().

Ok.

I thought separate variables for corrected timestamps would be less
confusing for someone reading the PTP spec where the corrections are
applied later in the delay calculation.

[Richard Cochran]

Post by Miroslav Lichvar
I thought separate variables for corrected timestamps would be less
confusing for someone reading the PTP spec where the corrections are
applied later in the delay calculation.

On second thought, there is one such usage (in port_peer_delay) where
the having the extra name does make the code more clear. So you can
just keep the tXc variables, for consistency.

[ For the first two functions, when reading the code, I was expecting
something complex to happen with tXc. ]

Thanks,
Richard