On Wed, Nov 14, 2018 at 02:10:12PM -0800, Vedang Patel wrote:
> Here, we are assuming that the gPTP synchronization is achieved whenever
> the last 'n' offsets calculated using the Sync messages are below a
> certain threshold. Both, the number of offsets to consider and the
> offset threshold are configurable.
Idea is okay, but the implementation? See below...
> Four new config options have been added to provide this functionality:
>
> - offset_threshold: All the offset values being considered should be
> below this value.
> - num_offset_values: number of previously received offset
> values to consider.
Prefix these with servo_
> @@ -1796,6 +1796,10 @@ static void handle_state_decision_event(struct clock
> *c)
> clock_update_slave(c);
> event = EV_RS_SLAVE;
> break;
> + /* This is a special case only applicable when BMCA == 'noop'.
> */
> + case PS_UNCALIBRATED:
> + event = EV_NONE;
> + break;
As I said before, we don't want special cases all over the place.
> @@ -2242,6 +2260,18 @@ enum fsm_event process_sync(struct port *p, struct
> ptp_message *m)
> }
> port_syfufsm(p, event, m);
>
> + /*
> + * If the master resets before the slave notices, there will be a
> + * descrepency in the interval. Slave should transition to
> + * PS_UNCALIBRATED to fix this.
> + */
> + if (p->offset_threshold &&
> + m->header.logMessageInterval < p->logSyncInterval) {
> + return EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES;
> + } else if (check_offset_threshold(p)) {
> + return EV_RS_SLAVE;
> + }
another special case, ugh
> @@ -2382,9 +2412,25 @@ static void port_p2p_transition(struct port *p, enum
> port_state next)
> case PS_UNCALIBRATED:
> flush_last_sync(p);
> flush_peer_delay(p);
> - /* fall through */
> + port_set_announce_tmo(p);
> + if (p->offset_threshold) {
> + p->curr_offset_values = p->num_offset_values;
> + p->logPdelayReqInterval = p->logMinPdelayReqInterval;
> + p->logSyncInterval = p->initialLogSyncInterval;
> + port_tx_signaling(p, SIGNAL_NO_CHANGE,
> + SIGNAL_SET_INITIAL,
> + SIGNAL_NO_CHANGE);
> + }
> + break;
> case PS_SLAVE:
> port_set_announce_tmo(p);
> + if (p->offset_threshold) {
> + p->logPdelayReqInterval = p->operLogPdelayReqInterval;
> + p->logSyncInterval = p->operLogSyncInterval;
> + port_tx_signaling(p, SIGNAL_NO_CHANGE,
> + p->operLogSyncInterval,
> + SIGNAL_NO_CHANGE);
and even more special cases.
Let's take a step back and consider the design. What you really want
is a new synchronization state. Something like:
enum servo_state {
SERVO_UNLOCKED,
SERVO_JUMP,
SERVO_LOCKED_ALPHA,
SERVO_LOCKED_BETA,
};
The new code should either go into a new servo type or possibly right
into servo.c. (In either case it should wrap the functionality of the
existing servos.)
When the underlying servo becomes LOCKED, then you enter ALPHA and
start your counter. When the time series of the offset has entered
the acceptable zone, you enter the BETA state.
At the top level (clock.c), on the transition from ALPHA:BETA you send
the signaling message to reduce the Sync rate.
See?
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
