[Linuxptp-devel] [PATCH 2/2] Clock Class Threshold Feature addition for PTP4L

2021-02-14 Thread Karthikkumar V
Implemented code review comments.
This code changes brings in the ability to program the acceptable
clockClass threshold beyond which device will move to holdover/free-run.
Default clockClass threshold is 248.
Example Use-Case
This is needed in the cases where T-SC/T-BC Slave might want to listen
only on PRC clockCLass and anything beyond that might not be acceptible
and would want to go to holdover (with SyncE backup or internal
oscillator).

Signed-off-by: Karthikkumar V 
Signed-off-by: Ramana Reddy 
---
 clock.c | 4 ++--
 config.c| 2 +-
 configs/default.cfg | 2 +-
 port.c  | 7 +--
 ptp4l.8 | 5 +
 5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/clock.c b/clock.c
index 542f409..c40476f 100644
--- a/clock.c
+++ b/clock.c
@@ -969,7 +969,7 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
c->default_dataset.localPriority =
config_get_int(config, NULL, "G.8275.defaultDS.localPriority");
c->max_steps_removed = config_get_int(config, NULL,"maxStepsRemoved");
-   c->clock_class_threshold = config_get_int(config, NULL, 
"clockClassThreshold");
+   c->clock_class_threshold = config_get_int(config, NULL, 
"clock_class_threshold");
 
/* Harmonize the twoStepFlag with the time_stamping option. */
if (config_harmonize_onestep(config)) {
@@ -1660,7 +1660,7 @@ UInteger8 clock_max_steps_removed(struct clock *c)
 
 UInteger8 clock_get_clock_class_threshold(struct clock *c)
 {
-   if(c != NULL){
+   if (c != NULL) {
return c->clock_class_threshold;
}
return CLOCK_CLASS_THRESHOLD_DEFAULT; /* Return Default Value  */
diff --git a/config.c b/config.c
index 41735d3..c840d98 100644
--- a/config.c
+++ b/config.c
@@ -231,6 +231,7 @@ struct config_item config_tab[] = {
GLOB_ITEM_INT("clockAccuracy", 0xfe, 0, UINT8_MAX),
GLOB_ITEM_INT("clockClass", 248, 0, UINT8_MAX),
GLOB_ITEM_STR("clockIdentity", "00..00"),
+   GLOB_ITEM_INT("clock_class_threshold", CLOCK_CLASS_THRESHOLD_DEFAULT, 
6, CLOCK_CLASS_THRESHOLD_DEFAULT),
GLOB_ITEM_ENU("clock_servo", CLOCK_SERVO_PI, clock_servo_enu),
GLOB_ITEM_ENU("clock_type", CLOCK_TYPE_ORDINARY, clock_type_enu),
GLOB_ITEM_ENU("dataset_comparison", DS_CMP_IEEE1588, dataset_comp_enu),
@@ -332,7 +333,6 @@ struct config_item config_tab[] = {
GLOB_ITEM_INT("utc_offset", CURRENT_UTC_OFFSET, 0, INT_MAX),
GLOB_ITEM_INT("verbose", 0, 0, 1),
GLOB_ITEM_INT("write_phase_mode", 0, 0, 1),
-   GLOB_ITEM_INT("clockClassThreshold", CLOCK_CLASS_THRESHOLD_DEFAULT, 6, 
CLOCK_CLASS_THRESHOLD_DEFAULT),
 };
 
 static struct unicast_master_table *current_uc_mtab;
diff --git a/configs/default.cfg b/configs/default.cfg
index 473bbb9..e863239 100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -29,7 +29,7 @@ logMinDelayReqInterval0
 logMinPdelayReqInterval0
 operLogPdelayReqInterval 0
 announceReceiptTimeout 3
-clockClassThreshold248
+clock_class_threshold  248
 syncReceiptTimeout 0
 delayAsymmetry 0
 fault_reset_interval   4
diff --git a/port.c b/port.c
index ae2a00e..024370f 100644
--- a/port.c
+++ b/port.c
@@ -1861,8 +1861,11 @@ int process_announce(struct port *p, struct ptp_message 
*m)
return result;
}
 
-   /* If the clock class is greater than clock_class_threshold , ignore 
this master */
-   if(m->announce.grandmasterClockQuality.clockClass > 
clock_get_clock_class_threshold(p->clock)){
+   if (m->announce.grandmasterClockQuality.clockClass >
+ clock_get_clock_class_threshold(p->clock)) {
+   pl_err(60, "%s: Master clock quality received is "
+  "greater than configured, ignoring master!",
+  p->log_name);
return result;
}
 
diff --git a/ptp4l.8 b/ptp4l.8
index 260aae3..b13471b 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -455,6 +455,11 @@ message is greater than or equal to the value of 
maxStepsRemoved the
 Announce message is not considered in the operation of the BMCA.
 The default value is 255.
 .TP
+.B clock_class_threshold
+The maximum clock class value from master, acceptible to sub-ordinate
+clock beyond which it moves out of lock state.
+The default value is 248.
+.TP
 
 .B domainNumber
 The domain attribute of the local clock.
-- 
1.8.3.1



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH v3 1/1] clock: Introduce step_window to free run x Sync events after after a clock step.

2021-02-14 Thread Richard Cochran
Vincent,

This is shaping up nicely.  I only have a few nits to pick...

On Sun, Feb 14, 2021 at 05:22:09PM -0500, vincent.cheng...@renesas.com wrote:

> @@ -1193,6 +1195,9 @@ struct clock *clock_create(enum clock_type type, struct 
> config *config,
>   return NULL;
>   }
>  
> + c->step_window = config_get_int(config, NULL, "step_window");
> + c->step_window_counter = 0;

No need to initialize to zero.

> @@ -1695,6 +1700,14 @@ int clock_switch_phc(struct clock *c, int phc_index)
>   return 0;
>  }
>  
> +static void clock_step_window(struct clock *c)
> +{
> + if (!c->step_window)
> + return;

Please always include curly brackets:  if (test) { ... }

> +
> + c->step_window_counter = c->step_window;
> +}
> +
>  static void clock_synchronize_locked(struct clock *c, double adj)
>  {
>   clockadj_set_freq(c->clkid, -adj);
> @@ -1730,7 +1743,9 @@ enum servo_state clock_synchronize(struct clock *c, 
> tmv_t ingress, tmv_t origin)
>  
>   c->cur.offsetFromMaster = tmv_to_TimeInterval(c->master_offset);
>  
> - if (c->free_running) {
> + if (c->free_running || (c->step_window_counter)) {

No need for parenthesis here --^

> + if (c->step_window_counter)
> + c->step_window_counter--;

Curly brackets, please.

>   return clock_no_adjust(c, ingress, origin);
>   }

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] [PATCH v3 1/1] clock: Introduce step_window to free run x Sync events after after a clock step.

2021-02-14 Thread vincent.cheng.xh
From: Vincent Cheng 

When clock stepping is unable to happen instantaneously the subsequent
timestamps after a clock step does not reflect the step result and
undesired clock freq and step adjustments will occur.

When using ts2phc to synchronize timestamping clock using external
1 PPS, it could take up to 1 second for the timestamps to reflect the
clock step.

step_window, when set, indicates the number of Sync events after a
clock step in which the clock servo will not do any frequency or
step adjustments.

Signed-off-by: Vincent Cheng 
---
 clock.c  | 18 +-
 config.c |  1 +
 ptp4l.8  |  8 
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index a66d189..02b1d55 100644
--- a/clock.c
+++ b/clock.c
@@ -132,6 +132,8 @@ struct clock {
struct interface *udsif;
LIST_HEAD(clock_subscribers_head, clock_subscriber) subscribers;
struct monitor *slave_event_monitor;
+   int step_window;
+   int step_window_counter;
 };
 
 struct clock the_clock;
@@ -1193,6 +1195,9 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
return NULL;
}
 
+   c->step_window = config_get_int(config, NULL, "step_window");
+   c->step_window_counter = 0;
+
/* Create the ports. */
STAILQ_FOREACH(iface, >interfaces, list) {
if (clock_add_port(c, phc_device, phc_index, timestamping, 
iface)) {
@@ -1695,6 +1700,14 @@ int clock_switch_phc(struct clock *c, int phc_index)
return 0;
 }
 
+static void clock_step_window(struct clock *c)
+{
+   if (!c->step_window)
+   return;
+
+   c->step_window_counter = c->step_window;
+}
+
 static void clock_synchronize_locked(struct clock *c, double adj)
 {
clockadj_set_freq(c->clkid, -adj);
@@ -1730,7 +1743,9 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t 
ingress, tmv_t origin)
 
c->cur.offsetFromMaster = tmv_to_TimeInterval(c->master_offset);
 
-   if (c->free_running) {
+   if (c->free_running || (c->step_window_counter)) {
+   if (c->step_window_counter)
+   c->step_window_counter--;
return clock_no_adjust(c, ingress, origin);
}
 
@@ -1747,6 +1762,7 @@ enum servo_state clock_synchronize(struct clock *c, tmv_t 
ingress, tmv_t origin)
case SERVO_JUMP:
clockadj_set_freq(c->clkid, -adj);
clockadj_step(c->clkid, -tmv_to_nanoseconds(c->master_offset));
+   clock_step_window(c);
c->ingress_ts = tmv_zero();
if (c->sanity_check) {
clockcheck_set_freq(c->sanity_check, -adj);
diff --git a/config.c b/config.c
index d237de9..65be360 100644
--- a/config.c
+++ b/config.c
@@ -302,6 +302,7 @@ struct config_item config_tab[] = {
GLOB_ITEM_INT("slaveOnly", 0, 0, 1),
GLOB_ITEM_INT("socket_priority", 0, 0, 15),
GLOB_ITEM_DBL("step_threshold", 0.0, 0.0, DBL_MAX),
+   GLOB_ITEM_INT("step_window", 0, 0, INT_MAX),
GLOB_ITEM_INT("summary_interval", 0, INT_MIN, INT_MAX),
PORT_ITEM_INT("syncReceiptTimeout", 0, 0, UINT8_MAX),
GLOB_ITEM_INT("tc_spanning_tree", 0, 0, 1),
diff --git a/ptp4l.8 b/ptp4l.8
index b179b81..51024cf 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -694,6 +694,14 @@ one-second offset slowly by changing the clock frequency 
(unless the
 option is set to correct such offset by stepping).
 Relevant only with software time stamping. The default is 1 (enabled).
 .TP
+.B step_window
+When set, indicates the number of Sync events after a clock step that
+the clock will not do any frequency or step adjustments.
+This is used in situations where clock stepping is unable to happen
+instantaneously so there is a lag before the timestamps can settle
+properly to reflect the clock step.
+The default is 0 (disabled).
+.TP
 .B timeSource
 The time source is a single byte code that gives an idea of the kind
 of local clock in use. The value is purely informational, having no
-- 
2.7.4



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] [PATCH v3 0/1] clock: Introduce step_window to free run x Sync events after a clock step.

2021-02-14 Thread vincent.cheng.xh
From: Vincent Cheng 

When clock stepping is unable to happen instantaneously the subsequent
timestamps after a clock step does not reflect the step result and
undesired clock freq and clock steps would occur.

When using ts2phc to synchronize timestamping clock using external
1 PPS, it could take up to 1 second for the timestamps to reflect the
clock step.

step_window, when set, indicates the number of Sync events after
a clock step in which the clock servo will not do any frequency or
step adjustments.

Below example illustrates the problem for 16 PPS when clock step
does not occur before the next set of timestamps are received.

Debug statements were added to show T1 and T2 timestamps and the freq
and step requests at clock_sychronize() for SERVO_JUMP.

ptp4l[255352.651]: selected best master clock 00b0ae.fffe.02e810
ptp4l[255352.651]: port 1: LISTENING to UNCALIBRATED on RS_SLAVE
ptp4l[255352.717]: debug: T1: 1611934788908411436T2: 10341336582
...
ptp4l[255352.904]: master offset -1611934778567080326 s0 freq-128 path 
delay  5436
ptp4l[255352.967]: debug: T1: 1611934789158411436T2: 10591336566
ptp4l[255352.967]: debug: adj freq -159.67232
ptp4l[255352.971]: debug: step 1611934778567080308
...
ptp4l[255353.217]: debug: T1: 1611934789408411436T2: 10841336502
ptp4l[255353.217]: debug: adj freq 0.27648
ptp4l[255353.221]: debug: step 1611934778567080368

At 16 PPS, the packet interval is 0.0625 seconds.

The first step occurs at [255352.971], T2 is around 10 seconds.
The next step occurs at [255353.221], T2 is still around 10 seconds.
In an ideal setup, the clock step would be reflected instantaneously
and the correct T2 should be around 1611934799 seconds.

Below shows result of adding step_window.

Clock step occurs at s1, SERVO_JUMP. s2 is SERVO_LOCKED.

The setup is using ts2phc to synhronize the network PHC to external time stamp 
signal.
The progation delay of the 1 PPS signals have a worst case of 2 seconds.

step_window is set to 0 (default 0), so retains original behavior,
ie. will use subsequent timestamps to calculate next clock adjustments.

logSyncInterval -4
logMinDelayReqInterval  -4
first_step_threshold0.00100
step_threshold  0.01000
step_window 0

ptp4l[3831.568]: port 1: LISTENING to UNCALIBRATED on RS_SLAVE
ptp4l[3831.634]: master offset -1613279867776376096 s0 freq  +0 path delay  
7711
ptp4l[3831.698]: master offset -1613279867776376100 s1 freq -64 path delay  
7707
ptp4l[3831.822]: master offset -1613279867776376124 s0 freq -64 path delay  
7711
ptp4l[3831.884]: master offset -1613279867776376140 s0 freq -64 path delay  
7711
ptp4l[3831.948]: master offset -1613279867776376124 s1 freq+192 path delay  
7703
...
ptp4l[3832.447]: master offset -1613279867776376180 s0 freq-128 path delay  
7707
ptp4l[3832.511]: master offset -1613279867776376166 s1 freq +96 path delay  
7705
ptp4l[3833.572]: rms 587555636859904 max 645311947089248 freq +67785 
+/- 119252 delay  8448 +/- 2020
ptp4l[3834.634]: rms 645311947031904 max 645311947031904 freq   -139 
+/- 360 delay  7711 +/-  21
ptp4l[3835.697]: rms 7237815975126660096 max 8136258558135193600 freq +75045 
+/- 113951 delay -137763073020509120 +/- 496711823640969536
ptp4l[3836.696]: rms 6068654064854322176 max 8136258558135192576 freq +69808 
+/- 117623 delay  8492 +/- 1500
ptp4l[3837.759]: rms 2174226957844811520 max 2174226957844814848 freq  -3409 
+/- 6863 delay  7697 +/-  43
ptp4l[3838.821]: rms 1348149014385676288 max 2174226957844810752 freq-85 
+/-  39 delay -113134119417210480 +/- 265323028344133024
ptp4l[3839.821]: rms 703483405420974336 max 703483405420974336 freq   -199 +/-  
52 delay  7712 +/-  11

The multiple clock steps (s1 - [3831.698], [3831.948], etc.) overlap each other
and causes convergence propblems for higher packet rates like 16 packets per 
second.

Setting step_window to 32 (2 seconds at 16 PPS):

logSyncInterval -4
logMinDelayReqInterval  -4
first_step_threshold0.00100
step_threshold  0.01000
step_window 32

ptp4l[4051.547]: port 1: LISTENING to UNCALIBRATED on RS_SLAVE
ptp4l[4051.676]: master offset -1613280090261264012 s0 freq  +0 path delay  
7723
ptp4l[4051.740]: master offset -1613280090261263998 s1 freq+224 path delay  
7701
ptp4l[4051.926]: master offset -1613280090261264042 s0 freq-256 path delay  
7709
ptp4l[4052.051]: master offset -261264088 s0 freq +10 path delay  
7727
ptp4l[4052.176]: master offset -261264116 s0 freq-384 path delay  7703
..
ptp4l[4053.551]: master offset -261264516 s0 freq-192 path delay  7699
ptp4l[4053.676]: master offset -261264558 s0 freq-192 path delay  7717
ptp4l[4053.801]: master offset   -598 s2 freq-374 path delay  7721
ptp4l[4053.802]: port 1: UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
ptp4l[4053.864]: master offset   

Re: [Linuxptp-devel] [PATCH v2 0/1] clock: Introduce step_window to free run n Sync events after a clock step.

2021-02-14 Thread Vincent Cheng
On Sun, Feb 14, 2021 at 04:30:29AM EST, Luigi 'Comio' Mantellini wrote:
>Hi,
>
>This patch cover my scenario where the timestamper can be aligned once at
>second.
>Can I suggest to a avoid to touch free_running variable and introduce
>another one?

Hi Luigi,

Sure. Fixed in v3 patch.

Vincent



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH v2 0/1] clock: Introduce step_window to free run n Sync events after a clock step.

2021-02-14 Thread Luigi 'Comio' Mantellini
Hi,

This patch cover my scenario where the timestamper can be aligned once at
second.
Can I suggest to a avoid to touch free_running variable and introduce
another one?

Thanks,

Luigi

Il dom 14 feb 2021, 06:55  ha scritto:

> From: Vincent Cheng 
>
> When clock stepping is unable to happen instantaneously the subsequent
> timestamps after a clock step does not reflect the step result and
> undesired clock freq and clock steps would occur.
>
> When using ts2phc to synchronize timestamping clock using external
> 1 PPS, it could take up to 1 second for the timestamps to reflect the
> clock step.
>
> step_window, when set, indicates the number of Sync events after
> a clock step in which the clock servo will not do any frequency or
> step adjustments.
>
> Below example illustrates the problem for 16 PPS when clock step
> does not occur before the next set of timestamps are received.
>
> Debug statements were added to show T1 and T2 timestamps and the freq
> and step requests at clock_sychronize() for SERVO_JUMP.
>
> ptp4l[255352.651]: selected best master clock 00b0ae.fffe.02e810
> ptp4l[255352.651]: port 1: LISTENING to UNCALIBRATED on RS_SLAVE
> ptp4l[255352.717]: debug: T1: 1611934788908411436T2: 10341336582
> ...
> ptp4l[255352.904]: master offset -1611934778567080326 s0 freq-128 path
> delay  5436
> ptp4l[255352.967]: debug: T1: 1611934789158411436T2: 10591336566
> ptp4l[255352.967]: debug: adj freq -159.67232
> ptp4l[255352.971]: debug: step 1611934778567080308
> ...
> ptp4l[255353.217]: debug: T1: 1611934789408411436T2: 10841336502
> ptp4l[255353.217]: debug: adj freq 0.27648
> ptp4l[255353.221]: debug: step 1611934778567080368
>
> At 16 PPS, the packet interval is 0.0625 seconds.
>
> The first step occurs at [255352.971], T2 is around 10 seconds.
> The next step occurs at [255353.221], T2 is still around 10 seconds.
> In an ideal setup, the clock step would be reflected instantaneously
> and the correct T2 should be around 1611934799 seconds.
>
> Below shows result of adding step_window.
>
> Clock step occurs at s1, SERVO_JUMP. s2 is SERVO_LOCKED.
>
> The setup is using ts2phc to synhronize the network PHC to external time
> stamp signal. The progation delay of the 1 PPS signals have a worst case
> of 2 seconds.
>
> step_window is set to 0 (default 0), so retains original behavior,
> ie. will use subsequent timestamps to calculate next clock adjustments.
>
> logSyncInterval -4
> logMinDelayReqInterval  -4
> first_step_threshold0.00100
> step_threshold  0.01000
> step_window 0
>
> ptp4l[3831.568]: port 1: LISTENING to UNCALIBRATED on RS_SLAVE
> ptp4l[3831.634]: master offset -1613279867776376096 s0 freq  +0 path
> delay  7711
> ptp4l[3831.698]: master offset -1613279867776376100 s1 freq -64 path
> delay  7707
> ptp4l[3831.822]: master offset -1613279867776376124 s0 freq -64 path
> delay  7711
> ptp4l[3831.884]: master offset -1613279867776376140 s0 freq -64 path
> delay  7711
> ptp4l[3831.948]: master offset -1613279867776376124 s1 freq+192 path
> delay  7703
> ...
> ptp4l[3832.447]: master offset -1613279867776376180 s0 freq-128 path
> delay  7707
> ptp4l[3832.511]: master offset -1613279867776376166 s1 freq +96 path
> delay  7705
> ptp4l[3833.572]: rms 587555636859904 max 645311947089248 freq
> +67785 +/- 119252 delay  8448 +/- 2020
> ptp4l[3834.634]: rms 645311947031904 max 645311947031904 freq
>  -139 +/- 360 delay  7711 +/-  21
> ptp4l[3835.697]: rms 7237815975126660096 max 8136258558135193600 freq
> +75045 +/- 113951 delay -137763073020509120 +/- 496711823640969536
> ptp4l[3836.696]: rms 6068654064854322176 max 8136258558135192576 freq
> +69808 +/- 117623 delay  8492 +/- 1500
> ptp4l[3837.759]: rms 2174226957844811520 max 2174226957844814848 freq
> -3409 +/- 6863 delay  7697 +/-  43
> ptp4l[3838.821]: rms 1348149014385676288 max 2174226957844810752 freq
> -85 +/-  39 delay -113134119417210480 +/- 265323028344133024
> ptp4l[3839.821]: rms 703483405420974336 max 703483405420974336 freq   -199
> +/-  52 delay  7712 +/-  11
>
> The multiple clock steps (s1 - [3831.698], [3831.948], etc.) overlap each
> other
> and causes convergence propblems for higher packet rates like 16 packets
> per second.
>
> Setting step_window to 32 (2 seconds at 16 PPS):
>
> logSyncInterval -4
> logMinDelayReqInterval  -4
> first_step_threshold0.00100
> step_threshold  0.01000
> step_window 32
>
> ptp4l[4051.547]: port 1: LISTENING to UNCALIBRATED on RS_SLAVE
> ptp4l[4051.676]: master offset -1613280090261264012 s0 freq  +0 path
> delay  7723
> ptp4l[4051.740]: master offset -1613280090261263998 s1 freq+224 path
> delay  7701
> ptp4l[4051.926]: master offset -1613280090261264042 s0 freq-256 path
> delay  7709
> ptp4l[4052.051]: master offset -261264088 s0 freq +10 path delay
> 7727
> ptp4l[4052.176]: master