[Linuxptp-devel] [PATCH 2/3] Updated the Best Master Clock Algorithm (BMCA) for 802.1AS devices.

[Erik Hons]

These changes are:
- Removed UNCALIBRATED and PRE_MASTER port states.
- Short circuited foreign master qualification mechanism.
- Correctly identify if the local PTP clock is the domain Grand Master.
- port: Reduce the foreign master threshold for 802.1AS

Updated the function that determines if the local clock is the domain
Grand Master by comparing the local clock identify to the GM identity
of the parent data set.

IEEE 802.1AS-2011 removed the notion of a foreign master and associated
qualification metrics defined by IEEE 1588. This was done the intent of
decreasing the response time of the BMCA in selecting a new master. When a
new announce message is received, the 802.1AS BMCA is act on the new
information immediately. Reducing the foreign master threshold to a message
of one has the same effect.

Signed-off-by: Rodney Greenstreet <***@ni.com>
---
clock.c | 9 +++++----
foreign.h | 3 ++-
fsm.c | 25 +++++++++++++++++++++++++
fsm.h | 1 +
port.c | 33 +++++++++++++++++++++++++++------
5 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/clock.c b/clock.c
index 0b8fb56..d1cf40b 100644
--- a/clock.c
+++ b/clock.c
@@ -1288,10 +1288,11 @@ int clock_gm_capable(struct clock *c)
int clock_is_gm(struct clock *c)
{
pr_debug("clock_is_gm()");
- pr_debug(" this clock: %s", cid2str(&c->dds.clockIdentity));
- pr_debug(" best clock: %s", cid2str(&c->best_id));
- pr_debug(" cid_eq: %d", cid_eq(&c->dds.clockIdentity, &c->best_id));
- return cid_eq(&c->dds.clockIdentity, &c->best_id);
+ pr_debug(" This clock ID: %s", cid2str(&c->dds.clockIdentity));
+ pr_debug(" GM clock ID: %s", cid2str(&c->dad.pds.grandmasterIdentity));
+ pr_debug(" cid_eq: %d", cid_eq(&c->dds.clockIdentity, &c->dad.pds.grandmasterIdentity));
+
+ return cid_eq(&c->dds.clockIdentity, &c->dad.pds.grandmasterIdentity);
}
struct ClockIdentity clock_identity(struct clock *c)
diff --git a/foreign.h b/foreign.h
index 0fdc8e3..6e935b1 100644
--- a/foreign.h
+++ b/foreign.h
@@ -25,7 +25,8 @@
#include "ds.h"
#include "port.h"
-#define FOREIGN_MASTER_THRESHOLD 2
+#define PTP_FOREIGN_MASTER_THRESHOLD 2
+#define GPTP_FOREIGN_MASTER_THRESHOLD 1
struct foreign_clock {
/**
diff --git a/fsm.c b/fsm.c
index ce6efad..8bdf7c6 100644
--- a/fsm.c
+++ b/fsm.c
@@ -18,6 +18,31 @@
*/
#include "fsm.h"
+enum port_state gptp_fsm(enum port_state state, enum fsm_event event, int mdiff)
+{
+ enum port_state next = ptp_fsm(state, event, mdiff);
+
+ switch (next) {
+ case PS_UNCALIBRATED:
+ next = PS_SLAVE;
+ break;
+ case PS_PRE_MASTER:
+ next = PS_MASTER;
+ break;
+ case PS_INITIALIZING:
+ case PS_FAULTY:
+ case PS_DISABLED:
+ case PS_LISTENING:
+ case PS_MASTER:
+ case PS_PASSIVE:
+ case PS_SLAVE:
+ case PS_GRAND_MASTER:
+ break;
+ }
+
+ return next;
+}
+
enum port_state ptp_fsm(enum port_state state, enum fsm_event event, int mdiff)
{
enum port_state next = state;
diff --git a/fsm.h b/fsm.h
index 0616daa..5a090d1 100644
--- a/fsm.h
+++ b/fsm.h
@@ -63,6 +63,7 @@ enum fsm_event {
* @return The new state for the port.
*/
enum port_state ptp_fsm(enum port_state state, enum fsm_event event, int mdiff);
+enum port_state gptp_fsm(enum port_state state, enum fsm_event event, int mdiff);
/**
* Run the state machine for a slave only clock.
diff --git a/port.c b/port.c
index aeddfbf..1b4e18c 100644
--- a/port.c
+++ b/port.c
@@ -323,10 +323,14 @@ static void fc_prune(struct foreign_clock *fc)
{
struct timespec now;
struct ptp_message *m;
+ uint32_t foreign_master_threshold;
clock_gettime(CLOCK_MONOTONIC, &now);
- while (fc->n_messages > FOREIGN_MASTER_THRESHOLD) {
+ foreign_master_threshold = port_is_ieee8021as(fc->port) ?
+ GPTP_FOREIGN_MASTER_THRESHOLD : PTP_FOREIGN_MASTER_THRESHOLD;
+
+ while (fc->n_messages > foreign_master_threshold) {
m = TAILQ_LAST(&fc->messages, messages);
TAILQ_REMOVE(&fc->messages, m, list);
fc->n_messages--;
@@ -374,6 +378,7 @@ static int add_foreign_master(struct port *p, struct ptp_message *m)
struct foreign_clock *fc;
struct ptp_message *tmp;
int broke_threshold = 0, diff = 0;
+ uint32_t foreign_master_threshold;
LIST_FOREACH(fc, &p->foreign_masters, list) {
if (msg_source_equal(m, fc))
@@ -393,15 +398,21 @@ static int add_foreign_master(struct port *p, struct ptp_message *m)
LIST_INSERT_HEAD(&p->foreign_masters, fc, list);
fc->port = p;
fc->dataset.sender = m->header.sourcePortIdentity;
- /* We do not count this first message, see 9.5.3(b) */
- return 0;
+
+ if (!port_is_ieee8021as(p))
+ /* We do not count this first message, see 9.5.3(b) */
+ return 0;
}
/*
* If this message breaks the threshold, that is an important change.
*/
fc_prune(fc);
- if (FOREIGN_MASTER_THRESHOLD - 1 == fc->n_messages)
+
+ foreign_master_threshold = port_is_ieee8021as(fc->port) ?
+ GPTP_FOREIGN_MASTER_THRESHOLD : PTP_FOREIGN_MASTER_THRESHOLD;
+
+ if (foreign_master_threshold - 1 == fc->n_messages)
broke_threshold = 1;
/*
@@ -2066,6 +2077,7 @@ struct foreign_clock *port_compute_best(struct port *p)
{
struct foreign_clock *fc;
struct ptp_message *tmp;
+ uint32_t foreign_master_threshold;
p->best = NULL;
@@ -2078,7 +2090,10 @@ struct foreign_clock *port_compute_best(struct port *p)
fc_prune(fc);
- if (fc->n_messages < FOREIGN_MASTER_THRESHOLD)
+ foreign_master_threshold = port_is_ieee8021as(fc->port) ?
+ GPTP_FOREIGN_MASTER_THRESHOLD : PTP_FOREIGN_MASTER_THRESHOLD;
+
+ if (fc->n_messages < foreign_master_threshold)
continue;
if (!p->best)
@@ -2594,7 +2609,6 @@ struct port *port_open(int phc_index,
memset(p, 0, sizeof(*p));
- p->state_machine = clock_slave_only(clock) ? ptp_slave_fsm : ptp_fsm;
p->phc_index = phc_index;
p->jbod = config_get_int(cfg, interface->name, "boundary_clock_jbod");
transport = config_get_int(cfg, interface->name, "network_transport");
@@ -2641,6 +2655,13 @@ struct port *port_open(int phc_index,
pr_warning("port %d: hybrid_e2e only works with E2E", number);
}
+ if (clock_slave_only(clock))
+ p->state_machine = ptp_slave_fsm;
+ else if (port_is_ieee8021as(p))
+ p->state_machine = gptp_fsm;
+ else
+ p->state_machine = ptp_fsm;
+
/* Set fault timeouts to a default value */
for (i = 0; i < FT_CNT; i++) {
p->flt_interval_pertype[i].type = FTMO_LOG2_SECONDS;
--
2.7.4

[Richard Cochran]

Post by Erik Hons
- Removed UNCALIBRATED and PRE_MASTER port states.
- Short circuited foreign master qualification mechanism.
- Correctly identify if the local PTP clock is the domain Grand Master.
- port: Reduce the foreign master threshold for 802.1AS

If you offer four changes, then there should be four patches.

Post by Erik Hons
diff --git a/fsm.c b/fsm.c
index ce6efad..8bdf7c6 100644
--- a/fsm.c
+++ b/fsm.c
@@ -18,6 +18,31 @@
*/
#include "fsm.h"
+enum port_state gptp_fsm(enum port_state state, enum fsm_event event, int mdiff)
+{
+ enum port_state next = ptp_fsm(state, event, mdiff);
+
+ switch (next) {
+ next = PS_SLAVE;
+ break;
+ next = PS_MASTER;
+ break;
+ break;
+ }
+
+ return next;
+}

There are two things I dislike about this:

1. I prefer to have new FSMs in their own files.
Please introduce a new C module, like fsm_gptp.c.

2. The "overloading" is a bit gross. I would rather see the entire
gPTP FSM by itself, despite the code duplication. The code savings
from overloading would be small, and the independence of the two
algorithms is more important. After all, the FSMs in the two
standards are defined in totally different ways.

Thanks,
Richard