On Thu, Apr 15, 2021 at 11:16:00AM +0300, Amar Subramanyam via Linuxptp-devel
wrote:
> This change brings slave clock jbod feature which allows ptp4l to work as a
> Ordinary Subordinate/Slave clock using "just a bunch of devices" that are not
> synchronized to each other but a collection of clocks synchronized by an
> external
> source.The best master will be decided by BMCA among the collection of the
> clocks.
> This feature gets enabled by ptp4l config option: slave_clock_jbod and
> by default it is disabled.
This patch is unacceptable for two reasons...
> bmc.c | 4 ++++
> clock.c | 14 ++++++++++++++
> clock.h | 7 +++++++
> config.c | 1 +
> designated_fsm.c | 4 ++--
> designated_fsm.h | 6 ++++--
> fsm.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
1. Your are changing the core state machines from IEEE 1588. We don't
deviate from the standard, especially not from the critical foundation.
If we do deviate, then only on minor points and only with a very
strong reason. However, you have given no justification at all for
this change.
2. You hack your feature all over the place, like:
if (my_special_flag) { do_my_special_thing(); }
This a both red flag and a code smell. Changes must respect the
architecture. If the architecture needs changes, then you must
change it, step by step.
But you haven't even identified a problem. clientOnly and
boundary_clock_jbod work just fine together:
./ptp4l -m -q --clientOnly=1 --boundary_clock_jbod=1 -i eth6 -i eth4 -i eth3
ptp4l[94622.316]: selected /dev/ptp1 as PTP clock
ptp4l[94622.316]: port 2 (eth4): just a bunch of devices
ptp4l[94622.316]: port 3 (eth3): just a bunch of devices
ptp4l[94622.318]: port 1 (eth6): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[94622.319]: port 2 (eth4): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[94622.320]: port 3 (eth3): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[94622.320]: port 0 (/var/run/ptp4l): INITIALIZING to LISTENING on
INIT_COMPLETE
ptp4l[94622.320]: port 0 (/var/run/ptp4lro): INITIALIZING to LISTENING on
INIT_COMPLETE
ptp4l[94624.185]: port 1 (eth6): new foreign master 00e00c.fffe.bce560-1
ptp4l[94628.224]: selected best master clock 00e00c.fffe.bce560
ptp4l[94628.224]: port 1 (eth6): LISTENING to UNCALIBRATED on RS_SLAVE
ptp4l[94628.825]: selected best master clock 00e00c.fffe.bce560
ptp4l[94629.136]: selected best master clock 00e00c.fffe.bce560
ptp4l[94629.767]: master offset 1619024566521062408 s0 freq +0 path delay
50273
ptp4l[94630.787]: master offset 1619024566520947488 s1 freq -112695 path delay
50273
ptp4l[94631.806]: master offset -5796 s2 freq -118491 path delay 50273
ptp4l[94631.806]: port 1 (eth6): UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
ptp4l[94632.826]: master offset 50923 s2 freq -63510 path delay -555
ptp4l[94633.846]: master offset 899 s2 freq -98258 path delay -555
ptp4l[94634.866]: master offset -15796 s2 freq -114683 path delay 1392
ptp4l[94635.885]: master offset -12462 s2 freq -116088 path delay 27
ptp4l[94636.058]: selected best master clock 00e00c.fffe.bce560
ptp4l[94636.130]: selected best master clock 00e00c.fffe.bce560
ptp4l[94636.905]: master offset -8424 s2 freq -115788 path delay -555
ptp4l[94637.925]: master offset -5263 s2 freq -115154 path delay -555
ptp4l[94638.945]: master offset -2474 s2 freq -113944 path delay -762
ptp4l[94639.965]: master offset -1157 s2 freq -113370 path delay -762
ptp4l[94640.984]: master offset -532 s2 freq -113092 path delay -658
ptp4l[94642.004]: master offset -7 s2 freq -112726 path delay -658
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
