[Linuxptp-devel] [PATCH v2] hwtstamp_ctl: use SIOCGHWTSTAMP ioctl before destructively setting policy

[Jacob Keller]

This patch modifies the hwtstamp_ctl program, so that it will (attempt
to) use the SIOCGHWTSTAMP ioctl to non-destructively read the current
hardware timestamping policy, prior to setting it with SIOCSHWTSTAMP.

This change has 3 primary advantages:

1) It allows reading the current settings of the device, which was
previously not possible since SIOCSHWTSTAMP is destructive.
2) The default behavior without rx-filter or tx-type selected on the
command line is no longer destructive, since it does not attempt to
set the values to 0. The user must explicitly request to disable the
settings, by using the provided options.
3) It allows only modifying tx-type or rx-filter separately, without
destroying the other setting.

This patch supersedes a previous submission which added a -g flag. This
new method of getting first is more advantageous and doesn't require
adding an additional option flag.

- v2
* Fix line lengths, and a few other minor changes

Signed-off-by: Jacob Keller <***@intel.com>
---
hwstamp_ctl.8 | 11 ++++++++++-
hwstamp_ctl.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/hwstamp_ctl.8 b/hwstamp_ctl.8
index cece74f61866..0ae4c4a6a456 100644
--- a/hwstamp_ctl.8
+++ b/hwstamp_ctl.8
@@ -15,7 +15,7 @@ hwstamp_ctl \- set time stamping policy at the driver level

.SH DESCRIPTION
.B hwstamp_ctl
-is a program used to set the hardware time stamping policy at the network
+is a program used to set and get the hardware time stamping policy at the network
driver level with the
.B SIOCSHWTSTAMP
.BR ioctl (2).
@@ -27,6 +27,15 @@ values are hints to the driver what it is expected to do. If the requested
fine-grained filtering for incoming packets is not supported, the driver may
time stamp more than just the requested types of packets.

+If neither
+.I tx-type
+nor
+.I rx-filter
+values are passed to the program, it will use the
+.B SIOCGHWTSTAMP
+.BR ioctl(2)
+to non-destructively read the current hardware time stamping policy.
+
This program is a debugging tool. The
.BR ptp4l (8)
program does not need this program to function, it will set the policy
diff --git a/hwstamp_ctl.c b/hwstamp_ctl.c
index 456d96b09147..149f019bf74c 100644
--- a/hwstamp_ctl.c
+++ b/hwstamp_ctl.c
@@ -30,6 +30,7 @@
#include <net/if.h>

#include "version.h"
+#include "missing.h"

static void usage(char *progname)
{
@@ -84,6 +85,7 @@ int main(int argc, char *argv[])
struct hwtstamp_config cfg;
char *device = NULL, *progname;
int c, err, fd, rxopt = HWTSTAMP_FILTER_NONE, txopt = HWTSTAMP_TX_OFF;
+ int setrx = 0, settx = 0;

/* Process the command line arguments. */
progname = strrchr(argv[0], '/');
@@ -94,9 +96,11 @@ int main(int argc, char *argv[])
device = optarg;
break;
case 'r':
+ setrx = 1;
rxopt = atoi(optarg);
break;
case 't':
+ settx = 1;
txopt = atoi(optarg);
break;
case 'v':
@@ -116,6 +120,7 @@ int main(int argc, char *argv[])
usage(progname);
return -1;
}
+
if (rxopt < HWTSTAMP_FILTER_NONE ||
rxopt > HWTSTAMP_FILTER_PTP_V2_DELAY_REQ ||
txopt < HWTSTAMP_TX_OFF || txopt > HWTSTAMP_TX_ON) {
@@ -129,8 +134,6 @@ int main(int argc, char *argv[])
strncpy(ifreq.ifr_name, device, sizeof(ifreq.ifr_name));

ifreq.ifr_data = (void *) &cfg;
- cfg.tx_type = txopt;
- cfg.rx_filter = rxopt;

fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (fd < 0) {
@@ -138,15 +141,54 @@ int main(int argc, char *argv[])
return -1;
}

- err = ioctl(fd, SIOCSHWTSTAMP, &ifreq);
+ /* First, attempt to get the current settings. */
+ err = ioctl(fd, SIOCGHWTSTAMP, &ifreq);
if (err < 0) {
err = errno;
- perror("SIOCSHWTSTAMP failed");
- if (err == ERANGE)
- fprintf(stderr, "The requested time stamping mode is not supported by the hardware.\n");
+ if (err == ENOTTY)
+ fprintf(stderr,
+ "Kernel does not have support"
+ "for non-destructive SIOCGHWTSTAMP.\n");
+ else if (err == EOPNOTSUPP)
+ fprintf(stderr,
+ "Device driver does not have support"
+ "for non-destructive SIOCGHWTSTAMP.\n");
+ else
+ perror("SIOCGHWTSTAMP failed");
+ } else {
+ printf("current settings:\n"
+ "tx_type %d\n"
+ "rx_filter %d\n",
+ cfg.tx_type, cfg.rx_filter);
}

- printf("tx_type %d\n" "rx_filter %d\n", cfg.tx_type, cfg.rx_filter);
+ /* Now, attempt to set values. Only change the values actually
+ * requested by user, rather than blindly resetting th zero if
+ * unrequested. */
+ if (settx || setrx) {
+
+ if (settx)
+ cfg.tx_type = txopt;
+
+ if (setrx)
+ cfg.rx_filter = rxopt;
+
+ err = ioctl(fd, SIOCSHWTSTAMP, &ifreq);
+ if (err < 0) {
+ err = errno;
+ perror("SIOCSHWTSTAMP failed");
+ if (err == ERANGE)
+ fprintf(stderr,
+ "The requested time stamping mode is"
+ "not supported by the hardware.\n");
+ }
+
+ printf("new settings:\n"
+ "tx_type %d\n"
+ "rx_filter %d\n",
+ cfg.tx_type, cfg.rx_filter);
+
+ }

return err;
}