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
