Re: [Linuxptp-devel] [PATCH v2 1/6] Add doubly attached clock support
Hello Richard,
Am 25.07.2023 um 19:38 hat Richard Cochran geschrieben:
> On Tue, Jul 04, 2023 at 12:30:38PM +0200, Stephan Wurm wrote:
>
> > 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*/
>
> NAK. There is no such state in IEEE 1588.
>
> > 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*/
>
> There is no such recommended state event.
>
> If you "profile" invents a new BMCA, then you should implement it
> explictily.
>
> We have ptp_fsm() and ptp_slave_fsm(), and you really should add
> ptp_iec_whatevet_fsm() rather than hacking in specialy cases to the
> 1588 state machines.
thanks for pointing out the proper entry points for this profile!
I think the implementation needs a major overhaul then.
Best regards
Stephan Wurm
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 1/6] Add doubly attached clock support
On Wed, 26 Jul 2023 at 04:39, Richard Cochran
wrote:
> On Tue, Jul 04, 2023 at 12:30:38PM +0200, Stephan Wurm wrote:
>
> > 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*/
>
> NAK. There is no such state in IEEE 1588.
>
I Wonder, I thought we already excluded this 2 month ago. :-(
>
> > 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*/
>
> There is no such recommended state event.
>
> If you "profile" invents a new BMCA, then you should implement it
> explictily.
>
> We have ptp_fsm() and ptp_slave_fsm(), and you really should add
> ptp_iec_whatevet_fsm() rather than hacking in specialy cases to the
> 1588 state machines.
>
> Thanks,
> Richard
>
>
> ___
> Linuxptp-devel mailing list
> Linuxptp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 1/6] Add doubly attached clock support
On Tue, Jul 04, 2023 at 12:30:38PM +0200, Stephan Wurm wrote:
> 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*/
NAK. There is no such state in IEEE 1588.
> 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*/
There is no such recommended state event.
If you "profile" invents a new BMCA, then you should implement it
explictily.
We have ptp_fsm() and ptp_slave_fsm(), and you really should add
ptp_iec_whatevet_fsm() rather than hacking in specialy cases to the
1588 state machines.
Thanks,
Richard
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 1/6] Add doubly attached clock support
Hello Jake
Am Mittwoch, dem 05.07.2023 um 10:50 -0700 schrieb Jacob Keller:
On 7/4/2023 3:30 AM, Stephan Wurm 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
mailto: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 ++
port.c| 52
port.h| 17 +
port_private.h| 4
port_signaling.c | 1 +
tc.c | 2 ++
unicast_service.c | 1 +
util.c| 4 +++-
14 files changed, 167 insertions(+), 5 deletions(-)
@@ -3406,6 +3427,10 @@ struct port *port_open(const char *phc_device,
config_get_int(cfg, p->name,
"power_profile.2017.totalTimeInaccuracy");
p->slave_event_monitor = clock_slave_monitor(clock);
+ p->paired_interface = config_get_string(cfg, p->name,
"paired_interface");
+ p->prpPairedPort = UINT16_MAX;
+ p->paired_port = NULL;
+
if (!port_is_uds(p) && unicast_client_initialize(p)) {
goto err_transport;
}
@@ -3548,3 +3573,22 @@ void port_update_unicast_state(struct port *p)
p->unicast_state_dirty = false;
}
}
+
+void port_pair(struct port *p, struct port *o)
+{
+ if ((strncmp(p->paired_interface, interface_name(o->iface),
+ MAX_IFNAME_SIZE) == 0) &&
+ (strncmp(o->paired_interface, interface_name(p->iface),
+ MAX_IFNAME_SIZE) == 0)) {
+ p->paired_port = o;
+ p->prpPairedPort = portnum(o);
+ o->paired_port = p;
+ o->prpPairedPort = portnum(p);
+ pr_info("Created redundancy pair from ports %s and %s",
p->name, o->name);
+ }
Given the name "doubly-attached clock" I would have assumed we would
need to check that both ports are tied to the same PTP hardware clock.
Is that not the case?
At least the IEC62439-3 specification does not handle this.
In fact, I performed my first tests on a hardware with shared PHC between both
interfaces of the doubly-attached (redundant) setup.
If they are not tied to the same clock, how do you ensure the two
separate clocks are actually in sync with each other? I guess if both
clocks are synchronized simultaneously with the GM then it would be
within some tolerance, so perhaps thats ok.
As both interfaces keep synchronizing to the same GM, the impact might indeed
be negligible.
In my recent test setup the interfaces have individual PHCs, so I can try to
have
a deeper look into that question.
Thanks for your feedback
Stephan
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH v2 1/6] Add doubly attached clock support
On 7/4/2023 3:30 AM, Stephan Wurm 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
> ---
> bmc.c | 10
> clock.c | 4
> config.c | 1 +
> e2e_tc.c | 1 +
> fsm.c | 71
> +++
> fsm.h | 2 ++
> p2p_tc.c | 2 ++
> port.c| 52
> port.h| 17 +
> port_private.h| 4
> port_signaling.c | 1 +
> tc.c | 2 ++
> unicast_service.c | 1 +
> util.c| 4 +++-
> 14 files changed, 167 insertions(+), 5 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 5a64613..cff2475 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) {
> @@ -2242,6 +2243,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 b104f1b..aadd3d9 100644
> --- a/config.c
> +++ b/config.c
> @@ -298,6 +298,7 @@ struct config_item config_tab[] = {
> GLOB_ITEM_INT("offsetScaledLogVariance", 0x, 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;
> br
[Linuxptp-devel] [PATCH v2 1/6] Add doubly attached clock support
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
---
bmc.c | 10
clock.c | 4
config.c | 1 +
e2e_tc.c | 1 +
fsm.c | 71 +++
fsm.h | 2 ++
p2p_tc.c | 2 ++
port.c| 52
port.h| 17 +
port_private.h| 4
port_signaling.c | 1 +
tc.c | 2 ++
unicast_service.c | 1 +
util.c| 4 +++-
14 files changed, 167 insertions(+), 5 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 5a64613..cff2475 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) {
@@ -2242,6 +2243,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 b104f1b..aadd3d9 100644
--- a/config.c
+++ b/config.c
@@ -298,6 +298,7 @@ struct config_item config_tab[] = {
GLOB_ITEM_INT("offsetScaledLogVariance", 0x, 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;
