[Linuxptp-devel] [PATCH 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.

There's probably a conflict with the yet unapplied RFC series "Configurable
UDS server path" but it should be easy to fix up, there's nothing
fundamental conflicting between these two sets.

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 | 307 +++++++++++++++++++++++++++++++++++++++-----------------------
clock.h | 20 +---
config.c | 60 ++++++------
config.h | 10 +-
port.c | 87 ++++++++++++++----
port.h | 45 +++++++++
ptp4l.c | 40 ++++-----
7 files changed, 367 insertions(+), 202 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 | 47 ++++++++++++++++++++++++++++++++++++++---------
port.h | 29 +++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 22 deletions(-)

diff --git a/clock.c b/clock.c
index 819421e..f1c2663 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;
@@ -110,10 +109,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);
@@ -133,7 +130,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);
@@ -141,11 +138,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);
@@ -666,12 +663,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 d98bb80..b8f59f6 100644
--- a/port.c
+++ b/port.c
@@ -66,6 +66,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;
@@ -194,6 +195,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 = {
@@ -231,6 +237,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;
@@ -1846,6 +1863,8 @@ void port_close(struct port *p)
}
transport_destroy(p->trp);
mave_destroy(p->avg_delay);
+ if (p->fault_fd >= 0)
+ close(p->fault_fd);
free(p);
}

@@ -2284,18 +2303,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;
@@ -2306,12 +2322,25 @@ struct port *port_open(int phc_index,
p->avg_delay = mave_create(PORT_MAVE_LENGTH);
if (!p->avg_delay) {
pr_err("Failed to create moving average");
- 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_transport;
+ }
+ }
return p;
+
+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 0065d9e..e66502e 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;
@@ -807,16 +808,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,
@@ -970,6 +979,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]

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 5b4686e..0065d9e 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 MAVE_LENGTH 10
#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;
@@ -100,18 +103,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));
}

+#define LIST_FOREACH_SAFE(var, tmp, head, field) \
+ for ((var) = ((head)->lh_first), \
+ (tmp) = (var) ? ((var)->field.le_next) : NULL; \
+ (var); \
+ (var) = (tmp), \
+ (tmp) = (var) ? ((var)->field.le_next) : NULL)
+
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, tmp, &c->ports, list) {
+ clock_remove_port(c, p);
+ }
port_close(c->uds_port);
+ free(c->pollfd);
if (c->clkid != CLOCK_REALTIME) {
phc_close(c->clkid);
}
@@ -124,25 +139,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);
@@ -563,12 +578,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;

@@ -650,37 +696,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;
@@ -733,21 +779,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,
@@ -768,14 +837,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) {
@@ -787,7 +859,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}
@@ -871,8 +944,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;
@@ -892,10 +965,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;
@@ -907,39 +982,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;
}
@@ -1005,22 +1079,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;
@@ -1138,10 +1196,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)
@@ -1169,10 +1228,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;
@@ -1196,7 +1255,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 309f731..1293ebb 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 b8f59f6..e37c475 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"
@@ -61,6 +62,7 @@ struct nrate_estimator {
};

struct port {
+ LIST_ENTRY(port) list;
char *name;
struct clock *clock;
struct transport *trp;
@@ -200,6 +202,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 = {
@@ -1286,6 +1293,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;
@@ -1296,12 +1311,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)
@@ -1345,7 +1361,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:
@@ -1361,16 +1377,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;
}

/*
@@ -2159,6 +2177,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;
@@ -2326,6 +2349,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]

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 e66502e..f192158 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;
@@ -580,14 +581,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;
@@ -700,6 +702,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.
@@ -717,7 +720,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]

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 f1c2663..5b4686e 100644
--- a/clock.c
+++ b/clock.c
@@ -38,7 +38,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 MAVE_LENGTH 10
#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;
@@ -111,7 +111,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);
}
@@ -312,7 +312,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;
}
@@ -550,7 +550,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;
@@ -668,10 +668,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;
}
@@ -681,7 +681,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;
}
@@ -736,10 +736,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];
@@ -747,26 +750,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++;
@@ -928,7 +939,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;
}
@@ -997,10 +1008,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 also when parsing command line
arguments.

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

diff --git a/clock.c b/clock.c
index f192158..93fb9ef 100644
--- a/clock.c
+++ b/clock.c
@@ -611,7 +611,7 @@ 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)
{
@@ -619,7 +619,7 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,
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), UDS_PATH);
@@ -719,9 +719,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 1293ebb..436b582 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 8cae57f..3fa3e20 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,31 +157,31 @@ 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;

- 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
@@ -531,7 +532,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;
+ int line_num;

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

@@ -567,8 +569,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;
}
@@ -580,7 +583,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;
}

@@ -596,7 +599,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",
@@ -630,33 +633,32 @@ 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);
-
- cfg->nports++;
-
- return i;
+ sk_get_ts_info(iface->name, &iface->ts_info);
}
diff --git a/config.h b/config.h
index 94704a4..38e5319 100644
--- a/config.h
+++ b/config.h
@@ -20,17 +20,19 @@
#ifndef HAVE_CONFIG_H
#define HAVE_CONFIG_H

+#include <sys/queue.h>
+
#include "ds.h"
#include "dm.h"
#include "transport.h"
#include "servo.h"
#include "sk.h"

-#define MAX_PORTS 8
#define MAX_IFNAME_SIZE 16

/** 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;
@@ -59,8 +61,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;
@@ -96,6 +97,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 b0d1c9c..3fbbf86 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -40,6 +40,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,
@@ -161,9 +163,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;
@@ -236,8 +236,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;
@@ -287,14 +287,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;
@@ -334,18 +327,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) {
@@ -355,8 +351,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"
@@ -368,12 +364,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) {

[Richard Cochran]

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.

Out of curiosity, this is series more theoretical, or do you have some
hardware that suffers under the current limitations?

I haven't found any hardware really suitable usable as a Boundary
Clock (that is, multiple ports all sharing one clock).

Thanks,
Richard

[Jiri Benc]

Post by Richard Cochran
Out of curiosity, this is series more theoretical, or do you have some
hardware that suffers under the current limitations?

I don't have such hardware, although I can imagine that the fixed limit
can be a limitation in the future. What I'm going after is the second
part of what I wrote, dynamic adding and removing of ports while ptp4l
is running.

Post by Richard Cochran
I haven't found any hardware really suitable usable as a Boundary
Clock (that is, multiple ports all sharing one clock).

Software time stamping on a machine with several interfaces would work.
Obviously, the accuracy won't be nowhere near "amazing".

One thing that may be interesting is a fake boundary clock, with slave
PHCs synchronized to a master PHC, but we'll need multiple clock support
in ptp4l for that.

Jiri

[Dale Smith]

On Sat, Oct 19, 2013 at 1:46 AM, Richard Cochran

Post by Richard Cochran
I haven't found any hardware really suitable usable as a Boundary
Clock (that is, multiple ports all sharing one clock).

The 1588 HW clock on the freescale P2020 (gianfar) is shared by all the
ethernet interfaces and could be used as a boundary clock.

-Dale

[Richard Cochran]

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.

Just getting around to reviewing this. Sorry for the delay. I applied
this series onto 52c5e0c, and it applied there cleanly, but I get some
compilation warnings (see below).

Thanks,
Richard


/home/richard/git/linuxptp/clock.c: In function ‘clock_create’:
/home/richard/git/linuxptp/clock.c:618:6: error: unused variable ‘i’ [-Werror=unused-variable]
cc1: all warnings being treated as errors
make: *** [clock.o] Error 1
make: *** Waiting for unfinished jobs....
/home/richard/git/linuxptp/config.c: In function ‘config_read’:
/home/richard/git/linuxptp/config.c:184:14: error: ‘current_port’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
/home/richard/git/linuxptp/config.c:535:20: note: ‘current_port’ was declared here
cc1: all warnings being treated as errors
make: *** [config.o] Error 1

[Jiri Benc]

Post by Richard Cochran
/home/richard/git/linuxptp/clock.c:618:6: error: unused variable ‘i’ [-Werror=unused-variable]

I wonder how I missed that, thanks.

Post by Richard Cochran
cc1: all warnings being treated as errors
make: *** [clock.o] Error 1
make: *** Waiting for unfinished jobs....
/home/richard/git/linuxptp/config.c:184:14: error: ‘current_port’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This is false positive, I actually thought of this when writing the
patch. My gcc doesn't warn about it. I'll silence the warning by
initializing the variable to NULL.

I'll send the series rebased to the current git shortly.

Jiri