[Linuxptp-devel] [PATCH RFC V4 0/5] Poor man's boundary clock

[Richard Cochran]

Here my fourth go at the "JBOD" Boundary Clock. In order to avoid a
frequency hiccup when changing the slave port, we need somehow to
reset the servo. After looking at adding a bunch more servo code to
allow changing the frequency offset and limits on the fly, I realized
it would be much easier to completely restart the servo instead. The
result is much less intrusive to the existing code base.

Thanks,
Richard

* ChangeLog
** V4
- Simplify the implementation by letting a new slave port call into
the clock to reset the PHC device
** V3
- Close all open PHC file descriptors on exit
- Less verbose JBOD BC start up messages
- Synchronize the group of devices, even if none is a slave

Richard Cochran (5):
config: add a option to enable a poor man's boundary clock.
clock: Introduce a function to switch the PTP Hardware Clock.
port: allow running a boundary clock with multiple clock devices.
phc2sys: automatic mode: synchronize all non-slave ports.
phc2sys: default to the first clock in automatic mode.

clock.c | 37 +++++++++++++++++++++++++++++++++++++
clock.h | 8 ++++++++
config.c | 13 +++++++++++++
config.h | 1 +
default.cfg | 1 +
ds.h | 1 +
fault.c | 1 +
fault.h | 1 +
gPTP.cfg | 1 +
phc2sys.c | 52 +++++++++++++++++++++++++++++++++++++++++++---------
port.c | 27 +++++++++++++++++++++++----
ptp4l.8 | 12 +++++++++++-
ptp4l.c | 1 +
13 files changed, 142 insertions(+), 14 deletions(-)

[Richard Cochran]

This patch adds a configuration option that allows running a boundary clock
using "just a bunch of devices". Normally each port is probed to make sure
they all share the same PTP hardware clock, but this option will allow a
heterogeneous collection of devices, should the user really want it.

Signed-off-by: Richard Cochran <***@gmail.com>
Acked-by: Jiri Benc <***@redhat.com>
---
config.c | 13 +++++++++++++
config.h | 1 +
default.cfg | 1 +
ds.h | 1 +
gPTP.cfg | 1 +
ptp4l.8 | 12 +++++++++++-
ptp4l.c | 1 +
7 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index 7679f44..ec8c538 100644
--- a/config.c
+++ b/config.c
@@ -217,6 +217,12 @@ static enum parser_result parse_port_setting(const char *option,
return r;
iface->delay_filter_length = val;

+ } else if (!strcmp(option, "boundary_clock_jbod")) {
+ r = get_ranged_int(value, &val, 0, 1);
+ if (r != PARSED_OK)
+ return r;
+ iface->boundary_clock_jbod = val;
+
} else
return NOT_PARSED;

@@ -574,6 +580,12 @@ static enum parser_result parse_global_setting(const char *option,
return r;
cfg->dds.delay_filter_length = val;

+ } else if (!strcmp(option, "boundary_clock_jbod")) {
+ r = get_ranged_int(value, &val, 0, 1);
+ if (r != PARSED_OK)
+ return r;
+ cfg->dds.boundary_clock_jbod = val;
+
} else
return NOT_PARSED;

@@ -762,6 +774,7 @@ void config_init_interface(struct interface *iface, struct config *cfg)

iface->delay_filter = cfg->dds.delay_filter;
iface->delay_filter_length = cfg->dds.delay_filter_length;
+ iface->boundary_clock_jbod = cfg->dds.boundary_clock_jbod;
}

void config_destroy(struct config *cfg)
diff --git a/config.h b/config.h
index d580496..c870e42 100644
--- a/config.h
+++ b/config.h
@@ -41,6 +41,7 @@ struct interface {
struct sk_ts_info ts_info;
enum filter_type delay_filter;
int delay_filter_length;
+ int boundary_clock_jbod;
};

#define CFG_IGNORE_DM (1 << 0)
diff --git a/default.cfg b/default.cfg
index 0e20726..ec2ce58 100644
--- a/default.cfg
+++ b/default.cfg
@@ -72,6 +72,7 @@ delay_filter moving_median
delay_filter_length 10
egressLatency 0
ingressLatency 0
+boundary_clock_jbod 0
#
# Clock description
#
diff --git a/ds.h b/ds.h
index 00260ed..8f44c3b 100644
--- a/ds.h
+++ b/ds.h
@@ -61,6 +61,7 @@ struct default_ds {
struct clock_description clock_desc;
enum filter_type delay_filter;
int delay_filter_length;
+ int boundary_clock_jbod;
};

struct dataset {
diff --git a/gPTP.cfg b/gPTP.cfg
index 689abd8..d917bd7 100644
--- a/gPTP.cfg
+++ b/gPTP.cfg
@@ -71,3 +71,4 @@ delay_filter moving_median
delay_filter_length 10
egressLatency 0
ingressLatency 0
+boundary_clock_jbod 0
diff --git a/ptp4l.8 b/ptp4l.8
index bee42e9..0d01f29 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -1,4 +1,4 @@
-.TH PTP4l 8 "October 2013" "linuxptp"
+.TH PTP4l 8 "December 2014" "linuxptp"
.SH NAME
ptp4l \- PTP Boundary/Ordinary Clock

@@ -217,6 +217,16 @@ Specifies the difference in nanoseconds between the reported receive
time stamp and the actual reception time at reference plane. This value
will be subtracted from ingress time stamps obtained from the hardware.
The default is 0.
+.TP
+.B boundary_clock_jbod
+When running as a boundary clock (that is, when more than one network
+interface is configured), ptp4l performs a sanity check to make sure
+that all of the ports share the same hardware clock device. This
+option allows ptp4l to work as a boundary clock using "just a bunch of
+devices" that are not synchronized to each other. For this mode, the
+collection of clocks must be synchronized by an external program, for
+example phc2sys(8) in "automatic" mode.
+The default is 0 (disabled).

.SH PROGRAM AND CLOCK OPTIONS

diff --git a/ptp4l.c b/ptp4l.c
index c18406f..1294d88 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -75,6 +75,7 @@ static struct config cfg_settings = {
},
.delay_filter = FILTER_MOVING_MEDIAN,
.delay_filter_length = 10,
+ .boundary_clock_jbod = 0,
},

.pod = {

[Richard Cochran]

If the user has configured the appropriate option, then simply warn
about the clock device mismatch, and then go on in "JBOD" mode.
Whenever the port enters the uncalibrated state, it tells the clock
to switch to the new PHC device.

Signed-off-by: Richard Cochran <***@gmail.com>
---
fault.c | 1 +
fault.h | 1 +
port.c | 27 +++++++++++++++++++++++----
3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/fault.c b/fault.c
index 7fcf019..3976f8c 100644
--- a/fault.c
+++ b/fault.c
@@ -21,6 +21,7 @@
static const char *fault_type_str[FT_CNT] = {
"FT_UNSPECIFIED",
"FT_BAD_PEER_NETWORK",
+ "FT_SWITCH_PHC",
};

const char *ft_str(enum fault_type ft)
diff --git a/fault.h b/fault.h
index 8d62c38..fd42b82 100644
--- a/fault.h
+++ b/fault.h
@@ -21,6 +21,7 @@
enum fault_type {
FT_UNSPECIFIED = 0,
FT_BAD_PEER_NETWORK,
+ FT_SWITCH_PHC,
FT_CNT,
};

diff --git a/port.c b/port.c
index aa9354b..968c818 100644
--- a/port.c
+++ b/port.c
@@ -29,6 +29,7 @@
#include "filter.h"
#include "missing.h"
#include "msg.h"
+#include "phc.h"
#include "port.h"
#include "print.h"
#include "sk.h"
@@ -68,6 +69,8 @@ struct port {
enum timestamp_type timestamping;
struct fdarray fda;
int fault_fd;
+ int phc_index;
+ int jbod;
struct foreign_clock *best;
enum syfu_state syfu;
struct ptp_message *last_syncfup;
@@ -2138,6 +2141,14 @@ int port_dispatch(struct port *p, enum fsm_event event, int mdiff)

p->state = next;
port_notify_event(p, NOTIFY_PORT_STATE);
+
+ if (p->jbod && next == PS_UNCALIBRATED) {
+ if (clock_switch_phc(p->clock, p->phc_index)) {
+ p->last_fault_type = FT_SWITCH_PHC;
+ return port_dispatch(p, EV_FAULT_DETECTED, 0);
+ }
+ }
+
return 0;
}

@@ -2483,15 +2494,23 @@ struct port *port_open(int phc_index,

memset(p, 0, sizeof(*p));

+ p->phc_index = phc_index;
+ p->jbod = interface->boundary_clock_jbod;
+
if (interface->transport == TRANS_UDS)
; /* UDS cannot have a PHC. */
else if (!interface->ts_info.valid)
pr_warning("port %d: get_ts_info not supported", number);
else if (phc_index >= 0 && phc_index != interface->ts_info.phc_index) {
- pr_err("port %d: PHC device mismatch", number);
- pr_err("port %d: /dev/ptp%d requested, but /dev/ptp%d attached",
- number, phc_index, interface->ts_info.phc_index);
- goto err_port;
+ if (interface->boundary_clock_jbod) {
+ pr_warning("port %d: just a bunch of devices", number);
+ p->phc_index = interface->ts_info.phc_index;
+ } else {
+ pr_err("port %d: PHC device mismatch", number);
+ pr_err("port %d: /dev/ptp%d requested, ptp%d attached",
+ number, phc_index, interface->ts_info.phc_index);
+ goto err_port;
+ }
}

p->pod = interface->pod;

[Richard Cochran]

Oops, should have tested this first. It doesn't work without the
additional one line patch, below.

Post by Richard Cochran
+ if (p->jbod && next == PS_UNCALIBRATED) {
+ if (clock_switch_phc(p->clock, p->phc_index)) {
+ p->last_fault_type = FT_SWITCH_PHC;
+ return port_dispatch(p, EV_FAULT_DETECTED, 0);
+ }
+ }

diff --git a/port.c b/port.c
index 968c818..b150ff2 100644
--- a/port.c
+++ b/port.c
@@ -2147,6 +2147,7 @@ int port_dispatch(struct port *p, enum fsm_event event, int mdiff)
p->last_fault_type = FT_SWITCH_PHC;
return port_dispatch(p, EV_FAULT_DETECTED, 0);
}
+ clock_sync_interval(p->clock, p->log_sync_interval);
}

return 0;

[Richard Cochran]

When running a "jbod" Boundary Clock, as long as we have one slaved port,
we always want the clocks on the other ports to be synchronized, regardless
of their port state.

Signed-off-by: Richard Cochran <***@gmail.com>
Acked-by: Jiri Benc <***@redhat.com>
---
phc2sys.c | 40 +++++++++++++++++++++++++++++++++-------
1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 67d8a58..9afc14e 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -313,14 +313,23 @@ static void reconfigure(struct node *node)
c->new_state = 0;
}

- if (c->state == PS_SLAVE) {
- src = c;
- src_cnt++;
- } else if (c->state == PS_UNCALIBRATED) {
- src_cnt++;
- } else if (c->state == PS_MASTER) {
+ switch (c->state) {
+ case PS_FAULTY:
+ case PS_DISABLED:
+ case PS_LISTENING:
+ case PS_PRE_MASTER:
+ case PS_MASTER:
+ case PS_PASSIVE:
pr_info("selecting %s for synchronization", c->device);
dst_cnt++;
+ break;
+ case PS_UNCALIBRATED:
+ src_cnt++;
+ break;
+ case PS_SLAVE:
+ src = c;
+ src_cnt++;
+ break;
}
}
if (src_cnt > 1) {
@@ -562,6 +571,23 @@ static int do_pps_loop(struct node *node, struct clock *clock, int fd)
return 0;
}

+static int update_needed(struct clock *c)
+{
+ switch (c->state) {
+ case PS_FAULTY:
+ case PS_DISABLED:
+ case PS_LISTENING:
+ case PS_PRE_MASTER:
+ case PS_MASTER:
+ case PS_PASSIVE:
+ return 1;
+ case PS_UNCALIBRATED:
+ case PS_SLAVE:
+ break;
+ }
+ return 0;
+}
+
static int do_loop(struct node *node, int subscriptions)
{
struct timespec interval;
@@ -593,7 +619,7 @@ static int do_loop(struct node *node, int subscriptions)
continue;

LIST_FOREACH(clock, &node->clocks, list) {
- if (clock->state != PS_MASTER)
+ if (!update_needed(clock))
continue;

if (clock->clkid == CLOCK_REALTIME &&

[Richard Cochran]

If we have clocks to synchronize but no source, just pick the last one in
the list, which is the first one from the ptp4l command line.

Signed-off-by: Richard Cochran <***@gmail.com>
---
phc2sys.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 9afc14e..d59814d 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -292,13 +292,12 @@ static void clock_reinit(struct clock *clock)

static void reconfigure(struct node *node)
{
- struct clock *c, *rt, *src;
+ struct clock *c, *rt = NULL, *src = NULL, *last = NULL;
int src_cnt = 0, dst_cnt = 0;

pr_info("reconfiguring after port state change");
node->state_changed = 0;

- src = rt = NULL;
LIST_FOREACH(c, &node->clocks, list) {
if (c->clkid == CLOCK_REALTIME) {
rt = c;
@@ -331,6 +330,15 @@ static void reconfigure(struct node *node)
src_cnt++;
break;
}
+ last = c;
+ }
+ if (dst_cnt && !src) {
+ if (!rt || rt->dest_only) {
+ node->master = last;
+ pr_info("no source, selecting %s as the default clock",
+ last->device);
+ return;
+ }
}
if (src_cnt > 1) {
pr_info("multiple master clocks available, postponing sync...");

[Richard Cochran]

When switching clock devices in JBOD mode, we need to be able to reset the
servo. The existing servo_reset() function will not serve us well, because
in this case we also need to seed the existing frequency offset and limit.
This patch adds a new method that simply starts the servo from scratch.

In the unlikely event of a resource allocation failure, the method will
simply continue to use the previous device, which is better than nothing
and certainly preferable to bailing out the program.

Signed-off-by: Richard Cochran <***@gmail.com>
---
clock.c | 37 +++++++++++++++++++++++++++++++++++++
clock.h | 8 ++++++++
2 files changed, 45 insertions(+)

diff --git a/clock.c b/clock.c
index 835d2c3..b841e81 100644
--- a/clock.c
+++ b/clock.c
@@ -74,6 +74,7 @@ struct clock_subscriber {
struct clock {
clockid_t clkid;
struct servo *servo;
+ enum servo_type servo_type;
struct defaultDS dds;
struct dataset default_dataset;
struct currentDS cur;
@@ -862,6 +863,7 @@ struct clock *clock_create(int phc_index, struct interfaces_head *ifaces,
return NULL;
}
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) {
@@ -1358,6 +1360,41 @@ UInteger16 clock_steps_removed(struct clock *c)
return c->cur.stepsRemoved;
}

+int clock_switch_phc(struct clock *c, int phc_index)
+{
+ struct servo *servo;
+ int fadj, max_adj;
+ clockid_t clkid;
+ char phc[32];
+
+ snprintf(phc, 31, "/dev/ptp%d", phc_index);
+ clkid = phc_open(phc);
+ if (clkid == CLOCK_INVALID) {
+ pr_err("Switching PHC, failed to open %s: %m", phc);
+ return -1;
+ }
+ max_adj = phc_max_adj(clkid);
+ if (!max_adj) {
+ pr_err("Switching PHC, clock is not adjustable");
+ phc_close(clkid);
+ return -1;
+ }
+ fadj = (int) clockadj_get_freq(clkid);
+ clockadj_set_freq(clkid, fadj);
+ servo = servo_create(c->servo_type, -fadj, max_adj, 0);
+ if (!servo) {
+ pr_err("Switching PHC, failed to create clock servo");
+ phc_close(clkid);
+ return -1;
+ }
+ phc_close(c->clkid);
+ servo_destroy(c->servo);
+ c->clkid = clkid;
+ c->servo = servo;
+ c->servo_state = SERVO_UNLOCKED;
+ return 0;
+}
+
enum servo_state clock_synchronize(struct clock *c,
struct timespec ingress_ts,
struct timestamp origin_ts,
diff --git a/clock.h b/clock.h
index a2b46be..4834464 100644
--- a/clock.h
+++ b/clock.h
@@ -205,6 +205,14 @@ int clock_slave_only(struct clock *c);
UInteger16 clock_steps_removed(struct clock *c);

/**
+ * Switch to a new PTP Hardware Clock, for use with the "jbod" mode.
+ * @param c The clock instance.
+ * @param phc_index The index of the PHC device to use.
+ * @return Zero on success, non-zero otherwise.
+ */
+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.