On Tue, 25 Apr 2023 at 10:16, Wurm, Stephan <stephan.w...@a-eberle.de> wrote:
> Am Montag, dem 24.04.2023 um 12:39 +0200 schrieb Erez: > > > > > > On Mon, 24 Apr 2023 at 11:08, Stephan Wurm <stephan.w...@a-eberle.de> > wrote: > > Standard IEC 21439-3:2016 Appendix A extends the PTPv2 standard by the > definition of doubly attached clocks (DAC) via redundant ports (either > connected by HSR or PRP). Therefore, the state machine is extended by > state PASSIVE_SLAVE and transition PSLAVE. > > In order to take advantage of the DAC feature, two interfaces need to > be configured as redundant port by explicitly selecting the respective > other interface via the `paired_interface` configuration option. > > The new state is reported as PASSIVE via the management interface, > remaining compatible with the PTPv2 standard. > > Signed-off-by: Stephan Wurm <stephan.w...@a-eberle.de> > --- > bmc.c | 10 ++++++++ > clock.c | 4 ++++ > config.c | 1 + > e2e_tc.c | 1 + > fsm.c | 71 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > fsm.h | 2 ++ > p2p_tc.c | 2 ++ > pmc.c | 4 ++-- > port.c | 44 +++++++++++++++++++++++++++++++--- > port.h | 47 ++++++++++++++++++++++++------------ > port_private.h | 4 ++++ > port_signaling.c | 1 + > tc.c | 2 ++ > unicast_service.c | 1 + > util.c | 4 +++- > 15 files changed, 177 insertions(+), 21 deletions(-) > > diff --git a/bmc.c b/bmc.c > index ebc0789..1e1d83f 100644 > --- a/bmc.c > +++ b/bmc.c > @@ -130,12 +130,14 @@ enum port_state bmc_state_decision(struct clock *c, > struct port *r, > int (*compare)(struct dataset *a, > struct dataset *b)) > { > struct dataset *clock_ds, *clock_best, *port_best; > + struct port *paired_port; > enum port_state ps; > > clock_ds = clock_default_ds(c); > clock_best = clock_best_foreign(c); > port_best = port_best_foreign(r); > ps = port_state(r); > + paired_port = port_paired_port(r); > > /* > * This scenario is particularly important in the > designated_slave_fsm > @@ -167,6 +169,14 @@ enum port_state bmc_state_decision(struct clock *c, > struct port *r, > return PS_SLAVE; /*S1*/ > } > > + /* > + * This scenario handles the PASSIVE_SLAVE transition according to > + * IEC 62439-3 standard in case of a doubly attached clock. > + */ > + if (paired_port && (clock_best_port(c) == paired_port)) { > + return PS_PASSIVE_SLAVE; > + } > + > if (compare(clock_best, port_best) == A_BETTER_TOPO) { > return PS_PASSIVE; /*P2*/ > } else { > diff --git a/clock.c b/clock.c > index 75d7c40..d52e826 100644 > --- a/clock.c > +++ b/clock.c > @@ -1009,6 +1009,7 @@ static int clock_add_port(struct clock *c, const > char *phc_device, > return -1; > } > LIST_FOREACH(piter, &c->ports, list) { > + port_pair(piter, p); > lastp = piter; > } > if (lastp) { > @@ -2235,6 +2236,9 @@ static void handle_state_decision_event(struct clock > *c) > clock_update_slave(c); > event = EV_RS_SLAVE; > break; > + case PS_PASSIVE_SLAVE: > + event = EV_RS_PSLAVE; > + break; > default: > event = EV_FAULT_DETECTED; > break; > diff --git a/config.c b/config.c > index cb4421f..28beb3b 100644 > --- a/config.c > +++ b/config.c > @@ -298,6 +298,7 @@ struct config_item config_tab[] = { > GLOB_ITEM_INT("offsetScaledLogVariance", 0xffff, 0, UINT16_MAX), > PORT_ITEM_INT("operLogPdelayReqInterval", 0, INT8_MIN, INT8_MAX), > PORT_ITEM_INT("operLogSyncInterval", 0, INT8_MIN, INT8_MAX), > + PORT_ITEM_STR("paired_interface", ""), > PORT_ITEM_INT("path_trace_enabled", 0, 0, 1), > PORT_ITEM_INT("phc_index", -1, -1, INT_MAX), > GLOB_ITEM_DBL("pi_integral_const", 0.0, 0.0, DBL_MAX), > diff --git a/e2e_tc.c b/e2e_tc.c > index 2f8e821..94099eb 100644 > --- a/e2e_tc.c > +++ b/e2e_tc.c > @@ -69,6 +69,7 @@ void e2e_dispatch(struct port *p, enum fsm_event event, > int mdiff) > flush_delay_req(p); > /* fall through */ > case PS_SLAVE: > + case PS_PASSIVE_SLAVE: > port_set_announce_tmo(p); > break; > }; > diff --git a/fsm.c b/fsm.c > index ce6efad..9679fea 100644 > --- a/fsm.c > +++ b/fsm.c > @@ -80,6 +80,9 @@ enum port_state ptp_fsm(enum port_state state, enum > fsm_event event, int mdiff) > case EV_RS_PASSIVE: > next = PS_PASSIVE; > break; > + case EV_RS_PSLAVE: > + next = PS_PASSIVE_SLAVE; > + break; > default: > break; > } > @@ -102,6 +105,9 @@ enum port_state ptp_fsm(enum port_state state, enum > fsm_event event, int mdiff) > case EV_RS_PASSIVE: > next = PS_PASSIVE; > break; > + case EV_RS_PSLAVE: > + next = PS_PASSIVE_SLAVE; > + break; > default: > break; > } > @@ -122,6 +128,9 @@ enum port_state ptp_fsm(enum port_state state, enum > fsm_event event, int mdiff) > case EV_RS_PASSIVE: > next = PS_PASSIVE; > break; > + case EV_RS_PSLAVE: > + next = PS_PASSIVE_SLAVE; > + break; > default: > break; > } > @@ -210,6 +219,40 @@ enum port_state ptp_fsm(enum port_state state, enum > fsm_event event, int mdiff) > case EV_RS_PASSIVE: > next = PS_PASSIVE; > break; > + case EV_RS_PSLAVE: > + next = PS_PASSIVE_SLAVE; > + break; > + default: > + break; > + } > + break; > + > + case PS_PASSIVE_SLAVE: > + switch (event) { > + case EV_DESIGNATED_DISABLED: > + next = PS_DISABLED; > + break; > + case EV_FAULT_DETECTED: > + next = PS_FAULTY; > + break; > + case EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES: > + next = PS_MASTER; > + break; > + case EV_SYNCHRONIZATION_FAULT: > + next = PS_LISTENING; > + break; > + case EV_RS_MASTER: > + next = PS_PRE_MASTER; > + break; > + case EV_RS_GRAND_MASTER: > + next = PS_GRAND_MASTER; > + break; > + case EV_RS_PASSIVE: > + next = PS_PASSIVE; > + break; > + case EV_RS_SLAVE: > + next = PS_SLAVE; > + break; > default: > break; > } > @@ -276,6 +319,9 @@ enum port_state ptp_slave_fsm(enum port_state state, > enum fsm_event event, > case EV_RS_SLAVE: > next = PS_UNCALIBRATED; > break; > + case EV_RS_PSLAVE: > + next = PS_PASSIVE_SLAVE; > + break; > default: > break; > } > @@ -324,6 +370,31 @@ enum port_state ptp_slave_fsm(enum port_state state, > enum fsm_event event, > if (mdiff) > next = PS_UNCALIBRATED; > break; > + case EV_RS_PSLAVE: > + next = PS_PASSIVE_SLAVE; > + break; > + default: > + break; > + } > + break; > + > + case PS_PASSIVE_SLAVE: > + switch (event) { > + case EV_DESIGNATED_DISABLED: > + next = PS_DISABLED; > + break; > + case EV_FAULT_DETECTED: > + next = PS_FAULTY; > + break; > + case EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES: > + case EV_RS_MASTER: > + case EV_RS_GRAND_MASTER: > + case EV_RS_PASSIVE: > + next = PS_LISTENING; > + break; > + case EV_RS_SLAVE: > + next = PS_SLAVE; > + break; > default: > break; > } > diff --git a/fsm.h b/fsm.h > index 857af05..919e934 100644 > --- a/fsm.h > +++ b/fsm.h > @@ -31,6 +31,7 @@ enum port_state { > PS_PASSIVE, > PS_UNCALIBRATED, > PS_SLAVE, > + PS_PASSIVE_SLAVE, /*according to IEC 62439-3 doubly attached > clocks*/ > PS_GRAND_MASTER, /*non-standard extension*/ > }; > > @@ -53,6 +54,7 @@ enum fsm_event { > EV_RS_GRAND_MASTER, > EV_RS_SLAVE, > EV_RS_PASSIVE, > + EV_RS_PSLAVE, /*according to IEC 62439-3 doubly attached clocks*/ > }; > > enum bmca_select { > diff --git a/p2p_tc.c b/p2p_tc.c > index 75cb3b9..2aabba8 100644 > --- a/p2p_tc.c > +++ b/p2p_tc.c > @@ -37,6 +37,7 @@ static int p2p_delay_request(struct port *p) > case PS_PASSIVE: > case PS_UNCALIBRATED: > case PS_SLAVE: > + case PS_PASSIVE_SLAVE: > case PS_GRAND_MASTER: > break; > } > @@ -85,6 +86,7 @@ void p2p_dispatch(struct port *p, enum fsm_event event, > int mdiff) > break; > case PS_UNCALIBRATED: > case PS_SLAVE: > + case PS_PASSIVE_SLAVE: > port_set_announce_tmo(p); > break; > }; > diff --git a/pmc.c b/pmc.c > index 00e691f..af797bb 100644 > --- a/pmc.c > +++ b/pmc.c > @@ -463,7 +463,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp) > break; > case MID_PORT_DATA_SET: > p = (struct portDS *) mgt->data; > - if (p->portState > PS_SLAVE) { > + if (p->portState > PS_PASSIVE_SLAVE) { > > > This is IEEE 1558 port state, you should add non IEEE 1558 port states > here! > Values are defined in IEEE 1588-2019 "8.2.15.3.1 portDS.portState". > I think we have 3 options: > Ask IEEE 1558 for a new port state. > Use enterprise management tlv > Perhaps Use high value like 0xf1? > > > On the one hand the IEC 62493-3:2016 specification adds the non IEEE 1588 > port state PASSIVE_SLAVE to the state machine, on the other hand it > requests > to map this new state to IEEE 1588 port state PASSIVE on 1588 management > tlvs. > Hence the non-1588 port state is never exported via management tlv. > > > p->portState = 0; > } > fprintf(fp, "PORT_DATA_SET " > @@ -494,7 +494,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp) > break; > case MID_PORT_PROPERTIES_NP: > ppn = (struct port_properties_np *) mgt->data; > - if (ppn->port_state > PS_SLAVE) { > + if (ppn->port_state > PS_PASSIVE_SLAVE) { > > > > Although this is a linuxptp enterprise tlv. > The port state are IEEE 1558. > > Would it be better to add explicit handling for the PASSIVE_SLAVE state, > keeping > the existing code as-is? > Currently the mapping is handled implicitly via the ps_str[] in util.c. > > Using internal or a flag is up to you. ps_str[] is just an internal conversion of states to strings. PORT_PROPERTIES_NP is a management TLV. Changing here means you are making PS_PASSIVE_SLAVE a public port state. The content of the management TLV is public! > > diff --git a/port.h b/port.h > index 57c8c2f..f1deaf1 100644 > --- a/port.h > +++ b/port.h > > > In this file it seems you just add spaces. > Please, only real changes, no spaces! > > > I think I did let checkpatch.pl or my editor do some automatic fixes, > also affecting some additional lines. > I will remove those unrelated space changes. > > checkpatch.pl does not remove white spaces, you should also review your patches manually before you send. Sometimes during development we do add white spaces, some editors like to "help" us. >
_______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel