[Linuxptp-devel] [PATCH v3 0/6] Dynamic port allocation

[Jiri Benc]

Currently, there's a limit of MAX_PORTS ports (8 by default). This patchset
removes this limitation by using linked lists instead of fixed size arrays.

In addition to allowing ptp4l to work on machines with many interfaces, this
will allow for later patches to add support for adding and removing ports
dynamically while ptp4l is running.

The core patches are 3/6 and 6/6, the rest are prerequisites: patch 1/6 is
moving per-port fields from struct clock to individual ports in order not to
have to maintain several linked lists, patch 2/6 moves the uds port from the
port array, as having it a part of linked list would make the various checks
for uds port unnecessary complicated, patch 5/6 replaces the port numbering
that is based on array of interface names by a per clock counter. The
remaining patch, 4/6, is a follow up optimization to 3/6.

Changes in v3: used the BSD implementation of LIST_FOREACH_SAFE, using the
system one where available.

Changes in v2: rebased on top of the current git, resolved warnings reported
by Richard.

Jiri Benc (6):
Put fault_fd into struct port
Make uds port a separate field in struct clock
Dynamic port allocation
Lazy regeneration of pollfd
Remember last used port number
Dynamic allocation of interface config entries

clock.c | 309 +++++++++++++++++++++++++++++++++++++++-----------------------
clock.h | 20 +---
config.c | 66 +++++++-------
config.h | 10 +-
port.c | 89 +++++++++++++++----
port.h | 45 +++++++++
ptp4l.c | 40 ++++-----
7 files changed, 373 insertions(+), 206 deletions(-)

[Jiri Benc]

The fault timer file descriptor is a per port item, put it inside struct
port where other per port file descriptors are kept.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 18 +++++-------------
port.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
port.h | 29 +++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 22 deletions(-)

diff --git a/clock.c b/clock.c
index 1563ba9..96117ba 100644
--- a/clock.c
+++ b/clock.c
@@ -72,7 +72,6 @@ struct clock {
struct ClockIdentity best_id;
struct port *port[CLK_N_PORTS];
struct pollfd pollfd[CLK_N_PORTS*N_CLOCK_PFD];
- int fault_fd[CLK_N_PORTS];
int nports; /* does not include the UDS port */
int free_running;
int freq_est_interval;
@@ -111,10 +110,8 @@ static int cid_eq(struct ClockIdentity *a, struct ClockIdentity *b)
void clock_destroy(struct clock *c)
{
int i;
- for (i = 0; i < c->nports; i++) {
+ for (i = 0; i < c->nports; i++)
port_close(c->port[i]);
- close(c->fault_fd[i]);
- }
port_close(c->port[i]); /*uds*/
if (c->clkid != CLOCK_REALTIME) {
phc_close(c->clkid);
@@ -134,7 +131,7 @@ static int clock_fault_timeout(struct clock *c, int index, int set)

if (!set) {
pr_debug("clearing fault on port %d", index + 1);
- return set_tmo_lin(c->fault_fd[index], 0);
+ return port_set_fault_timer_lin(c->port[index], 0);
}

fault_interval(c->port[index], last_fault_type(c->port[index]), &i);
@@ -142,11 +139,11 @@ static int clock_fault_timeout(struct clock *c, int index, int set)
if (i.type == FTMO_LINEAR_SECONDS) {
pr_debug("waiting %d seconds to clear fault on port %d",
i.val, index + 1);
- return set_tmo_lin(c->fault_fd[index], i.val);
+ return port_set_fault_timer_lin(c->port[index], i.val);
} else if (i.type == FTMO_LOG2_SECONDS) {
pr_debug("waiting 2^{%d} seconds to clear fault on port %d",
i.val, index + 1);
- return set_tmo_log(c->fault_fd[index], 1, i.val);
+ return port_set_fault_timer_log(c->port[index], 1, i.val);
}

pr_err("Unsupported fault interval type %d", i.type);
@@ -675,12 +672,7 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,
pr_err("failed to open port %s", iface[i].name);
return NULL;
}
- c->fault_fd[i] = timerfd_create(CLOCK_MONOTONIC, 0);
- if (c->fault_fd[i] < 0) {
- pr_err("timerfd_create failed: %m");
- return NULL;
- }
- c->pollfd[N_CLOCK_PFD * i + N_POLLFD].fd = c->fault_fd[i];
+ c->pollfd[N_CLOCK_PFD * i + N_POLLFD].fd = port_fault_fd(c->port[i]);
c->pollfd[N_CLOCK_PFD * i + N_POLLFD].events = POLLIN|POLLPRI;
}

diff --git a/port.c b/port.c
index 94c5cac..6553569 100644
--- a/port.c
+++ b/port.c
@@ -65,6 +65,7 @@ struct port {
struct transport *trp;
enum timestamp_type timestamping;
struct fdarray fda;
+ int fault_fd;
struct foreign_clock *best;
enum syfu_state syfu;
struct ptp_message *last_syncfup;
@@ -193,6 +194,11 @@ int fault_interval(struct port *port, enum fault_type ft,
return 0;
}

+int port_fault_fd(struct port *port)
+{
+ return port->fault_fd;
+}
+
int set_tmo_log(int fd, unsigned int scale, int log_seconds)
{
struct itimerspec tmo = {
@@ -230,6 +236,17 @@ int set_tmo_lin(int fd, int seconds)
return timerfd_settime(fd, 0, &tmo, NULL);
}

+int port_set_fault_timer_log(struct port *port,
+ unsigned int scale, int log_seconds)
+{
+ return set_tmo_log(port->fault_fd, scale, log_seconds);
+}
+
+int port_set_fault_timer_lin(struct port *port, int seconds)
+{
+ return set_tmo_lin(port->fault_fd, seconds);
+}
+
static void fc_clear(struct foreign_clock *fc)
{
struct ptp_message *m;
@@ -1845,6 +1862,8 @@ void port_close(struct port *p)
}
transport_destroy(p->trp);
filter_destroy(p->delay_filter);
+ if (p->fault_fd >= 0)
+ close(p->fault_fd);
free(p);
}

@@ -2286,18 +2305,15 @@ struct port *port_open(int 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);
- free(p);
- return NULL;
+ goto err_port;
}

p->pod = interface->pod;
p->name = interface->name;
p->clock = clock;
p->trp = transport_create(interface->transport);
- if (!p->trp) {
- free(p);
- return NULL;
- }
+ if (!p->trp)
+ goto err_port;
p->timestamping = timestamping;
p->portIdentity.clockIdentity = clock_identity(clock);
p->portIdentity.portNumber = number;
@@ -2309,12 +2325,27 @@ struct port *port_open(int phc_index,
interface->delay_filter_length);
if (!p->delay_filter) {
pr_err("Failed to create delay filter");
- transport_destroy(p->trp);
- free(p);
- return NULL;
+ goto err_transport;
}
p->nrate.ratio = 1.0;
+
+ p->fault_fd = -1;
+ if (number) {
+ p->fault_fd = timerfd_create(CLOCK_MONOTONIC, 0);
+ if (p->fault_fd < 0) {
+ pr_err("timerfd_create failed: %m");
+ goto err_filter;
+ }
+ }
return p;
+
+err_filter:
+ filter_destroy(p->delay_filter);
+err_transport:
+ transport_destroy(p->trp);
+err_port:
+ free(p);
+ return NULL;
}

enum port_state port_state(struct port *port)
diff --git a/port.h b/port.h
index 5fef028..37f02ae 100644
--- a/port.h
+++ b/port.h
@@ -158,6 +158,13 @@ struct port *port_open(int phc_index,
enum port_state port_state(struct port *port);

/**
+ * Return file descriptor of the port.
+ * @param port A port instance.
+ * @return File descriptor or -1 if not applicable.
+ */
+int port_fault_fd(struct port *port);
+
+/**
* Utility function for setting or resetting a file descriptor timer.
*
* This function sets the timer 'fd' to the value M(2^N), where M is
@@ -187,6 +194,28 @@ int set_tmo_log(int fd, unsigned int scale, int log_seconds);
int set_tmo_lin(int fd, int seconds);

/**
+ * Sets port's fault file descriptor timer.
+ * Passing both 'scale' and 'log_seconds' as zero disables the timer.
+ *
+ * @param fd A port instance.
+ * @param scale The multiplicative factor for the timer.
+ * @param log_seconds The exponential factor for the timer.
+ * @return Zero on success, non-zero otherwise.
+ */
+int port_set_fault_timer_log(struct port *port,
+ unsigned int scale, int log_seconds);
+
+/**
+ * Sets port's fault file descriptor timer.
+ * Passing 'seconds' as zero disables the timer.
+ *
+ * @param fd A port instance.
+ * @param seconds The timeout value for the timer.
+ * @return Zero on success, non-zero otherwise.
+ */
+int port_set_fault_timer_lin(struct port *port, int seconds);
+
+/**
* Returns a port's last fault type.
*
* @param port A port instance.

[Jiri Benc]

There's no need to regenerate pollfd multiple times during batches of port
operations (like creating of the clock). Just be lazy and regenerate it only
once it's needed.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/clock.c b/clock.c
index adc6b5e..23b6ebd 100644
--- a/clock.c
+++ b/clock.c
@@ -75,6 +75,7 @@ struct clock {
LIST_HEAD(ports_head, port) ports;
struct port *uds_port;
struct pollfd *pollfd;
+ int pollfd_valid;
int nports; /* does not include the UDS port */
int free_running;
int freq_est_interval;
@@ -816,16 +817,24 @@ static void clock_fill_pollfd(struct pollfd *dest, struct port *p)
dest[i].events = POLLIN|POLLPRI;
}

-void clock_fda_changed(struct clock *c)
+static void clock_check_pollfd(struct clock *c)
{
struct port *p;
struct pollfd *dest = c->pollfd;

+ if (c->pollfd_valid)
+ return;
LIST_FOREACH(p, &c->ports, list) {
clock_fill_pollfd(dest, p);
dest += N_CLOCK_PFD;
}
clock_fill_pollfd(dest, c->uds_port);
+ c->pollfd_valid = 1;
+}
+
+void clock_fda_changed(struct clock *c)
+{
+ c->pollfd_valid = 0;
}

static int clock_do_forward_mgmt(struct clock *c,
@@ -979,6 +988,7 @@ int clock_poll(struct clock *c)
struct pollfd *cur;
struct port *p;

+ clock_check_pollfd(c);
cnt = poll(c->pollfd, (c->nports + 1) * N_CLOCK_PFD, -1);
if (cnt < 0) {
if (EINTR == errno) {

[Jiri Benc]

The uds port is handled specially in almost all cases, it doesn't behave
like the rest of ports in the port array. Make it a standalone member of
struct clock.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 71 +++++++++++++++++++++++++++++++++++++-------------------------
1 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/clock.c b/clock.c
index 96117ba..e4a3e9e 100644
--- a/clock.c
+++ b/clock.c
@@ -39,7 +39,6 @@
#include "uds.h"
#include "util.h"

-#define CLK_N_PORTS (MAX_PORTS + 1) /* plus one for the UDS interface */
#define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault timer */
#define POW2_41 ((double)(1ULL << 41))

@@ -70,8 +69,9 @@ struct clock {
struct ClockIdentity ptl[PATH_TRACE_MAX];
struct foreign_clock *best;
struct ClockIdentity best_id;
- struct port *port[CLK_N_PORTS];
- struct pollfd pollfd[CLK_N_PORTS*N_CLOCK_PFD];
+ struct port *port[MAX_PORTS];
+ struct port *uds_port;
+ struct pollfd pollfd[(MAX_PORTS + 1) * N_CLOCK_PFD];
int nports; /* does not include the UDS port */
int free_running;
int freq_est_interval;
@@ -112,7 +112,7 @@ void clock_destroy(struct clock *c)
int i;
for (i = 0; i < c->nports; i++)
port_close(c->port[i]);
- port_close(c->port[i]); /*uds*/
+ port_close(c->uds_port);
if (c->clkid != CLOCK_REALTIME) {
phc_close(c->clkid);
}
@@ -313,7 +313,7 @@ static int clock_management_set(struct clock *c, struct port *p,

switch (id) {
case GRANDMASTER_SETTINGS_NP:
- if (p != c->port[c->nports]) {
+ if (p != c->uds_port) {
/* Sorry, only allowed on the UDS port. */
break;
}
@@ -551,7 +551,7 @@ static int forwarding(struct clock *c, struct port *p)
default:
break;
}
- if (p == c->port[c->nports] && ps != PS_FAULTY) { /*uds*/
+ if (p == c->uds_port && ps != PS_FAULTY) {
return 1;
}
return 0;
@@ -677,10 +677,10 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,
}

/*
- * One extra port is for the UDS interface.
+ * Create the UDS interface.
*/
- c->port[i] = port_open(phc_index, timestamping, 0, &udsif, c);
- if (!c->port[i]) {
+ c->uds_port = port_open(phc_index, timestamping, 0, &udsif, c);
+ if (!c->uds_port) {
pr_err("failed to open the UDS port");
return NULL;
}
@@ -690,7 +690,7 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,
for (i = 0; i < c->nports; i++)
port_dispatch(c->port[i], EV_INITIALIZE, 0);

- port_dispatch(c->port[i], EV_INITIALIZE, 0); /*uds*/
+ port_dispatch(c->uds_port, EV_INITIALIZE, 0);

return c;
}
@@ -745,10 +745,13 @@ struct ClockIdentity clock_identity(struct clock *c)
void clock_install_fda(struct clock *c, struct port *p, struct fdarray fda)
{
int i, j, k;
- for (i = 0; i < c->nports + 1; i++) {
+
+ for (i = 0; i < c->nports; i++) {
if (p == c->port[i])
break;
}
+ if (p == c->uds_port)
+ i = c->nports;
for (j = 0; j < N_POLLFD; j++) {
k = N_CLOCK_PFD * i + j;
c->pollfd[k].fd = fda.fd[j];
@@ -756,26 +759,34 @@ void clock_install_fda(struct clock *c, struct port *p, struct fdarray fda)
}
}

+static int clock_do_forward_mgmt(struct clock *c,
+ struct port *in, struct port *out,
+ struct ptp_message *msg, int msglen,
+ int *pre_sent)
+{
+ if (in == out || !forwarding(c, out))
+ return 0;
+ if (!*pre_sent) {
+ /* delay calling msg_pre_send until
+ * actually forwarding */
+ msg_pre_send(msg);
+ *pre_sent = 1;
+ }
+ return port_forward(out, msg, msglen);
+}
+
static void clock_forward_mgmt_msg(struct clock *c, struct port *p, struct ptp_message *msg)
{
int i, pdulen = 0, msg_ready = 0;
- struct port *fwd;
+
if (forwarding(c, p) && msg->management.boundaryHops) {
- for (i = 0; i < c->nports + 1; i++) {
- fwd = c->port[i];
- if (fwd != p && forwarding(c, fwd)) {
- /* delay calling msg_pre_send until
- * actually forwarding */
- if (!msg_ready) {
- msg_ready = 1;
- pdulen = msg->header.messageLength;
- msg->management.boundaryHops--;
- msg_pre_send(msg);
- }
- if (port_forward(fwd, msg, pdulen))
- pr_err("port %d: management forward failed", i + 1);
- }
- }
+ pdulen = msg->header.messageLength;
+ msg->management.boundaryHops--;
+ for (i = 0; i < c->nports; i++)
+ if (clock_do_forward_mgmt(c, p, c->port[i], msg, pdulen, &msg_ready))
+ pr_err("port %d: management forward failed", i + 1);
+ if (clock_do_forward_mgmt(c, p, c->uds_port, msg, pdulen, &msg_ready))
+ pr_err("uds port: management forward failed");
if (msg_ready) {
msg_post_recv(msg, pdulen);
msg->management.boundaryHops++;
@@ -937,7 +948,7 @@ int clock_poll(struct clock *c)
for (j = 0; j < N_POLLFD; j++) {
k = N_CLOCK_PFD * i + j;
if (c->pollfd[k].revents & (POLLIN|POLLPRI)) {
- event = port_event(c->port[i], j);
+ event = port_event(c->uds_port, j);
if (EV_STATE_DECISION_EVENT == event)
sde = 1;
}
@@ -1006,10 +1017,12 @@ void clock_peer_delay(struct clock *c, tmv_t ppd, double nrr)
void clock_remove_fda(struct clock *c, struct port *p, struct fdarray fda)
{
int i, j, k;
- for (i = 0; i < c->nports + 1; i++) {
+ for (i = 0; i < c->nports; i++) {
if (p == c->port[i])
break;
}
+ if (p == c->uds_port)
+ i = c->nports;
for (j = 0; j < N_POLLFD; j++) {
k = N_CLOCK_PFD * i + j;
c->pollfd[k].fd = -1;

[Jiri Benc]

Remove the limit of MAX_PORTS ports (default 8) and keep the ports in
a linked list. This allows ptp4l to be used on large machines and in the
future, it will allow dynamic adding and removing of ports while ptp4l is
running.

For this to work, pollfd needs to be dynamically allocated. Changed pollfd
handling from clock_install_fda/clock_remove_fda to notification
(clock_fda_changed), where the clock will rebuild pollfd by querying all its
ports.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 239 +++++++++++++++++++++++++++++++++++++++------------------------
clock.h | 15 +---
port.c | 40 +++++++++--
port.h | 16 ++++
4 files changed, 200 insertions(+), 110 deletions(-)

diff --git a/clock.c b/clock.c
index e4a3e9e..adc6b5e 100644
--- a/clock.c
+++ b/clock.c
@@ -21,6 +21,7 @@
#include <stdlib.h>
#include <string.h>
#include <time.h>
+#include <sys/queue.h>

#include "bmc.h"
#include "clock.h"
@@ -42,7 +43,9 @@
#define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault timer */
#define POW2_41 ((double)(1ULL << 41))

-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+struct port {
+ LIST_ENTRY(port) list;
+};

struct freq_estimator {
tmv_t origin1;
@@ -69,9 +72,9 @@ struct clock {
struct ClockIdentity ptl[PATH_TRACE_MAX];
struct foreign_clock *best;
struct ClockIdentity best_id;
- struct port *port[MAX_PORTS];
+ LIST_HEAD(ports_head, port) ports;
struct port *uds_port;
- struct pollfd pollfd[(MAX_PORTS + 1) * N_CLOCK_PFD];
+ struct pollfd *pollfd;
int nports; /* does not include the UDS port */
int free_running;
int freq_est_interval;
@@ -101,18 +104,30 @@ struct clock {
struct clock the_clock;

static void handle_state_decision_event(struct clock *c);
+static int clock_resize_pollfd(struct clock *c, int new_nports);
+static void clock_remove_port(struct clock *c, struct port *p);

static int cid_eq(struct ClockIdentity *a, struct ClockIdentity *b)
{
return 0 == memcmp(a, b, sizeof(*a));
}

+#ifndef LIST_FOREACH_SAFE
+#define LIST_FOREACH_SAFE(var, head, field, tvar) \
+ for ((var) = LIST_FIRST((head)); \
+ (var) && ((tvar) = LIST_NEXT((var), field), 1); \
+ (var) = (tvar))
+#endif
+
void clock_destroy(struct clock *c)
{
- int i;
- for (i = 0; i < c->nports; i++)
- port_close(c->port[i]);
+ struct port *p, *tmp;
+
+ LIST_FOREACH_SAFE(p, &c->ports, list, tmp) {
+ clock_remove_port(c, p);
+ }
port_close(c->uds_port);
+ free(c->pollfd);
if (c->clkid != CLOCK_REALTIME) {
phc_close(c->clkid);
}
@@ -125,25 +140,25 @@ void clock_destroy(struct clock *c)
msg_cleanup();
}

-static int clock_fault_timeout(struct clock *c, int index, int set)
+static int clock_fault_timeout(struct port *port, int set)
{
struct fault_interval i;

if (!set) {
- pr_debug("clearing fault on port %d", index + 1);
- return port_set_fault_timer_lin(c->port[index], 0);
+ pr_debug("clearing fault on port %d", port_number(port));
+ return port_set_fault_timer_lin(port, 0);
}

- fault_interval(c->port[index], last_fault_type(c->port[index]), &i);
+ fault_interval(port, last_fault_type(port), &i);

if (i.type == FTMO_LINEAR_SECONDS) {
pr_debug("waiting %d seconds to clear fault on port %d",
- i.val, index + 1);
- return port_set_fault_timer_lin(c->port[index], i.val);
+ i.val, port_number(port));
+ return port_set_fault_timer_lin(port, i.val);
} else if (i.type == FTMO_LOG2_SECONDS) {
pr_debug("waiting 2^{%d} seconds to clear fault on port %d",
- i.val, index + 1);
- return port_set_fault_timer_log(c->port[index], 1, i.val);
+ i.val, port_number(port));
+ return port_set_fault_timer_log(port, 1, i.val);
}

pr_err("Unsupported fault interval type %d", i.type);
@@ -564,12 +579,43 @@ UInteger8 clock_class(struct clock *c)
return c->dds.clockQuality.clockClass;
}

+static int clock_add_port(struct clock *c, int phc_index,
+ enum timestamp_type timestamping, int number,
+ struct interface *iface)
+{
+ struct port *p;
+
+ if (clock_resize_pollfd(c, c->nports + 1))
+ return -1;
+ p = port_open(phc_index, timestamping, number, iface, c);
+ if (!p) {
+ /* No need to shrink pollfd */
+ return -1;
+ }
+ LIST_INSERT_HEAD(&c->ports, p, list);
+ c->nports++;
+ clock_fda_changed(c);
+ return 0;
+}
+
+static void clock_remove_port(struct clock *c, struct port *p)
+{
+ /* Do not bother with pollfd resizing, we don't mind if it's a bit
+ * larger than needed and clock_destroy takes care of freeing it in
+ * case we're shutting down. */
+ LIST_REMOVE(p, list);
+ c->nports--;
+ clock_fda_changed(c);
+ port_close(p);
+}
+
struct clock *clock_create(int phc_index, struct interface *iface, int count,
enum timestamp_type timestamping, struct default_ds *dds,
enum servo_type servo)
{
int i, fadj = 0, max_adj = 0.0, sw_ts = timestamping == TS_SOFTWARE ? 1 : 0;
struct clock *c = &the_clock;
+ struct port *p;
char phc[32];
struct interface udsif;

@@ -659,37 +705,37 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,
c->dad.pds.observedParentClockPhaseChangeRate = 0x7fffffff;
c->dad.ptl = c->ptl;

- for (i = 0; i < ARRAY_SIZE(c->pollfd); i++) {
- c->pollfd[i].fd = -1;
- c->pollfd[i].events = 0;
- }
-
clock_sync_interval(c, 0);

- for (i = 0; i < count; i++) {
- c->port[i] = port_open(phc_index, timestamping, 1+i, &iface[i], c);
- if (!c->port[i]) {
- pr_err("failed to open port %s", iface[i].name);
- return NULL;
- }
- c->pollfd[N_CLOCK_PFD * i + N_POLLFD].fd = port_fault_fd(c->port[i]);
- c->pollfd[N_CLOCK_PFD * i + N_POLLFD].events = POLLIN|POLLPRI;
- }
+ LIST_INIT(&c->ports);

/*
* Create the UDS interface.
*/
+ if (clock_resize_pollfd(c, 0)) {
+ pr_err("failed to allocate pollfd");
+ return NULL;
+ }
c->uds_port = port_open(phc_index, timestamping, 0, &udsif, c);
if (!c->uds_port) {
pr_err("failed to open the UDS port");
return NULL;
}
+ clock_fda_changed(c);

- c->dds.numberPorts = c->nports = count;
+ /* Create the ports. */
+ for (i = 0; i < count; i++) {
+ if (clock_add_port(c, phc_index, timestamping, i + 1, &iface[i])) {
+ pr_err("failed to open port %s", iface[i].name);
+ return NULL;
+ }
+ }

- for (i = 0; i < c->nports; i++)
- port_dispatch(c->port[i], EV_INITIALIZE, 0);
+ c->dds.numberPorts = c->nports;

+ LIST_FOREACH(p, &c->ports, list) {
+ port_dispatch(p, EV_INITIALIZE, 0);
+ }
port_dispatch(c->uds_port, EV_INITIALIZE, 0);

return c;
@@ -742,21 +788,44 @@ struct ClockIdentity clock_identity(struct clock *c)
return c->dds.clockIdentity;
}

-void clock_install_fda(struct clock *c, struct port *p, struct fdarray fda)
+static int clock_resize_pollfd(struct clock *c, int new_nports)
+{
+ struct pollfd *new_pollfd;
+
+ /* Need to allocate one extra block of fds for uds */
+ new_pollfd = realloc(c->pollfd,
+ (new_nports + 1) * N_CLOCK_PFD *
+ sizeof(struct pollfd));
+ if (!new_pollfd)
+ return -1;
+ c->pollfd = new_pollfd;
+ return 0;
+}
+
+static void clock_fill_pollfd(struct pollfd *dest, struct port *p)
{
- int i, j, k;
+ struct fdarray *fda;
+ int i;

- for (i = 0; i < c->nports; i++) {
- if (p == c->port[i])
- break;
+ fda = port_fda(p);
+ for (i = 0; i < N_POLLFD; i++) {
+ dest[i].fd = fda->fd[i];
+ dest[i].events = POLLIN|POLLPRI;
}
- if (p == c->uds_port)
- i = c->nports;
- for (j = 0; j < N_POLLFD; j++) {
- k = N_CLOCK_PFD * i + j;
- c->pollfd[k].fd = fda.fd[j];
- c->pollfd[k].events = POLLIN|POLLPRI;
+ dest[i].fd = port_fault_fd(p);
+ dest[i].events = POLLIN|POLLPRI;
+}
+
+void clock_fda_changed(struct clock *c)
+{
+ struct port *p;
+ struct pollfd *dest = c->pollfd;
+
+ LIST_FOREACH(p, &c->ports, list) {
+ clock_fill_pollfd(dest, p);
+ dest += N_CLOCK_PFD;
}
+ clock_fill_pollfd(dest, c->uds_port);
}

static int clock_do_forward_mgmt(struct clock *c,
@@ -777,14 +846,17 @@ static int clock_do_forward_mgmt(struct clock *c,

static void clock_forward_mgmt_msg(struct clock *c, struct port *p, struct ptp_message *msg)
{
- int i, pdulen = 0, msg_ready = 0;
+ struct port *piter;
+ int pdulen = 0, msg_ready = 0;

if (forwarding(c, p) && msg->management.boundaryHops) {
pdulen = msg->header.messageLength;
msg->management.boundaryHops--;
- for (i = 0; i < c->nports; i++)
- if (clock_do_forward_mgmt(c, p, c->port[i], msg, pdulen, &msg_ready))
- pr_err("port %d: management forward failed", i + 1);
+ LIST_FOREACH(piter, &c->ports, list) {
+ if (clock_do_forward_mgmt(c, p, piter, msg, pdulen, &msg_ready))
+ pr_err("port %d: management forward failed",
+ port_number(piter));
+ }
if (clock_do_forward_mgmt(c, p, c->uds_port, msg, pdulen, &msg_ready))
pr_err("uds port: management forward failed");
if (msg_ready) {
@@ -796,7 +868,8 @@ static void clock_forward_mgmt_msg(struct clock *c, struct port *p, struct ptp_m

int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
{
- int changed = 0, i;
+ int changed = 0;
+ struct port *piter;
struct management_tlv *mgt;
struct ClockIdentity *tcid, wildcard = {
{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}
@@ -880,8 +953,8 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
clock_management_send_error(p, msg, NOT_SUPPORTED);
break;
default:
- for (i = 0; i < c->nports; i++) {
- if (port_manage(c->port[i], p, msg))
+ LIST_FOREACH(piter, &c->ports, list) {
+ if (port_manage(piter, p, msg))
break;
}
break;
@@ -901,10 +974,12 @@ struct PortIdentity clock_parent_identity(struct clock *c)

int clock_poll(struct clock *c)
{
- int cnt, err, i, j, k, sde = 0;
+ int cnt, err, i, sde = 0;
enum fsm_event event;
+ struct pollfd *cur;
+ struct port *p;

- cnt = poll(c->pollfd, ARRAY_SIZE(c->pollfd), -1);
+ cnt = poll(c->pollfd, (c->nports + 1) * N_CLOCK_PFD, -1);
if (cnt < 0) {
if (EINTR == errno) {
return 0;
@@ -916,39 +991,38 @@ int clock_poll(struct clock *c)
return 0;
}

- for (i = 0; i < c->nports; i++) {
-
+ cur = c->pollfd;
+ LIST_FOREACH(p, &c->ports, list) {
/* Let the ports handle their events. */
- for (j = err = 0; j < N_POLLFD && !err; j++) {
- k = N_CLOCK_PFD * i + j;
- if (c->pollfd[k].revents & (POLLIN|POLLPRI)) {
- event = port_event(c->port[i], j);
+ for (i = err = 0; i < N_POLLFD && !err; i++) {
+ if (cur[i].revents & (POLLIN|POLLPRI)) {
+ event = port_event(p, i);
if (EV_STATE_DECISION_EVENT == event)
sde = 1;
if (EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES == event)
sde = 1;
- err = port_dispatch(c->port[i], event, 0);
+ err = port_dispatch(p, event, 0);
/* Clear any fault after a little while. */
- if (PS_FAULTY == port_state(c->port[i])) {
- clock_fault_timeout(c, i, 1);
+ if (PS_FAULTY == port_state(p)) {
+ clock_fault_timeout(p, 1);
break;
}
}
}

/* Check the fault timer. */
- k = N_CLOCK_PFD * i + N_POLLFD;
- if (c->pollfd[k].revents & (POLLIN|POLLPRI)) {
- clock_fault_timeout(c, i, 0);
- port_dispatch(c->port[i], EV_FAULT_CLEARED, 0);
+ if (cur[N_POLLFD].revents & (POLLIN|POLLPRI)) {
+ clock_fault_timeout(p, 0);
+ port_dispatch(p, EV_FAULT_CLEARED, 0);
}
+
+ cur += N_CLOCK_PFD;
}

/* Check the UDS port. */
- for (j = 0; j < N_POLLFD; j++) {
- k = N_CLOCK_PFD * i + j;
- if (c->pollfd[k].revents & (POLLIN|POLLPRI)) {
- event = port_event(c->uds_port, j);
+ for (i = 0; i < N_POLLFD; i++) {
+ if (cur[i].revents & (POLLIN|POLLPRI)) {
+ event = port_event(c->uds_port, i);
if (EV_STATE_DECISION_EVENT == event)
sde = 1;
}
@@ -1014,22 +1088,6 @@ void clock_peer_delay(struct clock *c, tmv_t ppd, double nrr)
stats_add_value(c->stats.delay, tmv_to_nanoseconds(ppd));
}

-void clock_remove_fda(struct clock *c, struct port *p, struct fdarray fda)
-{
- int i, j, k;
- for (i = 0; i < c->nports; i++) {
- if (p == c->port[i])
- break;
- }
- if (p == c->uds_port)
- i = c->nports;
- for (j = 0; j < N_POLLFD; j++) {
- k = N_CLOCK_PFD * i + j;
- c->pollfd[k].fd = -1;
- c->pollfd[k].events = 0;
- }
-}
-
int clock_slave_only(struct clock *c)
{
return c->dds.flags & DDS_SLAVE_ONLY;
@@ -1154,10 +1212,11 @@ static void handle_state_decision_event(struct clock *c)
{
struct foreign_clock *best = NULL, *fc;
struct ClockIdentity best_id;
- int fresh_best = 0, i;
+ struct port *piter;
+ int fresh_best = 0;

- for (i = 0; i < c->nports; i++) {
- fc = port_compute_best(c->port[i]);
+ LIST_FOREACH(piter, &c->ports, list) {
+ fc = port_compute_best(piter);
if (!fc)
continue;
if (!best || dscmp(&fc->dataset, &best->dataset) > 0)
@@ -1185,10 +1244,10 @@ static void handle_state_decision_event(struct clock *c)
c->best = best;
c->best_id = best_id;

- for (i = 0; i < c->nports; i++) {
+ LIST_FOREACH(piter, &c->ports, list) {
enum port_state ps;
enum fsm_event event;
- ps = bmc_state_decision(c, c->port[i]);
+ ps = bmc_state_decision(c, piter);
switch (ps) {
case PS_LISTENING:
event = EV_NONE;
@@ -1212,7 +1271,7 @@ static void handle_state_decision_event(struct clock *c)
event = EV_FAULT_DETECTED;
break;
}
- port_dispatch(c->port[i], event, fresh_best);
+ port_dispatch(piter, event, fresh_best);
}
}

diff --git a/clock.h b/clock.h
index bc66dce..d1bedca 100644
--- a/clock.h
+++ b/clock.h
@@ -108,12 +108,11 @@ void clock_follow_up_info(struct clock *c, struct follow_up_info_tlv *f);
struct ClockIdentity clock_identity(struct clock *c);

/**
- * Install a port's file descriptor array into its controlling clock.
+ * Informs clock that a file descriptor of one of its ports changed. The
+ * clock will rebuild its array of file descriptors to poll.
* @param c The clock instance.
- * @param p The port installing the array.
- * @param fda The port's open file decriptors for its sockets and timers.
*/
-void clock_install_fda(struct clock *c, struct port *p, struct fdarray fda);
+void clock_fda_changed(struct clock *c);

/**
* Manage the clock according to a given message.
@@ -166,14 +165,6 @@ void clock_peer_delay(struct clock *c, tmv_t ppd, double nrr);
int clock_poll(struct clock *c);

/**
- * Remove a port's file descriptor array from its controlling clock.
- * @param c The clock instance.
- * @param p The port removing the array.
- * @param fda The port's file decriptor array.
- */
-void clock_remove_fda(struct clock *c, struct port *p, struct fdarray fda);
-
-/**
* Obtain the slave-only flag from a clock's default data set.
* @param c The clock instance.
* @return The value of the clock's slave-only flag.
diff --git a/port.c b/port.c
index 6553569..d8b5c55 100644
--- a/port.c
+++ b/port.c
@@ -22,6 +22,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <sys/queue.h>

#include "bmc.h"
#include "clock.h"
@@ -60,6 +61,7 @@ struct nrate_estimator {
};

struct port {
+ LIST_ENTRY(port) list;
char *name;
struct clock *clock;
struct transport *trp;
@@ -199,6 +201,11 @@ int port_fault_fd(struct port *port)
return port->fault_fd;
}

+struct fdarray *port_fda(struct port *port)
+{
+ return &port->fda;
+}
+
int set_tmo_log(int fd, unsigned int scale, int log_seconds)
{
struct itimerspec tmo = {
@@ -1285,6 +1292,14 @@ static void flush_peer_delay(struct port *p)
}
}

+static void port_clear_fda(struct port *p, int count)
+{
+ int i;
+
+ for (i = 0; i < count; i++)
+ p->fda.fd[i] = -1;
+}
+
static void port_disable(struct port *p)
{
int i;
@@ -1295,12 +1310,13 @@ static void port_disable(struct port *p)

p->best = NULL;
free_foreign_masters(p);
- clock_remove_fda(p->clock, p, p->fda);
transport_close(p->trp, &p->fda);

for (i = 0; i < N_TIMER_FDS; i++) {
close(p->fda.fd[FD_ANNOUNCE_TIMER + i]);
}
+ port_clear_fda(p, N_TIMER_FDS);
+ clock_fda_changed(p->clock);
}

static int port_initialize(struct port *p)
@@ -1344,7 +1360,7 @@ static int port_initialize(struct port *p)

port_nrate_initialize(p);

- clock_install_fda(p->clock, p, p->fda);
+ clock_fda_changed(p->clock);
return 0;

no_tmo:
@@ -1360,16 +1376,18 @@ no_timers:

static int port_renew_transport(struct port *p)
{
+ int res;
+
if (!port_is_enabled(p)) {
return 0;
}
- clock_remove_fda(p->clock, p, p->fda);
transport_close(p->trp, &p->fda);
- if (transport_open(p->trp, p->name, &p->fda, p->timestamping)) {
- return -1;
- }
- clock_install_fda(p->clock, p, p->fda);
- return 0;
+ port_clear_fda(p, FD_ANNOUNCE_TIMER);
+ res = transport_open(p->trp, p->name, &p->fda, p->timestamping);
+ /* Need to call clock_fda_changed even if transport_open failed in
+ * order to update clock to the now closed descriptors. */
+ clock_fda_changed(p->clock);
+ return res;
}

/*
@@ -2161,6 +2179,11 @@ struct PortIdentity port_identity(struct port *p)
return p->portIdentity;
}

+int port_number(struct port *p)
+{
+ return portnum(p);
+}
+
int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
{
struct management_tlv *mgt;
@@ -2329,6 +2352,7 @@ struct port *port_open(int phc_index,
}
p->nrate.ratio = 1.0;

+ port_clear_fda(p, N_TIMER_FDS);
p->fault_fd = -1;
if (number) {
p->fault_fd = timerfd_create(CLOCK_MONOTONIC, 0);
diff --git a/port.h b/port.h
index 37f02ae..a294094 100644
--- a/port.h
+++ b/port.h
@@ -100,6 +100,13 @@ int port_forward(struct port *p, struct ptp_message *msg, int msglen);
struct PortIdentity port_identity(struct port *p);

/**
+ * Obtain a port number.
+ * @param p A port instance.
+ * @return The port number of 'p'.
+ */
+int port_number(struct port *p);
+
+/**
* Manage a port according to a given message.
* @param p A pointer previously obtained via port_open().
* @param ingress The port on which 'msg' was received.
@@ -158,6 +165,15 @@ struct port *port_open(int phc_index,
enum port_state port_state(struct port *port);

/**
+ * Return array of file descriptors for this port. The fault fd is not
+ * included.
+ * @param port A port instance
+ * @return Array of file descriptors. Unused descriptors are guranteed
+ * to be set to -1.
+ */
+struct fdarray *port_fda(struct port *port);
+
+/**
* Return file descriptor of the port.
* @param port A port instance.
* @return File descriptor or -1 if not applicable.

[Jiri Benc]

Remove the limit of MAX_PORTS ports also when parsing command line
arguments.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 12 +++++-----
clock.h | 5 +--
config.c | 66 +++++++++++++++++++++++++++++++------------------------------
config.h | 10 +++++---
ptp4l.c | 40 ++++++++++++++++--------------------
5 files changed, 66 insertions(+), 67 deletions(-)

diff --git a/clock.c b/clock.c
index e820997..c385009 100644
--- a/clock.c
+++ b/clock.c
@@ -612,15 +612,15 @@ static void clock_remove_port(struct clock *c, struct port *p)
port_close(p);
}

-struct clock *clock_create(int phc_index, struct interface *iface, int count,
+struct clock *clock_create(int phc_index, struct interfaces_head *ifaces,
enum timestamp_type timestamping, struct default_ds *dds,
enum servo_type servo)
{
- int i, fadj = 0, max_adj = 0.0, sw_ts = timestamping == TS_SOFTWARE ? 1 : 0;
+ int fadj = 0, max_adj = 0.0, sw_ts = timestamping == TS_SOFTWARE ? 1 : 0;
struct clock *c = &the_clock;
struct port *p;
char phc[32];
- struct interface udsif;
+ struct interface udsif, *iface;

memset(&udsif, 0, sizeof(udsif));
snprintf(udsif.name, sizeof(udsif.name), "%s", uds_path);
@@ -728,9 +728,9 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,
clock_fda_changed(c);

/* Create the ports. */
- for (i = 0; i < count; i++) {
- if (clock_add_port(c, phc_index, timestamping, &iface[i])) {
- pr_err("failed to open port %s", iface[i].name);
+ STAILQ_FOREACH(iface, ifaces, list) {
+ if (clock_add_port(c, phc_index, timestamping, iface)) {
+ pr_err("failed to open port %s", iface->name);
return NULL;
}
}
diff --git a/clock.h b/clock.h
index d1bedca..26065dd 100644
--- a/clock.h
+++ b/clock.h
@@ -62,14 +62,13 @@ UInteger8 clock_class(struct clock *c);
*
* @param phc_index PTP hardware clock device to use.
* Pass -1 to select CLOCK_REALTIME.
- * @param interface An array of network interfaces.
- * @param count The number of elements in @a interfaces.
+ * @param ifaces A queue of network interfaces.
* @param timestamping The timestamping mode for this clock.
* @param dds A pointer to a default data set for the clock.
* @param servo The servo that this clock will use.
* @return A pointer to the single global clock instance.
*/
-struct clock *clock_create(int phc_index, struct interface *iface, int count,
+struct clock *clock_create(int phc_index, struct interfaces_head *ifaces,
enum timestamp_type timestamping, struct default_ds *dds,
enum servo_type servo);

diff --git a/config.c b/config.c
index c86ab01..e6e0a3e 100644
--- a/config.c
+++ b/config.c
@@ -20,6 +20,7 @@
#include <float.h>
#include <limits.h>
#include <stdio.h>
+#include <stdlib.h>
#include <string.h>
#include "config.h"
#include "ether.h"
@@ -156,40 +157,40 @@ static enum parser_result parse_pod_setting(const char *option,
static enum parser_result parse_port_setting(const char *option,
const char *value,
struct config *cfg,
- int p)
+ struct interface *iface)
{
enum parser_result r;
int val;

- r = parse_pod_setting(option, value, &cfg->iface[p].pod);
+ r = parse_pod_setting(option, value, &iface->pod);
if (r != NOT_PARSED)
return r;

if (!strcmp(option, "network_transport")) {
if (!strcasecmp("L2", value))
- cfg->iface[p].transport = TRANS_IEEE_802_3;
+ iface->transport = TRANS_IEEE_802_3;
else if (!strcasecmp("UDPv4", value))
- cfg->iface[p].transport = TRANS_UDP_IPV4;
+ iface->transport = TRANS_UDP_IPV4;
else if (!strcasecmp("UDPv6", value))
- cfg->iface[p].transport = TRANS_UDP_IPV6;
+ iface->transport = TRANS_UDP_IPV6;
else
return BAD_VALUE;

} else if (!strcmp(option, "delay_mechanism")) {
if (!strcasecmp("Auto", value))
- cfg->iface[p].dm = DM_AUTO;
+ iface->dm = DM_AUTO;
else if (!strcasecmp("E2E", value))
- cfg->iface[p].dm = DM_E2E;
+ iface->dm = DM_E2E;
else if (!strcasecmp("P2P", value))
- cfg->iface[p].dm = DM_P2P;
+ iface->dm = DM_P2P;
else
return BAD_VALUE;

} else if (!strcmp(option, "delay_filter")) {
if (!strcasecmp("moving_average", value))
- cfg->iface[p].delay_filter = FILTER_MOVING_AVERAGE;
+ iface->delay_filter = FILTER_MOVING_AVERAGE;
else if (!strcasecmp("moving_median", value))
- cfg->iface[p].delay_filter = FILTER_MOVING_MEDIAN;
+ iface->delay_filter = FILTER_MOVING_MEDIAN;
else
return BAD_VALUE;

@@ -197,7 +198,7 @@ static enum parser_result parse_port_setting(const char *option,
r = get_ranged_int(value, &val, 1, INT_MAX);
if (r != PARSED_OK)
return r;
- cfg->iface[p].delay_filter_length = val;
+ iface->delay_filter_length = val;

} else
return NOT_PARSED;
@@ -572,7 +573,8 @@ int config_read(char *name, struct config *cfg)
enum parser_result parser_res;
FILE *fp;
char buf[1024], *line, *c, *option, *value;
- int current_port = 0, line_num;
+ struct interface *current_port = NULL;
+ int line_num;

fp = 0 == strncmp(name, "-", 2) ? stdin : fopen(name, "r");

@@ -608,8 +610,9 @@ int config_read(char *name, struct config *cfg)
goto parse_error;
}
current_port = config_create_interface(port, cfg);
- if (current_port < 0)
+ if (!current_port)
goto parse_error;
+ config_init_interface(current_port, cfg);
}
continue;
}
@@ -621,7 +624,7 @@ int config_read(char *name, struct config *cfg)
fprintf(stderr, "could not parse line %d in %s section\n",
line_num,
current_section == GLOBAL_SECTION ?
- "global" : cfg->iface[current_port].name);
+ "global" : current_port->name);
goto parse_error;
}

@@ -637,7 +640,7 @@ int config_read(char *name, struct config *cfg)
fprintf(stderr, "unknown option %s at line %d in %s section\n",
option, line_num,
current_section == GLOBAL_SECTION ?
- "global" : cfg->iface[current_port].name);
+ "global" : current_port->name);
goto parse_error;
case BAD_VALUE:
fprintf(stderr, "%s is a bad value for option %s at line %d\n",
@@ -671,36 +674,35 @@ parse_error:
return -2;
}

-/* returns the number matching that interface, or -1 on failure */
-int config_create_interface(char *name, struct config *cfg)
+struct interface *config_create_interface(char *name, struct config *cfg)
{
struct interface *iface;
- int i;

- if (cfg->nports >= MAX_PORTS) {
- fprintf(stderr, "more than %d ports specified\n", MAX_PORTS);
- return -1;
+ /* only create each interface once (by name) */
+ STAILQ_FOREACH(iface, &cfg->interfaces, list) {
+ if (0 == strncmp(name, iface->name, MAX_IFNAME_SIZE))
+ return iface;
}

- iface = &cfg->iface[cfg->nports];
-
- /* only create each interface once (by name) */
- for(i = 0; i < cfg->nports; i++) {
- if (0 == strncmp(name, cfg->iface[i].name, MAX_IFNAME_SIZE))
- return i;
+ iface = malloc(sizeof(struct interface));
+ if (!iface) {
+ fprintf(stderr, "cannot allocate memory for a port\n");
+ return NULL;
}

strncpy(iface->name, name, MAX_IFNAME_SIZE);
+ STAILQ_INSERT_TAIL(&cfg->interfaces, iface, list);
+ return iface;
+}
+
+void config_init_interface(struct interface *iface, struct config *cfg)
+{
iface->dm = cfg->dm;
iface->transport = cfg->transport;
memcpy(&iface->pod, &cfg->pod, sizeof(cfg->pod));

- sk_get_ts_info(name, &iface->ts_info);
+ sk_get_ts_info(iface->name, &iface->ts_info);

iface->delay_filter = cfg->dds.delay_filter;
iface->delay_filter_length = cfg->dds.delay_filter_length;
-
- cfg->nports++;
-
- return i;
}
diff --git a/config.h b/config.h
index a380037..466a155 100644
--- a/config.h
+++ b/config.h
@@ -20,6 +20,8 @@
#ifndef HAVE_CONFIG_H
#define HAVE_CONFIG_H

+#include <sys/queue.h>
+
#include "ds.h"
#include "dm.h"
#include "filter.h"
@@ -27,11 +29,11 @@
#include "servo.h"
#include "sk.h"

-#define MAX_PORTS 8
#define MAX_IFNAME_SIZE 108 /* = UNIX_PATH_MAX */

/** Defines a network interface, with PTP options. */
struct interface {
+ STAILQ_ENTRY(interface) list;
char name[MAX_IFNAME_SIZE + 1];
enum delay_mechanism dm;
enum transport_type transport;
@@ -62,8 +64,7 @@ struct config {
int cfg_ignore;

/* configured interfaces */
- struct interface iface[MAX_PORTS];
- int nports;
+ STAILQ_HEAD(interfaces_head, interface) interfaces;

enum timestamp_type timestamping;
enum transport_type transport;
@@ -102,6 +103,7 @@ struct config {
};

int config_read(char *name, struct config *cfg);
-int config_create_interface(char *name, struct config *cfg);
+struct interface *config_create_interface(char *name, struct config *cfg);
+void config_init_interface(struct interface *iface, struct config *cfg);

#endif
diff --git a/ptp4l.c b/ptp4l.c
index 5d8749a..2161854 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -41,6 +41,8 @@ int assume_two_step = 0;
static int running = 1;

static struct config cfg_settings = {
+ .interfaces = STAILQ_HEAD_INITIALIZER(cfg_settings.interfaces),
+
.dds = {
.dds = {
.flags = DDS_TWO_STEP_FLAG,
@@ -166,9 +168,7 @@ int main(int argc, char *argv[])
{
char *config = NULL, *req_phc = NULL, *progname;
int c, i;
- struct interface *iface = cfg_settings.iface;
- char *ports[MAX_PORTS];
- int nports = 0;
+ struct interface *iface;
int *cfg_ignore = &cfg_settings.cfg_ignore;
enum delay_mechanism *dm = &cfg_settings.dm;
enum transport_type *transport = &cfg_settings.transport;
@@ -241,8 +241,8 @@ int main(int argc, char *argv[])
config = optarg;
break;
case 'i':
- ports[nports] = optarg;
- nports++;
+ if (!config_create_interface(optarg, &cfg_settings))
+ return -1;
break;
case 'p':
req_phc = optarg;
@@ -292,14 +292,7 @@ int main(int argc, char *argv[])
print_set_syslog(cfg_settings.use_syslog);
print_set_level(cfg_settings.print_level);

- for (i = 0; i < nports; i++) {
- if (config_create_interface(ports[i], &cfg_settings) < 0) {
- fprintf(stderr, "too many interfaces\n");
- return -1;
- }
- }
-
- if (!cfg_settings.nports) {
+ if (STAILQ_EMPTY(&cfg_settings.interfaces)) {
fprintf(stderr, "no interface specified\n");
usage(progname);
return -1;
@@ -339,18 +332,21 @@ int main(int argc, char *argv[])
break;
}

- /* check whether timestamping mode is supported. */
- for (i = 0; i < cfg_settings.nports; i++) {
- if (iface[i].ts_info.valid &&
- ((iface[i].ts_info.so_timestamping & required_modes) != required_modes)) {
+ /* Init interface configs and check whether timestamping mode is
+ * supported. */
+ STAILQ_FOREACH(iface, &cfg_settings.interfaces, list) {
+ config_init_interface(iface, &cfg_settings);
+ if (iface->ts_info.valid &&
+ ((iface->ts_info.so_timestamping & required_modes) != required_modes)) {
fprintf(stderr, "interface '%s' does not support "
"requested timestamping mode.\n",
- iface[i].name);
+ iface->name);
return -1;
}
}

/* determine PHC Clock index */
+ iface = STAILQ_FIRST(&cfg_settings.interfaces);
if (cfg_settings.dds.free_running) {
phc_index = -1;
} else if (*timestamping == TS_SOFTWARE || *timestamping == TS_LEGACY_HW) {
@@ -360,8 +356,8 @@ int main(int argc, char *argv[])
fprintf(stderr, "bad ptp device string\n");
return -1;
}
- } else if (iface[0].ts_info.valid) {
- phc_index = iface[0].ts_info.phc_index;
+ } else if (iface->ts_info.valid) {
+ phc_index = iface->ts_info.phc_index;
} else {
fprintf(stderr, "ptp device not specified and\n"
"automatic determination is not\n"
@@ -373,12 +369,12 @@ int main(int argc, char *argv[])
pr_info("selected /dev/ptp%d as PTP clock", phc_index);
}

- if (generate_clock_identity(&ds->clockIdentity, iface[0].name)) {
+ if (generate_clock_identity(&ds->clockIdentity, iface->name)) {
fprintf(stderr, "failed to generate a clock identity\n");
return -1;
}

- clock = clock_create(phc_index, iface, cfg_settings.nports,
+ clock = clock_create(phc_index, &cfg_settings.interfaces,
*timestamping, &cfg_settings.dds,
cfg_settings.clock_servo);
if (!clock) {

[Jiri Benc]

For the ports to be truly created and removed dynamically, the last used
port number has to be remembered by the clock and used for port creation.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/clock.c b/clock.c
index 23b6ebd..e820997 100644
--- a/clock.c
+++ b/clock.c
@@ -77,6 +77,7 @@ struct clock {
struct pollfd *pollfd;
int pollfd_valid;
int nports; /* does not include the UDS port */
+ int last_port_number;
int free_running;
int freq_est_interval;
int utc_timescale;
@@ -581,14 +582,15 @@ UInteger8 clock_class(struct clock *c)
}

static int clock_add_port(struct clock *c, int phc_index,
- enum timestamp_type timestamping, int number,
+ enum timestamp_type timestamping,
struct interface *iface)
{
struct port *p;

if (clock_resize_pollfd(c, c->nports + 1))
return -1;
- p = port_open(phc_index, timestamping, number, iface, c);
+ p = port_open(phc_index, timestamping, ++c->last_port_number,
+ iface, c);
if (!p) {
/* No need to shrink pollfd */
return -1;
@@ -709,6 +711,7 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,
clock_sync_interval(c, 0);

LIST_INIT(&c->ports);
+ c->last_port_number = 0;

/*
* Create the UDS interface.
@@ -726,7 +729,7 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,

/* Create the ports. */
for (i = 0; i < count; i++) {
- if (clock_add_port(c, phc_index, timestamping, i + 1, &iface[i])) {
+ if (clock_add_port(c, phc_index, timestamping, &iface[i])) {
pr_err("failed to open port %s", iface[i].name);
return NULL;
}

[Jiri Benc]

Post by Jiri Benc
For the ports to be truly created and removed dynamically, the last used
port number has to be remembered by the clock and used for port creation.

Sorry for the wrong order, not my fault:

-----
A message that you sent could not be delivered to one or more of its
recipients. This is a permanent error. The following address(es) failed:

linuxptp-***@lists.sourceforge.net
SMTP error from remote mail server after RCPT TO:<linuxptp-***@lists.sourceforge.net>:
host sfs-lb-ml.v29.ch3.sourceforge.com [172.29.29.17]:
550 Unknown user
-----

I had to resend this mail.

Jiri

[Jiri Benc]

Post by Jiri Benc
Currently, there's a limit of MAX_PORTS ports (8 by default). This patchset
removes this limitation by using linked lists instead of fixed size arrays.
In addition to allowing ptp4l to work on machines with many interfaces, this
will allow for later patches to add support for adding and removing ports
dynamically while ptp4l is running.
The core patches are 3/6 and 6/6, the rest are prerequisites: patch 1/6 is
moving per-port fields from struct clock to individual ports in order not to
have to maintain several linked lists, patch 2/6 moves the uds port from the
port array, as having it a part of linked list would make the various checks
for uds port unnecessary complicated, patch 5/6 replaces the port numbering
that is based on array of interface names by a per clock counter. The
remaining patch, 4/6, is a follow up optimization to 3/6.
Changes in v3: used the BSD implementation of LIST_FOREACH_SAFE, using the
system one where available.

Richard,

does the series look better now?

Thanks,

Jiri

[Richard Cochran]

Post by Jiri Benc
does the series look better now?

I have not really had a chance to review this yet. Sorry to say it,
but I have put this series last after some other recent patches,
because to me the dynamic port idea is a "nice to have", and also the
memory and list management needs to be checked carefully. The fixed
array code has the benefit that it is really simple.

You mentioned that you want to implement "hot pluggable" ports on top
of this series. Do you have anything yet? If so, please post that too,
so we can get the big picture.

Thanks,
Richard

[Jiri Benc]

Post by Richard Cochran
You mentioned that you want to implement "hot pluggable" ports on top
of this series. Do you have anything yet? If so, please post that too,
so we can get the big picture.

Not yet, I'm working on it but there's still a long road ahead. My main
motivation is to support bonding (which probably sounds scarier than it
actually is), or alternatively, some other sort of automatic failover.

I'm currently experimenting with multiple PHC support, which is needed
for the above and, perhaps more interestingly, would allow things like
boundary clock with separate NICs. Of course, for this to be fully
useful, phc2sys will need to be extended to follow ptp4l's state
machine for individual interfaces. IIRC Jake expressed interest in such
phc2sys extension, too
(http://sourceforge.net/mailarchive/message.php?msg_id=31176225).

(I'm aware that boundary clock with separate but synchronized clocks is
kind of out of PTP spec but I think it could be done in the way that's
indistinguishable from the single clock from the other device's point
of view, which is what really matters. There will be some impact on
precision, of course.)

Jiri

[Keller, Jacob E]

Post by Jiri Benc

Post by Richard Cochran
You mentioned that you want to implement "hot pluggable" ports on top
of this series. Do you have anything yet? If so, please post that too,
so we can get the big picture.

Not yet, I'm working on it but there's still a long road ahead. My main
motivation is to support bonding (which probably sounds scarier than it
actually is), or alternatively, some other sort of automatic failover.
I'm currently experimenting with multiple PHC support, which is needed
for the above and, perhaps more interestingly, would allow things like
boundary clock with separate NICs. Of course, for this to be fully
useful, phc2sys will need to be extended to follow ptp4l's state
machine for individual interfaces. IIRC Jake expressed interest in such
phc2sys extension, too
(http://sourceforge.net/mailarchive/message.php?msg_id=31176225).
(I'm aware that boundary clock with separate but synchronized clocks is
kind of out of PTP spec but I think it could be done in the way that's
indistinguishable from the single clock from the other device's point
of view, which is what really matters. There will be some impact on
precision, of course.)
Jiri

I like this idea. I also think if we supported the "push" updates from
ptp4l, where ptp4l sends data over the management interface it would
enable the phc2sys to update it's internal state without requiring
polling as it could update whenever it receives data on the management
socket.

This type of setup also enables us to support phc2sys working "out of
the box" by automatically setting up the slave/master relationship so
that it doesn't require manual setup which can be a little confusing.

I do think that, while this is not in spec, it would be very useful for
boards which don't support boundary clock due to design. Obviously there
is some precision impact.. I am curious how difficult it would be

[Richard Cochran]

Post by Keller, Jacob E
I like this idea. I also think if we supported the "push" updates from
ptp4l, where ptp4l sends data over the management interface it would
enable the phc2sys to update it's internal state without requiring
polling as it could update whenever it receives data on the management
socket.

Right, I like the push idea too, and for me it has higher priority
than removing the static port array.

Post by Keller, Jacob E
I do think that, while this is not in spec, it would be very useful for
boards which don't support boundary clock due to design. Obviously there
is some precision impact.. I am curious how difficult it would be to
measure that.

Here is one way to measure the time error in the second PHC device.
Attached two port to a grand master via a transparent switch. Run
ptp4l on one port normally and use phc2sys to discipline the second
port. Then, on the second port start another ptp4l but in free running
mode.

+--------+ +--------+ +--------+-------------------------+
| | | |----| Port 1 | ptp4l, phc2sys to port2 |
| GM |---| TC | +--------+ |
| | | |----| Port 2 | ptp4l free running |
+--------+ +--------+ +--------+-------------------------+

Thanks,
Richard

[Keller, Jacob E]

Post by Richard Cochran

Post by Keller, Jacob E
I like this idea. I also think if we supported the "push" updates from
ptp4l, where ptp4l sends data over the management interface it would
enable the phc2sys to update it's internal state without requiring
polling as it could update whenever it receives data on the management
socket.

Right, I like the push idea too, and for me it has higher priority
than removing the static port array.

I was thinking about ways to implement push-based messages...

What do you think about having an extended message which indicates "I am
ready to receive".. then once ptp4l gets this, it then starts pushing a
set of notifications on events.

I like this better than a configuration option, since it means ptp4l
only sends messages on the uds when some management interface has
requested it...

Thoughts?

It also doesn't seem too difficult to implement.. I haven't had time yet
to try my hand at something, but I just wanted to see if we could hash
out implementation

[Richard Cochran]

Post by Keller, Jacob E
What do you think about having an extended message which indicates "I am
ready to receive".. then once ptp4l gets this, it then starts pushing a
set of notifications on events.
I like this better than a configuration option, since it means ptp4l
only sends messages on the uds when some management interface has
requested it...
Thoughts?

I think this should be "configurable" at run time, *not* as a
configuration file option. I imagine having a custom management
message that allows the client to request different kinds of status
information, like port state, BMC result, and so on. (Or maybe the
client can specify management ID values.) Whenever any of the
"watched" status bits change value, then ptp4l will push RESPONSE
messages as if replying to a GET.

How does that sound to you?

Thanks,
Richard

[Stephan Gatzka]

Post by Richard Cochran
I think this should be "configurable" at run time, *not* as a
configuration file option. I imagine having a custom management
message that allows the client to request different kinds of status
information, like port state, BMC result, and so on. (Or maybe the
client can specify management ID values.) Whenever any of the
"watched" status bits change value, then ptp4l will push RESPONSE
messages as if replying to a GET.
How does that sound to you?

Sounds great to me.

Regards,

Stephan

[Keller, Jacob E]

Post by Richard Cochran

Post by Keller, Jacob E
What do you think about having an extended message which indicates "I am
ready to receive".. then once ptp4l gets this, it then starts pushing a
set of notifications on events.
I like this better than a configuration option, since it means ptp4l
only sends messages on the uds when some management interface has
requested it...
Thoughts?

I think this should be "configurable" at run time, *not* as a
configuration file option. I imagine having a custom management
message that allows the client to request different kinds of status
information, like port state, BMC result, and so on. (Or maybe the
client can specify management ID values.) Whenever any of the
"watched" status bits change value, then ptp4l will push RESPONSE
messages as if replying to a GET.
How does that sound to you?

That's exactly what I was imagining with my post above.

I also dislike the idea of static configured at start. This seems the
most useful.

How would this work with multiple management interfaces? Is that even
possible?

IE: multiple programs connected? Does the management interface properly
only send to a single client? If so we can use the client-request
details to send the data across to only that client. This way only
clients which perform the proper req

[Richard Cochran]

Post by Keller, Jacob E
How would this work with multiple management interfaces? Is that even
possible?
IE: multiple programs connected? Does the management interface properly
only send to a single client? If so we can use the client-request
details to send the data across to only that client. This way only
clients which perform the proper request message get the result.

On the UDS, the ptp4l server sends replies back to the particular
client who made the request. The client's address is stored in the
'sa' field of the 'struct uds'. There is never more that one
outstanding request, and so only the last address is stored.

[ There is no such thing as multicast unix domain sockets. ]

We will have to come up with a way of keeping track of clients wanting
pushed messages.

Thanks,
Richard

[Keller, Jacob E]

-----Original Message-----
Sent: Tuesday, December 10, 2013 10:22 AM
To: Keller, Jacob E
Subject: Re: [Linuxptp-devel] ptp4l push notifications

Post by Keller, Jacob E
How would this work with multiple management interfaces? Is that even
possible?
IE: multiple programs connected? Does the management interface

properly

Post by Keller, Jacob E
only send to a single client? If so we can use the client-request
details to send the data across to only that client. This way only
clients which perform the proper request message get the result.

On the UDS, the ptp4l server sends replies back to the particular
client who made the request. The client's address is stored in the
'sa' field of the 'struct uds'. There is never more that one
outstanding request, and so only the last address is stored.
[ There is no such thing as multicast unix domain sockets. ]
We will have to come up with a way of keeping track of clients wanting
pushed messages.
Thanks,
Richard

I'm not too familiar with management.. is it possible for management messages to come in via the network socket?

I think we should be able to store push requests via something like a hash..?

Create an array of things which can be pushed, then if the array slot is not null, it's a linked list of all the requests so far? (assuming correct that we want to support multiple clients requesting) Then each request could be a store of the address that requested that push notification? Linked list in this case would be fairly cheap, and we wouldn't have to sort them. The hash enables fast lookup of each push notification without having to scan a single list for every push notification.

Regards,
Jake

[Richard Cochran]

Post by Keller, Jacob E
I'm not too familiar with management.. is it possible for management
messages to come in via the network socket?

Yes, but I think this special push information should only appear on
the loacl UDS interface. Putting unsolicited messages on the network
would go against the standard.

The idea is to make life easier for local programs like phc2sys.

Post by Keller, Jacob E
Create an array of things which can be pushed, then if the array
slot is not null, it's a linked list of all the requests so far?
(assuming correct that we want to support multiple clients
requesting) Then each request could be a store of the address that
requested that push notification? Linked list in this case would be
fairly cheap, and we wouldn't have to sort them. The hash enables
fast lookup of each push notification without having to scan a
single list for every push notification.

First of all, we will not have that many local listeners, maybe just
phc2sys and perhaps one or two others. So, we don't need to get too
fancy.

In uds.c, we can keep a list of {address, msg_bitmap} where the bit
map has slots for the different management messages like "current data
set", "parent data set", and so on.

We will have to add hooks throughout the code that trigger when the
program state changes in such a way as to change one of the possible
data sets represented by msg_bitmap. The hook will simply send the
needed message or messages.

Then, the code uds.c will match the message type against each list
entry and send it if it matches.

Thanks,
Richard

[Keller, Jacob E]

Post by Richard Cochran

Post by Keller, Jacob E
I'm not too familiar with management.. is it possible for management
messages to come in via the network socket?

Yes, but I think this special push information should only appear on
the loacl UDS interface. Putting unsolicited messages on the network
would go against the standard.
The idea is to make life easier for local programs like phc2sys.

Post by Keller, Jacob E
Create an array of things which can be pushed, then if the array
slot is not null, it's a linked list of all the requests so far?
(assuming correct that we want to support multiple clients
requesting) Then each request could be a store of the address that
requested that push notification? Linked list in this case would be
fairly cheap, and we wouldn't have to sort them. The hash enables
fast lookup of each push notification without having to scan a
single list for every push notification.

First of all, we will not have that many local listeners, maybe just
phc2sys and perhaps one or two others. So, we don't need to get too
fancy.
In uds.c, we can keep a list of {address, msg_bitmap} where the bit
map has slots for the different management messages like "current data
set", "parent data set", and so on.
We will have to add hooks throughout the code that trigger when the
program state changes in such a way as to change one of the possible
data sets represented by msg_bitmap. The hook will simply send the
needed message or messages.
Then, the code uds.c will match the message type against each list
entry and send it if it matches.
Thanks,