Discussion:
[Linuxptp-devel] [PATCH v2 00/27] automatic phc2sys configuration
Jiri Benc
2014-03-24 08:53:17 UTC
Permalink
v2: rebased to the current HEAD

For the description of the set, see the v1 cover letter:
https://www.mail-archive.com/linuxptp-***@lists.sourceforge.net/msg00236.html

Jiri Benc (27):
Move check of TLV length for management COMMAND messages
Move common code into port_prepare_and_send
Allow sending to a specified (unicast) address
uds: don't output "Connection refused"
Event subscribing
Include TLV in replies to management commands
port: event notification
clock: event notification
Event notification: port state
Custom management TLV PORT_ENUMERATION_NP
Event notification: port addition/removal
Custom management TLV PORT_PROPERTIES_NP
phc2sys: generalize run_pmc
phc2sys: split update_sync_offset
phc2sys: split clock and node
phc2sys: store information about clocks being UTC or TAI
phc2sys: rearrange declarations
phc2sys: open devices in clock_add
phc2sys: track ports
pmc_common: easy way to set port and broadcast target
pmc_common: implement pmc_send_command_action
phc2sys: event subscription
phc2sys: autoconfiguration
phc2sys: autoconfigure realtime clock on demand only
phc2sys: check clockIdentity
Subscription time limit
phc2sys: man page update for -a and -r options

clock.c | 261 ++++++++++++-
clock.h | 19 +
msg.c | 17 +
msg.h | 11 +
notification.h | 28 ++
phc2sys.8 | 115 ++++--
phc2sys.c | 1037 ++++++++++++++++++++++++++++++++++++++-------------
pmc_common.c | 40 ++-
pmc_common.h | 4 +-
port.c | 260 ++++++++------
port.h | 57 +++
tlv.c | 76 ++++
tlv.h | 19 +
transport.c | 17 +
transport.h | 25 ++
transport_private.h | 5 +
uds.c | 28 ++-
17 files changed, 1582 insertions(+), 437 deletions(-)
create mode 100644 notification.h
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:18 UTC
Permalink
Currently, it is assumed that the management TLV data of management COMMAND
messages is always empty. This is not true for the INITIALIZE command and
also for a custom command we'll be introducing.

Move the check to msg_post_recv and let it check only the TLVs defined by
the standard.

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

diff --git a/clock.c b/clock.c
index 3aa2407d0bcc..10fa4b6621c4 100644
--- a/clock.c
+++ b/clock.c
@@ -848,10 +848,6 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
return changed;
break;
case COMMAND:
- if (mgt->length != 2) {
- clock_management_send_error(p, msg, WRONG_LENGTH);
- return changed;
- }
break;
default:
return changed;
diff --git a/tlv.c b/tlv.c
index f32514ff12c8..b8cdd3959c9b 100644
--- a/tlv.c
+++ b/tlv.c
@@ -242,6 +242,15 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
pdsnp->neighborPropDelayThresh = ntohl(pdsnp->neighborPropDelayThresh);
pdsnp->asCapable = ntohl(pdsnp->asCapable);
break;
+ case SAVE_IN_NON_VOLATILE_STORAGE:
+ case RESET_NON_VOLATILE_STORAGE:
+ case INITIALIZE:
+ case FAULT_LOG_RESET:
+ case ENABLE_PORT:
+ case DISABLE_PORT:
+ if (data_len != 0)
+ goto bad_length;
+ break;
}
if (extra_len) {
if (extra_len % 2)
--
1.7.6.5
Richard Cochran
2014-04-05 09:55:47 UTC
Permalink
Post by Jiri Benc
Currently, it is assumed that the management TLV data of management COMMAND
messages is always empty. This is not true for the INITIALIZE command and
also for a custom command we'll be introducing.
Move the check to msg_post_recv and let it check only the TLVs defined by
the standard.
---
Applied.

Thanks,
Richard
Jiri Benc
2014-03-24 08:53:21 UTC
Permalink
When phc2sys is started before ptp4l or it is interrupted before ptp4l has
a chance to reply to its query, the "uds: sendto failed: Connection refused"
message is output. This is not an interesting message.

Also, don't output the "failed to send message" error from pmc_send, as
all transports output errors in their send routine.

Signed-off-by: Jiri Benc <***@redhat.com>
---
pmc_common.c | 12 ++++--------
uds.c | 2 +-
2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/pmc_common.c b/pmc_common.c
index e52d68f5b5f7..266462575bd8 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -143,19 +143,15 @@ static struct ptp_message *pmc_message(struct pmc *pmc, uint8_t action)

static int pmc_send(struct pmc *pmc, struct ptp_message *msg, int pdulen)
{
- int cnt, err;
+ int err;
+
err = msg_pre_send(msg);
if (err) {
pr_err("msg_pre_send failed");
return -1;
}
- cnt = transport_send(pmc->transport, &pmc->fdarray, 0,
- msg, pdulen, &msg->hwts);
- if (cnt < 0) {
- pr_err("failed to send message");
- return -1;
- }
- return 0;
+ return transport_send(pmc->transport, &pmc->fdarray, 0,
+ msg, pdulen, &msg->hwts);
}

static int pmc_tlv_datalen(struct pmc *pmc, int id)
diff --git a/uds.c b/uds.c
index dc0dc756f85a..691d9e85db09 100644
--- a/uds.c
+++ b/uds.c
@@ -104,7 +104,7 @@ static int uds_sendto(struct transport *t, struct fdarray *fda,
int cnt, fd = fda->fd[FD_GENERAL];

cnt = sendto(fd, buf, buflen, 0, (struct sockaddr *)addr, addrlen);
- if (cnt <= 0) {
+ if (cnt <= 0 && errno != ECONNREFUSED) {
pr_err("uds: sendto failed: %m");
}
return cnt;
--
1.7.6.5
Richard Cochran
2014-04-05 10:43:35 UTC
Permalink
Post by Jiri Benc
When phc2sys is started before ptp4l or it is interrupted before ptp4l has
a chance to reply to its query, the "uds: sendto failed: Connection refused"
message is output. This is not an interesting message.
Also, don't output the "failed to send message" error from pmc_send, as
all transports output errors in their send routine.
---
This patch is fine for me to apply, but it (unnecessarily) depends on
the previous one that still needs work.

Thanks,
Richard
Jiri Benc
2014-03-24 08:53:19 UTC
Permalink
The task of preparing the message for transmission and sending it appears
at many places. Unify them into a new function.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 12 +----
port.c | 129 ++++++++++++++++++++------------------------------------------
port.h | 12 ++++++
3 files changed, 57 insertions(+), 96 deletions(-)

diff --git a/clock.c b/clock.c
index 10fa4b6621c4..c5dd873e64c5 100644
--- a/clock.c
+++ b/clock.c
@@ -174,7 +174,7 @@ static void clock_management_send_error(struct port *p,
static int clock_management_get_response(struct clock *c, struct port *p,
int id, struct ptp_message *req)
{
- int datalen = 0, err, pdulen, respond = 0;
+ int datalen = 0, respond = 0;
struct management_tlv *tlv;
struct management_tlv_datum *mtd;
struct ptp_message *rsp;
@@ -295,16 +295,10 @@ static int clock_management_get_response(struct clock *c, struct port *p,
datalen++;
}
tlv->length = sizeof(tlv->id) + datalen;
- pdulen = rsp->header.messageLength + sizeof(*tlv) + datalen;
- rsp->header.messageLength = pdulen;
+ rsp->header.messageLength += sizeof(*tlv) + datalen;
rsp->tlv_count = 1;
- err = msg_pre_send(rsp);
- if (err) {
- goto out;
- }
- err = port_forward(p, rsp, pdulen);
+ port_prepare_and_send(p, rsp, 0);
}
-out:
msg_put(rsp);
return respond ? 1 : 0;
}
diff --git a/port.c b/port.c
index 1505aca33564..568bd30c8815 100644
--- a/port.c
+++ b/port.c
@@ -607,7 +607,7 @@ static int port_management_get_response(struct port *target,
struct port *ingress, int id,
struct ptp_message *req)
{
- int datalen = 0, err, pdulen, respond = 0;
+ int datalen = 0, respond = 0;
struct management_tlv *tlv;
struct management_tlv_datum *mtd;
struct ptp_message *rsp;
@@ -774,16 +774,10 @@ static int port_management_get_response(struct port *target,
datalen++;
}
tlv->length = sizeof(tlv->id) + datalen;
- pdulen = rsp->header.messageLength + sizeof(*tlv) + datalen;
- rsp->header.messageLength = pdulen;
+ rsp->header.messageLength += sizeof(*tlv) + datalen;
rsp->tlv_count = 1;
- err = msg_pre_send(rsp);
- if (err) {
- goto out;
- }
- err = port_forward(ingress, rsp, pdulen);
+ port_prepare_and_send(ingress, rsp, 0);
}
-out:
msg_put(rsp);
return respond ? 1 : 0;
}
@@ -1049,7 +1043,7 @@ static void port_syfufsm(struct port *p, enum syfu_event event,
static int port_pdelay_request(struct port *p)
{
struct ptp_message *msg;
- int cnt, pdulen;
+ int err;

/* If multiple pdelay resp were not detected the counter can be reset */
if (!p->multiple_pdr_detected)
@@ -1060,12 +1054,11 @@ static int port_pdelay_request(struct port *p)
if (!msg)
return -1;

- pdulen = sizeof(struct pdelay_req_msg);
msg->hwts.type = p->timestamping;

msg->header.tsmt = PDELAY_REQ | p->transportSpecific;
msg->header.ver = PTP_VERSION;
- msg->header.messageLength = pdulen;
+ msg->header.messageLength = sizeof(struct pdelay_req_msg);
msg->header.domainNumber = clock_domain_number(p->clock);
msg->header.correction = -p->pod.asymmetry;
msg->header.sourcePortIdentity = p->portIdentity;
@@ -1074,11 +1067,8 @@ static int port_pdelay_request(struct port *p)
msg->header.logMessageInterval = port_is_ieee8021as(p) ?
p->logMinPdelayReqInterval : 0x7f;

- if (msg_pre_send(msg))
- goto out;
-
- cnt = transport_peer(p->trp, &p->fda, 1, msg, pdulen, &msg->hwts);
- if (cnt <= 0) {
+ err = port_prepare_and_send(p, msg, 1);
+ if (err) {
pr_err("port %hu: send peer delay request failed", portnum(p));
goto out;
}
@@ -1103,7 +1093,6 @@ out:
static int port_delay_request(struct port *p)
{
struct ptp_message *msg;
- int cnt, pdulen;

/* Time to send a new request, forget current pdelay resp and fup */
if (p->peer_delay_resp) {
@@ -1122,12 +1111,11 @@ static int port_delay_request(struct port *p)
if (!msg)
return -1;

- pdulen = sizeof(struct delay_req_msg);
msg->hwts.type = p->timestamping;

msg->header.tsmt = DELAY_REQ | p->transportSpecific;
msg->header.ver = PTP_VERSION;
- msg->header.messageLength = pdulen;
+ msg->header.messageLength = sizeof(struct delay_req_msg);
msg->header.domainNumber = clock_domain_number(p->clock);
msg->header.correction = -p->pod.asymmetry;
msg->header.sourcePortIdentity = p->portIdentity;
@@ -1135,11 +1123,7 @@ static int port_delay_request(struct port *p)
msg->header.control = CTL_DELAY_REQ;
msg->header.logMessageInterval = 0x7f;

- if (msg_pre_send(msg))
- goto out;
-
- cnt = transport_send(p->trp, &p->fda, 1, msg, pdulen, &msg->hwts);
- if (cnt <= 0) {
+ if (port_prepare_and_send(p, msg, 1)) {
pr_err("port %hu: send delay request failed", portnum(p));
goto out;
}
@@ -1163,7 +1147,7 @@ static int port_tx_announce(struct port *p)
struct parent_ds *dad = clock_parent_ds(p->clock);
struct timePropertiesDS *tp = clock_time_properties(p->clock);
struct ptp_message *msg;
- int cnt, err = 0, pdulen;
+ int err, pdulen;

if (!port_capable(p)) {
return 0;
@@ -1197,16 +1181,9 @@ static int port_tx_announce(struct port *p)
msg->announce.stepsRemoved = clock_steps_removed(p->clock);
msg->announce.timeSource = tp->timeSource;

- if (msg_pre_send(msg)) {
- err = -1;
- goto out;
- }
- cnt = transport_send(p->trp, &p->fda, 0, msg, pdulen, &msg->hwts);
- if (cnt <= 0) {
+ err = port_prepare_and_send(p, msg, 0);
+ if (err)
pr_err("port %hu: send announce failed", portnum(p));
- err = -1;
- }
-out:
msg_put(msg);
return err;
}
@@ -1214,7 +1191,7 @@ out:
static int port_tx_sync(struct port *p)
{
struct ptp_message *msg, *fup;
- int cnt, err = 0, pdulen;
+ int err, pdulen;
int event = p->timestamping == TS_ONESTEP ? TRANS_ONESTEP : TRANS_EVENT;

if (!port_capable(p)) {
@@ -1247,14 +1224,9 @@ static int port_tx_sync(struct port *p)
if (p->timestamping != TS_ONESTEP)
msg->header.flagField[0] |= TWO_STEP;

- if (msg_pre_send(msg)) {
- err = -1;
- goto out;
- }
- cnt = transport_send(p->trp, &p->fda, event, msg, pdulen, &msg->hwts);
- if (cnt <= 0) {
+ err = port_prepare_and_send(p, msg, event);
+ if (err) {
pr_err("port %hu: send sync failed", portnum(p));
- err = -1;
goto out;
}
if (p->timestamping == TS_ONESTEP) {
@@ -1285,15 +1257,9 @@ static int port_tx_sync(struct port *p)

ts_to_timestamp(&msg->hwts.ts, &fup->follow_up.preciseOriginTimestamp);

- if (msg_pre_send(fup)) {
- err = -1;
- goto out;
- }
- cnt = transport_send(p->trp, &p->fda, 0, fup, pdulen, &fup->hwts);
- if (cnt <= 0) {
+ err = port_prepare_and_send(p, fup, 0);
+ if (err)
pr_err("port %hu: send follow up failed", portnum(p));
- err = -1;
- }
out:
msg_put(msg);
msg_put(fup);
@@ -1521,7 +1487,7 @@ static int process_announce(struct port *p, struct ptp_message *m)
static int process_delay_req(struct port *p, struct ptp_message *m)
{
struct ptp_message *msg;
- int cnt, err = 0, pdulen;
+ int err;

if (p->state != PS_MASTER && p->state != PS_GRAND_MASTER)
return 0;
@@ -1535,12 +1501,11 @@ static int process_delay_req(struct port *p, struct ptp_message *m)
if (!msg)
return -1;

- pdulen = sizeof(struct delay_resp_msg);
msg->hwts.type = p->timestamping;

msg->header.tsmt = DELAY_RESP | p->transportSpecific;
msg->header.ver = PTP_VERSION;
- msg->header.messageLength = pdulen;
+ msg->header.messageLength = sizeof(struct delay_resp_msg);
msg->header.domainNumber = m->header.domainNumber;
msg->header.correction = m->header.correction;
msg->header.sourcePortIdentity = p->portIdentity;
@@ -1552,16 +1517,9 @@ static int process_delay_req(struct port *p, struct ptp_message *m)

msg->delay_resp.requestingPortIdentity = m->header.sourcePortIdentity;

- if (msg_pre_send(msg)) {
- err = -1;
- goto out;
- }
- cnt = transport_send(p->trp, &p->fda, 0, msg, pdulen, NULL);
- if (cnt <= 0) {
+ err = port_prepare_and_send(p, msg, 0);
+ if (err)
pr_err("port %hu: send delay response failed", portnum(p));
- err = -1;
- }
-out:
msg_put(msg);
return err;
}
@@ -1639,7 +1597,7 @@ static void process_follow_up(struct port *p, struct ptp_message *m)
static int process_pdelay_req(struct port *p, struct ptp_message *m)
{
struct ptp_message *rsp, *fup;
- int cnt, err = -1, rsp_len, fup_len;
+ int err;

if (p->delayMechanism == DM_E2E) {
pr_warning("port %hu: pdelay_req on E2E port", portnum(p));
@@ -1675,12 +1633,11 @@ static int process_pdelay_req(struct port *p, struct ptp_message *m)
return -1;
}

- rsp_len = sizeof(struct pdelay_resp_msg);
rsp->hwts.type = p->timestamping;

rsp->header.tsmt = PDELAY_RESP | p->transportSpecific;
rsp->header.ver = PTP_VERSION;
- rsp->header.messageLength = rsp_len;
+ rsp->header.messageLength = sizeof(struct pdelay_resp_msg);
rsp->header.domainNumber = m->header.domainNumber;
rsp->header.sourcePortIdentity = p->portIdentity;
rsp->header.sequenceId = m->header.sequenceId;
@@ -1699,12 +1656,11 @@ static int process_pdelay_req(struct port *p, struct ptp_message *m)
ts_to_timestamp(&m->hwts.ts, &rsp->pdelay_resp.requestReceiptTimestamp);
rsp->pdelay_resp.requestingPortIdentity = m->header.sourcePortIdentity;

- fup_len = sizeof(struct pdelay_resp_fup_msg);
fup->hwts.type = p->timestamping;

fup->header.tsmt = PDELAY_RESP_FOLLOW_UP | p->transportSpecific;
fup->header.ver = PTP_VERSION;
- fup->header.messageLength = fup_len;
+ fup->header.messageLength = sizeof(struct pdelay_resp_fup_msg);
fup->header.domainNumber = m->header.domainNumber;
fup->header.correction = m->header.correction;
fup->header.sourcePortIdentity = p->portIdentity;
@@ -1714,11 +1670,8 @@ static int process_pdelay_req(struct port *p, struct ptp_message *m)

fup->pdelay_resp_fup.requestingPortIdentity = m->header.sourcePortIdentity;

- if (msg_pre_send(rsp))
- goto out;
-
- cnt = transport_peer(p->trp, &p->fda, 1, rsp, rsp_len, &rsp->hwts);
- if (cnt <= 0) {
+ err = port_prepare_and_send(p, rsp, 1);
+ if (err) {
pr_err("port %hu: send peer delay response failed", portnum(p));
goto out;
}
@@ -1730,15 +1683,10 @@ static int process_pdelay_req(struct port *p, struct ptp_message *m)
ts_to_timestamp(&rsp->hwts.ts,
&fup->pdelay_resp_fup.responseOriginTimestamp);

- if (msg_pre_send(fup))
- goto out;
-
- cnt = transport_peer(p->trp, &p->fda, 0, fup, fup_len, &rsp->hwts);
- if (cnt <= 0) {
+ err = port_prepare_and_send(p, fup, 0);
+ if (err)
pr_err("port %hu: send pdelay_resp_fup failed", portnum(p));
- goto out;
- }
- err = 0;
+
out:
msg_put(rsp);
msg_put(fup);
@@ -2222,6 +2170,18 @@ int port_forward(struct port *p, struct ptp_message *msg, int msglen)
return cnt <= 0 ? -1 : 0;
}

+int port_prepare_and_send(struct port *p, struct ptp_message *msg, int event)
+{
+ UInteger16 msg_len;
+ int cnt;
+
+ msg_len = msg->header.messageLength;
+ if (msg_pre_send(msg))
+ return -1;
+ cnt = transport_send(p->trp, &p->fda, event, msg, msg_len, &msg->hwts);
+ return cnt <= 0 ? -1 : 0;
+}
+
struct PortIdentity port_identity(struct port *p)
{
return p->portIdentity;
@@ -2301,12 +2261,7 @@ int port_management_error(struct PortIdentity pid, struct port *ingress,
msg->header.messageLength = pdulen;
msg->tlv_count = 1;

- err = msg_pre_send(msg);
- if (err) {
- goto out;
- }
- err = port_forward(ingress, msg, pdulen);
-out:
+ err = port_prepare_and_send(ingress, msg, 0);
msg_put(msg);
return err;
}
diff --git a/port.h b/port.h
index e781f2445097..9d1ddc9e94a3 100644
--- a/port.h
+++ b/port.h
@@ -93,6 +93,18 @@ enum fsm_event port_event(struct port *port, int fd_index);
int port_forward(struct port *p, struct ptp_message *msg, int msglen);

/**
+ * Prepare message for transmission and send it to a given port. Note that
+ * a single message cannot be sent several times using this function, that
+ * would lead to corrupted data being sent. Use msg_pre_send and
+ * port_forward if you need to send single message to several ports.
+ * @param p A pointer previously obtained via port_open().
+ * @param msg The message to send.
+ * @param event 0 if the message is a general message, 1 if it is an
+ * event message.
+ */
+int port_prepare_and_send(struct port *p, struct ptp_message *msg, int event);
+
+/**
* Obtain a port's identity.
* @param p A pointer previously obtained via port_open().
* @return The port identity of 'p'.
--
1.7.6.5
Richard Cochran
2014-03-31 18:25:25 UTC
Permalink
Post by Jiri Benc
diff --git a/port.h b/port.h
index e781f2445097..9d1ddc9e94a3 100644
--- a/port.h
+++ b/port.h
@@ -93,6 +93,18 @@ enum fsm_event port_event(struct port *port, int fd_index);
int port_forward(struct port *p, struct ptp_message *msg, int msglen);
/**
+ * Prepare message for transmission and send it to a given port. Note that
+ * a single message cannot be sent several times using this function, that
+ * would lead to corrupted data being sent. Use msg_pre_send and
+ * port_forward if you need to send single message to several ports.
+ * event message.
Actually, we might also pass 2 in 'event'. Maybe the text should read,
"one of the 'transport_event' enumerated values" or something like that.

Otherwise, I couldn't find anything wrong with this patch.

Thanks,
Richard
Richard Cochran
2014-04-05 09:56:21 UTC
Permalink
Post by Jiri Benc
The task of preparing the message for transmission and sending it appears
at many places. Unify them into a new function.
---
Applied.

Thanks,
Richard
Jiri Benc
2014-03-24 08:53:20 UTC
Permalink
This will be needed for notifications. Only implemented for UDS.

Signed-off-by: Jiri Benc <***@redhat.com>
---
port.c | 14 ++++++++++++++
port.h | 22 ++++++++++++++++++++++
transport.c | 17 +++++++++++++++++
transport.h | 25 +++++++++++++++++++++++++
transport_private.h | 5 +++++
uds.c | 26 ++++++++++++++++++++++----
6 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/port.c b/port.c
index 568bd30c8815..d8974ed1156b 100644
--- a/port.c
+++ b/port.c
@@ -2182,6 +2182,20 @@ int port_prepare_and_send(struct port *p, struct ptp_message *msg, int event)
return cnt <= 0 ? -1 : 0;
}

+int port_recv_addr(struct port *p, uint8_t *addr)
+{
+ return transport_recv_addr(p->trp, addr);
+}
+
+int port_forward_to(struct port *p, struct ptp_message *msg, int msglen,
+ uint8_t *addr, int addrlen)
+{
+ int cnt;
+
+ cnt = transport_sendto(p->trp, &p->fda, msg, msglen, addr, addrlen);
+ return cnt <= 0 ? -1 : 0;
+}
+
struct PortIdentity port_identity(struct port *p)
{
return p->portIdentity;
diff --git a/port.h b/port.h
index 9d1ddc9e94a3..8d78370992ca 100644
--- a/port.h
+++ b/port.h
@@ -105,6 +105,28 @@ int port_forward(struct port *p, struct ptp_message *msg, int msglen);
int port_prepare_and_send(struct port *p, struct ptp_message *msg, int event);

/**
+ * Get the source address of the last received message.
+ * @param p A pointer previously obtained via port_open().
+ * @param addr Buffer large enough (at least TRANSPORT_RECV_ADDR_LEN
+ * bytes).
+ * @return The number of bytes written to the buffer (address
+ * length).
+ */
+int port_recv_addr(struct port *p, uint8_t *addr);
+
+/**
+ * Forward a message on a given port to the given address.
+ * @param p A pointer previously obtained via port_open().
+ * @param msg The message to send. Must be in network byte order.
+ * @param msglen The length of the message in bytes.
+ * @param addr The address to send the message to.
+ * @param addrlen The length of the address in bytes.
+ * @return Zero on success, non-zero otherwise.
+ */
+int port_forward_to(struct port *p, struct ptp_message *msg, int msglen,
+ uint8_t *addr, int addrlen);
+
+/**
* Obtain a port's identity.
* @param p A pointer previously obtained via port_open().
* @return The port identity of 'p'.
diff --git a/transport.c b/transport.c
index b5346e5e51f4..7b8df79b19a7 100644
--- a/transport.c
+++ b/transport.c
@@ -53,6 +53,15 @@ int transport_peer(struct transport *t, struct fdarray *fda, int event,
return t->send(t, fda, event, 1, buf, buflen, hwts);
}

+int transport_sendto(struct transport *t, struct fdarray *fda,
+ void *buf, int buflen, uint8_t *addr, int addrlen)
+{
+ if (t->sendto) {
+ return t->sendto(t, fda, buf, buflen, addr, addrlen);
+ }
+ return -1;
+}
+
int transport_physical_addr(struct transport *t, uint8_t *addr)
{
if (t->physical_addr) {
@@ -69,6 +78,14 @@ int transport_protocol_addr(struct transport *t, uint8_t *addr)
return 0;
}

+int transport_recv_addr(struct transport *t, uint8_t *addr)
+{
+ if (t->recv_addr) {
+ return t->recv_addr(t, addr);
+ }
+ return 0;
+}
+
enum transport_type transport_type(struct transport *t)
{
return t->type;
diff --git a/transport.h b/transport.h
index aa2018b4ecdf..fb7243f1fb16 100644
--- a/transport.h
+++ b/transport.h
@@ -76,6 +76,20 @@ int transport_send(struct transport *t, struct fdarray *fda, int event,
int transport_peer(struct transport *t, struct fdarray *fda, int event,
void *buf, int buflen, struct hw_timestamp *hwts);

+/*
+ * Send message to the given address. This is intended for management
+ * messages only, thus goes always to the general port.
+ * @param t The transport.
+ * @param fda Array of file descriptors to use.
+ * @param buf Buffer with the message.
+ * @param buflen Length of the buffer.
+ * @param addr Destination address.
+ * @param addrlen Length of the destination address.
+ * @return Number of bytes sent or -1 in case of error.
+ */
+int transport_sendto(struct transport *t, struct fdarray *fda,
+ void *buf, int buflen, uint8_t *addr, int addrlen);
+
/**
* Returns the transport's type.
*/
@@ -101,6 +115,17 @@ int transport_physical_addr(struct transport *t, uint8_t *addr);
*/
int transport_protocol_addr(struct transport *t, uint8_t *addr);

+#define TRANSPORT_RECV_ADDR_LEN 128
+
+/**
+ * Gets the source address of the last received message.
+ * @param t The transport.
+ * @param addr The address will be written to this buffer.
+ * @return The number of bytes written to the buffer. Will be 0-110
+ * bytes.
+ */
+int transport_recv_addr(struct transport *t, uint8_t *addr);
+
/**
* Allocate an instance of the specified transport.
* @param type Which transport to obtain.
diff --git a/transport_private.h b/transport_private.h
index 7124f94af424..745fb17069d2 100644
--- a/transport_private.h
+++ b/transport_private.h
@@ -39,11 +39,16 @@ struct transport {
int (*send)(struct transport *t, struct fdarray *fda, int event, int peer,
void *buf, int buflen, struct hw_timestamp *hwts);

+ int (*sendto)(struct transport *t, struct fdarray *fda,
+ void *buf, int buflen, uint8_t *addr, int addrlen);
+
void (*release)(struct transport *t);

int (*physical_addr)(struct transport *t, uint8_t *addr);

int (*protocol_addr)(struct transport *t, uint8_t *addr);
+
+ int (*recv_addr)(struct transport *t, uint8_t *addr);
};

#endif
diff --git a/uds.c b/uds.c
index 35f5eccf0545..dc0dc756f85a 100644
--- a/uds.c
+++ b/uds.c
@@ -98,24 +98,40 @@ static int uds_recv(struct transport *t, int fd, void *buf, int buflen,
return cnt;
}

-static int uds_send(struct transport *t, struct fdarray *fda, int event,
- int peer, void *buf, int buflen, struct hw_timestamp *hwts)
+static int uds_sendto(struct transport *t, struct fdarray *fda,
+ void *buf, int buflen, uint8_t *addr, int addrlen)
{
int cnt, fd = fda->fd[FD_GENERAL];
- struct uds *uds = container_of(t, struct uds, t);
- cnt = sendto(fd, buf, buflen, 0, (struct sockaddr *) &uds->sa, uds->len);
+
+ cnt = sendto(fd, buf, buflen, 0, (struct sockaddr *)addr, addrlen);
if (cnt <= 0) {
pr_err("uds: sendto failed: %m");
}
return cnt;
}

+static int uds_send(struct transport *t, struct fdarray *fda, int event,
+ int peer, void *buf, int buflen, struct hw_timestamp *hwts)
+{
+ struct uds *uds = container_of(t, struct uds, t);
+
+ return uds_sendto(t, fda, buf, buflen, (uint8_t *)&uds->sa, uds->len);
+}
+
static void uds_release(struct transport *t)
{
struct uds *uds = container_of(t, struct uds, t);
free(uds);
}

+int uds_recv_addr(struct transport *t, uint8_t *addr)
+{
+ struct uds *uds = container_of(t, struct uds, t);
+
+ memcpy(addr, &uds->sa, uds->len);
+ return uds->len;
+}
+
struct transport *uds_transport_create(void)
{
struct uds *uds;
@@ -126,7 +142,9 @@ struct transport *uds_transport_create(void)
uds->t.open = uds_open;
uds->t.recv = uds_recv;
uds->t.send = uds_send;
+ uds->t.sendto = uds_sendto;
uds->t.release = uds_release;
+ uds->t.recv_addr = uds_recv_addr;
return &uds->t;
}
--
1.7.6.5
Richard Cochran
2014-04-02 16:15:27 UTC
Permalink
Post by Jiri Benc
+/*
+ * Send message to the given address. This is intended for management
+ * messages only, thus goes always to the general port.
There are a couple of unicast options for PTP. We should try to make
this new interface in a way that will be useful later on to support
unicast, which includes events, IIRC.

Thanks,
Richard
Post by Jiri Benc
+ */
+int transport_sendto(struct transport *t, struct fdarray *fda,
+ void *buf, int buflen, uint8_t *addr, int addrlen);
+
Jiri Benc
2014-04-02 17:31:12 UTC
Permalink
Post by Richard Cochran
Post by Jiri Benc
+/*
+ * Send message to the given address. This is intended for management
+ * messages only, thus goes always to the general port.
There are a couple of unicast options for PTP. We should try to make
this new interface in a way that will be useful later on to support
unicast, which includes events, IIRC.
Okay, I can add a couple more parameters to match the "send" callback.
Not sure it's worth it, though - adding them later, when they are
actually needed, would be very easy.

I have no strong preference either way.

Jiri
--
Jiri Benc
Richard Cochran
2014-04-03 07:55:31 UTC
Permalink
Post by Jiri Benc
Okay, I can add a couple more parameters to match the "send" callback.
Not sure it's worth it, though - adding them later, when they are
actually needed, would be very easy.
Or how about this: Let transport_send accept an optional pointer to
an address structure. Also, transport_recv could always return the
source address, and we won't need a separate "get last address"
function.

Just brain storming... needs more thought.

Thanks,
Richard
Richard Cochran
2014-04-05 10:40:20 UTC
Permalink
I looked again at the unicast issue, and here is what I found.

* 1588 has a very complex unicast subscription protocol (16.1). This
is similar to what you have in this series, and so if we do it right
then we will have a good basis for adding this later.

* 1588 also allows basically any kind of unicast you might want
(7.3.2). This probably the reason why other PTP implementations just
implement whatever they dream up, ignoring (16.1).

* The enterprise profile [1] forbids using (16.1). Instead it has the
master send unicast delay responses to unicast delay requests and
use multicast when the delay request is multicast.

1. http://datatracker.ietf.org/doc/draft-ietf-tictoc-ptp-enterprise-profile/
Post by Richard Cochran
Or how about this: Let transport_send accept an optional pointer to
an address structure. Also, transport_recv could always return the
source address, and we won't need a separate "get last address"
function.
This means using recvfrom in the transports. Also, the struct
ptp_message should have a new field to hold the source address.

Let us introduce a struct to hold addresses, something like:

struct address {
uint8_t buf[MAXADDR];
socklen_t len;
}

In that way we won't need to add 'addr' and 'len' all over the place,
and if we end up needing the interface index, it will be easy to add
later on.

Does this make sense to you?

If not I'll whip up a patch to show what I mean.

Thanks,
Richard
Arnold kang
2014-04-11 01:49:53 UTC
Permalink
Dear Richard,
my linux kernel is 3.0, and my phy is DP83640, and my mac is stmmac,
and my cpu is arm i just cross-compile the linuxptp, and it seems ok, but
when i run it and use hardware timestamp (./ptp4l - H -i eth0 -p
/dev/ptp0) on my system, the kernel crashed.
so, i just see linuxptp can run in kernel v3.0 and suport DP83640, is
there any thing wrong? thanks!

and this is happed in the function hwts_init (sk.c line 45) , when ioctl
(fd, SIOCHWTSAMP, &ifreq) [sk.c line 62]. thanks again,....please help me
Post by Richard Cochran
I looked again at the unicast issue, and here is what I found.
* 1588 has a very complex unicast subscription protocol (16.1). This
is similar to what you have in this series, and so if we do it right
then we will have a good basis for adding this later.
* 1588 also allows basically any kind of unicast you might want
(7.3.2). This probably the reason why other PTP implementations just
implement whatever they dream up, ignoring (16.1).
* The enterprise profile [1] forbids using (16.1). Instead it has the
master send unicast delay responses to unicast delay requests and
use multicast when the delay request is multicast.
1.
http://datatracker.ietf.org/doc/draft-ietf-tictoc-ptp-enterprise-profile/
Post by Richard Cochran
Or how about this: Let transport_send accept an optional pointer to
an address structure. Also, transport_recv could always return the
source address, and we won't need a separate "get last address"
function.
This means using recvfrom in the transports. Also, the struct
ptp_message should have a new field to hold the source address.
struct address {
uint8_t buf[MAXADDR];
socklen_t len;
}
In that way we won't need to add 'addr' and 'len' all over the place,
and if we end up needing the interface index, it will be easy to add
later on.
Does this make sense to you?
If not I'll whip up a patch to show what I mean.
Thanks,
Richard
------------------------------------------------------------------------------
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Richard Cochran
2014-04-11 04:57:42 UTC
Permalink
Arnold,

Can you please post this question once again, but not as a reply to a
different thread?

Also, no top-posting please.

Thanks,
Richard
Jiri Benc
2014-04-11 10:26:44 UTC
Permalink
Post by Richard Cochran
struct address {
uint8_t buf[MAXADDR];
socklen_t len;
}
In that way we won't need to add 'addr' and 'len' all over the place,
and if we end up needing the interface index, it will be easy to add
later on.
Does this make sense to you?
It does. Just sent a patchset that implements this.

Jiri
--
Jiri Benc
Duncan
2015-08-24 18:17:46 UTC
Permalink
Post by Jiri Benc
Post by Richard Cochran
Post by Jiri Benc
+/*
+ * Send message to the given address. This is intended for management
+ * messages only, thus goes always to the general port.
There are a couple of unicast options for PTP. We should try to make
this new interface in a way that will be useful later on to support
unicast, which includes events, IIRC.
Okay, I can add a couple more parameters to match the "send" callback.
Not sure it's worth it, though - adding them later, when they are
actually needed, would be very easy.
I have no strong preference either way.
Jiri
Hi,
I have the same request about unicast either uds or pptp4l.
Actually, Shall it call hybrid mode? Master send sync and follow up by
multicast, slave send Delay_Req to master by unicast, master send
Delay_Resp to slave by unicast if E2E mode.
I don't know why you don't implement this, but unicast really reduce the
network loading than multicast, doesn't it? And maybe multicast have the
limitation of communication between master and slave if one of them is
behind the secure gateway or high-end router?
Anyway, would you provide the whole patch for unicast based on linuxptp-
1.5. After I change the address instead of multicast in trasnport_send,
it really send to host, but always failed in poll in sk_recceive.

Regards,
Duncan



------------------------------------------------------------------------------
Richard Cochran
2015-08-25 07:25:32 UTC
Permalink
Post by Duncan
I don't know why you don't implement this,
Why?

Because I don't have unlimited time to work on ptp4l for free.

Thanks,
Rihcard

------------------------------------------------------------------------------
Jiri Benc
2014-03-24 08:53:23 UTC
Permalink
The standard requires management TLV in replies to commands:

An acknowledge management message is a response to a command
management message. The value of the managementId shall be identical
to that in the command message.
(Table 38)

Just copy the TLV from the request.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 3 +++
msg.c | 17 +++++++++++++++++
msg.h | 11 +++++++++++
3 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/clock.c b/clock.c
index f06ebcd77032..80804cad1996 100644
--- a/clock.c
+++ b/clock.c
@@ -18,6 +18,7 @@
*/
#include <errno.h>
#include <poll.h>
+#include <stddef.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
@@ -453,6 +454,8 @@ static int clock_management_cmd_response(struct clock *c, struct port *p,
pr_err("failed to allocate response message");
return 1;
}
+ msg_copy_tlv(rsp, offsetof(struct ptp_message, management.suffix),
+ req, 0);
if (port_prepare_and_send(p, rsp, 0))
pr_err("failed to send response message");
msg_put(rsp);
diff --git a/msg.c b/msg.c
index 7edbdd2619b5..7b958e30bef9 100644
--- a/msg.c
+++ b/msg.c
@@ -470,3 +470,20 @@ int msg_sots_missing(struct ptp_message *m)
}
return (!m->hwts.ts.tv_sec && !m->hwts.ts.tv_nsec) ? 1 : 0;
}
+
+void msg_copy_tlv(struct ptp_message *dest, unsigned int dest_base,
+ struct ptp_message *src, unsigned int src_base)
+{
+ unsigned int len;
+
+ if (!src_base)
+ src_base = dest_base;
+
+ len = src->header.messageLength;
+ if (len <= src_base)
+ return;
+ len -= src_base;
+ memcpy((void *)dest + dest_base, (void *)src + src_base, len);
+ dest->header.messageLength = dest_base + len;
+ dest->tlv_count = src->tlv_count;
+}
diff --git a/msg.h b/msg.h
index 7f471ca555aa..a7155139fb2b 100644
--- a/msg.h
+++ b/msg.h
@@ -321,6 +321,17 @@ void msg_put(struct ptp_message *m);
int msg_sots_missing(struct ptp_message *m);

/**
+ * Copy TLVs from one message into another.
+ * @param dest Destination message.
+ * @param dest_base Offset of the suffix in the destination message.
+ * @param src Source message.
+ * @param src_base Offset of the suffix in the source message; 0 to use
+ * the value in dest_base.
+ */
+void msg_copy_tlv(struct ptp_message *dest, unsigned int dest_base,
+ struct ptp_message *src, unsigned int src_base);
+
+/**
* Work around buggy 802.1AS switches.
*/
extern int assume_two_step;
--
1.7.6.5
Richard Cochran
2014-04-05 10:56:16 UTC
Permalink
Post by Jiri Benc
An acknowledge management message is a response to a command
management message. The value of the managementId shall be identical
to that in the command message.
(Table 38)
Just copy the TLV from the request.
---
This would be fine for me to apply, but obviously it can't without its
predecessors.

Thanks,
Richard
Jiri Benc
2014-03-24 08:53:22 UTC
Permalink
This puts groundwork for event subscription and notification. The individual
events are added by subsequent patches.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
clock.h | 11 +++++
notification.h | 27 +++++++++++
tlv.c | 18 +++++++
tlv.h | 5 ++
5 files changed, 198 insertions(+), 0 deletions(-)
create mode 100644 notification.h

diff --git a/clock.c b/clock.c
index c5dd873e64c5..f06ebcd77032 100644
--- a/clock.c
+++ b/clock.c
@@ -21,6 +21,7 @@
#include <stdlib.h>
#include <string.h>
#include <time.h>
+#include <arpa/inet.h>

#include "bmc.h"
#include "clock.h"
@@ -59,6 +60,15 @@ struct clock_stats {
unsigned int max_count;
};

+struct clock_subscriber {
+ LIST_ENTRY(clock_subscriber) list;
+ unsigned int events;
+ struct PortIdentity targetPortIdentity;
+ uint8_t addr[TRANSPORT_RECV_ADDR_LEN];
+ int addrlen;
+ UInteger16 sequenceId;
+};
+
struct clock {
clockid_t clkid;
struct servo *servo;
@@ -99,6 +109,7 @@ struct clock {
int stats_interval;
struct clockcheck *sanity_check;
struct interface uds_interface;
+ LIST_HEAD(clock_subscribers_head, clock_subscriber) subscribers;
};

struct clock the_clock;
@@ -110,9 +121,96 @@ 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
+
+static void remove_subscriber(struct clock_subscriber *s)
+{
+ LIST_REMOVE(s, list);
+ free(s);
+}
+
+static void clock_update_subscription(struct clock *c, struct ptp_message *req,
+ unsigned int events)
+{
+ struct port *uds = c->port[c->nports];
+ uint8_t addr[TRANSPORT_RECV_ADDR_LEN];
+ int addrlen;
+ struct clock_subscriber *s;
+
+ addrlen = port_recv_addr(uds, addr);
+ if (!addrlen) {
+ pr_err("failed to get sender address");
+ return;
+ }
+ LIST_FOREACH(s, &c->subscribers, list) {
+ if (!memcmp(&s->targetPortIdentity, &req->header.sourcePortIdentity,
+ sizeof(struct PortIdentity))) {
+ /* Found, update the transport address and event
+ * mask. */
+ if (events) {
+ memcpy(s->addr, addr, addrlen);
+ s->addrlen = addrlen;
+ s->events = events;
+ } else {
+ remove_subscriber(s);
+ }
+ return;
+ }
+ }
+ if (!events)
+ return;
+ /* Not present yet, add the subscriber. */
+ s = malloc(sizeof(*s));
+ if (!s) {
+ pr_err("failed to allocate memory for a subscriber");
+ return;
+ }
+ s->targetPortIdentity = req->header.sourcePortIdentity;
+ memcpy(s->addr, addr, addrlen);
+ s->addrlen = addrlen;
+ s->events = events;
+ s->sequenceId = 0;
+ LIST_INSERT_HEAD(&c->subscribers, s, list);
+}
+
+static void clock_flush_subscriptions(struct clock *c)
+{
+ struct clock_subscriber *s, *tmp;
+
+ LIST_FOREACH_SAFE(s, &c->subscribers, list, tmp) {
+ remove_subscriber(s);
+ }
+}
+
+void clock_send_notification(struct clock *c, struct ptp_message *msg,
+ int msglen, enum notification event)
+{
+ unsigned int mask = 1 << event;
+ struct port *uds = c->port[c->nports];
+ struct clock_subscriber *s;
+
+ LIST_FOREACH(s, &c->subscribers, list) {
+ if (!(s->events & mask))
+ continue;
+ /* send event */
+ msg->header.sequenceId = htons(s->sequenceId);
+ s->sequenceId++;
+ msg->management.targetPortIdentity.clockIdentity = s->targetPortIdentity.clockIdentity;
+ msg->management.targetPortIdentity.portNumber = htons(s->targetPortIdentity.portNumber);
+ port_forward_to(uds, msg, msglen, s->addr, s->addrlen);
+ }
+}
+
void clock_destroy(struct clock *c)
{
int i;
+
+ clock_flush_subscriptions(c);
for (i = 0; i < c->nports; i++) {
port_close(c->port[i]);
close(c->fault_fd[i]);
@@ -328,6 +426,40 @@ static int clock_management_set(struct clock *c, struct port *p,
return respond ? 1 : 0;
}

+static int clock_management_cmd_response(struct clock *c, struct port *p,
+ int id, struct ptp_message *req)
+{
+ int respond = 0;
+ struct ptp_message *rsp = NULL;
+ struct management_tlv *tlv;
+ struct subscribe_events_np *sen;
+
+ tlv = (struct management_tlv *)req->management.suffix;
+
+ switch (id) {
+ case SUBSCRIBE_EVENTS_NP:
+ if (p != c->port[c->nports]) {
+ /* Only the UDS port allowed. */
+ break;
+ }
+ sen = (struct subscribe_events_np *)tlv->data;
+ clock_update_subscription(c, req, sen->bitmask);
+ respond = 1;
+ break;
+ }
+ if (respond) {
+ rsp = port_management_reply(port_identity(p), p, req);
+ if (!rsp) {
+ pr_err("failed to allocate response message");
+ return 1;
+ }
+ if (port_prepare_and_send(p, rsp, 0))
+ pr_err("failed to send response message");
+ msg_put(rsp);
+ }
+ return respond ? 1 : 0;
+}
+
static void clock_stats_update(struct clock_stats *s,
int64_t offset, double freq)
{
@@ -669,6 +801,8 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count,

clock_sync_interval(c, 0);

+ LIST_INIT(&c->subscribers);
+
for (i = 0; i < count; i++) {
c->port[i] = port_open(phc_index, timestamping, 1+i, &iface[i], c);
if (!c->port[i]) {
@@ -842,6 +976,8 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
return changed;
break;
case COMMAND:
+ if (clock_management_cmd_response(c, p, mgt->id, msg))
+ return changed;
break;
default:
return changed;
@@ -880,6 +1016,7 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
case PRIMARY_DOMAIN:
case TIME_STATUS_NP:
case GRANDMASTER_SETTINGS_NP:
+ case SUBSCRIBE_EVENTS_NP:
clock_management_send_error(p, msg, NOT_SUPPORTED);
break;
default:
diff --git a/clock.h b/clock.h
index 804640bdb8f4..8718f2db715b 100644
--- a/clock.h
+++ b/clock.h
@@ -23,6 +23,7 @@
#include "dm.h"
#include "ds.h"
#include "config.h"
+#include "notification.h"
#include "servo.h"
#include "tlv.h"
#include "tmv.h"
@@ -134,6 +135,16 @@ void clock_install_fda(struct clock *c, struct port *p, struct fdarray fda);
int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg);

/**
+ * Send notification about an event to all subscribers.
+ * @param c The clock instance.
+ * @param msg The PTP message to send, in network byte order.
+ * @param msglen The length of the message in bytes.
+ * @param event The event that occured.
+ */
+void clock_send_notification(struct clock *c, struct ptp_message *msg,
+ int msglen, enum notification event);
+
+/**
* Obtain a clock's parent data set.
* @param c The clock instance.
* @return A pointer to the parent data set of the clock.
diff --git a/notification.h b/notification.h
new file mode 100644
index 000000000000..57e7a7856360
--- /dev/null
+++ b/notification.h
@@ -0,0 +1,27 @@
+/**
+ * @file notification.h
+ * @brief Definitions for the notification framework.
+ * @note Copyright (C) 2014 Red Hat, Inc., Jiri Benc <***@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#ifndef HAVE_NOTIFICATION_H
+#define HAVE_NOTIFICATION_H
+
+enum notification {
+ NOTIFY_DUMMY,
+};
+
+#endif
diff --git a/tlv.c b/tlv.c
index b8cdd3959c9b..352026ee5f7a 100644
--- a/tlv.c
+++ b/tlv.c
@@ -242,6 +242,16 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
pdsnp->neighborPropDelayThresh = ntohl(pdsnp->neighborPropDelayThresh);
pdsnp->asCapable = ntohl(pdsnp->asCapable);
break;
+ case SUBSCRIBE_EVENTS_NP:
+ {
+ struct subscribe_events_np *sen;
+
+ if (data_len != sizeof(struct subscribe_events_np))
+ goto bad_length;
+ sen = (struct subscribe_events_np *)m->data;
+ sen->bitmask = ntohl(sen->bitmask);
+ }
+ break;
case SAVE_IN_NON_VOLATILE_STORAGE:
case RESET_NON_VOLATILE_STORAGE:
case INITIALIZE:
@@ -337,6 +347,14 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
pdsnp->neighborPropDelayThresh = htonl(pdsnp->neighborPropDelayThresh);
pdsnp->asCapable = htonl(pdsnp->asCapable);
break;
+ case SUBSCRIBE_EVENTS_NP:
+ {
+ struct subscribe_events_np *sen;
+
+ sen = (struct subscribe_events_np *)m->data;
+ sen->bitmask = htonl(sen->bitmask);
+ }
+ break;
}
}

diff --git a/tlv.h b/tlv.h
index 60c937db02ef..8eb840685a81 100644
--- a/tlv.h
+++ b/tlv.h
@@ -79,6 +79,7 @@ enum management_action {
#define PRIMARY_DOMAIN 0x4002
#define TIME_STATUS_NP 0xC000
#define GRANDMASTER_SETTINGS_NP 0xC001
+#define SUBSCRIBE_EVENTS_NP 0xC003

/* Port management ID values */
#define NULL_MANAGEMENT 0x0000
@@ -196,6 +197,10 @@ struct port_ds_np {
Integer32 asCapable;
} PACKED;

+struct subscribe_events_np {
+ UInteger32 bitmask;
+} PACKED;
+
enum clock_type {
CLOCK_TYPE_ORDINARY = 0x8000,
CLOCK_TYPE_BOUNDARY = 0x4000,
--
1.7.6.5
Richard Cochran
2014-04-05 10:53:55 UTC
Permalink
Post by Jiri Benc
This puts groundwork for event subscription and notification. The individual
events are added by subsequent patches.
This patch is basically okay, but I have to pick a few nits...
Post by Jiri Benc
+struct clock_subscriber {
+ LIST_ENTRY(clock_subscriber) list;
+ unsigned int events;
+ struct PortIdentity targetPortIdentity;
+ uint8_t addr[TRANSPORT_RECV_ADDR_LEN];
+ int addrlen;
+ UInteger16 sequenceId;
+};
This code looks good, and I think it will be able support unicast
subscriptions too, later on.
Post by Jiri Benc
diff --git a/tlv.c b/tlv.c
index b8cdd3959c9b..352026ee5f7a 100644
--- a/tlv.c
+++ b/tlv.c
@@ -242,6 +242,16 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
pdsnp->neighborPropDelayThresh = ntohl(pdsnp->neighborPropDelayThresh);
pdsnp->asCapable = ntohl(pdsnp->asCapable);
break;
+ {
+ struct subscribe_events_np *sen;
Please, no braces in a case.
Put 'sen' at the top with the others.
Post by Jiri Benc
+
+ if (data_len != sizeof(struct subscribe_events_np))
+ goto bad_length;
+ sen = (struct subscribe_events_np *)m->data;
+ sen->bitmask = ntohl(sen->bitmask);
+ }
+ break;
@@ -337,6 +347,14 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
pdsnp->neighborPropDelayThresh = htonl(pdsnp->neighborPropDelayThresh);
pdsnp->asCapable = htonl(pdsnp->asCapable);
break;
+ {
+ struct subscribe_events_np *sen;
+
+ sen = (struct subscribe_events_np *)m->data;
+ sen->bitmask = htonl(sen->bitmask);
+ }
+ break;
}
}
diff --git a/tlv.h b/tlv.h
index 60c937db02ef..8eb840685a81 100644
--- a/tlv.h
+++ b/tlv.h
@@ -79,6 +79,7 @@ enum management_action {
#define PRIMARY_DOMAIN 0x4002
#define TIME_STATUS_NP 0xC000
#define GRANDMASTER_SETTINGS_NP 0xC001
+#define SUBSCRIBE_EVENTS_NP 0xC003
/* Port management ID values */
#define NULL_MANAGEMENT 0x0000
@@ -196,6 +197,10 @@ struct port_ds_np {
Integer32 asCapable;
} PACKED;
+struct subscribe_events_np {
+ UInteger32 bitmask;
Please use c99 types instead (unless a field has a type like that in
some public standard). We hate camel case.

Also, I anticipate that we will want many more than 32 events. Lets
add a whole bunch of space here for future use.

Thanks,
Richard
Post by Jiri Benc
+} PACKED;
+
enum clock_type {
CLOCK_TYPE_ORDINARY = 0x8000,
CLOCK_TYPE_BOUNDARY = 0x4000,
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:26 UTC
Permalink
Notify subscribers about port state changes.

Signed-off-by: Jiri Benc <***@redhat.com>
---
notification.h | 2 +-
port.c | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/notification.h b/notification.h
index 57e7a7856360..47c9b56c4f7e 100644
--- a/notification.h
+++ b/notification.h
@@ -21,7 +21,7 @@
#define HAVE_NOTIFICATION_H

enum notification {
- NOTIFY_DUMMY,
+ NOTIFY_PORT_STATE,
};

#endif
diff --git a/port.c b/port.c
index 8b1f1927590b..fa70571678b6 100644
--- a/port.c
+++ b/port.c
@@ -2038,6 +2038,7 @@ int port_dispatch(struct port *p, enum fsm_event event, int mdiff)
if (next == PS_LISTENING && p->delayMechanism == DM_P2P) {
port_set_delay_tmo(p);
}
+ port_notify_event(p, NOTIFY_PORT_STATE);
return 1;
}

@@ -2053,6 +2054,7 @@ int port_dispatch(struct port *p, enum fsm_event event, int mdiff)
}

p->state = next;
+ port_notify_event(p, NOTIFY_PORT_STATE);
return 0;
}

@@ -2359,7 +2361,9 @@ void port_notify_event(struct port *p, enum notification event)
int id;

switch (event) {
- /* set id */
+ case NOTIFY_PORT_STATE:
+ id = PORT_DATA_SET;
+ break;
default:
return;
}
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:24 UTC
Permalink
Split management message creation to more fine-grained functions to allow
notification messages to be created.

Signed-off-by: Jiri Benc <***@redhat.com>
---
port.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++--------------
port.h | 9 ++++++
2 files changed, 82 insertions(+), 20 deletions(-)

diff --git a/port.c b/port.c
index d8974ed1156b..a78eab24f8aa 100644
--- a/port.c
+++ b/port.c
@@ -603,26 +603,19 @@ static void port_management_send_error(struct port *p, struct port *ingress,
static const Octet profile_id_drr[] = {0x00, 0x1B, 0x19, 0x00, 0x01, 0x00};
static const Octet profile_id_p2p[] = {0x00, 0x1B, 0x19, 0x00, 0x02, 0x00};

-static int port_management_get_response(struct port *target,
- struct port *ingress, int id,
- struct ptp_message *req)
+static int port_management_fill_response(struct port *target,
+ struct ptp_message *rsp, int id)
{
int datalen = 0, respond = 0;
struct management_tlv *tlv;
struct management_tlv_datum *mtd;
- struct ptp_message *rsp;
struct portDS *pds;
struct port_ds_np *pdsnp;
- struct PortIdentity pid = port_identity(target);
struct clock_description *desc;
struct mgmt_clock_description *cd;
uint8_t *buf;
uint16_t u16;

- rsp = port_management_reply(pid, ingress, req);
- if (!rsp) {
- return 0;
- }
tlv = (struct management_tlv *) rsp->management.suffix;
tlv->type = TLV_MANAGEMENT;
tlv->id = id;
@@ -776,10 +769,27 @@ static int port_management_get_response(struct port *target,
tlv->length = sizeof(tlv->id) + datalen;
rsp->header.messageLength += sizeof(*tlv) + datalen;
rsp->tlv_count = 1;
- port_prepare_and_send(ingress, rsp, 0);
}
+ return respond;
+}
+
+static int port_management_get_response(struct port *target,
+ struct port *ingress, int id,
+ struct ptp_message *req)
+{
+ struct PortIdentity pid = port_identity(target);
+ struct ptp_message *rsp;
+ int respond;
+
+ rsp = port_management_reply(pid, ingress, req);
+ if (!rsp) {
+ return 0;
+ }
+ respond = port_management_fill_response(target, rsp, id);
+ if (respond)
+ port_prepare_and_send(ingress, rsp, 0);
msg_put(rsp);
- return respond ? 1 : 0;
+ return respond;
}

static int port_management_set(struct port *target,
@@ -2280,9 +2290,11 @@ int port_management_error(struct PortIdentity pid, struct port *ingress,
return err;
}

-struct ptp_message *port_management_reply(struct PortIdentity pid,
- struct port *ingress,
- struct ptp_message *req)
+static struct ptp_message *
+port_management_construct(struct PortIdentity pid, struct port *ingress,
+ UInteger16 sequenceId,
+ struct PortIdentity *targetPortIdentity,
+ UInteger8 boundaryHops, uint8_t action)
{
struct ptp_message *msg;
int pdulen;
@@ -2299,16 +2311,16 @@ struct ptp_message *port_management_reply(struct PortIdentity pid,
msg->header.messageLength = pdulen;
msg->header.domainNumber = clock_domain_number(ingress->clock);
msg->header.sourcePortIdentity = pid;
- msg->header.sequenceId = req->header.sequenceId;
+ msg->header.sequenceId = sequenceId;
msg->header.control = CTL_MANAGEMENT;
msg->header.logMessageInterval = 0x7f;

- msg->management.targetPortIdentity = req->header.sourcePortIdentity;
- msg->management.startingBoundaryHops =
- req->management.startingBoundaryHops - req->management.boundaryHops;
- msg->management.boundaryHops = msg->management.startingBoundaryHops;
+ if (targetPortIdentity)
+ msg->management.targetPortIdentity = *targetPortIdentity;
+ msg->management.startingBoundaryHops = boundaryHops;
+ msg->management.boundaryHops = boundaryHops;

- switch (management_action(req)) {
+ switch (action) {
case GET: case SET:
msg->management.flags = RESPONSE;
break;
@@ -2319,6 +2331,47 @@ struct ptp_message *port_management_reply(struct PortIdentity pid,
return msg;
}

+struct ptp_message *port_management_reply(struct PortIdentity pid,
+ struct port *ingress,
+ struct ptp_message *req)
+{
+ UInteger8 boundaryHops;
+
+ boundaryHops = req->management.startingBoundaryHops - req->management.boundaryHops;
+ return port_management_construct(pid, ingress,
+ req->header.sequenceId,
+ &req->header.sourcePortIdentity,
+ boundaryHops,
+ management_action(req));
+}
+
+void port_notify_event(struct port *p, enum notification event)
+{
+ struct PortIdentity pid = port_identity(p);
+ struct ptp_message *msg;
+ UInteger16 msg_len;
+ int id;
+
+ switch (event) {
+ /* set id */
+ default:
+ return;
+ }
+ /* targetPortIdentity and sequenceId will be filled by
+ * clock_send_notification */
+ msg = port_management_construct(pid, p, 0, NULL, 1, GET);
+ if (!msg)
+ return;
+ if (!port_management_fill_response(p, msg, id))
+ goto err;
+ msg_len = msg->header.messageLength;
+ if (msg_pre_send(msg))
+ goto err;
+ clock_send_notification(p->clock, msg, msg_len, event);
+err:
+ msg_put(msg);
+}
+
struct port *port_open(int phc_index,
enum timestamp_type timestamping,
int number,
diff --git a/port.h b/port.h
index 8d78370992ca..f4941ad8ac23 100644
--- a/port.h
+++ b/port.h
@@ -23,6 +23,7 @@
#include "fd.h"
#include "foreign.h"
#include "fsm.h"
+#include "notification.h"
#include "transport.h"

/* forward declarations */
@@ -170,6 +171,14 @@ struct ptp_message *port_management_reply(struct PortIdentity pid,
struct ptp_message *req);

/**
+ * Construct and send notification to subscribers about an event that
+ * occured on the port.
+ * @param p The port.
+ * @param event The identification of the event.
+ */
+void port_notify_event(struct port *p, enum notification event);
+
+/**
* Open a network port.
* @param phc_index The PHC device index for the network device.
* @param timestamping The timestamping mode for this port.
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:25 UTC
Permalink
Split management message creation to more fine-grained functions to allow
notification messages to be created.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++----------
clock.h | 8 ++++++++
port.c | 8 +++++++-
port.h | 14 ++++++++++++++
4 files changed, 77 insertions(+), 11 deletions(-)

diff --git a/clock.c b/clock.c
index 80804cad1996..010262f522e3 100644
--- a/clock.c
+++ b/clock.c
@@ -270,22 +270,16 @@ static void clock_management_send_error(struct port *p,
pr_err("failed to send management error status");
}

-static int clock_management_get_response(struct clock *c, struct port *p,
- int id, struct ptp_message *req)
+static int clock_management_fill_response(struct clock *c,
+ struct ptp_message *rsp, int id)
{
int datalen = 0, respond = 0;
struct management_tlv *tlv;
struct management_tlv_datum *mtd;
- struct ptp_message *rsp;
struct time_status_np *tsn;
struct grandmaster_settings_np *gsn;
- struct PortIdentity pid = port_identity(p);
struct PTPText *text;

- rsp = port_management_reply(pid, p, req);
- if (!rsp) {
- return 0;
- }
tlv = (struct management_tlv *) rsp->management.suffix;
tlv->type = TLV_MANAGEMENT;
tlv->id = id;
@@ -396,10 +390,26 @@ static int clock_management_get_response(struct clock *c, struct port *p,
tlv->length = sizeof(tlv->id) + datalen;
rsp->header.messageLength += sizeof(*tlv) + datalen;
rsp->tlv_count = 1;
- port_prepare_and_send(p, rsp, 0);
}
+ return respond;
+}
+
+static int clock_management_get_response(struct clock *c, struct port *p,
+ int id, struct ptp_message *req)
+{
+ struct PortIdentity pid = port_identity(p);
+ struct ptp_message *rsp;
+ int respond;
+
+ rsp = port_management_reply(pid, p, req);
+ if (!rsp) {
+ return 0;
+ }
+ respond = clock_management_fill_response(c, rsp, id);
+ if (respond)
+ port_prepare_and_send(p, rsp, 0);
msg_put(rsp);
- return respond ? 1 : 0;
+ return respond;
}

static int clock_management_set(struct clock *c, struct port *p,
@@ -1032,6 +1042,34 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
return changed;
}

+void clock_notify_event(struct clock *c, enum notification event)
+{
+ struct port *uds = c->port[c->nports];
+ struct PortIdentity pid = port_identity(uds);
+ struct ptp_message *msg;
+ UInteger16 msg_len;
+ int id;
+
+ switch (event) {
+ /* set id */
+ default:
+ return;
+ }
+ /* targetPortIdentity and sequenceId will be filled by
+ * clock_send_notification */
+ msg = port_management_notify(pid, uds);
+ if (!msg)
+ return;
+ if (!clock_management_fill_response(c, msg, id))
+ goto err;
+ msg_len = msg->header.messageLength;
+ if (msg_pre_send(msg))
+ goto err;
+ clock_send_notification(c, msg, msg_len, event);
+err:
+ msg_put(msg);
+}
+
struct parent_ds *clock_parent_ds(struct clock *c)
{
return &c->dad;
diff --git a/clock.h b/clock.h
index 8718f2db715b..92ec163d962f 100644
--- a/clock.h
+++ b/clock.h
@@ -145,6 +145,14 @@ void clock_send_notification(struct clock *c, struct ptp_message *msg,
int msglen, enum notification event);

/**
+ * Construct and send notification to subscribers about an event that
+ * occured on the clock.
+ * @param c The clock instance.
+ * @param event The identification of the event.
+ */
+void clock_notify_event(struct clock *c, enum notification event);
+
+/**
* Obtain a clock's parent data set.
* @param c The clock instance.
* @return A pointer to the parent data set of the clock.
diff --git a/port.c b/port.c
index a78eab24f8aa..8b1f1927590b 100644
--- a/port.c
+++ b/port.c
@@ -2345,6 +2345,12 @@ struct ptp_message *port_management_reply(struct PortIdentity pid,
management_action(req));
}

+struct ptp_message *port_management_notify(struct PortIdentity pid,
+ struct port *port)
+{
+ return port_management_construct(pid, port, 0, NULL, 1, GET);
+}
+
void port_notify_event(struct port *p, enum notification event)
{
struct PortIdentity pid = port_identity(p);
@@ -2359,7 +2365,7 @@ void port_notify_event(struct port *p, enum notification event)
}
/* targetPortIdentity and sequenceId will be filled by
* clock_send_notification */
- msg = port_management_construct(pid, p, 0, NULL, 1, GET);
+ msg = port_management_notify(pid, p);
if (!msg)
return;
if (!port_management_fill_response(p, msg, id))
diff --git a/port.h b/port.h
index f4941ad8ac23..753c37958856 100644
--- a/port.h
+++ b/port.h
@@ -171,6 +171,20 @@ struct ptp_message *port_management_reply(struct PortIdentity pid,
struct ptp_message *req);

/**
+ * Allocate a standalone reply management message.
+ *
+ * See note in @ref port_management_reply description about freeing the
+ * message. Also note that the constructed message does not have
+ * targetPortIdentity and sequenceId filled.
+ *
+ * @param pid The id of the responding port.
+ * @param port The port to which the message will be sent.
+ * @return Pointer to a message on success, NULL otherwise.
+ */
+struct ptp_message *port_management_notify(struct PortIdentity pid,
+ struct port *port);
+
+/**
* Construct and send notification to subscribers about an event that
* occured on the port.
* @param p The port.
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:27 UTC
Permalink
Used to enumerate all ports. With the future dynamic port adding/removal,
there may be holes in the port number sequence. For now, just fill it with
the sequence numbers.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 16 ++++++++++++++++
tlv.c | 29 +++++++++++++++++++++++++++++
tlv.h | 6 ++++++
3 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/clock.c b/clock.c
index 010262f522e3..2e7794cc07cd 100644
--- a/clock.c
+++ b/clock.c
@@ -381,6 +381,21 @@ static int clock_management_fill_response(struct clock *c,
datalen = sizeof(*gsn);
respond = 1;
break;
+ case PORT_ENUMERATION_NP:
+ {
+ struct port_enumeration_np *pen;
+ int i;
+
+ if (c->nports > 720)
+ break;
+ pen = (struct port_enumeration_np *)tlv->data;
+ pen->numberPorts = c->nports;
+ for (i = 0; i < c->nports; i++)
+ pen->portNumber[i] = i + 1;
+ datalen = sizeof(*pen) + pen->numberPorts * sizeof(UInteger16);
+ respond = 1;
+ }
+ break;
}
if (respond) {
if (datalen % 2) {
@@ -1030,6 +1045,7 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
case TIME_STATUS_NP:
case GRANDMASTER_SETTINGS_NP:
case SUBSCRIBE_EVENTS_NP:
+ case PORT_ENUMERATION_NP:
clock_management_send_error(p, msg, NOT_SUPPORTED);
break;
default:
diff --git a/tlv.c b/tlv.c
index 352026ee5f7a..8d635c452905 100644
--- a/tlv.c
+++ b/tlv.c
@@ -252,6 +252,24 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
sen->bitmask = ntohl(sen->bitmask);
}
break;
+ case PORT_ENUMERATION_NP:
+ {
+ struct port_enumeration_np *pen;
+ size_t expected_len;
+ int i;
+
+ if (data_len < sizeof(struct port_enumeration_np))
+ goto bad_length;
+ pen = (struct port_enumeration_np *)m->data;
+ pen->numberPorts = ntohs(pen->numberPorts);
+ expected_len = sizeof(struct port_enumeration_np) +
+ pen->numberPorts * sizeof(UInteger16);
+ if (data_len != expected_len)
+ goto bad_length;
+ for (i = 0; i < pen->numberPorts; i++)
+ pen->portNumber[i] = ntohs(pen->portNumber[i]);
+ }
+ break;
case SAVE_IN_NON_VOLATILE_STORAGE:
case RESET_NON_VOLATILE_STORAGE:
case INITIALIZE:
@@ -355,6 +373,17 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
sen->bitmask = htonl(sen->bitmask);
}
break;
+ case PORT_ENUMERATION_NP:
+ {
+ struct port_enumeration_np *pen;
+ int i;
+
+ pen = (struct port_enumeration_np *)m->data;
+ for (i = 0; i < pen->numberPorts; i++)
+ pen->portNumber[i] = htons(pen->portNumber[i]);
+ pen->numberPorts = htons(pen->numberPorts);
+ }
+ break;
}
}

diff --git a/tlv.h b/tlv.h
index 8eb840685a81..dffdabda7d09 100644
--- a/tlv.h
+++ b/tlv.h
@@ -80,6 +80,7 @@ enum management_action {
#define TIME_STATUS_NP 0xC000
#define GRANDMASTER_SETTINGS_NP 0xC001
#define SUBSCRIBE_EVENTS_NP 0xC003
+#define PORT_ENUMERATION_NP 0xC004

/* Port management ID values */
#define NULL_MANAGEMENT 0x0000
@@ -201,6 +202,11 @@ struct subscribe_events_np {
UInteger32 bitmask;
} PACKED;

+struct port_enumeration_np {
+ UInteger16 numberPorts;
+ UInteger16 portNumber[0];
+} PACKED;
+
enum clock_type {
CLOCK_TYPE_ORDINARY = 0x8000,
CLOCK_TYPE_BOUNDARY = 0x4000,
--
1.7.6.5
Richard Cochran
2014-04-05 11:02:14 UTC
Permalink
Post by Jiri Benc
Used to enumerate all ports. With the future dynamic port adding/removal,
there may be holes in the port number sequence. For now, just fill it with
the sequence numbers.
---
clock.c | 16 ++++++++++++++++
tlv.c | 29 +++++++++++++++++++++++++++++
tlv.h | 6 ++++++
3 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/clock.c b/clock.c
index 010262f522e3..2e7794cc07cd 100644
--- a/clock.c
+++ b/clock.c
@@ -381,6 +381,21 @@ static int clock_management_fill_response(struct clock *c,
datalen = sizeof(*gsn);
respond = 1;
break;
+ {
No braces, please.
Post by Jiri Benc
+ struct port_enumeration_np *pen;
+ int i;
+
+ if (c->nports > 720)
+ break;
+ pen = (struct port_enumeration_np *)tlv->data;
+ pen->numberPorts = c->nports;
+ for (i = 0; i < c->nports; i++)
+ pen->portNumber[i] = i + 1;
+ datalen = sizeof(*pen) + pen->numberPorts * sizeof(UInteger16);
+ respond = 1;
+ }
+ break;
}
if (respond) {
if (datalen % 2) {
@@ -1030,6 +1045,7 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
clock_management_send_error(p, msg, NOT_SUPPORTED);
break;
diff --git a/tlv.c b/tlv.c
index 352026ee5f7a..8d635c452905 100644
--- a/tlv.c
+++ b/tlv.c
@@ -252,6 +252,24 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
sen->bitmask = ntohl(sen->bitmask);
}
break;
+ {
Same.
Post by Jiri Benc
+ struct port_enumeration_np *pen;
+ size_t expected_len;
+ int i;
+
+ if (data_len < sizeof(struct port_enumeration_np))
+ goto bad_length;
+ pen = (struct port_enumeration_np *)m->data;
+ pen->numberPorts = ntohs(pen->numberPorts);
+ expected_len = sizeof(struct port_enumeration_np) +
+ pen->numberPorts * sizeof(UInteger16);
+ if (data_len != expected_len)
+ goto bad_length;
+ for (i = 0; i < pen->numberPorts; i++)
+ pen->portNumber[i] = ntohs(pen->portNumber[i]);
+ }
+ break;
@@ -355,6 +373,17 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
sen->bitmask = htonl(sen->bitmask);
}
break;
+ {
Same.
Post by Jiri Benc
+ struct port_enumeration_np *pen;
+ int i;
+
+ pen = (struct port_enumeration_np *)m->data;
+ for (i = 0; i < pen->numberPorts; i++)
+ pen->portNumber[i] = htons(pen->portNumber[i]);
+ pen->numberPorts = htons(pen->numberPorts);
+ }
+ break;
}
}
diff --git a/tlv.h b/tlv.h
index 8eb840685a81..dffdabda7d09 100644
--- a/tlv.h
+++ b/tlv.h
@@ -80,6 +80,7 @@ enum management_action {
#define TIME_STATUS_NP 0xC000
#define GRANDMASTER_SETTINGS_NP 0xC001
#define SUBSCRIBE_EVENTS_NP 0xC003
+#define PORT_ENUMERATION_NP 0xC004
/* Port management ID values */
#define NULL_MANAGEMENT 0x0000
@@ -201,6 +202,11 @@ struct subscribe_events_np {
UInteger32 bitmask;
} PACKED;
+struct port_enumeration_np {
+ UInteger16 numberPorts;
+ UInteger16 portNumber[0];
+} PACKED;
+
I don't like this variable sized message. We should stick to fixed
sized items. Why do we need this at all?

BTW, regarding your dynamic port work, have you seen 1588
Interpretation 21?

Thanks,
Richard
Jiri Benc
2014-04-07 16:02:13 UTC
Permalink
Post by Richard Cochran
Post by Jiri Benc
+struct port_enumeration_np {
+ UInteger16 numberPorts;
+ UInteger16 portNumber[0];
+} PACKED;
+
I don't like this variable sized message. We should stick to fixed
sized items. Why do we need this at all?
The idea was to find out easily which ports are available. I wanted to
omit querying each port number in turn to find out whether it exists or
not (i.e. it's a "hole").

If you prefer that, or have even better idea, I'm happy to rewrite this.
Post by Richard Cochran
BTW, regarding your dynamic port work, have you seen 1588
Interpretation 21?
I did not, thanks for the pointer. If I'm reading it correctly, it
basically matches what I've been expecting.

Thanks,

Jiri
--
Jiri Benc
Richard Cochran
2014-04-07 17:15:28 UTC
Permalink
Post by Jiri Benc
Post by Richard Cochran
Post by Jiri Benc
+struct port_enumeration_np {
+ UInteger16 numberPorts;
+ UInteger16 portNumber[0];
+} PACKED;
+
I don't like this variable sized message. We should stick to fixed
sized items. Why do we need this at all?
The idea was to find out easily which ports are available. I wanted to
omit querying each port number in turn to find out whether it exists or
not (i.e. it's a "hole").
If you prefer that, or have even better idea, I'm happy to rewrite this.
I prefer the one-by-one query. This isn't a performance critical
operation.
Post by Jiri Benc
Post by Richard Cochran
BTW, regarding your dynamic port work, have you seen 1588
Interpretation 21?
I did not, thanks for the pointer. If I'm reading it correctly, it
basically matches what I've been expecting.
Okay, fine.

Thanks,
Richard
Jiri Benc
2014-03-24 08:53:28 UTC
Permalink
Add an event number for port adding/removal. As currently the ports are
static, this event is not emitted for now.

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

diff --git a/clock.c b/clock.c
index 2e7794cc07cd..20fea847ef68 100644
--- a/clock.c
+++ b/clock.c
@@ -1067,7 +1067,9 @@ void clock_notify_event(struct clock *c, enum notification event)
int id;

switch (event) {
- /* set id */
+ case NOTIFY_PORT_ENUM:
+ id = PORT_ENUMERATION_NP;
+ break;
default:
return;
}
diff --git a/notification.h b/notification.h
index 47c9b56c4f7e..9e202c7d5fe0 100644
--- a/notification.h
+++ b/notification.h
@@ -22,6 +22,7 @@

enum notification {
NOTIFY_PORT_STATE,
+ NOTIFY_PORT_ENUM,
};

#endif
--
1.7.6.5
Richard Cochran
2014-04-05 11:04:06 UTC
Permalink
Post by Jiri Benc
Add an event number for port adding/removal. As currently the ports are
static, this event is not emitted for now.
Then this patch should wait unit dynamic ports are ready.

Thanks,
Richard
Jiri Benc
2014-03-24 08:53:29 UTC
Permalink
Will be used by phc2sys to find out interfaces corresponding to ports.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 9 +++++++++
port.c | 16 ++++++++++++++++
tlv.c | 20 ++++++++++++++++++++
tlv.h | 8 ++++++++
4 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/clock.c b/clock.c
index 20fea847ef68..c893a58391ce 100644
--- a/clock.c
+++ b/clock.c
@@ -1012,6 +1012,15 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
}

switch (mgt->id) {
+ case PORT_PROPERTIES_NP:
+ if (p != c->port[c->nports]) {
+ /* Only the UDS port allowed. */
+ clock_management_send_error(p, msg, NOT_SUPPORTED);
+ return 0;
+ }
+ }
+
+ switch (mgt->id) {
case USER_DESCRIPTION:
case SAVE_IN_NON_VOLATILE_STORAGE:
case RESET_NON_VOLATILE_STORAGE:
diff --git a/port.c b/port.c
index fa70571678b6..261237654828 100644
--- a/port.c
+++ b/port.c
@@ -760,6 +760,22 @@ static int port_management_fill_response(struct port *target,
datalen = sizeof(*pdsnp);
respond = 1;
break;
+ case PORT_PROPERTIES_NP:
+ {
+ struct port_properties_np *ppn;
+
+ ppn = (struct port_properties_np *)tlv->data;
+ ppn->portIdentity = target->portIdentity;
+ if (target->state == PS_GRAND_MASTER)
+ ppn->portState = PS_MASTER;
+ else
+ ppn->portState = target->state;
+ ppn->timestamping = target->timestamping;
+ ptp_text_set(&ppn->interface, target->name);
+ datalen = sizeof(*ppn) + ppn->interface.length;
+ respond = 1;
+ }
+ break;
}
if (respond) {
if (datalen % 2) {
diff --git a/tlv.c b/tlv.c
index 8d635c452905..35c9773498be 100644
--- a/tlv.c
+++ b/tlv.c
@@ -270,6 +270,18 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
pen->portNumber[i] = ntohs(pen->portNumber[i]);
}
break;
+ case PORT_PROPERTIES_NP:
+ {
+ struct port_properties_np *ppn;
+
+ if (data_len < sizeof(struct port_properties_np))
+ goto bad_length;
+ ppn = (struct port_properties_np *)m->data;
+ ppn->portIdentity.portNumber = ntohs(ppn->portIdentity.portNumber);
+ extra_len = sizeof(struct port_properties_np);
+ extra_len += ppn->interface.length;
+ }
+ break;
case SAVE_IN_NON_VOLATILE_STORAGE:
case RESET_NON_VOLATILE_STORAGE:
case INITIALIZE:
@@ -384,6 +396,14 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
pen->numberPorts = htons(pen->numberPorts);
}
break;
+ case PORT_PROPERTIES_NP:
+ {
+ struct port_properties_np *ppn;
+
+ ppn = (struct port_properties_np *)m->data;
+ ppn->portIdentity.portNumber = htons(ppn->portIdentity.portNumber);
+ }
+ break;
}
}

diff --git a/tlv.h b/tlv.h
index dffdabda7d09..4d2f3f41562a 100644
--- a/tlv.h
+++ b/tlv.h
@@ -101,6 +101,7 @@ enum management_action {
#define DELAY_MECHANISM 0x6000
#define LOG_MIN_PDELAY_REQ_INTERVAL 0x6001
#define PORT_DATA_SET_NP 0xC002
+#define PORT_PROPERTIES_NP 0xC005

/* Management error ID values */
#define RESPONSE_TOO_BIG 0x0001
@@ -207,6 +208,13 @@ struct port_enumeration_np {
UInteger16 portNumber[0];
} PACKED;

+struct port_properties_np {
+ struct PortIdentity portIdentity;
+ Enumeration8 portState;
+ Enumeration8 timestamping;
+ struct PTPText interface;
+} PACKED;
+
enum clock_type {
CLOCK_TYPE_ORDINARY = 0x8000,
CLOCK_TYPE_BOUNDARY = 0x4000,
--
1.7.6.5
Richard Cochran
2014-04-05 11:08:57 UTC
Permalink
Post by Jiri Benc
Will be used by phc2sys to find out interfaces corresponding to ports.
This looks okay to me. Same nits as before...
Post by Jiri Benc
---
clock.c | 9 +++++++++
port.c | 16 ++++++++++++++++
tlv.c | 20 ++++++++++++++++++++
tlv.h | 8 ++++++++
4 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/clock.c b/clock.c
index 20fea847ef68..c893a58391ce 100644
--- a/clock.c
+++ b/clock.c
@@ -1012,6 +1012,15 @@ int clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
}
switch (mgt->id) {
+ if (p != c->port[c->nports]) {
+ /* Only the UDS port allowed. */
+ clock_management_send_error(p, msg, NOT_SUPPORTED);
+ return 0;
+ }
+ }
+
+ switch (mgt->id) {
diff --git a/port.c b/port.c
index fa70571678b6..261237654828 100644
--- a/port.c
+++ b/port.c
@@ -760,6 +760,22 @@ static int port_management_fill_response(struct port *target,
datalen = sizeof(*pdsnp);
respond = 1;
break;
+ {
No braces, here or below, please.
Post by Jiri Benc
+ struct port_properties_np *ppn;
+
+ ppn = (struct port_properties_np *)tlv->data;
+ ppn->portIdentity = target->portIdentity;
+ if (target->state == PS_GRAND_MASTER)
+ ppn->portState = PS_MASTER;
+ else
+ ppn->portState = target->state;
+ ppn->timestamping = target->timestamping;
+ ptp_text_set(&ppn->interface, target->name);
+ datalen = sizeof(*ppn) + ppn->interface.length;
+ respond = 1;
+ }
+ break;
}
if (respond) {
if (datalen % 2) {
diff --git a/tlv.c b/tlv.c
index 8d635c452905..35c9773498be 100644
--- a/tlv.c
+++ b/tlv.c
@@ -270,6 +270,18 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t data_len,
pen->portNumber[i] = ntohs(pen->portNumber[i]);
}
break;
+ {
+ struct port_properties_np *ppn;
+
+ if (data_len < sizeof(struct port_properties_np))
+ goto bad_length;
+ ppn = (struct port_properties_np *)m->data;
+ ppn->portIdentity.portNumber = ntohs(ppn->portIdentity.portNumber);
+ extra_len = sizeof(struct port_properties_np);
+ extra_len += ppn->interface.length;
+ }
+ break;
@@ -384,6 +396,14 @@ static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
pen->numberPorts = htons(pen->numberPorts);
}
break;
+ {
+ struct port_properties_np *ppn;
+
+ ppn = (struct port_properties_np *)m->data;
+ ppn->portIdentity.portNumber = htons(ppn->portIdentity.portNumber);
+ }
+ break;
}
}
diff --git a/tlv.h b/tlv.h
index dffdabda7d09..4d2f3f41562a 100644
--- a/tlv.h
+++ b/tlv.h
@@ -101,6 +101,7 @@ enum management_action {
#define DELAY_MECHANISM 0x6000
#define LOG_MIN_PDELAY_REQ_INTERVAL 0x6001
#define PORT_DATA_SET_NP 0xC002
+#define PORT_PROPERTIES_NP 0xC005
/* Management error ID values */
#define RESPONSE_TOO_BIG 0x0001
@@ -207,6 +208,13 @@ struct port_enumeration_np {
UInteger16 portNumber[0];
} PACKED;
+struct port_properties_np {
+ struct PortIdentity portIdentity;
+ Enumeration8 portState;
+ Enumeration8 timestamping;
Use stdint type please.
Post by Jiri Benc
+ struct PTPText interface;
+} PACKED;
+
Thanks,
Richard
Jiri Benc
2014-03-24 08:53:33 UTC
Permalink
For now, only CLOCK_REALTIME can be UTC. This may stay this way forever but
now we have a clean separation between codepaths where CLOCK_REALTIME is
required and codepaths any UTC clock should take.

The main motiviation behind this change is removal of sync_offset_direction.
It has to be computed on the fly based on the source and destination when we
have multiple clocks supported and automatic following of ptp4l state
changes implemented.

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

diff --git a/phc2sys.c b/phc2sys.c
index 825d7328af15..6c86b4d9f028 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -130,6 +130,7 @@ struct clock {
LIST_ENTRY(clock) list;
clockid_t clkid;
int sysoff_supported;
+ int is_utc;
struct servo *servo;
enum servo_state servo_state;
const char *source_label;
@@ -146,7 +147,7 @@ struct node {
int phc_readings;
double phc_interval;
int sync_offset;
- int sync_offset_direction;
+ int forced_sync_offset;
int leap;
int leap_set;
int kernel_leap;
@@ -161,6 +162,15 @@ static int update_sync_offset(struct node *node);
static int clock_handle_leap(struct node *node, struct clock *clock,
int64_t offset, uint64_t ts, int do_leap);

+static int64_t get_sync_offset(struct node *node, struct clock *src)
+{
+ int direction = node->forced_sync_offset;
+
+ if (!direction)
+ direction = node->master->is_utc - src->is_utc;
+ return (int64_t)node->sync_offset * NS_PER_SEC * direction;
+}
+
static void update_clock_stats(struct clock *clock, unsigned int max_count,
int64_t offset, double freq, int64_t delay)
{
@@ -206,9 +216,7 @@ static void update_clock(struct node *node, struct clock *clock,
if (clock_handle_leap(node, clock, offset, ts, do_leap))
return;

- if (node->sync_offset_direction)
- offset += node->sync_offset * NS_PER_SEC *
- node->sync_offset_direction;
+ offset += get_sync_offset(node, clock);

if (clock->sanity_check && clockcheck_sample(clock->sanity_check, ts))
servo_reset(clock->servo);
@@ -290,7 +298,7 @@ static int do_pps_loop(struct node *node, struct clock *clock, int fd)

if (src == CLOCK_INVALID) {
/* The sync offset can't be applied with PPS alone. */
- node->sync_offset_direction = 0;
+ node->sync_offset = 0;
} else {
enable_pps_output(node->master->clkid);
}
@@ -558,15 +566,14 @@ static int clock_handle_leap(struct node *node, struct clock *clock,
if (!node->leap && !do_leap)
return 0;

- if (clock->clkid != CLOCK_REALTIME &&
- node->master->clkid != CLOCK_REALTIME)
+ if (clock->is_utc == node->master->is_utc)
return 0;

/* If the system clock is the master clock, get a time stamp from
it, as it is the clock which will include the leap second. */
- if (node->master->clkid == CLOCK_REALTIME) {
+ if (node->master->is_utc) {
struct timespec tp;
- if (clock_gettime(CLOCK_REALTIME, &tp)) {
+ if (clock_gettime(node->master->clkid, &tp)) {
pr_err("failed to read clock: %m");
return -1;
}
@@ -575,11 +582,8 @@ static int clock_handle_leap(struct node *node, struct clock *clock,

/* If the clock will be stepped, the time stamp has to be the
target time. Ignore possible 1 second error in UTC offset. */
- if (clock->clkid == CLOCK_REALTIME &&
- clock->servo_state == SERVO_UNLOCKED) {
- ts -= offset + node->sync_offset * NS_PER_SEC *
- node->sync_offset_direction;
- }
+ if (clock->is_utc && clock->servo_state == SERVO_UNLOCKED)
+ ts -= get_sync_offset(node, clock);

/* Suspend clock updates in the last second before midnight. */
if (is_utc_ambiguous(ts)) {
@@ -610,10 +614,12 @@ static int clock_add(struct node *node, clockid_t clkid)
c->clkid = clkid;
c->servo_state = SERVO_UNLOCKED;

- if (c->clkid == CLOCK_REALTIME)
+ if (c->clkid == CLOCK_REALTIME) {
c->source_label = "sys";
- else
+ c->is_utc = 1;
+ } else {
c->source_label = "phc";
+ }

if (node->stats_max_count > 0) {
c->offset_stats = stats_create();
@@ -698,7 +704,7 @@ int main(int argc, char *argv[])
clockid_t src = CLOCK_INVALID;
clockid_t dst = CLOCK_REALTIME;
int c, domain_number = 0, pps_fd = -1;
- int r, wait_sync = 0, forced_sync_offset = 0;
+ int r, wait_sync = 0;
int print_level = LOG_INFO, use_syslog = 1, verbose = 0;
double phc_rate;
struct node node = {
@@ -779,8 +785,7 @@ int main(int argc, char *argv[])
if (get_arg_val_i(c, optarg, &node.sync_offset,
INT_MIN, INT_MAX))
return -1;
- node.sync_offset_direction = -1;
- forced_sync_offset = 1;
+ node.forced_sync_offset = -1;
break;
case 'L':
if (get_arg_val_i(c, optarg, &node.sanity_freq_limit, 0, INT_MAX))
@@ -841,7 +846,7 @@ int main(int argc, char *argv[])
goto bad_usage;
}

- if (!wait_sync && !forced_sync_offset) {
+ if (!wait_sync && !node.forced_sync_offset) {
fprintf(stderr,
"time offset must be specified using -w or -O\n");
goto bad_usage;
@@ -870,24 +875,17 @@ int main(int argc, char *argv[])
pr_notice("Waiting for ptp4l...");
}

- if (!forced_sync_offset) {
+ if (!node.forced_sync_offset) {
r = run_pmc_get_utc_offset(&node, 1000);
if (r <= 0) {
pr_err("failed to get UTC offset");
return -1;
}
-
- if (src != CLOCK_REALTIME &&
- dst == CLOCK_REALTIME)
- node.sync_offset_direction = 1;
- else if (src == CLOCK_REALTIME &&
- dst != CLOCK_REALTIME)
- node.sync_offset_direction = -1;
- else
- node.sync_offset_direction = 0;
}

- if (forced_sync_offset || !node.sync_offset_direction)
+ if (node.forced_sync_offset ||
+ (src != CLOCK_REALTIME && dst != CLOCK_REALTIME) ||
+ src == CLOCK_INVALID)
close_pmc(&node);
}
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:30 UTC
Permalink
Make run_pmc usable for any kind of management message. Create wrappers for
waiting for ptp4l and for getting UTC offset.

Signed-off-by: Jiri Benc <***@redhat.com>
---
phc2sys.c | 131 +++++++++++++++++++++++++++++++------------------------------
1 files changed, 66 insertions(+), 65 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 5ecb602120c3..0581eb5bcb24 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -141,7 +141,6 @@ struct clock {
int leap_set;
int kernel_leap;
struct pmc *pmc;
- int pmc_ds_idx;
int pmc_ds_requested;
uint64_t pmc_last_update;
struct clockcheck *sanity_check;
@@ -390,31 +389,14 @@ static int init_pmc(struct clock *clock, int domain_number)
return 0;
}

-static int run_pmc(struct clock *clock, int timeout,
- int wait_sync, int get_utc_offset)
+static int run_pmc(struct clock *clock, int timeout, int ds_id,
+ struct ptp_message **msg)
{
- struct ptp_message *msg;
- struct timePropertiesDS *tds;
- void *data;
#define N_FD 1
struct pollfd pollfd[N_FD];
- int cnt, ds_done;
-#define N_ID 2
- int ds_ids[N_ID] = {
- PORT_DATA_SET,
- TIME_PROPERTIES_DATA_SET
- };
-
- while (clock->pmc_ds_idx < N_ID) {
- /* Check if the data set is really needed. */
- if ((ds_ids[clock->pmc_ds_idx] == PORT_DATA_SET &&
- !wait_sync) ||
- (ds_ids[clock->pmc_ds_idx] == TIME_PROPERTIES_DATA_SET &&
- !get_utc_offset)) {
- clock->pmc_ds_idx++;
- continue;
- }
+ int cnt;

+ while (1) {
pollfd[0].fd = pmc_get_transport_fd(clock->pmc);
pollfd[0].events = POLLIN|POLLPRI;
if (!clock->pmc_ds_requested)
@@ -434,62 +416,76 @@ static int run_pmc(struct clock *clock, int timeout,
/* Send a new request if there are no pending messages. */
if ((pollfd[0].revents & POLLOUT) &&
!(pollfd[0].revents & (POLLIN|POLLPRI))) {
- pmc_send_get_action(clock->pmc,
- ds_ids[clock->pmc_ds_idx]);
+ pmc_send_get_action(clock->pmc, ds_id);
clock->pmc_ds_requested = 1;
}

if (!(pollfd[0].revents & (POLLIN|POLLPRI)))
continue;

- msg = pmc_recv(clock->pmc);
+ *msg = pmc_recv(clock->pmc);

- if (!msg)
+ if (!*msg)
continue;

- if (!is_msg_mgt(msg) ||
- get_mgt_id(msg) != ds_ids[clock->pmc_ds_idx]) {
- msg_put(msg);
+ if (!is_msg_mgt(*msg) ||
+ get_mgt_id(*msg) != ds_id) {
+ msg_put(*msg);
+ *msg = NULL;
continue;
}
+ clock->pmc_ds_requested = 0;
+ return 1;
+ }
+}

- data = get_mgt_data(msg);
- ds_done = 0;
-
- switch (get_mgt_id(msg)) {
- case PORT_DATA_SET:
- switch (((struct portDS *)data)->portState) {
- case PS_MASTER:
- case PS_SLAVE:
- ds_done = 1;
- break;
- }
+static int run_pmc_wait_sync(struct clock *clock, int timeout)
+{
+ struct ptp_message *msg;
+ int res;
+ void *data;
+ Enumeration8 portState;

- break;
- case TIME_PROPERTIES_DATA_SET:
- tds = (struct timePropertiesDS *)data;
- if (tds->flags & PTP_TIMESCALE) {
- clock->sync_offset = tds->currentUtcOffset;
- if (tds->flags & LEAP_61)
- clock->leap = 1;
- else if (tds->flags & LEAP_59)
- clock->leap = -1;
- else
- clock->leap = 0;
- }
- ds_done = 1;
- break;
- }
+ while (1) {
+ res = run_pmc(clock, timeout, PORT_DATA_SET, &msg);
+ if (res <= 0)
+ return res;

- if (ds_done) {
- /* Proceed with the next data set. */
- clock->pmc_ds_idx++;
- clock->pmc_ds_requested = 0;
- }
+ data = get_mgt_data(msg);
+ portState = ((struct portDS *)data)->portState;
msg_put(msg);
+
+ switch (portState) {
+ case PS_MASTER:
+ case PS_SLAVE:
+ return 1;
+ }
+ /* try to get more data sets (for other ports) */
+ clock->pmc_ds_requested = 1;
}
+}

- clock->pmc_ds_idx = 0;
+static int run_pmc_get_utc_offset(struct clock *clock, int timeout)
+{
+ struct ptp_message *msg;
+ int res;
+ struct timePropertiesDS *tds;
+
+ res = run_pmc(clock, timeout, TIME_PROPERTIES_DATA_SET, &msg);
+ if (res <= 0)
+ return res;
+
+ tds = (struct timePropertiesDS *)get_mgt_data(msg);
+ if (tds->flags & PTP_TIMESCALE) {
+ clock->sync_offset = tds->currentUtcOffset;
+ if (tds->flags & LEAP_61)
+ clock->leap = 1;
+ else if (tds->flags & LEAP_59)
+ clock->leap = -1;
+ else
+ clock->leap = 0;
+ }
+ msg_put(msg);
return 1;
}

@@ -506,7 +502,7 @@ static int update_sync_offset(struct clock *clock, int64_t offset, uint64_t ts)
if (clock->pmc &&
!(ts > clock->pmc_last_update &&
ts - clock->pmc_last_update < PMC_UPDATE_INTERVAL)) {
- if (run_pmc(clock, 0, 0, 1) > 0)
+ if (run_pmc_get_utc_offset(clock, 0) > 0)
clock->pmc_last_update = ts;
}

@@ -767,17 +763,22 @@ int main(int argc, char *argv[])
return -1;

while (1) {
- r = run_pmc(&dst_clock, 1000,
- wait_sync, !forced_sync_offset);
+ r = run_pmc_wait_sync(&dst_clock, 1000);
if (r < 0)
return -1;
- else if (r > 0)
+ if (r > 0)
break;
else
pr_notice("Waiting for ptp4l...");
}

if (!forced_sync_offset) {
+ r = run_pmc_get_utc_offset(&dst_clock, 1000);
+ if (r <= 0) {
+ pr_err("failed to get UTC offset");
+ return -1;
+ }
+
if (src != CLOCK_REALTIME &&
dst_clock.clkid == CLOCK_REALTIME)
dst_clock.sync_offset_direction = 1;
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:34 UTC
Permalink
This just moves code around to have related functions together and forward
declaration at the beginning of the file. No code changes.

Signed-off-by: Jiri Benc <***@redhat.com>
---
phc2sys.c | 208 ++++++++++++++++++++++++++++++-------------------------------
1 files changed, 103 insertions(+), 105 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 6c86b4d9f028..34f5f94ccb91 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -60,7 +60,41 @@
#define PHC_PPS_OFFSET_LIMIT 10000000
#define PMC_UPDATE_INTERVAL (60 * NS_PER_SEC)

-struct clock;
+struct clock {
+ LIST_ENTRY(clock) list;
+ clockid_t clkid;
+ int sysoff_supported;
+ int is_utc;
+ struct servo *servo;
+ enum servo_state servo_state;
+ const char *source_label;
+ struct stats *offset_stats;
+ struct stats *freq_stats;
+ struct stats *delay_stats;
+ struct clockcheck *sanity_check;
+};
+
+struct node {
+ unsigned int stats_max_count;
+ int sanity_freq_limit;
+ enum servo_type servo_type;
+ int phc_readings;
+ double phc_interval;
+ int sync_offset;
+ int forced_sync_offset;
+ int leap;
+ int leap_set;
+ int kernel_leap;
+ struct pmc *pmc;
+ int pmc_ds_requested;
+ uint64_t pmc_last_update;
+ LIST_HEAD(clock_head, clock) clocks;
+ struct clock *master;
+};
+
+static int update_sync_offset(struct node *node);
+static int clock_handle_leap(struct node *node, struct clock *clock,
+ int64_t offset, uint64_t ts, int do_leap);

static clockid_t clock_open(char *device)
{
@@ -95,6 +129,74 @@ static clockid_t clock_open(char *device)
return clkid;
}

+static int clock_add(struct node *node, clockid_t clkid)
+{
+ struct clock *c;
+ int max_ppb;
+ double ppb;
+
+ c = calloc(1, sizeof(*c));
+ if (!c) {
+ pr_err("failed to allocate memory for a clock");
+ return -1;
+ }
+ c->clkid = clkid;
+ c->servo_state = SERVO_UNLOCKED;
+
+ if (c->clkid == CLOCK_REALTIME) {
+ c->source_label = "sys";
+ c->is_utc = 1;
+ } else {
+ c->source_label = "phc";
+ }
+
+ if (node->stats_max_count > 0) {
+ c->offset_stats = stats_create();
+ c->freq_stats = stats_create();
+ c->delay_stats = stats_create();
+ if (!c->offset_stats ||
+ !c->freq_stats ||
+ !c->delay_stats) {
+ pr_err("failed to create stats");
+ return -1;
+ }
+ }
+ if (node->sanity_freq_limit) {
+ c->sanity_check = clockcheck_create(node->sanity_freq_limit);
+ if (!c->sanity_check) {
+ pr_err("failed to create clock check");
+ return -1;
+ }
+ }
+
+ clockadj_init(c->clkid);
+ ppb = clockadj_get_freq(c->clkid);
+ /* The reading may silently fail and return 0, reset the frequency to
+ make sure ppb is the actual frequency of the clock. */
+ clockadj_set_freq(c->clkid, ppb);
+ if (c->clkid == CLOCK_REALTIME) {
+ sysclk_set_leap(0);
+ max_ppb = sysclk_max_freq();
+ } else {
+ max_ppb = phc_max_adj(c->clkid);
+ if (!max_ppb) {
+ pr_err("clock is not adjustable");
+ return -1;
+ }
+ }
+
+ c->servo = servo_create(node->servo_type, -ppb, max_ppb, 0);
+ servo_sync_interval(c->servo, node->phc_interval);
+
+ if (clkid != CLOCK_REALTIME)
+ c->sysoff_supported = (SYSOFF_SUPPORTED ==
+ sysoff_probe(CLOCKID_TO_FD(clkid),
+ node->phc_readings));
+
+ LIST_INSERT_HEAD(&node->clocks, c, list);
+ return 0;
+}
+
static int read_phc(clockid_t clkid, clockid_t sysclk, int readings,
int64_t *offset, uint64_t *ts, int64_t *delay)
{
@@ -126,42 +228,6 @@ static int read_phc(clockid_t clkid, clockid_t sysclk, int readings,
return 1;
}

-struct clock {
- LIST_ENTRY(clock) list;
- clockid_t clkid;
- int sysoff_supported;
- int is_utc;
- struct servo *servo;
- enum servo_state servo_state;
- const char *source_label;
- struct stats *offset_stats;
- struct stats *freq_stats;
- struct stats *delay_stats;
- struct clockcheck *sanity_check;
-};
-
-struct node {
- unsigned int stats_max_count;
- int sanity_freq_limit;
- enum servo_type servo_type;
- int phc_readings;
- double phc_interval;
- int sync_offset;
- int forced_sync_offset;
- int leap;
- int leap_set;
- int kernel_leap;
- struct pmc *pmc;
- int pmc_ds_requested;
- uint64_t pmc_last_update;
- LIST_HEAD(clock_head, clock) clocks;
- struct clock *master;
-};
-
-static int update_sync_offset(struct node *node);
-static int clock_handle_leap(struct node *node, struct clock *clock,
- int64_t offset, uint64_t ts, int do_leap);
-
static int64_t get_sync_offset(struct node *node, struct clock *src)
{
int direction = node->forced_sync_offset;
@@ -600,74 +666,6 @@ static int clock_handle_leap(struct node *node, struct clock *clock,
return 0;
}

-static int clock_add(struct node *node, clockid_t clkid)
-{
- struct clock *c;
- int max_ppb;
- double ppb;
-
- c = calloc(1, sizeof(*c));
- if (!c) {
- pr_err("failed to allocate memory for a clock");
- return -1;
- }
- c->clkid = clkid;
- c->servo_state = SERVO_UNLOCKED;
-
- if (c->clkid == CLOCK_REALTIME) {
- c->source_label = "sys";
- c->is_utc = 1;
- } else {
- c->source_label = "phc";
- }
-
- if (node->stats_max_count > 0) {
- c->offset_stats = stats_create();
- c->freq_stats = stats_create();
- c->delay_stats = stats_create();
- if (!c->offset_stats ||
- !c->freq_stats ||
- !c->delay_stats) {
- pr_err("failed to create stats");
- return -1;
- }
- }
- if (node->sanity_freq_limit) {
- c->sanity_check = clockcheck_create(node->sanity_freq_limit);
- if (!c->sanity_check) {
- pr_err("failed to create clock check");
- return -1;
- }
- }
-
- clockadj_init(c->clkid);
- ppb = clockadj_get_freq(c->clkid);
- /* The reading may silently fail and return 0, reset the frequency to
- make sure ppb is the actual frequency of the clock. */
- clockadj_set_freq(c->clkid, ppb);
- if (c->clkid == CLOCK_REALTIME) {
- sysclk_set_leap(0);
- max_ppb = sysclk_max_freq();
- } else {
- max_ppb = phc_max_adj(c->clkid);
- if (!max_ppb) {
- pr_err("clock is not adjustable");
- return -1;
- }
- }
-
- c->servo = servo_create(node->servo_type, -ppb, max_ppb, 0);
- servo_sync_interval(c->servo, node->phc_interval);
-
- if (clkid != CLOCK_REALTIME)
- c->sysoff_supported = (SYSOFF_SUPPORTED ==
- sysoff_probe(CLOCKID_TO_FD(clkid),
- node->phc_readings));
-
- LIST_INSERT_HEAD(&node->clocks, c, list);
- return 0;
-}
-
static void usage(char *progname)
{
fprintf(stderr,
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:35 UTC
Permalink
Do not call clock_open to open a clock device but let clock_add do that and
return the newly created struct. Also, store the device (interface) name in
struct clock.

Signed-off-by: Jiri Benc <***@redhat.com>
---
phc2sys.c | 81 ++++++++++++++++++++++++++++++++++--------------------------
1 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 34f5f94ccb91..62e9b8c19e17 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -67,6 +67,7 @@ struct clock {
int is_utc;
struct servo *servo;
enum servo_state servo_state;
+ char *device;
const char *source_label;
struct stats *offset_stats;
struct stats *freq_stats;
@@ -129,19 +130,27 @@ static clockid_t clock_open(char *device)
return clkid;
}

-static int clock_add(struct node *node, clockid_t clkid)
+static struct clock *clock_add(struct node *node, char *device)
{
struct clock *c;
+ clockid_t clkid = CLOCK_INVALID;
int max_ppb;
double ppb;

+ if (device) {
+ clkid = clock_open(device);
+ if (clkid == CLOCK_INVALID)
+ return NULL;
+ }
+
c = calloc(1, sizeof(*c));
if (!c) {
pr_err("failed to allocate memory for a clock");
- return -1;
+ return NULL;
}
c->clkid = clkid;
c->servo_state = SERVO_UNLOCKED;
+ c->device = strdup(device);

if (c->clkid == CLOCK_REALTIME) {
c->source_label = "sys";
@@ -158,14 +167,14 @@ static int clock_add(struct node *node, clockid_t clkid)
!c->freq_stats ||
!c->delay_stats) {
pr_err("failed to create stats");
- return -1;
+ return NULL;
}
}
if (node->sanity_freq_limit) {
c->sanity_check = clockcheck_create(node->sanity_freq_limit);
if (!c->sanity_check) {
pr_err("failed to create clock check");
- return -1;
+ return NULL;
}
}

@@ -181,7 +190,7 @@ static int clock_add(struct node *node, clockid_t clkid)
max_ppb = phc_max_adj(c->clkid);
if (!max_ppb) {
pr_err("clock is not adjustable");
- return -1;
+ return NULL;
}
}

@@ -194,7 +203,7 @@ static int clock_add(struct node *node, clockid_t clkid)
node->phc_readings));

LIST_INSERT_HEAD(&node->clocks, c, list);
- return 0;
+ return c;
}

static int read_phc(clockid_t clkid, clockid_t sysclk, int readings,
@@ -699,8 +708,8 @@ static void usage(char *progname)
int main(int argc, char *argv[])
{
char *progname;
- clockid_t src = CLOCK_INVALID;
- clockid_t dst = CLOCK_REALTIME;
+ char *src_name = NULL, *dst_name = NULL;
+ struct clock *src, *dst;
int c, domain_number = 0, pps_fd = -1;
int r, wait_sync = 0;
int print_level = LOG_INFO, use_syslog = 1, verbose = 0;
@@ -723,7 +732,7 @@ int main(int argc, char *argv[])
"c:d:s:E:P:I:S:F:R:N:O:L:i:u:wn:xl:mqvh"))) {
switch (c) {
case 'c':
- dst = clock_open(optarg);
+ dst_name = strdup(optarg);
break;
case 'd':
pps_fd = open(optarg, O_RDONLY);
@@ -737,7 +746,7 @@ int main(int argc, char *argv[])
fprintf(stderr,
"'-i' has been deprecated. please use '-s' instead.\n");
case 's':
- src = clock_open(optarg);
+ src_name = strdup(optarg);
break;
case 'E':
if (!strcasecmp(optarg, "pi")) {
@@ -826,38 +835,46 @@ int main(int argc, char *argv[])
}
}

- if (pps_fd < 0 && src == CLOCK_INVALID) {
+ if (pps_fd < 0 && !src_name) {
fprintf(stderr,
"valid source clock must be selected.\n");
goto bad_usage;
}

- if (dst == CLOCK_INVALID) {
+ if (!wait_sync && !node.forced_sync_offset) {
fprintf(stderr,
- "valid destination clock must be selected.\n");
+ "time offset must be specified using -w or -O\n");
goto bad_usage;
}

- if (pps_fd >= 0 && dst != CLOCK_REALTIME) {
+ print_set_progname(progname);
+ print_set_verbose(verbose);
+ print_set_syslog(use_syslog);
+ print_set_level(print_level);
+
+ src = clock_add(&node, src_name);
+ free(src_name);
+ node.master = src;
+ dst = clock_add(&node, dst_name ? dst_name : "CLOCK_REALTIME");
+ free(dst_name);
+
+ if (!dst) {
fprintf(stderr,
- "cannot use a pps device unless destination is CLOCK_REALTIME\n");
+ "valid destination clock must be selected.\n");
goto bad_usage;
}

- if (!wait_sync && !node.forced_sync_offset) {
+ if (!src) {
fprintf(stderr,
- "time offset must be specified using -w or -O\n");
+ "valid source clock must be selected.\n");
goto bad_usage;
}

- print_set_progname(progname);
- print_set_verbose(verbose);
- print_set_syslog(use_syslog);
- print_set_level(print_level);
-
- clock_add(&node, src);
- node.master = LIST_FIRST(&node.clocks);
- clock_add(&node, dst);
+ if (pps_fd >= 0 && dst->clkid != CLOCK_REALTIME) {
+ fprintf(stderr,
+ "cannot use a pps device unless destination is CLOCK_REALTIME\n");
+ goto bad_usage;
+ }

if (wait_sync) {
if (init_pmc(&node, domain_number))
@@ -882,22 +899,16 @@ int main(int argc, char *argv[])
}

if (node.forced_sync_offset ||
- (src != CLOCK_REALTIME && dst != CLOCK_REALTIME) ||
- src == CLOCK_INVALID)
+ (src->clkid != CLOCK_REALTIME && dst->clkid != CLOCK_REALTIME) ||
+ src->clkid == CLOCK_INVALID)
close_pmc(&node);
}

if (pps_fd >= 0) {
/* only one destination clock allowed with PPS until we
* implement a mean to specify PTP port to PPS mapping */
- struct clock *dst_clock;
-
- LIST_FOREACH(dst_clock, &node.clocks, list) {
- if (dst_clock != node.master)
- break;
- }
- servo_sync_interval(dst_clock->servo, 1.0);
- return do_pps_loop(&node, dst_clock, pps_fd);
+ servo_sync_interval(dst->servo, 1.0);
+ return do_pps_loop(&node, dst, pps_fd);
}

return do_loop(&node);
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:37 UTC
Permalink
Implement pmc_target_port to set a port number, leaving clock identity
unchanged, and pmc_target_all to set clock identity and port number to all
1's.

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

diff --git a/pmc_common.c b/pmc_common.c
index 266462575bd8..061529e25b83 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -78,7 +78,7 @@ struct pmc *pmc_create(enum transport_type transport_type, const char *iface_nam
goto failed;
}
pmc->port_identity.portNumber = 1;
- memset(&pmc->target, 0xff, sizeof(pmc->target));
+ pmc_target_all(pmc);

pmc->boundary_hops = boundary_hops;
pmc->domain_number = domain_number;
@@ -327,3 +327,13 @@ int pmc_target(struct pmc *pmc, struct PortIdentity *pid)
pmc->target = *pid;
return 0;
}
+
+void pmc_target_port(struct pmc *pmc, UInteger16 portNumber)
+{
+ pmc->target.portNumber = portNumber;
+}
+
+void pmc_target_all(struct pmc *pmc)
+{
+ memset(&pmc->target, 0xff, sizeof(pmc->target));
+}
diff --git a/pmc_common.h b/pmc_common.h
index 850d85fe13e8..d55dfba56e7c 100644
--- a/pmc_common.h
+++ b/pmc_common.h
@@ -40,5 +40,7 @@ int pmc_send_set_action(struct pmc *pmc, int id, void *data, int datasize);
struct ptp_message *pmc_recv(struct pmc *pmc);

int pmc_target(struct pmc *pmc, struct PortIdentity *pid);
+void pmc_target_port(struct pmc *pmc, UInteger16 portNumber);
+void pmc_target_all(struct pmc *pmc);

#endif
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:38 UTC
Permalink
Signed-off-by: Jiri Benc <***@redhat.com>
---
pmc_common.c | 16 ++++++++++++++--
pmc_common.h | 2 +-
2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/pmc_common.c b/pmc_common.c
index 061529e25b83..763b06e8e4c5 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -260,12 +260,14 @@ int pmc_send_get_action(struct pmc *pmc, int id)
return 0;
}

-int pmc_send_set_action(struct pmc *pmc, int id, void *data, int datasize)
+static int pmc_send_data_action(struct pmc *pmc, int action, int id,
+ void *data, int datasize)
{
int pdulen;
struct ptp_message *msg;
struct management_tlv *mgt;
- msg = pmc_message(pmc, SET);
+
+ msg = pmc_message(pmc, action);
if (!msg) {
return -1;
}
@@ -283,6 +285,16 @@ int pmc_send_set_action(struct pmc *pmc, int id, void *data, int datasize)
return 0;
}

+int pmc_send_set_action(struct pmc *pmc, int id, void *data, int datasize)
+{
+ return pmc_send_data_action(pmc, GET, id, data, datasize);
+}
+
+int pmc_send_command_action(struct pmc *pmc, int id, void *data, int datasize)
+{
+ return pmc_send_data_action(pmc, COMMAND, id, data, datasize);
+}
+
struct ptp_message *pmc_recv(struct pmc *pmc)
{
struct ptp_message *msg;
diff --git a/pmc_common.h b/pmc_common.h
index d55dfba56e7c..c56c50ada06a 100644
--- a/pmc_common.h
+++ b/pmc_common.h
@@ -34,8 +34,8 @@ void pmc_destroy(struct pmc *pmc);
int pmc_get_transport_fd(struct pmc *pmc);

int pmc_send_get_action(struct pmc *pmc, int id);
-
int pmc_send_set_action(struct pmc *pmc, int id, void *data, int datasize);
+int pmc_send_command_action(struct pmc *pmc, int id, void *data, int datasize);

struct ptp_message *pmc_recv(struct pmc *pmc);
--
1.7.6.5
Miroslav Lichvar
2014-04-02 14:27:56 UTC
Permalink
This patch seems to break pmc SET actions. The 20-pmc test from the
testsuite fails.
Post by Jiri Benc
+int pmc_send_set_action(struct pmc *pmc, int id, void *data, int datasize)
+{
+ return pmc_send_data_action(pmc, GET, id, data, datasize);
Looks like typo, with SET it works again.
--
Miroslav Lichvar
Jiri Benc
2014-04-02 14:39:14 UTC
Permalink
Post by Miroslav Lichvar
This patch seems to break pmc SET actions. The 20-pmc test from the
testsuite fails.
Post by Jiri Benc
+int pmc_send_set_action(struct pmc *pmc, int id, void *data, int datasize)
+{
+ return pmc_send_data_action(pmc, GET, id, data, datasize);
Looks like typo, with SET it works again.
Indeed, thanks for catching it.

Jiri
--
Jiri Benc
Jiri Benc
2014-03-24 08:53:40 UTC
Permalink
Add automatic configuration option (-a).

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

diff --git a/phc2sys.c b/phc2sys.c
index 369fb7c177a5..bc5c4dc61073 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -22,6 +22,7 @@
#include <float.h>
#include <inttypes.h>
#include <limits.h>
+#include <net/if.h>
#include <poll.h>
#include <stdint.h>
#include <stdio.h>
@@ -108,6 +109,8 @@ struct node {
static int update_sync_offset(struct node *node);
static int clock_handle_leap(struct node *node, struct clock *clock,
int64_t offset, uint64_t ts, int do_leap);
+static int run_pmc_get_utc_offset(struct node *node, int timeout);
+static void run_pmc_events(struct node *node);

static clockid_t clock_open(char *device)
{
@@ -262,6 +265,78 @@ static struct port *port_add(struct node *node, unsigned int number,
return p;
}

+static void clock_reinit(struct clock *clock)
+{
+ servo_reset(clock->servo);
+ clock->servo_state = SERVO_UNLOCKED;
+
+ if (clock->offset_stats) {
+ stats_reset(clock->offset_stats);
+ stats_reset(clock->freq_stats);
+ stats_reset(clock->delay_stats);
+ }
+}
+
+static void reconfigure(struct node *node)
+{
+ struct clock *c, *rt, *src;
+ int src_cnt = 0, dst_cnt = 0;
+
+ pr_info("reconfiguring after port state change");
+ node->state_changed = 0;
+
+ src = rt = NULL;
+ LIST_FOREACH(c, &node->clocks, list) {
+ if (c->clkid == CLOCK_REALTIME) {
+ rt = c;
+ continue;
+ }
+
+ if (c->new_state == PS_MASTER)
+ clock_reinit(c);
+
+ c->state = c->new_state;
+ c->new_state = 0;
+
+ if (c->state == PS_SLAVE) {
+ src = c;
+ src_cnt++;
+ } else if (c->state == PS_UNCALIBRATED) {
+ src_cnt++;
+ } else if (c->state == PS_MASTER) {
+ pr_info("selecting %s for synchronization", c->device);
+ dst_cnt++;
+ }
+ }
+ if (src_cnt > 1) {
+ pr_info("multiple master clocks available, postponing sync...");
+ node->master = NULL;
+ return;
+ }
+ if (src_cnt > 0 && !src) {
+ pr_info("master clock not ready, waiting...");
+ node->master = NULL;
+ return;
+ }
+ if (!src_cnt && !dst_cnt) {
+ pr_info("no PHC ready, waiting...");
+ node->master = NULL;
+ return;
+ }
+ if (!src_cnt) {
+ src = rt;
+ rt->state = PS_SLAVE;
+ } else {
+ if (rt->state != PS_MASTER) {
+ rt->state = PS_MASTER;
+ clock_reinit(rt);
+ }
+ pr_info("selecting %s for synchronization", rt->device);
+ }
+ node->master = src;
+ pr_info("selecting %s as the master clock", src->device);
+}
+
static int read_phc(clockid_t clkid, clockid_t sysclk, int readings,
int64_t *offset, uint64_t *ts, int64_t *delay)
{
@@ -469,13 +544,12 @@ static int do_pps_loop(struct node *node, struct clock *clock, int fd)
return 0;
}

-static int do_loop(struct node *node)
+static int do_loop(struct node *node, int subscriptions)
{
struct timespec interval;
struct clock *clock;
uint64_t ts;
int64_t offset, delay;
- int src_fd = CLOCKID_TO_FD(node->master->clkid);
int do_leap;

interval.tv_sec = node->phc_interval;
@@ -487,14 +561,30 @@ static int do_loop(struct node *node)
if (do_leap < 0)
continue;

+ if (subscriptions) {
+ run_pmc_events(node);
+ if (node->state_changed) {
+ /* force getting offset, as it may have
+ * changed after the port state change */
+ if (run_pmc_get_utc_offset(node, 1000) <= 0) {
+ pr_err("failed to get UTC offset");
+ continue;
+ }
+ reconfigure(node);
+ }
+ }
+ if (!node->master)
+ continue;
+
LIST_FOREACH(clock, &node->clocks, list) {
- if (clock == node->master)
+ if (clock->state != PS_MASTER)
continue;

if (clock->clkid == CLOCK_REALTIME &&
node->master->sysoff_supported) {
/* use sysoff */
- if (sysoff_measure(src_fd, node->phc_readings,
+ if (sysoff_measure(CLOCKID_TO_FD(node->master->clkid),
+ node->phc_readings,
&offset, &ts, &delay))
return -1;
} else {
@@ -752,12 +842,123 @@ static void run_pmc_events(struct node *node)
run_pmc(node, 0, -1, &msg);
}

+static int run_pmc_port_properties(struct node *node, int timeout,
+ unsigned int port,
+ int *state, int *tstamping, char *iface)
+{
+ struct ptp_message *msg;
+ int res, len;
+ struct port_properties_np *ppn;
+
+ pmc_target_port(node->pmc, port);
+ while (1) {
+ res = run_pmc(node, timeout, PORT_PROPERTIES_NP, &msg);
+ if (res <= 0)
+ goto out;
+
+ ppn = get_mgt_data(msg);
+ if (ppn->portIdentity.portNumber != port) {
+ msg_put(msg);
+ continue;
+ }
+
+ *state = ppn->portState;
+ *tstamping = ppn->timestamping;
+ len = ppn->interface.length;
+ if (len > IFNAMSIZ - 1)
+ len = IFNAMSIZ - 1;
+ memcpy(iface, ppn->interface.text, len);
+ iface[len] = '\0';
+
+ msg_put(msg);
+ return 1;
+ }
+out:
+ pmc_target_all(node->pmc);
+ return res;
+}
+
static void close_pmc(struct node *node)
{
pmc_destroy(node->pmc);
node->pmc = NULL;
}

+static int auto_init_ports(struct node *node)
+{
+ struct ptp_message *msg;
+ struct port_enumeration_np *pen;
+ struct port *port;
+ struct clock *clock;
+ int res;
+ unsigned int i;
+ int state, timestamping;
+ char iface[IFNAMSIZ];
+
+ while (1) {
+ res = run_pmc(node, 1000, PORT_ENUMERATION_NP, &msg);
+ if (res < 0)
+ return -1;
+ if (res > 0)
+ break;
+ /* res == 0, timeout */
+ pr_notice("Waiting for ptp4l...");
+ }
+ pen = get_mgt_data(msg);
+
+ res = run_pmc_subscribe(node, 1000);
+ if (res <= 0) {
+ pr_err("failed to subscribe");
+ res = -1;
+ goto out;
+ }
+
+ for (i = 0; i < pen->numberPorts; i++) {
+ res = run_pmc_port_properties(node, 1000, pen->portNumber[i],
+ &state, &timestamping, iface);
+ if (res <= 0) {
+ pr_err("failed to get port properties");
+ res = -1;
+ goto out;
+ }
+ if (timestamping == TS_SOFTWARE) {
+ /* ignore ports with software time stamping */
+ continue;
+ }
+ port = port_add(node, pen->portNumber[i], iface);
+ if (!port) {
+ res = -1;
+ goto out;
+ }
+ port->state = normalize_state(state);
+ }
+ if (LIST_EMPTY(&node->clocks)) {
+ pr_err("no suitable ports available");
+ res = -1;
+ goto out;
+ }
+ LIST_FOREACH(clock, &node->clocks, list) {
+ clock->new_state = clock_compute_state(node, clock);
+ }
+ node->state_changed = 1;
+
+ if (!clock_add(node, "CLOCK_REALTIME")) {
+ res = -1;
+ goto out;
+ }
+
+ /* get initial offset */
+ if (run_pmc_get_utc_offset(node, 1000) <= 0) {
+ pr_err("failed to get UTC offset");
+ res = -1;
+ goto out;
+ }
+ res = 0;
+out:
+ msg_put(msg);
+ return res;
+}
+
/* Returns: -1 in case of error, 0 for normal sync, 1 to leap clock */
static int update_sync_offset(struct node *node)
{
@@ -838,9 +1039,16 @@ static void usage(char *progname)
fprintf(stderr,
"\n"
"usage: %s [options]\n\n"
+ "\n"
+ " automatic configuration:\n"
+ " -a turn on autoconfiguration\n"
+ " manual configuration:\n"
" -c [dev|name] slave clock (CLOCK_REALTIME)\n"
" -d [dev] master PPS device\n"
" -s [dev|name] master clock\n"
+ " -O [offset] slave-master time offset (0)\n"
+ " -w wait for ptp4l\n"
+ " common options:\n"
" -E [pi|linreg] clock servo (pi)\n"
" -P [kp] proportional constant (0.7)\n"
" -I [ki] integration constant (0.3)\n"
@@ -848,10 +1056,8 @@ static void usage(char *progname)
" -F [step] step threshold only on start (0.00002)\n"
" -R [rate] slave clock update rate in HZ (1.0)\n"
" -N [num] number of master clock readings per update (5)\n"
- " -O [offset] slave-master time offset (0)\n"
" -L [limit] sanity frequency limit in ppb (200000000)\n"
" -u [num] number of clock updates in summary stats (0)\n"
- " -w wait for ptp4l\n"
" -n [num] domain number (0)\n"
" -x apply leap seconds by servo instead of kernel\n"
" -l [num] set the logging level to 'num' (6)\n"
@@ -868,6 +1074,7 @@ int main(int argc, char *argv[])
char *progname;
char *src_name = NULL, *dst_name = NULL;
struct clock *src, *dst;
+ int autocfg = 0;
int c, domain_number = 0, pps_fd = -1;
int r, wait_sync = 0;
int print_level = LOG_INFO, use_syslog = 1, verbose = 0;
@@ -887,8 +1094,11 @@ int main(int argc, char *argv[])
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
while (EOF != (c = getopt(argc, argv,
- "c:d:s:E:P:I:S:F:R:N:O:L:i:u:wn:xl:mqvh"))) {
+ "ac:d:s:E:P:I:S:F:R:N:O:L:i:u:wn:xl:mqvh"))) {
switch (c) {
+ case 'a':
+ autocfg = 1;
+ break;
case 'c':
dst_name = strdup(optarg);
break;
@@ -993,13 +1203,18 @@ int main(int argc, char *argv[])
}
}

- if (pps_fd < 0 && !src_name) {
+ if (autocfg && (src_name || dst_name || pps_fd >= 0 || wait_sync || node.forced_sync_offset)) {
fprintf(stderr,
- "valid source clock must be selected.\n");
+ "autoconfiguration cannot be mixed with manual config options.\n");
+ goto bad_usage;
+ }
+ if (!autocfg && pps_fd < 0 && !src_name) {
+ fprintf(stderr,
+ "autoconfiguration or valid source clock must be selected.\n");
goto bad_usage;
}

- if (!wait_sync && !node.forced_sync_offset) {
+ if (!autocfg && !wait_sync && !node.forced_sync_offset) {
fprintf(stderr,
"time offset must be specified using -w or -O\n");
goto bad_usage;
@@ -1010,10 +1225,20 @@ int main(int argc, char *argv[])
print_set_syslog(use_syslog);
print_set_level(print_level);

+ if (autocfg) {
+ if (init_pmc(&node, domain_number))
+ return -1;
+ if (auto_init_ports(&node) < 0)
+ return -1;
+ return do_loop(&node, 1);
+ }
+
src = clock_add(&node, src_name);
+ src->state = PS_SLAVE;
free(src_name);
node.master = src;
dst = clock_add(&node, dst_name ? dst_name : "CLOCK_REALTIME");
+ dst->state = PS_MASTER;
free(dst_name);

if (!dst) {
@@ -1069,7 +1294,7 @@ int main(int argc, char *argv[])
return do_pps_loop(&node, dst, pps_fd);
}

- return do_loop(&node);
+ return do_loop(&node, 0);

bad_usage:
usage(progname);
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:41 UTC
Permalink
By default, do not synchronize CLOCK_REALTIME. To do it, -r option is
needed. That will only consider CLOCK_REALTIME as the destination. To
consider it also as a possible time source, use -rr.

Signed-off-by: Jiri Benc <***@redhat.com>
---
phc2sys.c | 33 +++++++++++++++++++++++++--------
1 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index bc5c4dc61073..e184fc9cd637 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -67,6 +67,7 @@ struct clock {
clockid_t clkid;
int sysoff_supported;
int is_utc;
+ int dest_only;
int state;
int new_state;
struct servo *servo;
@@ -323,10 +324,16 @@ static void reconfigure(struct node *node)
node->master = NULL;
return;
}
+ if ((!src_cnt && (!rt || rt->dest_only)) ||
+ (!dst_cnt && !rt)) {
+ pr_info("nothing to synchronize");
+ node->master = NULL;
+ return;
+ }
if (!src_cnt) {
src = rt;
rt->state = PS_SLAVE;
- } else {
+ } else if (rt) {
if (rt->state != PS_MASTER) {
rt->state = PS_MASTER;
clock_reinit(rt);
@@ -884,7 +891,7 @@ static void close_pmc(struct node *node)
node->pmc = NULL;
}

-static int auto_init_ports(struct node *node)
+static int auto_init_ports(struct node *node, int add_rt)
{
struct ptp_message *msg;
struct port_enumeration_np *pen;
@@ -942,9 +949,14 @@ static int auto_init_ports(struct node *node)
}
node->state_changed = 1;

- if (!clock_add(node, "CLOCK_REALTIME")) {
- res = -1;
- goto out;
+ if (add_rt) {
+ clock = clock_add(node, "CLOCK_REALTIME");
+ if (!clock) {
+ res = -1;
+ goto out;
+ }
+ if (add_rt == 1)
+ clock->dest_only = 1;
}

/* get initial offset */
@@ -1042,6 +1054,8 @@ static void usage(char *progname)
"\n"
" automatic configuration:\n"
" -a turn on autoconfiguration\n"
+ " -r synchronize system (realtime) clock\n"
+ " repeat -r to consider it also as a time source\n"
" manual configuration:\n"
" -c [dev|name] slave clock (CLOCK_REALTIME)\n"
" -d [dev] master PPS device\n"
@@ -1074,7 +1088,7 @@ int main(int argc, char *argv[])
char *progname;
char *src_name = NULL, *dst_name = NULL;
struct clock *src, *dst;
- int autocfg = 0;
+ int autocfg = 0, rt = 0;
int c, domain_number = 0, pps_fd = -1;
int r, wait_sync = 0;
int print_level = LOG_INFO, use_syslog = 1, verbose = 0;
@@ -1094,11 +1108,14 @@ int main(int argc, char *argv[])
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
while (EOF != (c = getopt(argc, argv,
- "ac:d:s:E:P:I:S:F:R:N:O:L:i:u:wn:xl:mqvh"))) {
+ "arc:d:s:E:P:I:S:F:R:N:O:L:i:u:wn:xl:mqvh"))) {
switch (c) {
case 'a':
autocfg = 1;
break;
+ case 'r':
+ rt++;
+ break;
case 'c':
dst_name = strdup(optarg);
break;
@@ -1228,7 +1245,7 @@ int main(int argc, char *argv[])
if (autocfg) {
if (init_pmc(&node, domain_number))
return -1;
- if (auto_init_ports(&node) < 0)
+ if (auto_init_ports(&node, rt) < 0)
return -1;
return do_loop(&node, 1);
}
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:42 UTC
Permalink
Make sure that we handle only one PTP clock (node). This is for an extra
safety.

Signed-off-by: Jiri Benc <***@redhat.com>
---
phc2sys.c | 39 +++++++++++++++++++++++++++++++++++++--
1 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index e184fc9cd637..108a03f83b8b 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -102,6 +102,8 @@ struct node {
int pmc_ds_requested;
uint64_t pmc_last_update;
int state_changed;
+ int clock_identity_set;
+ struct ClockIdentity clock_identity;
LIST_HEAD(port_head, port) ports;
LIST_HEAD(clock_head, clock) clocks;
struct clock *master;
@@ -607,6 +609,15 @@ static int do_loop(struct node *node, int subscriptions)
return 0; /* unreachable */
}

+static int check_clock_identity(struct node *node, struct ptp_message *msg)
+{
+ if (!node->clock_identity_set)
+ return 1;
+ return !memcmp(&node->clock_identity,
+ &msg->header.sourcePortIdentity.clockIdentity,
+ sizeof(struct ClockIdentity));
+}
+
static int is_msg_mgt(struct ptp_message *msg)
{
struct TLV *tlv;
@@ -767,7 +778,8 @@ static int run_pmc(struct node *node, int timeout, int ds_id,
if (!*msg)
continue;

- if (!is_msg_mgt(*msg) ||
+ if (!check_clock_identity(node, *msg) ||
+ !is_msg_mgt(*msg) ||
recv_subscribed(node, *msg, ds_id) ||
get_mgt_id(*msg) != ds_id) {
msg_put(*msg);
@@ -885,6 +897,24 @@ out:
return res;
}

+static int run_pmc_clock_identity(struct node *node, int timeout)
+{
+ struct ptp_message *msg;
+ struct defaultDS *dds;
+ int res;
+
+ res = run_pmc(node, timeout, DEFAULT_DATA_SET, &msg);
+ if (res <= 0)
+ return res;
+
+ dds = (struct defaultDS *)get_mgt_data(msg);
+ memcpy(&node->clock_identity, &dds->clockIdentity,
+ sizeof(struct ClockIdentity));
+ node->clock_identity_set = 1;
+ msg_put(msg);
+ return 1;
+}
+
static void close_pmc(struct node *node)
{
pmc_destroy(node->pmc);
@@ -903,7 +933,7 @@ static int auto_init_ports(struct node *node, int add_rt)
char iface[IFNAMSIZ];

while (1) {
- res = run_pmc(node, 1000, PORT_ENUMERATION_NP, &msg);
+ res = run_pmc_clock_identity(node, 1000);
if (res < 0)
return -1;
if (res > 0)
@@ -911,6 +941,11 @@ static int auto_init_ports(struct node *node, int add_rt)
/* res == 0, timeout */
pr_notice("Waiting for ptp4l...");
}
+ res = run_pmc(node, 1000, PORT_ENUMERATION_NP, &msg);
+ if (res <= 0) {
+ pr_err("failed to enumerate ports");
+ return -1;
+ }
pen = get_mgt_data(msg);

res = run_pmc_subscribe(node, 1000);
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:39 UTC
Permalink
Add support for subscribing to events (run_pmc_subscribe) and receiving and
handling of received events (run_pmc_events).

Add initial support for port status changes.

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

diff --git a/phc2sys.c b/phc2sys.c
index ece4560e0c67..369fb7c177a5 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -41,6 +41,7 @@
#include "ds.h"
#include "fsm.h"
#include "missing.h"
+#include "notification.h"
#include "phc.h"
#include "pi.h"
#include "pmc_common.h"
@@ -65,6 +66,8 @@ struct clock {
clockid_t clkid;
int sysoff_supported;
int is_utc;
+ int state;
+ int new_state;
struct servo *servo;
enum servo_state servo_state;
char *device;
@@ -78,6 +81,7 @@ struct clock {
struct port {
LIST_ENTRY(port) list;
unsigned int number;
+ int state;
struct clock *clock;
};

@@ -95,6 +99,7 @@ struct node {
struct pmc *pmc;
int pmc_ds_requested;
uint64_t pmc_last_update;
+ int state_changed;
LIST_HEAD(port_head, port) ports;
LIST_HEAD(clock_head, clock) clocks;
struct clock *master;
@@ -511,7 +516,8 @@ static int is_msg_mgt(struct ptp_message *msg)

if (msg_type(msg) != MANAGEMENT)
return 0;
- if (management_action(msg) != RESPONSE)
+ if (management_action(msg) != RESPONSE &&
+ management_action(msg) != ACKNOWLEDGE)
return 0;
if (msg->tlv_count != 1)
return 0;
@@ -533,6 +539,79 @@ static void *get_mgt_data(struct ptp_message *msg)
return mgt->data;
}

+static int normalize_state(int state)
+{
+ if (state != PS_MASTER && state != PS_SLAVE &&
+ state != PS_PRE_MASTER && state != PS_UNCALIBRATED) {
+ /* treat any other state as "not a master nor a slave" */
+ state = PS_DISABLED;
+ }
+ return state;
+}
+
+static int clock_compute_state(struct node *node, struct clock *clock)
+{
+ struct port *p;
+ int state = PS_DISABLED;
+
+ LIST_FOREACH(p, &node->ports, list) {
+ if (p->clock != clock)
+ continue;
+ /* PS_SLAVE takes the highest precedence, PS_UNCALIBRATED
+ * after that, PS_MASTER is third, PS_PRE_MASTER fourth and
+ * all of that overrides PS_DISABLED, which corresponds
+ * nicely with the numerical values */
+ if (p->state > state)
+ state = p->state;
+ }
+ return state;
+}
+
+static int recv_subscribed(struct node *node, struct ptp_message *msg,
+ int excluded)
+{
+ int mgt_id, state;
+ struct portDS *pds;
+ struct port *port;
+ struct clock *clock;
+
+ mgt_id = get_mgt_id(msg);
+ if (mgt_id == excluded)
+ return 0;
+ switch (mgt_id) {
+ case PORT_DATA_SET:
+ pds = get_mgt_data(msg);
+ port = port_get(node, pds->portIdentity.portNumber);
+ if (!port) {
+ pr_info("received data for unknown port %s",
+ pid2str(&pds->portIdentity));
+ return 1;
+ }
+ state = normalize_state(pds->portState);
+ if (port->state != state) {
+ pr_info("port %s changed state",
+ pid2str(&pds->portIdentity));
+ port->state = state;
+ clock = port->clock;
+ state = clock_compute_state(node, clock);
+ if (clock->state != state) {
+ clock->new_state = state;
+ node->state_changed = 1;
+ }
+ }
+ return 1;
+ }
+ return 0;
+}
+
+static void send_subscription(struct node *node)
+{
+ struct subscribe_events_np sen;
+
+ sen.bitmask = 1 << NOTIFY_PORT_STATE;
+ pmc_send_command_action(node->pmc, SUBSCRIBE_EVENTS_NP, &sen, sizeof(sen));
+}
+
static int init_pmc(struct node *node, int domain_number)
{
node->pmc = pmc_create(TRANS_UDS, "/var/run/phc2sys", 0,
@@ -555,7 +634,7 @@ static int run_pmc(struct node *node, int timeout, int ds_id,
while (1) {
pollfd[0].fd = pmc_get_transport_fd(node->pmc);
pollfd[0].events = POLLIN|POLLPRI;
- if (!node->pmc_ds_requested)
+ if (!node->pmc_ds_requested && ds_id >= 0)
pollfd[0].events |= POLLOUT;

cnt = poll(pollfd, N_FD, timeout);
@@ -572,7 +651,14 @@ static int run_pmc(struct node *node, int timeout, int ds_id,
/* Send a new request if there are no pending messages. */
if ((pollfd[0].revents & POLLOUT) &&
!(pollfd[0].revents & (POLLIN|POLLPRI))) {
- pmc_send_get_action(node->pmc, ds_id);
+ switch (ds_id) {
+ case SUBSCRIBE_EVENTS_NP:
+ send_subscription(node);
+ break;
+ default:
+ pmc_send_get_action(node->pmc, ds_id);
+ break;
+ }
node->pmc_ds_requested = 1;
}

@@ -585,6 +671,7 @@ static int run_pmc(struct node *node, int timeout, int ds_id,
continue;

if (!is_msg_mgt(*msg) ||
+ recv_subscribed(node, *msg, ds_id) ||
get_mgt_id(*msg) != ds_id) {
msg_put(*msg);
*msg = NULL;
@@ -645,6 +732,26 @@ static int run_pmc_get_utc_offset(struct node *node, int timeout)
return 1;
}

+static int run_pmc_subscribe(struct node *node, int timeout)
+{
+ struct ptp_message *msg;
+ int res;
+
+ res = run_pmc(node, timeout, SUBSCRIBE_EVENTS_NP, &msg);
+ if (res <= 0)
+ return res;
+ res = management_action(msg) == ACKNOWLEDGE;
+ msg_put(msg);
+ return res;
+}
+
+static void run_pmc_events(struct node *node)
+{
+ struct ptp_message *msg;
+
+ run_pmc(node, 0, -1, &msg);
+}
+
static void close_pmc(struct node *node)
{
pmc_destroy(node->pmc);
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:31 UTC
Permalink
Split the generic (global) part of update_sync_offset and the part that
affects individual clocks. This is in preparation for phc2sys handling
synchronization of more clocks.

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

diff --git a/phc2sys.c b/phc2sys.c
index 0581eb5bcb24..19dce45964eb 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -60,7 +60,9 @@
#define PMC_UPDATE_INTERVAL (60 * NS_PER_SEC)

struct clock;
-static int update_sync_offset(struct clock *clock, int64_t offset, uint64_t ts);
+static int update_sync_offset(struct clock *clock);
+static int clock_handle_leap(struct clock *clock, clockid_t src,
+ int64_t offset, uint64_t ts, int do_leap);

static clockid_t clock_open(char *device)
{
@@ -181,13 +183,14 @@ static void update_clock_stats(struct clock *clock,
stats_reset(clock->delay_stats);
}

-static void update_clock(struct clock *clock,
- int64_t offset, uint64_t ts, int64_t delay)
+static void update_clock(struct clock *clock, clockid_t src,
+ int64_t offset, uint64_t ts, int64_t delay,
+ int do_leap)
{
enum servo_state state;
double ppb;

- if (update_sync_offset(clock, offset, ts))
+ if (clock_handle_leap(clock, src, offset, ts, do_leap))
return;

if (clock->sync_offset_direction)
@@ -268,6 +271,7 @@ static int do_pps_loop(struct clock *clock, int fd,
{
int64_t pps_offset, phc_offset, phc_delay;
uint64_t pps_ts, phc_ts;
+ int do_leap;

clock->source_label = "pps";

@@ -304,7 +308,10 @@ static int do_pps_loop(struct clock *clock, int fd,
pps_offset = pps_ts - phc_ts;
}

- update_clock(clock, pps_offset, pps_ts, -1);
+ do_leap = update_sync_offset(clock);
+ if (do_leap <= 0)
+ continue;
+ update_clock(clock, src, pps_offset, pps_ts, -1, do_leap);
}
close(fd);
return 0;
@@ -316,6 +323,7 @@ static int do_sysoff_loop(struct clock *clock, clockid_t src,
uint64_t ts;
int64_t offset, delay;
int err = 0, fd = CLOCKID_TO_FD(src);
+ int do_leap;

clock->source_label = "sys";

@@ -325,7 +333,10 @@ static int do_sysoff_loop(struct clock *clock, clockid_t src,
err = -1;
break;
}
- update_clock(clock, offset, ts, delay);
+ do_leap = update_sync_offset(clock);
+ if (do_leap <= 0)
+ continue;
+ update_clock(clock, src, offset, ts, delay, do_leap);
}
return err;
}
@@ -335,6 +346,7 @@ static int do_phc_loop(struct clock *clock, clockid_t src,
{
uint64_t ts;
int64_t offset, delay;
+ int do_leap;

clock->source_label = "phc";

@@ -344,7 +356,10 @@ static int do_phc_loop(struct clock *clock, clockid_t src,
&offset, &ts, &delay)) {
continue;
}
- update_clock(clock, offset, ts, delay);
+ do_leap = update_sync_offset(clock);
+ if (do_leap <= 0)
+ continue;
+ update_clock(clock, src, offset, ts, delay, do_leap);
}
return 0;
}
@@ -495,10 +510,19 @@ static void close_pmc(struct clock *clock)
clock->pmc = NULL;
}

-static int update_sync_offset(struct clock *clock, int64_t offset, uint64_t ts)
+/* Returns: -1 in case of error, 0 for normal sync, 1 to leap clock */
+static int update_sync_offset(struct clock *clock)
{
+ struct timespec tp;
+ uint64_t ts;
int clock_leap;

+ if (clock_gettime(CLOCK_REALTIME, &tp)) {
+ pr_err("failed to read clock: %m");
+ return -1;
+ }
+ ts = tp.tv_sec * NS_PER_SEC + tp.tv_nsec;
+
if (clock->pmc &&
!(ts > clock->pmc_last_update &&
ts - clock->pmc_last_update < PMC_UPDATE_INTERVAL)) {
@@ -511,9 +535,28 @@ static int update_sync_offset(struct clock *clock, int64_t offset, uint64_t ts)
if (!clock->leap && !clock->leap_set)
return 0;

+ clock_leap = leap_second_status(ts, clock->leap_set,
+ &clock->leap, &clock->sync_offset);
+ if (clock->leap_set != clock_leap) {
+ clock->leap_set = clock_leap;
+ return 1;
+ }
+ return 0;
+}
+
+/* Returns: non-zero to skip clock update */
+static int clock_handle_leap(struct clock *clock, clockid_t src,
+ int64_t offset, uint64_t ts, int do_leap)
+{
+ if (!clock->leap && !do_leap)
+ return 0;
+
+ if (clock->clkid != CLOCK_REALTIME && src != CLOCK_REALTIME)
+ return 0;
+
/* If the system clock is the master clock, get a time stamp from
it, as it is the clock which will include the leap second. */
- if (clock->clkid != CLOCK_REALTIME) {
+ if (src == CLOCK_REALTIME) {
struct timespec tp;
if (clock_gettime(CLOCK_REALTIME, &tp)) {
pr_err("failed to read clock: %m");
@@ -533,17 +576,13 @@ static int update_sync_offset(struct clock *clock, int64_t offset, uint64_t ts)
/* Suspend clock updates in the last second before midnight. */
if (is_utc_ambiguous(ts)) {
pr_info("clock update suspended due to leap second");
- return -1;
+ return 1;
}

- clock_leap = leap_second_status(ts, clock->leap_set,
- &clock->leap, &clock->sync_offset);
-
- if (clock->leap_set != clock_leap) {
+ if (do_leap) {
/* Only the system clock can leap. */
if (clock->clkid == CLOCK_REALTIME && clock->kernel_leap)
- sysclk_set_leap(clock_leap);
- clock->leap_set = clock_leap;
+ sysclk_set_leap(clock->leap_set);
}

return 0;
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:32 UTC
Permalink
Split members that apply to all synchronized clocks and members that apply
to an individual clock. Keep all clocks in a list, with a pointer to the
source clock. This will allow to support multiple clocks synchronization.

Signed-off-by: Jiri Benc <***@redhat.com>
---
phc2sys.c | 397 +++++++++++++++++++++++++++++++++---------------------------
1 files changed, 218 insertions(+), 179 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 19dce45964eb..825d7328af15 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -28,6 +28,7 @@
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
+#include <sys/queue.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
@@ -60,9 +61,6 @@
#define PMC_UPDATE_INTERVAL (60 * NS_PER_SEC)

struct clock;
-static int update_sync_offset(struct clock *clock);
-static int clock_handle_leap(struct clock *clock, clockid_t src,
- int64_t offset, uint64_t ts, int do_leap);

static clockid_t clock_open(char *device)
{
@@ -129,14 +127,24 @@ static int read_phc(clockid_t clkid, clockid_t sysclk, int readings,
}

struct clock {
+ LIST_ENTRY(clock) list;
clockid_t clkid;
+ int sysoff_supported;
struct servo *servo;
enum servo_state servo_state;
const char *source_label;
struct stats *offset_stats;
struct stats *freq_stats;
struct stats *delay_stats;
+ struct clockcheck *sanity_check;
+};
+
+struct node {
unsigned int stats_max_count;
+ int sanity_freq_limit;
+ enum servo_type servo_type;
+ int phc_readings;
+ double phc_interval;
int sync_offset;
int sync_offset_direction;
int leap;
@@ -145,10 +153,15 @@ struct clock {
struct pmc *pmc;
int pmc_ds_requested;
uint64_t pmc_last_update;
- struct clockcheck *sanity_check;
+ LIST_HEAD(clock_head, clock) clocks;
+ struct clock *master;
};

-static void update_clock_stats(struct clock *clock,
+static int update_sync_offset(struct node *node);
+static int clock_handle_leap(struct node *node, struct clock *clock,
+ int64_t offset, uint64_t ts, int do_leap);
+
+static void update_clock_stats(struct clock *clock, unsigned int max_count,
int64_t offset, double freq, int64_t delay)
{
struct stats_result offset_stats, freq_stats, delay_stats;
@@ -158,7 +171,7 @@ static void update_clock_stats(struct clock *clock,
if (delay >= 0)
stats_add_value(clock->delay_stats, delay);

- if (stats_get_num_values(clock->offset_stats) < clock->stats_max_count)
+ if (stats_get_num_values(clock->offset_stats) < max_count)
return;

stats_get_result(clock->offset_stats, &offset_stats);
@@ -183,19 +196,19 @@ static void update_clock_stats(struct clock *clock,
stats_reset(clock->delay_stats);
}

-static void update_clock(struct clock *clock, clockid_t src,
+static void update_clock(struct node *node, struct clock *clock,
int64_t offset, uint64_t ts, int64_t delay,
int do_leap)
{
enum servo_state state;
double ppb;

- if (clock_handle_leap(clock, src, offset, ts, do_leap))
+ if (clock_handle_leap(node, clock, offset, ts, do_leap))
return;

- if (clock->sync_offset_direction)
- offset += clock->sync_offset * NS_PER_SEC *
- clock->sync_offset_direction;
+ if (node->sync_offset_direction)
+ offset += node->sync_offset * NS_PER_SEC *
+ node->sync_offset_direction;

if (clock->sanity_check && clockcheck_sample(clock->sanity_check, ts))
servo_reset(clock->servo);
@@ -221,15 +234,15 @@ static void update_clock(struct clock *clock, clockid_t src,
}

if (clock->offset_stats) {
- update_clock_stats(clock, offset, ppb, delay);
+ update_clock_stats(clock, node->stats_max_count, offset, ppb, delay);
} else {
if (delay >= 0) {
pr_info("%s offset %9" PRId64 " s%d freq %+7.0f "
"delay %6" PRId64,
- clock->source_label, offset, state, ppb, delay);
+ node->master->source_label, offset, state, ppb, delay);
} else {
pr_info("%s offset %9" PRId64 " s%d freq %+7.0f",
- clock->source_label, offset, state, ppb);
+ node->master->source_label, offset, state, ppb);
}
}
}
@@ -266,20 +279,20 @@ static int read_pps(int fd, int64_t *offset, uint64_t *ts)
return 1;
}

-static int do_pps_loop(struct clock *clock, int fd,
- clockid_t src, int n_readings)
+static int do_pps_loop(struct node *node, struct clock *clock, int fd)
{
int64_t pps_offset, phc_offset, phc_delay;
uint64_t pps_ts, phc_ts;
+ clockid_t src = node->master->clkid;
int do_leap;

- clock->source_label = "pps";
+ node->master->source_label = "pps";

if (src == CLOCK_INVALID) {
/* The sync offset can't be applied with PPS alone. */
- clock->sync_offset_direction = 0;
+ node->sync_offset_direction = 0;
} else {
- enable_pps_output(src);
+ enable_pps_output(node->master->clkid);
}

while (1) {
@@ -290,7 +303,7 @@ static int do_pps_loop(struct clock *clock, int fd,
/* If a PHC is available, use it to get the whole number
of seconds in the offset and PPS for the rest. */
if (src != CLOCK_INVALID) {
- if (!read_phc(src, clock->clkid, n_readings,
+ if (!read_phc(src, clock->clkid, node->phc_readings,
&phc_offset, &phc_ts, &phc_delay))
return -1;

@@ -308,60 +321,54 @@ static int do_pps_loop(struct clock *clock, int fd,
pps_offset = pps_ts - phc_ts;
}

- do_leap = update_sync_offset(clock);
- if (do_leap <= 0)
+ do_leap = update_sync_offset(node);
+ if (do_leap < 0)
continue;
- update_clock(clock, src, pps_offset, pps_ts, -1, do_leap);
+ update_clock(node, clock, pps_offset, pps_ts, -1, do_leap);
}
close(fd);
return 0;
}

-static int do_sysoff_loop(struct clock *clock, clockid_t src,
- struct timespec *interval, int n_readings)
+static int do_loop(struct node *node)
{
+ struct timespec interval;
+ struct clock *clock;
uint64_t ts;
int64_t offset, delay;
- int err = 0, fd = CLOCKID_TO_FD(src);
+ int src_fd = CLOCKID_TO_FD(node->master->clkid);
int do_leap;

- clock->source_label = "sys";
+ interval.tv_sec = node->phc_interval;
+ interval.tv_nsec = (node->phc_interval - interval.tv_sec) * 1e9;

while (1) {
- clock_nanosleep(CLOCK_MONOTONIC, 0, interval, NULL);
- if (sysoff_measure(fd, n_readings, &offset, &ts, &delay)) {
- err = -1;
- break;
- }
- do_leap = update_sync_offset(clock);
- if (do_leap <= 0)
+ clock_nanosleep(CLOCK_MONOTONIC, 0, &interval, NULL);
+ do_leap = update_sync_offset(node);
+ if (do_leap < 0)
continue;
- update_clock(clock, src, offset, ts, delay, do_leap);
- }
- return err;
-}
-
-static int do_phc_loop(struct clock *clock, clockid_t src,
- struct timespec *interval, int n_readings)
-{
- uint64_t ts;
- int64_t offset, delay;
- int do_leap;

- clock->source_label = "phc";
+ LIST_FOREACH(clock, &node->clocks, list) {
+ if (clock == node->master)
+ continue;

- while (1) {
- clock_nanosleep(CLOCK_MONOTONIC, 0, interval, NULL);
- if (!read_phc(src, clock->clkid, n_readings,
- &offset, &ts, &delay)) {
- continue;
+ if (clock->clkid == CLOCK_REALTIME &&
+ node->master->sysoff_supported) {
+ /* use sysoff */
+ if (sysoff_measure(src_fd, node->phc_readings,
+ &offset, &ts, &delay))
+ return -1;
+ } else {
+ /* use phc */
+ if (!read_phc(node->master->clkid, clock->clkid,
+ node->phc_readings,
+ &offset, &ts, &delay))
+ continue;
+ }
+ update_clock(node, clock, offset, ts, delay, do_leap);
}
- do_leap = update_sync_offset(clock);
- if (do_leap <= 0)
- continue;
- update_clock(clock, src, offset, ts, delay, do_leap);
}
- return 0;
+ return 0; /* unreachable */
}

static int is_msg_mgt(struct ptp_message *msg)
@@ -392,11 +399,11 @@ static void *get_mgt_data(struct ptp_message *msg)
return mgt->data;
}

-static int init_pmc(struct clock *clock, int domain_number)
+static int init_pmc(struct node *node, int domain_number)
{
- clock->pmc = pmc_create(TRANS_UDS, "/var/run/phc2sys", 0,
+ node->pmc = pmc_create(TRANS_UDS, "/var/run/phc2sys", 0,
domain_number, 0, 1);
- if (!clock->pmc) {
+ if (!node->pmc) {
pr_err("failed to create pmc");
return -1;
}
@@ -404,7 +411,7 @@ static int init_pmc(struct clock *clock, int domain_number)
return 0;
}

-static int run_pmc(struct clock *clock, int timeout, int ds_id,
+static int run_pmc(struct node *node, int timeout, int ds_id,
struct ptp_message **msg)
{
#define N_FD 1
@@ -412,9 +419,9 @@ static int run_pmc(struct clock *clock, int timeout, int ds_id,
int cnt;

while (1) {
- pollfd[0].fd = pmc_get_transport_fd(clock->pmc);
+ pollfd[0].fd = pmc_get_transport_fd(node->pmc);
pollfd[0].events = POLLIN|POLLPRI;
- if (!clock->pmc_ds_requested)
+ if (!node->pmc_ds_requested)
pollfd[0].events |= POLLOUT;

cnt = poll(pollfd, N_FD, timeout);
@@ -424,21 +431,21 @@ static int run_pmc(struct clock *clock, int timeout, int ds_id,
}
if (!cnt) {
/* Request the data set again in the next run. */
- clock->pmc_ds_requested = 0;
+ node->pmc_ds_requested = 0;
return 0;
}

/* Send a new request if there are no pending messages. */
if ((pollfd[0].revents & POLLOUT) &&
!(pollfd[0].revents & (POLLIN|POLLPRI))) {
- pmc_send_get_action(clock->pmc, ds_id);
- clock->pmc_ds_requested = 1;
+ pmc_send_get_action(node->pmc, ds_id);
+ node->pmc_ds_requested = 1;
}

if (!(pollfd[0].revents & (POLLIN|POLLPRI)))
continue;

- *msg = pmc_recv(clock->pmc);
+ *msg = pmc_recv(node->pmc);

if (!*msg)
continue;
@@ -449,12 +456,12 @@ static int run_pmc(struct clock *clock, int timeout, int ds_id,
*msg = NULL;
continue;
}
- clock->pmc_ds_requested = 0;
+ node->pmc_ds_requested = 0;
return 1;
}
}

-static int run_pmc_wait_sync(struct clock *clock, int timeout)
+static int run_pmc_wait_sync(struct node *node, int timeout)
{
struct ptp_message *msg;
int res;
@@ -462,7 +469,7 @@ static int run_pmc_wait_sync(struct clock *clock, int timeout)
Enumeration8 portState;

while (1) {
- res = run_pmc(clock, timeout, PORT_DATA_SET, &msg);
+ res = run_pmc(node, timeout, PORT_DATA_SET, &msg);
if (res <= 0)
return res;

@@ -476,42 +483,42 @@ static int run_pmc_wait_sync(struct clock *clock, int timeout)
return 1;
}
/* try to get more data sets (for other ports) */
- clock->pmc_ds_requested = 1;
+ node->pmc_ds_requested = 1;
}
}

-static int run_pmc_get_utc_offset(struct clock *clock, int timeout)
+static int run_pmc_get_utc_offset(struct node *node, int timeout)
{
struct ptp_message *msg;
int res;
struct timePropertiesDS *tds;

- res = run_pmc(clock, timeout, TIME_PROPERTIES_DATA_SET, &msg);
+ res = run_pmc(node, timeout, TIME_PROPERTIES_DATA_SET, &msg);
if (res <= 0)
return res;

tds = (struct timePropertiesDS *)get_mgt_data(msg);
if (tds->flags & PTP_TIMESCALE) {
- clock->sync_offset = tds->currentUtcOffset;
+ node->sync_offset = tds->currentUtcOffset;
if (tds->flags & LEAP_61)
- clock->leap = 1;
+ node->leap = 1;
else if (tds->flags & LEAP_59)
- clock->leap = -1;
+ node->leap = -1;
else
- clock->leap = 0;
+ node->leap = 0;
}
msg_put(msg);
return 1;
}

-static void close_pmc(struct clock *clock)
+static void close_pmc(struct node *node)
{
- pmc_destroy(clock->pmc);
- clock->pmc = NULL;
+ pmc_destroy(node->pmc);
+ node->pmc = NULL;
}

/* Returns: -1 in case of error, 0 for normal sync, 1 to leap clock */
-static int update_sync_offset(struct clock *clock)
+static int update_sync_offset(struct node *node)
{
struct timespec tp;
uint64_t ts;
@@ -523,40 +530,41 @@ static int update_sync_offset(struct clock *clock)
}
ts = tp.tv_sec * NS_PER_SEC + tp.tv_nsec;

- if (clock->pmc &&
- !(ts > clock->pmc_last_update &&
- ts - clock->pmc_last_update < PMC_UPDATE_INTERVAL)) {
- if (run_pmc_get_utc_offset(clock, 0) > 0)
- clock->pmc_last_update = ts;
+ if (node->pmc &&
+ !(ts > node->pmc_last_update &&
+ ts - node->pmc_last_update < PMC_UPDATE_INTERVAL)) {
+ if (run_pmc_get_utc_offset(node, 0) > 0)
+ node->pmc_last_update = ts;
}

/* Handle leap seconds. */

- if (!clock->leap && !clock->leap_set)
+ if (!node->leap && !node->leap_set)
return 0;

- clock_leap = leap_second_status(ts, clock->leap_set,
- &clock->leap, &clock->sync_offset);
- if (clock->leap_set != clock_leap) {
- clock->leap_set = clock_leap;
+ clock_leap = leap_second_status(ts, node->leap_set,
+ &node->leap, &node->sync_offset);
+ if (node->leap_set != clock_leap) {
+ node->leap_set = clock_leap;
return 1;
}
return 0;
}

/* Returns: non-zero to skip clock update */
-static int clock_handle_leap(struct clock *clock, clockid_t src,
+static int clock_handle_leap(struct node *node, struct clock *clock,
int64_t offset, uint64_t ts, int do_leap)
{
- if (!clock->leap && !do_leap)
+ if (!node->leap && !do_leap)
return 0;

- if (clock->clkid != CLOCK_REALTIME && src != CLOCK_REALTIME)
+ if (clock->clkid != CLOCK_REALTIME &&
+ node->master->clkid != CLOCK_REALTIME)
return 0;

/* If the system clock is the master clock, get a time stamp from
it, as it is the clock which will include the leap second. */
- if (src == CLOCK_REALTIME) {
+ if (node->master->clkid == CLOCK_REALTIME) {
struct timespec tp;
if (clock_gettime(CLOCK_REALTIME, &tp)) {
pr_err("failed to read clock: %m");
@@ -569,8 +577,8 @@ static int clock_handle_leap(struct clock *clock, clockid_t src,
target time. Ignore possible 1 second error in UTC offset. */
if (clock->clkid == CLOCK_REALTIME &&
clock->servo_state == SERVO_UNLOCKED) {
- ts -= offset + clock->sync_offset * NS_PER_SEC *
- clock->sync_offset_direction;
+ ts -= offset + node->sync_offset * NS_PER_SEC *
+ node->sync_offset_direction;
}

/* Suspend clock updates in the last second before midnight. */
@@ -581,13 +589,79 @@ static int clock_handle_leap(struct clock *clock, clockid_t src,

if (do_leap) {
/* Only the system clock can leap. */
- if (clock->clkid == CLOCK_REALTIME && clock->kernel_leap)
- sysclk_set_leap(clock->leap_set);
+ if (clock->clkid == CLOCK_REALTIME && node->kernel_leap)
+ sysclk_set_leap(node->leap_set);
}

return 0;
}

+static int clock_add(struct node *node, clockid_t clkid)
+{
+ struct clock *c;
+ int max_ppb;
+ double ppb;
+
+ c = calloc(1, sizeof(*c));
+ if (!c) {
+ pr_err("failed to allocate memory for a clock");
+ return -1;
+ }
+ c->clkid = clkid;
+ c->servo_state = SERVO_UNLOCKED;
+
+ if (c->clkid == CLOCK_REALTIME)
+ c->source_label = "sys";
+ else
+ c->source_label = "phc";
+
+ if (node->stats_max_count > 0) {
+ c->offset_stats = stats_create();
+ c->freq_stats = stats_create();
+ c->delay_stats = stats_create();
+ if (!c->offset_stats ||
+ !c->freq_stats ||
+ !c->delay_stats) {
+ pr_err("failed to create stats");
+ return -1;
+ }
+ }
+ if (node->sanity_freq_limit) {
+ c->sanity_check = clockcheck_create(node->sanity_freq_limit);
+ if (!c->sanity_check) {
+ pr_err("failed to create clock check");
+ return -1;
+ }
+ }
+
+ clockadj_init(c->clkid);
+ ppb = clockadj_get_freq(c->clkid);
+ /* The reading may silently fail and return 0, reset the frequency to
+ make sure ppb is the actual frequency of the clock. */
+ clockadj_set_freq(c->clkid, ppb);
+ if (c->clkid == CLOCK_REALTIME) {
+ sysclk_set_leap(0);
+ max_ppb = sysclk_max_freq();
+ } else {
+ max_ppb = phc_max_adj(c->clkid);
+ if (!max_ppb) {
+ pr_err("clock is not adjustable");
+ return -1;
+ }
+ }
+
+ c->servo = servo_create(node->servo_type, -ppb, max_ppb, 0);
+ servo_sync_interval(c->servo, node->phc_interval);
+
+ if (clkid != CLOCK_REALTIME)
+ c->sysoff_supported = (SYSOFF_SUPPORTED ==
+ sysoff_probe(CLOCKID_TO_FD(clkid),
+ node->phc_readings));
+
+ LIST_INSERT_HEAD(&node->clocks, c, list);
+ return 0;
+}
+
static void usage(char *progname)
{
fprintf(stderr,
@@ -622,16 +696,16 @@ int main(int argc, char *argv[])
{
char *progname;
clockid_t src = CLOCK_INVALID;
- int c, domain_number = 0, phc_readings = 5, pps_fd = -1;
- int max_ppb, r, wait_sync = 0, forced_sync_offset = 0;
+ clockid_t dst = CLOCK_REALTIME;
+ int c, domain_number = 0, pps_fd = -1;
+ int r, wait_sync = 0, forced_sync_offset = 0;
int print_level = LOG_INFO, use_syslog = 1, verbose = 0;
- int sanity_freq_limit = 200000000;
- enum servo_type servo = CLOCK_SERVO_PI;
- double ppb, phc_interval = 1.0, phc_rate;
- struct timespec phc_interval_tp;
- struct clock dst_clock = {
- .clkid = CLOCK_REALTIME,
- .servo_state = SERVO_UNLOCKED,
+ double phc_rate;
+ struct node node = {
+ .sanity_freq_limit = 200000000,
+ .servo_type = CLOCK_SERVO_PI,
+ .phc_readings = 5,
+ .phc_interval = 1.0,
.kernel_leap = 1,
};

@@ -645,7 +719,7 @@ int main(int argc, char *argv[])
"c:d:s:E:P:I:S:F:R:N:O:L:i:u:wn:xl:mqvh"))) {
switch (c) {
case 'c':
- dst_clock.clkid = clock_open(optarg);
+ dst = clock_open(optarg);
break;
case 'd':
pps_fd = open(optarg, O_RDONLY);
@@ -663,9 +737,9 @@ int main(int argc, char *argv[])
break;
case 'E':
if (!strcasecmp(optarg, "pi")) {
- servo = CLOCK_SERVO_PI;
+ node.servo_type = CLOCK_SERVO_PI;
} else if (!strcasecmp(optarg, "linreg")) {
- servo = CLOCK_SERVO_LINREG;
+ node.servo_type = CLOCK_SERVO_LINREG;
} else {
fprintf(stderr,
"invalid servo name %s\n", optarg);
@@ -695,25 +769,25 @@ int main(int argc, char *argv[])
case 'R':
if (get_arg_val_d(c, optarg, &phc_rate, 1e-9, DBL_MAX))
return -1;
- phc_interval = 1.0 / phc_rate;
+ node.phc_interval = 1.0 / phc_rate;
break;
case 'N':
- if (get_arg_val_i(c, optarg, &phc_readings, 1, INT_MAX))
+ if (get_arg_val_i(c, optarg, &node.phc_readings, 1, INT_MAX))
return -1;
break;
case 'O':
- if (get_arg_val_i(c, optarg, &dst_clock.sync_offset,
+ if (get_arg_val_i(c, optarg, &node.sync_offset,
INT_MIN, INT_MAX))
return -1;
- dst_clock.sync_offset_direction = -1;
+ node.sync_offset_direction = -1;
forced_sync_offset = 1;
break;
case 'L':
- if (get_arg_val_i(c, optarg, &sanity_freq_limit, 0, INT_MAX))
+ if (get_arg_val_i(c, optarg, &node.sanity_freq_limit, 0, INT_MAX))
return -1;
break;
case 'u':
- if (get_arg_val_ui(c, optarg, &dst_clock.stats_max_count,
+ if (get_arg_val_ui(c, optarg, &node.stats_max_count,
0, UINT_MAX))
return -1;
break;
@@ -725,7 +799,7 @@ int main(int argc, char *argv[])
return -1;
break;
case 'x':
- dst_clock.kernel_leap = 0;
+ node.kernel_leap = 0;
break;
case 'l':
if (get_arg_val_i(c, optarg, &print_level,
@@ -755,13 +829,13 @@ int main(int argc, char *argv[])
goto bad_usage;
}

- if (dst_clock.clkid == CLOCK_INVALID) {
+ if (dst == CLOCK_INVALID) {
fprintf(stderr,
"valid destination clock must be selected.\n");
goto bad_usage;
}

- if (pps_fd >= 0 && dst_clock.clkid != CLOCK_REALTIME) {
+ if (pps_fd >= 0 && dst != CLOCK_REALTIME) {
fprintf(stderr,
"cannot use a pps device unless destination is CLOCK_REALTIME\n");
goto bad_usage;
@@ -773,36 +847,21 @@ int main(int argc, char *argv[])
goto bad_usage;
}

- if (dst_clock.stats_max_count > 0) {
- dst_clock.offset_stats = stats_create();
- dst_clock.freq_stats = stats_create();
- dst_clock.delay_stats = stats_create();
- if (!dst_clock.offset_stats ||
- !dst_clock.freq_stats ||
- !dst_clock.delay_stats) {
- fprintf(stderr, "failed to create stats");
- return -1;
- }
- }
- if (sanity_freq_limit) {
- dst_clock.sanity_check = clockcheck_create(sanity_freq_limit);
- if (!dst_clock.sanity_check) {
- fprintf(stderr, "failed to create clock check");
- return -1;
- }
- }
-
print_set_progname(progname);
print_set_verbose(verbose);
print_set_syslog(use_syslog);
print_set_level(print_level);

+ clock_add(&node, src);
+ node.master = LIST_FIRST(&node.clocks);
+ clock_add(&node, dst);
+
if (wait_sync) {
- if (init_pmc(&dst_clock, domain_number))
+ if (init_pmc(&node, domain_number))
return -1;

while (1) {
- r = run_pmc_wait_sync(&dst_clock, 1000);
+ r = run_pmc_wait_sync(&node, 1000);
if (r < 0)
return -1;
if (r > 0)
@@ -812,60 +871,40 @@ int main(int argc, char *argv[])
}

if (!forced_sync_offset) {
- r = run_pmc_get_utc_offset(&dst_clock, 1000);
+ r = run_pmc_get_utc_offset(&node, 1000);
if (r <= 0) {
pr_err("failed to get UTC offset");
return -1;
}

if (src != CLOCK_REALTIME &&
- dst_clock.clkid == CLOCK_REALTIME)
- dst_clock.sync_offset_direction = 1;
+ dst == CLOCK_REALTIME)
+ node.sync_offset_direction = 1;
else if (src == CLOCK_REALTIME &&
- dst_clock.clkid != CLOCK_REALTIME)
- dst_clock.sync_offset_direction = -1;
+ dst != CLOCK_REALTIME)
+ node.sync_offset_direction = -1;
else
- dst_clock.sync_offset_direction = 0;
+ node.sync_offset_direction = 0;
}

- if (forced_sync_offset || !dst_clock.sync_offset_direction)
- close_pmc(&dst_clock);
+ if (forced_sync_offset || !node.sync_offset_direction)
+ close_pmc(&node);
}

- clockadj_init(dst_clock.clkid);
- ppb = clockadj_get_freq(dst_clock.clkid);
- /* The reading may silently fail and return 0, reset the frequency to
- make sure ppb is the actual frequency of the clock. */
- clockadj_set_freq(dst_clock.clkid, ppb);
- if (dst_clock.clkid == CLOCK_REALTIME) {
- sysclk_set_leap(0);
- max_ppb = sysclk_max_freq();
- } else {
- max_ppb = phc_max_adj(dst_clock.clkid);
- if (!max_ppb) {
- pr_err("clock is not adjustable");
- return -1;
- }
- }
-
- dst_clock.servo = servo_create(servo, -ppb, max_ppb, 0);
-
if (pps_fd >= 0) {
- servo_sync_interval(dst_clock.servo, 1.0);
- return do_pps_loop(&dst_clock, pps_fd, src, phc_readings);
- }
-
- servo_sync_interval(dst_clock.servo, phc_interval);
+ /* only one destination clock allowed with PPS until we
+ * implement a mean to specify PTP port to PPS mapping */
+ struct clock *dst_clock;

- phc_interval_tp.tv_sec = phc_interval;
- phc_interval_tp.tv_nsec = (phc_interval - phc_interval_tp.tv_sec) * 1e9;
-
- if (dst_clock.clkid == CLOCK_REALTIME && src != CLOCK_REALTIME &&
- SYSOFF_SUPPORTED == sysoff_probe(CLOCKID_TO_FD(src), phc_readings))
- return do_sysoff_loop(&dst_clock, src, &phc_interval_tp,
- phc_readings);
+ LIST_FOREACH(dst_clock, &node.clocks, list) {
+ if (dst_clock != node.master)
+ break;
+ }
+ servo_sync_interval(dst_clock->servo, 1.0);
+ return do_pps_loop(&node, dst_clock, pps_fd);
+ }

- return do_phc_loop(&dst_clock, src, &phc_interval_tp, phc_readings);
+ return do_loop(&node);

bad_usage:
usage(progname);
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:36 UTC
Permalink
Add tracking of which ports have been added and to which clock they belong.

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

diff --git a/phc2sys.c b/phc2sys.c
index 62e9b8c19e17..ece4560e0c67 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -75,6 +75,12 @@ struct clock {
struct clockcheck *sanity_check;
};

+struct port {
+ LIST_ENTRY(port) list;
+ unsigned int number;
+ struct clock *clock;
+};
+
struct node {
unsigned int stats_max_count;
int sanity_freq_limit;
@@ -89,6 +95,7 @@ struct node {
struct pmc *pmc;
int pmc_ds_requested;
uint64_t pmc_last_update;
+ LIST_HEAD(port_head, port) ports;
LIST_HEAD(clock_head, clock) clocks;
struct clock *master;
};
@@ -206,6 +213,50 @@ static struct clock *clock_add(struct node *node, char *device)
return c;
}

+static struct port *port_get(struct node *node, unsigned int number)
+{
+ struct port *p;
+
+ LIST_FOREACH(p, &node->ports, list) {
+ if (p->number == number)
+ return p;
+ }
+ return NULL;
+}
+
+static struct port *port_add(struct node *node, unsigned int number,
+ char *device)
+{
+ struct port *p;
+ struct clock *c = NULL, *tmp;
+
+ p = port_get(node, number);
+ if (p)
+ return p;
+ /* port is a new one, look whether we have the device already on
+ * a different port */
+ LIST_FOREACH(tmp, &node->clocks, list) {
+ if (!strcmp(tmp->device, device)) {
+ c = tmp;
+ break;
+ }
+ }
+ if (!c) {
+ c = clock_add(node, device);
+ if (!c)
+ return NULL;
+ }
+ p = malloc(sizeof(*p));
+ if (!p) {
+ pr_err("failed to allocate memory for a port");
+ return NULL;
+ }
+ p->number = number;
+ p->clock = c;
+ LIST_INSERT_HEAD(&node->ports, p, list);
+ return p;
+}
+
static int read_phc(clockid_t clkid, clockid_t sysclk, int readings,
int64_t *offset, uint64_t *ts, int64_t *delay)
{
--
1.7.6.5
Jiri Benc
2014-03-24 08:53:43 UTC
Permalink
Require subscriptions to be renewed regularly. This way, the subscription
automatically times out when phc2sys is killed.

Signed-off-by: Jiri Benc <***@redhat.com>
---
clock.c | 24 ++++++++++++++++++++++++
phc2sys.c | 10 ++++++----
2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/clock.c b/clock.c
index c893a58391ce..d9ee6c4c77e2 100644
--- a/clock.c
+++ b/clock.c
@@ -68,6 +68,7 @@ struct clock_subscriber {
uint8_t addr[TRANSPORT_RECV_ADDR_LEN];
int addrlen;
UInteger16 sequenceId;
+ time_t subscribed;
};

struct clock {
@@ -142,6 +143,7 @@ static void clock_update_subscription(struct clock *c, struct ptp_message *req,
uint8_t addr[TRANSPORT_RECV_ADDR_LEN];
int addrlen;
struct clock_subscriber *s;
+ struct timespec now;

addrlen = port_recv_addr(uds, addr);
if (!addrlen) {
@@ -157,6 +159,8 @@ static void clock_update_subscription(struct clock *c, struct ptp_message *req,
memcpy(s->addr, addr, addrlen);
s->addrlen = addrlen;
s->events = events;
+ clock_gettime(CLOCK_MONOTONIC, &now);
+ s->subscribed = now.tv_sec;
} else {
remove_subscriber(s);
}
@@ -175,6 +179,8 @@ static void clock_update_subscription(struct clock *c, struct ptp_message *req,
memcpy(s->addr, addr, addrlen);
s->addrlen = addrlen;
s->events = events;
+ clock_gettime(CLOCK_MONOTONIC, &now);
+ s->subscribed = now.tv_sec;
s->sequenceId = 0;
LIST_INSERT_HEAD(&c->subscribers, s, list);
}
@@ -188,6 +194,23 @@ static void clock_flush_subscriptions(struct clock *c)
}
}

+static void clock_prune_subscriptions(struct clock *c)
+{
+ struct clock_subscriber *s, *tmp;
+ struct timespec now;
+
+ clock_gettime(CLOCK_MONOTONIC, &now);
+ LIST_FOREACH_SAFE(s, &c->subscribers, list, tmp) {
+ /* prune subscribers older than 3 minutes */
+ if (s->subscribed + 180 < now.tv_sec) {
+ pr_info("subscriber %s timed out (not renewed in %ld seconds)",
+ pid2str(&s->targetPortIdentity),
+ now.tv_sec - s->subscribed);
+ remove_subscriber(s);
+ }
+ }
+}
+
void clock_send_notification(struct clock *c, struct ptp_message *msg,
int msglen, enum notification event)
{
@@ -1165,6 +1188,7 @@ int clock_poll(struct clock *c)
if (sde)
handle_state_decision_event(c);

+ clock_prune_subscriptions(c);
return 0;
}

diff --git a/phc2sys.c b/phc2sys.c
index 108a03f83b8b..88d14178b948 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -109,7 +109,7 @@ struct node {
struct clock *master;
};

-static int update_sync_offset(struct node *node);
+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, int do_leap);
static int run_pmc_get_utc_offset(struct node *node, int timeout);
@@ -544,7 +544,7 @@ static int do_pps_loop(struct node *node, struct clock *clock, int fd)
pps_offset = pps_ts - phc_ts;
}

- do_leap = update_sync_offset(node);
+ do_leap = update_pmc(node, 0);
if (do_leap < 0)
continue;
update_clock(node, clock, pps_offset, pps_ts, -1, do_leap);
@@ -566,7 +566,7 @@ static int do_loop(struct node *node, int subscriptions)

while (1) {
clock_nanosleep(CLOCK_MONOTONIC, 0, &interval, NULL);
- do_leap = update_sync_offset(node);
+ do_leap = update_pmc(node, subscriptions);
if (do_leap < 0)
continue;

@@ -1007,7 +1007,7 @@ out:
}

/* Returns: -1 in case of error, 0 for normal sync, 1 to leap clock */
-static int update_sync_offset(struct node *node)
+static int update_pmc(struct node *node, int subscribe)
{
struct timespec tp;
uint64_t ts;
@@ -1022,6 +1022,8 @@ static int update_sync_offset(struct node *node)
if (node->pmc &&
!(ts > node->pmc_last_update &&
ts - node->pmc_last_update < PMC_UPDATE_INTERVAL)) {
+ if (subscribe)
+ run_pmc_subscribe(node, 0);
if (run_pmc_get_utc_offset(node, 0) > 0)
node->pmc_last_update = ts;
}
--
1.7.6.5
Richard Cochran
2014-04-05 11:13:04 UTC
Permalink
Post by Jiri Benc
Require subscriptions to be renewed regularly. This way, the subscription
automatically times out when phc2sys is killed.
---
clock.c | 24 ++++++++++++++++++++++++
phc2sys.c | 10 ++++++----
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/clock.c b/clock.c
index c893a58391ce..d9ee6c4c77e2 100644
--- a/clock.c
+++ b/clock.c
@@ -68,6 +68,7 @@ struct clock_subscriber {
uint8_t addr[TRANSPORT_RECV_ADDR_LEN];
int addrlen;
UInteger16 sequenceId;
+ time_t subscribed;
Make this "expiration" instead, and then we can implement 1588 unicast
in the same way...
Post by Jiri Benc
};
struct clock {
@@ -142,6 +143,7 @@ static void clock_update_subscription(struct clock *c, struct ptp_message *req,
uint8_t addr[TRANSPORT_RECV_ADDR_LEN];
int addrlen;
struct clock_subscriber *s;
+ struct timespec now;
addrlen = port_recv_addr(uds, addr);
if (!addrlen) {
@@ -157,6 +159,8 @@ static void clock_update_subscription(struct clock *c, struct ptp_message *req,
memcpy(s->addr, addr, addrlen);
s->addrlen = addrlen;
s->events = events;
+ clock_gettime(CLOCK_MONOTONIC, &now);
+ s->subscribed = now.tv_sec;
} else {
remove_subscriber(s);
}
@@ -175,6 +179,8 @@ static void clock_update_subscription(struct clock *c, struct ptp_message *req,
memcpy(s->addr, addr, addrlen);
s->addrlen = addrlen;
s->events = events;
+ clock_gettime(CLOCK_MONOTONIC, &now);
+ s->subscribed = now.tv_sec;
s->sequenceId = 0;
LIST_INSERT_HEAD(&c->subscribers, s, list);
}
@@ -188,6 +194,23 @@ static void clock_flush_subscriptions(struct clock *c)
}
}
+static void clock_prune_subscriptions(struct clock *c)
+{
+ struct clock_subscriber *s, *tmp;
+ struct timespec now;
+
+ clock_gettime(CLOCK_MONOTONIC, &now);
+ LIST_FOREACH_SAFE(s, &c->subscribers, list, tmp) {
+ /* prune subscribers older than 3 minutes */
+ if (s->subscribed + 180 < now.tv_sec) {
and you won't need a hard coded magic number here.
Post by Jiri Benc
+ pr_info("subscriber %s timed out (not renewed in %ld seconds)",
+ pid2str(&s->targetPortIdentity),
+ now.tv_sec - s->subscribed);
+ remove_subscriber(s);
+ }
+ }
+}
+
Thanks,
Richard
Richard Cochran
2014-04-05 11:14:51 UTC
Permalink
Post by Jiri Benc
Require subscriptions to be renewed regularly. This way, the subscription
automatically times out when phc2sys is killed.
Can't this patch go before all of the phc2sys stuff?

Thanks,
Richard
Jiri Benc
2014-03-24 08:53:44 UTC
Permalink
Signed-off-by: Jiri Benc <***@redhat.com>
---
phc2sys.8 | 115 ++++++++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 79 insertions(+), 36 deletions(-)

diff --git a/phc2sys.8 b/phc2sys.8
index fa3ae206f3df..37a39d0ba60f 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -1,12 +1,17 @@
.TH PHC2SYS 8 "November 2012" "linuxptp"
.SH NAME
-phc2sys \- synchronize two clocks
+phc2sys \- synchronize two or more clocks

.SH SYNOPSIS
-.B phc2sys
+.B phc2sys \-a
[
-.B \-wmqvx
+.B \-r
] [
+.B \-r
+] [ options ]
+.br
+.B phc2sys
+[
.BI \-d " pps-device"
] [
.BI \-s " device"
@@ -15,45 +20,53 @@ phc2sys \- synchronize two clocks
] [
.BI \-O " offset"
] [
-.BI \-E " servo"
-] [
-.BI \-P " kp"
-] [
-.BI \-I " ki"
-] [
-.BI \-S " step"
-] [
-.BI \-F " step"
-] [
-.BI \-R " update-rate"
-] [
-.BI \-N " clock-readings"
-] [
-.BI \-L " freq-limit"
-] [
-.BI \-u " summary-updates"
-] [
-.BI \-n " domain-number"
-] [
-.BI \-l " print-level"
-]
+.BI \-w
+] [ options ]

.SH DESCRIPTION
.B phc2sys
-is a program which synchronizes two clocks in the system. Typically, it is used
-to synchronize the system clock to a PTP hardware clock (PHC), which itself is
-synchronized by the
+is a program which synchronizes two or more clocks in the system. Typically,
+it is used to synchronize the system clock to a PTP hardware clock (PHC),
+which itself is synchronized by the
.BR ptp4l (8)
program.

-Two synchronization modes are supported, one uses a pulse per second (PPS)
+With the
+.B \-a
+option, the clocks to synchronize are fetched from the running
+.B ptp4l
+daemon and the direction of synchronization automatically follows changes of
+the PTP port states.
+
+Manual configuration is also possible. When using manual configuration, two
+synchronization modes are supported, one uses a pulse per second (PPS)
signal provided by the source clock and the other mode reads time from the
source clock directly. Some clocks can be used in both modes, the mode which
-will synchronize the slave clock with better accuracy depends on hardware and
-driver implementation.
+will synchronize the slave clock with better accuracy depends on hardware
+and driver implementation.

.SH OPTIONS
.TP
+.BI \-a
+Read the clocks to synchronize from running
+.B ptp4l
+and follow changes in the port states, adjusting the synchronization
+direction automatically. The system clock (CLOCK_REALTIME) is not
+synchronized, unless the
+.B \-r
+option is also specified.
+.TP
+.BI \-r
+Only valid together with the
+.B \-a
+option. Instructs
+.B phc2sys
+to also synchronize the system clock (CLOCK_REALTIME). By default, the
+system clock is not considered as a possible time source. If you want the
+system clock to be eligible to become a time source, specify the
+.B \-r
+option twice.
+.TP
.BI \-d " pps-device"
Specify the PPS device of the master clock (e.g. /dev/pps0). With this option
the PPS synchronization mode is used instead of the direct mode. As the PPS
@@ -68,7 +81,10 @@ option the PPS signal of the master clock is enabled automatically, otherwise
it has to be enabled before
.B phc2sys
is started (e.g. by running \f(CWecho 1 > /sys/class/ptp/ptp0/pps_enable\fP).
-This option can be used only with the system clock as the slave clock.
+This option can be used only with the system clock as the slave clock. Not
+compatible with the
+.B \-a
+option.
.TP
.BI \-s " device"
Specify the master clock by device (e.g. /dev/ptp0) or interface (e.g. eth0) or
@@ -76,7 +92,9 @@ by name (e.g. CLOCK_REALTIME for the system clock). When this option is used
together with the
.B \-d
option, the master clock is used only to correct the offset by whole number of
-seconds, which cannot be fixed with PPS alone.
+seconds, which cannot be fixed with PPS alone. Not compatible with the
+.B \-a
+option.
.TP
.BI \-i " interface"
Performs the exact same function as
@@ -89,7 +107,10 @@ should no longer be used.
.TP
.BI \-c " device"
Specify the slave clock by device (e.g. /dev/ptp1) or interface (e.g. eth1) or
-by name. The default is CLOCK_REALTIME (the system clock).
+by name. The default is CLOCK_REALTIME (the system clock). Not compatible
+with the
+.B \-a
+option.
.TP
.BI \-E " servo"
Specify which clock servo should be used. Valid values are pi for a PI
@@ -128,7 +149,10 @@ minimize the error caused by random delays in scheduling and bus utilization.
The default is 5.
.TP
.BI \-O " offset"
-Specify the offset between the slave and master times in seconds. See
+Specify the offset between the slave and master times in seconds. Not
+compatible with the
+.B \-a
+option. See
.SM
.B TIME SCALE USAGE
below.
@@ -154,7 +178,9 @@ Wait until ptp4l is in a synchronized state. If the
.B \-O
option is not used, also keep the offset between the slave and master
times updated according to the currentUtcOffset value obtained from ptp4l and
-the direction of the clock synchronization.
+the direction of the clock synchronization. Not compatible with the
+.B \-a
+option.
.TP
.BI \-n " domain-number"
Specify the domain number used by ptp4l. The default is 0.
@@ -198,6 +224,8 @@ clock follows UTC. Time offset between these two is maintained by

.B Phc2sys
acquires the offset value either by reading it from ptp4l when
+.B \-a
+or
.B \-w
is in effect or from command line when
.B \-O
@@ -208,6 +236,21 @@ master.

.SH EXAMPLES

+Synchronize time automatically according to the current
+.B ptp4l
+state, synchronize the system clock to the remote master.
+
+.RS
+\f(CWphc2sys \-a \-r\fP
+.RE
+
+Same as above, but when the host becomes the domain master, synchronize time
+in the domain to its system clock.
+
+.RS
+\f(CWphc2sys \-a \-rr\fP
+.RE
+
The host is a domain master, PTP clock is synchronized to system clock and the
time offset is obtained from
.BR ptp4l .
--
1.7.6.5
Jiri Benc
2014-03-31 13:14:11 UTC
Permalink
Post by Jiri Benc
v2: rebased to the current HEAD
Has anybody have a chance to review this patchset? I'd appreciate any
feedback.

Thanks,

Jiri
--
Jiri Benc
Richard Cochran
2014-03-31 14:41:58 UTC
Permalink
Post by Jiri Benc
Has anybody have a chance to review this patchset? I'd appreciate any
feedback.
Not yet, been super busy with other stuff.

Sorry,
Richard
Miroslav Lichvar
2014-04-03 09:53:40 UTC
Permalink
Post by Jiri Benc
Post by Jiri Benc
v2: rebased to the current HEAD
Has anybody have a chance to review this patchset? I'd appreciate any
feedback.
In my testing with an ordinary PTP clock and system clock this works
well. I had a brief look at the code and didn't see any problems.

As a future improvement, I think it would be nice if ptp4l sent a
notification when exiting so phc2sys could stop its synchronization
and wait until ptp4l is running again.
--
Miroslav Lichvar
Jiri Benc
2014-06-19 14:18:55 UTC
Permalink
Post by Miroslav Lichvar
As a future improvement, I think it would be nice if ptp4l sent a
notification when exiting so phc2sys could stop its synchronization
and wait until ptp4l is running again.
I was thinking about how to implement this. In particular, I'd like
this to be more robust and detect also ptp4l being killed.

The most obvious option, detecting whether other side closed its part
of the unix socket, would require changing it from SOCK_DGRAM to
SOCK_SEQPACKET. Which means ptp4l would have to keep track of all
uds clients, deal with accept calls, etc., and most importantly, poll
these additional file descriptors. That would require quite substantial
changes throughout the whole code. But then, uds is treated specially
in most places, perhaps stopping pretending it's another transport
wouldn't be bad.

Other option is polling from phc2sys every phc_interval. That's ugly,
consuming unnecessary resources and unreliable anyway.

Better option would be asking ptp4l about its PID and monitoring it.
That would be reliable enough but still polling, although rather cheap
polling.

Thoughts? Different ideas?

Thanks,

Jiri
--
Jiri Benc
Keller, Jacob E
2014-06-19 16:25:39 UTC
Permalink
Post by Jiri Benc
Post by Miroslav Lichvar
As a future improvement, I think it would be nice if ptp4l sent a
notification when exiting so phc2sys could stop its synchronization
and wait until ptp4l is running again.
I was thinking about how to implement this. In particular, I'd like
this to be more robust and detect also ptp4l being killed.
The most obvious option, detecting whether other side closed its part
of the unix socket, would require changing it from SOCK_DGRAM to
SOCK_SEQPACKET. Which means ptp4l would have to keep track of all
uds clients, deal with accept calls, etc., and most importantly, poll
these additional file descriptors. That would require quite substantial
changes throughout the whole code. But then, uds is treated specially
in most places, perhaps stopping pretending it's another transport
wouldn't be bad.
That seems like a whole lot of work, though it does sound like the most
robust solution. Especially difficult since we would be un-abstracting
uds as a transport type..
Post by Jiri Benc
Other option is polling from phc2sys every phc_interval. That's ugly,
consuming unnecessary resources and unreliable anyway.
This is probably not good.
Post by Jiri Benc
Better option would be asking ptp4l about its PID and monitoring it.
That would be reliable enough but still polling, although rather cheap
polling.
This is most likely the best compromise, and easiest to implement..

Thanks,
Jake
Post by Jiri Benc
Thoughts? Different ideas?
Stephan Gatzka
2014-06-19 21:01:36 UTC
Permalink
Post by Jiri Benc
Better option would be asking ptp4l about its PID and monitoring it.
That would be reliable enough but still polling, although rather cheap
polling.
Is ptp4l writing a PID file? If so, you can use inotify for monitoring.

Regards,

Stephan
Jiri Benc
2014-06-20 07:32:51 UTC
Permalink
Post by Stephan Gatzka
Post by Jiri Benc
Better option would be asking ptp4l about its PID and monitoring it.
That would be reliable enough but still polling, although rather cheap
polling.
Is ptp4l writing a PID file? If so, you can use inotify for monitoring.
The problem is not detecting ptp4l start, that we can easily do using
management messages. The problem is ptp4l going away, and inotify
cannot help with that.

Moreover, pid file won't work with multiple domains, unless you ask
ptp4l about the pid file path, and then you can just ask about the pid
directly.

Jiri
--
Jiri Benc
Miroslav Lichvar
2014-06-20 07:52:49 UTC
Permalink
Post by Jiri Benc
Post by Miroslav Lichvar
As a future improvement, I think it would be nice if ptp4l sent a
notification when exiting so phc2sys could stop its synchronization
and wait until ptp4l is running again.
I was thinking about how to implement this. In particular, I'd like
this to be more robust and detect also ptp4l being killed.
I think a combination of ptp4l sending notification on exit and
phc2sys timing out when no reply is received in the pmc update
interval (1 minute) would be good enough.

Switching to the SEQPACKET socket seems like a lot of work and it
wouldn't detect unresponsive ptp4l, which might be a useful thing to
have (the same applies to the process monitoring).
--
Miroslav Lichvar
Jiri Benc
2014-06-20 08:13:06 UTC
Permalink
Post by Miroslav Lichvar
I think a combination of ptp4l sending notification on exit and
phc2sys timing out when no reply is received in the pmc update
interval (1 minute) would be good enough.
Except when ptp4l is killed/crashes and is restarted. That's something
that wouldn't be caught by this and would create quite a messy
situation, especially when it's restarted automatically by system
startup scripts (e.g. systemd).
Post by Miroslav Lichvar
Switching to the SEQPACKET socket seems like a lot of work and it
wouldn't detect unresponsive ptp4l, which might be a useful thing to
have (the same applies to the process monitoring).
I don't think that unresponsive ptp4l is a likely state. How could this
happen, sans a horrible bug?

Jiri
--
Jiri Benc
Miroslav Lichvar
2014-06-20 08:49:33 UTC
Permalink
Post by Jiri Benc
Post by Miroslav Lichvar
I think a combination of ptp4l sending notification on exit and
phc2sys timing out when no reply is received in the pmc update
interval (1 minute) would be good enough.
Except when ptp4l is killed/crashes and is restarted. That's something
that wouldn't be caught by this and would create quite a messy
situation, especially when it's restarted automatically by system
startup scripts (e.g. systemd).
Well, I wouldn't expect ptp4l to crash or be killed (SIGKILL) so
often that it needs such special care. A restart could be detected by
adding a random ID of the running ptp4l instance to the messages.
Post by Jiri Benc
Post by Miroslav Lichvar
Switching to the SEQPACKET socket seems like a lot of work and it
wouldn't detect unresponsive ptp4l, which might be a useful thing to
have (the same applies to the process monitoring).
I don't think that unresponsive ptp4l is a likely state. How could this
happen, sans a horrible bug?
Yes, a bug. Although it might be less likely than ptp4l crashing.
--
Miroslav Lichvar
Richard Cochran
2014-06-22 06:49:32 UTC
Permalink
Post by Jiri Benc
Post by Miroslav Lichvar
As a future improvement, I think it would be nice if ptp4l sent a
notification when exiting so phc2sys could stop its synchronization
and wait until ptp4l is running again.
I was thinking about how to implement this. In particular, I'd like
this to be more robust and detect also ptp4l being killed.
...
Post by Jiri Benc
Thoughts? Different ideas?
Really, I don't see the need for any changes in this area. Does ptp4l
have a problem with hanging or halting? I don't think so, but if it
does, then let us fix that.

Any script (or monolithic system control monstrosity) should start and
stop ptp4l and phc2sys (and ntp) together. These are single threaded
programs that never fork, and so keeping track of PIDs is trivial. How
you run these programs depends on the use case within a larger
network, and so the overall control decisions should be made at the
init script level.

If the goal is to be able to keep phc2sys running while restarting
ptp4l, then I think it better to have phc2sys accept a "holdover"
signal (using SIGUSR1 for example). During holdover, phc2sys knows not
to bother with ptp4l.

Thanks,
Richard
Richard Cochran
2014-04-05 10:02:37 UTC
Permalink
Post by Jiri Benc
Has anybody have a chance to review this patchset? I'd appreciate any
feedback.
I now have some cycles to work this in. It is a little too long of a
series for me to merge all at once. I would like to focus first on
patches 1-12. Please limit yourself to those next time you post (two
of them are already in).

Basically this is looking pretty good, and I think we should have it
all in, but there are a couple of points I would like to have done
differently.

Thanks,
Richard
Loading...