[Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.

[Miroslav Lichvar]

When running multiple instances of ptp4l or phc2sys, it's difficult to
tell which log message belongs to which instance. Add new options to
ptp4l and phc2sys which can specify a prefix for all messages printed to
the standard output or system log, so messages from different instances
can have different prefixes.

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
config.c | 1 +
phc2sys.8 | 4 ++++
phc2sys.c | 9 +++++++--
print.c | 18 ++++++++++++++----
print.h | 1 +
ptp4l.8 | 10 ++++++++++
ptp4l.c | 8 +++++++-
7 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/config.c b/config.c
index 5da5ecc..89e349e 100644
--- a/config.c
+++ b/config.c
@@ -199,6 +199,7 @@ struct config_item config_tab[] = {
PORT_ITEM_INT("logMinPdelayReqInterval", 0, INT8_MIN, INT8_MAX),
PORT_ITEM_INT("logSyncInterval", 0, INT8_MIN, INT8_MAX),
GLOB_ITEM_INT("logging_level", LOG_INFO, PRINT_LEVEL_MIN, PRINT_LEVEL_MAX),
+ GLOB_ITEM_STR("logging_prefix", NULL),
GLOB_ITEM_STR("manufacturerIdentity", "00:00:00"),
GLOB_ITEM_INT("max_frequency", 900000000, 0, INT_MAX),
PORT_ITEM_INT("min_neighbor_prop_delay", -20000000, INT_MIN, -1),
diff --git a/phc2sys.8 b/phc2sys.8
index 22d02c2..e515bd1 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -206,6 +206,10 @@ The default is /var/run/ptp4l.
Set the maximum syslog level of messages which should be printed or sent to
the system logger. The default is 6 (LOG_INFO).
.TP
+.BI \-o " print-prefix"
+Specifies a prefix for messages printed to the standard output or system log.
+The default is empty string.
+.TP
.B \-m
Print messages to the standard output.
.TP
diff --git a/phc2sys.c b/phc2sys.c
index 35cf6fa..a37e9bc 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -1209,6 +1209,7 @@ static void usage(char *progname)
" -x apply leap seconds by servo instead of kernel\n"
" -z [path] server address for UDS (/var/run/ptp4l)\n"
" -l [num] set the logging level to 'num' (6)\n"
+ " -o [prefix] set prefix for log messages\n"
" -m print messages to stdout\n"
" -q do not print messages to the syslog\n"
" -v prints the software version and exits\n"
@@ -1219,7 +1220,7 @@ static void usage(char *progname)

int main(int argc, char *argv[])
{
- char *progname;
+ char *progname, *prefix = NULL;
char *src_name = NULL, *dst_name = NULL;
struct clock *src, *dst;
struct config *cfg;
@@ -1251,7 +1252,7 @@ int main(int argc, char *argv[])
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
while (EOF != (c = getopt(argc, argv,
- "arc:d:s:E:P:I:S:F:R:N:O:L:M:i:u:wn:xz:l:mqvh"))) {
+ "arc:d:s:E:P:I:S:F:R:N:O:L:M:i:u:wn:xz:l:o:mqvh"))) {
switch (c) {
case 'a':
autocfg = 1;
@@ -1363,6 +1364,9 @@ int main(int argc, char *argv[])
PRINT_LEVEL_MIN, PRINT_LEVEL_MAX))
goto end;
break;
+ case 'o':
+ prefix = optarg;
+ break;
case 'm':
verbose = 1;
break;
@@ -1405,6 +1409,7 @@ int main(int argc, char *argv[])
}

print_set_progname(progname);
+ print_set_prefix(prefix);
print_set_verbose(verbose);
print_set_syslog(use_syslog);
print_set_level(print_level);
diff --git a/print.c b/print.c
index a82d0e7..c1dd867 100644
--- a/print.c
+++ b/print.c
@@ -28,12 +28,18 @@ static int verbose = 0;
static int print_level = LOG_INFO;
static int use_syslog = 1;
static const char *progname;
+static const char *prefix;

void print_set_progname(const char *name)
{
progname = name;
}

+void print_set_prefix(const char *string)
+{
+ prefix = string;
+}
+
void print_set_syslog(int value)
{
use_syslog = value ? 1 : 0;
@@ -67,13 +73,17 @@ void print(int level, char const *format, ...)

if (verbose) {
f = level >= LOG_NOTICE ? stdout : stderr;
- fprintf(f, "%s[%ld.%03ld]: %s\n",
+ fprintf(f, "%s[%ld.%03ld]: %s%s%s\n",
progname ? progname : "",
- ts.tv_sec, ts.tv_nsec / 1000000, buf);
+ ts.tv_sec, ts.tv_nsec / 1000000,
+ prefix ? prefix : "", prefix ? " " : "",
+ buf);
fflush(f);
}
if (use_syslog) {
- syslog(level, "[%ld.%03ld] %s",
- ts.tv_sec, ts.tv_nsec / 1000000, buf);
+ syslog(level, "[%ld.%03ld] %s%s%s",
+ ts.tv_sec, ts.tv_nsec / 1000000,
+ prefix ? prefix : "", prefix ? " " : "",
+ buf);
}
}
diff --git a/print.h b/print.h
index e8f2c8e..6236a14 100644
--- a/print.h
+++ b/print.h
@@ -33,6 +33,7 @@ __attribute__ ((format (printf, 2, 3)))
void print(int level, char const *format, ...);

void print_set_progname(const char *name);
+void print_set_prefix(const char *prefix);
void print_set_syslog(int value);
void print_set_level(int level);
void print_set_verbose(int value);
diff --git a/ptp4l.8 b/ptp4l.8
index 63e9abd..6755ad7 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -12,6 +12,8 @@ ptp4l - PTP Boundary/Ordinary Clock
.BI \-p " phc-device"
] [
.BI \-l " print-level"
+] [
+.BI \-o " print-prefix"
]
[
.BI \-i " interface"
@@ -82,6 +84,10 @@ Enable the slaveOnly mode.
Set the maximum syslog level of messages which should be printed or sent to
the system logger. The default is 6 (LOG_INFO).
.TP
+.BI \-o " print-prefix"
+Specifies a prefix for messages printed to the standard output or system log.
+The default is empty string.
+.TP
.B \-m
Print messages to the standard output.
.TP
@@ -466,6 +472,10 @@ is 0.
The maximum logging level of messages which should be printed.
The default is 6 (LOG_INFO).
.TP
+.B logging_prefix
+Prefix for messages printed to the standard output or system log.
+The default is empty string.
+.TP
.B verbose
Print messages to the standard output if enabled.
The default is 0 (disabled).
diff --git a/ptp4l.c b/ptp4l.c
index a87e7e6..c4813cb 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -62,6 +62,7 @@ static void usage(char *progname)
" (ignored for SOFTWARE/LEGACY HW time stamping)\n"
" -s slave only mode (overrides configuration file)\n"
" -l [num] set the logging level to 'num'\n"
+ " -o [str] set prefix for log messages\n"
" -m print messages to stdout\n"
" -q do not print messages to the syslog\n"
" -v prints the software version and exits\n"
@@ -88,7 +89,7 @@ int main(int argc, char *argv[])
/* Process the command line arguments. */
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
- while (EOF != (c = getopt(argc, argv, "AEP246HSLf:i:p:sl:mqvh"))) {
+ while (EOF != (c = getopt(argc, argv, "AEP246HSLf:i:p:sl:o:mqvh"))) {
switch (c) {
case 'A':
if (config_set_int(cfg, "delay_mechanism", DM_AUTO))
@@ -150,6 +151,10 @@ int main(int argc, char *argv[])
goto out;
config_set_int(cfg, "logging_level", print_level);
break;
+ case 'o':
+ if (config_set_string(cfg, "logging_prefix", optarg))
+ goto out;
+ break;
case 'm':
config_set_int(cfg, "verbose", 1);
break;
@@ -176,6 +181,7 @@ int main(int argc, char *argv[])
}

print_set_progname(progname);
+ print_set_prefix(config_get_string(cfg, NULL, "logging_prefix"));
print_set_verbose(config_get_int(cfg, NULL, "verbose"));
print_set_syslog(config_get_int(cfg, NULL, "use_syslog"));
print_set_level(config_get_int(cfg, NULL, "logging_level"));

[Miroslav Lichvar]

Use the new options of ptp4l and phc2sys to prefix their log messages
with the PTP domain number and name of interface(s).

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
timemaster.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/timemaster.c b/timemaster.c
index 66ac521..9d6cc32 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -599,7 +599,8 @@ static char **get_ptp4l_command(struct program_config *config,
}

static char **get_phc2sys_command(struct program_config *config, int domain,
- int poll, int shm_segment, char *uds_path)
+ int poll, int shm_segment, char *uds_path,
+ char *log_prefix)
{
char **command = (char **)parray_new();

@@ -610,6 +611,7 @@ static char **get_phc2sys_command(struct program_config *config, int domain,
xstrdup("-R"), string_newf("%.2f", poll > 0 ?
1.0 / (1 << poll) : 1 << -poll),
xstrdup("-z"), xstrdup(uds_path),
+ xstrdup("-o"), xstrdup(log_prefix),
xstrdup("-n"), string_newf("%d", domain),
xstrdup("-E"), xstrdup("ntpshm"),
xstrdup("-M"), string_newf("%d", shm_segment), NULL);
@@ -671,7 +673,7 @@ static int add_ptp_source(struct ptp_domain *source,
struct script *script)
{
struct config_file *config_file;
- char **command, *uds_path, **interfaces;
+ char **command, *uds_path, **interfaces, *log_prefix;
int i, j, num_interfaces, *phc, *phcs, hw_ts;
struct sk_ts_info ts_info;

@@ -749,6 +751,12 @@ static int add_ptp_source(struct ptp_domain *source,
uds_path = string_newf("%s/ptp4l.%d.socket",
config->rundir, *shm_segment);

+ log_prefix = string_newf("[%d", source->domain);
+ for (j = 0; interfaces[j]; j++)
+ string_appendf(&log_prefix, "%s%s", j ? "+" : ":",
+ interfaces[j]);
+ string_appendf(&log_prefix, "]");
+
config_file = xmalloc(sizeof(*config_file));
config_file->path = string_newf("%s/ptp4l.%d.conf",
config->rundir, *shm_segment);
@@ -760,8 +768,9 @@ static int add_ptp_source(struct ptp_domain *source,
string_appendf(&config_file->content,
"slaveOnly 1\n"
"domainNumber %d\n"
- "uds_address %s\n",
- source->domain, uds_path);
+ "uds_address %s\n"
+ "logging_prefix %s\n",
+ source->domain, uds_path, log_prefix);

if (phcs[i] >= 0) {
/* HW time stamping */
@@ -772,7 +781,8 @@ static int add_ptp_source(struct ptp_domain *source,
command = get_phc2sys_command(&config->phc2sys,
source->domain,
source->phc2sys_poll,
- *shm_segment, uds_path);
+ *shm_segment, uds_path,
+ log_prefix);
parray_append((void ***)&script->commands, command);
} else {
/* SW time stamping */
@@ -793,6 +803,7 @@ static int add_ptp_source(struct ptp_domain *source,

(*shm_segment)++;

+ free(log_prefix);
free(uds_path);
free(interfaces);
}

[Miroslav Lichvar]

When an interface doesn't support HW time stamping, before falling back
to SW time stamping, check if it's actually supported and exit with an
error message if not.

Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
timemaster.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/timemaster.c b/timemaster.c
index 9d6cc32..d5b20d4 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -674,13 +674,15 @@ static int add_ptp_source(struct ptp_domain *source,
{
struct config_file *config_file;
char **command, *uds_path, **interfaces, *log_prefix;
- int i, j, num_interfaces, *phc, *phcs, hw_ts;
+ int i, j, num_interfaces, *phc, *phcs, hw_ts, sw_ts;
struct sk_ts_info ts_info;

pr_debug("adding PTP domain %d", source->domain);

hw_ts = SOF_TIMESTAMPING_TX_HARDWARE | SOF_TIMESTAMPING_RX_HARDWARE |
SOF_TIMESTAMPING_RAW_HARDWARE;
+ sw_ts = SOF_TIMESTAMPING_TX_SOFTWARE | SOF_TIMESTAMPING_RX_SOFTWARE |
+ SOF_TIMESTAMPING_SOFTWARE;

for (num_interfaces = 0;
source->interfaces[num_interfaces]; num_interfaces++)
@@ -702,9 +704,14 @@ static int add_ptp_source(struct ptp_domain *source,
return 1;
}

- if (!ts_info.valid ||
- ((ts_info.so_timestamping & hw_ts) != hw_ts)) {
+ if (((ts_info.so_timestamping & hw_ts) != hw_ts)) {
pr_debug("interface %s: no PHC", source->interfaces[i]);
+ if ((ts_info.so_timestamping & sw_ts) != sw_ts) {
+ pr_err("time stamping not supported on %s",
+ source->interfaces[i]);
+ free(phcs);
+ return 1;
+ }
continue;
}

[Keller, Jacob E]

When there are multiple instances of ptp4l or phc2sys running on the
system, it's difficult to tell which message belongs to which
instance. The first patch adds new options to ptp4l and phc2sys which
can be used to specify a different prefix for each instance, so
different instances can use different prefixes. One issue is that
it's
not possible to set the prefix in ptp4l config to its default value
(empty string) and the setting can't be included in defaults.cfg.

Is it possible to update configuration somehow to allow passing a
variable as "empty" which will assign it to empty string, and then
update your prefix code flow so that it correctly works and doesn't add
a " " when the string is actually empty? Not sure how easy it would be
to fix that or not.

Thanks,
Jake

The second patch adds support for this feature to timemaster.
The third patch is an unrelated improvement in timemaster to exit
early with an error message when an interface doesn't support neither
HW nor SW timestamping. (The patch applies only with the second
patch,
so it's included here.)
  Add options to prefix ptp4l and phc2sys log messages.
  timemaster: prefix ptp4l and phc2sys messages.
  timemaster: check support for SW time stamping.
 config.c     |  1 +
 phc2sys.8    |  4 ++++
 phc2sys.c    |  9 +++++++--
 print.c      | 18 ++++++++++++++----
 print.h      |  1 +
 ptp4l.8      | 10 ++++++++++
 ptp4l.c      |  9 ++++++++-
 timemaster.c | 34 ++++++++++++++++++++++++++--------
 8 files changed, 71 insertions(+), 15 deletions(-)

[Miroslav Lichvar]

Post by Keller, Jacob E

When there are multiple instances of ptp4l or phc2sys running on the
system, it's difficult to tell which message belongs to which
instance. The first patch adds new options to ptp4l and phc2sys which
can be used to specify a different prefix for each instance, so
different instances can use different prefixes. One issue is that
it's
not possible to set the prefix in ptp4l config to its default value
(empty string) and the setting can't be included in defaults.cfg.

Is it possible to update configuration somehow to allow passing a
variable as "empty" which will assign it to empty string, and then
update your prefix code flow so that it correctly works and doesn't add
a " " when the string is actually empty? Not sure how easy it would be
to fix that or not.

I think that would be an easy fix.

Another possibility is to allow double-quotes around strings in the
config. If the first and last character of the string was ", they
would be removed. The default value for logging_prefix would simply be
"". If someone is already using quoted strings in userDescription,
productDescription or revisionData, they would have to have an extra
pair of quotes around them.

What do you think?

[Richard Cochran]

Post by Miroslav Lichvar

Post by Keller, Jacob E
Is it possible to update configuration somehow to allow passing a
variable as "empty" which will assign it to empty string, and then
update your prefix code flow so that it correctly works and doesn't add
a " " when the string is actually empty? Not sure how easy it would be
to fix that or not.

I think that would be an easy fix.
Another possibility is to allow double-quotes around strings in the
config. If the first and last character of the string was ", they
would be removed. The default value for logging_prefix would simply be
"". If someone is already using quoted strings in userDescription,
productDescription or revisionData, they would have to have an extra
pair of quotes around them.
What do you think?

I don't think it is worth the effort to support null strings in the
config file. This opens up a whole can of worms of quoting and
escaping issues.

We already don't have every option as a default anyhow. Let's keep it
simple for now, and say "omit option from config for null".

Thanks,
Richard

[Richard Cochran]

Post by Miroslav Lichvar
When running multiple instances of ptp4l or phc2sys, it's difficult to
tell which log message belongs to which instance. Add new options to
ptp4l and phc2sys which can specify a prefix for all messages printed to
the standard output or system log, so messages from different instances
can have different prefixes.

This is a useful feature, but ...

Post by Miroslav Lichvar
+.BI \-o " print-prefix"
+Specifies a prefix for messages printed to the standard output or system log.
+The default is empty string.

Here we should add:

Omit this option from the configuration file in order to set the
tag to the empty string.

I said "tag" and not "prefix". See below...

Post by Miroslav Lichvar
@@ -67,13 +73,17 @@ void print(int level, char const *format, ...)
if (verbose) {
f = level >= LOG_NOTICE ? stdout : stderr;
- fprintf(f, "%s[%ld.%03ld]: %s\n",
+ fprintf(f, "%s[%ld.%03ld]: %s%s%s\n",

The string comes after the time stamp and after the program name. So
it really isn't a prefix at all. Maybe this should be called "tag"
instead?

Post by Miroslav Lichvar
progname ? progname : "",
- ts.tv_sec, ts.tv_nsec / 1000000, buf);
+ ts.tv_sec, ts.tv_nsec / 1000000,
+ prefix ? prefix : "", prefix ? " " : "",
+ buf);
fflush(f);
}
if (use_syslog) {
- syslog(level, "[%ld.%03ld] %s",
- ts.tv_sec, ts.tv_nsec / 1000000, buf);
+ syslog(level, "[%ld.%03ld] %s%s%s",
+ ts.tv_sec, ts.tv_nsec / 1000000,
+ prefix ? prefix : "", prefix ? " " : "",
+ buf);
}
}
diff --git a/ptp4l.c b/ptp4l.c
index a87e7e6..c4813cb 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -62,6 +62,7 @@ static void usage(char *progname)
" (ignored for SOFTWARE/LEGACY HW time stamping)\n"
" -s slave only mode (overrides configuration file)\n"
" -l [num] set the logging level to 'num'\n"
+ " -o [str] set prefix for log messages\n"

Do we really have to have a new command line option?

I don't want another one of these, especially not for ptp4l, and not
even for phc2sys or pmc. It just gets unwieldy and confusing.
Instead we should add '-f config' for phc2sys and pmc.

Post by Miroslav Lichvar
" -m print messages to stdout\n"
" -q do not print messages to the syslog\n"
" -v prints the software version and exits\n"

Thanks,
Richard

[Keller, Jacob E]

Post by Richard Cochran
Do we really have to have a new command line option?
I don't want another one of these, especially not for ptp4l, and not
even for phc2sys or pmc.  It just gets unwieldy and confusing.
Instead we should add '-f config' for phc2sys and pmc.

I agree. I think we can do that a lot easier now thanks to the work you
did to modernize the configuration settings.

Thanks,
Jake

[Miroslav Lichvar]

Post by Richard Cochran

Post by Miroslav Lichvar
@@ -67,13 +73,17 @@ void print(int level, char const *format, ...)
if (verbose) {
f = level >= LOG_NOTICE ? stdout : stderr;
- fprintf(f, "%s[%ld.%03ld]: %s\n",
+ fprintf(f, "%s[%ld.%03ld]: %s%s%s\n",

The string comes after the time stamp and after the program name. So
it really isn't a prefix at all. Maybe this should be called "tag"
instead?

Ok, so the option should be called print-tag, log-tag, message-tag, or
something else?

Post by Richard Cochran

Post by Miroslav Lichvar
@@ -62,6 +62,7 @@ static void usage(char *progname)
" (ignored for SOFTWARE/LEGACY HW time stamping)\n"
" -s slave only mode (overrides configuration file)\n"
" -l [num] set the logging level to 'num'\n"
+ " -o [str] set prefix for log messages\n"

Do we really have to have a new command line option?
I don't want another one of these, especially not for ptp4l, and not
even for phc2sys or pmc. It just gets unwieldy and confusing.
Instead we should add '-f config' for phc2sys and pmc.

I wouldn't mind if phc2sys and pmc supported config files, but I think
this particular option might be one of those that make more sense to
be used on command line rather than config file. If I wanted to run
multiple ptp4l instances with different tags for their log messages,
I could still share the config file between them if I specified the
interface and tag on the command line.

Would it be less confusing if we introduced GNU-style long option
names?

[Richard Cochran]

Post by Miroslav Lichvar
Ok, so the option should be called print-tag, log-tag, message-tag, or
something else?

'message_tag' seems best to me.

Post by Miroslav Lichvar
I wouldn't mind if phc2sys and pmc supported config files, but I think
this particular option might be one of those that make more sense to
be used on command line rather than config file. If I wanted to run
multiple ptp4l instances with different tags for their log messages,
I could still share the config file between them if I specified the
interface and tag on the command line.

Ok, that is a reason. But please can we have '-t' instead of '-o'?

-o makes me think of "output file".

Post by Miroslav Lichvar
Would it be less confusing if we introduced GNU-style long option
names?

Not to me. PTP has tons of options, and every new profile forces more
and more of these. I simply find it gross when programs have a
gazillion command line options. It makes for poor usability. I
really wanted ptp4l to have the "top ten" options available on the
command line, in order to keep it simple for the most common use
cases.

I think we should consider the long options for v2 if people really
want them...

Thanks,
Richard

[Dale Smith]

Post by Richard Cochran

Post by Miroslav Lichvar
Would it be less confusing if we introduced GNU-style long option
names?

Not to me. PTP has tons of options, and every new profile forces more
and more of these. I simply find it gross when programs have a
gazillion command line options. It makes for poor usability. I
really wanted ptp4l to have the "top ten" options available on the
command line, in order to keep it simple for the most common use
cases.

My $0.02

For an embeded linux system, I find command line args much easier to deal
with. Just pass in some $VARIABLE or maybe a $(command) in the script that
execs the command.
But for a config file, I need to edit it somehow. Maybe saving it in a
non-vol filesystem. Editing is either generating the whole file directly
or having
a template somewhere with some sed scripting (like autotools *.in) files.

A combination would be good I think. So a config file for the basic setup,
and command line overrides/additions for things that need to be
configurable or host-specific.

-Dale

[Keller, Jacob E]

-----Original Message-----
Sent: Friday, November 04, 2016 11:02 AM
Subject: Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and
phc2sys log messages.

Post by Miroslav Lichvar
Ok, so the option should be called print-tag, log-tag, message-tag, or
something else?

'message_tag' seems best to me.

I think you could still consider it a (as in one of many) prefix, as both of the things you said that appear before it may also be considered prefix. I don't mind the name "message_tag", though.

Post by Miroslav Lichvar
I wouldn't mind if phc2sys and pmc supported config files, but I think
this particular option might be one of those that make more sense to
be used on command line rather than config file. If I wanted to run
multiple ptp4l instances with different tags for their log messages,
I could still share the config file between them if I specified the
interface and tag on the command line.

Ok, that is a reason. But please can we have '-t' instead of '-o'?
-o makes me think of "output file".

Agreed, -t is better.

Post by Miroslav Lichvar
Would it be less confusing if we introduced GNU-style long option
names?

Not to me. PTP has tons of options, and every new profile forces more
and more of these. I simply find it gross when programs have a
gazillion command line options. It makes for poor usability. I
really wanted ptp4l to have the "top ten" options available on the
command line, in order to keep it simple for the most common use
cases.

I think in this case introducing a "long" option would make more sense than using more small options. I do agree that we shouldn't proliferate command line options for every single thing in the config, (as many of these are esoteric and we would have to add more and more options as time goes on). However, I think that using long options can provide clarity that a small 1letter option name doesn't.

That being said, "-t" makes sense to me.

I think we should consider the long options for v2 if people really
want them...
Thanks,
Richard

Thanks,
Jake

[Richard Cochran]

Post by Keller, Jacob E
I think in this case introducing a "long" option would make more
sense than using more small options. I do agree that we shouldn't
proliferate command line options for every single thing in the
config, (as many of these are esoteric and we would have to add more
and more options as time goes on). However, I think that using long
options can provide clarity that a small 1letter option name
doesn't.

Post by Richard Cochran
I think we should consider the long options for v2 if people really
want them...

So it sounds like people do want long options. I'll see if I can come
up with a way to automatically support the config_tab[] entries in
config.c as command line options. For example

PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),

becomes

--announceReceiptTimeout val
--assume_two_step
--boundary_clock_jbod

It should be possible, I would think.

Thanks,
Richard

[Keller, Jacob E]

Post by Keller, Jacob E
I think in this case introducing a "long" option would make more
sense than using more small options. I do agree that we shouldn't
proliferate command line options for every single thing in the
config, (as many of these are esoteric and we would have to add more
and more options as time goes on). However, I think that using long
options can provide clarity that a small 1letter option name
doesn't.

Post by Richard Cochran
I think we should consider the long options for v2 if people really
want them...

So it sounds like people do want long options.  I'll see if I can
come
up with a way to automatically support the config_tab[] entries in
config.c as command line options.  For example
PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
becomes
--announceReceiptTimeout val
--assume_two_step
--boundary_clock_jbod
It should be possible, I would think.
Thanks,
Richard

That could work. I wouldn't necessarily include all options, you could
add a new field in the config block that indicates whether that option
should be considered? Or just supporting all long options also works
too I suppose if that's easier?

I do agree that we don't need to proliferate every long option, but I
think that some might be valuable and wouldn't want to waste our small
space of letters and numbers on them.

I'm personally ok with every config being available from its long name
as an option, so if that is easier then we can just go that route.

Thanks,
Jake

[Miroslav Lichvar]

Post by Richard Cochran
So it sounds like people do want long options. I'll see if I can come
up with a way to automatically support the config_tab[] entries in
config.c as command line options. For example
PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
becomes
--announceReceiptTimeout val
--assume_two_step
--boundary_clock_jbod

I think this would be nice. If it's implemented, it would probably
make sense to include all options, even the obscure ones, so there is
no confusion when someone tries to use them on command line and the
man page doesn't have to explain which can be used only in the config
file.

[Keller, Jacob E]

-----Original Message-----
Sent: Wednesday, November 09, 2016 3:38 AM
Subject: Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and
phc2sys log messages.

Post by Richard Cochran
So it sounds like people do want long options. I'll see if I can come
up with a way to automatically support the config_tab[] entries in
config.c as command line options. For example
PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
becomes
--announceReceiptTimeout val
--assume_two_step
--boundary_clock_jbod

I think this would be nice. If it's implemented, it would probably
make sense to include all options, even the obscure ones, so there is
no confusion when someone tries to use them on command line and the
man page doesn't have to explain which can be used only in the config
file.

I think I changed my opinion and agree with you here. It makes sense since the long names will be the same as the config file.

Thanks,
Jake

--
Miroslav Lichvar