From: Vincent Cheng
Problem
===
Unselected master port may unintentionally continue to request
announce/sync/delay_resp packets. Expecting port with unselected master
to only maintain announce messages.
Cause
=
In handle_state_decision_event() loop, the update of the clock's parent pid
to reflect best master change is non-deterministic. It depends on the port
processing order and bmc_state_decision() results.
However, port_dispatch() called at the end of each loop iteration will use
the clock's parent pid in subsequent unicast_client_state_changed() to
drive the unicast FSM.
If the clock parent pid is not updated properly prior to
unicast_client_state_changed(), the unicast FSM may not
transition properly.
Solution v2
===
Refactor port_state_update() to mark unicast_state_dirty instead
of calling unicast_client_state_changed().
Add new port loop after handle_state_decision_event() port loop
to call unicast_client_state_changed() after all necessary
parent pid has been updated.
Signed-off-by: Vincent Cheng
---
clock.c| 6 ++
port.c | 12 ++--
port.h | 7 +++
port_private.h | 1 +
4 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/clock.c b/clock.c
index 134c7c3..a68a732 100644
--- a/clock.c
+++ b/clock.c
@@ -669,7 +669,9 @@ static void clock_update_grandmaster(struct clock *c)
struct parentDS *pds = &c->dad.pds;
memset(&c->cur, 0, sizeof(c->cur));
memset(c->ptl, 0, sizeof(c->ptl));
+
pds->parentPortIdentity.clockIdentity = c->dds.clockIdentity;
+ /* Follow IEEE 1588 Table 30: Updates for state decision code M1 and M2
*/
pds->parentPortIdentity.portNumber = 0;
pds->grandmasterIdentity= c->dds.clockIdentity;
pds->grandmasterClockQuality= c->dds.clockQuality;
@@ -2049,6 +2051,10 @@ static void handle_state_decision_event(struct clock *c)
}
port_dispatch(piter, event, fresh_best);
}
+
+ LIST_FOREACH(piter, &c->ports, list) {
+ port_update_unicast_state(piter);
+ }
}
struct clock_description *clock_description(struct clock *c)
diff --git a/port.c b/port.c
index 6baf5c8..c8cba5b 100644
--- a/port.c
+++ b/port.c
@@ -3445,13 +3445,13 @@ int port_state_update(struct port *p, enum fsm_event
event, int mdiff)
}
if (mdiff) {
- unicast_client_state_changed(p);
+ p->unicast_state_dirty = true;
}
if (next != p->state) {
port_show_transition(p, next, event);
p->state = next;
port_notify_event(p, NOTIFY_PORT_STATE);
- unicast_client_state_changed(p);
+ p->unicast_state_dirty = true;
return 1;
}
@@ -3462,3 +3462,11 @@ enum bmca_select port_bmca(struct port *p)
{
return p->bmca;
}
+
+void port_update_unicast_state(struct port *p)
+{
+ if (p->unicast_state_dirty) {
+ unicast_client_state_changed(p);
+ p->unicast_state_dirty = false;
+ }
+}
diff --git a/port.h b/port.h
index 4854698..57c8c2f 100644
--- a/port.h
+++ b/port.h
@@ -357,4 +357,11 @@ enum bmca_select port_bmca(struct port *p);
*/
void tc_cleanup(void);
+/**
+ * Update port's unicast state if port's unicast_state_dirty is true.
+ *
+ * @param port A port instance.
+ */
+void port_update_unicast_state(struct port *p);
+
#endif
diff --git a/port_private.h b/port_private.h
index d27dceb..13a6c6a 100644
--- a/port_private.h
+++ b/port_private.h
@@ -158,6 +158,7 @@ struct port {
int inhibit_multicast_service;
/* slave event monitoring */
struct monitor *slave_event_monitor;
+ bool unicast_state_dirty;
};
#define portnum(p) (p->portIdentity.portNumber)
--
2.34.1
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
