Re: [Linuxptp-devel] [PATCH v3 4/6] Add a push notification for the CMLDS TLV.

2023-12-05 Thread Miroslav Lichvar
On Mon, Dec 04, 2023 at 11:25:00PM +0100, Andrew Zaborowski wrote:
> On Sun, 3 Dec 2023 at 00:39, Richard Cochran  wrote:
> > @@ -306,13 +307,15 @@ static void do_set_action(struct pmc *pmc, int 
> > action, int index, char *str)
> >  "duration  %hu "
> >  "NOTIFY_PORT_STATE %3s "
> >  "NOTIFY_TIME_SYNC  %3s "
> > -"NOTIFY_PARENT_DATA_SET %3s ",
> > +"NOTIFY_PARENT_DATA_SET %3s "
> > +"NOTIFY_CMLDS %3s ",
> >  &sen.duration,
> >  onoff_port_state,
> >  onoff_time_status,
> > -onoff_parent_data_set);
> > -   if (cnt != 4) {
> > -   fprintf(stderr, "%s SET needs 4 values\n",
> > +onoff_parent_data_set,
> > +onoff_cmlds);
> > +   if (cnt != 5) {
> > +   fprintf(stderr, "%s SET needs 5 values\n",
> 
> Doing cnt != 4 && cnt != 5 should just work as sscanf() should stop
> parsing when it doesn't find NOTIFY_CMLDS and leave onoff_cmlds as
> "off".  Obviously this won't scale as more events are added but it'd
> keep syntax backwards-compatible for scripts.

Good idea. The PARENT_DATA_SET notification is new too (added after
4.1) and I guess there is no reason to require even the original two
notifications, so change that to "cnt < 1" with "%s SET needs at least
1 value", requiring only the duration, which would effectively reset
all existing notifications?

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v3 5/6] Implement the COMMON_P2P delay mechanism.

2023-12-04 Thread Miroslav Lichvar
On Mon, Dec 04, 2023 at 04:06:31PM +0100, Miroslav Lichvar wrote:
> On Sat, Dec 02, 2023 at 03:38:18PM -0800, Richard Cochran wrote:
> > --- a/config.c
> > +++ b/config.c
> > @@ -171,6 +171,7 @@ static struct config_enum delay_filter_enu[] = {
> >  
> >  static struct config_enum delay_mech_enu[] = {
> > { "Auto", DM_AUTO },
> > +   { "COMMON_P2P", DM_COMMON_P2P },
> 
> The new option is missing in the ptp4l usage text.

My bad. I just realized there is no new getopt option to be documented.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v3 5/6] Implement the COMMON_P2P delay mechanism.

2023-12-04 Thread Miroslav Lichvar
On Sat, Dec 02, 2023 at 03:38:18PM -0800, Richard Cochran wrote:
> --- a/config.c
> +++ b/config.c
> @@ -171,6 +171,7 @@ static struct config_enum delay_filter_enu[] = {
>  
>  static struct config_enum delay_mech_enu[] = {
>   { "Auto", DM_AUTO },
> + { "COMMON_P2P", DM_COMMON_P2P },

The new option is missing in the ptp4l usage text.

>   { "E2E",  DM_E2E },
>   { "P2P",  DM_P2P },
>   { "NONE", DM_NO_MECHANISM },
> @@ -249,6 +250,11 @@ struct config_item config_tab[] = {
>   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),
> + PORT_ITEM_STR("cmlds.client_address", "/var/run/cmlds_cleint"),

Swapped e and i in "cleint" is not intentional, right? It's the same
in the man page and the example default config.

Other than that, the patchset looks ok to me. I didn't run any tests,
just a quick look at the code. I'll update the pmc test in the
testsuite.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v2] Add support for DELAY_REQ and SYNC packets RX filters

2023-11-29 Thread Miroslav Lichvar
On Tue, Nov 28, 2023 at 03:34:54PM -0800, Richard Cochran wrote:
> On Tue, Nov 28, 2023 at 12:53:38PM +0100, Miroslav Lichvar wrote:
> > However, timestamping only sync messages has an advantage with very
> > large number of clients as they don't have timestamp each other's
> > delay requets and timestamping of sync messages is more reliable.
> 
> Actually, in hardware is it much simpler and more efficient just to
> time stamp every frame.  (The reporting can be selectable if the
> application doesn't care about some frame types)

I think it depends on the hardware design, which impacts its cost. If
all hardware was able to accurately and reliably timestamp all
packets, we wouldn't need PTP. PTP was specifically designed to make
the hardware support as simple/cheap as possible. It separates event
messages from the rest and uses multicast messaging in order to reduce
the required timestamping rate.

Timestamping only sync messages on clients is just another trick to
reduce the required timestamping rate. The hardware can keep just one
timestamp at a time and it doesn't matter that if it takes 20
milliseconds to read it over MDIO. It's good enough for clients, even
if there is a very large number of them on the same communication
path.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v2] Add support for DELAY_REQ and SYNC packets RX filters

2023-11-28 Thread Miroslav Lichvar
On Sun, Nov 26, 2023 at 11:10:05AM -0800, Richard Cochran wrote:
> The Rx filters are applied globally at the device level, but the PTP
> operates at the application level.  This means that the Rx filter are
> shared between applications.  And so you can see that one application
> cannot simply change the shared global settings at run time.

This problem already exists, e.g. with ptpv2-l4-event vs ptpv2-l2-event.
We could say that all hardware that cannot timestamp all packets is
broken, but it's so common that it has to be supported. If the
hardware which cannot timestamp all event messages is very rare, ok,
maybe it's not worth the trouble.

However, timestamping only sync messages has an advantage with very
large number of clients as they don't have timestamp each other's
delay requets and timestamping of sync messages is more reliable.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] Add notification for changes in PARENT_DATA_SET.

2023-11-21 Thread Miroslav Lichvar
On Tue, Nov 21, 2023 at 08:52:30PM -0800, Richard Cochran wrote:
> linuxptp-testsuite test 20-pmc now fails with
> 
>   "SUBSCRIBE_EVENTS_NP SET needs 4 values"
> 
> Can you provide an update?

Done. Thanks.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] Add support for DELAY_REQ and SYNC packets rx_filter

2023-11-20 Thread Miroslav Lichvar
On Tue, Nov 14, 2023 at 06:50:26PM +0300, IlorDash wrote:
> Honestly, I don't know, is it necessary to dynamically switch RX filters,
> If the device supports EVENT RX filters, because with EVENT RX filters it
> can handle all PTP messages. Maybe you could suggest appropriate behaviour
> for utility in that case?

Selectively timestamping only sync messages can be useful in networks
with very large numbers of clients sharing the same communication
path, so they don't need to timestamp each others delay requests and
possibly miss the timestamp of server's sync message.

If the hardware supports the type-specific filters, I think ptp4l
should be able to set it.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] print: Support log level in message tag.

2023-11-14 Thread Miroslav Lichvar
If the string specified by the message_tag option contains "{level}",
replace it with the log level of the message as a number.

This allows users to filter printed log messages by their level.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.8 |  4 +++-
 print.c   | 27 ++-
 ptp4l.8   |  3 ++-
 ts2phc.8  |  6 --
 tz2alt.8  |  6 --
 5 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/phc2sys.8 b/phc2sys.8
index 9a37778..df0608a 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -323,7 +323,9 @@ The default is 6 (LOG_INFO). Same as option
 .TP
 .B message_tag
 The tag which is added to all messages printed to the standard output
-or system log. The default is an empty string (which cannot be set in
+or system log. If the tag contains the string "{level}", it will be replaced
+with the log level of the message as a number.
+The default is an empty string (which cannot be set in
 the configuration file as the option requires an argument).
 Same as option
 .B \-t
diff --git a/print.c b/print.c
index 02bdcd0..7fd9d53 100644
--- a/print.c
+++ b/print.c
@@ -57,9 +57,10 @@ void print_set_verbose(int value)
 
 void print(int level, char const *format, ...)
 {
+   char buf[1024], tag[128], *s;
struct timespec ts;
+   const char *v;
va_list ap;
-   char buf[1024];
FILE *f;
 
if (level > print_level)
@@ -71,19 +72,27 @@ void print(int level, char const *format, ...)
vsnprintf(buf, sizeof(buf), format, ap);
va_end(ap);
 
+   if (message_tag) {
+   snprintf(tag, sizeof(tag), "%s ", message_tag);
+   v = "{level}";
+   s = strstr(tag, v);
+   if (s) {
+   *s = '0' + level;
+   memmove(s + 1, s + strlen(v), strlen(s + strlen(v)) + 
1);
+   }
+   } else {
+   tag[0] = '\0';
+   }
+
if (verbose) {
f = level >= LOG_NOTICE ? stdout : stderr;
-   fprintf(f, "%s[%lld.%03ld]: %s%s%s\n",
+   fprintf(f, "%s[%lld.%03ld]: %s%s\n",
progname ? progname : "",
-   (long long)ts.tv_sec, ts.tv_nsec / 100,
-   message_tag ? message_tag : "", message_tag ? " " : "",
-   buf);
+   (long long)ts.tv_sec, ts.tv_nsec / 100, tag, buf);
fflush(f);
}
if (use_syslog) {
-   syslog(level, "[%lld.%03ld] %s%s%s",
-  (long long)ts.tv_sec, ts.tv_nsec / 100,
-  message_tag ? message_tag : "", message_tag ? " " : "",
-  buf);
+   syslog(level, "[%lld.%03ld] %s%s",
+  (long long)ts.tv_sec, ts.tv_nsec / 100, tag, buf);
}
 }
diff --git a/ptp4l.8 b/ptp4l.8
index 40c66c2..504cc9f 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -705,7 +705,8 @@ The default value is 255.
 .TP
 .B message_tag
 The tag which is added to all messages printed to the standard output or system
-log.
+log. If the tag contains the string "{level}", it will be replaced with the log
+level of the message as a number.
 The default is an empty string (which cannot be set in the configuration file
 as the option requires an argument).
 
diff --git a/ts2phc.8 b/ts2phc.8
index 3c71d47..852a527 100644
--- a/ts2phc.8
+++ b/ts2phc.8
@@ -156,8 +156,10 @@ The default is 9 (90%).
 .TP
 .B message_tag
 The tag which is added to all messages printed to the standard output
-or system log.  The default is an empty string (which cannot be set in
-the configuration file as the option requires an argument).
+or system log.  If the tag contains the string "{level}", it will be replaced
+with the log level of the message as a number.  The default is an empty string
+(which cannot be set in the configuration file as the option requires an
+argument).
 
 .TP
 .B step_threshold
diff --git a/tz2alt.8 b/tz2alt.8
index 66a6605..8cf790b 100644
--- a/tz2alt.8
+++ b/tz2alt.8
@@ -94,8 +94,10 @@ The default is 6 (LOG_INFO).
 .TP
 .B message_tag
 The tag which is added to all messages printed to the standard output
-or system log.  The default is an empty string (which cannot be set in
-the configuration file as the option requires an argument).
+or system log.  If the tag contains the string "{level}", it will be replaced
+with the log level of the message as a number.  The default is an empty string
+(which cannot be set in the configuration file as the option requires an
+argument).
 
 .TP
 .B transportSpecific
-- 
2.41.0



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


Re: [Linuxptp-devel] [PATCH] Add support for DELAY_REQ and SYNC packets rx_filter

2023-11-14 Thread Miroslav Lichvar
On Mon, Nov 13, 2023 at 08:47:48PM +0300, IlorDash wrote:
> I assumed, if these filters are defined in Kernel, they might be used in
> ptp4l utility to fix this. So I added adv_rx_filter config that allows
> to send SIOCSHWTSTAMP ioctl with HWTSTAMP_FILTER_PTP_XXX_SYNC or
> HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ rx filters for all transport levels.

With this dynamic switching of the RX filter on port state changes is
there a reason to not do it by default? Or instead of adding a new
option add it as a new enum to the existing hwts_filter option?

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH v2 2/4] pmc_agent: Make update interval configurable.

2023-11-14 Thread Miroslav Lichvar
Add a new parameter to pmc_agent_subscribe() to specify the maximum
expected interval between pmc_agent_update() calls to avoid gaps in the
subscription coverage when phc2sys is configured with an update rate
smaller than 1 per minute.

Define a minimum update interval of 10 seconds (corresponding to 30
second subscription duration) to avoid unnecessarily frequent renewals
with the default update intervals of phc2sys and ts2phc, but still be
reasonably fast in detecting missing responses.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c   |  2 +-
 pmc_agent.c | 24 ++--
 pmc_agent.h |  5 -
 ts2phc.c|  2 +-
 4 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 7ea6929..3c94673 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -942,7 +942,7 @@ static int auto_init_ports(struct domain *domain)
return -1;
}
 
-   err = pmc_agent_subscribe(domain->agent, 1000);
+   err = pmc_agent_subscribe(domain->agent, 1000, domain->phc_interval);
if (err) {
pr_err("failed to subscribe");
return -1;
diff --git a/pmc_agent.c b/pmc_agent.c
index 370b27b..494c174 100644
--- a/pmc_agent.c
+++ b/pmc_agent.c
@@ -27,16 +27,16 @@
 #include "print.h"
 #include "util.h"
 
-#define PMC_UPDATE_INTERVAL (60 * NS_PER_SEC)
-#define PMC_SUBSCRIBE_DURATION 180 /* 3 minutes */
-/* Note that PMC_SUBSCRIBE_DURATION has to be longer than
- * PMC_UPDATE_INTERVAL otherwise subscription will time out before it is
- * renewed.
- */
+/* The subscription duration needs to be longer than the update interval to be
+   able to process the messages in time without losing notifications. Define
+   a minimum interval to avoid renewing the subscription too frequently. */
+#define UPDATES_PER_SUBSCRIPTION 3
+#define MIN_UPDATE_INTERVAL 10
 
 struct pmc_agent {
struct pmc *pmc;
uint64_t pmc_last_update;
+   uint64_t update_interval;
 
struct defaultDS dds;
bool dds_valid;
@@ -56,7 +56,7 @@ static void send_subscription(struct pmc_agent *node)
struct subscribe_events_np sen;
 
memset(&sen, 0, sizeof(sen));
-   sen.duration = PMC_SUBSCRIBE_DURATION;
+   sen.duration = UPDATES_PER_SUBSCRIPTION * node->update_interval;
event_bitmask_set(sen.bitmask, NOTIFY_PORT_STATE, TRUE);
pmc_send_set_action(node->pmc, MID_SUBSCRIBE_EVENTS_NP, &sen, 
sizeof(sen));
 }
@@ -391,9 +391,12 @@ void pmc_agent_set_sync_offset(struct pmc_agent *agent, 
int offset)
agent->sync_offset = offset;
 }
 
-int pmc_agent_subscribe(struct pmc_agent *node, int timeout)
+int pmc_agent_subscribe(struct pmc_agent *node, int timeout, int interval)
 {
node->stay_subscribed = true;
+   if (interval < MIN_UPDATE_INTERVAL)
+   interval = MIN_UPDATE_INTERVAL;
+   node->update_interval = interval * NS_PER_SEC;
return renew_subscription(node, timeout);
 }
 
@@ -412,7 +415,7 @@ int pmc_agent_update(struct pmc_agent *node)
}
ts = tp.tv_sec * NS_PER_SEC + tp.tv_nsec;
 
-   if (ts - node->pmc_last_update >= PMC_UPDATE_INTERVAL) {
+   if (ts - node->pmc_last_update >= node->update_interval) {
if (node->stay_subscribed) {
renew_subscription(node, 0);
}
@@ -438,7 +441,8 @@ int pmc_agent_is_subscribed(struct pmc_agent *agent)
ts = tp.tv_sec * NS_PER_SEC + tp.tv_nsec;
 
return agent->pmc_last_update > 0 &&
-   ts - agent->pmc_last_update <= PMC_SUBSCRIBE_DURATION * 
NS_PER_SEC;
+   ts - agent->pmc_last_update <= UPDATES_PER_SUBSCRIPTION *
+  agent->update_interval;
 }
 
 bool pmc_agent_utc_offset_traceable(struct pmc_agent *agent)
diff --git a/pmc_agent.h b/pmc_agent.h
index 93b1e8a..ef5552e 100644
--- a/pmc_agent.h
+++ b/pmc_agent.h
@@ -138,9 +138,12 @@ void pmc_agent_set_sync_offset(struct pmc_agent *agent, 
int offset);
  * Subscribes to push notifications of changes in port state.
  * @param agent  Pointer to a PMC instance obtained via @ref 
pmc_agent_create().
  * @param timeout  Transmit and receive timeout in milliseconds.
+ * @param interval Maximum expected interval between @ref pmc_agent_update()
+ * calls in seconds. This value controls the ptp4l subscription
+ * duration.
  * @return Zero on success, negative error code otherwise.
  */
-int pmc_agent_subscribe(struct pmc_agent *agent, int timeout);
+int pmc_agent_subscribe(struct pmc_agent *agent, int timeout, int interval);
 
 /**
  * Polls for push notifications from the local ptp4l service.
diff --git a/ts2phc.c b/ts2phc.c
index 3d3..03c88b1 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -279,7 +279,7 @@ static int ts2phc_auto_init_ports(struct ts2phc_private 
*priv)
return -1;

[Linuxptp-devel] [PATCH v2 3/4] phc2sys: Better indicate domain with realtime clock.

2023-11-14 Thread Miroslav Lichvar
Add a new field to the domain struct to mark the domain which contains
the system realtime clock instead of relying on the fact that the domain
doesn't have a subscribed pmc agent.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 3c94673..9a5a296 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -108,6 +108,7 @@ struct domain {
int kernel_leap;
int state_changed;
int free_running;
+   int has_rt_clock;
struct pmc_agent *agent;
int agent_subscribed;
LIST_HEAD(port_head, port) ports;
@@ -492,10 +493,8 @@ static void reconfigure(struct domain *domains, int 
n_domains)
src_domain = &domains[i];
}
 
-   if (!LIST_EMPTY(&domains[i].clocks) &&
-   LIST_FIRST(&domains[i].clocks)->clkid == CLOCK_REALTIME) {
+   if (domains[i].has_rt_clock)
rt_domain = &domains[i];
-   }
}
 
if (n_domains <= 1 || !src_domain) {
@@ -997,6 +996,7 @@ static int auto_init_rt(struct domain *domain, int 
dest_only)
return -1;
clock->dest_only = dest_only;
domain->src_priority = 0;
+   domain->has_rt_clock = 1;
return 0;
 }
 
@@ -1008,7 +1008,7 @@ static int clock_handle_leap(struct domain *domain, 
struct clock *clock,
struct pmc_agent *agent;
 
/* The system clock's domain doesn't have a subscribed agent */
-   agent = domain->agent_subscribed ? domain->agent : 
domain->src_domain->agent;
+   agent = domain->has_rt_clock ? domain->src_domain->agent : 
domain->agent;
 
node_leap = pmc_agent_get_leap(agent);
clock->sync_offset = pmc_agent_get_sync_offset(agent);
-- 
2.41.0



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


[Linuxptp-devel] [PATCH v2 4/4] phc2sys: Stop synchronization when ptp4l stops responding.

2023-11-14 Thread Miroslav Lichvar
phc2sys in the automatic mode waits for ptp4l on start, but doesn't care
if it stopped providing state notifications and responding to UTC offset
requests, e.g. if it stopped, crashed, freezed, or its socket was
removed by a misconfigured ptp4l instance.

To better handle broken communication with ptp4l and make it evident,
if the pmc subscription runs out, log an error message and stop
synchronization in the domain until the pmc agent is subscribed again.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 9a5a296..1745558 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -390,6 +390,11 @@ static int reconfigure_domain(struct domain *domain)
LIST_REMOVE(LIST_FIRST(&domain->dst_clocks), dst_list);
}
 
+   if (!domain->has_rt_clock && !domain->agent_subscribed) {
+   domain->src_clock = NULL;
+   return 0;
+   }
+
LIST_FOREACH(c, &domain->clocks, list) {
if (c->clkid == CLOCK_REALTIME) {
/* If present, it can always be a sink clock */
@@ -803,9 +808,9 @@ static int update_domain_clocks(struct domain *domain)
 
 static int do_loop(struct domain *domains, int n_domains)
 {
+   int i, state_changed, prev_sub;
struct timespec interval;
struct domain *domain;
-   int i, state_changed;
 
/* All domains have the same interval */
interval.tv_sec = domains[0].phc_interval;
@@ -821,6 +826,17 @@ static int do_loop(struct domain *domains, int n_domains)
continue;
}
 
+   prev_sub = domain->agent_subscribed;
+   domain->agent_subscribed =
+   pmc_agent_is_subscribed(domain->agent);
+   if (!domain->has_rt_clock && !domain->agent_subscribed) 
{
+   if (prev_sub) {
+   pr_err("Lost connection to ptp4l #%d", 
i + 1);
+   state_changed = 1;
+   }
+   continue;
+   }
+
if (domain->state_changed) {
state_changed = 1;
 
@@ -880,8 +896,6 @@ static int phc2sys_recv_subscribed(void *context, struct 
ptp_message *msg,
struct port *port;
struct clock *clock;
 
-   domain->agent_subscribed = 1;
-
mgt_id = management_tlv_id(msg);
if (mgt_id == excluded)
return 0;
-- 
2.41.0



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


[Linuxptp-devel] [PATCH v2 0/4] Handle unresponsive ptp4l in phc2sys

2023-11-14 Thread Miroslav Lichvar
v2:
- changed pmc_agent_subscribe() to accept update interval instead of
  subscription duration to simplify the code and keep constants in one place
- reordered and merged patches to better follow the previous change
- improved coding style per suggestions

This series improves the automatic mode of phc2sys to handle
unresponsive ptp4l, e.g. stopped, crashed or accidentaly removed socket.
The first three patches prepare the code for the actual handling
implemented in the fifth patch.

Miroslav Lichvar (4):
  pmc_agent: Add function to check if still subscribed.
  pmc_agent: Make update interval configurable.
  phc2sys: Better indicate domain with realtime clock.
  phc2sys: Stop synchronization when ptp4l stops responding.

 phc2sys.c   | 30 ++
 pmc_agent.c | 37 -
 pmc_agent.h | 13 -
 ts2phc.c|  2 +-
 4 files changed, 63 insertions(+), 19 deletions(-)

-- 
2.41.0



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


[Linuxptp-devel] [PATCH v2 1/4] pmc_agent: Add function to check if still subscribed.

2023-11-14 Thread Miroslav Lichvar
Add function to compare the current time with the time of the last
successful pmc update to determine if ptp4l is still responding.

Signed-off-by: Miroslav Lichvar 
---
 pmc_agent.c | 15 +++
 pmc_agent.h |  8 
 2 files changed, 23 insertions(+)

diff --git a/pmc_agent.c b/pmc_agent.c
index 62d1a86..370b27b 100644
--- a/pmc_agent.c
+++ b/pmc_agent.c
@@ -426,6 +426,21 @@ int pmc_agent_update(struct pmc_agent *node)
return 0;
 }
 
+int pmc_agent_is_subscribed(struct pmc_agent *agent)
+{
+   struct timespec tp;
+   uint64_t ts;
+
+   if (clock_gettime(CLOCK_MONOTONIC, &tp)) {
+   pr_err("failed to read clock: %m");
+   return 0;
+   }
+   ts = tp.tv_sec * NS_PER_SEC + tp.tv_nsec;
+
+   return agent->pmc_last_update > 0 &&
+   ts - agent->pmc_last_update <= PMC_SUBSCRIBE_DURATION * 
NS_PER_SEC;
+}
+
 bool pmc_agent_utc_offset_traceable(struct pmc_agent *agent)
 {
return agent->utc_offset_traceable;
diff --git a/pmc_agent.h b/pmc_agent.h
index 2fb1cc8..93b1e8a 100644
--- a/pmc_agent.h
+++ b/pmc_agent.h
@@ -163,6 +163,14 @@ int pmc_agent_subscribe(struct pmc_agent *agent, int 
timeout);
  */
 int pmc_agent_update(struct pmc_agent *agent);
 
+/**
+ * Checks if last successful subscription did not run out, i.e. ptp4l is still
+ * responding (assuming pmc_agent_update() is called frequently enough).
+ * @param agent  Pointer to a PMC instance obtained via @ref 
pmc_agent_create().
+ * @return   True if subscribed, false otherwise.
+ */
+int pmc_agent_is_subscribed(struct pmc_agent *agent);
+
 /**
  * Tests whether the current UTC offset is traceable.
  * @param agent  Pointer to a PMC instance obtained via @ref 
pmc_agent_create().
-- 
2.41.0



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


Re: [Linuxptp-devel] [PATCH 5/5] phc2sys: Stop synchronization when ptp4l stops responding.

2023-11-14 Thread Miroslav Lichvar
On Thu, Nov 09, 2023 at 10:54:31PM -0800, Richard Cochran wrote:
> On Thu, Oct 26, 2023 at 02:40:11PM +0200, Miroslav Lichvar wrote:
> 
> > @@ -942,8 +957,8 @@ static int auto_init_ports(struct domain *domain)
> > }
> >  
> > err = pmc_agent_subscribe(domain->agent, 1000,
> > - (60 > domain->phc_interval ?
> > -  60 : domain->phc_interval) * 3);
> > + (10 > domain->phc_interval ?
> > +  10 : domain->phc_interval) * 3);
> 
> Would it simplify things just to change the 180,60 constants
> into 30,10 ?

I think this could be simplified by changing pmc_agent_subscribe() to
accept the update interval instead of subscription duration. The
minimum and the magic constant of 3 can be kept there and defined with
macros.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH 4/5] phc2sys: Better indicate domain with realtime clock.

2023-10-26 Thread Miroslav Lichvar
Add a new field to the domain struct to mark the domain which contains
the system realtime clock instead of relying on the fact that the domain
doesn't have a subscribed pmc agent.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 451b85f..d46184e 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -108,6 +108,7 @@ struct domain {
int kernel_leap;
int state_changed;
int free_running;
+   int has_rt_clock;
struct pmc_agent *agent;
int agent_subscribed;
LIST_HEAD(port_head, port) ports;
@@ -492,10 +493,8 @@ static void reconfigure(struct domain *domains, int 
n_domains)
src_domain = &domains[i];
}
 
-   if (!LIST_EMPTY(&domains[i].clocks) &&
-   LIST_FIRST(&domains[i].clocks)->clkid == CLOCK_REALTIME) {
+   if (domains[i].has_rt_clock)
rt_domain = &domains[i];
-   }
}
 
if (n_domains <= 1 || !src_domain) {
@@ -999,6 +998,7 @@ static int auto_init_rt(struct domain *domain, int 
dest_only)
return -1;
clock->dest_only = dest_only;
domain->src_priority = 0;
+   domain->has_rt_clock = 1;
return 0;
 }
 
@@ -1010,7 +1010,7 @@ static int clock_handle_leap(struct domain *domain, 
struct clock *clock,
struct pmc_agent *agent;
 
/* The system clock's domain doesn't have a subscribed agent */
-   agent = domain->agent_subscribed ? domain->agent : 
domain->src_domain->agent;
+   agent = !domain->has_rt_clock ? domain->agent : 
domain->src_domain->agent;
 
node_leap = pmc_agent_get_leap(agent);
clock->sync_offset = pmc_agent_get_sync_offset(agent);
-- 
2.41.0



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


[Linuxptp-devel] [PATCH 3/5] phc2sys: Adjust pmc subscription duration for clock update interval.

2023-10-26 Thread Miroslav Lichvar
The pmc_agent_update() function is called once per clock update. If the
clock update rate configured with -R is smaller than 1 per minute,
adjust the subscription interval to avoid gaps in its coverage.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/phc2sys.c b/phc2sys.c
index 4dc7a9a..451b85f 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -942,7 +942,9 @@ static int auto_init_ports(struct domain *domain)
return -1;
}
 
-   err = pmc_agent_subscribe(domain->agent, 1000, 180);
+   err = pmc_agent_subscribe(domain->agent, 1000,
+ (60 > domain->phc_interval ?
+  60 : domain->phc_interval) * 3);
if (err) {
pr_err("failed to subscribe");
return -1;
-- 
2.41.0



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


[Linuxptp-devel] [PATCH 2/5] pmc_agent: Add function to check if still subscribed.

2023-10-26 Thread Miroslav Lichvar
Add function to compare the current time with the time of the last
successful pmc update to determine if ptp4l is still responding.

Signed-off-by: Miroslav Lichvar 
---
 pmc_agent.c | 15 +++
 pmc_agent.h |  8 
 2 files changed, 23 insertions(+)

diff --git a/pmc_agent.c b/pmc_agent.c
index 59f031d..9261c57 100644
--- a/pmc_agent.c
+++ b/pmc_agent.c
@@ -421,6 +421,21 @@ int pmc_agent_update(struct pmc_agent *node)
return 0;
 }
 
+int pmc_agent_is_subscribed(struct pmc_agent *agent)
+{
+   struct timespec tp;
+   uint64_t ts;
+
+   if (clock_gettime(CLOCK_MONOTONIC, &tp)) {
+   pr_err("failed to read clock: %m");
+   return 0;
+   }
+   ts = tp.tv_sec * NS_PER_SEC + tp.tv_nsec;
+
+   return agent->pmc_last_update > 0 &&
+   ts - agent->pmc_last_update <= agent->subscribe_duration;
+}
+
 bool pmc_agent_utc_offset_traceable(struct pmc_agent *agent)
 {
return agent->utc_offset_traceable;
diff --git a/pmc_agent.h b/pmc_agent.h
index fa53f56..16be614 100644
--- a/pmc_agent.h
+++ b/pmc_agent.h
@@ -165,6 +165,14 @@ int pmc_agent_subscribe(struct pmc_agent *agent, int 
timeout, int duration);
  */
 int pmc_agent_update(struct pmc_agent *agent);
 
+/**
+ * Checks if last successful subscription did not run out, i.e. ptp4l is still
+ * responding (assuming pmc_agent_update() is called frequently enough).
+ * @param agent  Pointer to a PMC instance obtained via @ref 
pmc_agent_create().
+ * @return   True if subscribed, false otherwise.
+ */
+int pmc_agent_is_subscribed(struct pmc_agent *agent);
+
 /**
  * Tests whether the current UTC offset is traceable.
  * @param agent  Pointer to a PMC instance obtained via @ref 
pmc_agent_create().
-- 
2.41.0



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


[Linuxptp-devel] [PATCH 1/5] pmc_agent: Make subscription duration configurable.

2023-10-26 Thread Miroslav Lichvar
Replace the constant 1-minute update interval and 3-minute subscription
duration with a duration parameter of pmc_agent_subscribe().

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c   |  2 +-
 pmc_agent.c | 15 +--
 pmc_agent.h |  4 +++-
 ts2phc.c|  2 +-
 4 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 7ea6929..4dc7a9a 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -942,7 +942,7 @@ static int auto_init_ports(struct domain *domain)
return -1;
}
 
-   err = pmc_agent_subscribe(domain->agent, 1000);
+   err = pmc_agent_subscribe(domain->agent, 1000, 180);
if (err) {
pr_err("failed to subscribe");
return -1;
diff --git a/pmc_agent.c b/pmc_agent.c
index 62d1a86..59f031d 100644
--- a/pmc_agent.c
+++ b/pmc_agent.c
@@ -27,16 +27,10 @@
 #include "print.h"
 #include "util.h"
 
-#define PMC_UPDATE_INTERVAL (60 * NS_PER_SEC)
-#define PMC_SUBSCRIBE_DURATION 180 /* 3 minutes */
-/* Note that PMC_SUBSCRIBE_DURATION has to be longer than
- * PMC_UPDATE_INTERVAL otherwise subscription will time out before it is
- * renewed.
- */
-
 struct pmc_agent {
struct pmc *pmc;
uint64_t pmc_last_update;
+   uint64_t subscribe_duration;
 
struct defaultDS dds;
bool dds_valid;
@@ -56,7 +50,7 @@ static void send_subscription(struct pmc_agent *node)
struct subscribe_events_np sen;
 
memset(&sen, 0, sizeof(sen));
-   sen.duration = PMC_SUBSCRIBE_DURATION;
+   sen.duration = node->subscribe_duration;
event_bitmask_set(sen.bitmask, NOTIFY_PORT_STATE, TRUE);
pmc_send_set_action(node->pmc, MID_SUBSCRIBE_EVENTS_NP, &sen, 
sizeof(sen));
 }
@@ -391,9 +385,10 @@ void pmc_agent_set_sync_offset(struct pmc_agent *agent, 
int offset)
agent->sync_offset = offset;
 }
 
-int pmc_agent_subscribe(struct pmc_agent *node, int timeout)
+int pmc_agent_subscribe(struct pmc_agent *node, int timeout, int duration)
 {
node->stay_subscribed = true;
+   node->subscribe_duration = duration * NS_PER_SEC;
return renew_subscription(node, timeout);
 }
 
@@ -412,7 +407,7 @@ int pmc_agent_update(struct pmc_agent *node)
}
ts = tp.tv_sec * NS_PER_SEC + tp.tv_nsec;
 
-   if (ts - node->pmc_last_update >= PMC_UPDATE_INTERVAL) {
+   if (ts - node->pmc_last_update >= node->subscribe_duration / 3) {
if (node->stay_subscribed) {
renew_subscription(node, 0);
}
diff --git a/pmc_agent.h b/pmc_agent.h
index 2fb1cc8..fa53f56 100644
--- a/pmc_agent.h
+++ b/pmc_agent.h
@@ -138,9 +138,11 @@ void pmc_agent_set_sync_offset(struct pmc_agent *agent, 
int offset);
  * Subscribes to push notifications of changes in port state.
  * @param agent  Pointer to a PMC instance obtained via @ref 
pmc_agent_create().
  * @param timeout  Transmit and receive timeout in milliseconds.
+ * @param duration Subscription duration in seconds. @ref pmc_agent_update()
+ * must be called at least once per third of the interval.
  * @return Zero on success, negative error code otherwise.
  */
-int pmc_agent_subscribe(struct pmc_agent *agent, int timeout);
+int pmc_agent_subscribe(struct pmc_agent *agent, int timeout, int duration);
 
 /**
  * Polls for push notifications from the local ptp4l service.
diff --git a/ts2phc.c b/ts2phc.c
index 3d3..d02de3a 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -279,7 +279,7 @@ static int ts2phc_auto_init_ports(struct ts2phc_private 
*priv)
return -1;
}
 
-   err = pmc_agent_subscribe(priv->agent, 1000);
+   err = pmc_agent_subscribe(priv->agent, 1000, 180);
if (err) {
pr_err("failed to subscribe");
return -1;
-- 
2.41.0



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


[Linuxptp-devel] [PATCH 0/5] Handle unresponsive ptp4l in phc2sys

2023-10-26 Thread Miroslav Lichvar
This series improves the automatic mode of phc2sys to handle
unresponsive ptp4l, e.g. stopped, crashed or accidentaly removed socket.
The first four patches prepare the code for the actual handling
implemented in the fifth patch.

Miroslav Lichvar (5):
  pmc_agent: Make subscription duration configurable.
  pmc_agent: Add function to check if still subscribed.
  phc2sys: Adjust pmc subscription duration for clock update interval.
  phc2sys: Better indicate domain with realtime clock.
  phc2sys: Stop synchronization when ptp4l stops responding.

 phc2sys.c   | 33 +
 pmc_agent.c | 30 --
 pmc_agent.h | 12 +++-
 ts2phc.c|  2 +-
 4 files changed, 57 insertions(+), 20 deletions(-)

-- 
2.41.0



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


[Linuxptp-devel] [PATCH 5/5] phc2sys: Stop synchronization when ptp4l stops responding.

2023-10-26 Thread Miroslav Lichvar
phc2sys in the automatic mode waits for ptp4l on start, but doesn't care
if it stopped providing state notifications and responding to UTC offset
requests, e.g. if it stopped, crashed, freezed, or its socket was
removed by a misconfigured ptp4l instance.

To better handle broken communication with ptp4l and make it evident,
if the pmc subscription runs out, log an error message and stop
synchronization in the domain until the pmc agent is subscribed again.

Also shorten the minimum subscription interval from 180 seconds to 30
seconds to react faster.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index d46184e..f2dc97e 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -390,6 +390,11 @@ static int reconfigure_domain(struct domain *domain)
LIST_REMOVE(LIST_FIRST(&domain->dst_clocks), dst_list);
}
 
+   if (!domain->has_rt_clock && !domain->agent_subscribed) {
+   domain->src_clock = NULL;
+   return 0;
+   }
+
LIST_FOREACH(c, &domain->clocks, list) {
if (c->clkid == CLOCK_REALTIME) {
/* If present, it can always be a sink clock */
@@ -803,9 +808,9 @@ static int update_domain_clocks(struct domain *domain)
 
 static int do_loop(struct domain *domains, int n_domains)
 {
+   int i, state_changed, prev_sub;
struct timespec interval;
struct domain *domain;
-   int i, state_changed;
 
/* All domains have the same interval */
interval.tv_sec = domains[0].phc_interval;
@@ -821,6 +826,18 @@ static int do_loop(struct domain *domains, int n_domains)
continue;
}
 
+   prev_sub = domain->agent_subscribed;
+   domain->agent_subscribed =
+   pmc_agent_is_subscribed(domain->agent);
+   if (!domain->has_rt_clock && !domain->agent_subscribed) 
{
+   if (prev_sub) {
+   pr_err("Lost connection to ptp4l #%d",
+  i + 1);
+   state_changed = 1;
+   }
+   continue;
+   }
+
if (domain->state_changed) {
state_changed = 1;
 
@@ -880,8 +897,6 @@ static int phc2sys_recv_subscribed(void *context, struct 
ptp_message *msg,
struct port *port;
struct clock *clock;
 
-   domain->agent_subscribed = 1;
-
mgt_id = management_tlv_id(msg);
if (mgt_id == excluded)
return 0;
@@ -942,8 +957,8 @@ static int auto_init_ports(struct domain *domain)
}
 
err = pmc_agent_subscribe(domain->agent, 1000,
- (60 > domain->phc_interval ?
-  60 : domain->phc_interval) * 3);
+ (10 > domain->phc_interval ?
+  10 : domain->phc_interval) * 3);
if (err) {
pr_err("failed to subscribe");
return -1;
-- 
2.41.0



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


Re: [Linuxptp-devel] [PATCH 1/1] port: set_tmo_log() timer interval calculation fix

2023-10-10 Thread Miroslav Lichvar
On Tue, Oct 10, 2023 at 12:29:22PM +0200, Paweł Modrzejewski wrote:
> Make it possible to set timeout values bigger than 2.147 sec
> and print error message in case port_set_sync_rx_tmo() fails.

I think this limitation was specific to 32-bit systems that don't use
64-bit time_t.

> --- a/port.c
> +++ b/port.c
> @@ -245,12 +245,9 @@ int set_tmo_log(int fd, unsigned int scale, int 
> log_seconds)
>   for (i = 1, ns = scale * 5ULL; i < log_seconds; i++) {
>   ns >>= 1;
>   }
> - tmo.it_value.tv_nsec = ns;
>  
> - while (tmo.it_value.tv_nsec >= NS_PER_SEC) {
> - tmo.it_value.tv_nsec -= NS_PER_SEC;
> - tmo.it_value.tv_sec++;
> - }
> + tmo.it_value.tv_sec = ns / NS_PER_SEC;
> + tmo.it_value.tv_nsec = ns % NS_PER_SEC;

The fix looks good to me. Replacing the repeated subtraction with
division also fixes a potential slowdown with large scales.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] clock: Downgrade log message about failed uds forward.

2023-09-25 Thread Miroslav Lichvar
If multiple management clients are used in the network and ptp4l
responded at least once over UDS, it will try to forward all management
responses received from network to the last UDS client. ptp4l doesn't
track the messages and doesn't know if they are responses to the UDS
client or other clients in the network. If the UDS client is no longer
running (receiving messages on its address), ptp4l logs "uds port:
management forward failed" error message. With frequent management
requests in the network this can lead to flooding of the system log.

Downgrade the error message to debug to disable it in the default log
level.

Signed-off-by: Miroslav Lichvar 
---
 clock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index 5a64613..74911fd 100644
--- a/clock.c
+++ b/clock.c
@@ -1585,7 +1585,7 @@ static void clock_forward_mgmt_msg(struct clock *c, 
struct port *p, struct ptp_m
   port_log_name(piter));
}
if (clock_do_forward_mgmt(c, p, c->uds_rw_port, msg, 
&msg_ready))
-   pr_err("uds port: management forward failed");
+   pr_debug("uds port: management forward failed");
if (msg_ready) {
msg_post_recv(msg, pdulen);
msg->management.boundaryHops++;
-- 
2.41.0



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


[Linuxptp-devel] [PATCH] phc2sys: Fix -n option with -w.

2023-09-04 Thread Miroslav Lichvar
The domain number used for communication with ptp4l specified by the -n
option is ignored in the non-automatic mode (-w option). Set the domain
number to the last specified value.

Fixes: 417de97d098b ("phc2sys: Add multi-domain synchronization.")
Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/phc2sys.c b/phc2sys.c
index 9d8d42f..7ea6929 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -1494,6 +1494,9 @@ int main(int argc, char *argv[])
if (uds_remote_cnt > 0)
config_set_string(cfg, "uds_address",
  uds_remotes[uds_remote_cnt - 1]);
+   if (domain_number_cnt > 0)
+   config_set_int(cfg, "domainNumber",
+  domain_numbers[domain_number_cnt - 1]);
 
if (init_pmc_node(cfg, domains[0].agent, uds_local,
  phc2sys_recv_subscribed, &domains[0]))
-- 
2.41.0



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


Re: [Linuxptp-devel] [PATCH] phc2sys: Avoid segfault with default UDS address.

2023-09-04 Thread Miroslav Lichvar
On Mon, Sep 04, 2023 at 11:20:57AM -0700, Richard Cochran wrote:
> The recently added phc2sys multi-domain mode introduced a regression.
> In the non-automatic mode (using flag -w to wait for ptp4l) the code
> will attempt to set the UDS address using an uninitialized pointer,
> unless the -z flag is also specified.
> 
> Fix the issue by testing whether the -z flag was present on the
> command line before changing the UDS address.

Looks good to me. Good idea with using the last value if there are
multiple -z options.

However, I noticed the -n option is now ignored with -w. I'll submit a
patch. Sorry for the bugs.

Thanks,

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] bug in phc2sys.c

2023-09-04 Thread Miroslav Lichvar
On Thu, Aug 31, 2023 at 01:40:13PM -0700, Trey Harrison wrote:
> So this morning I downloaded and built the latest code from github and
> found that phc2sys would occasionally segfault on startup, due to the
> following:
> 
> phc2sys.c line 1481:
>   config_set_string(cfg, "uds_address", uds_remotes[0]);
> 
> I fixed it as follows:
> 
> if (uds_remote_cnt>0)
>   config_set_string(cfg, "uds_address", uds_remotes[0]);

Good catch. The default value of the -z option is broken with -w.

The fix looks good to me, except it should have "(uds_remote_cnt > 0)"
to follow the coding style.

Will you submit it as a git-formatted patch with subject and commit
message, or would you prefer me to do it?

Thanks,

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] phc2sys: Improve logging with single domain.

2023-08-29 Thread Miroslav Lichvar
When phc2sys in the automatic mode has only one non-CLOCK_REALTIME
domain, avoid logging the potentially confusing "selecting ... as
out-of-domain source clock" message. The source clock in the domain is
already logged. Also, log a message when CLOCK_REALTIME is selected for
synchronization to match the behavior of phc2sys before it supported
multiple domains.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 2750691..313cdcf 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -476,7 +476,7 @@ static int compare_domains(struct domain *a, struct domain 
*b)
 
 static void reconfigure(struct domain *domains, int n_domains)
 {
-   struct domain *src_domain = NULL;
+   struct domain *src_domain = NULL, *rt_domain = NULL;
int i;
 
pr_info("reconfiguring after port state change");
@@ -491,14 +491,26 @@ static void reconfigure(struct domain *domains, int 
n_domains)
if (compare_domains(src_domain, &domains[i]) > 0) {
src_domain = &domains[i];
}
+
+   if (!LIST_EMPTY(&domains[i].clocks) &&
+   LIST_FIRST(&domains[i].clocks)->clkid == CLOCK_REALTIME) {
+   rt_domain = &domains[i];
+   }
}
 
if (n_domains <= 1 || !src_domain) {
return;
}
 
-   pr_info("selecting %s as out-of-domain source clock",
-   src_domain->src_clock->device);
+   if (rt_domain && src_domain != rt_domain) {
+   pr_info("selecting CLOCK_REALTIME for synchronization");
+   }
+   if (src_domain == rt_domain) {
+   pr_info("selecting CLOCK_REALTIME as source clock");
+   } else if (n_domains - !!rt_domain > 1) {
+   pr_info("selecting %s as out-of-domain source clock",
+   src_domain->src_clock->device);
+   }
 
for (i = 0; i < n_domains; i++) {
if (domains[i].src_clock && domains[i].src_priority > 0)
-- 
2.41.0



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


Re: [Linuxptp-devel] [PATCH v1] pi: Fix drift history when entering SERVO_LOCKED state

2023-08-22 Thread Miroslav Lichvar
On Tue, Aug 22, 2023 at 12:34:35PM +0200, Maciek Machnikowski wrote:
> root@equinox:[~]#: phc_ctl ens6f0np0 set freq 5000
> root@equinox:[~]#: sudo ptp4l -m -i ens6f0np0 -2 -s

> ptp4l[248.614]: master offset -301008874 s0 freq -5000 path delay
> 1136508
> ptp4l[249.615]: master offset -316872413 s1 freq -5000 path delay
> 1460911

You set the clock to run 5% fast, but the change between the offsets
tells us it's running about 1.6% slow. The servo then tries to speed
up the clock even more to correct that error, but it's already at
the maximum value.

The HW or driver is broken. It might need to decrease the frequency
limit. The only reason your change "fixes" the problem is that it
resets the frequency to a non-buggy range.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v1] pi: Fix drift history when entering SERVO_LOCKED state

2023-08-22 Thread Miroslav Lichvar
On Mon, Aug 21, 2023 at 07:02:55PM +0200, Maciek Machnikowski wrote:
> On 8/21/2023 2:04 PM, Miroslav Lichvar wrote:
> > s->drift is the drift of the uncompensated clock. It's not supposed to be
> > scaled by anything.
> If that's the intended use - then the line 147:
> s->drift += ki_term;
> 
> where (line 140)
> ki_term = s->ki * offset * weight;
> 
> is incorrect. s->drift is used as the ki accumulator in the SERVO_LOCKED
> state.

I don't follow. s->drift accumulates the ki term. That's what the code
does and is supposed to do. Maybe you are misinterpreting frequency as
time (integrated frequency)?

> I connected my DUT B2B to the GM and ran the "default" ptp4l profile.
> In this case it wouldn't matter whether I use E2E or P2P, as this is
> just a B2B connection.

The absolute value of the delay doesn't matter. E2E is sensitive to
frequency errors due to long interval between sync and delay
request messages. P2P has much shorter interval and is much less
sensitive to frequency errors.

> To set the offset I used phc_ctl freq 500 (which is the max default)
> 
> I did that test on couple different NICs and in all cases this the clock
> frequency converged faster and with a smaller swing.

Please post some logs. I cannot actually reproduce your improvement
even if I use E2E.
> 
> How did you test it?

I used SW timestamping and P2P mechanism, everything else default.
With HW timestamping I get the same result, but it's less apparent
than with SW (which uses smaller I constant).

Here is my log with the current code:

[105.622] master offset  122869904 s0 freq -50 path delay  8401
[106.623] master offset  123562072 s0 freq -50 path delay  7981
[107.623] master offset  124253734 s0 freq -50 path delay  7561
[108.624] master offset  124938607 s0 freq -50 path delay  7538
[109.625] master offset  125627975 s0 freq -50 path delay  7515
[110.626] master offset  126320033 s0 freq -50 path delay  7538
[111.626] master offset  127013885 s0 freq -50 path delay  7538
[112.627] master offset  127698254 s0 freq -50 path delay  7538
[113.628] master offset  128390502 s0 freq -50 path delay  7708
[114.628] master offset  129080142 s0 freq -50 path delay  8037
[115.629] master offset  129773245 s0 freq -50 path delay  8037
[116.630] master offset  130460300 s0 freq -50 path delay  8037
[117.630] master offset  131148113 s0 freq -50 path delay  8244
[118.631] master offset  131838752 s0 freq -50 path delay  8244
[119.632] master offset  132532898 s0 freq -50 path delay  8244
[120.632] master offset  133219091 s0 freq -50 path delay  8190
[121.633] master offset  133908331 s1 freq +189771 path delay  7983
[122.633] master offset   2239 s2 freq +189997 path delay  7992
[122.633] port 1 (eth0): UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
[123.633] master offset   8287 s2 freq +190610 path delay  7992
[124.633] master offset   2298 s2 freq +190013 path delay  7992
[125.633] master offset210 s2 freq +189805 path delay  7992
[126.633] master offset   -140 s2 freq +189770 path delay  7992
[127.633] master offset   1205 s2 freq +189905 path delay  7887
[128.633] master offset   3860 s2 freq +190175 path delay  7887
[129.633] master offset475 s2 freq +189837 path delay  7464
[130.633] master offset   4924 s2 freq +190287 path delay  7887
[131.633] master offset   2660 s2 freq +190063 path delay  8667
[132.633] master offset   6939 s2 freq +190498 path delay  8633
[133.633] master offset575 s2 freq +189862 path delay  8140
[134.633] master offset909 s2 freq +189896 path delay  8140
[135.633] master offset   -337 s2 freq +189771 path delay  8011
[136.633] master offset   5063 s2 freq +190316 path delay  7622
[137.633] master offset   2403 s2 freq +190053 path delay  7622
[138.633] master offset   2967 s2 freq +190112 path delay  8298
[139.633] master offset  -1035 s2 freq +189711 path delay  8298
[140.633] master offset   1932 s2 freq +190009 path delay  7587
[141.633] master offset   1133 s2 freq +189931 path delay  7587

Here is a log with your change:

[196.603] master offset  140133759 s0 freq -50 path delay  7080
[197.604] master offset  140824960 s0 freq -50 path delay  7011
[198.605] master offset  141519552 s0 freq -50 path delay  6942
[199.605] master offset  142205850 s0 freq -50 path delay  6989
[200.606] master offset  142900936 s0 freq -50 path delay  6942
[201.607] master offset  143589637 s0 freq -50 path delay  6554
[202.608] master offset  144278674 s0 freq -50 path delay  6159
[203.608] master offset  144965917 s0 fre

Re: [Linuxptp-devel] [PATCH v1] pi: Fix drift history when entering SERVO_LOCKED state

2023-08-21 Thread Miroslav Lichvar
On Fri, Aug 18, 2023 at 12:31:47PM +0200, Maciek Machnikowski wrote:
> Current implementation saves s->drift as an offset between current clock
> frequency and the remote clock frequency calculated on first two samples.
> 
> Locked state of the PI servo assumes the s->drift holds the history
> of clock frequency difference scaled by s->ki * weight.

s->drift is the drift of the uncompensated clock. It's not supposed to be
scaled by anything.

> As a result the lock needs more time to stabilize when the frequency
> offset is very large (ex. set to extreme values with phc_ctl) or may
> totally prevent servo from correctly locking.

How exactly did you test it? Have you tried it with the P2P delay
mechanism?

I suspect you are trying to compensate for the error in E2E delay
measured before the frequency is corrected. I think a proper fix would
be to adjust the timestamps from E2E and recalculate the delay before
stepping the clock.

> This patch fixes the s->drift when entering the stable state to hold
> the scaled drift.

That looks wrong to me and in my test with a clock which has a large
frequency error it performs worse.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] PHC Index is invalid in phc2sys when ptp4l is in free_running mode.

2023-07-27 Thread Miroslav Lichvar
On Thu, Jul 27, 2023 at 12:50:03AM +, Arun Saravanan wrote:
> Didn’t realize that Maciek Machnikowski’s proposal for PTP Time of Day 
> proposal is still pending. I have attached here for reference.

Ok, that explains what it's supposed to do. Thanks.

Should another process be steering the clock when ptp4l assumes it's
running free? The rate calculations will be wrong and other things
could potentially break if ptp4l more relied on it, right?

Maybe I'm missing an important detail here, but to me I think it would
make more sense to use the refclock_sock or ntpshm servo to feed
ts2phc instead of the slave event monitor.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] PHC Index is invalid in phc2sys when ptp4l is in free_running mode.

2023-07-26 Thread Miroslav Lichvar
On Tue, Jul 25, 2023 at 06:12:50PM +, Arun Saravanan via Linuxptp-devel 
wrote:
> Use case is –
> ptp4l in free_running mode – PTP syncs the TOD
> PHC disciplined by other means, say PPS
> phc2sys source is PHC.

I'm sorry, I still don't follow.

What exactly is "PTP" and "TOD" here? Is the ptp4l's PTP port in
master or slave state? If former, why would you need
free_running_mode. If latter, what is the point of running ptp4l?

> With autocfg option phc2sys tries to get the phc_index from ptp4l to 
> determine the source. But since ptp4l is free running mode, 
> pmc_agent_query_port_properties will not give the phc_index.

> Ptp4l doesn’t know about PHC since it is not disciplining it.

Well, to me that would seem like the right thing to do. The PHC is not
synchronized, so phc2sys shouldn't be using it for synchronization of
other clocks.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] PHC Index is invalid in phc2sys when ptp4l is in free_running mode.

2023-07-24 Thread Miroslav Lichvar
On Sun, Jul 23, 2023 at 12:09:52PM -0700, Saravanan Arunachalam via 
Linuxptp-devel wrote:
> The pmc call to ptp4l to get phc_index returns invalid value since in 
> free_running mode, ptp4l sets phc_index to -1. Adding a socket call in 
> phc2sys to get the phc_index.

Can you please describe the use case? It's not clear to me why should
be a free-running PHC adjusted or or used as a source clock.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH v1 2/4] phc2sys: Create pmc agent after processing options.

2023-07-20 Thread Miroslav Lichvar
Keep configuration settings in a separate domain and copy them to the
actual domain just before creating the pmc agent. This will be needed to
share the settings between multiple domains.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 43 ---
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index a010f9f..d0f5adb 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -1075,7 +1075,8 @@ int main(int argc, char *argv[])
struct config *cfg;
struct option *opts;
double phc_rate, tmp;
-   struct domain domain = {
+   struct domain domain;
+   struct domain settings = {
.phc_readings = 5,
.phc_interval = 1.0,
};
@@ -1086,10 +1087,6 @@ int main(int argc, char *argv[])
if (!cfg) {
return -1;
}
-   domain.agent = pmc_agent_create();
-   if (!domain.agent) {
-   return -1;
-   }
 
opts = config_long_options(cfg);
 
@@ -1181,22 +1178,21 @@ int main(int argc, char *argv[])
case 'R':
if (get_arg_val_d(c, optarg, &phc_rate, 1e-9, DBL_MAX))
goto end;
-   domain.phc_interval = 1.0 / phc_rate;
+   settings.phc_interval = 1.0 / phc_rate;
break;
case 'N':
-   if (get_arg_val_i(c, optarg, &domain.phc_readings, 1, 
INT_MAX))
+   if (get_arg_val_i(c, optarg, &settings.phc_readings, 1, 
INT_MAX))
goto end;
break;
case 'O':
if (get_arg_val_i(c, optarg, &offset, INT_MIN, 
INT_MAX)) {
goto end;
}
-   pmc_agent_set_sync_offset(domain.agent, offset);
-   domain.forced_sync_offset = -1;
+   settings.forced_sync_offset = -1;
break;
case 'L':
-   if (get_arg_val_i(c, optarg, &domain.sanity_freq_limit, 
0, INT_MAX) ||
-   config_set_int(cfg, "sanity_freq_limit", 
domain.sanity_freq_limit)) {
+   if (get_arg_val_i(c, optarg, 
&settings.sanity_freq_limit, 0, INT_MAX) ||
+   config_set_int(cfg, "sanity_freq_limit", 
settings.sanity_freq_limit)) {
goto end;
}
break;
@@ -1206,7 +1202,7 @@ int main(int argc, char *argv[])
goto end;
break;
case 'u':
-   if (get_arg_val_ui(c, optarg, &domain.stats_max_count,
+   if (get_arg_val_ui(c, optarg, &settings.stats_max_count,
  0, UINT_MAX))
goto end;
break;
@@ -1278,7 +1274,7 @@ int main(int argc, char *argv[])
}
 
if (autocfg && (src_name || dst_cnt > 0 || hardpps_configured(pps_fd) ||
-   wait_sync || domain.forced_sync_offset)) {
+   wait_sync || settings.forced_sync_offset)) {
fprintf(stderr,
"autoconfiguration cannot be mixed with manual config 
options.\n");
goto bad_usage;
@@ -1289,7 +1285,7 @@ int main(int argc, char *argv[])
goto bad_usage;
}
 
-   if (!autocfg && !wait_sync && !domain.forced_sync_offset) {
+   if (!autocfg && !wait_sync && !settings.forced_sync_offset) {
fprintf(stderr,
"time offset must be specified using -w or -O\n");
goto bad_usage;
@@ -1308,14 +1304,23 @@ int main(int argc, char *argv[])
print_set_syslog(config_get_int(cfg, NULL, "use_syslog"));
print_set_level(config_get_int(cfg, NULL, "logging_level"));
 
-   domain.free_running = config_get_int(cfg, NULL, "free_running");
-   domain.servo_type = config_get_int(cfg, NULL, "clock_servo");
-   if (domain.free_running || domain.servo_type == CLOCK_SERVO_NTPSHM) {
+   settings.free_running = config_get_int(cfg, NULL, "free_running");
+   settings.servo_type = config_get_int(cfg, NULL, "clock_servo");
+   if (settings.free_running || settings.servo_type == CLOCK_SERVO_NTPSHM) 
{
config_set_int(cfg, "kernel_leap", 0);
config_set_int(cfg, "sanity_freq_limit", 0);
}
-   domain.kernel_leap = config_get_int(cfg, NULL, "kernel_leap");
-   domain.sanity_freq_limit = config_get_int(cfg, N

[Linuxptp-devel] [PATCH v1 4/4] phc2sys: Add multi-domain synchronization.

2023-07-20 Thread Miroslav Lichvar
Add support for synchronization between clocks controlled by different
ptp4l instances in the automatic mode (-a option) to allow switching to
a backup source on failures.

The -z and -n options can be used multiple times to specify sockets and
domain numbers of different ptp4l instances (up to 16 instances).

Clocks from one ptp4l instance are considered a domain. If the domain
doesn't have a source clock (i.e. a port in the slave state), the clocks
in the domain can be synchronized to a source clock from a different
domain. If multiple domains have a source clock, the first one in the
order of specified sockets is selected. The selection can be later
extended to compare some specific clock attributes.

The real-time clock is a separate domain, which is created only with the
-r option and can be a source for other domains if the -r option is
specified twice.

In the non-automatic modes only one domain is created for all clocks.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.8 |  12 +-
 phc2sys.c | 337 +-
 2 files changed, 241 insertions(+), 108 deletions(-)

diff --git a/phc2sys.8 b/phc2sys.8
index 6c6c7e8..9a37778 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -199,7 +199,12 @@ the direction of the clock synchronization. Not compatible 
with the
 option.
 .TP
 .BI \-n " domain-number"
-Specify the domain number used by ptp4l. The default is 0.
+Specify the domain number used by ptp4l. This option can be used up to 16 times
+to specify different domain numbers for different sockets specified by the
+.B \-z
+option. The domain numbers are assigned according to the order of the options
+on the command line.
+The default is 0.
 .TP
 .B \-x
 When a leap second is announced, don't apply it in the kernel by stepping the
@@ -209,7 +214,10 @@ clock frequency (unless the
 option is used).
 .TP
 .BI \-z " uds-address"
-Specifies the address of the server's UNIX domain socket.
+Specifies the address of the server's UNIX domain socket. This option can be
+used up to 16 times in the automatic mode to synchronize clocks between 
multiple
+.B ptp4l
+instances.
 The default is /var/run/ptp4l.
 .TP
 .BI \-l " print-level"
diff --git a/phc2sys.c b/phc2sys.c
index c1a1c6c..2750691 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -66,6 +66,8 @@
 
 #define MAX_DST_CLOCKS 128
 
+#define MAX_DOMAINS 16
+
 struct clock {
LIST_ENTRY(clock) list;
LIST_ENTRY(clock) dst_list;
@@ -107,10 +109,13 @@ struct domain {
int state_changed;
int free_running;
struct pmc_agent *agent;
+   int agent_subscribed;
LIST_HEAD(port_head, port) ports;
LIST_HEAD(clock_head, clock) clocks;
LIST_HEAD(dst_clock_head, clock) dst_clocks;
struct clock *src_clock;
+   struct domain *src_domain;
+   int src_priority;
 };
 
 static struct config *phc2sys_config;
@@ -372,13 +377,13 @@ static struct clock *find_dst_clock(struct domain *domain,
return c;
 }
 
-static void reconfigure(struct domain *domain)
+static int reconfigure_domain(struct domain *domain)
 {
-   struct clock *c, *rt = NULL, *src = NULL, *last = NULL, *dup = NULL;
+   struct clock *c, *src = NULL, *dup = NULL;
int src_cnt = 0, dst_cnt = 0;
 
-   pr_info("reconfiguring after port state change");
domain->state_changed = 0;
+   domain->src_domain = domain;
 
while (domain->dst_clocks.lh_first != NULL) {
LIST_REMOVE(LIST_FIRST(&domain->dst_clocks), dst_list);
@@ -386,8 +391,10 @@ static void reconfigure(struct domain *domain)
 
LIST_FOREACH(c, &domain->clocks, list) {
if (c->clkid == CLOCK_REALTIME) {
-   rt = c;
-   continue;
+   /* If present, it can always be a sink clock */
+   LIST_INSERT_HEAD(&domain->dst_clocks, c, dst_list);
+   domain->src_clock = c->dest_only ? NULL : c;
+   return 0;
}
 
if (c->new_state) {
@@ -424,57 +431,81 @@ static void reconfigure(struct domain *domain)
src_cnt++;
break;
}
-   last = c;
}
if (src_cnt > 1) {
pr_info("multiple source clocks available, postponing sync...");
domain->src_clock = NULL;
-   return;
+   return -1;
}
if (src_cnt > 0 && !src) {
pr_info("source clock not ready, waiting...");
domain->src_clock = NULL;
-   return;
+   return -1;
}
if (!src_cnt && !dst_cnt) {
pr_info("no PHC ready, waiting...");
domain->src_clock = NULL;
-   return;
+  

[Linuxptp-devel] [PATCH v1 0/4] Add multi-domain synchronization to phc2sys

2023-07-20 Thread Miroslav Lichvar
RFC->v1:
- split into multiple patches
- fixed non-automatic modes
- fixed handling of TAI-UTC offset and leap seconds
- fixed other bugs
- updated man page

This series improves the automatic mode in phc2sys to support
synchronization of clocks between multiple ptp4l instances to enable a
fallback on failures. The first three patches refactor the code to
minimize the fourth patch adding the actual support.

Miroslav Lichvar (4):
  phc2sys: Rename phc2sys_private to domain.
  phc2sys: Create pmc agent after processing options.
  phc2sys: Shallow do_loop().
  phc2sys: Add multi-domain synchronization.

 phc2sys.8 |  12 +-
 phc2sys.c | 622 +-
 2 files changed, 390 insertions(+), 244 deletions(-)

-- 
2.40.1



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


[Linuxptp-devel] [PATCH v1 1/4] phc2sys: Rename phc2sys_private to domain.

2023-07-20 Thread Miroslav Lichvar
Rename the phc2sys_private structure to domain in preparation for
support of multiple domains.

While touching the lines of code, also rename master to source clock.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 326 +++---
 1 file changed, 163 insertions(+), 163 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 19e8012..a010f9f 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -96,7 +96,7 @@ struct port {
struct clock *clock;
 };
 
-struct phc2sys_private {
+struct domain {
unsigned int stats_max_count;
int sanity_freq_limit;
enum servo_type servo_type;
@@ -110,16 +110,16 @@ struct phc2sys_private {
LIST_HEAD(port_head, port) ports;
LIST_HEAD(clock_head, clock) clocks;
LIST_HEAD(dst_clock_head, clock) dst_clocks;
-   struct clock *master;
+   struct clock *src_clock;
 };
 
 static struct config *phc2sys_config;
 
-static int clock_handle_leap(struct phc2sys_private *priv,
+static int clock_handle_leap(struct domain *domain,
 struct clock *clock,
 int64_t offset, uint64_t ts);
 
-static struct servo *servo_add(struct phc2sys_private *priv,
+static struct servo *servo_add(struct domain *domain,
   struct clock *clock)
 {
double ppb;
@@ -139,19 +139,19 @@ static struct servo *servo_add(struct phc2sys_private 
*priv,
}
}
 
-   servo = servo_create(phc2sys_config, priv->servo_type,
+   servo = servo_create(phc2sys_config, domain->servo_type,
 -ppb, max_ppb, 0);
if (!servo) {
pr_err("Failed to create servo");
return NULL;
}
 
-   servo_sync_interval(servo, priv->phc_interval);
+   servo_sync_interval(servo, domain->phc_interval);
 
return servo;
 }
 
-static struct clock *clock_add(struct phc2sys_private *priv, const char 
*device,
+static struct clock *clock_add(struct domain *domain, const char *device,
   int phc_index)
 {
struct clock *c;
@@ -187,7 +187,7 @@ static struct clock *clock_add(struct phc2sys_private 
*priv, const char *device,
c->source_label = "phc";
}
 
-   if (priv->stats_max_count > 0) {
+   if (domain->stats_max_count > 0) {
c->offset_stats = stats_create();
c->freq_stats = stats_create();
c->delay_stats = stats_create();
@@ -198,8 +198,8 @@ static struct clock *clock_add(struct phc2sys_private 
*priv, const char *device,
return NULL;
}
}
-   if (priv->sanity_freq_limit) {
-   c->sanity_check = clockcheck_create(priv->sanity_freq_limit);
+   if (domain->sanity_freq_limit) {
+   c->sanity_check = clockcheck_create(domain->sanity_freq_limit);
if (!c->sanity_check) {
pr_err("failed to create clock check");
return NULL;
@@ -208,17 +208,17 @@ static struct clock *clock_add(struct phc2sys_private 
*priv, const char *device,
 
if (clkid != CLOCK_INVALID && clkid != CLOCK_REALTIME)
c->sysoff_method = sysoff_probe(CLOCKID_TO_FD(clkid),
-   priv->phc_readings);
+   domain->phc_readings);
 
-   LIST_INSERT_HEAD(&priv->clocks, c, list);
+   LIST_INSERT_HEAD(&domain->clocks, c, list);
return c;
 }
 
-static void clock_cleanup(struct phc2sys_private *priv)
+static void clock_cleanup(struct domain *domain)
 {
struct clock *c, *tmp;
 
-   LIST_FOREACH_SAFE(c, &priv->clocks, list, tmp) {
+   LIST_FOREACH_SAFE(c, &domain->clocks, list, tmp) {
if (c->servo) {
servo_destroy(c->servo);
}
@@ -241,45 +241,45 @@ static void clock_cleanup(struct phc2sys_private *priv)
}
 }
 
-static void port_cleanup(struct phc2sys_private *priv)
+static void port_cleanup(struct domain *domain)
 {
struct port *p, *tmp;
 
-   LIST_FOREACH_SAFE(p, &priv->ports, list, tmp) {
+   LIST_FOREACH_SAFE(p, &domain->ports, list, tmp) {
free(p);
}
 }
 
-static struct port *port_get(struct phc2sys_private *priv, unsigned int number)
+static struct port *port_get(struct domain *domain, unsigned int number)
 {
struct port *p;
 
-   LIST_FOREACH(p, &priv->ports, list) {
+   LIST_FOREACH(p, &domain->ports, list) {
if (p->number == number)
return p;
}
return NULL;
 }
 
-static struct port *port_add(struct phc2sys_private *priv, unsigned int number,
+static struct port *port_add(struct do

[Linuxptp-devel] [PATCH v1 3/4] phc2sys: Shallow do_loop().

2023-07-20 Thread Miroslav Lichvar
Move the measurements and updates of clocks from do_loop() to separate
function.

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 100 +-
 1 file changed, 54 insertions(+), 46 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index d0f5adb..c1a1c6c 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -703,14 +703,63 @@ static int update_needed(struct clock *c)
return 0;
 }
 
-static int do_loop(struct domain *domain)
+static int update_domain_clocks(struct domain *domain)
 {
-   struct timespec interval;
+   int64_t offset, delay;
struct clock *clock;
uint64_t ts;
-   int64_t offset, delay;
int err;
 
+   LIST_FOREACH(clock, &domain->dst_clocks, dst_list) {
+   if (!update_needed(clock))
+   continue;
+
+   /* don't try to synchronize the clock to itself */
+   if (clock->clkid == domain->src_clock->clkid ||
+   (clock->phc_index >= 0 &&
+clock->phc_index == domain->src_clock->phc_index) ||
+   !strcmp(clock->device, domain->src_clock->device))
+   continue;
+
+   if (clock->clkid == CLOCK_REALTIME &&
+   domain->src_clock->sysoff_method >= 0) {
+   /* use sysoff */
+   err = 
sysoff_measure(CLOCKID_TO_FD(domain->src_clock->clkid),
+domain->src_clock->sysoff_method,
+domain->phc_readings,
+&offset, &ts, &delay);
+   } else if (domain->src_clock->clkid == CLOCK_REALTIME &&
+  clock->sysoff_method >= 0) {
+   /* use reversed sysoff */
+   err = sysoff_measure(CLOCKID_TO_FD(clock->clkid),
+clock->sysoff_method,
+domain->phc_readings,
+&offset, &ts, &delay);
+   if (!err) {
+   offset = -offset;
+   ts += offset;
+   }
+   } else {
+   /* use phc */
+   err = clockadj_compare(domain->src_clock->clkid,
+  clock->clkid,
+  domain->phc_readings,
+  &offset, &ts, &delay);
+   }
+   if (err == -EBUSY)
+   continue;
+   if (err)
+   return -1;
+   update_clock(domain, clock, offset, ts, delay);
+   }
+
+   return 0;
+}
+
+static int do_loop(struct domain *domain)
+{
+   struct timespec interval;
+
interval.tv_sec = domain->phc_interval;
interval.tv_nsec = (domain->phc_interval - interval.tv_sec) * 1e9;
 
@@ -731,49 +780,8 @@ static int do_loop(struct domain *domain)
}
if (!domain->src_clock)
continue;
-
-   LIST_FOREACH(clock, &domain->dst_clocks, dst_list) {
-   if (!update_needed(clock))
-   continue;
-
-   /* don't try to synchronize the clock to itself */
-   if (clock->clkid == domain->src_clock->clkid ||
-   (clock->phc_index >= 0 &&
-clock->phc_index == domain->src_clock->phc_index) 
||
-   !strcmp(clock->device, domain->src_clock->device))
-   continue;
-
-   if (clock->clkid == CLOCK_REALTIME &&
-   domain->src_clock->sysoff_method >= 0) {
-   /* use sysoff */
-   err = 
sysoff_measure(CLOCKID_TO_FD(domain->src_clock->clkid),
-
domain->src_clock->sysoff_method,
-domain->phc_readings,
-&offset, &ts, &delay);
-   } else if (domain->src_clock->clkid == CLOCK_REALTIME &&
-  clock->sysoff_method >= 0) {
-   /* use reversed sysoff */
-   err = 
sysoff_measure(CLOCKID_TO_FD(clock->clkid),
-clock->sysoff_method,
-

Re: [Linuxptp-devel] [PATCH v1] servo: Implement offset statistics

2023-07-10 Thread Miroslav Lichvar
On Mon, Jul 10, 2023 at 11:18:36AM +0200, Maciek Machnikowski wrote:
> Do you mean the one in clock.c? If so, the clock_stats_display resets
> them, so there's no way of printing the stats from a whole run -
> especially if we want to add some periodic read of these stats.

The logged stats can be combined over any number of intervals as you
like.

If that is not acceptable for some reason, I'd prefer to add a new
option to not reset the stats after printing instead of duplicating
the functionality in a different place and printing the data in a
different format.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v1] servo: Implement offset statistics

2023-07-10 Thread Miroslav Lichvar
On Mon, Jul 03, 2023 at 04:18:57PM +0200, Maciek Machnikowski wrote:
> Currently running servo reports current offsets in numerous tools, but lacks
> statistical data. Having the basic statistical data makes it easier to monitor
> running servo (especially on long runs) and determine servo performance.

There is already the summary_interval option to print the offset
statistics. Is that not sufficient?

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] Add notification for changes in PARENT_DATA_SET.

2023-06-26 Thread Miroslav Lichvar
Add a new notification event for pmc clients to receive
PARENT_DATA_SET when a change is made there.

Signed-off-by: Miroslav Lichvar 
---
 clock.c| 26 +++---
 notification.h |  1 +
 pmc.c  | 10 ++
 pmc_common.c   | 15 +++
 4 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/clock.c b/clock.c
index 5a64613..b0df2e4 100644
--- a/clock.c
+++ b/clock.c
@@ -819,9 +819,14 @@ static enum servo_state clock_no_adjust(struct clock *c, 
tmv_t ingress,
return state;
 }
 
+static int clock_compare_pds(struct parentDS *pds1, struct parentDS *pds2)
+{
+   return memcmp(pds1, pds2, sizeof (*pds1));
+}
+
 static void clock_update_grandmaster(struct clock *c)
 {
-   struct parentDS *pds = &c->dad.pds;
+   struct parentDS *pds = &c->dad.pds, old_pds = *pds;
memset(&c->cur, 0, sizeof(c->cur));
memset(c->ptl, 0, sizeof(c->ptl));
 
@@ -836,11 +841,14 @@ static void clock_update_grandmaster(struct clock *c)
c->tds.currentUtcOffset = c->utc_offset;
c->tds.flags= c->time_flags;
c->tds.timeSource   = c->time_source;
+
+   if (clock_compare_pds(&old_pds, pds))
+   clock_notify_event(c, NOTIFY_PARENT_DATA_SET);
 }
 
 static void clock_update_slave(struct clock *c)
 {
-   struct parentDS *pds = &c->dad.pds;
+   struct parentDS *pds = &c->dad.pds, old_pds = *pds;
struct timePropertiesDS tds;
struct ptp_message *msg;
 
@@ -864,6 +872,9 @@ static void clock_update_slave(struct clock *c)
pr_warning("running in a temporal vortex");
}
clock_update_time_properties(c, tds);
+
+   if (clock_compare_pds(&old_pds, pds))
+   clock_notify_event(c, NOTIFY_PARENT_DATA_SET);
 }
 
 static int clock_utc_correct(struct clock *c, tmv_t ingress)
@@ -1721,14 +1732,23 @@ int clock_manage(struct clock *c, struct port *p, 
struct ptp_message *msg)
 void clock_notify_event(struct clock *c, enum notification event)
 {
struct port *uds = c->uds_rw_port;
-   struct PortIdentity pid = port_identity(uds);
+   struct PortIdentity pid;
struct ptp_message *msg;
int id;
 
+   /* A notification may come before UDS is created */
+   if (!uds)
+   return;
+
+   pid = port_identity(uds);
+
switch (event) {
case NOTIFY_TIME_SYNC:
id = MID_TIME_STATUS_NP;
break;
+   case NOTIFY_PARENT_DATA_SET:
+   id = MID_PARENT_DATA_SET;
+   break;
default:
return;
}
diff --git a/notification.h b/notification.h
index 115f864..c1a6395 100644
--- a/notification.h
+++ b/notification.h
@@ -44,6 +44,7 @@ static inline bool event_bitmask_get(uint8_t *bitmask, 
unsigned int event)
 enum notification {
NOTIFY_PORT_STATE,
NOTIFY_TIME_SYNC,
+   NOTIFY_PARENT_DATA_SET,
 };
 
 #endif
diff --git a/pmc.c b/pmc.c
index 00e691f..357c40a 100644
--- a/pmc.c
+++ b/pmc.c
@@ -449,12 +449,14 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
case MID_SUBSCRIBE_EVENTS_NP:
sen = (struct subscribe_events_np *) mgt->data;
fprintf(fp, "SUBSCRIBE_EVENTS_NP "
-   IFMT "duration  %hu"
-   IFMT "NOTIFY_PORT_STATE %s"
-   IFMT "NOTIFY_TIME_SYNC  %s",
+   IFMT "duration   %hu"
+   IFMT "NOTIFY_PORT_STATE  %s"
+   IFMT "NOTIFY_TIME_SYNC   %s"
+   IFMT "NOTIFY_PARENT_DATA_SET %s",
sen->duration,
event_bitmask_get(sen->bitmask, NOTIFY_PORT_STATE) ? 
"on" : "off",
-   event_bitmask_get(sen->bitmask, NOTIFY_TIME_SYNC) ? 
"on" : "off");
+   event_bitmask_get(sen->bitmask, NOTIFY_TIME_SYNC) ? 
"on" : "off",
+   event_bitmask_get(sen->bitmask, NOTIFY_PARENT_DATA_SET) 
? "on" : "off");
break;
case MID_SYNCHRONIZATION_UNCERTAIN_NP:
mtd = (struct management_tlv_datum *) mgt->data;
diff --git a/pmc_common.c b/pmc_common.c
index 9e251c4..8efeacd 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -178,6 +178,7 @@ static void do_set_action(struct pmc *pmc, int action, int 
index, char *str)
struct port_ds_np pnp;
char onoff_port_state[4] = "off";
char onoff_time_status[4] = "off";
+   char onoff_parent_data_set[4] = "off";
char display_name[11] = {0};
 

Re: [Linuxptp-devel] [Linuxptp-users] Quarterly release schedule

2023-06-12 Thread Miroslav Lichvar
On Fri, Jun 09, 2023 at 08:35:50AM -0700, Richard Cochran wrote:
> With the HIL framework in place, I am able run the smoke tests very
> easily, and that allows me to release the software with confidence.

That's great news.

With the new schedule, will there be a deadline for submitting new
features and the rest focused on testing and stabilization?

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] clock: Fix summary interval in free-running mode.

2023-05-18 Thread Miroslav Lichvar
In the free-running mode the stats are updated only once per the
frequency estimation interval instead of once per sync message. The
stats max_count calculation didn't account for that, which caused a
reduced rate of printed summaries. Fix the calculation for this case.

Signed-off-by: Miroslav Lichvar 
---
 clock.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index fe08d48..5a64613 100644
--- a/clock.c
+++ b/clock.c
@@ -2086,7 +2086,12 @@ void clock_sync_interval(struct clock *c, int n)
}
c->fest.max_count = (1U << shift);
 
-   shift = c->stats_interval - n;
+   /* In free-running mode stats accumulate once per freq_est_interval */
+   if (c->free_running)
+   shift = c->stats_interval - n - shift;
+   else
+   shift = c->stats_interval - n;
+
if (shift < 0)
shift = 0;
else if (shift >= sizeof(int) * 8) {
-- 
2.40.1



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


Re: [Linuxptp-devel] [PATCH] Fix power profile config option range to UINT32_MAX

2023-05-11 Thread Miroslav Lichvar
On Wed, May 10, 2023 at 06:57:26PM -0700, Richard Cochran wrote:
> On Sun, Apr 30, 2023 at 01:50:34PM -0700, Richard Cochran wrote:
> > Thanks for finding this.  I'd like to fix it in a simpler way.
> 
> FWIW I am going ahead with my patch.
> 
> @Miroslav it changes the default value for one field, and so the test
> suite would need the following change.

The test is now updated. Thanks.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] sk: Reset timestamping mode on exit, use locks

2023-05-09 Thread Miroslav Lichvar
On Tue, May 09, 2023 at 02:49:23PM +0200, Andrew Zaborowski wrote:
> Yep, I guess that will break unless you take care to stop the
> instances in the reverse order to that in which they were started.  Is
> there a use case for multiple ptp4l instances in different containers?

I'm sure there are containers that cannot be configured to run
multiple instances. It might also be a policy for some reason.

> > or a different program is using HW timestamping (e.g.
> > chronyd).
> 
> True.  To handle that case I'd need to drop the cleanup part in this
> patch,... or add a compatible mechanism in chronyd.  Or again the user
> would have to keep track of the shutdown order.

Or make the cleanup part configurable (disabled by default)?

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] sk: Reset timestamping mode on exit, use locks

2023-05-09 Thread Miroslav Lichvar
On Tue, May 09, 2023 at 02:02:05PM +0200, Andrew Zaborowski wrote:
> Per https://www.kernel.org/doc/Documentation/networking/timestamping.txt
> section 3:
> "User space is responsible to ensure that multiple processes don't interfere
> with each other and that the settings are reset."
> 
> Add locking for the interface's HW timestamping mode to ensure that in a
> setup with multiple ptp4l sessions sharing interfaces the sessions don't
> overwrite each other's timestamping mode and that there is one session
> responsible for resetting the mode on exit.

Is this intended to catch misconfigurations where different ptp4l
instances are setting incompatible RX filters?

If I understand the solution correctly, stopping ptp4l would break
another ptp4l instance if they don't share the same filesystem (e.g.
containers), or a different program is using HW timestamping (e.g.
chronyd).

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v3 0/6] Use precise frequency for TX SYNC messages

2023-04-26 Thread Miroslav Lichvar
On Wed, Apr 26, 2023 at 02:25:12PM +0200, Luigi 'Comio' Mantellini wrote:
>  Having servers on different
> nodes will give a drift because there is not any shared clock in this case.

If the nodes had their system clocks synchronized, as would normally
be expected, the sync transmissions would not drift.

I think it is a desired property of any network protocol to evenly
spread the traffic if possible. Imagine a large number of identical
PTP servers powered on at the same time. If their sync transmissions
were clumped together in a fraction of the interval, I'd not consider
it to be working well.

> BTW in a scenario you can have just a couple of servers to handle two
> domains, and collision is improbable and must not be a problem.
> In addition, using periodic timers will permit you to identify easily
> missing ticks for scheduling issues. This is very useful during debugging
> of your node when your CPU is not dedicated.

You can do that without periodic timers too by comparing the
difference between last two transmit timestamps to the expected
interval.

> During test and qualification the SW implementation has been compared with
> HW implementations on Telecom grade devices. A warning message has been
> reported to me from a customer regarding the TX tolerance.

Unless there is a specification requiring such tight timing, I'd
consider it a feature, not a bug.

1588-2019 in 9.5.9.2 requires for the timing of sync messages that
"the value of the arithmetic mean of the intervals, in seconds,
between PTP message transmissions is within ± 30% of the value of
2^portDS.logSyncInterval" and "at least 90% of the inter-message
intervals are within ± 30% of the value of 2^portDS.logSyncInterval.
The interval between successive PTP messages should not exceed twice
the value of 2^portDS.logSyncInterval."

Consider what happens when the CPU or network is heavily overloaded.
Periodic timer doesn't allow any slippage, so at some point it will
skip the transmission. If it's bad enough, the average can get over
the 30% limit, while with the current solution the interval can
gradually increase with the load and stay within the limits under
worse conditions.

Instead of trying to make the interval constant I propose add extra
randomness, maybe +/- 1%. This seems to be well within the spec.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] Clear pending errors on sockets.

2023-04-26 Thread Miroslav Lichvar
When the netlink socket of a port (used for receiving link up/down
events) had an error (e.g. ENOBUFS due to the kernel sending too many
messages), ptp4l switched the port to the faulty state, but it kept
getting POLLERR on the socket and logged "port 1: unexpected socket
error" in an infinite loop.

Unlike the PTP event and general sockets, the netlink sockets cannot be
closed in the faulty state as they are needed to receive the link up event.

Instead, receive and clear the error on all descriptors getting POLLERR
with getsockopt(SO_ERROR). Include the error in the log message together
with the descriptor index to make it easier to debug issues like this in
future.

Signed-off-by: Miroslav Lichvar 
---
 clock.c |  6 --
 sk.c| 14 ++
 sk.h|  7 +++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/clock.c b/clock.c
index 75d7c40..fe08d48 100644
--- a/clock.c
+++ b/clock.c
@@ -1790,8 +1790,10 @@ int clock_poll(struct clock *c)
if (cur[i].revents & (POLLIN|POLLPRI|POLLERR)) {
prior_state = port_state(p);
if (cur[i].revents & POLLERR) {
-   pr_err("%s: unexpected socket error",
-  port_log_name(p));
+   int error = sk_get_error(cur[i].fd);
+   pr_err("%s: error on fda[%d]: %s",
+  port_log_name(p), i,
+  strerror(error));
event = EV_FAULT_DETECTED;
} else {
event = port_event(p, i);
diff --git a/sk.c b/sk.c
index 2e4ef2c..6a9e5b8 100644
--- a/sk.c
+++ b/sk.c
@@ -491,6 +491,20 @@ int sk_receive(int fd, void *buf, int buflen,
return cnt < 0 ? -errno : cnt;
 }
 
+int sk_get_error(int fd)
+{
+   socklen_t len;
+   int error;
+
+   len = sizeof (error);
+   if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &error, &len) < 0) {
+   pr_err("getsockopt SO_ERROR failed: %m");
+   return -1;
+   }
+
+   return error;
+}
+
 int sk_set_priority(int fd, int family, uint8_t dscp)
 {
int level, optname, tos;
diff --git a/sk.h b/sk.h
index df105d6..76062d0 100644
--- a/sk.h
+++ b/sk.h
@@ -130,6 +130,13 @@ int sk_interface_addr(const char *name, int family, struct 
address *addr);
 int sk_receive(int fd, void *buf, int buflen,
   struct address *addr, struct hw_timestamp *hwts, int flags);
 
+/**
+ * Get and clear a pending socket error.
+ * @param fd  An open socket.
+ * @returnThe error.
+ */
+int sk_get_error(int fd);
+
 /**
  * Set DSCP value for socket.
  * @param fd An open socket.
-- 
2.40.0



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


Re: [Linuxptp-devel] [PATCH v3 0/6] Use precise frequency for TX SYNC messages

2023-04-26 Thread Miroslav Lichvar
On Wed, Apr 26, 2023 at 10:43:52AM +0200, Luigi 'Comio' Mantellini wrote:
> Hi Miroslav,
> 
> Sync is already sent using a constant interval as required by Standard even
> if it is lower than the nominal frequency for the actual implementation
> (stop and rearm).

It's not constant. It's randomized by the scheduling of the ptp4l
process. If you see two servers sending sync messages at the same
time, you can expect their transmissions to slowly drift away from
each other.

> The sync is forged by master as a one-to-many message that does not
> saturate the link, the randomization is required for the messages from
> Clients to Master (Delay-Req) in order to avoid congestion on Master.
> General messages are randomized also.

The same thing needs to be considered for multiple servers in
different domains on the same communication path.

> Speaking about messages collision in TX path, this is not an issue here
> because your HW timestamp will (should) set the correct TS value at the end
> of TX queue. On the RX side, the TS is applied just before the RX queue.

If all NICs and switches/routers had perfect HW timestamping, I might
agree.

> As reported by Richard, the conformance is not impacted by the proposed fix.

You have still not explained what is the issue you are trying to fix.

Is there some specification that requires the average sync interval to
be within 0.1% of the logSyncInterval? I hope it's not someone
complaining about a test report not having a round number.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v3 0/6] Use precise frequency for TX SYNC messages

2023-04-26 Thread Miroslav Lichvar
On Thu, Mar 30, 2023 at 08:08:12AM +0200, Luigi Mantellini wrote:
> The actual ptp4l implementation rearms timers after the expiration. This 
> approach doesn't permit to have a precise TX SYNC message scheduling.
> During my test the TX SYNC frequency is slightly lower the expectation (eg 
> 15.99Hz vs 16Hz).

Can you please explain advantages of using a constant interval?

Generally, sending timing sensitive messages over network in a
constant interval sounds like a bad idea to me. If there are multiple
hosts doing that (e.g. PTP servers in different domains) and they
happen to collide in their transmissions times (which requires
queueing and potentially impacts timestamp accuracy), this is more
likely to repeat frequently. If anything, I'd expect more randomness
to be added to the interval.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v2 0/1] Last (?) patch before version 4 release

2023-04-17 Thread Miroslav Lichvar
On Sat, Apr 15, 2023 at 05:29:32PM -0700, Richard Cochran wrote:
> Based on Erez's review, I expanded Christopher's patch to remove the
> control field codes completely.  I've tested this with the following
> hardware, and nothing broke:
> 
> Intel I210
> Intel X550T
> LabX Titanium 411 AVB switch
> 
> Please give this a try with your favorite hardware time stamping
> device.  I'd like to know how many devices stop working when the
> controlField is clear.

I have only one NIC with a PTP RX filter easily accessible for testing
and that is BCM5720. It seems to work fine with all three transports.

>From experiments with RX filters I did some time ago to see what parts
of messages the hardware looks at, I'm pretty sure Intel XL710, X540,
82576 work too.

As for the 4.0 release, can you please have a look at this bug fix?
https://www.mail-archive.com/linuxptp-devel@lists.sourceforge.net/msg06420.html

Thanks,

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v2 0/1] Last (?) patch before version 4 release

2023-04-17 Thread Miroslav Lichvar
On Sat, Apr 15, 2023 at 09:57:52PM -0400, Vincent Cheng wrote:
> Compile issue after applying patch against master commit:
> 
>   commit 60d829b102be9b8b13357a0f7a93f3e02d44b4ce
>   Author: Maciek Machnikowski 
>   Date:   Tue Apr 11 15:16:47 2023 +0200
> 
> ---
> port.c: In function ‘port_tx_sync’:
> port.c:1778:35: error: ‘CTL_FOLLOW_UP’ undeclared (first use in this function)
>   fup->header.control= CTL_FOLLOW_UP;

That's odd. These lines should be removed by the patch. I see only one
line (a comment) containing "header.control":

$ git grep 'header.control'
msg.c://m->header.control,

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v1] ntpshm: Invalidate SHM data before releasing the servo

2023-03-16 Thread Miroslav Lichvar
On Thu, Mar 16, 2023 at 10:05:30AM +0100, Maciek Machnikowski wrote:
> Would that work better:
> 
> The SHM is not cleared upon servo release when exiting the tool. This
> leaves the last update in the shared memory region marked as valid.

Ok.

> If the tool that writes to the shared memory fails it can leave
> incorrect timestamps marked as valid upon exit.

I'd suggest to expand a bit:
Users may not expect that. If the configuration is changed or the tool
was restarted to reset the state after an unexpected step of the clock
(e.g. system suspend), an incorrect timestamp might be accepted by the
consumer.

> To prevent such behavior - invalidate the SHM data when releasing the
> SHM servo.


-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v1] ntpshm: Invalidate SHM data before releasing the servo

2023-03-16 Thread Miroslav Lichvar
On Wed, Mar 15, 2023 at 05:44:43PM +0100, Maciek Machnikowski wrote:
> If you start the synchronization process and see that it doesn't work
> correctly then, this cleanup will prevent leaving invalid timestamps in
> memory.

Ok, this would be a good reason to invalidate the sample. So, it's not
about the data getting stale (which should be detected by the consumer),
but rather about users expecting restart of the process to reset
everything.
> 
> If timestamps were correct, it doesn't matter only as long as no one
> else touches the clocks.

I think that is always the case. It's not related to the producer
exiting.

> If the NTP process restarts, gets the time from a different source, and
> returns to the stale timestamp (which will sit in memory forever) it may
> incorrectly interpret this value as the right one.

This can happen even without stopping phc2sys.

> It also may backfire on embedded systems that stop the clock when going
> to sleep. After resuming, the value kept in there will be incorrect as well.

Again, that's unrelated to the process exit. You could have a command
to drop old measurements after resuming, but restarting everything is
easier.

If you reword the commit message, I'm ok with it.

Thanks,

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v1] ntpshm: Invalidate SHM data before releasing the servo

2023-03-15 Thread Miroslav Lichvar
On Wed, Mar 15, 2023 at 12:55:56PM +0100, Maciek Machnikowski wrote:
> The SHM is not cleared upon servo release when exiting the tool. This leaves
> the last update in the shared memory region marked as valid. As a result, the
> NTP daemon starting later may consume stale data if the producer process
> exited.
> 
> To prevent such behavior - invalidate the SHM data when releasing the SHM 
> servo.

It probably doesn't matter much either way, but this doesn't seem
right to me. The sample stays valid after ptp4l/phc2sys exited. There
is a timestamp when the measurement was made. It's up to the consumer
to check if it's still acceptable.

The same situation happens when the servo stops due the port switching
to another PTP state without exiting. The last sample was made when
the source clock was considered synchronized and it can be used for
synchronization of the system clock.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] port: Don't switch to PHC with SW timestamping.

2023-03-15 Thread Miroslav Lichvar
When ptp4l was configured to use SW timestamping, but the interface
supported also HW timestamping, ptp4l detected a change of the PHC on
start, switched to it from the system clock, and tried to control the
PHC using SW timestamps.

Don't switch the PHC if the current PHC index is -1, which is expected
with SW timestamping and in the free-running mode.

Fixes: afeabf3c90ed ("ptp4l: add VLAN over bond support")
Signed-off-by: Miroslav Lichvar 
---
 port.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/port.c b/port.c
index 3453716..9746df8 100644
--- a/port.c
+++ b/port.c
@@ -2765,6 +2765,7 @@ static void port_change_phc(struct port *p)
/* Try to switch only if the interface is up, it has HW time stamping
   using a non-vclock PHC, and the PHC actually changed. */
if (!(p->link_status & LINK_UP) ||
+   p->phc_index < 0 ||
!interface_tsinfo_valid(p->iface) ||
interface_get_vclock(p->iface) >= 0 ||
interface_phc_index(p->iface) < 0 ||
-- 
2.39.2



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


[Linuxptp-devel] [RFC PATCH] phc2sys: Add multi-domain synchronization.

2023-02-22 Thread Miroslav Lichvar
This patch adds support for synchronization between clocks controlled by
different ptp4l instances. phc2sys can now communicate with multiple
ptp4l instances (specified with multiple -z options). Clocks from one
instance are considered a domain. If the domain doesn't have a source
clock (port in the slave state), the clocks in the domain can be
synchronized to a source clock from a different domain. The real-time
clock is a separate domain, which can be a source for other domains if
the -r option is specified twice.

This allows the system clock or a PHC to be switched to a backup source
on some (e.g. network) failures. The selection of the source could
include a configurable priority or compare specific PTP attributes.
Currently, it's the first synchronized source in the order of specified
sockets.

TODO:
- fix non-automatic modes
- extend configuration to support domain-specific settings
- split into multiple patches
- ...

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.c | 594 --
 1 file changed, 349 insertions(+), 245 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index 19e8012..46499a2 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -96,7 +96,7 @@ struct port {
struct clock *clock;
 };
 
-struct phc2sys_private {
+struct domain {
unsigned int stats_max_count;
int sanity_freq_limit;
enum servo_type servo_type;
@@ -111,15 +111,16 @@ struct phc2sys_private {
LIST_HEAD(clock_head, clock) clocks;
LIST_HEAD(dst_clock_head, clock) dst_clocks;
struct clock *master;
+   int src_priority;
 };
 
 static struct config *phc2sys_config;
 
-static int clock_handle_leap(struct phc2sys_private *priv,
+static int clock_handle_leap(struct domain *domain,
 struct clock *clock,
 int64_t offset, uint64_t ts);
 
-static struct servo *servo_add(struct phc2sys_private *priv,
+static struct servo *servo_add(struct domain *domain,
   struct clock *clock)
 {
double ppb;
@@ -139,19 +140,19 @@ static struct servo *servo_add(struct phc2sys_private 
*priv,
}
}
 
-   servo = servo_create(phc2sys_config, priv->servo_type,
+   servo = servo_create(phc2sys_config, domain->servo_type,
 -ppb, max_ppb, 0);
if (!servo) {
pr_err("Failed to create servo");
return NULL;
}
 
-   servo_sync_interval(servo, priv->phc_interval);
+   servo_sync_interval(servo, domain->phc_interval);
 
return servo;
 }
 
-static struct clock *clock_add(struct phc2sys_private *priv, const char 
*device,
+static struct clock *clock_add(struct domain *domain, const char *device,
   int phc_index)
 {
struct clock *c;
@@ -187,7 +188,7 @@ static struct clock *clock_add(struct phc2sys_private 
*priv, const char *device,
c->source_label = "phc";
}
 
-   if (priv->stats_max_count > 0) {
+   if (domain->stats_max_count > 0) {
c->offset_stats = stats_create();
c->freq_stats = stats_create();
c->delay_stats = stats_create();
@@ -198,8 +199,8 @@ static struct clock *clock_add(struct phc2sys_private 
*priv, const char *device,
return NULL;
}
}
-   if (priv->sanity_freq_limit) {
-   c->sanity_check = clockcheck_create(priv->sanity_freq_limit);
+   if (domain->sanity_freq_limit) {
+   c->sanity_check = clockcheck_create(domain->sanity_freq_limit);
if (!c->sanity_check) {
pr_err("failed to create clock check");
return NULL;
@@ -208,17 +209,17 @@ static struct clock *clock_add(struct phc2sys_private 
*priv, const char *device,
 
if (clkid != CLOCK_INVALID && clkid != CLOCK_REALTIME)
c->sysoff_method = sysoff_probe(CLOCKID_TO_FD(clkid),
-   priv->phc_readings);
+   domain->phc_readings);
 
-   LIST_INSERT_HEAD(&priv->clocks, c, list);
+   LIST_INSERT_HEAD(&domain->clocks, c, list);
return c;
 }
 
-static void clock_cleanup(struct phc2sys_private *priv)
+static void clock_cleanup(struct domain *domain)
 {
struct clock *c, *tmp;
 
-   LIST_FOREACH_SAFE(c, &priv->clocks, list, tmp) {
+   LIST_FOREACH_SAFE(c, &domain->clocks, list, tmp) {
if (c->servo) {
servo_destroy(c->servo);
}
@@ -241,45 +242,46 @@ static void clock_cleanup(struct phc2sys_private *priv)
}
 }
 
-static void port_cleanup(struct phc2sys_private *priv)
+static void port_cleanup(struct domain *domai

Re: [Linuxptp-devel] [PATCH v6 00/11] Profile support for IEEE C37.238-2011 and IEEE C37.238-2017

2023-02-21 Thread Miroslav Lichvar
On Mon, Feb 20, 2023 at 12:57:48PM -0800, Richard Cochran wrote:
> ChangeLog
> ~
> v6:
>  Add new configuration options into default.cfg and man page
>  Make pmc SET command ready for 2106 (Miroslav)
>  Use SPDX tag for new file tz.h 

Looks good to me.

Thanks,

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v5 07/11] Add the ALTERNATE_TIME_OFFSET_PROPERTIES management message.

2023-02-15 Thread Miroslav Lichvar
On Tue, Feb 14, 2023 at 09:00:54PM -0800, Richard Cochran wrote:
> @@ -205,6 +206,24 @@ static void do_set_action(struct pmc *pmc, int action, 
> int index, char *str)
>   }
>   pmc_send_set_action(pmc, code, &mtd, sizeof(mtd));
>   break;
> + case MID_ALTERNATE_TIME_OFFSET_PROPERTIES:
> + memset(&atop, 0, sizeof(atop));
> + cnt = sscanf(str, " %*s %*s "
> +  "keyField   %hhu "
> +  "currentOffset  %d "
> +  "jumpSeconds%d "
> +  "timeOfNextJump %u ",
> +  &atop.keyField,
> +  &atop.currentOffset,
> +  &atop.jumpSeconds,
> +  &atop.timeOfNextJump.seconds_lsb);

Another instance of the Y2106 problem :).

With this patch it works for me (new test in the testsuite):

--- a/pmc_common.c
+++ b/pmc_common.c
@@ -179,6 +179,7 @@ static void do_set_action(struct pmc *pmc, int action, int 
index, char *str)
char onoff_port_state[4] = "off";
char onoff_time_status[4] = "off";
char display_name[11] = {0};
+   uint64_t jump;
uint8_t key;
int enable;
 
@@ -239,16 +240,18 @@ static void do_set_action(struct pmc *pmc, int action, 
int index, char *str)
 "keyField   %hhu "
 "currentOffset  %d "
 "jumpSeconds%d "
-"timeOfNextJump %u ",
+"timeOfNextJump %" SCNu64,
 &atop.keyField,
 &atop.currentOffset,
 &atop.jumpSeconds,
-&atop.timeOfNextJump.seconds_lsb);
+&jump);
if (cnt != 4) {
fprintf(stderr, "%s SET needs 4 values\n",
idtab[index].name);
break;
}
+   atop.timeOfNextJump.seconds_lsb = jump & 0x;
+   atop.timeOfNextJump.seconds_msb = jump >> 32;
pmc_send_set_action(pmc, code, &atop, sizeof(atop));
break;
case MID_GRANDMASTER_SETTINGS_NP:

The rest of the patchset looks good to me.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v5 03/11] Introduce the power profile.

2023-02-15 Thread Miroslav Lichvar
On Tue, Feb 14, 2023 at 09:00:50PM -0800, Richard Cochran wrote:
> @@ -300,6 +308,11 @@ struct config_item config_tab[] = {
>   GLOB_ITEM_DBL("pi_proportional_exponent", -0.3, -DBL_MAX, DBL_MAX),
>   GLOB_ITEM_DBL("pi_proportional_norm_max", 0.7, DBL_MIN, 1.0),
>   GLOB_ITEM_DBL("pi_proportional_scale", 0.0, 0.0, DBL_MAX),
> + PORT_ITEM_ENU("power_profile.version", IEEE_C37_238_VERSION_NONE, 
> ieee_c37_238_enu),
> + PORT_ITEM_INT("power_profile.2011.grandmasterTimeInaccuracy", 
> 0x, 0, INT_MAX),
> + PORT_ITEM_INT("power_profile.2011.networkTimeInaccuracy", 0, 0, 
> INT_MAX),
> + PORT_ITEM_INT("power_profile.2017.totalTimeInaccuracy", 0x, 0, 
> INT_MAX),
> + PORT_ITEM_INT("power_profile.grandmasterID", 0, 0, 0x),

I'm sorry I didn't notice this earlier. Can you please add these to
configs/default.cfg?

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v4 11/11] Introduce a time zone helper program.

2023-02-13 Thread Miroslav Lichvar
On Mon, Feb 13, 2023 at 11:32:42AM +0100, Erez wrote:
> "Implementations in which *time_t* is a 32-bit signed integer (many
> historical implementations) fail in the year 2038.

> Seems that instead of 'BUG 2000', we will have 'Bug 2038' :-)

This shouldn't be a problem for PTP.

The lsb field of timeOfNextJump is unsigned, so it will overflow in
year 2106 (in the TAI timescale).

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v4 00/11] Profile support for IEEE C37.238-2011 and IEEE C37.238-2017

2023-02-13 Thread Miroslav Lichvar
On Sun, Feb 12, 2023 at 07:40:36PM -0800, Richard Cochran wrote:
> > > On Thu, Feb 02, 2023 at 11:52:09AM +0100, Miroslav Lichvar wrote:
> > > > It would be nice to have the new options documented in the ptp4l man
> > > > page. Please consider renaming tztool to something more PTP-specific,
> > > > (maybe tz2ptp4l?), to not confuse people that is does something with
> > > > the system tz database.

> or even "tz2alt", short and sweet.

Sounds good to me. Thanks.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v4 11/11] Introduce a time zone helper program.

2023-02-13 Thread Miroslav Lichvar
On Sat, Feb 11, 2023 at 07:51:27AM -0800, Richard Cochran wrote:
> On Wed, Feb 01, 2023 at 04:52:15PM +0100, Miroslav Lichvar wrote:
> 
> > > + if (next) {
> > > + atop.jumpSeconds = next->local_tai_offset - 
> > > tz->local_tai_offset;
> > > + atop.timeOfNextJump.seconds_lsb = next->timestamp;
> > > + }
> > 
> > Is this intentionally not setting the _msb field, ignoring distant future?
> 
> Yes, it is intentional.  The LSB is 32 bits of seconds, so the range is
> 
> (/ (/ (/ 4294967295 3600) 24) 365) = 136 years
> 
> It is hard for me to understand why a time zone should change more
> that one year in the future from a given date?

Isn't timeOfNextJump an absolute time, i.e. will it not overflow 136
years after the epoch?

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] unicast: Avoid undefined integer shifts.

2023-02-07 Thread Miroslav Lichvar
Deny client requests and ignore server responses that have
logInterMessagePeriod outside of [-30..30] to avoid undefined
integer shifts in calculation of the interval.

Signed-off-by: Miroslav Lichvar 
---
 unicast_client.c  | 6 ++
 unicast_service.c | 4 
 2 files changed, 10 insertions(+)

diff --git a/unicast_client.c b/unicast_client.c
index d7b7243..0843554 100644
--- a/unicast_client.c
+++ b/unicast_client.c
@@ -454,6 +454,12 @@ void unicast_client_grant(struct port *p, struct 
ptp_message *m,
}
return;
}
+   if (abs(g->logInterMessagePeriod) > 30) {
+   pr_warning("%s: ignore bogus unicast message interval 2^%d",
+  p->log_name, g->logInterMessagePeriod);
+   return;
+   }
+
pr_debug("%s: unicast %s granted for %u sec",
 p->log_name, msg_type_string(mtype), g->durationField);
 
diff --git a/unicast_service.c b/unicast_service.c
index 1078041..687468c 100644
--- a/unicast_service.c
+++ b/unicast_service.c
@@ -327,6 +327,10 @@ int unicast_service_add(struct port *p, struct ptp_message 
*m,
return SERVICE_DENIED;
}
 
+   if (abs(req->logInterMessagePeriod) > 30) {
+   return SERVICE_DENIED;
+   }
+
LIST_FOREACH(itmp, &p->unicast_service->intervals, list) {
/*
 * Remember the interval of interest.
-- 
2.39.0



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


[Linuxptp-devel] [PATCH v3 4/4] timemaster: Use refclock_sock servo with chrony.

2023-02-06 Thread Miroslav Lichvar
If chronyd is selected as the NTP program, use the SOCK refclock instead
of SHM to improve security and reduce delay in receiving of samples.

Signed-off-by: Miroslav Lichvar 
---
 timemaster.8 | 10 -
 timemaster.c | 58 +++-
 2 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/timemaster.8 b/timemaster.8
index 2891a97..bc0b5b6 100644
--- a/timemaster.8
+++ b/timemaster.8
@@ -17,9 +17,9 @@ timemaster - run NTP with PTP as reference clocks
 \fBtimemaster\fR is a program that uses \fBptp4l\fR and \fBphc2sys\fR in
 combination with \fBchronyd\fR or \fBntpd\fR to synchronize the system clock to
 NTP and PTP time sources. The PTP time is provided by \fBphc2sys\fR and
-\fBptp4l\fR via SHM reference clocks to \fBchronyd\fR/\fBntpd\fR, which
-can compare all time sources and use the best sources to synchronize the system
-clock.
+\fBptp4l\fR via SOCK reference clock to \fBchronyd\fR or SHM reference clock to
+\fBntpd\fR, which can compare all time sources and use the best sources to
+synchronize the system clock.
 
 On start, \fBtimemaster\fR reads a configuration file that specifies the NTP
 and PTP time sources, checks which network interfaces have and share a PTP
@@ -154,14 +154,14 @@ specified also in other PTP domains and virtual clocks 
are disabled, only the
 
 .TP
 .B ntp_poll
-Specify the polling interval of the NTP SHM reference clock reading samples
+Specify the polling interval of the SOCK/SHM reference clock reading samples
 from \fBptp4l\fR or \fBphc2sys\fR. It's specified as a power of two in seconds.
 The default value is 2 (4 seconds).
 
 .TP
 .B phc2sys_poll
 Specify the polling interval used by \fBphc2sys\fR to read a PTP clock
-synchronized by \fBptp4l\fR and update the SHM sample for
+synchronized by \fBptp4l\fR and update the SOCK/SHM sample for
 \fBchronyd\fR/\fBntpd\fR. It's specified as a power of two in seconds. The
 default value is 0 (1 second).
 
diff --git a/timemaster.c b/timemaster.c
index 9fea011..fab71fc 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -674,10 +674,25 @@ static char **get_phc2sys_command(struct program_config 
*config,
  xstrdup("-z"), xstrdup(uds_path),
  xstrdup("-t"), xstrdup(message_tag),
  xstrdup("-n"), string_newf("%d", domain),
- xstrdup("-E"), xstrdup("ntpshm"),
- xstrdup("-M"), string_newf("%d", refclock_id +
-tconfig->first_shm_segment),
- NULL);
+ xstrdup("-E"), NULL);
+
+   switch (tconfig->ntp_program) {
+   case CHRONYD:
+   parray_extend((void ***)&command,
+ xstrdup("refclock_sock"),
+ xstrdup("--refclock_sock_address"),
+ string_newf("%s/chrony.SOCK%d",
+ tconfig->rundir, refclock_id),
+ NULL);
+   break;
+   case NTPD:
+   parray_extend((void ***)&command,
+ xstrdup("ntpshm"), xstrdup("-M"),
+ string_newf("%d", refclock_id +
+ tconfig->first_shm_segment),
+ NULL);
+   break;
+   }
 
return command;
 }
@@ -705,21 +720,24 @@ static void add_command(char **command, int command_group,
parray_append((void ***)&script->command_groups, group);
 }
 
-static void add_refclock_source(int refclock_id, int poll, int dpoll,
+static void add_refclock_source(int refclock_id, int poll, int phc_poll,
double delay, char *ntp_options, char *prefix,
struct timemaster_config *config,
char **ntp_config)
 {
-   int shm_segment = refclock_id + config->first_shm_segment;
+   int filter, shm_segment = refclock_id + config->first_shm_segment;
char *refid = get_refid(prefix, refclock_id);
 
switch (config->ntp_program) {
case CHRONYD:
+   /* set filter to the expected number of samples per poll */
+   filter = (poll >= phc_poll) ? 1 << (poll - phc_poll) : 1;
string_appendf(ntp_config,
-  "refclock SHM %d poll %d dpoll %d "
-  "refid %s precision 1.0e-9 delay %.1e %s\n",
-  shm_segment, poll, dpoll, refid, delay,
-  ntp_options);
+  "refclock SOCK %s/chrony.SOCK%d poll %d "
+  "filter %d refid %s pr

[Linuxptp-devel] [PATCH v3 0/4] Support for chrony SOCK refclock

2023-02-06 Thread Miroslav Lichvar
v3:
- fixed order of new entry in config_tab
- fixed renamed option in man pages
- changed default refclock_sock_address to /var/run/refclock.ptp.sock

v2:
- renamed servo to "refclock_sock"

This patchset fixes a bug in man page, adds a new servo for the chrony
SOCK refclock protocol as a more secure alternative to the NTP SHM, and
updates timemaster to use it.

Miroslav Lichvar (4):
  Remove obsolete statement in ptp4l man page.
  Add refclock_sock servo.
  timemaster: Replace shm_segment with refclock_id.
  timemaster: Use refclock_sock servo with chrony.

 config.c|   2 +
 configs/default.cfg |   1 +
 makefile|   2 +-
 phc2sys.8   |   9 ++-
 phc2sys.c   |   3 +
 ptp4l.8 |  12 ++--
 refclock_sock.c | 163 
 refclock_sock.h |  26 +++
 servo.c |   4 ++
 servo.h |   1 +
 timemaster.8|  10 +--
 timemaster.c|  92 +
 12 files changed, 285 insertions(+), 40 deletions(-)
 create mode 100644 refclock_sock.c
 create mode 100644 refclock_sock.h

-- 
2.39.0



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


[Linuxptp-devel] [PATCH v3 2/4] Add refclock_sock servo.

2023-02-06 Thread Miroslav Lichvar
Add a second servo that provides samples to other processes in order to
control the clock. The chrony SOCK refclock uses a Unix domain socket
instead of a shared memory segment.

The main advantage over the NTP SHM refclock is better security as the
socket can be located in a directory with restricted access, while the
shared memory segment (using a predictable key) can be created by
untrusted users or applications if they can run before ptp4l/phc2sys and
chronyd/ntpd, similarly to the issue with binding to an unprivileged
TCP/UDP port.

Signed-off-by: Miroslav Lichvar 
---
 config.c|   2 +
 configs/default.cfg |   1 +
 makefile|   2 +-
 phc2sys.8   |   9 ++-
 phc2sys.c   |   3 +
 ptp4l.8 |  12 ++--
 refclock_sock.c | 163 
 refclock_sock.h |  26 +++
 servo.c |   4 ++
 servo.h |   1 +
 10 files changed, 216 insertions(+), 7 deletions(-)
 create mode 100644 refclock_sock.c
 create mode 100644 refclock_sock.h

diff --git a/config.c b/config.c
index 2fa95fc..9b86b35 100644
--- a/config.c
+++ b/config.c
@@ -144,6 +144,7 @@ static struct config_enum clock_servo_enu[] = {
{ "linreg", CLOCK_SERVO_LINREG },
{ "ntpshm", CLOCK_SERVO_NTPSHM },
{ "nullf",  CLOCK_SERVO_NULLF  },
+   { "refclock_sock", CLOCK_SERVO_REFCLOCK_SOCK },
{ NULL, 0 },
 };
 
@@ -304,6 +305,7 @@ struct config_item config_tab[] = {
GLOB_ITEM_STR("productDescription", ";;"),
PORT_ITEM_STR("ptp_dst_mac", "01:1B:19:00:00:00"),
PORT_ITEM_STR("p2p_dst_mac", "01:80:C2:00:00:0E"),
+   GLOB_ITEM_STR("refclock_sock_address", "/var/run/refclock.ptp.sock"),
GLOB_ITEM_STR("revisionData", ";;"),
GLOB_ITEM_INT("sanity_freq_limit", 2, 0, INT_MAX),
PORT_ITEM_INT("serverOnly", 0, 0, 1),
diff --git a/configs/default.cfg b/configs/default.cfg
index 1b5b806..297a99d 100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -78,6 +78,7 @@ first_step_threshold  0.2
 max_frequency  9
 clock_servopi
 sanity_freq_limit  2
+refclock_sock_address  /var/run/refclock.ptp.sock
 ntpshm_segment 0
 msg_interval_request   0
 servo_num_offset_values 10
diff --git a/makefile b/makefile
index ba3fb38..0f8f185 100644
--- a/makefile
+++ b/makefile
@@ -24,7 +24,7 @@ CFLAGS= -Wall $(VER) $(incdefs) $(DEBUG) 
$(EXTRA_CFLAGS)
 LDLIBS = -lm -lrt -pthread $(EXTRA_LDFLAGS)
 PRG= ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster ts2phc
 FILTERS= filter.o mave.o mmedian.o
-SERVOS = linreg.o ntpshm.o nullf.o pi.o servo.o
+SERVOS = linreg.o ntpshm.o nullf.o pi.o refclock_sock.o servo.o
 TRANSP = raw.o transport.o udp.o udp6.o uds.o
 TS2PHC = ts2phc.o lstab.o nmea.o serial.o sock.o ts2phc_generic_pps_source.o \
  ts2phc_nmea_pps_source.o ts2phc_phc_pps_source.o ts2phc_pps_sink.o 
ts2phc_pps_source.o
diff --git a/phc2sys.8 b/phc2sys.8
index 51063b7..e6d9675 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -125,8 +125,8 @@ option. This option may be given up to 128 times.
 .BI \-E " servo"
 Specify which clock servo should be used. Valid values are pi for a PI
 controller, linreg for an adaptive controller using linear regression, and
-ntpshm for the NTP SHM reference clock to allow another process to synchronize
-the local clock.
+ntpshm and refclock_sock for the NTP SHM and chrony SOCK reference clocks
+respectively to allow another process to synchronize the local clock.
 The default is pi.
 .TP
 .BI \-P " kp"
@@ -382,6 +382,11 @@ Same as option
 .B \-F
 (see above).
 
+.TP
+.B refclock_sock_address
+The address of the UNIX domain socket to be used by the refclock_sock servo.
+The default is /var/run/refclock.ptp.sock.
+
 .TP
 .B ntpshm_segment
 The number of the SHM segment used by ntpshm servo.  The default is 0.
diff --git a/phc2sys.c b/phc2sys.c
index 88ed00c..836e63c 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -1140,6 +1140,9 @@ int main(int argc, char *argv[])
} else if (!strcasecmp(optarg, "ntpshm")) {
config_set_int(cfg, "clock_servo",
   CLOCK_SERVO_NTPSHM);
+   } else if (!strcasecmp(optarg, "refclock_sock")) {
+   config_set_int(cfg, "clock_servo",
+  CLOCK_SERVO_REFCLOCK_SOCK);
} else {
fprintf(stderr,
"invalid servo name %s\n", optarg);
diff --git a/ptp4l.8 b/ptp4l.8
index fbdaacf..8b477f0 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -527,10 +527,10 @@ The default is 0 (disabled).
 .B clo

[Linuxptp-devel] [PATCH v3 1/4] Remove obsolete statement in ptp4l man page.

2023-02-06 Thread Miroslav Lichvar
The NTP SHM number is no longer the PTP domain number. It was made
configurable and the default value is 0.

Fixes: 3760f8b6537e ("Add option to set NTP SHM segment number.")
Signed-off-by: Miroslav Lichvar 
---
 ptp4l.8 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ptp4l.8 b/ptp4l.8
index f09f44d..fbdaacf 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -528,8 +528,8 @@ The default is 0 (disabled).
 The servo which is used to synchronize the local clock. Valid values
 are "pi" for a PI controller, "linreg" for an adaptive controller
 using linear regression, "ntpshm" for the NTP SHM reference clock to
-allow another process to synchronize the local clock (the SHM segment
-number is set to the domain number), and "nullf" for a servo that
+allow another process to synchronize the local clock, and "nullf"
+for a servo that
 always dials frequency offset zero (for use in SyncE nodes).
 The default is "pi."
 .TP
-- 
2.39.0



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


[Linuxptp-devel] [PATCH v3 3/4] timemaster: Replace shm_segment with refclock_id.

2023-02-06 Thread Miroslav Lichvar
Replace the shm_segment number with a more general refclock_id and shift
by first_shm_index only in the SHM-specific context to allow other types
of refclocks to be used.

Signed-off-by: Miroslav Lichvar 
---
 timemaster.c | 48 +++-
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/timemaster.c b/timemaster.c
index cafd3a3..9fea011 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -658,8 +658,9 @@ static char **get_ptp4l_command(struct program_config 
*config,
return command;
 }
 
-static char **get_phc2sys_command(struct program_config *config, int domain,
- int poll, int shm_segment, char *uds_path,
+static char **get_phc2sys_command(struct program_config *config,
+ struct timemaster_config *tconfig, int domain,
+ int poll, int refclock_id, char *uds_path,
  char *message_tag)
 {
char **command = (char **)parray_new();
@@ -674,7 +675,9 @@ static char **get_phc2sys_command(struct program_config 
*config, int domain,
  xstrdup("-t"), xstrdup(message_tag),
  xstrdup("-n"), string_newf("%d", domain),
  xstrdup("-E"), xstrdup("ntpshm"),
- xstrdup("-M"), string_newf("%d", shm_segment), NULL);
+ xstrdup("-M"), string_newf("%d", refclock_id +
+tconfig->first_shm_segment),
+ NULL);
 
return command;
 }
@@ -702,11 +705,13 @@ static void add_command(char **command, int command_group,
parray_append((void ***)&script->command_groups, group);
 }
 
-static void add_shm_source(int shm_segment, int poll, int dpoll, double delay,
-  char *ntp_options, char *prefix,
-  struct timemaster_config *config, char **ntp_config)
+static void add_refclock_source(int refclock_id, int poll, int dpoll,
+   double delay, char *ntp_options, char *prefix,
+   struct timemaster_config *config,
+   char **ntp_config)
 {
-   char *refid = get_refid(prefix, shm_segment);
+   int shm_segment = refclock_id + config->first_shm_segment;
+   char *refid = get_refid(prefix, refclock_id);
 
switch (config->ntp_program) {
case CHRONYD:
@@ -758,7 +763,7 @@ static int add_vclock(struct script *script, int 
pclock_index)
 }
 
 static int add_ptp_source(struct ptp_domain *source,
- struct timemaster_config *config, int *shm_segment,
+ struct timemaster_config *config, int *refclock_id,
  int *command_group, int ***allocated_phcs,
  char **ntp_config, struct script *script)
 {
@@ -867,9 +872,9 @@ static int add_ptp_source(struct ptp_domain *source,
}
 
uds_path = string_newf("%s/ptp4l.%d.socket",
-  config->rundir, *shm_segment);
+  config->rundir, *refclock_id);
uds_path2 = string_newf("%s/ptp4lro.%d.socket",
-   config->rundir, *shm_segment);
+   config->rundir, *refclock_id);
 
message_tag = string_newf("[%d", source->domain);
for (j = 0; interfaces[j]; j++)
@@ -879,7 +884,7 @@ static int add_ptp_source(struct ptp_domain *source,
 
config_file = xmalloc(sizeof(*config_file));
config_file->path = string_newf("%s/ptp4l.%d.conf",
-   config->rundir, *shm_segment);
+   config->rundir, *refclock_id);
 
config_file->content = xstrdup("[global]\n");
if (*config->ptp4l.settings) {
@@ -905,10 +910,10 @@ static int add_ptp_source(struct ptp_domain *source,
vclock_index, 1);
add_command(command, *command_group, script);
 
-   command = get_phc2sys_command(&config->phc2sys,
+   command = get_phc2sys_command(&config->phc2sys, config,
  source->domain,
  source->phc2sys_poll,
- *shm_segment, uds_path,
+ *refclock_id, uds_path,
  message_tag);
add_co

Re: [Linuxptp-devel] [PATCH v4 00/11] Profile support for IEEE C37.238-2011 and IEEE C37.238-2017

2023-02-02 Thread Miroslav Lichvar
On Sat, Jan 28, 2023 at 02:43:41PM -0800, Richard Cochran wrote:
> The Power Profile specifies two new TLVs:

It would be nice to have the new options documented in the ptp4l man
page. Please consider renaming tztool to something more PTP-specific,
(maybe tz2ptp4l?), to not confuse people that is does something with
the system tz database.

Other than that and the issues I pointed out in the other mail, the
patches look good to me. I have no devices that support the power
profile. I juts ran few tests and it seemed to work as expected.
Wireshark prints the new TLV. No issues seen with valgrind and address
sanitizer.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH v4 11/11] Introduce a time zone helper program.

2023-02-01 Thread Miroslav Lichvar
On Sat, Jan 28, 2023 at 02:43:52PM -0800, Richard Cochran wrote:
> +++ b/makefile
> @@ -22,7 +22,7 @@ CC  = $(CROSS_COMPILE)gcc
>  VER = -DVER=$(version)
>  CFLAGS   = -Wall $(VER) $(incdefs) $(DEBUG) $(EXTRA_CFLAGS)
>  LDLIBS   = -lm -lrt -pthread $(EXTRA_LDFLAGS)
> -PRG  = ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster ts2phc
> +PRG  = ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster ts2phc tztool

The new program doesn't have a man page and is listed here last,
which causes "make install" to fail. That "[ -f $$x ]" in the install
recipe needs to be followed by something returning zero, e.g. ":".

> +++ b/tztool.c

> +static int update_ptp_serivce(struct tzinfo *tz, struct tzinfo *next)

Typo in the function name   ^^.

> + if (next) {
> + atop.jumpSeconds = next->local_tai_offset - 
> tz->local_tai_offset;
> + atop.timeOfNextJump.seconds_lsb = next->timestamp;
> +     }

Is this intentionally not setting the _msb field, ignoring distant future?

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH v2 3/4] timemaster: Replace shm_segment with refclock_id.

2023-02-01 Thread Miroslav Lichvar
Replace the shm_segment number with a more general refclock_id and shift
by first_shm_index only in the SHM-specific context to allow other types
of refclocks to be used.

Signed-off-by: Miroslav Lichvar 
---
 timemaster.c | 48 +++-
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/timemaster.c b/timemaster.c
index cafd3a3..9fea011 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -658,8 +658,9 @@ static char **get_ptp4l_command(struct program_config 
*config,
return command;
 }
 
-static char **get_phc2sys_command(struct program_config *config, int domain,
- int poll, int shm_segment, char *uds_path,
+static char **get_phc2sys_command(struct program_config *config,
+ struct timemaster_config *tconfig, int domain,
+ int poll, int refclock_id, char *uds_path,
  char *message_tag)
 {
char **command = (char **)parray_new();
@@ -674,7 +675,9 @@ static char **get_phc2sys_command(struct program_config 
*config, int domain,
  xstrdup("-t"), xstrdup(message_tag),
  xstrdup("-n"), string_newf("%d", domain),
  xstrdup("-E"), xstrdup("ntpshm"),
- xstrdup("-M"), string_newf("%d", shm_segment), NULL);
+ xstrdup("-M"), string_newf("%d", refclock_id +
+tconfig->first_shm_segment),
+ NULL);
 
return command;
 }
@@ -702,11 +705,13 @@ static void add_command(char **command, int command_group,
parray_append((void ***)&script->command_groups, group);
 }
 
-static void add_shm_source(int shm_segment, int poll, int dpoll, double delay,
-  char *ntp_options, char *prefix,
-  struct timemaster_config *config, char **ntp_config)
+static void add_refclock_source(int refclock_id, int poll, int dpoll,
+   double delay, char *ntp_options, char *prefix,
+   struct timemaster_config *config,
+   char **ntp_config)
 {
-   char *refid = get_refid(prefix, shm_segment);
+   int shm_segment = refclock_id + config->first_shm_segment;
+   char *refid = get_refid(prefix, refclock_id);
 
switch (config->ntp_program) {
case CHRONYD:
@@ -758,7 +763,7 @@ static int add_vclock(struct script *script, int 
pclock_index)
 }
 
 static int add_ptp_source(struct ptp_domain *source,
- struct timemaster_config *config, int *shm_segment,
+ struct timemaster_config *config, int *refclock_id,
  int *command_group, int ***allocated_phcs,
  char **ntp_config, struct script *script)
 {
@@ -867,9 +872,9 @@ static int add_ptp_source(struct ptp_domain *source,
}
 
uds_path = string_newf("%s/ptp4l.%d.socket",
-  config->rundir, *shm_segment);
+  config->rundir, *refclock_id);
uds_path2 = string_newf("%s/ptp4lro.%d.socket",
-   config->rundir, *shm_segment);
+   config->rundir, *refclock_id);
 
message_tag = string_newf("[%d", source->domain);
for (j = 0; interfaces[j]; j++)
@@ -879,7 +884,7 @@ static int add_ptp_source(struct ptp_domain *source,
 
config_file = xmalloc(sizeof(*config_file));
config_file->path = string_newf("%s/ptp4l.%d.conf",
-   config->rundir, *shm_segment);
+   config->rundir, *refclock_id);
 
config_file->content = xstrdup("[global]\n");
if (*config->ptp4l.settings) {
@@ -905,10 +910,10 @@ static int add_ptp_source(struct ptp_domain *source,
vclock_index, 1);
add_command(command, *command_group, script);
 
-   command = get_phc2sys_command(&config->phc2sys,
+   command = get_phc2sys_command(&config->phc2sys, config,
  source->domain,
  source->phc2sys_poll,
- *shm_segment, uds_path,
+ *refclock_id, uds_path,
  message_tag);
add_co

[Linuxptp-devel] [PATCH v2 2/4] Add refclock_sock servo.

2023-02-01 Thread Miroslav Lichvar
Add a second servo that provides samples to other processes in order to
control the clock. The chrony SOCK refclock uses a Unix domain socket
instead of a shared memory segment.

The main advantage over the NTP SHM refclock is better security as the
socket can be located in a directory with restricted access, while the
shared memory segment (using a predictable key) can be created by
untrusted users or applications if they can run before ptp4l/phc2sys and
chronyd/ntpd, similarly to the issue with binding to an unprivileged
TCP/UDP port.

Signed-off-by: Miroslav Lichvar 
---
 config.c|   2 +
 configs/default.cfg |   1 +
 makefile|   2 +-
 phc2sys.8   |   9 ++-
 phc2sys.c   |   3 +
 ptp4l.8 |  12 ++--
 refclock_sock.c | 163 
 refclock_sock.h |  26 +++
 servo.c |   4 ++
 servo.h |   1 +
 10 files changed, 216 insertions(+), 7 deletions(-)
 create mode 100644 refclock_sock.c
 create mode 100644 refclock_sock.h

diff --git a/config.c b/config.c
index 08e3346..d3088e7 100644
--- a/config.c
+++ b/config.c
@@ -144,6 +144,7 @@ static struct config_enum clock_servo_enu[] = {
{ "linreg", CLOCK_SERVO_LINREG },
{ "ntpshm", CLOCK_SERVO_NTPSHM },
{ "nullf",  CLOCK_SERVO_NULLF  },
+   { "refclock_sock", CLOCK_SERVO_REFCLOCK_SOCK },
{ NULL, 0 },
 };
 
@@ -232,6 +233,7 @@ struct config_item config_tab[] = {
PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
PORT_ITEM_ENU("BMCA", BMCA_PTP, bmca_enu),
GLOB_ITEM_INT("check_fup_sync", 0, 0, 1),
+   GLOB_ITEM_STR("refclock_sock_address", "/var/run/chrony/refclock.sock"),
GLOB_ITEM_INT("clientOnly", 0, 0, 1),
GLOB_ITEM_INT("clockAccuracy", 0xfe, 0, UINT8_MAX),
GLOB_ITEM_INT("clockClass", 248, 0, UINT8_MAX),
diff --git a/configs/default.cfg b/configs/default.cfg
index 1b5b806..f5a2bb1 100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -78,6 +78,7 @@ first_step_threshold  0.2
 max_frequency  9
 clock_servopi
 sanity_freq_limit  2
+refclock_sock_address  /var/run/chrony/refclock.sock
 ntpshm_segment 0
 msg_interval_request   0
 servo_num_offset_values 10
diff --git a/makefile b/makefile
index ba3fb38..0f8f185 100644
--- a/makefile
+++ b/makefile
@@ -24,7 +24,7 @@ CFLAGS= -Wall $(VER) $(incdefs) $(DEBUG) 
$(EXTRA_CFLAGS)
 LDLIBS = -lm -lrt -pthread $(EXTRA_LDFLAGS)
 PRG= ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster ts2phc
 FILTERS= filter.o mave.o mmedian.o
-SERVOS = linreg.o ntpshm.o nullf.o pi.o servo.o
+SERVOS = linreg.o ntpshm.o nullf.o pi.o refclock_sock.o servo.o
 TRANSP = raw.o transport.o udp.o udp6.o uds.o
 TS2PHC = ts2phc.o lstab.o nmea.o serial.o sock.o ts2phc_generic_pps_source.o \
  ts2phc_nmea_pps_source.o ts2phc_phc_pps_source.o ts2phc_pps_sink.o 
ts2phc_pps_source.o
diff --git a/phc2sys.8 b/phc2sys.8
index 9825ec7..69fd734 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -125,8 +125,8 @@ option. This option may be given up to 128 times.
 .BI \-E " servo"
 Specify which clock servo should be used. Valid values are pi for a PI
 controller, linreg for an adaptive controller using linear regression, and
-ntpshm for the NTP SHM reference clock to allow another process to synchronize
-the local clock.
+ntpshm and refclock_sock for the NTP SHM and chrony SOCK reference clocks
+respectively to allow another process to synchronize the local clock.
 The default is pi.
 .TP
 .BI \-P " kp"
@@ -382,6 +382,11 @@ Same as option
 .B \-F
 (see above).
 
+.TP
+.B chrony_sock_address
+The address of the chronyd's UNIX domain socket configured as a SOCK refclock
+to be used by the sock servo. The default is /var/run/chrony/refclock.sock.
+
 .TP
 .B ntpshm_segment
 The number of the SHM segment used by ntpshm servo.  The default is 0.
diff --git a/phc2sys.c b/phc2sys.c
index 88ed00c..836e63c 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -1140,6 +1140,9 @@ int main(int argc, char *argv[])
} else if (!strcasecmp(optarg, "ntpshm")) {
config_set_int(cfg, "clock_servo",
   CLOCK_SERVO_NTPSHM);
+   } else if (!strcasecmp(optarg, "refclock_sock")) {
+   config_set_int(cfg, "clock_servo",
+  CLOCK_SERVO_REFCLOCK_SOCK);
} else {
fprintf(stderr,
"invalid servo name %s\n", optarg);
diff --git a/ptp4l.8 b/ptp4l.8
index d2a038e..52f11d6 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -527,10 +527,10 @@ The default is 0 (disabled).
 .B cloc

[Linuxptp-devel] [PATCHv2 0/4] Support for chrony SOCK refclock

2023-02-01 Thread Miroslav Lichvar
v2:
- renamed servo to "refclock_sock"

This patchset fixes a bug in man page, adds a new servo for the chrony
SOCK refclock protocol as a more secure alternative to the NTP SHM, and
updates timemaster to use it.

Miroslav Lichvar (4):
  Remove obsolete statement in ptp4l man page.
  Add refclock_sock servo.
  timemaster: Replace shm_segment with refclock_id.
  timemaster: Use refclock_sock servo with chrony.

 config.c|   2 +
 configs/default.cfg |   1 +
 makefile|   2 +-
 phc2sys.8   |   9 ++-
 phc2sys.c   |   3 +
 ptp4l.8 |  12 ++--
 refclock_sock.c | 163 
 refclock_sock.h |  26 +++
 servo.c |   4 ++
 servo.h |   1 +
 timemaster.8|  10 +--
 timemaster.c|  92 +
 12 files changed, 285 insertions(+), 40 deletions(-)
 create mode 100644 refclock_sock.c
 create mode 100644 refclock_sock.h

-- 
2.39.0



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


[Linuxptp-devel] [PATCH v2 4/4] timemaster: Use refclock_sock servo with chrony.

2023-02-01 Thread Miroslav Lichvar
If chronyd is selected as the NTP program, use the SOCK refclock instead
of SHM to improve security and reduce delay in receiving of samples.

Signed-off-by: Miroslav Lichvar 
---
 timemaster.8 | 10 -
 timemaster.c | 58 +++-
 2 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/timemaster.8 b/timemaster.8
index 2891a97..bc0b5b6 100644
--- a/timemaster.8
+++ b/timemaster.8
@@ -17,9 +17,9 @@ timemaster - run NTP with PTP as reference clocks
 \fBtimemaster\fR is a program that uses \fBptp4l\fR and \fBphc2sys\fR in
 combination with \fBchronyd\fR or \fBntpd\fR to synchronize the system clock to
 NTP and PTP time sources. The PTP time is provided by \fBphc2sys\fR and
-\fBptp4l\fR via SHM reference clocks to \fBchronyd\fR/\fBntpd\fR, which
-can compare all time sources and use the best sources to synchronize the system
-clock.
+\fBptp4l\fR via SOCK reference clock to \fBchronyd\fR or SHM reference clock to
+\fBntpd\fR, which can compare all time sources and use the best sources to
+synchronize the system clock.
 
 On start, \fBtimemaster\fR reads a configuration file that specifies the NTP
 and PTP time sources, checks which network interfaces have and share a PTP
@@ -154,14 +154,14 @@ specified also in other PTP domains and virtual clocks 
are disabled, only the
 
 .TP
 .B ntp_poll
-Specify the polling interval of the NTP SHM reference clock reading samples
+Specify the polling interval of the SOCK/SHM reference clock reading samples
 from \fBptp4l\fR or \fBphc2sys\fR. It's specified as a power of two in seconds.
 The default value is 2 (4 seconds).
 
 .TP
 .B phc2sys_poll
 Specify the polling interval used by \fBphc2sys\fR to read a PTP clock
-synchronized by \fBptp4l\fR and update the SHM sample for
+synchronized by \fBptp4l\fR and update the SOCK/SHM sample for
 \fBchronyd\fR/\fBntpd\fR. It's specified as a power of two in seconds. The
 default value is 0 (1 second).
 
diff --git a/timemaster.c b/timemaster.c
index 9fea011..fab71fc 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -674,10 +674,25 @@ static char **get_phc2sys_command(struct program_config 
*config,
  xstrdup("-z"), xstrdup(uds_path),
  xstrdup("-t"), xstrdup(message_tag),
  xstrdup("-n"), string_newf("%d", domain),
- xstrdup("-E"), xstrdup("ntpshm"),
- xstrdup("-M"), string_newf("%d", refclock_id +
-tconfig->first_shm_segment),
- NULL);
+ xstrdup("-E"), NULL);
+
+   switch (tconfig->ntp_program) {
+   case CHRONYD:
+   parray_extend((void ***)&command,
+ xstrdup("refclock_sock"),
+ xstrdup("--refclock_sock_address"),
+ string_newf("%s/chrony.SOCK%d",
+ tconfig->rundir, refclock_id),
+ NULL);
+   break;
+   case NTPD:
+   parray_extend((void ***)&command,
+ xstrdup("ntpshm"), xstrdup("-M"),
+ string_newf("%d", refclock_id +
+ tconfig->first_shm_segment),
+ NULL);
+   break;
+   }
 
return command;
 }
@@ -705,21 +720,24 @@ static void add_command(char **command, int command_group,
parray_append((void ***)&script->command_groups, group);
 }
 
-static void add_refclock_source(int refclock_id, int poll, int dpoll,
+static void add_refclock_source(int refclock_id, int poll, int phc_poll,
double delay, char *ntp_options, char *prefix,
struct timemaster_config *config,
char **ntp_config)
 {
-   int shm_segment = refclock_id + config->first_shm_segment;
+   int filter, shm_segment = refclock_id + config->first_shm_segment;
char *refid = get_refid(prefix, refclock_id);
 
switch (config->ntp_program) {
case CHRONYD:
+   /* set filter to the expected number of samples per poll */
+   filter = (poll >= phc_poll) ? 1 << (poll - phc_poll) : 1;
string_appendf(ntp_config,
-  "refclock SHM %d poll %d dpoll %d "
-  "refid %s precision 1.0e-9 delay %.1e %s\n",
-  shm_segment, poll, dpoll, refid, delay,
-  ntp_options);
+  "refclock SOCK %s/chrony.SOCK%d poll %d "
+  "filter %d refid %s pr

[Linuxptp-devel] [PATCH v2 1/4] Remove obsolete statement in ptp4l man page.

2023-02-01 Thread Miroslav Lichvar
The NTP SHM number is no longer the PTP domain number. It was made
configurable and the default value is 0.

Fixes: 3760f8b6537e ("Add option to set NTP SHM segment number.")
Signed-off-by: Miroslav Lichvar 
---
 ptp4l.8 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ptp4l.8 b/ptp4l.8
index cd6299f..d2a038e 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -528,8 +528,8 @@ The default is 0 (disabled).
 The servo which is used to synchronize the local clock. Valid values
 are "pi" for a PI controller, "linreg" for an adaptive controller
 using linear regression, "ntpshm" for the NTP SHM reference clock to
-allow another process to synchronize the local clock (the SHM segment
-number is set to the domain number), and "nullf" for a servo that
+allow another process to synchronize the local clock, and "nullf"
+for a servo that
 always dials frequency offset zero (for use in SyncE nodes).
 The default is "pi."
 .TP
-- 
2.39.0



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


Re: [Linuxptp-devel] [PATCH] Handle error returned by kernel for clock adjustments

2023-01-30 Thread Miroslav Lichvar
On Mon, Jan 30, 2023 at 03:39:45PM +, Wojtek Wasko via Linuxptp-devel wrote:
> @@ -62,8 +62,11 @@ void clockadj_set_freq(clockid_t clkid, double freq)
>  
>   tx.modes |= ADJ_FREQUENCY;
>   tx.freq = (long) (freq * 65.536);
> - if (clock_adjtime(clkid, &tx) < 0)
> + if (clock_adjtime(clkid, &tx) < 0) {
>   pr_err("failed to adjust the clock: %m");
> + return -1;
> + }

Why not follow clockadj_get_freq() and simply exit here instead of
trying to recover?

What PHC/driver is expected to fail temporarily?

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] port: do not apply TS_LABEL_CHANGED when phc_from_cmdline is true

2022-11-24 Thread Miroslav Lichvar
On Wed, Nov 23, 2022 at 04:14:40PM -0500, Min Li wrote:
> This bug was found on the Broadcom OneSync platform, where phc is
> specified from command line and ptp4l receives link status notification.
> When the above condition happens, program will fall in a loop of
> keep getting EV_FAULT_DETECTED.

There was a patch submitted for this issue and a similar issue with
vclocks earlier: "port: Avoid faults with vclocks and PHC from command
line."

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] Don't re-arm fault clearing timer

2022-11-23 Thread Miroslav Lichvar
On Wed, Nov 23, 2022 at 09:32:38PM +1100, David Mirabito wrote:
> In summary:
> 
> Previously: if eth0 goes FAULTY, it may *never* recover if totally
> unrelated eth1 is flapping and producing a stream of useless RTNL messages.
> With this patch (or an equivalent): if eth0 goes faulty, the fault clears
> and retries after 16 seconds no matter what eth1 might be doing.

I see now, thanks for the explanation. The patch looks good to me, but
I'd suggest to improve the commit message with what you said here,
at least mention it's unrelated netlink messages what is interfering
with the recovery. I completely missed that.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] Don't re-arm fault clearing timer

2022-11-22 Thread Miroslav Lichvar
On Tue, Nov 22, 2022 at 08:59:48PM -0800, davidjm via Linuxptp-devel wrote:
> Set the timer only when an event causes the port to transition to the
> FAULTY state. Previously this was done based only on the port's current
> state, and events on that port (such as RTNL messages) cause it to
> repeatedly re-arm itself, potentially never clearing.

Do you please provide an example where this makes a difference?

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] port: Avoid faults with vclocks and PHC from command line.

2022-11-22 Thread Miroslav Lichvar
After commit afeabf3c90ed ("ptp4l: add VLAN over bond support") the
TS_LABEL_CHANGED flag was set on link status changes when the used
PHC index was different from the PHC index of the interface.

This caused the port to be constantly switching to the faulty state when
using vclocks, or a different PHC device was forced with the -p option,
where it is expected the used PHC doesn't match the interface's PHC.

Rework port_link_status() and port_change_phc() to avoid setting the
flag and switch the clock only in the cases where it is expected.

Fixes: afeabf3c90ed ("ptp4l: add VLAN over bond support")
Signed-off-by: Miroslav Lichvar 
---
 port.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/port.c b/port.c
index 756ff3a..c88f1be 100644
--- a/port.c
+++ b/port.c
@@ -2685,10 +2685,13 @@ static void port_change_phc(struct port *p)
 {
int required_modes;
 
-   /* Only switch a non-vclock PHC with HW time stamping. */
-   if (!interface_tsinfo_valid(p->iface) ||
+   /* Try to switch only if the interface is up, it has HW time stamping
+  using a non-vclock PHC, and the PHC actually changed. */
+   if (!(p->link_status & LINK_UP) ||
+   !interface_tsinfo_valid(p->iface) ||
interface_get_vclock(p->iface) >= 0 ||
-   interface_phc_index(p->iface) < 0)
+   interface_phc_index(p->iface) < 0 ||
+   p->phc_index == interface_phc_index(p->iface))
return;
 
required_modes = clock_required_modes(p->clock);
@@ -2702,7 +2705,7 @@ static void port_change_phc(struct port *p)
   "command line, not the attached ptp%d",
   p->log_name, p->phc_index,
   interface_phc_index(p->iface));
-   } else if (p->phc_index != interface_phc_index(p->iface)) {
+   } else {
p->phc_index = interface_phc_index(p->iface);
 
if (clock_switch_phc(p->clock, p->phc_index)) {
@@ -2737,18 +2740,12 @@ void port_link_status(void *ctx, int linkup, int 
ts_index)
pr_notice("%s: ts label changed to %s", p->log_name, ts_label);
}
 
-   /* phc index may changed while ts_label keeps the same after failover.
-* e.g. vlan over bond. Since the lower link changed, we still set
-* the TS_LABEL_CHANGED flag.
-*/
+   /* The PHC index may change even with the same ts_label, e.g. after
+  failover with VLAN over bond. */
interface_get_tsinfo(p->iface);
-   if (p->phc_index != interface_phc_index(p->iface))
-   p->link_status |= TS_LABEL_CHANGED;
 
-   /* Both link down/up and change ts_label may change phc index. */
-   if (p->link_status & LINK_UP &&
-   (p->link_status & LINK_STATE_CHANGED || p->link_status & 
TS_LABEL_CHANGED))
-   port_change_phc(p);
+   /* Switch the clock if needed */
+   port_change_phc(p);
 
/*
 * A port going down can affect the BMCA result.
-- 
2.37.3



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


Re: [Linuxptp-devel] [PATCH] servo: stop rounding initial frequency to nearest ppb

2022-11-16 Thread Miroslav Lichvar
On Tue, Nov 15, 2022 at 04:41:09PM -0800, Jacob Keller wrote:
> Refactor the servo_create API to take a double value instead of an integer
> value. Fix the clockadj_get_freq and clockadj_set_freq initialization to
> avoid casting to an integer and thus rounding the value.

Looks good to me. Thanks for submitting the patch.

I think this patch conflicts with the clockcheck patchset. I'll update
it and resend if this one is accepted earlier.

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-14 Thread Miroslav Lichvar
On Mon, Nov 14, 2022 at 04:34:05AM -0800, Hal Murray wrote:
> Is there a URL for the spec?  I don't want an RFC.  Good comments in a header 
> file may be enough.  A separate document may be better if there are 
> complications that need explaining.

Here is the structure of the message with some comments:
https://git.tuxfamily.org/chrony/chrony.git/tree/refclock_sock.c#n38

If you started with a copy of the SHM driver in ntpd, it would need to
be modified to use a datagram Unix socket instead of the shared memory
segment, listen for messages on the socket (io_addclock()) instead of
polling in the _timer() function, and interpret the fields in the
message like this:

- "tv" field in SOCK is the same as receiveTimeStamp in SHM
- tv + offset is clockTimestamp
- messages with non-zero pulse can be ignored in the initial
  implementation as nothing really uses it
- leap has the same meaning
- magic is a protocol sanity+version check, should be always
  0x534f434b

The missing fields precision and samples can be set to -30 and 1
respectively.

That should be it. I can help with the patch if interested.

> is there a version number?

No. If there was a need to make an incompatible change, we can change
the magic number.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH] config: Fix -Wformat-truncation warnings.

2022-11-14 Thread Miroslav Lichvar
Check the snprintf() return value in order to avoid the following
warnings from gcc:

config.c: In function ‘config_create’:
config.c:921:52: warning: ‘%s’ directive output may be truncated writing up to 
9679 bytes into a region of size 33 [-Wformat-truncation=]
  921 | snprintf(buf, sizeof(buf), "global.%s", ci->label);
  |^~
config.c:921:17: note: ‘snprintf’ output between 8 and 9687 bytes into a 
destination of size 40
  921 | snprintf(buf, sizeof(buf), "global.%s", ci->label);
  | ^~
config.c:364:40: warning: ‘%s’ directive output may be truncated writing up to 
9679 bytes into a region of size 133  -Wformat-truncation=]
  364 | snprintf(buf, sizeof(buf), "%s.%s", section, name);
  |^~
In function ‘config_section_item’,
inlined from ‘config_global_item’ at config.c:371:9,
inlined from ‘config_create’ at config.c:931:8:
config.c:364:9: note: ‘snprintf’ output between 8 and 9687 bytes into a 
destination of size 140
  364 | snprintf(buf, sizeof(buf), "%s.%s", section, name);
  | ^~~~~~

Signed-off-by: Miroslav Lichvar 
---
 config.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index e454c91..e21cde7 100644
--- a/config.c
+++ b/config.c
@@ -361,7 +361,8 @@ static struct config_item *config_section_item(struct 
config *cfg,
 {
char buf[CONFIG_LABEL_SIZE + MAX_IFNAME_SIZE];
 
-   snprintf(buf, sizeof(buf), "%s.%s", section, name);
+   if (snprintf(buf, sizeof(buf), "%s.%s", section, name) >= sizeof(buf))
+   return NULL;
return hash_lookup(cfg->htab, buf);
 }
 
@@ -918,7 +919,11 @@ struct config *config_create(void)
for (i = 0; i < N_CONFIG_ITEMS; i++) {
ci = &config_tab[i];
ci->flags |= CFG_ITEM_STATIC;
-   snprintf(buf, sizeof(buf), "global.%s", ci->label);
+   if (snprintf(buf, sizeof(buf), "global.%s", ci->label) >=
+   sizeof(buf)) {
+   fprintf(stderr, "option %s too long\n", ci->label);
+   goto fail;
+   }
if (hash_insert(cfg->htab, buf, ci)) {
fprintf(stderr, "duplicate item %s\n", ci->label);
goto fail;
-- 
2.37.3



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


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-14 Thread Miroslav Lichvar
On Thu, Nov 10, 2022 at 09:51:52AM -0800, Jacob Keller wrote:
> How specific is this to chronyd?

AFAIK no other application implements the server side of the protocol.

> Would it make sense to call this chronysock
> instead of just sock?

Yes, that makes sense. If there are no other issues with the patches,
I can resend.

> The implementation seems fine but its using an interface that was defined by
> chrony. I suppose another application could implement the same interface
> though..

ntpsec might be interested in implementing it. We'll see.

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH 3/4] timemaster: Replace shm_segment with refclock_id.

2022-11-10 Thread Miroslav Lichvar
Replace the shm_segment number with a more general refclock_id and shift
by first_shm_index only in the SHM-specific context to allow other types
of refclocks to be used.

Signed-off-by: Miroslav Lichvar 
---
 timemaster.c | 48 +++-
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/timemaster.c b/timemaster.c
index cafd3a3..9fea011 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -658,8 +658,9 @@ static char **get_ptp4l_command(struct program_config 
*config,
return command;
 }
 
-static char **get_phc2sys_command(struct program_config *config, int domain,
- int poll, int shm_segment, char *uds_path,
+static char **get_phc2sys_command(struct program_config *config,
+ struct timemaster_config *tconfig, int domain,
+ int poll, int refclock_id, char *uds_path,
  char *message_tag)
 {
char **command = (char **)parray_new();
@@ -674,7 +675,9 @@ static char **get_phc2sys_command(struct program_config 
*config, int domain,
  xstrdup("-t"), xstrdup(message_tag),
  xstrdup("-n"), string_newf("%d", domain),
  xstrdup("-E"), xstrdup("ntpshm"),
- xstrdup("-M"), string_newf("%d", shm_segment), NULL);
+ xstrdup("-M"), string_newf("%d", refclock_id +
+tconfig->first_shm_segment),
+ NULL);
 
return command;
 }
@@ -702,11 +705,13 @@ static void add_command(char **command, int command_group,
parray_append((void ***)&script->command_groups, group);
 }
 
-static void add_shm_source(int shm_segment, int poll, int dpoll, double delay,
-  char *ntp_options, char *prefix,
-  struct timemaster_config *config, char **ntp_config)
+static void add_refclock_source(int refclock_id, int poll, int dpoll,
+   double delay, char *ntp_options, char *prefix,
+   struct timemaster_config *config,
+   char **ntp_config)
 {
-   char *refid = get_refid(prefix, shm_segment);
+   int shm_segment = refclock_id + config->first_shm_segment;
+   char *refid = get_refid(prefix, refclock_id);
 
switch (config->ntp_program) {
case CHRONYD:
@@ -758,7 +763,7 @@ static int add_vclock(struct script *script, int 
pclock_index)
 }
 
 static int add_ptp_source(struct ptp_domain *source,
- struct timemaster_config *config, int *shm_segment,
+ struct timemaster_config *config, int *refclock_id,
  int *command_group, int ***allocated_phcs,
  char **ntp_config, struct script *script)
 {
@@ -867,9 +872,9 @@ static int add_ptp_source(struct ptp_domain *source,
}
 
uds_path = string_newf("%s/ptp4l.%d.socket",
-  config->rundir, *shm_segment);
+  config->rundir, *refclock_id);
uds_path2 = string_newf("%s/ptp4lro.%d.socket",
-   config->rundir, *shm_segment);
+   config->rundir, *refclock_id);
 
message_tag = string_newf("[%d", source->domain);
for (j = 0; interfaces[j]; j++)
@@ -879,7 +884,7 @@ static int add_ptp_source(struct ptp_domain *source,
 
config_file = xmalloc(sizeof(*config_file));
config_file->path = string_newf("%s/ptp4l.%d.conf",
-   config->rundir, *shm_segment);
+   config->rundir, *refclock_id);
 
config_file->content = xstrdup("[global]\n");
if (*config->ptp4l.settings) {
@@ -905,10 +910,10 @@ static int add_ptp_source(struct ptp_domain *source,
vclock_index, 1);
add_command(command, *command_group, script);
 
-   command = get_phc2sys_command(&config->phc2sys,
+   command = get_phc2sys_command(&config->phc2sys, config,
  source->domain,
  source->phc2sys_poll,
- *shm_segment, uds_path,
+ *refclock_id, uds_path,
  message_tag);
add_co

[Linuxptp-devel] [PATCH 0/4] Support for chrony SOCK refclock

2022-11-10 Thread Miroslav Lichvar
This patchset fixes a bug in man page, adds a new servo for the chrony
SOCK refclock protocol as a more secure alternative to the NTP SHM
(the commit message describes the issue), and updates timemaster to
use it.

Miroslav Lichvar (4):
  Remove obsolete statement in ptp4l man page.
  Add sock servo.
  timemaster: Replace shm_segment with refclock_id.
  timemaster: Use sock servo with chrony.

 chronysock.c| 163 
 chronysock.h|  26 +++
 config.c|   2 +
 configs/default.cfg |   1 +
 makefile|   2 +-
 phc2sys.8   |   9 ++-
 phc2sys.c   |   3 +
 ptp4l.8 |  12 ++--
 servo.c |   4 ++
 servo.h |   1 +
 timemaster.8|  10 +--
 timemaster.c|  91 +
 12 files changed, 284 insertions(+), 40 deletions(-)
 create mode 100644 chronysock.c
 create mode 100644 chronysock.h

-- 
2.37.3



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


[Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-10 Thread Miroslav Lichvar
Add a second servo that provides samples to other processes in order to
control the clock. The chrony SOCK refclock uses a Unix domain socket
instead of a shared memory segment.

The main advantage over the NTP SHM refclock is better security as the
socket can be located in a directory with restricted access, while the
shared memory segment (using a predictable key) can be created by
untrusted users or applications if they can run before ptp4l/phc2sys and
chronyd/ntpd. There doesn't seem to a backward-compatible fix of the
protocol as both sides are expected to be able to create the segment if
it doesn't exist yet, possibly under a non-root owner, there is no
authentication of messages, and the protocol cannot be restarted if one
side decides to remove and recreate the segment.

Signed-off-by: Miroslav Lichvar 
---
 chronysock.c| 163 
 chronysock.h|  26 +++
 config.c|   2 +
 configs/default.cfg |   1 +
 makefile|   2 +-
 phc2sys.8   |   9 ++-
 phc2sys.c   |   3 +
 ptp4l.8 |  12 ++--
 servo.c |   4 ++
 servo.h |   1 +
 10 files changed, 216 insertions(+), 7 deletions(-)
 create mode 100644 chronysock.c
 create mode 100644 chronysock.h

diff --git a/chronysock.c b/chronysock.c
new file mode 100644
index 000..a8c0033
--- /dev/null
+++ b/chronysock.c
@@ -0,0 +1,163 @@
+/**
+ * @file chronysock.c
+ * @brief Implements a servo providing samples to chronyd over socket.
+ * @note Copyright (C) 2022 Miroslav Lichvar 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#include 
+#include 
+#include 
+#include 
+
+#include "chronysock.h"
+#include "config.h"
+#include "print.h"
+#include "servo_private.h"
+
+#define LEAP_NORMAL 0
+#define LEAP_INSERT 1
+#define LEAP_DELETE 2
+#define SOCK_MAGIC 0x534f434b
+
+/* Copied from chrony-3.2/refclock_sock.c */
+struct sock_sample {
+   /* Time of the measurement (system time) */
+   struct timeval tv;
+
+   /* Offset between the true time and the system time (in seconds) */
+   double offset;
+
+   /* Non-zero if the sample is from a PPS signal, i.e. another source
+  is needed to obtain seconds */
+   int pulse;
+
+   /* 0 - normal, 1 - insert leap second, 2 - delete leap second */
+   int leap;
+
+   /* Padding, ignored */
+   int _pad;
+
+   /* Protocol identifier (0x534f434b) */
+   int magic;
+};
+
+struct sock_servo {
+   struct servo servo;
+   int sock_fd;
+   int leap;
+};
+
+static void chronysock_destroy(struct servo *servo)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+   free(s);
+}
+
+static double chronysock_sample(struct servo *servo,
+   int64_t offset,
+   uint64_t local_ts,
+   double weight,
+   enum servo_state *state)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+   struct sock_sample sample;
+
+   memset(&sample, 0, sizeof(sample));
+   sample.tv.tv_sec = local_ts / 10ULL;
+   sample.tv.tv_usec = local_ts % 10ULL / 1000U;
+   sample.offset = -offset / 1e9;
+   sample.magic = SOCK_MAGIC;
+
+   switch (s->leap) {
+   case -1:
+   sample.leap = LEAP_DELETE;
+   break;
+   case 1:
+   sample.leap = LEAP_INSERT;
+   break;
+   default:
+   sample.leap = LEAP_NORMAL;
+   }
+
+   if (send(s->sock_fd, &sample, sizeof sample, 0) != sizeof sample) {
+   pr_err("chronysock: send failed: %m");
+   return 0;
+   }
+
+   *state = SERVO_UNLOCKED;
+   return 0.0;
+}
+
+static void chronysock_sync_interval(struct servo *servo, double interval)
+{
+}
+
+static void chronysock_reset(struct servo *servo)
+{
+}
+
+static void chronysock_leap(struct servo *servo, int leap)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+
+   s->leap = leap;
+}
+
+struct servo *chronysock_servo_create(struct config *cfg)
+{
+   char *addr = confi

[Linuxptp-devel] [PATCH 1/4] Remove obsolete statement in ptp4l man page.

2022-11-10 Thread Miroslav Lichvar
The NTP SHM number is no longer the PTP domain number. It was made
configurable and the default value is 0.

Fixes: 3760f8b6537e ("Add option to set NTP SHM segment number.")
Signed-off-by: Miroslav Lichvar 
---
 ptp4l.8 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ptp4l.8 b/ptp4l.8
index 1268802..280d9e1 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -528,8 +528,8 @@ The default is 0 (disabled).
 The servo which is used to synchronize the local clock. Valid values
 are "pi" for a PI controller, "linreg" for an adaptive controller
 using linear regression, "ntpshm" for the NTP SHM reference clock to
-allow another process to synchronize the local clock (the SHM segment
-number is set to the domain number), and "nullf" for a servo that
+allow another process to synchronize the local clock, and "nullf"
+for a servo that
 always dials frequency offset zero (for use in SyncE nodes).
 The default is "pi."
 .TP
-- 
2.37.3



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


[Linuxptp-devel] [PATCH 4/4] timemaster: Use sock servo with chrony.

2022-11-10 Thread Miroslav Lichvar
If chronyd is selected as the NTP program, use the SOCK refclock instead
of SHM to improve security and reduce delay in receiving of samples.

Signed-off-by: Miroslav Lichvar 
---
 timemaster.8 | 10 -
 timemaster.c | 57 +++-
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/timemaster.8 b/timemaster.8
index 2891a97..bc0b5b6 100644
--- a/timemaster.8
+++ b/timemaster.8
@@ -17,9 +17,9 @@ timemaster - run NTP with PTP as reference clocks
 \fBtimemaster\fR is a program that uses \fBptp4l\fR and \fBphc2sys\fR in
 combination with \fBchronyd\fR or \fBntpd\fR to synchronize the system clock to
 NTP and PTP time sources. The PTP time is provided by \fBphc2sys\fR and
-\fBptp4l\fR via SHM reference clocks to \fBchronyd\fR/\fBntpd\fR, which
-can compare all time sources and use the best sources to synchronize the system
-clock.
+\fBptp4l\fR via SOCK reference clock to \fBchronyd\fR or SHM reference clock to
+\fBntpd\fR, which can compare all time sources and use the best sources to
+synchronize the system clock.
 
 On start, \fBtimemaster\fR reads a configuration file that specifies the NTP
 and PTP time sources, checks which network interfaces have and share a PTP
@@ -154,14 +154,14 @@ specified also in other PTP domains and virtual clocks 
are disabled, only the
 
 .TP
 .B ntp_poll
-Specify the polling interval of the NTP SHM reference clock reading samples
+Specify the polling interval of the SOCK/SHM reference clock reading samples
 from \fBptp4l\fR or \fBphc2sys\fR. It's specified as a power of two in seconds.
 The default value is 2 (4 seconds).
 
 .TP
 .B phc2sys_poll
 Specify the polling interval used by \fBphc2sys\fR to read a PTP clock
-synchronized by \fBptp4l\fR and update the SHM sample for
+synchronized by \fBptp4l\fR and update the SOCK/SHM sample for
 \fBchronyd\fR/\fBntpd\fR. It's specified as a power of two in seconds. The
 default value is 0 (1 second).
 
diff --git a/timemaster.c b/timemaster.c
index 9fea011..e3cdbc0 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -674,10 +674,24 @@ static char **get_phc2sys_command(struct program_config 
*config,
  xstrdup("-z"), xstrdup(uds_path),
  xstrdup("-t"), xstrdup(message_tag),
  xstrdup("-n"), string_newf("%d", domain),
- xstrdup("-E"), xstrdup("ntpshm"),
- xstrdup("-M"), string_newf("%d", refclock_id +
-tconfig->first_shm_segment),
- NULL);
+ xstrdup("-E"), NULL);
+
+   switch (tconfig->ntp_program) {
+   case CHRONYD:
+   parray_extend((void ***)&command,
+ xstrdup("sock"), xstrdup("--chrony_sock_address"),
+ string_newf("%s/chrony.SOCK%d",
+ tconfig->rundir, refclock_id),
+ NULL);
+   break;
+   case NTPD:
+   parray_extend((void ***)&command,
+ xstrdup("ntpshm"), xstrdup("-M"),
+ string_newf("%d", refclock_id +
+ tconfig->first_shm_segment),
+ NULL);
+   break;
+   }
 
return command;
 }
@@ -705,21 +719,24 @@ static void add_command(char **command, int command_group,
parray_append((void ***)&script->command_groups, group);
 }
 
-static void add_refclock_source(int refclock_id, int poll, int dpoll,
+static void add_refclock_source(int refclock_id, int poll, int phc_poll,
double delay, char *ntp_options, char *prefix,
struct timemaster_config *config,
char **ntp_config)
 {
-   int shm_segment = refclock_id + config->first_shm_segment;
+   int filter, shm_segment = refclock_id + config->first_shm_segment;
char *refid = get_refid(prefix, refclock_id);
 
switch (config->ntp_program) {
case CHRONYD:
+   /* set filter to the expected number of samples per poll */
+   filter = (poll >= phc_poll) ? 1 << (poll - phc_poll) : 1;
string_appendf(ntp_config,
-  "refclock SHM %d poll %d dpoll %d "
-  "refid %s precision 1.0e-9 delay %.1e %s\n",
-  shm_segment, poll, dpoll, refid, delay,
-  ntp_options);
+  "refclock SOCK %s/chrony.SOCK%d poll %d "
+  "filter %d refid %s precision 1.0e-9 "
+

Re: [Linuxptp-devel] [PATCH] phc_ctl: add pin_cfg command to display pin functions

2022-10-27 Thread Miroslav Lichvar
On Thu, Oct 27, 2022 at 03:32:48AM -0700, Jacob Keller wrote:
> Add a new function to phc_ctl to display the devices pin configuration
> data. First, obtain the device capabilities to determine the number of
> pins. Then, for each pin, print the name, function, and channel
> information.

Nice feature. In a quick test, it printed the state correctly after
setting it with "testptp -L".

Do you plan to add also pin_set?

-- 
Miroslav Lichvar



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


Re: [Linuxptp-devel] [PATCH] config: allow fractional freq_est_interval

2022-10-27 Thread Miroslav Lichvar
On Thu, Oct 27, 2022 at 03:29:40AM -0700, Jacob Keller wrote:
> --- a/config.c
> +++ b/config.c
> @@ -254,7 +254,7 @@ struct config_item config_tab[] = {
>   GLOB_ITEM_DBL("first_step_threshold", 0.2, 0.0, DBL_MAX),
>   PORT_ITEM_INT("follow_up_info", 0, 0, 1),
>   GLOB_ITEM_INT("free_running", 0, 0, 1),
> - PORT_ITEM_INT("freq_est_interval", 1, 0, INT_MAX),
> + PORT_ITEM_INT("freq_est_interval", 1, INT_MIN, INT_MAX),
>   GLOB_ITEM_INT("G.8275.defaultDS.localPriority", 128, 1, UINT8_MAX),
>   PORT_ITEM_INT("G.8275.portDS.localPriority", 128, 1, UINT8_MAX),
>   GLOB_ITEM_INT("gmCapable", 1, 0, 1),

Looks good to me.

Thanks,

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH v3 3/4] Don't accept errors in clockadj_get_freq().

2022-10-24 Thread Miroslav Lichvar
Exit if an error is returned from the clock_adjtime() call in
clockadj_get_freq(). No recoverable errors are expected.

Signed-off-by: Miroslav Lichvar 
---
 clockadj.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clockadj.c b/clockadj.c
index 957dc57..4c920b9 100644
--- a/clockadj.c
+++ b/clockadj.c
@@ -19,6 +19,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -72,6 +73,7 @@ double clockadj_get_freq(clockid_t clkid)
memset(&tx, 0, sizeof(tx));
if (clock_adjtime(clkid, &tx) < 0) {
pr_err("failed to read out the clock frequency adjustment: %m");
+   exit(1);
} else {
f = tx.freq / 65.536;
if (clkid == CLOCK_REALTIME && realtime_nominal_tick && tx.tick)
-- 
2.37.3



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


[Linuxptp-devel] [PATCH v3 4/4] Extend clockcheck to check for changes in frequency.

2022-10-24 Thread Miroslav Lichvar
Before setting the new frequency offset on a clock update, compare the
current frequency returned by the kernel with the value saved from the
previous update. Print a warning message if the difference is larger
than 1 ppb, allowing for rounding errors in conversion to and from
double. The kernel caches the value set by clock_adjtime() in shifted
ppm, it doesn't request it from the driver, which can have a lower
resulution.

This should detect misconfigurations where multiple processes are trying
to control the clock (e.g. another ptp4l/phc2sys instance or an NTP
client), even when they don't step the clock.

Signed-off-by: Miroslav Lichvar 
---
 clock.c  |  3 +++
 clockcheck.c | 10 ++
 clockcheck.h |  8 
 phc2sys.c|  3 +++
 ptp4l.8  |  6 --
 5 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/clock.c b/clock.c
index 46ac9c2..8177e77 100644
--- a/clock.c
+++ b/clock.c
@@ -1776,6 +1776,9 @@ static void clock_step_window(struct clock *c)
 
 static void clock_synchronize_locked(struct clock *c, double adj)
 {
+   if (c->sanity_check) {
+   clockcheck_freq(c->sanity_check, clockadj_get_freq(c->clkid));
+   }
clockadj_set_freq(c->clkid, -adj);
if (c->clkid == CLOCK_REALTIME) {
sysclk_set_sync();
diff --git a/clockcheck.c b/clockcheck.c
index f0141be..b5a69cc 100644
--- a/clockcheck.c
+++ b/clockcheck.c
@@ -123,6 +123,16 @@ void clockcheck_set_freq(struct clockcheck *cc, int freq)
cc->freq_known = 1;
 }
 
+int clockcheck_freq(struct clockcheck *cc, int freq)
+{
+   /* Allow difference of 1 ppb due to conversion to/from double */
+   if (cc->freq_known && abs(cc->current_freq - freq) > 1) {
+   pr_warning("clockcheck: clock frequency changed unexpectedly!");
+   return 1;
+   }
+   return 0;
+}
+
 void clockcheck_step(struct clockcheck *cc, int64_t step)
 {
if (cc->last_ts)
diff --git a/clockcheck.h b/clockcheck.h
index 1ff86eb..4b09b98 100644
--- a/clockcheck.h
+++ b/clockcheck.h
@@ -54,6 +54,14 @@ int clockcheck_sample(struct clockcheck *cc, uint64_t ts);
  */
 void clockcheck_set_freq(struct clockcheck *cc, int freq);
 
+/**
+ * Check whether the frequency correction did not change unexpectedly.
+ * @param cc   Pointer to a clock check obtained via @ref clockcheck_create().
+ * @param freq Current reading of the frequency correction in ppb.
+ * @return Zero if the frequency did not change, non-zero otherwise.
+ */
+int clockcheck_freq(struct clockcheck *cc, int freq);
+
 /**
  * Inform clock check that the clock was stepped.
  * @param cc   Pointer to a clock check obtained via @ref clockcheck_create().
diff --git a/phc2sys.c b/phc2sys.c
index ebc43e5..88ed00c 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -561,6 +561,9 @@ static void update_clock(struct phc2sys_private *priv, 
struct clock *clock,
/* Fall through. */
case SERVO_LOCKED:
case SERVO_LOCKED_STABLE:
+   if (clock->sanity_check)
+   clockcheck_freq(clock->sanity_check,
+   clockadj_get_freq(clock->clkid));
clockadj_set_freq(clock->clkid, -ppb);
if (clock->clkid == CLOCK_REALTIME)
sysclk_set_sync();
diff --git a/ptp4l.8 b/ptp4l.8
index 1268802..cd6299f 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -619,8 +619,10 @@ This option used to be called
 The maximum allowed frequency offset between uncorrected clock and the system
 monotonic clock in parts per billion (ppb). This is used as a sanity check of
 the synchronized clock. When a larger offset is measured, a warning message
-will be printed and the servo will be reset. When set to 0, the sanity check is
-disabled. The default is 2 (20%).
+will be printed and the servo will be reset. If the frequency correction set by
+ptp4l changes unexpectedly between updates of the clock (e.g. due to another
+process trying to control the clock), a warning message will be printed. When
+set to 0, the sanity check is disabled. The default is 2 (20%).
 .TP
 .B initial_delay
 The initial path delay of the clock in nanoseconds used for synchronization of
-- 
2.37.3



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


[Linuxptp-devel] [PATCH v3 0/4] Bug fix and improved clockcheck

2022-10-24 Thread Miroslav Lichvar
v3:
- improved documentation

v2:
- added patch to exit on errors returned by read-only clock_adjtime()
- simplified last patch to reuse the set frequency and allow +/-1 ppb
  error due to conversions between int, double and scaled ppm
- improved commit messages

This set improves the clock check to consider the last frequency set by
ptp4l/phc2sys in order to detect cases where multiple processes are
trying to control the same clock due to a misconfiguration or bug.

Miroslav Lichvar (4):
  phc2sys: Add clocks after processing configuration.
  Drop support for old kernels returning zero frequency.
  Don't accept errors in clockadj_get_freq().
  Extend clockcheck to check for changes in frequency.

 clock.c  | 10 +++---
 clockadj.c   |  2 ++
 clockcheck.c | 10 ++
 clockcheck.h |  8 
 phc2sys.8|  2 +-
 phc2sys.c| 48 ++--
 ptp4l.8  |  4 +++-
 ts2phc.c |  7 ---
 8 files changed, 53 insertions(+), 38 deletions(-)

-- 
2.37.3



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


[Linuxptp-devel] [PATCH v3 1/4] phc2sys: Add clocks after processing configuration.

2022-10-24 Thread Miroslav Lichvar
Clocks specified by the -c option, or the default CLOCK_REALTIME, were
added before the default or specified values (e.g. sanity_freq_limit)
were set in the private structure used by clock_add(), causing the
clocks to work with unexpected values.

Rework the code to save the names of sink clocks from command line and
add them only after the configuration is complete.

This also fixes the automatic mode to not add CLOCK_REALTIME twice.

Fixes: 33ac7d25cd92 ("phc2sys: Allow multiple sink clocks")

Signed-off-by: Miroslav Lichvar 
---
 phc2sys.8 |  2 +-
 phc2sys.c | 41 +++--
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/phc2sys.8 b/phc2sys.8
index 6de2d25..9825ec7 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -120,7 +120,7 @@ Specify the time sink by device (e.g. /dev/ptp1) or 
interface (e.g. eth1) or
 by  name. The default is CLOCK_REALTIME (the system clock). Not compatible
 with the
 .B \-a
-option. This option may be given multiple times.
+option. This option may be given up to 128 times.
 .TP
 .BI \-E " servo"
 Specify which clock servo should be used. Valid values are pi for a PI
diff --git a/phc2sys.c b/phc2sys.c
index 2c8e905..8d2624f 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -64,6 +64,8 @@
 
 #define PHC_PPS_OFFSET_LIMIT 1000
 
+#define MAX_DST_CLOCKS 128
+
 struct clock {
LIST_ENTRY(clock) list;
LIST_ENTRY(clock) dst_list;
@@ -1056,9 +1058,10 @@ static void usage(char *progname)
 
 int main(int argc, char *argv[])
 {
-   char *config = NULL, *last_dst_name = NULL, *progname, *src_name = NULL;
+   char *config = NULL, *progname, *src_name = NULL;
+   const char *dst_names[MAX_DST_CLOCKS];
char uds_local[MAX_IFNAME_SIZE + 1];
-   int autocfg = 0, c, domain_number = 0, index, ntpshm_segment, offset;
+   int i, autocfg = 0, c, domain_number = 0, index, ntpshm_segment, offset;
int pps_fd = -1, print_level = LOG_INFO, r = -1, rt = 0;
int wait_sync = 0, dst_cnt = 0;
struct config *cfg;
@@ -1104,13 +1107,11 @@ int main(int argc, char *argv[])
rt++;
break;
case 'c':
-   last_dst_name = optarg;
-   r = phc2sys_static_dst_configuration(&priv,
-last_dst_name);
-   if (r) {
-   goto end;
+   if (dst_cnt == MAX_DST_CLOCKS) {
+   fprintf(stderr, "too many sink clocks\n");
+   goto bad_usage;
}
-   dst_cnt++;
+   dst_names[dst_cnt++] = optarg;
break;
case 'd':
pps_fd = open(optarg, O_RDONLY);
@@ -1261,7 +1262,11 @@ int main(int argc, char *argv[])
return c;
}
 
-   if (autocfg && (src_name || last_dst_name || hardpps_configured(pps_fd) 
||
+   if (!autocfg && dst_cnt == 0) {
+   dst_names[dst_cnt++] = "CLOCK_REALTIME";
+   }
+
+   if (autocfg && (src_name || dst_cnt > 0 || hardpps_configured(pps_fd) ||
wait_sync || priv.forced_sync_offset)) {
fprintf(stderr,
"autoconfiguration cannot be mixed with manual config 
options.\n");
@@ -1279,15 +1284,8 @@ int main(int argc, char *argv[])
goto bad_usage;
}
 
-   if (!last_dst_name) {
-   last_dst_name = "CLOCK_REALTIME";
-   r = phc2sys_static_dst_configuration(&priv, last_dst_name);
-   if (r) {
-   goto end;
-   }
-   }
-   if (hardpps_configured(pps_fd) && (dst_cnt > 1 ||
-   strcmp(last_dst_name, "CLOCK_REALTIME"))) {
+   if (hardpps_configured(pps_fd) && (dst_cnt != 1 ||
+   strcmp(dst_names[0], "CLOCK_REALTIME"))) {
fprintf(stderr,
"cannot use a pps device unless destination is 
CLOCK_REALTIME\n");
goto bad_usage;
@@ -1308,6 +1306,13 @@ int main(int argc, char *argv[])
priv.kernel_leap = config_get_int(cfg, NULL, "kernel_leap");
priv.sanity_freq_limit = config_get_int(cfg, NULL, "sanity_freq_limit");
 
+   for (i = 0; i < dst_cnt; i++) {
+   r = phc2sys_static_dst_configuration(&priv, dst_names[i]);
+   if (r) {
+   goto end;
+   }
+   }
+
snprintf(uds_local, sizeof(uds_local), "/var/run/phc2sys.%d",
 getpid());
 
-- 
2.37.3



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


[Linuxptp-devel] [PATCH v3 2/4] Drop support for old kernels returning zero frequency.

2022-10-24 Thread Miroslav Lichvar
Kernels before 3.10 had a bug in reading of the system clock frequency,
which was worked around by commit da347d7a36f2 ("ptp4l: Set clock
frequency on start").

Drop this workaround and support for the old kernels to make
clockadj_get_freq() useful.

Signed-off-by: Miroslav Lichvar 
---
 clock.c   | 7 ---
 phc2sys.c | 4 
 ts2phc.c  | 7 ---
 3 files changed, 18 deletions(-)

diff --git a/clock.c b/clock.c
index d37bb87..46ac9c2 100644
--- a/clock.c
+++ b/clock.c
@@ -1144,12 +1144,6 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
 
if (c->clkid != CLOCK_INVALID) {
fadj = (int) clockadj_get_freq(c->clkid);
-   /* Due to a bug in older kernels, the reading may silently fail
-  and return 0. Set the frequency back to make sure fadj is
-  the actual frequency of the clock. */
-   if (!c->free_running) {
-   clockadj_set_freq(c->clkid, fadj);
-   }
/* Disable write phase mode if not implemented by driver */
if (c->write_phase_mode && !phc_has_writephase(c->clkid)) {
pr_err("clock does not support write phase mode");
@@ -1755,7 +1749,6 @@ int clock_switch_phc(struct clock *c, int phc_index)
return -1;
}
fadj = (int) clockadj_get_freq(clkid);
-   clockadj_set_freq(clkid, fadj);
servo = servo_create(c->config, c->servo_type, -fadj, max_adj, 0);
if (!servo) {
pr_err("Switching PHC, failed to create clock servo");
diff --git a/phc2sys.c b/phc2sys.c
index 8d2624f..ebc43e5 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -128,10 +128,6 @@ static struct servo *servo_add(struct phc2sys_private 
*priv,
 
clockadj_init(clock->clkid);
ppb = clockadj_get_freq(clock->clkid);
-   /* The reading may silently fail and return 0, reset the frequency to
-  make sure ppb is the actual frequency of the clock. */
-   if (!priv->free_running)
-   clockadj_set_freq(clock->clkid, ppb);
if (clock->clkid == CLOCK_REALTIME) {
sysclk_set_leap(0);
max_ppb = sysclk_max_freq();
diff --git a/ts2phc.c b/ts2phc.c
index f7a57e4..f345370 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -129,13 +129,6 @@ static struct servo *ts2phc_servo_create(struct 
ts2phc_private *priv,
int fadj, max_adj;
 
fadj = (int) clockadj_get_freq(clock->clkid);
-   /* Due to a bug in older kernels, the reading may silently fail
-* and return 0. Set the frequency back to make sure fadj is
-* the actual frequency of the clock.
-*/
-   if (!clock->no_adj) {
-   clockadj_set_freq(clock->clkid, fadj);
-   }
 
max_adj = phc_max_adj(clock->clkid);
 
-- 
2.37.3



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


Re: [Linuxptp-devel] [PATCH v2 4/4] Extend clockcheck to check for changes in frequency.

2022-10-24 Thread Miroslav Lichvar
On Thu, Oct 20, 2022 at 04:50:37PM +, Keller, Jacob E wrote:
> > index f0141be..b5a69cc 100644
> > --- a/clockcheck.c
> > +++ b/clockcheck.c
> > @@ -123,6 +123,16 @@ void clockcheck_set_freq(struct clockcheck *cc, int 
> > freq)
> > cc->freq_known = 1;
> >  }
> > 
> > +int clockcheck_freq(struct clockcheck *cc, int freq)
> > +{
> > +   /* Allow difference of 1 ppb due to conversion to/from double */
> > +   if (cc->freq_known && abs(cc->current_freq - freq) > 1) {
> > +   pr_warning("clockcheck: clock frequency changed
> > unexpectedly!");
> 
> 
> The modified documentation would make me think this should allow up to the 
> sanity_freq_limit as a difference? Perhaps the documentation should be 
> improved somewhat to clarify the difference?

If I expand the description a bit, is it better?

.B sanity_freq_limit
 
The maximum allowed frequency offset between uncorrected clock and the system   
 
monotonic clock in parts per billion (ppb). This is used as a sanity check of   
 
the synchronized clock. When a larger offset is measured, a warning message 
 
will be printed and the servo will be reset. If the frequency correction set by 
 
ptp4l changes unexpectedly between updates of the clock (e.g. due to another
 
process trying to control the clock), a warning message will be printed. When   
 
set to 0, the sanity check is disabled. The default is 2 (20%). 
 


If not, what would you suggest?

Thanks,

-- 
Miroslav Lichvar



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


[Linuxptp-devel] [PATCH v2 3/4] Don't accept errors in clockadj_get_freq().

2022-10-20 Thread Miroslav Lichvar
Exit if an error is returned from the clock_adjtime() call in
clockadj_get_freq(). No recoverable errors are expected.

Signed-off-by: Miroslav Lichvar 
---
 clockadj.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clockadj.c b/clockadj.c
index 957dc57..4c920b9 100644
--- a/clockadj.c
+++ b/clockadj.c
@@ -19,6 +19,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -72,6 +73,7 @@ double clockadj_get_freq(clockid_t clkid)
memset(&tx, 0, sizeof(tx));
if (clock_adjtime(clkid, &tx) < 0) {
pr_err("failed to read out the clock frequency adjustment: %m");
+   exit(1);
} else {
f = tx.freq / 65.536;
if (clkid == CLOCK_REALTIME && realtime_nominal_tick && tx.tick)
-- 
2.37.3



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


[Linuxptp-devel] [PATCH v2 0/4] Bug fix and improved clockcheck

2022-10-20 Thread Miroslav Lichvar
v2:
- added patch to exit on errors returned by read-only clock_adjtime()
- simplified last patch to reuse the set frequency and allow +/-1 ppb
  error due to conversions between int, double and scaled ppm
- improved commit messages

This set improves the clock check to consider the last frequency set by
ptp4l/phc2sys in order to detect cases where multiple processes are
trying to control the same clock due to a misconfiguration or bug.

Miroslav Lichvar (4):
  phc2sys: Add clocks after processing configuration.
  Drop support for old kernels returning zero frequency.
  Don't accept errors in clockadj_get_freq().
  Extend clockcheck to check for changes in frequency.

 clock.c  | 10 +++---
 clockadj.c   |  2 ++
 clockcheck.c | 10 ++
 clockcheck.h |  8 
 phc2sys.8|  2 +-
 phc2sys.c| 48 ++--
 ptp4l.8  |  4 +++-
 ts2phc.c |  7 ---
 8 files changed, 53 insertions(+), 38 deletions(-)

-- 
2.37.3



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


  1   2   3   4   5   6   7   8   >