[Linuxptp-devel] [PATCH 1/1] Ensure TLV_PORT_STATS_NP statistics uses little endian.

2021-03-23 Thread Erez Geva
As machine byte order may vary.
Ensure TLV_PORT_STATS_NP statistics use defined order.

As most of us use little endian hardware and
 to retain backward compatible with most of us,
 we decide to use little endian for the statistics.
All other TLVs messages remain in network order.

Signed-off-by: Erez Geva 
---
 tlv.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tlv.c b/tlv.c
index 98ef6e1..549f2b9 100644
--- a/tlv.c
+++ b/tlv.c
@@ -128,6 +128,7 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t 
data_len,
int extra_len = 0, len;
uint8_t *buf;
uint16_t u16;
+   int i;
switch (m->id) {
case TLV_CLOCK_DESCRIPTION:
cd = >cd;
@@ -324,6 +325,10 @@ static int mgt_post_recv(struct management_tlv *m, 
uint16_t data_len,
psn = (struct port_stats_np *)m->data;
psn->portIdentity.portNumber =
ntohs(psn->portIdentity.portNumber);
+   for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
+   psn->stats.rxMsgType[i] = 
__le64_to_cpu(psn->stats.rxMsgType[i]);
+   psn->stats.txMsgType[i] = 
__le64_to_cpu(psn->stats.txMsgType[i]);
+   }
extra_len = sizeof(struct port_stats_np);
break;
case TLV_SAVE_IN_NON_VOLATILE_STORAGE:
@@ -349,6 +354,7 @@ bad_length:
 
 static void mgt_pre_send(struct management_tlv *m, struct tlv_extra *extra)
 {
+   int i;
struct defaultDS *dds;
struct currentDS *cds;
struct parentDS *pds;
@@ -436,6 +442,10 @@ static void mgt_pre_send(struct management_tlv *m, struct 
tlv_extra *extra)
psn = (struct port_stats_np *)m->data;
psn->portIdentity.portNumber =
htons(psn->portIdentity.portNumber);
+   for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
+   psn->stats.rxMsgType[i] = 
__cpu_to_le64(psn->stats.rxMsgType[i]);
+   psn->stats.txMsgType[i] = 
__cpu_to_le64(psn->stats.txMsgType[i]);
+   }
break;
}
 }
-- 
2.20.1



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


[Linuxptp-devel] [PATCH 0/1] Ensure TLV_PORT_STATS_NP statistics uses little end

2021-03-23 Thread Erez Geva
* Fourth round
  Sorry, somehow i forgot to compile it. (too many things in paraller).

We use TLV_PORT_STATS_NP localy and over network.
To prevent mismatch when the pmc and the ptp4l runs on mechine using
 different byte order.
We change the statistics to little endian.
We choose little endian since most of us use machines with little endian
 and we want to be backward compatible with most.

Erez Geva (1):
  Ensure TLV_PORT_STATS_NP statistics uses little endian.

 tlv.c | 10 ++
 1 file changed, 10 insertions(+)

-- 
2.20.1



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


Re: [Linuxptp-devel] FYI: a summary scribble on my i210 SDP / PPS-in + 25 MHz ref etc sprees

2021-03-23 Thread Frantisek Rysanek
On 23 Mar 2021 at 10:05, Miroslav Lichvar wrote:
>
> That issue with oscillations around resolution of the clock, I think
> you could try smaller PI constants, or you could switch to the nullf
> servo, which is intended for cases like this.
> 
Yes I've tried fiddling with the P and I contributions and some other 
coefficients that I've found in that area, also with the parameters 
of the linear regression servo - to not much avail, if memory serves. 
I haven't tried the nullf servo - will give it a try when I have 
another opportunity (time and all the boxes together in the test 
rig). 

I did suspect some rounding error in the servo feedback stats/math, 
and I'm aware that there are conversions from the native data type / 
resolution coming out of the i210 HW registers (integer / fixed 
point), to the floating-point data types being used by the LinuxPTP 
servo library - but my relatively superficial conclusion was, that 
those anomalous "outliers" indeed come from the i210 hardware 
(timestamping registers). The offset reported is essentially the raw 
i210 TS register sample taken that particular second, maybe just 
type-converted from s32 to float - but no averaging / smearing / 
filtering on that.  Obviously I cannot see what's happening in the 
silicon - those details under the i210's hood are a bit of a mystery 
to me, and that's probably the way Intel would like them to remain 
:-)

I'm actually a little surprised that the servo typically is able to 
converge to a clean 0 and stay that way, with no random glitches, no 
flapping around the 8ns lsb boundary. I understand that the i210 HW 
does support fractional (below 1ppb) frequency adjustments - so it 
wouldn't be all that surprising to see scenaria where the frequency 
adjustment (which is some weighted average product of past offsets) 
would remain non-zero (fraction of a ppb) for a couple seconds and 
then the cumulative phase offset would exceed 8 ns, the servo would 
get a kick from the TS register and "fractionally creep" the other 
way. This doesn't seem to be the case... 

I have implemented the following trigger in my own code of cmp.c and 
cmp2.c

   if ((offset == 0) && (abs(ppb) < 0.01))
  phcs[i]->aligned_periods++;
   else
  phcs[i]->aligned_periods = 0;

   if (phcs[i]->aligned_periods > 4)   {
  // switch to measurement stage
  phcs[i]->initial_sync_stage = 0;

...etc
I.e. after 4 servo periods where the ppb offset was within +/- 10^-6,
I'd call the servo settled forever.
This is my heuristic based on practical observation that, while not 
entirely settled, there would be some fraction on the order of 10^-1 
or 10^-2 fluttering in the ppb, which would result in an occasional 
non-zero phase offset - but eventually I'd get the ppb float residues 
way below 10^-6 and they'd stay that way forever. Servo converged.
Which is where I'd start to take my measurements.
I was certainly hoping for this to happen while I was building my 
25MHz synth :-)

Then afterwards, when I distilled that "anomaly of not settling",
I also tried just nailing ppb to 0 the moment when phase offset 
becomes 0 - this is still present in a commented section at line 625 
in my i210_ext_pps.c .
And, it didn't work. In the anomalous state, despite the ppb being 
bolted to a clean 0 and written to the PHC, the phase offset (TS 
register output) would run at 0 for a couple samples, then give a 
shot of 8 ns, then probably return to zero...

Actually, I've spotted a similar behavior on part of the linreg 
servo, possibly as an inherent effect of the algorithm. After a 
couple dozen samples with phase offset = 0 (and ppb fluttering on the 
order of 0.1 to 0.01), the ppb value suddenly drops to a pure 0.
(This doesn't happen with the PI, where I can always see some debris 
reported below 10^-14 ppb if I count the zeroes correctly.)

While looking for a possible explanation, in the new v3.1 I've found 
SERVO_LOCKED_STABLE and servo_offset_threshold / 
servo_num_offset_values (config parameters). This logic is absent in 
v1.8, and likely not related to the behavior of the linreg servo 
(converging to pure zero ppb).
And I'm wondering if I should add SERVO_LOCKED_STABLE to the switch 
case that does clockadj_set_freq() in my proggies :-)  So far I only 
check for SERVO_LOCKED .  == I'll have to return to that, even though 
SERVO_LOCKED_STATE is possibly quite a corner case, with the 
servo_offset_threshold configured for 0 by default.

I hope to check that stuff again in a few weeks.

Frank



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


Re: [Linuxptp-devel] Unable to have UNCALIBRAT to SLAVE transition

2021-03-23 Thread Richard Cochran
On Mon, Mar 15, 2021 at 12:38:58PM +0100, Luigi 'Comio' Mantellini wrote:
> Inside port_synchronize() I noticed this:
> 
> case SERVO_LOCKED:
> port_dispatch(p, EV_MASTER_CLOCK_SELECTED, 0);
> break;
> case SERVO_LOCKED_STABLE:
> message_interval_request(p, last_state, sync_interval);
> break;
> 
> Supposing to have the port X as SLAVE with "SERVO_LOCKED_STABLE" state,
> after a while I disable the ingoing traffic (SYNC/ANNUNCE) to the port X,
> switching to another port Y still keeping the SERVO_LOCKED_STABLE
> condition. The check_offset_threshold(), called by sample_sample(), should
> always return "1' (true) because the s->curr_offset_values == 0  (and it is
> fixed to servo_num_offset_values only in SERV_UNLOCKED and SERVO_JUMP
> conditions).
> 
> In these conditions the SERVO_LOCKED will not happen and the
> port_dispatach(p, EV_MASTER_CLOCK_SELECTED, 0) should never be called,
> resulting in a forever "UNCALIBRATED" condition.

Does changing port_synchronize() to this fix the issue for you?

case SERVO_LOCKED_STABLE:
message_interval_request(p, last_state, sync_interval);
+   port_dispatch(p, EV_MASTER_CLOCK_SELECTED, 0);
break;

Thanks,
Richard


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


Re: [Linuxptp-devel] [PATCH 1/1] Ensure TLV_PORT_STATS_NP statistics uses little endian.

2021-03-23 Thread Richard Cochran
On Mon, Mar 15, 2021 at 04:58:09PM +0100, Erez Geva wrote:

> diff --git a/tlv.c b/tlv.c
> index 98ef6e1..6e919e6 100644
> --- a/tlv.c
> +++ b/tlv.c
> @@ -324,6 +324,10 @@ static int mgt_post_recv(struct management_tlv *m, 
> uint16_t data_len,
>   psn = (struct port_stats_np *)m->data;
>   psn->portIdentity.portNumber =
>   ntohs(psn->portIdentity.portNumber);
> + for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
> + psn->stats.rxMsgType[i] = 
> le64_to_cpu(psn->stats.rxMsgType[i]);
> + psn->stats.txMsgType[i] = 
> le64_to_cpu(psn->stats.txMsgType[i]);
> + }

Thanks for taking this approach.  I'm getting build errors.  Could you
please take a look?

/home/richard/git/linuxptp/tlv.c: In function ‘mgt_post_recv’:
/home/richard/git/linuxptp/tlv.c:327:8: error: ‘i’ undeclared (first use in 
this function)
   for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
^
/home/richard/git/linuxptp/tlv.c:327:8: note: each undeclared identifier is 
reported only once for each function it appears in
/home/richard/git/linuxptp/tlv.c:328:30: error: implicit declaration of 
function ‘le64_to_cpu’; did you mean ‘le64toh’? 
[-Werror=implicit-function-declaration]
psn->stats.rxMsgType[i] = le64_to_cpu(psn->stats.rxMsgType[i]);
  ^~~
  le64toh
/home/richard/git/linuxptp/tlv.c: In function ‘mgt_pre_send’:
/home/richard/git/linuxptp/tlv.c:443:8: error: ‘i’ undeclared (first use in 
this function)
   for (i = 0 ; i < MAX_MESSAGE_TYPES; i++) {
^
/home/richard/git/linuxptp/tlv.c:444:30: error: implicit declaration of 
function ‘cpu_to_le64’; did you mean ‘htole64’? 
[-Werror=implicit-function-declaration]
psn->stats.rxMsgType[i] = cpu_to_le64(psn->stats.rxMsgType[i]);
  ^~~
  htole64

Thanks,
Richard


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


Re: [Linuxptp-devel] [PATCH] Revert "phc2sys: Expand the validation of the PPS mode."

2021-03-23 Thread Richard Cochran
On Mon, Mar 22, 2021 at 05:23:34PM +0100, Miroslav Lichvar wrote:
> Allow the -s option to be used together with the -d option again. The
> PHC is used in the PPS mode to fix the offset by an integer number of
> seconds to get the system clock close to the PHC.
> 
> This reverts commit 228325c1bda4.

Applied.

Thanks,
Richard


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


Re: [Linuxptp-devel] [PATCHv2] Avoid undefined integer operations.

2021-03-23 Thread Richard Cochran
On Mon, Mar 22, 2021 at 05:04:04PM +0100, Miroslav Lichvar wrote:
> This fixes errors reported by the -fsanitize=undefined sanitizer.
> 
> Before accepting the message interval value from a sync message, check
> if it is between -10 and 22, same as required for the delay request
> interval.
> 
> In the calculation of fest/stats/nrate max_count use unsigned 1 to avoid
> an invalid shift by 31.
> 
> In tmv.h operations cast values to uint64_t to avoid signed overflows
> and a left-shift of a negative value.
> 
> Signed-off-by: Miroslav Lichvar 

Applied.

Thanks,
Richard


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


Re: [Linuxptp-devel] [PATCH] pmc: Fix printed totalCorrectionField.

2021-03-23 Thread Richard Cochran
On Mon, Mar 15, 2021 at 11:46:58AM +0100, Miroslav Lichvar wrote:
> The value needs to be shifted to right to get nanoseconds.
> 
> Signed-off-by: Miroslav Lichvar 

Applied.

Thanks,
Richard


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


Re: [Linuxptp-devel] Quation regarding the new 'uds_ro_address'

2021-03-23 Thread Richard Cochran
On Tue, Mar 16, 2021 at 12:38:38PM +, Geva, Erez wrote:
> Can we set the read only file with a group, so we can run quaries 
> without root?

You can change the permissions to whatever you want.

> How about adding a "group" configuration for the uds_ro_address?

I don't think we need yet another config. option for this.  After all,
it is trivial for the sysadmin to script the desired permissions.

Thanks,
Richard


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


Re: [Linuxptp-devel] FYI: a summary scribble on my i210 SDP / PPS-in + 25 MHz ref etc sprees

2021-03-23 Thread Frantisek Rysanek
On 23 Mar 2021 at 8:34, Frank Rysanek wrote:
>
> On 22 Mar 2021 at 21:15, Richard Cochran wrote:
> > 
> > FYI, the new ts2phc program does everything that synbc did, and
> > more.
> > 
> > Thanks,
> > Richard
> 
> Thanks for the tip, I'm gonna have to try that :-)
> 
...allright, reading through the manpage, it's tugging at my sleeve 
that my next project in this vein would be a DIY Boundary Clock (sort 
of - not necessarily a bridge and certainly not a HW switch) that 
would contain a custom 25 MHz VC-OCXO in the servo loop :-)
But this is probably gonna have to wait, as I have no immediate use 
case for that...

Frank


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


Re: [Linuxptp-devel] FYI: a summary scribble on my i210 SDP / PPS-in + 25 MHz ref etc sprees

2021-03-23 Thread Miroslav Lichvar
On Mon, Mar 22, 2021 at 06:35:48PM +0100, Frantisek Rysanek wrote:
> Anyway the time has come for me to post a link.
> Here goes my newest scribble on the aforementioned topics:
> 
> http://support.fccps.cz/industry/i210_hacks/i210_hacks.html

Interesting stuff. Thanks for the post.

That issue with oscillations around resolution of the clock, I think
you could try smaller PI constants, or you could switch to the nullf
servo, which is intended for cases like this.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] FYI: a summary scribble on my i210 SDP / PPS-in + 25 MHz ref etc sprees

2021-03-23 Thread Frantisek Rysanek
On 22 Mar 2021 at 21:15, Richard Cochran wrote:
> 
> FYI, the new ts2phc program does everything that synbc did, and more.
> 
> Thanks,
> Richard

Thanks for the tip, I'm gonna have to try that :-)

Frank


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