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

[ethtool v2 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.

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

[ethtool v2 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.26.2

[ethtool v2 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.26.2

[ethtool v2 3/6] json_writer: 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.

Maybe some time in the future it would make sense to either have a
shared library, or to merge ethtool into the iproute2 source
distribution?

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 Makefile.am   |   3 +-
 json_writer.c | 389 ++++++++++++++++++++++++++++++++++++++++++++++++++
 json_writer.h |  76 ++++++++++
 3 files changed, 467 insertions(+), 1 deletion(-)
 create mode 100644 json_writer.c
 create mode 100644 json_writer.h

diff --git a/Makefile.am b/Makefile.am
index a818cf8..7edbd5b 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
 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_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.26.2

Re: [ethtool v2 3/6] json_writer: Import the iproute2 helper code for JSON output

[Stephen Hemminger]

On Thu, 25 Jun 2020 02:12:41 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> 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.
> 
> Maybe some time in the future it would make sense to either have a
> shared library, or to merge ethtool into the iproute2 source
> distribution?
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

LGTM

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

A shared library is hard to version and gets to be a chore.

Re: [ethtool v2 3/6] json_writer: Import the iproute2 helper code for JSON output

[Stephen Hemminger]

On Thu, 25 Jun 2020 02:12:41 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> 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.
> 
> Maybe some time in the future it would make sense to either have a
> shared library, or to merge ethtool into the iproute2 source
> distribution?
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

You might also want to borrow the json_print code that merges both
output formats.  Or not

[ethtool v2 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>
---
 ethtool.c            |  33 ++++++---
 internal.h           |   4 ++
 netlink/cable_test.c | 162 ++++++++++++++++++++++++++++++++-----------
 3 files changed, 146 insertions(+), 53 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..7135140 100644
--- a/internal.h
+++ b/internal.h
@@ -23,6 +23,8 @@
 #include <sys/ioctl.h>
 #include <net/if.h>
 
+#include "json_writer.h"
+
 #define maybe_unused __attribute__((__unused__))
 
 /* internal for netlink interface */
@@ -221,6 +223,8 @@ 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 */
+	json_writer_t *jw;      /* JSON writer instance */
 #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..be98247 100644
--- a/netlink/cable_test.c
+++ b/netlink/cable_test.c
@@ -86,8 +86,10 @@ 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)
 {
+	struct cmd_context *ctx =  nlctx->ctx;
 	unsigned int cm;
 	uint16_t code;
 	uint8_t pair;
@@ -99,29 +101,44 @@ 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));
+		if (ctx->jw) {
+			jsonw_start_object(ctx->jw);
+			jsonw_string_field(ctx->jw, "pair", nl_pair2txt(pair));
+			jsonw_string_field(ctx->jw, "code", nl_code2txt(code));
+			jsonw_end_object(ctx->jw);
+		} else {
+			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);
+		if (ctx->jw) {
+			jsonw_start_object(ctx->jw);
+			jsonw_string_field(ctx->jw, "pair", nl_pair2txt(pair));
+			jsonw_float_field(ctx->jw, "length", (float)cm / 100);
+			jsonw_end_object(ctx->jw);
+		} else {
+			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)
+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;
 	}
@@ -134,6 +151,7 @@ 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;
+	struct cmd_context *ctx =  nlctx->ctx;
 	DECLARE_ATTR_TB_INFO(tb);
 	bool silent;
 	int err_ret;
@@ -152,21 +170,23 @@ static int cable_test_ntf_stop_cb(const struct nlmsghdr *nlhdr, void *data)
 	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 (!ctx->json) {
+		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]);
+		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 +272,21 @@ 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 {
+		if (ctx->json) {
+			ctx->jw =  jsonw_new(stdout);
+			jsonw_pretty(ctx->jw, true);
+			jsonw_start_array(ctx->jw);
+		}
+
 		ret = nl_cable_test_process_results(ctx);
+
+		if (ctx->json) {
+			jsonw_end_array(ctx->jw);
+			jsonw_destroy(&ctx->jw);
+		}
+	}
+
 	return ret;
 }
 
@@ -316,8 +349,10 @@ 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)
 {
+	struct cmd_context *ctx =  nlctx->ctx;
 	uint32_t first, last, step;
 	uint8_t pair;
 	int ret;
@@ -331,7 +366,16 @@ 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);
+		if (ctx->jw) {
+			jsonw_start_object(ctx->jw);
+			jsonw_string_field(ctx->jw, "pair", nl_pair2txt(pair));
+			jsonw_int_field(ctx->jw, "amplitude", mV);
+			jsonw_end_object(ctx->jw);
+		} else {
+			printf("Pair: %s, amplitude %4d\n",
+			       nl_pair2txt(pair), mV);
+		}
+
 		break;
 	}
 	case ETHTOOL_A_CABLE_TDR_NEST_PULSE: {
@@ -341,7 +385,14 @@ static int nl_cable_test_tdr_ntf_attr(struct nlattr *evattr)
 		if (ret < 0)
 			return ret;
 
-		printf("TDR pulse %dmV\n", mV);
+		if (ctx->jw) {
+			jsonw_start_object(ctx->jw);
+			jsonw_uint_field(ctx->jw, "pulse", mV);
+			jsonw_end_object(ctx->jw);
+		} else {
+			printf("TDR pulse %dmV\n", mV);
+		}
+
 		break;
 	}
 	case ETHTOOL_A_CABLE_TDR_NEST_STEP:
@@ -349,21 +400,30 @@ 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);
+		if (ctx->jw) {
+			jsonw_start_object(ctx->jw);
+			jsonw_float_field(ctx->jw, "first", (float)first / 100);
+			jsonw_float_field(ctx->jw, "last", (float)last / 100);
+			jsonw_float_field(ctx->jw, "step", (float)step / 100);
+			jsonw_end_object(ctx->jw);
+		} else {
+			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)
+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 +436,8 @@ 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;
+	struct cmd_context *ctx =  nlctx->ctx;
+
 	DECLARE_ATTR_TB_INFO(tb);
 	bool silent;
 	int err_ret;
@@ -394,21 +456,24 @@ int cable_test_tdr_ntf_stop_cb(const struct nlmsghdr *nlhdr, void *data)
 	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 (!ctx->json) {
+		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]);
+		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 +605,20 @@ 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 {
+		if (ctx->json) {
+			ctx->jw =  jsonw_new(stdout);
+			jsonw_pretty(ctx->jw, true);
+			jsonw_start_array(ctx->jw);
+		}
+
 		ret = nl_cable_test_tdr_process_results(ctx);
+
+		if (ctx->json) {
+			jsonw_end_array(ctx->jw);
+			jsonw_destroy(&ctx->jw);
+		}
+	}
+
 	return ret;
 }
-- 
2.26.2

Re: [ethtool v2 4/6] Add --json command line argument parsing

[Stephen Hemminger]

On Thu, 25 Jun 2020 02:12:42 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> diff --git a/internal.h b/internal.h
> index edb07bd..7135140 100644
> --- a/internal.h
> +++ b/internal.h
> @@ -23,6 +23,8 @@
>  #include <sys/ioctl.h>
>  #include <net/if.h>
>  
> +#include "json_writer.h"
> +
>  #define maybe_unused __attribute__((__unused__))
>  
>  /* internal for netlink interface */
> @@ -221,6 +223,8 @@ 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 */
> +	json_writer_t *jw;      /* JSON writer instance */

You can avoid the boolean by just checking for NULL jw variable.

Re: [ethtool v2 4/6] Add --json command line argument parsing

[Andrew Lunn]

On Wed, Jun 24, 2020 at 10:32:58PM -0700, Stephen Hemminger wrote:
> On Thu, 25 Jun 2020 02:12:42 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > diff --git a/internal.h b/internal.h
> > index edb07bd..7135140 100644
> > --- a/internal.h
> > +++ b/internal.h
> > @@ -23,6 +23,8 @@
> >  #include <sys/ioctl.h>
> >  #include <net/if.h>
> >  
> > +#include "json_writer.h"
> > +
> >  #define maybe_unused __attribute__((__unused__))
> >  
> >  /* internal for netlink interface */
> > @@ -221,6 +223,8 @@ 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 */
> > +	json_writer_t *jw;      /* JSON writer instance */
> 
> You can avoid the boolean by just checking for NULL jw variable.

Hi Stephen

It is a while since i wrote this code, but i think i considered
that. The problem is, only a few commands support json output. I could
call json_new() unconditional of if the command actually support json
or not, that is not a problem. But then json_destory() should also be
unconditionally called. And that does fputs("\n", self->out); So you
end up with an extra blank line.

Using the boolean allows me to defer json_new()/json_destroy() into
the actual commands which supports json.

       Andrew

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

[Andrew Lunn]

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

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: Fixup some grammar/English errors.
---
 ethtool.8.in | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9c5f45c..8b78b17 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 maybe 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.26.2

[ethtool v2 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 8b78b17..b2bdd43 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.26.2