[PATCH ethtool v3 0/6] ethtool(1) cable test support

[PATCH ethtool v3 0/6] ethtool(1) cable test support

[Andrew Lunn]

Add the user space side of the ethtool cable test.

The TDR output is most useful when fed to some other tool which can
visualize the data. So add JSON support, by borrowing code from
iproute2.

v2:
man page fixes.

v3
More man page fixes.
Use json_print from iproute2.

Andrew Lunn (6):
  Add cable test support
  Add cable test TDR support
  json_writer: Import the iproute2 helper code for JSON output
  Add --json command line argument parsing
  ethtool.8.in: Document the cable test commands
  ethtool.8.in: Add --json option

 Makefile.am          |   5 +-
 ethtool.8.in         |  53 ++++
 ethtool.c            |  46 +++-
 internal.h           |   4 +
 json_writer.c        | 389 +++++++++++++++++++++++++++
 json_writer.h        |  76 ++++++
 netlink/cable_test.c | 624 +++++++++++++++++++++++++++++++++++++++++++
 netlink/extapi.h     |   4 +
 netlink/monitor.c    |   8 +
 netlink/netlink.h    |   5 +-
 netlink/parser.c     |  41 +++
 netlink/parser.h     |   4 +
 12 files changed, 1245 insertions(+), 14 deletions(-)
 create mode 100644 json_writer.c
 create mode 100644 json_writer.h
 create mode 100644 netlink/cable_test.c

-- 
2.26.2

[PATCH ethtool v3 1/6] Add cable test support

[Andrew Lunn]

Add support for starting a cable test, and report the results.

This code does not follow the usual patterns because of the way the
kernel reports the results of the cable test. It can take a number of
seconds for cable testing to occur. So the action request messages is
immediately acknowledges, and the test is then performed asynchronous.
Once the test has completed, the results are returned as a
notification.

This means the command starts as usual. It then monitors multicast
messages until it receives the results.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 Makefile.am          |   2 +-
 ethtool.c            |   5 +
 netlink/cable_test.c | 257 +++++++++++++++++++++++++++++++++++++++++++
 netlink/extapi.h     |   2 +
 netlink/monitor.c    |   4 +
 netlink/netlink.h    |   3 +-
 6 files changed, 271 insertions(+), 2 deletions(-)
 create mode 100644 netlink/cable_test.c

diff --git a/Makefile.am b/Makefile.am
index 63c3fb3..a818cf8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -35,7 +35,7 @@ ethtool_SOURCES += \
 		  netlink/channels.c netlink/coalesce.c netlink/pause.c \
 		  netlink/eee.c netlink/tsinfo.c \
 		  netlink/desc-ethtool.c netlink/desc-genlctrl.c \
-		  netlink/desc-rtnl.c \
+		  netlink/desc-rtnl.c netlink/cable_test.c \
 		  uapi/linux/ethtool_netlink.h \
 		  uapi/linux/netlink.h uapi/linux/genetlink.h \
 		  uapi/linux/rtnetlink.h uapi/linux/if_link.h
diff --git a/ethtool.c b/ethtool.c
index 8522d6b..a616943 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5482,6 +5482,11 @@ static const struct option args[] = {
 		.xhelp	= "The supported sub commands include --show-coalesce, --coalesce"
 			  "             [queue_mask %x] SUB_COMMAND\n",
 	},
+	{
+		.opts	= "--cable-test",
+		.nlfunc	= nl_cable_test,
+		.help	= "Perform a cable test",
+	},
 	{
 		.opts	= "-h|--help",
 		.no_dev	= true,
diff --git a/netlink/cable_test.c b/netlink/cable_test.c
new file mode 100644
index 0000000..b77346e
--- /dev/null
+++ b/netlink/cable_test.c
@@ -0,0 +1,257 @@
+/*
+ * cable_test.c - netlink implementation of cable test command
+ *
+ * Implementation of ethtool <dev> --cable-test
+ */
+
+#include <errno.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "../internal.h"
+#include "../common.h"
+#include "netlink.h"
+
+static bool breakout;
+
+static int nl_get_cable_test_result(const struct nlattr *nest, uint8_t *pair,
+				    uint16_t *code)
+{
+	const struct nlattr *tb[ETHTOOL_A_CABLE_RESULT_MAX+1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	int ret;
+
+	ret = mnl_attr_parse_nested(nest, attr_cb, &tb_info);
+	if (ret < 0 ||
+	    !tb[ETHTOOL_A_CABLE_RESULT_PAIR] ||
+	    !tb[ETHTOOL_A_CABLE_RESULT_CODE])
+		return -EFAULT;
+
+	*pair = mnl_attr_get_u8(tb[ETHTOOL_A_CABLE_RESULT_PAIR]);
+	*code = mnl_attr_get_u8(tb[ETHTOOL_A_CABLE_RESULT_CODE]);
+
+	return 0;
+}
+
+static int nl_get_cable_test_fault_length(const struct nlattr *nest,
+					  uint8_t *pair, unsigned int *cm)
+{
+	const struct nlattr *tb[ETHTOOL_A_CABLE_FAULT_LENGTH_MAX+1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	int ret;
+
+	ret = mnl_attr_parse_nested(nest, attr_cb, &tb_info);
+	if (ret < 0 ||
+	    !tb[ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR] ||
+	    !tb[ETHTOOL_A_CABLE_FAULT_LENGTH_CM])
+		return -EFAULT;
+
+	*pair = mnl_attr_get_u8(tb[ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR]);
+	*cm = mnl_attr_get_u32(tb[ETHTOOL_A_CABLE_FAULT_LENGTH_CM]);
+
+	return 0;
+}
+
+static char *nl_code2txt(uint16_t code)
+{
+	switch(code) {
+	case ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC:
+	default:
+		return "Unknown";
+	case ETHTOOL_A_CABLE_RESULT_CODE_OK:
+		return "OK";
+	case ETHTOOL_A_CABLE_RESULT_CODE_OPEN:
+		return "Open Circuit";
+	case ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT:
+		return "Short within Pair";
+	case ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT:
+		return "Short to another pair";
+	}
+}
+
+static char *nl_pair2txt(uint8_t pair)
+{
+	switch (pair) {
+	case ETHTOOL_A_CABLE_PAIR_A:
+		return "Pair A";
+	case ETHTOOL_A_CABLE_PAIR_B:
+		return "Pair B";
+	case ETHTOOL_A_CABLE_PAIR_C:
+		return "Pair C";
+	case ETHTOOL_A_CABLE_PAIR_D:
+		return "Pair D";
+	default:
+		return "Unexpected pair";
+	}
+}
+
+static int nl_cable_test_ntf_attr(struct nlattr *evattr)
+{
+	unsigned int cm;
+	uint16_t code;
+	uint8_t pair;
+	int ret;
+
+	switch (mnl_attr_get_type(evattr)) {
+	case ETHTOOL_A_CABLE_NEST_RESULT:
+		ret = nl_get_cable_test_result(evattr, &pair, &code);
+		if (ret < 0)
+			return ret;
+
+		printf("Pair: %s, result: %s\n", nl_pair2txt(pair),
+		       nl_code2txt(code));
+		break;
+
+	case ETHTOOL_A_CABLE_NEST_FAULT_LENGTH:
+		ret = nl_get_cable_test_fault_length(evattr, &pair, &cm);
+		if (ret < 0)
+			return ret;
+
+		printf("Pair: %s, fault length: %0.2fm\n",
+		       nl_pair2txt(pair), (float)cm / 100);
+		break;
+	}
+	return 0;
+}
+
+static void cable_test_ntf_nest(const struct nlattr *nest)
+{
+	struct nlattr *pos;
+	int ret;
+
+	mnl_attr_for_each_nested(pos, nest) {
+		ret = nl_cable_test_ntf_attr(pos);
+		if (ret < 0)
+			return;
+	}
+}
+
+/* Returns MNL_CB_STOP when the test is complete. Used when executing
+   a test, but not suitable for monitor. */
+static int cable_test_ntf_stop_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	const struct nlattr *tb[ETHTOOL_A_CABLE_TEST_NTF_MAX + 1] = {};
+	u8 status = ETHTOOL_A_CABLE_TEST_NTF_STATUS_UNSPEC;
+	struct nl_context *nlctx = data;
+	DECLARE_ATTR_TB_INFO(tb);
+	bool silent;
+	int err_ret;
+	int ret;
+
+	silent = nlctx->is_dump || nlctx->is_monitor;
+	err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR;
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return err_ret;
+
+	nlctx->devname = get_dev_name(tb[ETHTOOL_A_CABLE_TEST_HEADER]);
+	if (!dev_ok(nlctx))
+		return err_ret;
+
+	if (tb[ETHTOOL_A_CABLE_TEST_NTF_STATUS])
+		status = mnl_attr_get_u8(tb[ETHTOOL_A_CABLE_TEST_NTF_STATUS]);
+
+	switch (status) {
+	case ETHTOOL_A_CABLE_TEST_NTF_STATUS_STARTED:
+		printf("Cable test started for device %s.\n",
+		       nlctx->devname);
+		break;
+	case ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED:
+		printf("Cable test completed for device %s.\n",
+		       nlctx->devname);
+		break;
+	default:
+		break;
+	}
+
+	if (tb[ETHTOOL_A_CABLE_TEST_NTF_NEST])
+		cable_test_ntf_nest(tb[ETHTOOL_A_CABLE_TEST_NTF_NEST]);
+
+	if (status == ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED) {
+		breakout = true;
+		return MNL_CB_STOP;
+	}
+
+	return MNL_CB_OK;
+}
+
+/* Wrapper around cable_test_ntf_stop_cb() which does not return STOP,
+ * used for monitor */
+int cable_test_ntf_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	int status = cable_test_ntf_stop_cb(nlhdr, data);
+
+	if (status == MNL_CB_STOP)
+		status = MNL_CB_OK;
+
+	return status;
+}
+
+static int nl_cable_test_results_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	const struct genlmsghdr *ghdr = (const struct genlmsghdr *)(nlhdr + 1);
+
+	if (ghdr->cmd != ETHTOOL_MSG_CABLE_TEST_NTF) {
+		return MNL_CB_OK;
+	}
+
+	return cable_test_ntf_stop_cb(nlhdr, data);
+}
+
+/* Receive the broadcasted messages until we get the cable test
+ * results
+ */
+static int nl_cable_test_process_results(struct cmd_context *ctx)
+{
+        struct nl_context *nlctx = ctx->nlctx;
+	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	int err;
+
+        nlctx->is_monitor = true;
+        nlsk->port = 0;
+	nlsk->seq = 0;
+
+	breakout = false;
+
+	while (!breakout) {
+		err = nlsock_process_reply(nlsk, nl_cable_test_results_cb,
+					   nlctx);
+		if (err)
+			return err;
+	}
+
+	return err;
+}
+
+int nl_cable_test(struct cmd_context *ctx)
+{
+	struct nl_context *nlctx = ctx->nlctx;
+	struct nl_socket *nlsk = nlctx->ethnl_socket;
+        uint32_t grpid = nlctx->ethnl_mongrp;
+	int ret;
+
+	/* Join the multicast group so we can receive the results in a
+	 * race free way.
+	 */
+        if (!grpid) {
+                fprintf(stderr, "multicast group 'monitor' not found\n");
+                return -EOPNOTSUPP;
+        }
+
+        ret = mnl_socket_setsockopt(nlsk->sk, NETLINK_ADD_MEMBERSHIP,
+                                    &grpid, sizeof(grpid));
+        if (ret < 0)
+                return ret;
+
+	ret = nlsock_prep_get_request(nlsk, ETHTOOL_MSG_CABLE_TEST_ACT,
+				      ETHTOOL_A_CABLE_TEST_HEADER, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = nlsock_sendmsg(nlsk, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Cannot start cable test\n");
+	else
+		ret = nl_cable_test_process_results(ctx);
+	return ret;
+}
diff --git a/netlink/extapi.h b/netlink/extapi.h
index 1b39ed9..a2293c1 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -35,6 +35,7 @@ int nl_spause(struct cmd_context *ctx);
 int nl_geee(struct cmd_context *ctx);
 int nl_seee(struct cmd_context *ctx);
 int nl_tsinfo(struct cmd_context *ctx);
+int nl_cable_test(struct cmd_context *ctx);
 int nl_monitor(struct cmd_context *ctx);
 
 void nl_monitor_usage(void);
@@ -74,6 +75,7 @@ static inline void nl_monitor_usage(void)
 #define nl_geee			NULL
 #define nl_seee			NULL
 #define nl_tsinfo		NULL
+#define nl_cable_test		NULL
 
 #endif /* ETHTOOL_ENABLE_NETLINK */
 
diff --git a/netlink/monitor.c b/netlink/monitor.c
index 18d4efd..1af11ee 100644
--- a/netlink/monitor.c
+++ b/netlink/monitor.c
@@ -59,6 +59,10 @@ static struct {
 		.cmd	= ETHTOOL_MSG_EEE_NTF,
 		.cb	= eee_reply_cb,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_CABLE_TEST_NTF,
+		.cb	= cable_test_ntf_cb,
+	},
 };
 
 static void clear_filter(struct nl_context *nlctx)
diff --git a/netlink/netlink.h b/netlink/netlink.h
index fe53fd0..2a2b60e 100644
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -78,6 +78,8 @@ int channels_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int coalesce_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int pause_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int eee_reply_cb(const struct nlmsghdr *nlhdr, void *data);
+int cable_test_reply_cb(const struct nlmsghdr *nlhdr, void *data);
+int cable_test_ntf_cb(const struct nlmsghdr *nlhdr, void *data);
 
 /* dump helpers */
 
@@ -108,7 +110,6 @@ static inline void show_bool(const struct nlattr *attr, const char *label)
 }
 
 /* misc */
-
 static inline void copy_devname(char *dst, const char *src)
 {
 	strncpy(dst, src, ALTIFNAMSIZ);
-- 
2.27.0

[PATCH ethtool v3 2/6] Add cable test TDR support

[Andrew Lunn]

Add support for accessing the cable test time domain reflectromatry
data. Add a new command --cable-test-tdr, and support for dumping the
data which is returned.

signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 ethtool.c            |   8 ++
 netlink/cable_test.c | 289 +++++++++++++++++++++++++++++++++++++++++++
 netlink/extapi.h     |   2 +
 netlink/monitor.c    |   4 +
 netlink/netlink.h    |   2 +
 netlink/parser.c     |  41 ++++++
 netlink/parser.h     |   4 +
 7 files changed, 350 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index a616943..a6bb9ac 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5487,6 +5487,14 @@ static const struct option args[] = {
 		.nlfunc	= nl_cable_test,
 		.help	= "Perform a cable test",
 	},
+	{
+		.opts	= "--cable-test-tdr",
+		.nlfunc	= nl_cable_test_tdr,
+		.help	= "Print cable test time domain reflectrometery data",
+		.xhelp	= "		[ first N ]\n"
+			  "		[ last N ]\n"
+			  "		[ step N ]\n"
+	},
 	{
 		.opts	= "-h|--help",
 		.no_dev	= true,
diff --git a/netlink/cable_test.c b/netlink/cable_test.c
index b77346e..1b9ae9c 100644
--- a/netlink/cable_test.c
+++ b/netlink/cable_test.c
@@ -11,6 +11,7 @@
 #include "../internal.h"
 #include "../common.h"
 #include "netlink.h"
+#include "parser.h"
 
 static bool breakout;
 
@@ -255,3 +256,291 @@ int nl_cable_test(struct cmd_context *ctx)
 		ret = nl_cable_test_process_results(ctx);
 	return ret;
 }
+
+static int nl_get_cable_test_tdr_amplitude(const struct nlattr *nest,
+					   uint8_t *pair, int16_t *mV)
+{
+	const struct nlattr *tb[ETHTOOL_A_CABLE_AMPLITUDE_MAX+1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	uint16_t mV_unsigned;
+	int ret;
+
+	ret = mnl_attr_parse_nested(nest, attr_cb, &tb_info);
+	if (ret < 0 ||
+	    !tb[ETHTOOL_A_CABLE_AMPLITUDE_PAIR] ||
+	    !tb[ETHTOOL_A_CABLE_AMPLITUDE_mV])
+		return -EFAULT;
+
+	*pair = mnl_attr_get_u8(tb[ETHTOOL_A_CABLE_AMPLITUDE_PAIR]);
+	mV_unsigned = mnl_attr_get_u16(tb[ETHTOOL_A_CABLE_AMPLITUDE_mV]);
+	*mV = (int16_t)(mV_unsigned);
+
+	return 0;
+}
+
+static int nl_get_cable_test_tdr_pulse(const struct nlattr *nest, uint16_t *mV)
+{
+	const struct nlattr *tb[ETHTOOL_A_CABLE_PULSE_MAX+1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	int ret;
+
+	ret = mnl_attr_parse_nested(nest, attr_cb, &tb_info);
+	if (ret < 0 ||
+	    !tb[ETHTOOL_A_CABLE_PULSE_mV])
+		return -EFAULT;
+
+	*mV = mnl_attr_get_u16(tb[ETHTOOL_A_CABLE_PULSE_mV]);
+
+	return 0;
+}
+
+static int nl_get_cable_test_tdr_step(const struct nlattr *nest,
+				      uint32_t *first, uint32_t *last,
+				      uint32_t *step)
+{
+	const struct nlattr *tb[ETHTOOL_A_CABLE_STEP_MAX+1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	int ret;
+
+	ret = mnl_attr_parse_nested(nest, attr_cb, &tb_info);
+	if (ret < 0 ||
+	    !tb[ETHTOOL_A_CABLE_STEP_FIRST_DISTANCE] ||
+	    !tb[ETHTOOL_A_CABLE_STEP_LAST_DISTANCE] ||
+	    !tb[ETHTOOL_A_CABLE_STEP_STEP_DISTANCE])
+		return -EFAULT;
+
+	*first = mnl_attr_get_u32(tb[ETHTOOL_A_CABLE_STEP_FIRST_DISTANCE]);
+	*last = mnl_attr_get_u32(tb[ETHTOOL_A_CABLE_STEP_LAST_DISTANCE]);
+	*step = mnl_attr_get_u32(tb[ETHTOOL_A_CABLE_STEP_STEP_DISTANCE]);
+
+	return 0;
+}
+
+static int nl_cable_test_tdr_ntf_attr(struct nlattr *evattr)
+{
+	uint32_t first, last, step;
+	uint8_t pair;
+	int ret;
+
+	switch (mnl_attr_get_type(evattr)) {
+	case ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE: {
+		int16_t mV;
+
+		ret = nl_get_cable_test_tdr_amplitude(
+			evattr, &pair, &mV);
+		if (ret < 0)
+			return ret;
+
+		printf("Pair: %s, amplitude %4d\n", nl_pair2txt(pair), mV);
+		break;
+	}
+	case ETHTOOL_A_CABLE_TDR_NEST_PULSE: {
+		uint16_t mV;
+
+		ret = nl_get_cable_test_tdr_pulse(evattr, &mV);
+		if (ret < 0)
+			return ret;
+
+		printf("TDR pulse %dmV\n", mV);
+		break;
+	}
+	case ETHTOOL_A_CABLE_TDR_NEST_STEP:
+		ret = nl_get_cable_test_tdr_step(evattr, &first, &last, &step);
+		if (ret < 0)
+			return ret;
+
+		printf("Step configuration, %.2f-%.2f meters in %.2fm steps\n",
+		       (float)first / 100, (float)last /  100,
+		       (float)step /  100);
+		break;
+	}
+	return 0;
+}
+
+static void cable_test_tdr_ntf_nest(const struct nlattr *nest)
+{
+	struct nlattr *pos;
+	int ret;
+
+	mnl_attr_for_each_nested(pos, nest) {
+		ret = nl_cable_test_tdr_ntf_attr(pos);
+		if (ret < 0)
+			return;
+	}
+}
+
+/* Returns MNL_CB_STOP when the test is complete. Used when executing
+   a test, but not suitable for monitor. */
+int cable_test_tdr_ntf_stop_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	const struct nlattr *tb[ETHTOOL_A_CABLE_TEST_TDR_NTF_MAX + 1] = {};
+	u8 status = ETHTOOL_A_CABLE_TEST_NTF_STATUS_UNSPEC;
+	struct nl_context *nlctx = data;
+	DECLARE_ATTR_TB_INFO(tb);
+	bool silent;
+	int err_ret;
+	int ret;
+
+	silent = nlctx->is_dump || nlctx->is_monitor;
+	err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR;
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return err_ret;
+
+	nlctx->devname = get_dev_name(tb[ETHTOOL_A_CABLE_TEST_TDR_HEADER]);
+	if (!dev_ok(nlctx))
+		return err_ret;
+
+	if (tb[ETHTOOL_A_CABLE_TEST_NTF_STATUS])
+		status = mnl_attr_get_u8(tb[ETHTOOL_A_CABLE_TEST_NTF_STATUS]);
+
+	switch (status) {
+	case ETHTOOL_A_CABLE_TEST_NTF_STATUS_STARTED:
+		printf("Cable test TDR started for device %s.\n",
+		       nlctx->devname);
+		break;
+	case ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED:
+		printf("Cable test TDR completed for device %s.\n",
+		       nlctx->devname);
+		break;
+	default:
+		break;
+	}
+
+	if (tb[ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST])
+		cable_test_tdr_ntf_nest(tb[ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST]);
+
+	if (status == ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED) {
+		breakout = true;
+		return MNL_CB_STOP;
+	}
+
+	return MNL_CB_OK;
+}
+
+/* Wrapper around cable_test_tdr_ntf_stop_cb() which does not return
+ * STOP, used for monitor */
+int cable_test_tdr_ntf_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	int status = cable_test_tdr_ntf_stop_cb(nlhdr, data);
+
+	if (status == MNL_CB_STOP)
+		status = MNL_CB_OK;
+
+	return status;
+}
+
+static int nl_cable_test_tdr_results_cb(const struct nlmsghdr *nlhdr,
+					void *data)
+{
+	const struct genlmsghdr *ghdr = (const struct genlmsghdr *)(nlhdr + 1);
+
+	if (ghdr->cmd != ETHTOOL_MSG_CABLE_TEST_TDR_NTF) {
+		return MNL_CB_OK;
+	}
+
+	cable_test_tdr_ntf_cb(nlhdr, data);
+
+	return MNL_CB_STOP;
+}
+
+/* Receive the broadcasted messages until we get the cable test
+ * results
+ */
+static int nl_cable_test_tdr_process_results(struct cmd_context *ctx)
+{
+	struct nl_context *nlctx = ctx->nlctx;
+	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	int err;
+
+	nlctx->is_monitor = true;
+	nlsk->port = 0;
+	nlsk->seq = 0;
+
+	breakout = false;
+
+	while (!breakout) {
+		err = nlsock_process_reply(nlsk, nl_cable_test_tdr_results_cb,
+					   nlctx);
+		if (err)
+			return err;
+	}
+
+	return err;
+}
+
+static const struct param_parser tdr_params[] = {
+	{
+		.arg		= "first",
+		.type		= ETHTOOL_A_CABLE_TEST_TDR_CFG_FIRST,
+		.group		= ETHTOOL_A_CABLE_TEST_TDR_CFG,
+		.handler	= nl_parse_direct_m2cm,
+	},
+	{
+		.arg		= "last",
+		.type		= ETHTOOL_A_CABLE_TEST_TDR_CFG_LAST,
+		.group		= ETHTOOL_A_CABLE_TEST_TDR_CFG,
+		.handler	= nl_parse_direct_m2cm,
+	},
+	{
+		.arg		= "step",
+		.type		= ETHTOOL_A_CABLE_TEST_TDR_CFG_STEP,
+		.group		= ETHTOOL_A_CABLE_TEST_TDR_CFG,
+		.handler	= nl_parse_direct_m2cm,
+	},
+	{
+		.arg		= "pair",
+		.type		= ETHTOOL_A_CABLE_TEST_TDR_CFG_PAIR,
+		.group		= ETHTOOL_A_CABLE_TEST_TDR_CFG,
+		.handler	= nl_parse_direct_u8,
+	},
+	{}
+};
+
+int nl_cable_test_tdr(struct cmd_context *ctx)
+{
+	struct nl_context *nlctx = ctx->nlctx;
+	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	uint32_t grpid = nlctx->ethnl_mongrp;
+	struct nl_msg_buff *msgbuff;
+	int ret;
+
+	nlctx->cmd = "--cable-test-tdr";
+	nlctx->argp = ctx->argp;
+	nlctx->argc = ctx->argc;
+	nlctx->devname = ctx->devname;
+	msgbuff = &nlsk->msgbuff;
+
+	/* Join the multicast group so we can receive the results in a
+	 * race free way.
+	 */
+	if (!grpid) {
+		fprintf(stderr, "multicast group 'monitor' not found\n");
+		return -EOPNOTSUPP;
+	}
+
+	ret = mnl_socket_setsockopt(nlsk->sk, NETLINK_ADD_MEMBERSHIP,
+				    &grpid, sizeof(grpid));
+	if (ret < 0)
+		return ret;
+
+	ret = msg_init(nlctx, msgbuff, ETHTOOL_MSG_CABLE_TEST_TDR_ACT,
+		       NLM_F_REQUEST | NLM_F_ACK);
+	if (ret < 0)
+		return 2;
+
+	if (ethnla_fill_header(msgbuff, ETHTOOL_A_CABLE_TEST_TDR_HEADER,
+			       ctx->devname, 0))
+		return -EMSGSIZE;
+
+	ret = nl_parser(nlctx, tdr_params, NULL, PARSER_GROUP_NEST);
+	if (ret < 0)
+		return ret;
+
+	ret = nlsock_sendmsg(nlsk, NULL);
+	if (ret < 0)
+		fprintf(stderr, "Cannot start cable test TDR\n");
+	else
+		ret = nl_cable_test_tdr_process_results(ctx);
+	return ret;
+}
diff --git a/netlink/extapi.h b/netlink/extapi.h
index a2293c1..c5bfde9 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -36,6 +36,7 @@ int nl_geee(struct cmd_context *ctx);
 int nl_seee(struct cmd_context *ctx);
 int nl_tsinfo(struct cmd_context *ctx);
 int nl_cable_test(struct cmd_context *ctx);
+int nl_cable_test_tdr(struct cmd_context *ctx);
 int nl_monitor(struct cmd_context *ctx);
 
 void nl_monitor_usage(void);
@@ -76,6 +77,7 @@ static inline void nl_monitor_usage(void)
 #define nl_seee			NULL
 #define nl_tsinfo		NULL
 #define nl_cable_test		NULL
+#define nl_cable_test_tdr	NULL
 
 #endif /* ETHTOOL_ENABLE_NETLINK */
 
diff --git a/netlink/monitor.c b/netlink/monitor.c
index 1af11ee..280fd0b 100644
--- a/netlink/monitor.c
+++ b/netlink/monitor.c
@@ -63,6 +63,10 @@ static struct {
 		.cmd	= ETHTOOL_MSG_CABLE_TEST_NTF,
 		.cb	= cable_test_ntf_cb,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_CABLE_TEST_TDR_NTF,
+		.cb	= cable_test_tdr_ntf_cb,
+	},
 };
 
 static void clear_filter(struct nl_context *nlctx)
diff --git a/netlink/netlink.h b/netlink/netlink.h
index 2a2b60e..d330c21 100644
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -80,6 +80,8 @@ int pause_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int eee_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int cable_test_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int cable_test_ntf_cb(const struct nlmsghdr *nlhdr, void *data);
+int cable_test_tdr_reply_cb(const struct nlmsghdr *nlhdr, void *data);
+int cable_test_tdr_ntf_cb(const struct nlmsghdr *nlhdr, void *data);
 
 /* dump helpers */
 
diff --git a/netlink/parser.c b/netlink/parser.c
index bd3526f..dbefc08 100644
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -54,6 +54,22 @@ static bool __prefix_0x(const char *p)
 	return p[0] == '0' && (p[1] == 'x' || p[1] == 'X');
 }
 
+static float parse_float(const char *arg, float *result, float min,
+			 float max)
+{
+        char *endptr;
+        float val;
+
+        if (!arg || !arg[0])
+                return -EINVAL;
+        val = strtof(arg, &endptr);
+        if (*endptr || val < min || val > max)
+                return -EINVAL;
+
+        *result = val;
+        return 0;
+}
+
 static int __parse_u32(const char *arg, uint32_t *result, uint32_t min,
 		       uint32_t max, int base)
 {
@@ -211,6 +227,31 @@ int nl_parse_direct_u8(struct nl_context *nlctx, uint16_t type,
 	return (type && ethnla_put_u8(msgbuff, type, val)) ? -EMSGSIZE : 0;
 }
 
+/* Parser handler for float meters and convert it to cm. Generates
+ * NLA_U32 or fills an uint32_t.*/
+int nl_parse_direct_m2cm(struct nl_context *nlctx, uint16_t type,
+			 const void *data, struct nl_msg_buff *msgbuff,
+			 void *dest)
+{
+	const char *arg = *nlctx->argp;
+	float meters;
+	uint32_t cm;
+	int ret;
+
+	nlctx->argp++;
+	nlctx->argc--;
+	ret = parse_float(arg, &meters, 0, 150);
+	if (ret < 0) {
+		parser_err_invalid_value(nlctx, arg);
+		return ret;
+	}
+
+	cm = (uint32_t)(meters * 100);
+	if (dest)
+		*(uint32_t *)dest = cm;
+	return (type && ethnla_put_u32(msgbuff, type, cm)) ? -EMSGSIZE : 0;
+}
+
 /* Parser handler for (tri-state) bool. Expects "name on|off", generates
  * NLA_U8 which is 1 for "on" and 0 for "off".
  */
diff --git a/netlink/parser.h b/netlink/parser.h
index 3cc26d2..fd55bc7 100644
--- a/netlink/parser.h
+++ b/netlink/parser.h
@@ -111,6 +111,10 @@ int nl_parse_direct_u32(struct nl_context *nlctx, uint16_t type,
 int nl_parse_direct_u8(struct nl_context *nlctx, uint16_t type,
 		       const void *data, struct nl_msg_buff *msgbuff,
 		       void *dest);
+/* NLA_U32 represented as float number of meters, converted to cm. */
+int nl_parse_direct_m2cm(struct nl_context *nlctx, uint16_t type,
+			 const void *data, struct nl_msg_buff *msgbuff,
+			 void *dest);
 /* NLA_U8 represented as on | off */
 int nl_parse_u8bool(struct nl_context *nlctx, uint16_t type, const void *data,
 		    struct nl_msg_buff *msgbuff, void *dest);
-- 
2.27.0

[PATCH ethtool v3 3/6] json_writer/json_print: Import the iproute2 helper code for JSON output

[Andrew Lunn]

In general, Linux network tools use JSON for machine readable output.
See for example -json for iproute2 and devlink. In order to support
JSON output from ethtool, import the iproute2 helper code.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 Makefile.am   |   3 +-
 json_print.c  | 229 +++++++++++++++++++++++++++++
 json_print.h  |  67 +++++++++
 json_writer.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++++++
 json_writer.h |  76 ++++++++++
 5 files changed, 763 insertions(+), 1 deletion(-)
 create mode 100644 json_print.c
 create mode 100644 json_print.h
 create mode 100644 json_writer.c
 create mode 100644 json_writer.h

diff --git a/Makefile.am b/Makefile.am
index a818cf8..a736237 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -7,7 +7,8 @@ EXTRA_DIST = LICENSE ethtool.8 ethtool.spec.in aclocal.m4 ChangeLog autogen.sh
 
 sbin_PROGRAMS = ethtool
 ethtool_SOURCES = ethtool.c uapi/linux/ethtool.h internal.h \
-		  uapi/linux/net_tstamp.h rxclass.c common.c common.h
+		  uapi/linux/net_tstamp.h rxclass.c common.c common.h \
+		  json_writer.c json_writer.h json_print.c json_print.h
 if ETHTOOL_ENABLE_PRETTY_DUMP
 ethtool_SOURCES += \
 		  amd8111e.c de2104x.c dsa.c e100.c e1000.c et131x.c igb.c	\
diff --git a/json_print.c b/json_print.c
new file mode 100644
index 0000000..1ce2e69
--- /dev/null
+++ b/json_print.c
@@ -0,0 +1,229 @@
+/*
+ * json_print.c		"print regular or json output, based on json_writer".
+ *
+ *             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.
+ *
+ * Authors:    Julien Fortin, <julien@cumulusnetworks.com>
+ */
+
+#include <stdarg.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#include "json_print.h"
+
+#define SPRINT_BSIZE 64
+#define SPRINT_BUF(x)   char x[SPRINT_BSIZE]
+
+static json_writer_t *_jw;
+
+#define _IS_JSON_CONTEXT(type) ((type & PRINT_JSON || type & PRINT_ANY) && _jw)
+#define _IS_FP_CONTEXT(type) (!_jw && (type & PRINT_FP || type & PRINT_ANY))
+
+void new_json_obj(int json)
+{
+	if (json) {
+		_jw = jsonw_new(stdout);
+		if (!_jw) {
+			perror("json object");
+			exit(1);
+		}
+		jsonw_pretty(_jw, true);
+		jsonw_start_array(_jw);
+	}
+}
+
+void delete_json_obj(void)
+{
+	if (_jw) {
+		jsonw_end_array(_jw);
+		jsonw_destroy(&_jw);
+	}
+}
+
+bool is_json_context(void)
+{
+	return _jw != NULL;
+}
+
+json_writer_t *get_json_writer(void)
+{
+	return _jw;
+}
+
+void open_json_object(const char *str)
+{
+	if (_IS_JSON_CONTEXT(PRINT_JSON)) {
+		if (str)
+			jsonw_name(_jw, str);
+		jsonw_start_object(_jw);
+	}
+}
+
+void close_json_object(void)
+{
+	if (_IS_JSON_CONTEXT(PRINT_JSON))
+		jsonw_end_object(_jw);
+}
+
+/*
+ * Start json array or string array using
+ * the provided string as json key (if not null)
+ * or as array delimiter in non-json context.
+ */
+void open_json_array(enum output_type type, const char *str)
+{
+	if (_IS_JSON_CONTEXT(type)) {
+		if (str)
+			jsonw_name(_jw, str);
+		jsonw_start_array(_jw);
+	} else if (_IS_FP_CONTEXT(type)) {
+		printf("%s", str);
+	}
+}
+
+/*
+ * End json array or string array
+ */
+void close_json_array(enum output_type type, const char *str)
+{
+	if (_IS_JSON_CONTEXT(type)) {
+		jsonw_end_array(_jw);
+	} else if (_IS_FP_CONTEXT(type)) {
+		printf("%s", str);
+	}
+}
+
+/*
+ * pre-processor directive to generate similar
+ * functions handling different types
+ */
+#define _PRINT_FUNC(type_name, type)					\
+	__attribute__((format(printf, 3, 0)))				\
+	void print_##type_name(enum output_type t,			\
+			       const char *key,				\
+			       const char *fmt,				\
+			       type value)				\
+	{								\
+		if (_IS_JSON_CONTEXT(t)) {				\
+			if (!key)					\
+				jsonw_##type_name(_jw, value);		\
+			else						\
+				jsonw_##type_name##_field(_jw, key, value); \
+		} else if (_IS_FP_CONTEXT(t)) {				\
+			fprintf(stdout, fmt, value);			\
+		}							\
+	}
+_PRINT_FUNC(int, int);
+_PRINT_FUNC(s64, int64_t);
+_PRINT_FUNC(hhu, unsigned char);
+_PRINT_FUNC(hu, unsigned short);
+_PRINT_FUNC(uint, unsigned int);
+_PRINT_FUNC(u64, uint64_t);
+_PRINT_FUNC(luint, unsigned long);
+_PRINT_FUNC(lluint, unsigned long long);
+_PRINT_FUNC(float, double);
+#undef _PRINT_FUNC
+
+void print_string(enum output_type type,
+		  const char *key,
+		  const char *fmt,
+		  const char *value)
+{
+	if (_IS_JSON_CONTEXT(type)) {
+		if (key && !value)
+			jsonw_name(_jw, key);
+		else if (!key && value)
+			jsonw_string(_jw, value);
+		else
+			jsonw_string_field(_jw, key, value);
+	} else if (_IS_FP_CONTEXT(type)) {
+		fprintf(stdout, fmt, value);
+	}
+}
+
+/*
+ * value's type is bool. When using this function in FP context you can't pass
+ * a value to it, you will need to use "is_json_context()" to have different
+ * branch for json and regular output. grep -r "print_bool" for example
+ */
+void print_bool(enum output_type type,
+		const char *key,
+		const char *fmt,
+		bool value)
+{
+	if (_IS_JSON_CONTEXT(type)) {
+		if (key)
+			jsonw_bool_field(_jw, key, value);
+		else
+			jsonw_bool(_jw, value);
+	} else if (_IS_FP_CONTEXT(type)) {
+		fprintf(stdout, fmt, value ? "true" : "false");
+	}
+}
+
+/*
+ * In JSON context uses hardcode %#x format: 42 -> 0x2a
+ */
+void print_0xhex(enum output_type type,
+		 const char *key,
+		 const char *fmt,
+		 unsigned long long hex)
+{
+	if (_IS_JSON_CONTEXT(type)) {
+		SPRINT_BUF(b1);
+
+		snprintf(b1, sizeof(b1), "%#llx", hex);
+		print_string(PRINT_JSON, key, NULL, b1);
+	} else if (_IS_FP_CONTEXT(type)) {
+		fprintf(stdout, fmt, hex);
+	}
+}
+
+void print_hex(enum output_type type,
+	       const char *key,
+	       const char *fmt,
+	       unsigned int hex)
+{
+	if (_IS_JSON_CONTEXT(type)) {
+		SPRINT_BUF(b1);
+
+		snprintf(b1, sizeof(b1), "%x", hex);
+		if (key)
+			jsonw_string_field(_jw, key, b1);
+		else
+			jsonw_string(_jw, b1);
+	} else if (_IS_FP_CONTEXT(type)) {
+		fprintf(stdout, fmt, hex);
+	}
+}
+
+/*
+ * In JSON context we don't use the argument "value" we simply call jsonw_null
+ * whereas FP context can use "value" to output anything
+ */
+void print_null(enum output_type type,
+		const char *key,
+		const char *fmt,
+		const char *value)
+{
+	if (_IS_JSON_CONTEXT(type)) {
+		if (key)
+			jsonw_null_field(_jw, key);
+		else
+			jsonw_null(_jw);
+	} else if (_IS_FP_CONTEXT(type)) {
+		fprintf(stdout, fmt, value);
+	}
+}
+
+/* Print line seperator (if not in JSON mode) */
+void print_nl(void)
+{
+	if (!_jw)
+		printf("%s", "\n");
+}
diff --git a/json_print.h b/json_print.h
new file mode 100644
index 0000000..d035ba2
--- /dev/null
+++ b/json_print.h
@@ -0,0 +1,67 @@
+/*
+ * json_print.h		"print regular or json output, based on json_writer".
+ *
+ *             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.
+ *
+ * Authors:    Julien Fortin, <julien@cumulusnetworks.com>
+ */
+
+#ifndef _JSON_PRINT_H_
+#define _JSON_PRINT_H_
+
+#include "json_writer.h"
+
+json_writer_t *get_json_writer(void);
+
+/*
+ * use:
+ *      - PRINT_ANY for context based output
+ *      - PRINT_FP for non json specific output
+ *      - PRINT_JSON for json specific output
+ */
+enum output_type {
+	PRINT_FP = 1,
+	PRINT_JSON = 2,
+	PRINT_ANY = 4,
+};
+
+void new_json_obj(int json);
+void delete_json_obj(void);
+
+bool is_json_context(void);
+
+void fflush_fp(void);
+
+void open_json_object(const char *str);
+void close_json_object(void);
+void open_json_array(enum output_type type, const char *delim);
+void close_json_array(enum output_type type, const char *delim);
+
+void print_nl(void);
+
+#define _PRINT_FUNC(type_name, type)					\
+	void print_##type_name(enum output_type t,			\
+			       const char *key,				\
+			       const char *fmt,				\
+			       type value);				\
+
+_PRINT_FUNC(int, int);
+_PRINT_FUNC(s64, int64_t);
+_PRINT_FUNC(bool, bool);
+_PRINT_FUNC(null, const char*);
+_PRINT_FUNC(string, const char*);
+_PRINT_FUNC(uint, unsigned int);
+_PRINT_FUNC(u64, uint64_t);
+_PRINT_FUNC(hhu, unsigned char);
+_PRINT_FUNC(hu, unsigned short);
+_PRINT_FUNC(hex, unsigned int);
+_PRINT_FUNC(0xhex, unsigned long long);
+_PRINT_FUNC(luint, unsigned long);
+_PRINT_FUNC(lluint, unsigned long long);
+_PRINT_FUNC(float, double);
+#undef _PRINT_FUNC
+
+#endif /* _JSON_PRINT_H_ */
diff --git a/json_writer.c b/json_writer.c
new file mode 100644
index 0000000..88c5eb8
--- /dev/null
+++ b/json_writer.c
@@ -0,0 +1,389 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
+/*
+ * Simple streaming JSON writer
+ *
+ * This takes care of the annoying bits of JSON syntax like the commas
+ * after elements
+ *
+ * Authors:	Stephen Hemminger <stephen@networkplumber.org>
+ */
+
+#include <stdio.h>
+#include <stdbool.h>
+#include <stdarg.h>
+#include <assert.h>
+#include <malloc.h>
+#include <inttypes.h>
+#include <stdint.h>
+
+#include "json_writer.h"
+
+struct json_writer {
+	FILE		*out;	/* output file */
+	unsigned	depth;  /* nesting */
+	bool		pretty; /* optional whitepace */
+	char		sep;	/* either nul or comma */
+};
+
+/* indentation for pretty print */
+static void jsonw_indent(json_writer_t *self)
+{
+	unsigned i;
+	for (i = 0; i < self->depth; ++i)
+		fputs("    ", self->out);
+}
+
+/* end current line and indent if pretty printing */
+static void jsonw_eol(json_writer_t *self)
+{
+	if (!self->pretty)
+		return;
+
+	putc('\n', self->out);
+	jsonw_indent(self);
+}
+
+/* If current object is not empty print a comma */
+static void jsonw_eor(json_writer_t *self)
+{
+	if (self->sep != '\0')
+		putc(self->sep, self->out);
+	self->sep = ',';
+}
+
+
+/* Output JSON encoded string */
+/* Handles C escapes, does not do Unicode */
+static void jsonw_puts(json_writer_t *self, const char *str)
+{
+	putc('"', self->out);
+	for (; *str; ++str)
+		switch (*str) {
+		case '\t':
+			fputs("\\t", self->out);
+			break;
+		case '\n':
+			fputs("\\n", self->out);
+			break;
+		case '\r':
+			fputs("\\r", self->out);
+			break;
+		case '\f':
+			fputs("\\f", self->out);
+			break;
+		case '\b':
+			fputs("\\b", self->out);
+			break;
+		case '\\':
+			fputs("\\\\", self->out);
+			break;
+		case '"':
+			fputs("\\\"", self->out);
+			break;
+		case '\'':
+			fputs("\\\'", self->out);
+			break;
+		default:
+			putc(*str, self->out);
+		}
+	putc('"', self->out);
+}
+
+/* Create a new JSON stream */
+json_writer_t *jsonw_new(FILE *f)
+{
+	json_writer_t *self = malloc(sizeof(*self));
+	if (self) {
+		self->out = f;
+		self->depth = 0;
+		self->pretty = false;
+		self->sep = '\0';
+	}
+	return self;
+}
+
+/* End output to JSON stream */
+void jsonw_destroy(json_writer_t **self_p)
+{
+	json_writer_t *self = *self_p;
+
+	assert(self->depth == 0);
+	fputs("\n", self->out);
+	fflush(self->out);
+	free(self);
+	*self_p = NULL;
+}
+
+void jsonw_pretty(json_writer_t *self, bool on)
+{
+	self->pretty = on;
+}
+
+/* Basic blocks */
+static void jsonw_begin(json_writer_t *self, int c)
+{
+	jsonw_eor(self);
+	putc(c, self->out);
+	++self->depth;
+	self->sep = '\0';
+}
+
+static void jsonw_end(json_writer_t *self, int c)
+{
+	assert(self->depth > 0);
+
+	--self->depth;
+	if (self->sep != '\0')
+		jsonw_eol(self);
+	putc(c, self->out);
+	self->sep = ',';
+}
+
+
+/* Add a JSON property name */
+void jsonw_name(json_writer_t *self, const char *name)
+{
+	jsonw_eor(self);
+	jsonw_eol(self);
+	self->sep = '\0';
+	jsonw_puts(self, name);
+	putc(':', self->out);
+	if (self->pretty)
+		putc(' ', self->out);
+}
+
+__attribute__((format(printf, 2, 3)))
+void jsonw_printf(json_writer_t *self, const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	jsonw_eor(self);
+	vfprintf(self->out, fmt, ap);
+	va_end(ap);
+}
+
+/* Collections */
+void jsonw_start_object(json_writer_t *self)
+{
+	jsonw_begin(self, '{');
+}
+
+void jsonw_end_object(json_writer_t *self)
+{
+	jsonw_end(self, '}');
+}
+
+void jsonw_start_array(json_writer_t *self)
+{
+	jsonw_begin(self, '[');
+	if (self->pretty)
+		putc(' ', self->out);
+}
+
+void jsonw_end_array(json_writer_t *self)
+{
+	if (self->pretty && self->sep)
+		putc(' ', self->out);
+	self->sep = '\0';
+	jsonw_end(self, ']');
+}
+
+/* JSON value types */
+void jsonw_string(json_writer_t *self, const char *value)
+{
+	jsonw_eor(self);
+	jsonw_puts(self, value);
+}
+
+void jsonw_bool(json_writer_t *self, bool val)
+{
+	jsonw_printf(self, "%s", val ? "true" : "false");
+}
+
+void jsonw_null(json_writer_t *self)
+{
+	jsonw_printf(self, "null");
+}
+
+void jsonw_float(json_writer_t *self, double num)
+{
+	jsonw_printf(self, "%g", num);
+}
+
+void jsonw_hhu(json_writer_t *self, unsigned char num)
+{
+	jsonw_printf(self, "%hhu", num);
+}
+
+void jsonw_hu(json_writer_t *self, unsigned short num)
+{
+	jsonw_printf(self, "%hu", num);
+}
+
+void jsonw_uint(json_writer_t *self, unsigned int num)
+{
+	jsonw_printf(self, "%u", num);
+}
+
+void jsonw_u64(json_writer_t *self, uint64_t num)
+{
+	jsonw_printf(self, "%"PRIu64, num);
+}
+
+void jsonw_xint(json_writer_t *self, uint64_t num)
+{
+	jsonw_printf(self, "%"PRIx64, num);
+}
+
+void jsonw_luint(json_writer_t *self, unsigned long num)
+{
+	jsonw_printf(self, "%lu", num);
+}
+
+void jsonw_lluint(json_writer_t *self, unsigned long long num)
+{
+	jsonw_printf(self, "%llu", num);
+}
+
+void jsonw_int(json_writer_t *self, int num)
+{
+	jsonw_printf(self, "%d", num);
+}
+
+void jsonw_s64(json_writer_t *self, int64_t num)
+{
+	jsonw_printf(self, "%"PRId64, num);
+}
+
+/* Basic name/value objects */
+void jsonw_string_field(json_writer_t *self, const char *prop, const char *val)
+{
+	jsonw_name(self, prop);
+	jsonw_string(self, val);
+}
+
+void jsonw_bool_field(json_writer_t *self, const char *prop, bool val)
+{
+	jsonw_name(self, prop);
+	jsonw_bool(self, val);
+}
+
+void jsonw_float_field(json_writer_t *self, const char *prop, double val)
+{
+	jsonw_name(self, prop);
+	jsonw_float(self, val);
+}
+
+void jsonw_uint_field(json_writer_t *self, const char *prop, unsigned int num)
+{
+	jsonw_name(self, prop);
+	jsonw_uint(self, num);
+}
+
+void jsonw_u64_field(json_writer_t *self, const char *prop, uint64_t num)
+{
+	jsonw_name(self, prop);
+	jsonw_u64(self, num);
+}
+
+void jsonw_xint_field(json_writer_t *self, const char *prop, uint64_t num)
+{
+	jsonw_name(self, prop);
+	jsonw_xint(self, num);
+}
+
+void jsonw_hhu_field(json_writer_t *self, const char *prop, unsigned char num)
+{
+	jsonw_name(self, prop);
+	jsonw_hhu(self, num);
+}
+
+void jsonw_hu_field(json_writer_t *self, const char *prop, unsigned short num)
+{
+	jsonw_name(self, prop);
+	jsonw_hu(self, num);
+}
+
+void jsonw_luint_field(json_writer_t *self,
+			const char *prop,
+			unsigned long num)
+{
+	jsonw_name(self, prop);
+	jsonw_luint(self, num);
+}
+
+void jsonw_lluint_field(json_writer_t *self,
+			const char *prop,
+			unsigned long long num)
+{
+	jsonw_name(self, prop);
+	jsonw_lluint(self, num);
+}
+
+void jsonw_int_field(json_writer_t *self, const char *prop, int num)
+{
+	jsonw_name(self, prop);
+	jsonw_int(self, num);
+}
+
+void jsonw_s64_field(json_writer_t *self, const char *prop, int64_t num)
+{
+	jsonw_name(self, prop);
+	jsonw_s64(self, num);
+}
+
+void jsonw_null_field(json_writer_t *self, const char *prop)
+{
+	jsonw_name(self, prop);
+	jsonw_null(self);
+}
+
+#ifdef TEST
+int main(int argc, char **argv)
+{
+	json_writer_t *wr = jsonw_new(stdout);
+
+	jsonw_start_object(wr);
+	jsonw_pretty(wr, true);
+	jsonw_name(wr, "Vyatta");
+	jsonw_start_object(wr);
+	jsonw_string_field(wr, "url", "http://vyatta.com");
+	jsonw_uint_field(wr, "downloads", 2000000ul);
+	jsonw_float_field(wr, "stock", 8.16);
+
+	jsonw_name(wr, "ARGV");
+	jsonw_start_array(wr);
+	while (--argc)
+		jsonw_string(wr, *++argv);
+	jsonw_end_array(wr);
+
+	jsonw_name(wr, "empty");
+	jsonw_start_array(wr);
+	jsonw_end_array(wr);
+
+	jsonw_name(wr, "NIL");
+	jsonw_start_object(wr);
+	jsonw_end_object(wr);
+
+	jsonw_null_field(wr, "my_null");
+
+	jsonw_name(wr, "special chars");
+	jsonw_start_array(wr);
+	jsonw_string_field(wr, "slash", "/");
+	jsonw_string_field(wr, "newline", "\n");
+	jsonw_string_field(wr, "tab", "\t");
+	jsonw_string_field(wr, "ff", "\f");
+	jsonw_string_field(wr, "quote", "\"");
+	jsonw_string_field(wr, "tick", "\'");
+	jsonw_string_field(wr, "backslash", "\\");
+	jsonw_end_array(wr);
+
+	jsonw_end_object(wr);
+
+	jsonw_end_object(wr);
+	jsonw_destroy(&wr);
+	return 0;
+}
+
+#endif
diff --git a/json_writer.h b/json_writer.h
new file mode 100644
index 0000000..b52dc2d
--- /dev/null
+++ b/json_writer.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
+/*
+ * Simple streaming JSON writer
+ *
+ * This takes care of the annoying bits of JSON syntax like the commas
+ * after elements
+ *
+ * Authors:	Stephen Hemminger <stephen@networkplumber.org>
+ */
+
+#ifndef _JSON_WRITER_H_
+#define _JSON_WRITER_H_
+
+#include <stdbool.h>
+#include <stdint.h>
+
+/* Opaque class structure */
+typedef struct json_writer json_writer_t;
+
+/* Create a new JSON stream */
+json_writer_t *jsonw_new(FILE *f);
+/* End output to JSON stream */
+void jsonw_destroy(json_writer_t **self_p);
+
+/* Cause output to have pretty whitespace */
+void jsonw_pretty(json_writer_t *self, bool on);
+
+/* Add property name */
+void jsonw_name(json_writer_t *self, const char *name);
+
+/* Add value  */
+__attribute__((format(printf, 2, 3)))
+void jsonw_printf(json_writer_t *self, const char *fmt, ...);
+void jsonw_string(json_writer_t *self, const char *value);
+void jsonw_bool(json_writer_t *self, bool value);
+void jsonw_float(json_writer_t *self, double number);
+void jsonw_float_fmt(json_writer_t *self, const char *fmt, double num);
+void jsonw_uint(json_writer_t *self, unsigned int number);
+void jsonw_u64(json_writer_t *self, uint64_t number);
+void jsonw_xint(json_writer_t *self, uint64_t number);
+void jsonw_hhu(json_writer_t *self, unsigned char num);
+void jsonw_hu(json_writer_t *self, unsigned short number);
+void jsonw_int(json_writer_t *self, int number);
+void jsonw_s64(json_writer_t *self, int64_t number);
+void jsonw_null(json_writer_t *self);
+void jsonw_luint(json_writer_t *self, unsigned long num);
+void jsonw_lluint(json_writer_t *self, unsigned long long num);
+
+/* Useful Combinations of name and value */
+void jsonw_string_field(json_writer_t *self, const char *prop, const char *val);
+void jsonw_bool_field(json_writer_t *self, const char *prop, bool value);
+void jsonw_float_field(json_writer_t *self, const char *prop, double num);
+void jsonw_uint_field(json_writer_t *self, const char *prop, unsigned int num);
+void jsonw_u64_field(json_writer_t *self, const char *prop, uint64_t num);
+void jsonw_xint_field(json_writer_t *self, const char *prop, uint64_t num);
+void jsonw_hhu_field(json_writer_t *self, const char *prop, unsigned char num);
+void jsonw_hu_field(json_writer_t *self, const char *prop, unsigned short num);
+void jsonw_int_field(json_writer_t *self, const char *prop, int num);
+void jsonw_s64_field(json_writer_t *self, const char *prop, int64_t num);
+void jsonw_null_field(json_writer_t *self, const char *prop);
+void jsonw_luint_field(json_writer_t *self, const char *prop,
+			unsigned long num);
+void jsonw_lluint_field(json_writer_t *self, const char *prop,
+			unsigned long long num);
+
+/* Collections */
+void jsonw_start_object(json_writer_t *self);
+void jsonw_end_object(json_writer_t *self);
+
+void jsonw_start_array(json_writer_t *self);
+void jsonw_end_array(json_writer_t *self);
+
+/* Override default exception handling */
+typedef void (jsonw_err_handler_fn)(const char *);
+
+#endif /* _JSON_WRITER_H_ */
-- 
2.27.0

[PATCH ethtool v3 4/6] Add --json command line argument parsing

[Andrew Lunn]

Allow --json to be passed as an option to select JSON output. The
option is handled in the same way as --debug, setting a variable in
the command context, which can then later be used per option to select
JSON outputters.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
v3:
Make use of json_print to simplify the code.
---
 ethtool.c            | 33 ++++++++++------
 internal.h           |  4 ++
 netlink/cable_test.c | 92 ++++++++++++++++++++++++++++++--------------
 3 files changed, 90 insertions(+), 39 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index a6bb9ac..ed132d4 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5518,10 +5518,10 @@ static int show_usage(struct cmd_context *ctx maybe_unused)
 	fprintf(stdout, PACKAGE " version " VERSION "\n");
 	fprintf(stdout,
 		"Usage:\n"
-		"        ethtool [ --debug MASK ] DEVNAME\t"
+		"        ethtool [ --debug MASK ][ --json ] DEVNAME\t"
 		"Display standard information about device\n");
 	for (i = 0; args[i].opts; i++) {
-		fputs("        ethtool [ --debug MASK ] ", stdout);
+		fputs("        ethtool [ --debug MASK ][ --json ] ", stdout);
 		fprintf(stdout, "%s %s\t%s\n",
 			args[i].opts,
 			args[i].no_dev ? "\t" : "DEVNAME",
@@ -5530,6 +5530,7 @@ static int show_usage(struct cmd_context *ctx maybe_unused)
 			fputs(args[i].xhelp, stdout);
 	}
 	nl_monitor_usage();
+	fprintf(stdout, "Not all options support JSON output\n");
 
 	return 0;
 }
@@ -5768,17 +5769,27 @@ int main(int argc, char **argp)
 	argp++;
 	argc--;
 
-	if (*argp && !strcmp(*argp, "--debug")) {
-		char *eptr;
+	while (true) {
+		if (*argp && !strcmp(*argp, "--debug")) {
+			char *eptr;
 
-		if (argc < 2)
-			exit_bad_args();
-		ctx.debug = strtoul(argp[1], &eptr, 0);
-		if (!argp[1][0] || *eptr)
-			exit_bad_args();
+			if (argc < 2)
+				exit_bad_args();
+			ctx.debug = strtoul(argp[1], &eptr, 0);
+			if (!argp[1][0] || *eptr)
+				exit_bad_args();
 
-		argp += 2;
-		argc -= 2;
+			argp += 2;
+			argc -= 2;
+			continue;
+		}
+		if (*argp && !strcmp(*argp, "--json")) {
+			ctx.json = true;
+			argp += 1;
+			argc -= 1;
+			continue;
+		}
+		break;
 	}
 	if (*argp && !strcmp(*argp, "--monitor")) {
 		ctx.argp = ++argp;
diff --git a/internal.h b/internal.h
index edb07bd..45b63b7 100644
--- a/internal.h
+++ b/internal.h
@@ -23,6 +23,9 @@
 #include <sys/ioctl.h>
 #include <net/if.h>
 
+#include "json_writer.h"
+#include "json_print.h"
+
 #define maybe_unused __attribute__((__unused__))
 
 /* internal for netlink interface */
@@ -221,6 +224,7 @@ struct cmd_context {
 	int argc;		/* number of arguments to the sub-command */
 	char **argp;		/* arguments to the sub-command */
 	unsigned long debug;	/* debugging mask */
+	bool json;		/* Output JSON, if supported */
 #ifdef ETHTOOL_ENABLE_NETLINK
 	struct nl_context *nlctx;	/* netlink context (opaque) */
 #endif
diff --git a/netlink/cable_test.c b/netlink/cable_test.c
index 1b9ae9c..3fee7a0 100644
--- a/netlink/cable_test.c
+++ b/netlink/cable_test.c
@@ -86,7 +86,8 @@ static char *nl_pair2txt(uint8_t pair)
 	}
 }
 
-static int nl_cable_test_ntf_attr(struct nlattr *evattr)
+static int nl_cable_test_ntf_attr(struct nlattr *evattr,
+				  struct nl_context *nlctx)
 {
 	unsigned int cm;
 	uint16_t code;
@@ -99,29 +100,34 @@ static int nl_cable_test_ntf_attr(struct nlattr *evattr)
 		if (ret < 0)
 			return ret;
 
-		printf("Pair: %s, result: %s\n", nl_pair2txt(pair),
-		       nl_code2txt(code));
+		open_json_object(NULL);
+		print_string(PRINT_ANY, "pair", "%s ", nl_pair2txt(pair));
+		print_string(PRINT_ANY, "code", "code %s\n", nl_code2txt(code));
+		close_json_object();
 		break;
 
 	case ETHTOOL_A_CABLE_NEST_FAULT_LENGTH:
 		ret = nl_get_cable_test_fault_length(evattr, &pair, &cm);
 		if (ret < 0)
 			return ret;
-
-		printf("Pair: %s, fault length: %0.2fm\n",
-		       nl_pair2txt(pair), (float)cm / 100);
+		open_json_object(NULL);
+		print_string(PRINT_ANY, "pair", "%s, ", nl_pair2txt(pair));
+		print_float(PRINT_ANY, "length", "fault length: %0.2fm\n",
+			    (float)cm / 100);
+		close_json_object();
 		break;
 	}
 	return 0;
 }
 
-static void cable_test_ntf_nest(const struct nlattr *nest)
+static void cable_test_ntf_nest(const struct nlattr *nest,
+				struct nl_context *nlctx)
 {
 	struct nlattr *pos;
 	int ret;
 
 	mnl_attr_for_each_nested(pos, nest) {
-		ret = nl_cable_test_ntf_attr(pos);
+		ret = nl_cable_test_ntf_attr(pos, nlctx);
 		if (ret < 0)
 			return;
 	}
@@ -154,19 +160,21 @@ static int cable_test_ntf_stop_cb(const struct nlmsghdr *nlhdr, void *data)
 
 	switch (status) {
 	case ETHTOOL_A_CABLE_TEST_NTF_STATUS_STARTED:
-		printf("Cable test started for device %s.\n",
-		       nlctx->devname);
+		print_string(PRINT_FP, "status",
+			     "Cable test started for device %s.\n",
+			     nlctx->devname);
 		break;
 	case ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED:
-		printf("Cable test completed for device %s.\n",
-		       nlctx->devname);
+		print_string(PRINT_FP, "status",
+			     "Cable test completed for device %s.\n",
+			     nlctx->devname);
 		break;
 	default:
 		break;
 	}
 
 	if (tb[ETHTOOL_A_CABLE_TEST_NTF_NEST])
-		cable_test_ntf_nest(tb[ETHTOOL_A_CABLE_TEST_NTF_NEST]);
+		cable_test_ntf_nest(tb[ETHTOOL_A_CABLE_TEST_NTF_NEST], nlctx);
 
 	if (status == ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED) {
 		breakout = true;
@@ -252,8 +260,14 @@ int nl_cable_test(struct cmd_context *ctx)
 	ret = nlsock_sendmsg(nlsk, NULL);
 	if (ret < 0)
 		fprintf(stderr, "Cannot start cable test\n");
-	else
+	else {
+		new_json_obj(ctx->json);
+
 		ret = nl_cable_test_process_results(ctx);
+
+		delete_json_obj();
+	}
+
 	return ret;
 }
 
@@ -316,7 +330,8 @@ static int nl_get_cable_test_tdr_step(const struct nlattr *nest,
 	return 0;
 }
 
-static int nl_cable_test_tdr_ntf_attr(struct nlattr *evattr)
+static int nl_cable_test_tdr_ntf_attr(struct nlattr *evattr,
+				      struct nl_context *nlctx)
 {
 	uint32_t first, last, step;
 	uint8_t pair;
@@ -331,7 +346,10 @@ static int nl_cable_test_tdr_ntf_attr(struct nlattr *evattr)
 		if (ret < 0)
 			return ret;
 
-		printf("Pair: %s, amplitude %4d\n", nl_pair2txt(pair), mV);
+		open_json_object(NULL);
+		print_string(PRINT_ANY, "pair", "%s ", nl_pair2txt(pair));
+		print_uint(PRINT_ANY, "amplitude", "Amplitude %4d\n", mV);
+		close_json_object();
 		break;
 	}
 	case ETHTOOL_A_CABLE_TDR_NEST_PULSE: {
@@ -341,7 +359,9 @@ static int nl_cable_test_tdr_ntf_attr(struct nlattr *evattr)
 		if (ret < 0)
 			return ret;
 
-		printf("TDR pulse %dmV\n", mV);
+		open_json_object(NULL);
+		print_uint(PRINT_ANY, "pulse", "TDR Pulse %dmV\n", mV);
+		close_json_object();
 		break;
 	}
 	case ETHTOOL_A_CABLE_TDR_NEST_STEP:
@@ -349,21 +369,27 @@ static int nl_cable_test_tdr_ntf_attr(struct nlattr *evattr)
 		if (ret < 0)
 			return ret;
 
-		printf("Step configuration, %.2f-%.2f meters in %.2fm steps\n",
-		       (float)first / 100, (float)last /  100,
-		       (float)step /  100);
+		open_json_object(NULL);
+		print_float(PRINT_ANY, "first", "Step configuration: %.2f-",
+			    (float)first / 100);
+		print_float(PRINT_ANY, "last", "%.2f meters ",
+			    (float)last / 100);
+		print_float(PRINT_ANY, "step", "in %.2fm steps\n",
+			    (float)step / 100);
+		close_json_object();
 		break;
 	}
 	return 0;
 }
 
-static void cable_test_tdr_ntf_nest(const struct nlattr *nest)
+static void cable_test_tdr_ntf_nest(const struct nlattr *nest,
+				    struct nl_context *nlctx)
 {
 	struct nlattr *pos;
 	int ret;
 
 	mnl_attr_for_each_nested(pos, nest) {
-		ret = nl_cable_test_tdr_ntf_attr(pos);
+		ret = nl_cable_test_tdr_ntf_attr(pos, nlctx);
 		if (ret < 0)
 			return;
 	}
@@ -376,6 +402,7 @@ int cable_test_tdr_ntf_stop_cb(const struct nlmsghdr *nlhdr, void *data)
 	const struct nlattr *tb[ETHTOOL_A_CABLE_TEST_TDR_NTF_MAX + 1] = {};
 	u8 status = ETHTOOL_A_CABLE_TEST_NTF_STATUS_UNSPEC;
 	struct nl_context *nlctx = data;
+
 	DECLARE_ATTR_TB_INFO(tb);
 	bool silent;
 	int err_ret;
@@ -396,19 +423,22 @@ int cable_test_tdr_ntf_stop_cb(const struct nlmsghdr *nlhdr, void *data)
 
 	switch (status) {
 	case ETHTOOL_A_CABLE_TEST_NTF_STATUS_STARTED:
-		printf("Cable test TDR started for device %s.\n",
-		       nlctx->devname);
+		print_string(PRINT_FP, "status",
+			     "Cable test TDR started for device %s.\n",
+			     nlctx->devname);
 		break;
 	case ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED:
-		printf("Cable test TDR completed for device %s.\n",
-		       nlctx->devname);
+		print_string(PRINT_FP, "status",
+			     "Cable test TDR completed for device %s.\n",
+			     nlctx->devname);
 		break;
 	default:
 		break;
 	}
 
 	if (tb[ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST])
-		cable_test_tdr_ntf_nest(tb[ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST]);
+		cable_test_tdr_ntf_nest(tb[ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST],
+					nlctx);
 
 	if (status == ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED) {
 		breakout = true;
@@ -540,7 +570,13 @@ int nl_cable_test_tdr(struct cmd_context *ctx)
 	ret = nlsock_sendmsg(nlsk, NULL);
 	if (ret < 0)
 		fprintf(stderr, "Cannot start cable test TDR\n");
-	else
+	else {
+		new_json_obj(ctx->json);
+
 		ret = nl_cable_test_tdr_process_results(ctx);
+
+		delete_json_obj();
+	}
+
 	return ret;
 }
-- 
2.27.0

[PATCH ethtool v3 5/6] ethtool.8.in: Document the cable test commands

[Andrew Lunn]

Extend the man page with --cable-test and --cable-test-tdr commands.

Reviewed-by: Chris Healy <cphealy@gmail.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>

---
v2: Fixup some grammar/English errors.
v3: More grammar/English errors fixed.
---
 ethtool.8.in | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9c5f45c..60ca37c 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -434,6 +434,16 @@ ethtool \- query or control network driver and hardware settings
 .I sub_command
 .RB ...
  .
+.HP
+.B ethtool \-\-cable\-test
+.I devname
+.HP
+.B ethtool \-\-cable\-test\-tdr
+.I devname
+.BN first N
+.BN last N
+.BN step N
+.BN pair N
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -1277,6 +1287,41 @@ Sub command to apply. The supported sub commands include --show-coalesce and
 --coalesce.
 .RE
 .TP
+q.B \-\-cable\-test
+Perform a cable test and report the results. What results are returned depends
+on the capabilities of the network interface. Typically open pairs and shorted
+pairs can be reported, along with pairs being O.K. When a fault is detected
+the approximate distance to the fault may be reported.
+.TP
+.B \-\-cable\-test\-tdr
+Perform a cable test and report the raw Time Domain Reflectometer
+data.  A pulse is sent down a cable pair and the amplitude of the
+reflection, for a given distance, is reported. A break in the cable
+returns a big reflection. Minor damage to the cable returns a small
+reflection. If the cable is shorted, the amplitude of the reflection
+can be negative. By default, data is returned for lengths between 0
+and 150m at 1m steps, for all pairs. However parameters can be passed
+to restrict the collection of data. It should be noted, that the
+interface will round the distances to whatever granularity is actually
+implemented. This is often 0.8 of a meter. The results should include
+the actual rounded first and last distance and step size.
+.RS 4
+.TP
+.B first \ N
+Distance along the cable, in meters, where the first measurement
+should be made.
+.TP
+.B last \ N
+Distance along the cable, in meters, where the last measurement should
+be made.
+.TP
+.B step \ N
+Distance, in meters, between each measurement.
+.TP
+.B pair \ N
+Which pair should be measured. Typically a cable has 4 pairs. 0 = Pair A, 1 = Pair B, ...
+.RE
+.TP
 .B \-\-monitor
 Listens to netlink notification and displays them.
 .RS 4
-- 
2.27.0

[PATCH ethtool v3 6/6] ethtool.8.in: Add --json option

[Andrew Lunn]

Document the --json option, which the --cable-test and
--cable-test-tdr options support.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 ethtool.8.in | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 60ca37c..689822e 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -137,6 +137,9 @@ ethtool \- query or control network driver and hardware settings
 .BN --debug
 .I args
 .HP
+.B ethtool [--json]
+.I args
+.HP
 .B ethtool \-\-monitor
 [
 .I command
@@ -476,6 +479,11 @@ lB	l.
 0x01  Parser information
 .TE
 .TP
+.BI \-\-json
+Output results in JavaScript Object Notation (JSON). Only a subset of
+options support this. Those which do not will continue to output
+plain text in the presence of this option.
+.TP
 .B \-a \-\-show\-pause
 Queries the specified Ethernet device for pause parameter information.
 .TP
-- 
2.27.0

Re: [PATCH ethtool v3 1/6] Add cable test support

[Michal Kubecek]

On Thu, Jun 25, 2020 at 09:24:41PM +0200, Andrew Lunn wrote:
> Add support for starting a cable test, and report the results.
> 
> This code does not follow the usual patterns because of the way the
> kernel reports the results of the cable test. It can take a number of
> seconds for cable testing to occur. So the action request messages is
> immediately acknowledges, and the test is then performed asynchronous.
> Once the test has completed, the results are returned as a
> notification.
> 
> This means the command starts as usual. It then monitors multicast
> messages until it receives the results.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
[...]
> --- /dev/null
> +++ b/netlink/cable_test.c
> @@ -0,0 +1,257 @@
> +/*
> + * cable_test.c - netlink implementation of cable test command
> + *
> + * Implementation of ethtool <dev> --cable-test

This should be "ethtool --cable-test <dev>", I believe.

> + */
> +
> +#include <errno.h>
> +#include <string.h>
> +#include <stdio.h>
> +
> +#include "../internal.h"
> +#include "../common.h"
> +#include "netlink.h"
> +
> +static bool breakout;

I would prefer to avoid global state variables to prevent possible
reentrancy issues in the future. First, monitor will almost certainly
have to process events from multiple sockets (ethtool netlink, devlink,
rtnetlink) and I'm still not sure which implementation to use. Second,
there is an idea (perhaps too ambitious) of transforming part of the
code into a library which could be used not only by ethtool but also by
other tools.

nl_context::cmd_private can be used for this purpose (there is an
example in nl_sfeatures() and related code).

[...]
> +static char *nl_pair2txt(uint8_t pair)
> +{
> +	switch (pair) {
> +	case ETHTOOL_A_CABLE_PAIR_A:
> +		return "Pair A";
> +	case ETHTOOL_A_CABLE_PAIR_B:
> +		return "Pair B";
> +	case ETHTOOL_A_CABLE_PAIR_C:
> +		return "Pair C";
> +	case ETHTOOL_A_CABLE_PAIR_D:
> +		return "Pair D";
> +	default:
> +		return "Unexpected pair";
> +	}
> +}
> +
> +static int nl_cable_test_ntf_attr(struct nlattr *evattr)
> +{
> +	unsigned int cm;
> +	uint16_t code;
> +	uint8_t pair;
> +	int ret;
> +
> +	switch (mnl_attr_get_type(evattr)) {
> +	case ETHTOOL_A_CABLE_NEST_RESULT:
> +		ret = nl_get_cable_test_result(evattr, &pair, &code);
> +		if (ret < 0)
> +			return ret;
> +
> +		printf("Pair: %s, result: %s\n", nl_pair2txt(pair),
> +		       nl_code2txt(code));

AFAICS this will produce output like "Pair: Pair A, ..." which looks
a bit strange, is it intended? (The same below).

> +		break;
> +
> +	case ETHTOOL_A_CABLE_NEST_FAULT_LENGTH:
> +		ret = nl_get_cable_test_fault_length(evattr, &pair, &cm);
> +		if (ret < 0)
> +			return ret;
> +
> +		printf("Pair: %s, fault length: %0.2fm\n",
> +		       nl_pair2txt(pair), (float)cm / 100);
> +		break;
> +	}
> +	return 0;
> +}
[...]
> +/* Receive the broadcasted messages until we get the cable test
> + * results
> + */
> +static int nl_cable_test_process_results(struct cmd_context *ctx)
> +{
> +        struct nl_context *nlctx = ctx->nlctx;
> +	struct nl_socket *nlsk = nlctx->ethnl_socket;
> +	int err;
> +
> +        nlctx->is_monitor = true;
> +        nlsk->port = 0;
> +	nlsk->seq = 0;

Some of the lines above have wrong indentation (and some more in
nl_cable_test()).

Michal

Re: [PATCH ethtool v3 2/6] Add cable test TDR support

[Michal Kubecek]

On Thu, Jun 25, 2020 at 09:24:42PM +0200, Andrew Lunn wrote:
> Add support for accessing the cable test time domain reflectromatry
> data. Add a new command --cable-test-tdr, and support for dumping the
> data which is returned.
> 
> signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---

Looks good to me, except for the use of global variable breakout that I
commented at patch 1/6.

Michal

Re: [PATCH ethtool v3 1/6] Add cable test support

[Andrew Lunn]

> > +static int nl_cable_test_ntf_attr(struct nlattr *evattr)
> > +{
> > +	unsigned int cm;
> > +	uint16_t code;
> > +	uint8_t pair;
> > +	int ret;
> > +
> > +	switch (mnl_attr_get_type(evattr)) {
> > +	case ETHTOOL_A_CABLE_NEST_RESULT:
> > +		ret = nl_get_cable_test_result(evattr, &pair, &code);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		printf("Pair: %s, result: %s\n", nl_pair2txt(pair),
> > +		       nl_code2txt(code));
> 
> AFAICS this will produce output like "Pair: Pair A, ..." which looks
> a bit strange, is it intended? (The same below).

Hi Michal

The later patch which adds JSON support changes this. The Pair: gets
removed. I thought it was odd as well. But it did not go back through
the patches and change where is was initially added.

    Andrew