[Linuxptp-devel] [PATCH RFC v1 1/9] clock: store the configuration in the clock data structure.

[Richard Cochran]

This will help us to simplify the passing of parameters between the main
program, clock, and ports.

Signed-off-by: Richard Cochran <***@gmail.com>
---
clock.c | 9 ++++++---
clock.h | 8 +++++---
ptp4l.c | 3 ++-
3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/clock.c b/clock.c
index 9e9feba..1569108 100644
--- a/clock.c
+++ b/clock.c
@@ -73,6 +73,7 @@ struct clock_subscriber {
};

struct clock {
+ struct config *config;
clockid_t clkid;
struct servo *servo;
enum servo_type servo_type;
@@ -792,9 +793,10 @@ static void clock_remove_port(struct clock *c, struct port *p)
port_close(p);
}

-struct clock *clock_create(int phc_index, struct interfaces_head *ifaces,
- enum timestamp_type timestamping, struct default_ds *dds,
- enum servo_type servo)
+struct clock *clock_create(struct config *config, int phc_index,
+ struct interfaces_head *ifaces,
+ enum timestamp_type timestamping,
+ struct default_ds *dds, enum servo_type servo)
{
int fadj = 0, max_adj = 0, sw_ts = timestamping == TS_SOFTWARE ? 1 : 0;
struct clock *c = &the_clock;
@@ -814,6 +816,7 @@ struct clock *clock_create(int phc_index, struct interfaces_head *ifaces,
udsif->transport = TRANS_UDS;
udsif->delay_filter_length = 1;

+ c->config = config;
c->free_running = dds->free_running;
c->freq_est_interval = dds->freq_est_interval;
c->grand_master_capable = dds->grand_master_capable;
diff --git a/clock.h b/clock.h
index a8286dd..1e6cd98 100644
--- a/clock.h
+++ b/clock.h
@@ -61,6 +61,7 @@ UInteger8 clock_class(struct clock *c);
* Create a clock instance. There can only be one clock in any system,
* so subsequent calls will destroy the previous clock instance.
*
+ * @param config Pointer to the configuration database.
* @param phc_index PTP hardware clock device to use.
* Pass -1 to select CLOCK_REALTIME.
* @param ifaces A queue of network interfaces.
@@ -69,9 +70,10 @@ UInteger8 clock_class(struct clock *c);
* @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 interfaces_head *ifaces,
- enum timestamp_type timestamping, struct default_ds *dds,
- enum servo_type servo);
+struct clock *clock_create(struct config *config, int phc_index,
+ struct interfaces_head *ifaces,
+ enum timestamp_type timestamping,
+ struct default_ds *dds, enum servo_type servo);

/**
* Obtains a clock's default data set.
diff --git a/ptp4l.c b/ptp4l.c
index 61c5854..56cb8bd 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -377,7 +377,8 @@ int main(int argc, char *argv[])
return -1;
}

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

[Richard Cochran]

This function allows the ports to read configuration variables without
changing the port method signatures.

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

diff --git a/clock.c b/clock.c
index 1569108..a47e1e9 100644
--- a/clock.c
+++ b/clock.c
@@ -755,6 +755,11 @@ UInteger8 clock_class(struct clock *c)
return c->dds.clockQuality.clockClass;
}

+struct config *clock_config(struct clock *c)
+{
+ return c->config;
+}
+
static int clock_add_port(struct clock *c, int phc_index,
enum timestamp_type timestamping,
struct interface *iface)
diff --git a/clock.h b/clock.h
index 1e6cd98..44dac3a 100644
--- a/clock.h
+++ b/clock.h
@@ -58,6 +58,13 @@ struct port *clock_best_port(struct clock *c);
UInteger8 clock_class(struct clock *c);

/**
+ * Obrains a reference to the configuration database.
+ * @param c The clock instance.
+ * @return A pointer to the configuration, without fail.
+ */
+struct config *clock_config(struct clock *c);
+
+/**
* Create a clock instance. There can only be one clock in any system,
* so subsequent calls will destroy the previous clock instance.
*

[Delio Brignoli]

Hi Richard,

Post by Richard Cochran
This function allows the ports to read configuration variables without
changing the port method signatures.
---
clock.c | 5 +++++
clock.h | 7 +++++++
2 files changed, 12 insertions(+)

[…]

Post by Richard Cochran
diff --git a/clock.h b/clock.h
index 1e6cd98..44dac3a 100644
--- a/clock.h
+++ b/clock.h
@@ -58,6 +58,13 @@ struct port *clock_best_port(struct clock *c);
UInteger8 clock_class(struct clock *c);
/**
+ * Obrains a reference to the configuration database.

Typo ^^^^^^^

Regards
—
Delio




------------------------------------------------------------------------------

[Richard Cochran]

This will allow modules to read out various user options.

Signed-off-by: Richard Cochran <***@gmail.com>
---
pmc_common.c | 2 +-
port.c | 2 +-
transport.c | 7 +++++--
transport.h | 6 +++++-
transport_private.h | 1 +
5 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/pmc_common.c b/pmc_common.c
index 0a365f5..d92b0cd 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -85,7 +85,7 @@ struct pmc *pmc_create(struct config *cfg, enum transport_type transport_type,
pmc->domain_number = domain_number;
pmc->transport_specific = transport_specific;

- pmc->transport = transport_create(transport_type);
+ pmc->transport = transport_create(cfg, transport_type);
if (!pmc->transport) {
pr_err("failed to create transport");
goto failed;
diff --git a/port.c b/port.c
index 3ed5e7b..e5d62b9 100644
--- a/port.c
+++ b/port.c
@@ -2524,7 +2524,7 @@ struct port *port_open(int phc_index,
p->pod = interface->pod;
p->name = interface->name;
p->clock = clock;
- p->trp = transport_create(interface->transport);
+ p->trp = transport_create(clock_config(clock), interface->transport);
if (!p->trp)
goto err_port;
p->timestamping = timestamping;
diff --git a/transport.c b/transport.c
index fc18740..d24c05b 100644
--- a/transport.c
+++ b/transport.c
@@ -87,7 +87,8 @@ enum transport_type transport_type(struct transport *t)
return t->type;
}

-struct transport *transport_create(enum transport_type type)
+struct transport *transport_create(struct config *cfg,
+ enum transport_type type)
{
struct transport *t = NULL;
switch (type) {
@@ -108,8 +109,10 @@ struct transport *transport_create(enum transport_type type)
case TRANS_PROFINET:
break;
}
- if (t)
+ if (t) {
t->type = type;
+ t->cfg = cfg;
+ }
return t;
}

diff --git a/transport.h b/transport.h
index 8e0d421..5d6ba98 100644
--- a/transport.h
+++ b/transport.h
@@ -26,6 +26,8 @@
#include "fd.h"
#include "msg.h"

+struct config;
+
/* Values from networkProtocol enumeration 7.4.1 Table 3 */
enum transport_type {
/* 0 is Reserved in spec. Use it for UDS */
@@ -123,10 +125,12 @@ int transport_protocol_addr(struct transport *t, uint8_t *addr);

/**
* Allocate an instance of the specified transport.
+ * @param config Pointer to the configuration database.
* @param type Which transport to obtain.
* @return Pointer to a transport instance on success, NULL otherwise.
*/
-struct transport *transport_create(enum transport_type type);
+struct transport *transport_create(struct config *cfg,
+ enum transport_type type);

/**
* Free an instance of a transport.
diff --git a/transport_private.h b/transport_private.h
index 3e0ba9a..b54f32a 100644
--- a/transport_private.h
+++ b/transport_private.h
@@ -28,6 +28,7 @@

struct transport {
enum transport_type type;
+ struct config *cfg;

int (*close)(struct transport *t, struct fdarray *fda);

[Richard Cochran]

In the near future, the transports will need to consult the configuration
database in order to obtain various options. This patch also introduces
the idea of a configuration file into the phc2sys and pmc programs.

Signed-off-by: Richard Cochran <***@gmail.com>
---
phc2sys.c | 10 ++++++----
pmc.c | 3 ++-
pmc_common.c | 7 ++++---
pmc_common.h | 8 +++++---
4 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 9ff5bf9..5ba99c3 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -119,6 +119,8 @@ struct node {
struct clock *master;
};

+static struct config phc2sys_config;
+
static int update_pmc(struct node *node, int subscribe);
static int clock_handle_leap(struct node *node, struct clock *clock,
int64_t offset, uint64_t ts);
@@ -777,13 +779,13 @@ static void send_subscription(struct node *node)
pmc_send_set_action(node->pmc, TLV_SUBSCRIBE_EVENTS_NP, &sen, sizeof(sen));
}

-static int init_pmc(struct node *node, int domain_number)
+static int init_pmc(struct config *cfg, struct node *node, int domain_number)
{
char uds_local[MAX_IFNAME_SIZE + 1];

snprintf(uds_local, sizeof(uds_local), "/var/run/phc2sys.%d",
getpid());
- node->pmc = pmc_create(TRANS_UDS, uds_local, 0, domain_number, 0, 1);
+ node->pmc = pmc_create(cfg, TRANS_UDS, uds_local, 0, domain_number, 0, 1);
if (!node->pmc) {
pr_err("failed to create pmc");
return -1;
@@ -1395,7 +1397,7 @@ int main(int argc, char *argv[])
print_set_level(print_level);

if (autocfg) {
- if (init_pmc(&node, domain_number))
+ if (init_pmc(&phc2sys_config, &node, domain_number))
return -1;
if (auto_init_ports(&node, rt) < 0)
return -1;
@@ -1431,7 +1433,7 @@ int main(int argc, char *argv[])
r = -1;

if (wait_sync) {
- if (init_pmc(&node, domain_number))
+ if (init_pmc(&phc2sys_config, &node, domain_number))
goto end;

while (is_running()) {
diff --git a/pmc.c b/pmc.c
index 0774f88..1fe8183 100644
--- a/pmc.c
+++ b/pmc.c
@@ -42,6 +42,7 @@
#define P41 ((double)(1ULL << 41))

static struct pmc *pmc;
+static struct config pmc_config;

static void do_get_action(int action, int index, char *str);
static void do_set_action(int action, int index, char *str);
@@ -816,7 +817,7 @@ int main(int argc, char *argv[])
print_set_syslog(1);
print_set_verbose(1);

- pmc = pmc_create(transport_type, iface_name, boundary_hops,
+ pmc = pmc_create(&pmc_config, transport_type, iface_name, boundary_hops,
domain_number, transport_specific, zero_datalen);
if (!pmc) {
fprintf(stderr, "failed to create pmc\n");
diff --git a/pmc_common.c b/pmc_common.c
index afd6e5f..0a365f5 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -61,9 +61,10 @@ struct pmc {
int zero_length_gets;
};

-struct pmc *pmc_create(enum transport_type transport_type, const char *iface_name,
- UInteger8 boundary_hops, UInteger8 domain_number,
- UInteger8 transport_specific, int zero_datalen)
+struct pmc *pmc_create(struct config *cfg, enum transport_type transport_type,
+ const char *iface_name, UInteger8 boundary_hops,
+ UInteger8 domain_number, UInteger8 transport_specific,
+ int zero_datalen)
{
struct pmc *pmc;

diff --git a/pmc_common.h b/pmc_common.h
index 9adb9d1..bb3a1f1 100644
--- a/pmc_common.h
+++ b/pmc_common.h
@@ -21,14 +21,16 @@
#ifndef HAVE_PMC_COMMON_H
#define HAVE_PMC_COMMON_H

+#include "config.h"
#include "msg.h"
#include "transport.h"

struct pmc;

-struct pmc *pmc_create(enum transport_type transport_type, const char *iface_name,
- UInteger8 boundary_hops, UInteger8 domain_number,
- UInteger8 transport_specific, int zero_datalen);
+struct pmc *pmc_create(struct config *cfg, enum transport_type transport_type,
+ const char *iface_name, UInteger8 boundary_hops,
+ UInteger8 domain_number, UInteger8 transport_specific,
+ int zero_datalen);

void pmc_destroy(struct pmc *pmc);

[Richard Cochran]

Signed-off-by: Richard Cochran <***@gmail.com>
---
hash.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
hash.h | 59 +++++++++++++++++++++++++++++++++
makefile | 2 +-
3 files changed, 173 insertions(+), 1 deletion(-)
create mode 100644 hash.c
create mode 100644 hash.h

diff --git a/hash.c b/hash.c
new file mode 100644
index 0000000..e768692
--- /dev/null
+++ b/hash.c
@@ -0,0 +1,113 @@
+/**
+ * @file hash.c
+ * @brief Implements a simple hash table.
+ * @note Copyright (C) 2015 Richard Cochran <***@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#include <stdlib.h>
+#include <string.h>
+
+#include "hash.h"
+
+#define HASH_TABLE_SIZE 200
+
+struct node {
+ char *key;
+ void *data;
+ struct node *next;
+};
+
+struct hash {
+ struct node *table[HASH_TABLE_SIZE];
+};
+
+static unsigned int hash_function(const char* s)
+{
+ unsigned int i;
+
+ for (i = 0; *s; s++) {
+ i = 131 * i + *s;
+ }
+ return i % HASH_TABLE_SIZE;
+}
+
+struct hash *hash_create(void)
+{
+ struct hash *ht = calloc(1, sizeof(*ht));
+ return ht;
+}
+
+void hash_destroy(struct hash *ht, void (*func)(void *))
+{
+ unsigned int i;
+ struct node *n, *next, **table = ht->table;
+
+ for (i = 0; i < HASH_TABLE_SIZE; i++) {
+ for (n = table[i] ; n; n = next) {
+ next = n->next;
+ if (func) {
+ func(n->data);
+ }
+ free(n->key);
+ free(n);
+ }
+ }
+
+ free(ht);
+}
+
+int hash_insert(struct hash *ht, const char* key, void *data)
+{
+ unsigned int h;
+ struct node *n, **table = ht->table;
+
+ h = hash_function(key);
+
+ for (n = table[h] ; n; n = n->next) {
+ if (!strcmp(n->key, key)) {
+ /* reject duplicate keys */
+ return -1;
+ }
+ }
+ n = calloc(1, sizeof(*n));
+ if (!n) {
+ return -1;
+ }
+ n->key = strdup(key);
+ if (!n->key) {
+ free(n);
+ return -1;
+ }
+ n->data = data;
+ n->next = table[h];
+ table[h] = n;
+ return 0;
+}
+
+void *hash_lookup(struct hash *ht, const char* key)
+{
+ unsigned int h;
+ struct node *n, **table = ht->table;
+
+ h = hash_function(key);
+
+ for (n = table[h] ; n; n = n->next) {
+ if (!strcmp(n->key, key)) {
+ return n->data;
+ }
+ }
+ return NULL;
+}
diff --git a/hash.h b/hash.h
new file mode 100644
index 0000000..74aae0b
--- /dev/null
+++ b/hash.h
@@ -0,0 +1,59 @@
+/**
+ * @file hash.h
+ * @brief Implements a simple hash table.
+ * @note Copyright (C) 2015 Richard Cochran <***@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#ifndef HAVE_HASH_H
+#define HAVE_HASH_H
+
+struct hash;
+
+/**
+ * Create a new hash table.
+ * @return A pointer to a new hash table on success, NULL otherwise.
+ */
+struct hash *hash_create(void);
+
+/**
+ * Destroy an instance of a hash table.
+ * @param ht Pointer to a hash table obtained via @ref hash_create().
+ * @param func Callback function, possibly NULL, to apply to the
+ * data of each element in the table.
+ */
+void hash_destroy(struct hash *ht, void (*func)(void *));
+
+/**
+ * Inserts an element into a hash table.
+ * @param ht Hash table into which the element is to be stored.
+ * @param key Key that identifies the element.
+ * @param data Pointer to the user data to be stored.
+ * @return Zero on success and non-zero on error. Attempting to
+ * insert a duplicate key will fail with an error.
+ */
+int hash_insert(struct hash *ht, const char* key, void *data);
+
+/**
+ * Looks up an element from the hash table.
+ * @param ht Hash table to consult.
+ * @param key Key identifying the element of interest.
+ * @return Pointer to the element's data, or NULL if the key is not found.
+ */
+void *hash_lookup(struct hash *ht, const char* key);
+
+#endif
+
+
diff --git a/makefile b/makefile
index a8e5ce8..922769b 100644
--- a/makefile
+++ b/makefile
@@ -24,7 +24,7 @@ CFLAGS = -Wall $(VER) $(incdefs) $(DEBUG) $(EXTRA_CFLAGS)
LDLIBS = -lm -lrt $(EXTRA_LDFLAGS)
PRG = ptp4l pmc phc2sys hwstamp_ctl phc_ctl timemaster
OBJ = bmc.o clock.o clockadj.o clockcheck.o config.o fault.o \
- filter.o fsm.o linreg.o mave.o mmedian.o msg.o ntpshm.o nullf.o phc.o \
+ filter.o fsm.o hash.o linreg.o mave.o mmedian.o msg.o ntpshm.o nullf.o phc.o \
pi.o port.o print.o ptp4l.o raw.o servo.o sk.o stats.o tlv.o \
transport.o tsproc.o udp.o udp6.o uds.o util.o version.o

[Richard Cochran]

Signed-off-by: Richard Cochran <***@gmail.com>
---
config.c | 11 +++++++++++
config.h | 4 ++++
2 files changed, 15 insertions(+)

diff --git a/config.c b/config.c
index 934caa2..9ef8be1 100644
--- a/config.c
+++ b/config.c
@@ -24,6 +24,7 @@
#include <string.h>
#include "config.h"
#include "ether.h"
+#include "hash.h"
#include "print.h"
#include "util.h"

@@ -803,6 +804,15 @@ void config_init_interface(struct interface *iface, struct config *cfg)
iface->boundary_clock_jbod = cfg->dds.boundary_clock_jbod;
}

+int config_init(struct config *cfg)
+{
+ cfg->htab = hash_create();
+ if (!cfg->htab) {
+ return -1;
+ }
+ return 0;
+}
+
void config_destroy(struct config *cfg)
{
struct interface *iface;
@@ -811,4 +821,5 @@ void config_destroy(struct config *cfg)
STAILQ_REMOVE_HEAD(&cfg->interfaces, list);
free(iface);
}
+ hash_destroy(cfg->htab, free);
}
diff --git a/config.h b/config.h
index 7bff11f..222883a 100644
--- a/config.h
+++ b/config.h
@@ -60,6 +60,10 @@ struct config {
/* configured interfaces */
STAILQ_HEAD(interfaces_head, interface) interfaces;

+ /* hash of all non-legacy items */
+ struct hash *htab;
+
+ /* the rest are legacy fields */
enum timestamp_type timestamping;
enum transport_type transport;
enum delay_mechanism dm;

[Richard Cochran]

This patch adds method to initialize a configuration and look up values.
All port options are also global option, with the global option giving the
default value. The port look up method first checks for a port specific
value, falling back to the global default when missing.

Users are required to call 'config_init', and so this patch add that into
all three programs, ptp4l, phc2sys and pmc.

Signed-off-by: Richard Cochran <***@gmail.com>
---
config.c | 22 ++++++++++++++++++++++
config.h | 28 ++++++++++++++++++++++++++++
makefile | 10 +++++-----
phc2sys.c | 3 +++
pmc.c | 3 +++
ptp4l.c | 3 +++
6 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/config.c b/config.c
index 9ef8be1..f81ac83 100644
--- a/config.c
+++ b/config.c
@@ -823,3 +823,25 @@ void config_destroy(struct config *cfg)
}
hash_destroy(cfg->htab, free);
}
+
+struct config_item *config_global_item(struct config *cfg, const char *name)
+{
+ struct config_item *ci;
+ char buf[CONFIG_LABEL_SIZE + 8];
+ snprintf(buf, sizeof(buf), "global.%s", name);
+ ci = hash_lookup(cfg->htab, buf);
+ return ci;
+}
+
+struct config_item *config_port_item(struct config *cfg, const char *port,
+ const char *name)
+{
+ struct config_item *ci;
+ char buf[CONFIG_LABEL_SIZE + MAX_IFNAME_SIZE];
+ snprintf(buf, sizeof(buf), "%s.%s", port, name);
+ ci = hash_lookup(cfg->htab, buf);
+ if (ci) {
+ return ci;
+ }
+ return config_global_item(cfg, name);
+}
diff --git a/config.h b/config.h
index 222883a..78c57df 100644
--- a/config.h
+++ b/config.h
@@ -105,4 +105,32 @@ struct interface *config_create_interface(char *name, struct config *cfg);
void config_init_interface(struct interface *iface, struct config *cfg);
void config_destroy(struct config *cfg);

+/* New, hash table based methods: */
+
+#define CONFIG_LABEL_SIZE 32
+
+enum config_type {
+ CFG_TYPE_INT,
+ CFG_TYPE_UINT,
+ CFG_TYPE_DOUBLE,
+};
+
+typedef union {
+ int i;
+ unsigned int u;
+ double d;
+} any_t;
+
+struct config_item {
+ char label[CONFIG_LABEL_SIZE];
+ enum config_type type;
+ any_t val;
+ any_t min;
+ any_t max;
+};
+
+int config_init(struct config *cfg);
+struct config_item *config_global_item(struct config *cfg, const char *name);
+struct config_item *config_port_item(struct config *cfg, const char *port,
+ const char *name);
#endif
diff --git a/makefile b/makefile
index 922769b..bcdbb92 100644
--- a/makefile
+++ b/makefile
@@ -46,13 +46,13 @@ all: $(PRG)

ptp4l: $(OBJ)

-pmc: msg.o pmc.o pmc_common.o print.o raw.o sk.o tlv.o transport.o udp.o \
- udp6.o uds.o util.o version.o
-
-phc2sys: clockadj.o clockcheck.o linreg.o msg.o ntpshm.o nullf.o phc.o \
- phc2sys.o pi.o pmc_common.o print.o raw.o servo.o sk.o stats.o sysoff.o tlv.o \
+pmc: config.o hash.o msg.o pmc.o pmc_common.o print.o raw.o sk.o tlv.o \
transport.o udp.o udp6.o uds.o util.o version.o

+phc2sys: clockadj.o clockcheck.o config.o hash.o linreg.o msg.o ntpshm.o \
+ nullf.o phc.o phc2sys.o pi.o pmc_common.o print.o raw.o servo.o sk.o stats.o \
+ sysoff.o tlv.o transport.o udp.o udp6.o uds.o util.o version.o
+
hwstamp_ctl: hwstamp_ctl.o version.o

phc_ctl: phc_ctl.o phc.o sk.o util.o clockadj.o sysoff.o print.o version.o
diff --git a/phc2sys.c b/phc2sys.c
index 5ba99c3..c737142 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -1369,6 +1369,9 @@ int main(int argc, char *argv[])
}
}

+ if (config_init(&phc2sys_config)) {
+ return -1;
+ }
if (autocfg && (src_name || dst_name || pps_fd >= 0 || wait_sync || node.forced_sync_offset)) {
fprintf(stderr,
"autoconfiguration cannot be mixed with manual config options.\n");
diff --git a/pmc.c b/pmc.c
index 1fe8183..d26a87e 100644
--- a/pmc.c
+++ b/pmc.c
@@ -800,6 +800,9 @@ int main(int argc, char *argv[])
}
}

+ if (config_init(&pmc_config)) {
+ return -1;
+ }
if (!iface_name) {
if (transport_type == TRANS_UDS) {
snprintf(uds_local, sizeof(uds_local),
diff --git a/ptp4l.c b/ptp4l.c
index 56cb8bd..6048eb6 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -272,6 +272,9 @@ int main(int argc, char *argv[])
}
}

+ if (config_init(&cfg_settings)) {
+ return -1;
+ }
if (config && (c = config_read(config, &cfg_settings))) {
return c;
}

[Keller, Jacob E]

Post by Richard Cochran
This patch adds method to initialize a configuration and look up values.
All port options are also global option, with the global option giving the
default value. The port look up method first checks for a port specific
value, falling back to the global default when missing.
Users are required to call 'config_init', and so this patch add that into
all three programs, ptp4l, phc2sys and pmc.
---
config.c | 22 ++++++++++++++++++++++
config.h | 28 ++++++++++++++++++++++++++++
makefile | 10 +++++-----
phc2sys.c | 3 +++
pmc.c | 3 +++
ptp4l.c | 3 +++
6 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/config.c b/config.c
index 9ef8be1..f81ac83 100644
--- a/config.c
+++ b/config.c
@@ -823,3 +823,25 @@ void config_destroy(struct config *cfg)
}
hash_destroy(cfg->htab, free);
}
+
+struct config_item *config_global_item(struct config *cfg, const char *name)
+{
+ struct config_item *ci;
+ char buf[CONFIG_LABEL_SIZE + 8];
+ snprintf(buf, sizeof(buf), "global.%s", name);
+ ci = hash_lookup(cfg->htab, buf);
+ return ci;
+}
+
+struct config_item *config_port_item(struct config *cfg, const char *port,
+ const char *name)
+{
+ struct config_item *ci;
+ char buf[CONFIG_LABEL_SIZE + MAX_IFNAME_SIZE];
+ snprintf(buf, sizeof(buf), "%s.%s", port, name);
+ ci = hash_lookup(cfg->htab, buf);
+ if (ci) {
+ return ci;
+ }
+ return config_global_item(cfg, name);
+}
diff --git a/config.h b/config.h
index 222883a..78c57df 100644
--- a/config.h
+++ b/config.h
@@ -105,4 +105,32 @@ struct interface *config_create_interface(char
*name, struct config *cfg);
void config_init_interface(struct interface *iface, struct config *cfg);
void config_destroy(struct config *cfg);
+/* New, hash table based methods: */
+
+#define CONFIG_LABEL_SIZE 32
+
+enum config_type {
+ CFG_TYPE_INT,
+ CFG_TYPE_UINT,
+ CFG_TYPE_DOUBLE,
+};
+
+typedef union {
+ int i;
+ unsigned int u;
+ double d;
+} any_t;
+
+struct config_item {
+ char label[CONFIG_LABEL_SIZE];
+ enum config_type type;
+ any_t val;
+ any_t min;
+ any_t max;
+};
+
+int config_init(struct config *cfg);
+struct config_item *config_global_item(struct config *cfg, const char *name);
+struct config_item *config_port_item(struct config *cfg, const char *port,
+ const char *name);
#endif
diff --git a/makefile b/makefile
index 922769b..bcdbb92 100644
--- a/makefile
+++ b/makefile
@@ -46,13 +46,13 @@ all: $(PRG)
ptp4l: $(OBJ)
-pmc: msg.o pmc.o pmc_common.o print.o raw.o sk.o tlv.o transport.o udp.o \
- udp6.o uds.o util.o version.o
-
-phc2sys: clockadj.o clockcheck.o linreg.o msg.o ntpshm.o nullf.o phc.o \
- phc2sys.o pi.o pmc_common.o print.o raw.o servo.o sk.o stats.o sysoff.o tlv.o \
+pmc: config.o hash.o msg.o pmc.o pmc_common.o print.o raw.o sk.o tlv.o \
transport.o udp.o udp6.o uds.o util.o version.o
+phc2sys: clockadj.o clockcheck.o config.o hash.o linreg.o msg.o ntpshm.o \
+ nullf.o phc.o phc2sys.o pi.o pmc_common.o print.o raw.o servo.o sk.o stats.o \
+ sysoff.o tlv.o transport.o udp.o udp6.o uds.o util.o version.o
+
hwstamp_ctl: hwstamp_ctl.o version.o
phc_ctl: phc_ctl.o phc.o sk.o util.o clockadj.o sysoff.o print.o version.o
diff --git a/phc2sys.c b/phc2sys.c
index 5ba99c3..c737142 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -1369,6 +1369,9 @@ int main(int argc, char *argv[])
}
}
+ if (config_init(&phc2sys_config)) {
+ return -1;
+ }

Is there a reason to not drop these extra parens? I don't recall if we
keep them for style reason in this codebase or not. (too used to kernel
style to drop unnecessary ones)

Regards,
Jake

Post by Richard Cochran
if (autocfg && (src_name || dst_name || pps_fd >= 0 ||
wait_sync || node.forced_sync_offset)) {
fprintf(stderr,
"autoconfiguration cannot be mixed with
manual config options.\n");
diff --git a/pmc.c b/pmc.c
index 1fe8183..d26a87e 100644
--- a/pmc.c
+++ b/pmc.c
@@ -800,6 +800,9 @@ int main(int argc, char *argv[])
}
}
+ if (config_init(&pmc_config)) {
+ return -1;
+ }
if (!iface_name) {
if (transport_type == TRANS_UDS) {
snprintf(uds_local, sizeof(uds_local),
diff --git a/ptp4l.c b/ptp4l.c
index 56cb8bd..6048eb6 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -272,6 +272,9 @@ int main(int argc, char *argv[])
}
}
+ if (config_init(&cfg_settings)) {
+ return -1;
+ }
if (config && (c = config_read(config, &cfg_settings))) {
return c;
}

------------------------------------------------------------------------------

[Richard Cochran]

Post by Keller, Jacob E
Is there a reason to not drop these extra parens? I don't recall if we
keep them for style reason in this codebase or not. (too used to kernel
style to drop unnecessary ones)

The reason is given in CODING_STYLE.org.

Thanks,
Richard

------------------------------------------------------------------------------

[Richard Cochran]

This patch also introduces the generic code for adding and parsing new
options.

Signed-off-by: Richard Cochran <***@gmail.com>
---
config.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 115 insertions(+), 6 deletions(-)

diff --git a/config.c b/config.c
index f81ac83..3d53981 100644
--- a/config.c
+++ b/config.c
@@ -34,6 +34,47 @@ enum config_section {
UNKNOWN_SECTION,
};

+static struct config_item *config_item_alloc(struct config *cfg,
+ const char *section,
+ const char *name,
+ enum config_type type)
+{
+ struct config_item *ci;
+ char buf[CONFIG_LABEL_SIZE + MAX_IFNAME_SIZE];
+
+ ci = calloc(1, sizeof(*ci));
+ if (!ci) {
+ fprintf(stderr, "low memory\n");
+ return NULL;
+ }
+ strncpy(ci->label, name, CONFIG_LABEL_SIZE - 1);
+ ci->type = type;
+
+ snprintf(buf, sizeof(buf), "%s.%s", section, ci->label);
+ if (hash_insert(cfg->htab, buf, ci)) {
+ fprintf(stderr, "low memory or duplicate item %s\n", name);
+ free(ci);
+ return NULL;
+ }
+
+ return ci;
+}
+
+static int config_add_uint(struct config *cfg, const char *name,
+ unsigned int default_value,
+ unsigned int min, unsigned int max)
+{
+ struct config_item *ci;
+ ci = config_item_alloc(cfg, "global", name, CFG_TYPE_UINT);
+ if (!ci) {
+ return -1;
+ }
+ ci->val.u = default_value;
+ ci->min.u = min;
+ ci->max.u = max;
+ return 0;
+}
+
static enum parser_result parse_section_line(char *s, enum config_section *section)
{
if (!strcasecmp(s, "[global]")) {
@@ -52,6 +93,66 @@ static enum parser_result parse_section_line(char *s, enum config_section *secti
return PARSED_OK;
}

+static enum parser_result parse_item(struct config *cfg,
+ const char *port,
+ const char *option,
+ const char *value)
+{
+ struct config_item *cgi, *dst;
+ enum parser_result r;
+ unsigned int uval;
+ double df;
+ int val;
+
+ /* If there is no default value, then the option is bogus. */
+ cgi = config_global_item(cfg, option);
+ if (!cgi) {
+ return NOT_PARSED;
+ }
+
+ switch (cgi->type) {
+ case CFG_TYPE_INT:
+ r = get_ranged_int(value, &val, cgi->min.i, cgi->max.i);
+ break;
+ case CFG_TYPE_UINT:
+ r = get_ranged_uint(value, &uval, cgi->min.u, cgi->max.u);
+ break;
+ case CFG_TYPE_DOUBLE:
+ r = get_ranged_double(value, &df, cgi->min.d, cgi->max.d);
+ break;
+ }
+ if (r != PARSED_OK) {
+ return r;
+ }
+
+ if (port) {
+ /* Create or update this port specific item. */
+ dst = config_port_item(cfg, port, option);
+ if (!dst) {
+ dst = config_item_alloc(cfg, port, option, cgi->type);
+ if (!dst) {
+ return NOT_PARSED;
+ }
+ }
+ } else {
+ /* Update the global default value. */
+ dst = cgi;
+ }
+
+ switch (dst->type) {
+ case CFG_TYPE_INT:
+ dst->val.i = val;
+ break;
+ case CFG_TYPE_UINT:
+ dst->val.u = uval;
+ break;
+ case CFG_TYPE_DOUBLE:
+ dst->val.d = df;
+ break;
+ }
+ return PARSED_OK;
+}
+
static enum parser_result parse_pod_setting(const char *option,
const char *value,
struct port_defaults *pod)
@@ -173,9 +274,10 @@ static enum parser_result parse_pod_setting(const char *option,
return PARSED_OK;
}

-static enum parser_result parse_port_setting(const char *option,
- const char *value,
- struct interface *iface)
+static enum parser_result parse_port_setting(struct config *cfg,
+ const char *option,
+ const char *value,
+ struct interface *iface)
{
enum parser_result r;
int val;
@@ -237,7 +339,7 @@ static enum parser_result parse_port_setting(const char *option,
iface->boundary_clock_jbod = val;

} else
- return NOT_PARSED;
+ return parse_item(cfg, iface->name, option, value);

return PARSED_OK;
}
@@ -614,7 +716,7 @@ static enum parser_result parse_global_setting(const char *option,
cfg->dds.boundary_clock_jbod = val;

} else
- return NOT_PARSED;
+ return parse_item(cfg, NULL, option, value);

return PARSED_OK;
}
@@ -727,7 +829,7 @@ int config_read(char *name, struct config *cfg)
if (current_section == GLOBAL_SECTION)
parser_res = parse_global_setting(option, value, cfg);
else
- parser_res = parse_port_setting(option, value, current_port);
+ parser_res = parse_port_setting(cfg, option, value, current_port);

switch (parser_res) {
case PARSED_OK:
@@ -810,7 +912,14 @@ int config_init(struct config *cfg)
if (!cfg->htab) {
return -1;
}
+
+ if (config_add_uint(cfg, "udp_ttl", 1, 0, UINT8_MAX))
+ goto fail;
+
return 0;
+fail:
+ hash_destroy(cfg->htab, free);
+ return -1;
}

void config_destroy(struct config *cfg)

[Richard Cochran]

Signed-off-by: Richard Cochran <***@gmail.com>
---
udp.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/udp.c b/udp.c
index b100dbf..b1cc6e7 100644
--- a/udp.c
+++ b/udp.c
@@ -30,6 +30,7 @@
#include <unistd.h>

#include "address.h"
+#include "config.h"
#include "contain.h"
#include "print.h"
#include "sk.h"
@@ -92,7 +93,8 @@ static int udp_close(struct transport *t, struct fdarray *fda)
return 0;
}

-static int open_socket(const char *name, struct in_addr mc_addr[2], short port)
+static int open_socket(const char *name, struct in_addr mc_addr[2], short port,
+ int ttl)
{
struct sockaddr_in addr;
int fd, index, on = 1;
@@ -123,6 +125,10 @@ static int open_socket(const char *name, struct in_addr mc_addr[2], short port)
pr_err("setsockopt SO_BINDTODEVICE failed: %m");
goto no_option;
}
+ if (setsockopt(fd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl))) {
+ pr_err("setsockopt IP_MULTICAST_TTL failed: %m");
+ goto no_option;
+ }
addr.sin_addr = mc_addr[0];
if (mcast_join(fd, index, (struct sockaddr *) &addr, sizeof(addr))) {
pr_err("mcast_join failed");
@@ -151,8 +157,16 @@ static int udp_open(struct transport *t, const char *name, struct fdarray *fda,
enum timestamp_type ts_type)
{
struct udp *udp = container_of(t, struct udp, t);
+ struct config_item *ttl_ci;
int efd, gfd;
+ int ttl;
+
+ ttl_ci = config_port_item(t->cfg, name, "udp_ttl");
+ if (!ttl_ci) {
+ return -1;
+ }

+ ttl = ttl_ci->val.u;
udp->mac.len = 0;
sk_interface_macaddr(name, &udp->mac);

@@ -165,11 +179,11 @@ static int udp_open(struct transport *t, const char *name, struct fdarray *fda,
if (!inet_aton(PTP_PDELAY_MCAST_IPADDR, &mcast_addr[MC_PDELAY]))
return -1;

- efd = open_socket(name, mcast_addr, EVENT_PORT);
+ efd = open_socket(name, mcast_addr, EVENT_PORT, ttl);
if (efd < 0)
goto no_event;

- gfd = open_socket(name, mcast_addr, GENERAL_PORT);
+ gfd = open_socket(name, mcast_addr, GENERAL_PORT, ttl);
if (gfd < 0)
goto no_general;

[Richard Cochran]

Post by Richard Cochran
@@ -151,8 +157,16 @@ static int udp_open(struct transport *t, const char *name, struct fdarray *fda,
enum timestamp_type ts_type)
{
struct udp *udp = container_of(t, struct udp, t);
+ struct config_item *ttl_ci;
int efd, gfd;
+ int ttl;
+
+ ttl_ci = config_port_item(t->cfg, name, "udp_ttl");
+ if (!ttl_ci) {
+ return -1;
+ }
+ ttl = ttl_ci->val.u;

FWIW, I am already working on V2. I want to simplify the API to
safely support a single line call, like this:

ttl = config_get_int(...);

Thanks,
Richard

------------------------------------------------------------------------------