Re: [Linuxptp-devel] [PATCH] phc2sys: Notify kernel if clock is not in sync

2020-03-06 Thread Richard Cochran
On Thu, Mar 05, 2020 at 08:29:03AM +0100, Miroslav Lichvar wrote:
> 
> I'd stick with the meaning "the clock has is believed to have an error
> larger than 16 seconds". With ptp4l/phc2sys, if someone needs to check
> if the clock was updated in the last X seconds, they can check the
> maxerror value if it's not larger than X * 500us. No need to set the
> UNSYNC flag.

Okay, I trust your judgement on this, and I am reverting that commit.

Thanks,
Richard


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


Re: [Linuxptp-devel] [PATCH] phc2sys: Notify kernel if clock is not in sync

2020-03-04 Thread Miroslav Lichvar
On Wed, Mar 04, 2020 at 08:49:45AM -0800, Richard Cochran wrote:
> On Mon, Feb 17, 2020 at 01:46:26PM +0100, Miroslav Lichvar wrote:
> > On Mon, Feb 17, 2020 at 02:34:14PM +0200, Heikkinen, Ville (Nokia - 
> > FI/Espoo) wrote:
> > > (https://www.eecis.udel.edu/~mills/database/reports/kern/kernb.pdf). In
> > > there, it's specified that "STA_UNSYNC set/cleared by the caller to 
> > > indicate
> > > clock unsynchronized (e.g., when no peers are reachable)". So, this is the
> > > reasoning why the flag could be used like this.
> > 
> > Good find.
> > 
> > FWIW, I'm not aware of any software that would unset the flag when the
> 
> you meant, "set", right?

Right.

> 
> > clock stops being updated. That includes software that was maintained
> > by Dr. Mills.
> > 
> > It seems wrong to me to do that, but I'm not strongly opposed.
> 
> I think we can go ahead and give a flag a meaning.  If the kernel
> meant to enforce some other kind of meaning, then it would have had to
> check the value that user space provides.

I'd stick with the meaning "the clock has is believed to have an error
larger than 16 seconds". With ptp4l/phc2sys, if someone needs to check
if the clock was updated in the last X seconds, they can check the
maxerror value if it's not larger than X * 500us. No need to set the
UNSYNC flag.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] phc2sys: Notify kernel if clock is not in sync

2020-03-04 Thread Richard Cochran
On Fri, Feb 14, 2020 at 01:23:44PM +0200, Ville Heikkinen wrote:
> In case there is no connection to the server, notify the kernel
> that the clock is currently unsynchronized.
> 
> Signed-off-by: Ville Heikkinen 

Applied.

Thanks,
Richard


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


Re: [Linuxptp-devel] [PATCH] phc2sys: Notify kernel if clock is not in sync

2020-03-04 Thread Richard Cochran
On Mon, Feb 17, 2020 at 01:46:26PM +0100, Miroslav Lichvar wrote:
> On Mon, Feb 17, 2020 at 02:34:14PM +0200, Heikkinen, Ville (Nokia - FI/Espoo) 
> wrote:
> > (https://www.eecis.udel.edu/~mills/database/reports/kern/kernb.pdf). In
> > there, it's specified that "STA_UNSYNC set/cleared by the caller to indicate
> > clock unsynchronized (e.g., when no peers are reachable)". So, this is the
> > reasoning why the flag could be used like this.
> 
> Good find.
> 
> FWIW, I'm not aware of any software that would unset the flag when the

you meant, "set", right?

> clock stops being updated. That includes software that was maintained
> by Dr. Mills.
> 
> It seems wrong to me to do that, but I'm not strongly opposed.

I think we can go ahead and give a flag a meaning.  If the kernel
meant to enforce some other kind of meaning, then it would have had to
check the value that user space provides.

Thanks,
Richard



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


Re: [Linuxptp-devel] [PATCH] phc2sys: Notify kernel if clock is not in sync

2020-02-17 Thread Heikkinen, Ville (Nokia - FI/Espoo)

On 2/17/20 11:06 AM, Miroslav Lichvar wrote:

On Fri, Feb 14, 2020 at 01:23:44PM +0200, Ville Heikkinen wrote:

In case there is no connection to the server, notify the kernel
that the clock is currently unsynchronized.


Is the idea to use the flag for monitoring phc2sys?


The idea is to monitor clock synchronization status: there could be 
processes running in a container with a limited knowledge of the system. 
Setting the flag would provide an easy way to those processes achieve this.



I'm not sure if that's how the UNSYNC flag is supposed to be used.
When the clock stops being updated by phc2sys, it doesn't mean it's
"unsynchronized", does it? It will slowly drift away and the kernel
will set the flag automatically when the maximum error (rising at 500
ppm) reaches 16 seconds.


It's true that the kernel sets the flag after a while, but it seems that 
in some scenarios this takes too long.


It's not that clear that how the interfaces should be used, and the 
specification is 25 years old. In any case, the interface is specified 
in RFC 1589. It doesn't give much info how the interface should be used. 
However, the author of the RFC, Mills, D.L., has written article "Unix 
kernel modifications for precision time synchronization" 
(https://www.eecis.udel.edu/~mills/database/reports/kern/kernb.pdf). In 
there, it's specified that "STA_UNSYNC set/cleared by the caller to 
indicate clock unsynchronized (e.g., when no peers are reachable)". So, 
this is the reasoning why the flag could be used like this.


--
Ville Heikkinen


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


Re: [Linuxptp-devel] [PATCH] phc2sys: Notify kernel if clock is not in sync

2020-02-17 Thread Miroslav Lichvar
On Mon, Feb 17, 2020 at 02:34:14PM +0200, Heikkinen, Ville (Nokia - FI/Espoo) 
wrote:
> (https://www.eecis.udel.edu/~mills/database/reports/kern/kernb.pdf). In
> there, it's specified that "STA_UNSYNC set/cleared by the caller to indicate
> clock unsynchronized (e.g., when no peers are reachable)". So, this is the
> reasoning why the flag could be used like this.

Good find.

FWIW, I'm not aware of any software that would unset the flag when the
clock stops being updated. That includes software that was maintained
by Dr. Mills.

It seems wrong to me to do that, but I'm not strongly opposed.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] phc2sys: Notify kernel if clock is not in sync

2020-02-17 Thread Miroslav Lichvar
On Fri, Feb 14, 2020 at 01:23:44PM +0200, Ville Heikkinen wrote:
> In case there is no connection to the server, notify the kernel
> that the clock is currently unsynchronized.

Is the idea to use the flag for monitoring phc2sys?

I'm not sure if that's how the UNSYNC flag is supposed to be used.
When the clock stops being updated by phc2sys, it doesn't mean it's
"unsynchronized", does it? It will slowly drift away and the kernel
will set the flag automatically when the maximum error (rising at 500
ppm) reaches 16 seconds.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] phc2sys: Notify kernel if clock is not in sync

2020-02-14 Thread Ville Heikkinen
In case there is no connection to the server, notify the kernel
that the clock is currently unsynchronized.

Signed-off-by: Ville Heikkinen 
---
 clockadj.c | 11 +++
 clockadj.h |  5 +
 phc2sys.c  |  4 +++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/clockadj.c b/clockadj.c
index 0485d8c..7b8feef 100644
--- a/clockadj.c
+++ b/clockadj.c
@@ -182,3 +182,14 @@ void sysclk_set_sync(void)
if (clock_adjtime(clkid, ) < 0)
pr_err("failed to set clock status and maximum error: %m");
 }
+
+void sysclk_set_unsync(void)
+{
+   clockid_t clkid = CLOCK_REALTIME;
+   struct timex tx;
+   memset(, 0, sizeof(tx));
+   tx.modes = ADJ_STATUS;
+   tx.status = STA_UNSYNC;
+   if (clock_adjtime(clkid, ) < 0)
+   pr_err("failed to set clock status: %m");
+}
diff --git a/clockadj.h b/clockadj.h
index 4ea98c1..8a98627 100644
--- a/clockadj.h
+++ b/clockadj.h
@@ -80,4 +80,9 @@ int sysclk_max_freq(void);
  * the real-time clock (RTC) to it.
  */
 void sysclk_set_sync(void);
+
+/**
+ * Mark the system clock as unsynchronized.
+ */
+void sysclk_set_unsync(void);
 #endif
diff --git a/phc2sys.c b/phc2sys.c
index c0b7b3d..80d0b8e 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -735,8 +735,10 @@ static int do_loop(struct node *node, int subscriptions)
reconfigure(node);
}
}
-   if (!node->master)
+   if (!node->master) {
+   sysclk_set_unsync();
continue;
+   }
 
LIST_FOREACH(clock, >dst_clocks, dst_list) {
if (!update_needed(clock))
-- 
2.17.1



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