Discussion:
[Linuxptp-devel] [PATCH 2/4] Reset clock parent DS to default DS when no best master clock is available
Delio Brignoli
2013-11-19 13:42:39 UTC
Permalink
Before this fix when in slave mode, if a former foreign master disappeared
for some reason and nothing replaced it, we started announcing ourselves
using the former master DS

Signed-off-by: Delio Brignoli <***@audioscience.com>
---
clock.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/clock.c b/clock.c
index 1563ba9..a36afc9 100644
--- a/clock.c
+++ b/clock.c
@@ -1163,6 +1163,7 @@ static void handle_state_decision_event(struct clock *c)
best_id = best->dataset.identity;
} else {
best_id = c->dds.clockIdentity;
+ clock_update_grandmaster(c);
}

pr_notice("selected best master clock %s",
--
1.7.0.4
Delio Brignoli
2013-11-19 13:42:41 UTC
Permalink
Signed-off-by: Delio Brignoli <***@audioscience.com>
---
port.c | 21 ++++++++++++++++-----
1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/port.c b/port.c
index 63b922e..cca0f68 100644
--- a/port.c
+++ b/port.c
@@ -57,6 +57,7 @@ struct nrate_estimator {
tmv_t ingress1;
unsigned int max_count;
unsigned int count;
+ int ratio_valid;
};

struct port {
@@ -474,6 +475,13 @@ static int port_capable(struct port *p)
goto not_capable;
}

+ if (!p->nrate.ratio_valid) {
+ if (p->asCapable)
+ pr_debug("port %hu: invalid nrate, "
+ "resetting asCapable", portnum(p));
+ goto not_capable;
+ }
+
capable:
if (!p->asCapable)
pr_debug("port %hu: setting asCapable", portnum(p));
@@ -733,6 +741,12 @@ static void port_nrate_calculate(struct port *p, tmv_t t3, tmv_t t4, tmv_t c)
tmv_t origin2;
struct nrate_estimator *n = &p->nrate;

+ /*
+ * We experienced a successful exchanges of peer delay request
+ * and response, reset pdr_missing for this port.
+ */
+ p->pdr_missing = 0;
+
if (!n->ingress1) {
n->ingress1 = t4;
n->origin1 = tmv_add(t3, c);
@@ -753,11 +767,7 @@ static void port_nrate_calculate(struct port *p, tmv_t t3, tmv_t t4, tmv_t c)
n->ingress1 = t4;
n->origin1 = origin2;
n->count = 0;
- /*
- * We experienced a successful series of exchanges of peer
- * delay request and response, and so the port is now capable.
- */
- p->pdr_missing = 0;
+ n->ratio_valid = 1;
}

static void port_nrate_initialize(struct port *p)
@@ -781,6 +791,7 @@ static void port_nrate_initialize(struct port *p)
p->nrate.ingress1 = tmv_zero();
p->nrate.max_count = (1 << shift);
p->nrate.count = 0;
+ p->nrate.ratio_valid = 0;
}

static int port_set_announce_tmo(struct port *p)
--
1.7.0.4
Richard Cochran
2013-11-30 19:41:24 UTC
Permalink
Post by Delio Brignoli
---
port.c | 21 ++++++++++++++++-----
1 files changed, 16 insertions(+), 5 deletions(-)
Applied, adding a bit more description into the change log message.

Thanks,
Richard
Delio Brignoli
2013-11-19 13:42:40 UTC
Permalink
Sync rx timeout should be set only after receiving the first sync, see
section 10.2.7, figure 10-4 PortSyncSyncReceive state machine in 802.1AS

Signed-off-by: Delio Brignoli <***@audioscience.com>
---
port.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/port.c b/port.c
index 5604987..63b922e 100644
--- a/port.c
+++ b/port.c
@@ -1914,7 +1914,6 @@ static void port_e2e_transition(struct port *p, enum port_state next)
/* fall through */
case PS_SLAVE:
port_set_announce_tmo(p);
- port_set_sync_rx_tmo(p);
port_set_delay_tmo(p);
break;
};
@@ -1957,7 +1956,6 @@ static void port_p2p_transition(struct port *p, enum port_state next)
/* fall through */
case PS_SLAVE:
port_set_announce_tmo(p);
- port_set_sync_rx_tmo(p);
break;
};
}
--
1.7.0.4
Richard Cochran
2013-11-30 19:21:36 UTC
Permalink
Post by Delio Brignoli
Sync rx timeout should be set only after receiving the first sync, see
section 10.2.7, figure 10-4 PortSyncSyncReceive state machine in 802.1AS
Applied.

Thanks,
Richard
Delio Brignoli
2013-11-25 19:04:16 UTC
Permalink
Hello Richard,

Did you see this patch series? Any comments?

Thanks
--
Delio
Subject: [Linuxptp-devel] [PATCH 1/4] Schedule announce TX timeout also when in slave-only mode
Date: November 19, 2013 2:42:38 PM GMT+01:00
---
port.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/port.c b/port.c
index 94c5cac..5604987 100644
--- a/port.c
+++ b/port.c
@@ -1937,6 +1937,7 @@ static void port_p2p_transition(struct port *p, enum port_state next)
port_disable(p);
break;
+ set_tmo_log(p->fda.fd[FD_MANNO_TIMER], 1, -10); /*~1ms*/
port_set_announce_tmo(p);
break;
@@ -1993,6 +1994,7 @@ int port_dispatch(struct port *p, enum fsm_event event, int mdiff)
port_show_transition(p, next, event);
p->state = next;
if (next == PS_LISTENING && p->delayMechanism == DM_P2P) {
+ set_tmo_log(p->fda.fd[FD_MANNO_TIMER], 1, -10); /*~1ms*/
port_set_delay_tmo(p);
}
return 1;
--
1.7.0.4
------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Richard Cochran
2013-11-26 08:48:25 UTC
Permalink
Post by Delio Brignoli
Hello Richard,
Did you see this patch series? Any comments?
I did scan them briefly, and they seemed to be clear cut bug fixes. I
am a little behind in getting recent patches merged, but I will get to
it, hopefully soon.

Thanks,
Richard
Delio Brignoli
2013-11-26 09:28:12 UTC
Permalink
Post by Richard Cochran
Post by Delio Brignoli
Hello Richard,
Did you see this patch series? Any comments?
I did scan them briefly, and they seemed to be clear cut bug fixes. I
am a little behind in getting recent patches merged, but I will get to
it, hopefully soon.
Thanks Richard, I thought you might be busy, I just wanted to make sure you had seen the patches and they weren't forgotten.

Regards
--
Delio
Richard Cochran
2013-11-30 08:24:39 UTC
Permalink
---
port.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/port.c b/port.c
index 94c5cac..5604987 100644
--- a/port.c
+++ b/port.c
@@ -1937,6 +1937,7 @@ static void port_p2p_transition(struct port *p, enum port_state next)
port_disable(p);
break;
+ set_tmo_log(p->fda.fd[FD_MANNO_TIMER], 1, -10); /*~1ms*/
Delio, I don't think this is right. The MANNO timer is for controlling
when to send announce messages. According to 1588, announce messages
may only be sent by ports in the MASTER state.

Or what where you trying to fix here?

Thanks,
Richard
port_set_announce_tmo(p);
break;
@@ -1993,6 +1994,7 @@ int port_dispatch(struct port *p, enum fsm_event event, int mdiff)
port_show_transition(p, next, event);
p->state = next;
if (next == PS_LISTENING && p->delayMechanism == DM_P2P) {
+ set_tmo_log(p->fda.fd[FD_MANNO_TIMER], 1, -10); /*~1ms*/
port_set_delay_tmo(p);
}
return 1;
--
1.7.0.4
------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Delio Brignoli
2013-11-30 09:35:25 UTC
Permalink
Post by Richard Cochran
---
port.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/port.c b/port.c
index 94c5cac..5604987 100644
--- a/port.c
+++ b/port.c
@@ -1937,6 +1937,7 @@ static void port_p2p_transition(struct port *p, enum port_state next)
port_disable(p);
break;
+ set_tmo_log(p->fda.fd[FD_MANNO_TIMER], 1, -10); /*~1ms*/
Delio, I don't think this is right. The MANNO timer is for controlling
when to send announce messages. According to 1588, announce messages
may only be sent by ports in the MASTER state.
I see, I didn't have time to look up 1588 when I was fixing this issue.
Post by Richard Cochran
Or what where you trying to fix here?
From memory, the problem was that when running in slave-only mode linuxptp didn't start announcing when the port became asCapable. According to 802.1AS we should be announcing ourselves immediately when the port becomes asCapable even if we do not support being master.

Thanks
--
Delio
Post by Richard Cochran
Thanks,
Richard
port_set_announce_tmo(p);
break;
@@ -1993,6 +1994,7 @@ int port_dispatch(struct port *p, enum fsm_event event, int mdiff)
port_show_transition(p, next, event);
p->state = next;
if (next == PS_LISTENING && p->delayMechanism == DM_P2P) {
+ set_tmo_log(p->fda.fd[FD_MANNO_TIMER], 1, -10); /*~1ms*/
port_set_delay_tmo(p);
}
return 1;
--
1.7.0.4
------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Richard Cochran
2013-11-30 11:02:54 UTC
Permalink
On Sat, Nov 30, 2013 at 10:35:25AM +0100, Delio Brignoli wrote:
Richard Cochran
2013-11-30 16:34:26 UTC
Permalink
On Sat, Nov 30, 2013 at 10:35:25AM +0100, Delio Brignoli wrote:
Delio Brignoli
2013-11-30 20:46:46 UTC
Permalink
Post by Delio Brignoli
From memory, the problem was that when running in slave-only mode linuxptp didn't start announcing when the port became asCapable. According to 802.1AS we should be announcing ourselves immediately when the port becomes asCapable even if we do not support being master.
AFAICT, the only part of 802.1AS-2011 that mentions when to transmit an
announce message is Figure 10-15 on page 84. On the right hand side
there is the state TRANSMIT_ANNOUNCE which calls the txAnnounce method.
Thanks for taking the time to look into this. See below for my comments.
That state is only reachable if (selectedRole == MasterPort), and so
it looks like we should not transmit when is slave only mode, just
like in 1588.
Yes, but... 802.1AS does not have a different state machine for slave-only operation, instead a non grand-master capable time-aware system has a property1 value of 255 (see second paragraph of section 8.6.2.1). Additionally, according to point f.5 of section 10.3.12.1.4 updtRolesTree(), if the current portPriorityVector is the portPriorityVector of the time-aware system (i.e. we have not yet received an announce message with better portPriorityVector) the portRole should be set to MasterPort which makes the txAnnounce method reachable.

Thanks
--
Delio
Richard Cochran
2013-12-01 06:48:03 UTC
Permalink
Post by Delio Brignoli
Yes, but... 802.1AS does not have a different state machine for
slave-only operation, instead a non grand-master capable time-aware
system has a property1 value of 255 (see second paragraph of section
8.6.2.1). Additionally, according to point f.5 of section
10.3.12.1.4 updtRolesTree(), if the current portPriorityVector is
the portPriorityVector of the time-aware system (i.e. we have not
yet received an announce message with better portPriorityVector) the
portRole should be set to MasterPort which makes the txAnnounce
method reachable.
This brings to mind two questions.

1. Does omitting the announce message from a slave only node cause you
interoperability problems?

2. What is the point of the slave only node's announce messages?

[ The announce messages will not affect the outcome of the peer's
BMC election, except to make it slower. ]

Thanks,
Richard
Delio Brignoli
2013-12-01 09:47:02 UTC
Permalink
Post by Richard Cochran
Post by Delio Brignoli
Yes, but... 802.1AS does not have a different state machine for
slave-only operation, instead a non grand-master capable time-aware
system has a property1 value of 255 (see second paragraph of section
8.6.2.1). Additionally, according to point f.5 of section
10.3.12.1.4 updtRolesTree(), if the current portPriorityVector is
the portPriorityVector of the time-aware system (i.e. we have not
yet received an announce message with better portPriorityVector) the
portRole should be set to MasterPort which makes the txAnnounce
method reachable.
This brings to mind two questions.
1. Does omitting the announce message from a slave only node cause you
interoperability problems?
Yes, we are failing interoperability tests without this fix.
Post by Richard Cochran
2. What is the point of the slave only node's announce messages?
[ The announce messages will not affect the outcome of the peer's
BMC election, except to make it slower. ]
Knowing the node exists and it is a slave I guess...

From point i.4 of section 7.5 "Differences between gPTP and PTP": "all time-aware systems are required to participate in best master selection (even if it is not grandmaster capable)".

Thanks
--
Delio
Delio Brignoli
2013-12-01 09:57:57 UTC
Permalink
Post by Richard Cochran
Post by Delio Brignoli
Yes, but... 802.1AS does not have a different state machine for
slave-only operation, instead a non grand-master capable time-aware
system has a property1 value of 255 (see second paragraph of section
8.6.2.1). Additionally, according to point f.5 of section
10.3.12.1.4 updtRolesTree(), if the current portPriorityVector is
the portPriorityVector of the time-aware system (i.e. we have not
yet received an announce message with better portPriorityVector) the
portRole should be set to MasterPort which makes the txAnnounce
method reachable.
This brings to mind two questions.
1. Does omitting the announce message from a slave only node cause you
interoperability problems?
Yes, we are failing interoperability tests without this fix.
Post by Richard Cochran
2. What is the point of the slave only node's announce messages?
[ The announce messages will not affect the outcome of the peer's
BMC election, except to make it slower. ]
Knowing the node exists and it is a slave I guess...

From point i.4 of section 7.5 "Differences between gPTP and PTP": "all time-aware systems are required to participate in best master selection (even if it is not grandmaster capable)".

Thanks
--
Delio
Keller, Jacob E
2013-12-02 19:35:52 UTC
Permalink
Post by Delio Brignoli
Post by Richard Cochran
Post by Delio Brignoli
Yes, but... 802.1AS does not have a different state machine for
slave-only operation, instead a non grand-master capable time-aware
system has a property1 value of 255 (see second paragraph of section
8.6.2.1). Additionally, according to point f.5 of section
10.3.12.1.4 updtRolesTree(), if the current portPriorityVector is
the portPriorityVector of the time-aware system (i.e. we have not
yet received an announce message with better portPriorityVector) the
portRole should be set to MasterPort which makes the txAnnounce
method reachable.
This brings to mind two questions.
1. Does omitting the announce message from a slave only node cause you
interoperability problems?
Yes, we are failing interoperability tests without this fix.
Post by Richard Cochran
2. What is the point of the slave only node's announce messages?
[ The announce messages will not affect the outcome of the peer's
BMC election, except to make it slower. ]
Knowing the node exists and it is a slave I guess...
I believe it's in order to know that the node is asCapable.. I know at
least one switch which will ignore a port unless it receives an announce
from it that indicates it's asCapable...

Regards,
Jake
Post by Delio Brignoli
Post by Richard Cochran
From point i.4 of section 7.5 "Differences between gPTP and PTP": "all time-aware systems are required to participate in best master selection (even if it is not grandmaster capable)".
Delio Brignoli
2013-12-03 10:37:48 UTC
Permalink
Post by Keller, Jacob E
Post by Delio Brignoli
Post by Richard Cochran
2. What is the point of the slave only node's announce messages?
[ The announce messages will not affect the outcome of the peer's
BMC election, except to make it slower. ]
Knowing the node exists and it is a slave I guess...
I believe it's in order to know that the node is asCapable.. I know at
least one switch which will ignore a port unless it receives an announce
from it that indicates it's asCapable...
According to the standard the peer delay messages exchange is used to establish if a peer is asCapable.

--
Delio
Richard Cochran
2013-12-03 17:19:11 UTC
Permalink
Post by Delio Brignoli
Post by Keller, Jacob E
I believe it's in order to know that the node is asCapable.. I know at
least one switch which will ignore a port unless it receives an announce
from it that indicates it's asCapable...
According to the standard the peer delay messages exchange is used to establish if a peer is asCapable.
Yes, that is true, but still Jacob's example underscores the need to
have a gPTP node always transmit announce messages.

Thanks,
Richard

Richard Cochran
2013-12-01 06:52:36 UTC
Permalink
... the portRole should be set to MasterPort which makes the
txAnnounce method reachable.
So, once the port's role becomes "master", doesn't it also transmit
sync messages?

Thanks,
Richard
Delio Brignoli
2013-12-01 09:49:22 UTC
Permalink
Post by Richard Cochran
... the portRole should be set to MasterPort which makes the
txAnnounce method reachable.
So, once the port's role becomes "master", doesn't it also transmit
sync messages?
I do not think so because the second paragraph of section 10.2.1 says the SiteSyncSync state machine ignores sync information sent to them by the ClockMasterSyncSend state machine if the time-aware system is not grandmaster-capable. This is also re-iterated in the second paragraph of section 10.2.8.3. This seems to imply, albeit not in a very obvious way, that PortSyncSyncSend and MDSyncSend never operate in a time-aware system that is not grandmaster capable, hence no sync TX.

Thanks
--
Delio
Richard Cochran
2013-11-30 11:32:47 UTC
Permalink
Post by Delio Brignoli
Before this fix when in slave mode, if a former foreign master disappeared
for some reason and nothing replaced it, we started announcing ourselves
using the former master DS
It looks like this patch is only needed in connection with patch #1.
Without the first patch, then there is no issue with the announce
messages after loss of master.

(or maybe I am not seeing it?)

Thanks,
Richard
Delio Brignoli
2013-11-30 13:14:57 UTC
Permalink
Post by Richard Cochran
Post by Delio Brignoli
Before this fix when in slave mode, if a former foreign master disappeared
for some reason and nothing replaced it, we started announcing ourselves
using the former master DS
It looks like this patch is only needed in connection with patch #1.
Without the first patch, then there is no issue with the announce
messages after loss of master.
(or maybe I am not seeing it?)
I believe you are correct. This issue arises only if we go back to announcing after master goes away.

Thanks
--
Delio
Post by Richard Cochran
Thanks,
Richard
Loading...