Discussion:
[Linuxptp-devel] [PATCH RFC 0/2] time stamp processing
Miroslav Lichvar
2015-03-13 10:47:28 UTC
Permalink
This is an implementation of the time stamp processor as discussed in
the thread on improving accuracy with software timestamping. I would
like to get some feedback before finishing it and rebasing the other
patches on top of it.

Is this a step in the right direction?

Miroslav Lichvar (2):
Refactor time stamp processing.
tsproc: add raw and weighting modes.

clock.c | 144 ++++++++++++++++------------------------------------
clock.h | 22 ++++----
makefile | 2 +-
port.c | 61 ++++++++++++----------
tsproc.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tsproc.h | 57 +++++++++++++++++++++
6 files changed, 318 insertions(+), 140 deletions(-)
create mode 100644 tsproc.c
create mode 100644 tsproc.h
--
2.1.0
Miroslav Lichvar
2015-03-13 10:47:30 UTC
Permalink
---
clock.c | 5 +++--
port.c | 2 +-
tsproc.c | 27 ++++++++++++++++++++++-----
tsproc.h | 5 +++--
4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/clock.c b/clock.c
index f52dc0b..1ec2b0d 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->delay_filter, dds->delay_filter_length,
+ 0, 0);
if (!c->tsproc) {
pr_err("Failed to create time stamp processor");
return NULL;
@@ -1358,7 +1359,7 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t ingress_ts,

tsproc_down_ts(c->tsproc, origin_ts, ingress_ts);

- 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_ts))
diff --git a/port.c b/port.c
index 50926dc..fd4e91d 100644
--- a/port.c
+++ b/port.c
@@ -2537,7 +2537,7 @@ struct port *port_open(int phc_index,
p->versionNumber = PTP_VERSION;

p->tsproc = tsproc_create(interface->delay_filter,
- interface->delay_filter_length);
+ interface->delay_filter_length, 0, 0);
if (!p->tsproc) {
pr_err("Failed to create time stamp processor");
goto err_transport;
diff --git a/tsproc.c b/tsproc.c
index f8d5195..882e64b 100644
--- a/tsproc.c
+++ b/tsproc.c
@@ -25,6 +25,9 @@
#include "print.h"

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

@@ -41,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 filter_type delay_filter, int filter_length,
+ int raw_mode, int weighting)
{
struct tsproc *tsp;

@@ -55,6 +59,8 @@ struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length)
return NULL;
}

+ tsp->raw_mode = raw_mode;
+ tsp->weighting = weighting;
tsp->clock_rate_ratio = 1.0;

return tsp;
@@ -79,7 +85,6 @@ void tsproc_set_clock_rate_ratio(struct tsproc *tsp, double clock_rate_ratio)

void tsproc_set_delay(struct tsproc *tsp, tmv_t delay)
{
- tsp->raw_delay = delay;
tsp->filtered_delay = delay;
}

@@ -112,12 +117,12 @@ int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay)
tsp->filtered_delay, tsp->raw_delay);

if (delay)
- *delay = tsp->filtered_delay;
+ *delay = tsp->raw_mode ? tsp->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 t21;

@@ -130,7 +135,19 @@ int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset)
/* offset = t2 - t1 - delay */

t21 = tmv_sub(tsp->t2, tsp->t1);
- *offset = tmv_sub(t21, tsp->filtered_delay);
+ *offset = tmv_sub(t21, tsp->raw_mode ?
+ tsp->raw_delay : tsp->filtered_delay);
+
+ if (!weight)
+ return 0;
+
+ if (tsp->weighting && tsp->filtered_delay > 0 && tsp->raw_delay > 0) {
+ *weight = (double)tsp->filtered_delay / tsp->raw_delay;
+ if (*weight > 1.0)
+ *weight = 1.0;
+ } else {
+ *weight = 1.0;
+ }

return 0;
}
diff --git a/tsproc.h b/tsproc.h
index 6ea91ed..b55e896 100644
--- a/tsproc.h
+++ b/tsproc.h
@@ -31,7 +31,8 @@ struct tsproc;
*
* @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 filter_type delay_filter, int filter_length,
+ int raw_mode, int weighting);

void tsproc_down_ts(struct tsproc *tsp, tmv_t remote_ts, tmv_t local_ts);

@@ -43,7 +44,7 @@ void tsproc_set_delay(struct tsproc *tsp, tmv_t delay);

int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay);

-int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset);
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset, double *weight);

void tsproc_reset(struct tsproc *tsp, int full);
--
2.1.0
Richard Cochran
2015-03-13 12:36:13 UTC
Permalink
Post by Miroslav Lichvar
diff --git a/tsproc.h b/tsproc.h
index 6ea91ed..b55e896 100644
--- a/tsproc.h
+++ b/tsproc.h
@@ -31,7 +31,8 @@ struct tsproc;
*
*/
-struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length);
+struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length,
+ int raw_mode, int weighting);
I was thinking more along the lines of filter.c/servo.c, with "create"
taking an enum that specifies the type of algorithm to use.

Thanks,
Richard
Miroslav Lichvar
2015-03-13 12:53:34 UTC
Permalink
Post by Richard Cochran
Post by Miroslav Lichvar
diff --git a/tsproc.h b/tsproc.h
index 6ea91ed..b55e896 100644
--- a/tsproc.h
+++ b/tsproc.h
@@ -31,7 +31,8 @@ struct tsproc;
*
*/
-struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length);
+struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length,
+ int raw_mode, int weighting);
I was thinking more along the lines of filter.c/servo.c, with "create"
taking an enum that specifies the type of algorithm to use.
Ok, so enum with four combinations for raw enabled/disabled and
weighting enabled/disabled?

TSPROC_FILTERED (the old mode)
TSPROC_RAW
TSPROC_FILTERED_WEIGHT
TSPROC_RAW_WEIGHT

Should it be one string option in the configuration file, or two
boolean options?
--
Miroslav Lichvar
Richard Cochran
2015-03-13 17:11:09 UTC
Permalink
Post by Miroslav Lichvar
TSPROC_FILTERED (the old mode)
TSPROC_RAW
TSPROC_FILTERED_WEIGHT
TSPROC_RAW_WEIGHT
Should it be one string option in the configuration file, or two
boolean options?
One string, please!

Thanks,
Richard

Miroslav Lichvar
2015-03-13 10:47:29 UTC
Permalink
Introduce a time stamp processor for offset/delay calculations and use
it in the clock and port modules. Also, convert and correct the time
stamps early to reduce the number of parameters.
---
clock.c | 143 +++++++++++++++++-----------------------------------------
clock.h | 22 ++++-----
makefile | 2 +-
port.c | 61 ++++++++++++++-----------
tsproc.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tsproc.h | 56 +++++++++++++++++++++++
6 files changed, 299 insertions(+), 140 deletions(-)
create mode 100644 tsproc.c
create mode 100644 tsproc.h

diff --git a/clock.c b/clock.c
index b841e81..f52dc0b 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,14 +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 c1;
- tmv_t c2;
- tmv_t t1;
- tmv_t t2;
struct clock_description desc;
struct clock_stats stats;
int stats_interval;
@@ -273,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);
@@ -415,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);
@@ -545,20 +543,14 @@ 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_ts,
+ tmv_t origin_ts)
{
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:
*
@@ -570,25 +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 = tmv_add(c->t1, tmv_add(c->c1, c->c2));
+ f->ingress1 = ingress_ts;
+ f->origin1 = origin_ts;
return state;
}
f->count++;
if (f->count < f->max_count) {
return state;
}
- if (tmv_eq(c->t2, f->ingress1)) {
+ if (tmv_eq(ingress_ts, f->ingress1)) {
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)) /
- tmv_dbl(tmv_sub(c->t2, f->ingress1));
+ ratio = tmv_dbl(tmv_sub(origin_ts, f->origin1)) /
+ tmv_dbl(tmv_sub(ingress_ts, f->ingress1));
freq = (1.0 - ratio) * 1e9;

if (c->stats.max_count > 1) {
@@ -610,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 = origin2;
+ f->ingress1 = ingress_ts;
+ f->origin1 = origin_ts;
f->count = 0;

return state;
@@ -864,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;
@@ -1289,63 +1276,29 @@ 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;
- 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;

- 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);
- 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;
- */
-
- 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_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("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);
-
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));
}
@@ -1395,44 +1348,29 @@ 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_ts,
+ tmv_t origin_ts)
{
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->ingress_ts = ingress_ts;

- c->c1 = correction_to_tmv(correction1);
- c->c2 = correction_to_tmv(correction2);
+ tsproc_down_ts(c->tsproc, origin_ts, ingress_ts);

- /*
- * c->master_offset = ingress - origin - c->path_delay - c->c1 - c->c2;
- */
- c->master_offset = tmv_sub(ingress,
- tmv_add(origin, tmv_add(c->path_delay, tmv_add(c->c1, c->c2))));
-
- if (!c->path_delay)
+ if (tsproc_update_offset(c->tsproc, &c->master_offset))
return state;

- if (clock_utc_correct(c, ingress))
+ if (clock_utc_correct(c, ingress_ts))
return c->servo_state;

c->cur.offsetFromMaster = tmv_to_TimeInterval(c->master_offset);

if (c->free_running)
- return clock_no_adjust(c);
+ return clock_no_adjust(c, ingress_ts, origin_ts);

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

if (c->stats.max_count > 1) {
@@ -1445,19 +1383,21 @@ enum servo_state clock_synchronize(struct clock *c,
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);
@@ -1531,9 +1471,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 4834464..7d4f25b 100644
--- a/clock.h
+++ b/clock.h
@@ -169,19 +169,21 @@ 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.
* @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.
@@ -216,17 +218,15 @@ 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 origin_ts 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_ts,
+ tmv_t origin_ts);

/**
* Inform a slaved clock about the master's sync interval.
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 18a956d..50926dc 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;
@@ -1012,11 +1013,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 +1630,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 +1647,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,11 +1801,10 @@ 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;
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. */

@@ -1837,24 +1848,22 @@ static void port_peer_delay(struct port *p)
t3 = timestamp_to_tmv(fup->ts.pdu);
c2 = correction_to_tmv(fup->header.correction);
calc:
- 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_div(pd, 2);
-
- p->peer_delay = filter_sample(p->delay_filter, pd);
+ t3c = tmv_add(t3, tmv_add(c1, c2));
+ 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, t3, t4, tmv_add(c1, c2));

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);
@@ -1973,7 +1982,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);
@@ -2527,10 +2536,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;
@@ -2541,13 +2550,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..f8d5195
--- /dev/null
+++ b/tsproc.c
@@ -0,0 +1,155 @@
+/**
+ * @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 raw and filtered delay */
+ tmv_t raw_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_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->raw_delay = delay;
+ tsp->filtered_delay = delay;
+}
+
+int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay)
+{
+ tmv_t t23, t41;
+
+ if (!tsp->t1 || !tsp->t2 || !tsp->t3 || !tsp->t4)
+ return -1;
+
+ /* 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);
+ tsp->raw_delay = tmv_div(tmv_add(t23, t41), 2);
+
+ if (tsp->raw_delay < 0) {
+ pr_debug("negative delay %10" PRId64, tsp->raw_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);
+ }
+
+ tsp->filtered_delay = filter_sample(tsp->delay_filter, tsp->raw_delay);
+
+ pr_debug("delay filtered %10" PRId64 " raw %10" PRId64,
+ tsp->filtered_delay, tsp->raw_delay);
+
+ if (delay)
+ *delay = tsp->filtered_delay;
+
+ return 0;
+}
+
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset)
+{
+ tmv_t t21;
+
+ if (!tsp->t1 || !tsp->t2 || !tsp->t3 || !tsp->t4)
+ return -1;
+
+ if (!offset)
+ return 0;
+
+ /* offset = t2 - t1 - delay */
+
+ t21 = tmv_sub(tsp->t2, tsp->t1);
+ *offset = tmv_sub(t21, tsp->filtered_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);
+ }
+}
+
+void tsproc_destroy(struct tsproc *tsp)
+{
+ filter_destroy(tsp->delay_filter);
+ free(tsp);
+}
diff --git a/tsproc.h b/tsproc.h
new file mode 100644
index 0000000..6ea91ed
--- /dev/null
+++ b/tsproc.h
@@ -0,0 +1,56 @@
+/**
+ * @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
+ *
+ * @return A pointer to a new tsproc on success, NULL otherwise.
+ */
+struct tsproc *tsproc_create(enum filter_type delay_filter, int filter_length);
+
+void tsproc_down_ts(struct tsproc *tsp, tmv_t remote_ts, tmv_t local_ts);
+
+void tsproc_up_ts(struct tsproc *tsp, tmv_t local_ts, tmv_t remote_ts);
+
+void tsproc_set_clock_rate_ratio(struct tsproc *tsp, double clock_rate_ratio);
+
+void tsproc_set_delay(struct tsproc *tsp, tmv_t delay);
+
+int tsproc_update_delay(struct tsproc *tsp, tmv_t *delay);
+
+int tsproc_update_offset(struct tsproc *tsp, tmv_t *offset);
+
+void tsproc_reset(struct tsproc *tsp, int full);
+
+/**
+ * Destroy a time stamp processor.
+ * @param tsproc Pointer obtained via @ref tsproc_create().
+ */
+void tsproc_destroy(struct tsproc *tsp);
+
+#endif
--
2.1.0
Richard Cochran
2015-03-13 12:31:47 UTC
Permalink
Post by Miroslav Lichvar
Introduce a time stamp processor for offset/delay calculations and use
it in the clock and port modules. Also, convert and correct the time
stamps early to reduce the number of parameters.
The "early correction" part should come first as a separate patch.

Thanks,
Richard
Richard Cochran
2015-03-13 12:30:12 UTC
Permalink
Post by Miroslav Lichvar
This is an implementation of the time stamp processor as discussed in
the thread on improving accuracy with software timestamping. I would
like to get some feedback before finishing it and rebasing the other
patches on top of it.
Is this a step in the right direction?
Yes, I think so. I have two general comments on the patches...

Thanks,
Richard
Loading...