Re: [Linuxptp-devel] [PATCH RFC 0/3] Poor man's boundary clock
On Wed, 5 Nov 2014 17:34:23 +0100, Richard Cochran wrote: I like the idea of automatic phc2sys, if it would only work right. It's definitely not complete yet and I'm not surprised there are bugs (although I did my best to support also future cases, it's hard to get it right without the actual code to test it). My plan for the next steps has been allowing ptp4l to work with multiple independent PHCs that would form a PTP clock (and rely on phc2sys to sync those PHCs). This needs separation of struct clock into two structures, as some fields are per-PTP clock and some are per-PHC. I have a patch that does this but it needs to be rebased. I think this is a prerequisite to having a boundary clock that uses multiple PHCs, I cannot imagine how it could work without this separation. What is not solved is the problem that Miroslav pointed out (and also pointed out to me earlier): the sync messages cannot be sent until the clock is really synced. This will need further communication between ptp4l and phc2sys. I still intend to work on this but have less time to work on linuxptp than before. Jiri -- Jiri Benc -- ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 0/3] Poor man's boundary clock
On Wed, Nov 05, 2014 at 05:56:28PM +0100, Jiri Benc wrote: My plan for the next steps has been allowing ptp4l to work with multiple independent PHCs that would form a PTP clock (and rely on phc2sys to sync those PHCs). Doesn't my patch #2 do this? This needs separation of struct clock into two structures, as some fields are per-PTP clock and some are per-PHC. I have a patch that does this but it needs to be rebased. You mean, the phc2sys 'struct clock', I guess? Please post the patch, so we see what you are talking about. I think this is a prerequisite to having a boundary clock that uses multiple PHCs, I cannot imagine how it could work without this separation. So, in the current form, what does the 'automatic' mode (I mean, -a without -r) provide? Nothing useful? We can't do a release with half baked stuff like that. Thanks, Richard -- ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 0/3] Poor man's boundary clock
On Wed, 5 Nov 2014 19:58:32 +0100, Richard Cochran wrote:
On Wed, Nov 05, 2014 at 05:56:28PM +0100, Jiri Benc wrote:
My plan for the next steps has been allowing ptp4l to work with
multiple independent PHCs that would form a PTP clock (and rely on
phc2sys to sync those PHCs).
Doesn't my patch #2 do this?
It's surely the intention. I just don't know how it works with things
like servo state shared across all of the ports.
This needs separation of struct clock into two
structures, as some fields are per-PTP clock and some are per-PHC.
I have a patch that does this but it needs to be rebased.
You mean, the phc2sys 'struct clock', I guess?
I really mean ptp4l.
Please post the patch, so we see what you are talking about.
See below. It's very outdated, does not apply and is untested. I also
don't like the structure name (although wasn't able to come up with
anything better). It depends on a few more patches but those basically
do some code reorganization only and should not be crucial to
understanding of the intention.
I'm not sure whether this patch was enough to support the boundary
clock or more was needed but I remember I had the boundary clock stuff
done (though untested) and I cannot find anything on top of this, so
this was probably enough.
I think this is a prerequisite to having a boundary clock that uses
multiple PHCs, I cannot imagine how it could work without this
separation.
So, in the current form, what does the 'automatic' mode (I mean, -a
without -r) provide? Nothing useful? We can't do a release with half
baked stuff like that.
-a without -r can indeed be considered as not much useful with the
current code. I intended to send the rebased support for boundary
clock shortly afterwards but as the review was much slower than
I anticipated, I ran out of time allocated for this.
I wouldn't call that half baked but if you want to have the boundary
clock support before a new release, I can try to get some time to work
on this. If it's really needed--maybe your patchset works okay, I admit
I'm not 100% sure, I would have to spend some time looking into the
code, my original patchset is obsolete, the patches it depended on were
redesigned heavily.
Thanks,
Jiri
commit abf3fde8ff5878b3aeab6ecafeae951ca0d6b876
Author: Jiri Benc jb...@redhat.com
Date: Wed Dec 4 16:35:22 2013 +0100
Support for multiple physical clocks
Add support for multiple physical clocks. The clock term refers to the PTP
node (as defined in IEEE 1588), the physical clock term refers to the
piece of hardware that keeps time.
With this patch, ptp4l still represents a single clock (= single PTP node).
The ports it handles can have separate physical clocks attached, though.
Obviously, such physical clocks have to be synchronized. For now, it is the
responsiblity of the amdinistrator to set up phc2sys to synchronize the
physical clocks in the correct direction. The ultimate plan is to modify
phc2sys to do this automatically.
Signed-off-by: Jiri Benc jb...@redhat.com
diff --git a/clock.c b/clock.c
index 1bdf18445b13..e70d58f9eb84 100644
--- a/clock.c
+++ b/clock.c
@@ -57,9 +57,19 @@ struct clock_stats {
unsigned int max_count;
};
-struct clock {
+struct phys_clock {
+ LIST_ENTRY(phys_clock) list;
+ struct clock *clock;
clockid_t clkid;
struct servo *servo;
+ enum servo_state servo_state;
+ struct clockcheck *sanity_check;
+ int nports;
+ int phc_index;
+};
+
+struct clock {
+ LIST_HEAD(phys_head, phys_clock) phys;
struct defaultDS dds;
struct dataset default_dataset;
struct currentDS cur;
@@ -79,10 +89,11 @@ struct clock {
int utc_timescale;
int leap_set;
int kernel_leap;
+ int sanity_freq_limit;
+ enum servo_type default_servo;
int utc_offset; /* grand master role */
int time_flags; /* grand master role */
int time_source; /* grand master role */
- enum servo_state servo_state;
tmv_t master_offset;
tmv_t path_delay;
struct filter *delay_filter;
@@ -96,7 +107,6 @@ struct clock {
struct clock_description desc;
struct clock_stats stats;
int stats_interval;
- struct clockcheck *sanity_check;
};
struct clock the_clock;
@@ -126,10 +136,6 @@ void clock_destroy(struct clock *c)
}
port_close(c-uds_port);
free(c-pollfd);
- if (c-clkid != CLOCK_REALTIME) {
- phc_close(c-clkid);
- }
- servo_destroy(c-servo);
filter_destroy(c-delay_filter);
stats_destroy(c-stats.offset);
stats_destroy(c-stats.freq);
@@ -488,8 +494,9 @@ static void clock_update_slave(struct clock *c)
}
}
-static int clock_utc_correct(struct clock *c, tmv_t ingress)
+static int clock_utc_correct(struct phys_clock *pc, tmv_t ingress)
{
+ struct clock *c = pc-clock;
Re: [Linuxptp-devel] [PATCH RFC 0/3] Poor man's boundary clock
On Wed, Nov 05, 2014 at 08:28:04PM +0100, Jiri Benc wrote: I'm not sure whether this patch was enough to support the boundary clock or more was needed but I remember I had the boundary clock stuff done (though untested) and I cannot find anything on top of this, so this was probably enough. Having looked at the patch, and now I understand what you are saying, and why ptp4l must steer the SLAVE port's clock, duh. -a without -r can indeed be considered as not much useful with the current code. I intended to send the rebased support for boundary clock shortly afterwards but as the review was much slower than I anticipated, I ran out of time allocated for this. Yeah, but now I have time to review :P I wouldn't call that half baked but if you want to have the boundary clock support before a new release, I can try to get some time to work on this. If it's really needed--maybe your patchset works okay, I admit I'm not 100% sure, I would have to spend some time looking into the code, my original patchset is obsolete, the patches it depended on were redesigned heavily. No, my patch isn't going to fix it. But I think there is an easier way than what you posted. We only need to push the clockid into the port structure and then pass it along into clock_synchronize(). Well, I will take a closer look anyhow... Thanks, Richard -- ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 0/3] Poor man's boundary clock
On Sat, 2014-11-01 at 20:00 +0100, Richard Cochran wrote: Before releasing v1.5, I wanted to test phc2sys's new automatic mode in order to run a BC using a bunch of PCIe cards, but I found it did not quite work. First of all, the port logic bails out when the PHC device don't match, and secondly phc2sys has one line bug. After hacking those two problems away, the BC worked okay. With a GM-BC-slave setup, the slave had about an additional 9 usec offset. I made the jbod a non-default option, so that users who want this really know not to expect perfect performance. After this series and the SO_SELECT_ERR_QUEUE get settled, I will release the 1.5 version. Feedback is most welcome. Thanks, Richard This looks great. Hopefully in a few days I will be able to set this up and see how it works with one of the Intel NICs. Thanks, Jake Richard Cochran (3): config: add a option to enable a poor man's boundary clock. port: allow running a boundary clock with multiple clock devices. phc2sys: make automatic mode actually work. config.c| 13 + config.h|1 + default.cfg |1 + ds.h|1 + gPTP.cfg|1 + phc2sys.c |1 - port.c |6 +- ptp4l.8 | 12 +++- ptp4l.c |1 + 9 files changed, 34 insertions(+), 3 deletions(-) -- ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH RFC 0/3] Poor man's boundary clock
Before releasing v1.5, I wanted to test phc2sys's new automatic mode in order to run a BC using a bunch of PCIe cards, but I found it did not quite work. First of all, the port logic bails out when the PHC device don't match, and secondly phc2sys has one line bug. After hacking those two problems away, the BC worked okay. With a GM-BC-slave setup, the slave had about an additional 9 usec offset. I made the jbod a non-default option, so that users who want this really know not to expect perfect performance. After this series and the SO_SELECT_ERR_QUEUE get settled, I will release the 1.5 version. Feedback is most welcome. Thanks, Richard Richard Cochran (3): config: add a option to enable a poor man's boundary clock. port: allow running a boundary clock with multiple clock devices. phc2sys: make automatic mode actually work. config.c| 13 + config.h|1 + default.cfg |1 + ds.h|1 + gPTP.cfg|1 + phc2sys.c |1 - port.c |6 +- ptp4l.8 | 12 +++- ptp4l.c |1 + 9 files changed, 34 insertions(+), 3 deletions(-) -- 1.7.10.4 -- ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
