Discussion:
[Linuxptp-devel] [PATCH] Ignore delay_resp messages from foreign masters.
Miroslav Lichvar
2013-10-11 15:30:28 UTC
Permalink
When a new master appears, it will start to respond to our delay_req
messages. Make sure we process only responses from our current master
before switching to the new master.

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
port.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/port.c b/port.c
index 3fc262a..d98bb80 100644
--- a/port.c
+++ b/port.c
@@ -1486,10 +1486,12 @@ static void process_delay_resp(struct port *p, struct ptp_message *m)
{
struct delay_req_msg *req;
struct delay_resp_msg *rsp = &m->delay_resp;
+ struct PortIdentity master;

if (!p->delay_req)
return;

+ master = clock_parent_identity(p->clock);
req = &p->delay_req->delay_req;

if (p->state != PS_UNCALIBRATED && p->state != PS_SLAVE)
@@ -1498,6 +1500,8 @@ static void process_delay_resp(struct port *p, struct ptp_message *m)
return;
if (rsp->hdr.sequenceId != ntohs(req->hdr.sequenceId))
return;
+ if (!pid_eq(&master, &m->header.sourcePortIdentity))
+ return;

clock_path_delay(p->clock, p->delay_req->hwts.ts, m->ts.pdu,
m->header.correction);
--
1.8.3.1
Miroslav Lichvar
2013-10-11 15:30:29 UTC
Permalink
When a new master is selected, drop the old sync time stamp to prevent
calculating invalid delay in case delay_resp will be received before
first sync from the new master.

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
clock.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/clock.c b/clock.c
index 419a6b7..300d2d7 100644
--- a/clock.c
+++ b/clock.c
@@ -1155,6 +1155,8 @@ static void handle_state_decision_event(struct clock *c)
if (!cid_eq(&best_id, &c->best_id)) {
clock_freq_est_reset(c);
mave_reset(c->avg_delay);
+ c->t1 = tmv_zero();
+ c->t2 = tmv_zero();
c->path_delay = 0;
fresh_best = 1;
}
--
1.8.3.1
Richard Cochran
2013-10-11 17:49:12 UTC
Permalink
Post by Miroslav Lichvar
When a new master is selected, drop the old sync time stamp to prevent
calculating invalid delay in case delay_resp will be received before
first sync from the new master.
Both patches applied.

Thanks,
Richard
Keller, Jacob E
2013-10-11 16:26:19 UTC
Permalink
Post by Miroslav Lichvar
When a new master appears, it will start to respond to our delay_req
messages. Make sure we process only responses from our current master
before switching to the new master.
---
Nice catch!

Regards,
Jak
Miroslav Lichvar
2013-10-14 12:30:04 UTC
Permalink
Post by Keller, Jacob E
Post by Miroslav Lichvar
When a new master appears, it will start to respond to our delay_req
messages. Make sure we process only responses from our current master
before switching to the new master.
---
Nice catch!
The two bugs were causing the large negative delays I noticed earlier
in the log of the 07-latereselect test from the test suite.

I've extended the test to parse the delay from the log and check if it
stays below a limit to allow us to catch such bugs easily.
--
Miroslav Lichvar
Keller, Jacob E
2013-10-14 20:14:24 UTC
Permalink
Post by Miroslav Lichvar
Post by Keller, Jacob E
Post by Miroslav Lichvar
When a new master appears, it will start to respond to our delay_req
messages. Make sure we process only responses from our current master
before switching to the new master.
---
Nice catch!
The two bugs were causing the large negative delays I noticed earlier
in the log of the 07-latereselect test from the test suite.
I've extended the test to parse the delay from the log and check if it
stays below a limit to allow us to catch such bugs easily.
Sorry if you mentioned this before, but where can I get the test suite?
Would
Miroslav Lichvar
2013-10-15 09:05:05 UTC
Permalink
Post by Keller, Jacob E
Post by Miroslav Lichvar
The two bugs were causing the large negative delays I noticed earlier
in the log of the 07-latereselect test from the test suite.
I've extended the test to parse the delay from the log and check if it
stays below a limit to allow us to catch such bugs easily.
Sorry if you mentioned this before, but where can I get the test suite?
Would be interested to look at it.
$ git clone https://github.com/mlichvar/linuxptp-testsuite.git
$ cd linuxptp-testsuite
$ ./run

It will clone and build clknetsim from its repo if it's not found in
the current directory. You can run each test individually. After the
test, in the tmp directory there will be logs from clknetsim (stats,
log.offset and log.freq) and logs from ptp4l/phc2sys in log.[0-9]+.
--
Miroslav Lichvar
Loading...