On Mon, Mar 18, 2019 at 03:51:30PM -0700, Vedang Patel wrote:
> @@ -1123,10 +1123,16 @@ static void port_synchronize(struct port *p,
> c2 = correction_to_tmv(correction2);
> t1c = tmv_add(t1, tmv_add(c1, c2));
>
> + last_state = clock_servo_state(p->clock);
> state = clock_synchronize(p->clock, t2, t1c);
> switch (state) {
> case SERVO_UNLOCKED:
> port_dispatch(p, EV_SYNCHRONIZATION_FAULT, 0);
> + p->logPdelayReqInterval = p->logMinPdelayReqInterval;
> + p->logSyncInterval = p->initialLogSyncInterval;
> + port_tx_interval_request(p, SIGNAL_NO_CHANGE,
> + p->logSyncInterval,
> + SIGNAL_NO_CHANGE);
This sends a message with every Sync. That isn't what we want.
Instead, we should send the request only when entering SERVO_UNLOCKED,
and only if the relevant options are configured.
> break;
> case SERVO_JUMP:
> port_dispatch(p, EV_SYNCHRONIZATION_FAULT, 0);
> @@ -1139,6 +1145,16 @@ static void port_synchronize(struct port *p,
> case SERVO_LOCKED:
> port_dispatch(p, EV_MASTER_CLOCK_SELECTED, 0);
> break;
> + case SERVO_LOCKED_STABLE:
> + if (last_state == SERVO_LOCKED) {
> + p->logPdelayReqInterval = p->operLogPdelayReqInterval;
> + p->logSyncInterval = p->operLogSyncInterval;
> + port_tx_interval_request(p, SIGNAL_NO_CHANGE,
> + p->logSyncInterval,
> + SIGNAL_NO_CHANGE);
In contrast, this one is okay, since it only sends once, and only if
the options are set (otherwise you can't enter SERVO_LOCKED_STABLE).
> + }
> + port_dispatch(p, EV_MASTER_CLOCK_SELECTED, 0);
> + break;
> }
> }
>
> @@ -1613,8 +1629,10 @@ int port_initialize(struct port *p)
> p->localPriority = config_get_int(cfg, p->name,
> "G.8275.portDS.localPriority");
> p->initialLogSyncInterval = config_get_int(cfg, p->name,
> "logSyncInterval");
> p->logSyncInterval = p->initialLogSyncInterval;
> + p->operLogSyncInterval = config_get_int(cfg, p->name,
> "operLogSyncInterval");
> p->logMinPdelayReqInterval = config_get_int(cfg, p->name,
> "logMinPdelayReqInterval");
> p->logPdelayReqInterval = p->logMinPdelayReqInterval;
> + p->operLogPdelayReqInterval = config_get_int(cfg, p->name,
> "operLogPdelayReqInterval");
> p->neighborPropDelayThresh = config_get_int(cfg, p->name,
> "neighborPropDelayThresh");
> p->min_neighbor_prop_delay = config_get_int(cfg, p->name,
> "min_neighbor_prop_delay");
>
> diff --git a/port_private.h b/port_private.h
> index d28ab0712798..bb1d86e08fb5 100644
> --- a/port_private.h
> +++ b/port_private.h
> @@ -115,9 +115,11 @@ struct port {
> UInteger8 transportSpecific;
> UInteger8 localPriority;
> Integer8 initialLogSyncInterval;
> + Integer8 operLogSyncInterval;
> Integer8 logSyncInterval;
> Enumeration8 delayMechanism;
> Integer8 logMinPdelayReqInterval;
> + Integer8 operLogPdelayReqInterval;
> Integer8 logPdelayReqInterval;
> UInteger32 neighborPropDelayThresh;
> int follow_up_info;
> diff --git a/ptp4l.8 b/ptp4l.8
> index 99b085c148c9..fad21bf57724 100644
> --- a/ptp4l.8
> +++ b/ptp4l.8
> @@ -161,6 +161,13 @@ The mean time interval between Sync messages. A shorter
> interval may improve
> accuracy of the local clock. It's specified as a power of two in seconds.
> The default is 0 (1 second).
> .TP
> +.B operLogSyncInterval
> +The mean time interval between Sync messages. This value is only used by the
> +slave device when interval_update_timer is enabled. Slave will send this
> +interval to the master to switch to. This is done via a signaling message
> after
> +interval_update_timer expires. It's specified as a power of two in seconds.
> The
> +default value is 0 (1 second).
> +.TP
> .B logMinDelayReqInterval
> The minimum permitted mean time interval between Delay_Req messages. A
> shorter
> interval makes ptp4l react faster to the changes in the path delay. It's
> @@ -172,6 +179,13 @@ The minimum permitted mean time interval between
> Pdelay_Req messages. It's
> specified as a power of two in seconds.
> The default is 0 (1 second).
> .TP
> +.B operLogPdelayReqInterval
> +The mean time interval between Pdelay Request messages. This value is only
> used
> +by the slave device when interval_update_timer is enabled. Slave will switch
> to
> +the interval specified by this config option after the interval_update_timer
> +expires. It's specified as a power of two in seconds. The default value is 0
> (1
> +second).
> +.TP
> .B announceReceiptTimeout
> The number of missed Announce messages before the last Announce messages
> expires.
> @@ -715,6 +729,22 @@ This will disable source port identity checking for Sync
> and Follow_Up
> messages. This is useful when the announce messages are disabled in the
> master
> and the slave does not have any way to know it's identity. The default is 0
> (disabled).
> +.TP
> +.B servo_num_offset_values
> +The number of offset values calculated in previously received Sync messages
> to
> +consider when adjusting the Sync and Pdelay request intervals. More
> information
> +provided in the description of 'offset_threshold'. The default value is 10.
> +.TP
> +.B servo_offset_threshold
> +This value is used by the slave for adjusting the intervals for Sync and
> Pdelay
> +request messages. The slave will check the last 'num_offset_values' offsets
> and
> +if all those offsets are less than the offset_threshold, it will adjust both
> +the intervals. The Sync interval is adjusted via the signaling mechanism and
> +the pdelay request interval is just adjusted locally. The new values to use
> for
> +sync message intervals and pdelay request intervals can be indicated by
> +operLogSyncInterval and operLogPdelayReqInterval respectively. This mechanism
> +is currently only supported when BMCA == 'noop'. The default
> +value of offset_threshold is 0 (disabled).
>
> .SH UNICAST DISCOVERY OPTIONS
>
> diff --git a/servo.c b/servo.c
> index 8be4b92875de..483ad31487f1 100644
> --- a/servo.c
> +++ b/servo.c
> @@ -17,6 +17,7 @@
> * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> */
> #include <string.h>
> +#include <stdlib.h>
>
> #include "config.h"
> #include "linreg.h"
> @@ -79,6 +80,8 @@ struct servo *servo_create(struct config *cfg, enum
> servo_type type,
> }
>
> servo->first_update = 1;
> + servo->offset_threshold = config_get_int(cfg, NULL,
> "servo_offset_threshold");
> + servo->num_offset_values = config_get_int(cfg, NULL,
> "servo_num_offset_values");
>
> return servo;
> }
> @@ -88,6 +91,19 @@ void servo_destroy(struct servo *servo)
> servo->destroy(servo);
> }
>
> +static int check_offset_threshold(struct servo *s, int64_t offset)
> +{
> + uint64_t abs_offset = abs(offset);
abs() returns an int...
> + if (s->offset_threshold) {
> + if (abs_offset < INT64_MAX && abs_offset < s->offset_threshold
^^^^^^^^^^^^^^^^^^^^^^
... so this test is always true
> + && s->curr_offset_values)
> + s->curr_offset_values--;
> + return s->curr_offset_values ? 0 : 1;
> + }
> + return 0;
> +}
> +
> double servo_sample(struct servo *servo,
> int64_t offset,
> uint64_t local_ts,
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
