This patch doesn't let ptp4l work as a Time Aware Bridge!
On Thu, Jun 01, 2017 at 04:05:57PM +0000, Erik Hons wrote:
> This commit only focuses on time transfer and not BMCA changes.
This change log is totally inadequate. You need to tell us why your
changes are needed. Is something not in compliance with 802.1AS? If
so, what? Or are you adding a new feature? If so, why do we need it?
> clock.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> clock.h | 11 ++++++++
> foreign.h | 2 +-
> port.c | 92 ++++++++++++++++++++++++++++++++++++++++++---------------------
> tmv.h | 16 +++++++++++
> 5 files changed, 171 insertions(+), 32 deletions(-)
This patch is rather large. You are completely re-writing the
synchronization logic without any explanation. Sorry, but that is not
acceptable.
> @@ -1074,6 +1071,7 @@ static void port_syfufsm(struct port *p, enum
> syfu_event event,
> struct ptp_message *m)
> {
> struct ptp_message *syn, *fup;
> + Boolean have_match = FALSE;
This is not a C language type.
> @@ -1370,7 +1389,23 @@ static int port_tx_sync(struct port *p)
> fup->header.control = CTL_FOLLOW_UP;
> fup->header.logMessageInterval = p->logSyncInterval;
> - ts_to_timestamp(&msg->hwts.ts,
> &fup->follow_up.preciseOriginTimestamp);
> + if (port_is_ieee8021as(p) && !clock_is_gm(p->clock)) {
> + tmv_t fup_correction;
> +
> + fup->follow_up.preciseOriginTimestamp =
> tmv_to_Timestamp(
> +
> clock_sync_origin_ts(p->clock));
> + fup_correction = timespec_to_tmv(msg->hwts.ts) -
> +
> clock_sync_upstream_tx_time(p->clock);
> + fup_correction =
> dbl_tmv(round(tmv_dbl(fup_correction) *
> +
> clock_to_gm_rate_ratio(p->clock)));
> + fup_correction +=
> clock_sync_fup_correction(p->clock);
> + fup->header.correction =
> tmv_to_correction(fup_correction);
What? This is totally wrong.
The function, port_tx_sync(), is used by the Boundary Clock code. You
cannot simply paste on the TAB functionality here. There are multiple
differences between a BC and a TAB, one of which is the fact that a BC
transmits new Sync messages on an internal timer, but the TAB simply
forwards received Syncs immediately. This is an important behavioral
difference that affects synchronization performance.
If you want to implement a TAB, please do it correctly. Didn't you
see the series that I sent to you, Erik, personally?
16.Oct'16 └─>[PATCH RFC 0/6] TC proof of concept
16.Oct'16 ├─>[PATCH RFC 1/6] ptp4l: add a command line switch for TC
mode.
16.Oct'16 ├─>[PATCH RFC 2/6] port: make the dispatch and event methods
variable based on clock type.
16.Oct'16 ├─>[PATCH RFC 3/6] port: export a private interface.
16.Oct'16 ├─>[PATCH RFC 4/6] port: share init code, peer delay code,
and helpers
16.Oct'16 ├─>[PATCH RFC 5/6] tc: add the transparent clock
implementation.
16.Oct'16 └─>[PATCH RFC 6/6] p2p_tc: implement a peer to peer
transparent clock.
Actually, I am rather annoyed that you totally ignored those patches
and the TODO list. Instead of implementing a proper TC/TAB, you
disfigured the existing BC in a gross way.
If you really want a TAB, please use the proof of concept as your
starting point.
Thanks,
Richard
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
