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

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH ethtool v4 0/6] ethtool(1) cable test support
@ 2020-07-01  1:07 Andrew Lunn
  2020-07-01  1:07 ` [PATCH ethtool v4 1/6] Add " Andrew Lunn
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Andrew Lunn @ 2020-07-01  1:07 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy, 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.

v4:
checkpatch cleanup
ethtool --cable-test dev
Place breakout into cable_test_context
Remove Pair: Pair output

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH ethtool v4 1/6] Add cable test support
  2020-07-01  1:07 [PATCH ethtool v4 0/6] ethtool(1) cable test support Andrew Lunn
@ 2020-07-01  1:07 ` Andrew Lunn
  2020-07-04 23:43   ` Michal Kubecek
  2020-07-01  1:07 ` [PATCH ethtool v4 2/6] Add cable test TDR support Andrew Lunn
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2020-07-01  1:07 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy, 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 | 266 +++++++++++++++++++++++++++++++++++++++++++
 netlink/extapi.h     |   2 +
 netlink/monitor.c    |   4 +
 netlink/netlink.h    |   3 +-
 6 files changed, 280 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..a961654
--- /dev/null
+++ b/netlink/cable_test.c
@@ -0,0 +1,266 @@
+/*
+ * cable_test.c - netlink implementation of cable test command
+ *
+ * Implementation of ethtool --cable-test <dev>
+ */
+
+#include <errno.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "../internal.h"
+#include "../common.h"
+#include "netlink.h"
+
+struct cable_test_context {
+	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 cable_test_context *ctctx;
+	struct nl_context *nlctx = data;
+	DECLARE_ATTR_TB_INFO(tb);
+	bool silent;
+	int err_ret;
+	int ret;
+
+	ctctx = nlctx->cmd_private;
+
+	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) {
+		if (ctctx)
+			ctctx->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;
+	struct cable_test_context ctctx;
+	int err;
+
+	nlctx->is_monitor = true;
+	nlsk->port = 0;
+	nlsk->seq = 0;
+
+	ctctx.breakout = false;
+	nlctx->cmd_private = &ctctx;
+
+	while (!ctctx.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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH ethtool v4 1/6] Add cable test support
  2020-07-01  1:07 ` [PATCH ethtool v4 1/6] Add " Andrew Lunn
@ 2020-07-04 23:43   ` Michal Kubecek
  0 siblings, 0 replies; 14+ messages in thread
From: Michal Kubecek @ 2020-07-04 23:43 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Chris Healy

On Wed, Jul 01, 2020 at 03:07:38AM +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>
> ---
[...]
> 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,
> +	},
>  };
>  

Please add also an entry to monitor_opts[] so that user can use
--cable-test as filter for --monitor.

Looks good otherwise.

Michal

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH ethtool v4 2/6] Add cable test TDR support
  2020-07-01  1:07 [PATCH ethtool v4 0/6] ethtool(1) cable test support Andrew Lunn
  2020-07-01  1:07 ` [PATCH ethtool v4 1/6] Add " Andrew Lunn
@ 2020-07-01  1:07 ` Andrew Lunn
  2020-07-04 23:58   ` Michal Kubecek
  2020-07-01  1:07 ` [PATCH ethtool v4 3/6] json_writer/json_print: Import the iproute2 helper code for JSON output Andrew Lunn
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2020-07-01  1:07 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy, 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 | 296 +++++++++++++++++++++++++++++++++++++++++++
 netlink/extapi.h     |   2 +
 netlink/monitor.c    |   4 +
 netlink/netlink.h    |   2 +
 netlink/parser.c     |  42 ++++++
 netlink/parser.h     |   4 +
 7 files changed, 358 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 a961654..1672f55 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"
 
 struct cable_test_context {
 	bool breakout;
@@ -264,3 +265,298 @@ 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 cable_test_context *ctctx;
+	struct nl_context *nlctx = data;
+	DECLARE_ATTR_TB_INFO(tb);
+	bool silent;
+	int err_ret;
+	int ret;
+
+	ctctx = nlctx->cmd_private;
+
+	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) {
+		if (ctctx)
+			ctctx->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;
+	struct cable_test_context ctctx;
+	int err;
+
+	nlctx->is_monitor = true;
+	nlsk->port = 0;
+	nlsk->seq = 0;
+
+	ctctx.breakout = false;
+	nlctx->cmd_private = &ctctx;
+
+	while (!ctctx.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..67134f1 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,32 @@ 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH ethtool v4 2/6] Add cable test TDR support
  2020-07-01  1:07 ` [PATCH ethtool v4 2/6] Add cable test TDR support Andrew Lunn
@ 2020-07-04 23:58   ` Michal Kubecek
  0 siblings, 0 replies; 14+ messages in thread
From: Michal Kubecek @ 2020-07-04 23:58 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Chris Healy

On Wed, Jul 01, 2020 at 03:07:39AM +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, only three minor issues:

> --- 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"

The "pair" parameter is missing here.

> +	},
>  	{
>  		.opts	= "-h|--help",
>  		.no_dev	= true,
[...]
> 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,
> +	},
>  };
>  

Please add also an entry to monitor_opts[] to allow filtering monitor
events.

[...]
> diff --git a/netlink/parser.c b/netlink/parser.c
> index bd3526f..67134f1 100644
> --- a/netlink/parser.c
> +++ b/netlink/parser.c
[...]
> @@ -211,6 +227,32 @@ 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);

When I tried to test the generated requests, I noticed that e.g. "20.3"
was sent as 2029 rather than 2030. It's unlikely to cause an actual
problem but we should probably use e.g.

	cm = (uint32_t)(meters * 100 + 0.5);

to prevent such rounding errors.

Michal

> +	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".
>   */

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH ethtool v4 3/6] json_writer/json_print: Import the iproute2 helper code for JSON output
  2020-07-01  1:07 [PATCH ethtool v4 0/6] ethtool(1) cable test support Andrew Lunn
  2020-07-01  1:07 ` [PATCH ethtool v4 1/6] Add " Andrew Lunn
  2020-07-01  1:07 ` [PATCH ethtool v4 2/6] Add cable test TDR support Andrew Lunn
@ 2020-07-01  1:07 ` Andrew Lunn
  2020-07-01  1:07 ` [PATCH ethtool v4 4/6] Add --json command line argument parsing Andrew Lunn
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2020-07-01  1:07 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy, Andrew Lunn, Stephen Hemminger

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  | 228 +++++++++++++++++++++++++++++
 json_print.h  |  67 +++++++++
 json_writer.c | 391 ++++++++++++++++++++++++++++++++++++++++++++++++++
 json_writer.h |  76 ++++++++++
 5 files changed, 764 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..56d5b43
--- /dev/null
+++ b/json_print.c
@@ -0,0 +1,228 @@
+/*
+ * 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 separator (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..cc0c2ea
--- /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..e8b3926
--- /dev/null
+++ b/json_writer.c
@@ -0,0 +1,391 @@
+// 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 int	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 int 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH ethtool v4 4/6] Add --json command line argument parsing
  2020-07-01  1:07 [PATCH ethtool v4 0/6] ethtool(1) cable test support Andrew Lunn
                   ` (2 preceding siblings ...)
  2020-07-01  1:07 ` [PATCH ethtool v4 3/6] json_writer/json_print: Import the iproute2 helper code for JSON output Andrew Lunn
@ 2020-07-01  1:07 ` Andrew Lunn
  2020-07-01  1:07 ` [PATCH ethtool v4 5/6] ethtool.8.in: Document the cable test commands Andrew Lunn
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2020-07-01  1:07 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy, 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 1672f55..c2b9c97 100644
--- a/netlink/cable_test.c
+++ b/netlink/cable_test.c
@@ -88,7 +88,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;
@@ -101,29 +102,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;
 	}
@@ -160,19 +166,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) {
 		if (ctctx)
@@ -261,8 +269,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;
 }
 
@@ -325,7 +339,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;
@@ -340,7 +355,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: {
@@ -350,7 +368,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:
@@ -358,21 +378,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;
 	}
@@ -387,6 +413,7 @@ int cable_test_tdr_ntf_stop_cb(const struct nlmsghdr *nlhdr, void *data)
 	u8 status = ETHTOOL_A_CABLE_TEST_NTF_STATUS_UNSPEC;
 	struct cable_test_context *ctctx;
 	struct nl_context *nlctx = data;
+
 	DECLARE_ATTR_TB_INFO(tb);
 	bool silent;
 	int err_ret;
@@ -409,19 +436,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) {
 		if (ctctx)
@@ -556,7 +586,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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH ethtool v4 5/6] ethtool.8.in: Document the cable test commands
  2020-07-01  1:07 [PATCH ethtool v4 0/6] ethtool(1) cable test support Andrew Lunn
                   ` (3 preceding siblings ...)
  2020-07-01  1:07 ` [PATCH ethtool v4 4/6] Add --json command line argument parsing Andrew Lunn
@ 2020-07-01  1:07 ` Andrew Lunn
  2020-07-01  1:07 ` [PATCH ethtool v4 6/6] ethtool.8.in: Add --json option Andrew Lunn
  2020-07-05  0:44 ` [PATCH ethtool v4 0/6] ethtool(1) cable test support Michal Kubecek
  6 siblings, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2020-07-01  1:07 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy, 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH ethtool v4 6/6] ethtool.8.in: Add --json option
  2020-07-01  1:07 [PATCH ethtool v4 0/6] ethtool(1) cable test support Andrew Lunn
                   ` (4 preceding siblings ...)
  2020-07-01  1:07 ` [PATCH ethtool v4 5/6] ethtool.8.in: Document the cable test commands Andrew Lunn
@ 2020-07-01  1:07 ` Andrew Lunn
  2020-07-05  0:44 ` [PATCH ethtool v4 0/6] ethtool(1) cable test support Michal Kubecek
  6 siblings, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2020-07-01  1:07 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy, 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH ethtool v4 0/6] ethtool(1) cable test support
  2020-07-01  1:07 [PATCH ethtool v4 0/6] ethtool(1) cable test support Andrew Lunn
                   ` (5 preceding siblings ...)
  2020-07-01  1:07 ` [PATCH ethtool v4 6/6] ethtool.8.in: Add --json option Andrew Lunn
@ 2020-07-05  0:44 ` Michal Kubecek
  2020-07-05 16:24   ` Andrew Lunn
  2020-07-05 18:25   ` Andrew Lunn
  6 siblings, 2 replies; 14+ messages in thread
From: Michal Kubecek @ 2020-07-05  0:44 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Chris Healy

On Wed, Jul 01, 2020 at 03:07:37AM +0200, Andrew Lunn wrote:
> 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.
> 
> v4:
> checkpatch cleanup
> ethtool --cable-test dev
> Place breakout into cable_test_context
> Remove Pair: Pair output

Hello Andrew,

could you please test this update of netlink/desc-ethtool.c on top of
your series? The userspace messages look as expected but I'm not sure if
I have a device with cable test support available to test pretty
printing of kernel messages. (And even if I do, I almost certainly won't
have physical access to it.)

Thank you,
Michal

diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c
index 98b898e03255..54f057915d6d 100644
--- a/netlink/desc-ethtool.c
+++ b/netlink/desc-ethtool.c
@@ -206,6 +206,84 @@ static const struct pretty_nla_desc __tsinfo_desc[] = {
 	NLATTR_DESC_U32(ETHTOOL_A_TSINFO_PHC_INDEX),
 };
 
+static const struct pretty_nla_desc __cable_test_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_TEST_UNSPEC),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TEST_HEADER, header),
+};
+
+static const struct pretty_nla_desc __cable_test_result_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_RESULT_UNSPEC),
+	NLATTR_DESC_U8(ETHTOOL_A_CABLE_RESULT_PAIR),
+	NLATTR_DESC_U8(ETHTOOL_A_CABLE_RESULT_CODE),
+};
+
+static const struct pretty_nla_desc __cable_test_flength_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_FAULT_LENGTH_UNSPEC),
+	NLATTR_DESC_U8(ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR),
+	NLATTR_DESC_U32(ETHTOOL_A_CABLE_FAULT_LENGTH_CM),
+};
+
+static const struct pretty_nla_desc __cable_nest_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_NEST_UNSPEC),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_NEST_RESULT, cable_test_result),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_NEST_FAULT_LENGTH,
+			   cable_test_flength),
+};
+
+static const struct pretty_nla_desc __cable_test_ntf_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_TEST_NTF_UNSPEC),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TEST_NTF_HEADER, header),
+	NLATTR_DESC_U8(ETHTOOL_A_CABLE_TEST_NTF_STATUS),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TEST_NTF_NEST, cable_nest),
+};
+
+static const struct pretty_nla_desc __cable_test_tdr_cfg_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_TEST_TDR_CFG_UNSPEC),
+	NLATTR_DESC_U32(ETHTOOL_A_CABLE_TEST_TDR_CFG_FIRST),
+	NLATTR_DESC_U32(ETHTOOL_A_CABLE_TEST_TDR_CFG_LAST),
+	NLATTR_DESC_U32(ETHTOOL_A_CABLE_TEST_TDR_CFG_STEP),
+	NLATTR_DESC_U8(ETHTOOL_A_CABLE_TEST_TDR_CFG_PAIR),
+};
+
+static const struct pretty_nla_desc __cable_test_tdr_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_TEST_TDR_UNSPEC),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TEST_TDR_HEADER, header),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TEST_TDR_CFG, cable_test_tdr_cfg),
+};
+
+static const struct pretty_nla_desc __cable_step_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_STEP_UNSPEC),
+	NLATTR_DESC_U32(ETHTOOL_A_CABLE_STEP_FIRST_DISTANCE),
+	NLATTR_DESC_U32(ETHTOOL_A_CABLE_STEP_LAST_DISTANCE),
+	NLATTR_DESC_U32(ETHTOOL_A_CABLE_STEP_STEP_DISTANCE),
+};
+
+static const struct pretty_nla_desc __cable_amplitude_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_AMPLITUDE_UNSPEC),
+	NLATTR_DESC_U8(ETHTOOL_A_CABLE_AMPLITUDE_PAIR),
+	NLATTR_DESC_S16(ETHTOOL_A_CABLE_AMPLITUDE_mV),
+};
+
+static const struct pretty_nla_desc __cable_pulse_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_PULSE_UNSPEC),
+	NLATTR_DESC_S16(ETHTOOL_A_CABLE_PULSE_mV),
+};
+
+static const struct pretty_nla_desc __cable_test_tdr_nest_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_TDR_NEST_UNSPEC),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TDR_NEST_STEP, cable_step),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE, cable_amplitude),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TDR_NEST_PULSE, cable_pulse),
+};
+
+static const struct pretty_nla_desc __cable_test_tdr_ntf_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_CABLE_TEST_TDR_UNSPEC),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TEST_TDR_NTF_HEADER, header),
+	NLATTR_DESC_U8(ETHTOOL_A_CABLE_TEST_TDR_NTF_STATUS),
+	NLATTR_DESC_NESTED(ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST,
+			   cable_test_tdr_nest),
+};
+
 const struct pretty_nlmsg_desc ethnl_umsg_desc[] = {
 	NLMSG_DESC_INVALID(ETHTOOL_MSG_USER_NONE),
 	NLMSG_DESC(ETHTOOL_MSG_STRSET_GET, strset),
@@ -233,6 +311,8 @@ const struct pretty_nlmsg_desc ethnl_umsg_desc[] = {
 	NLMSG_DESC(ETHTOOL_MSG_EEE_GET, eee),
 	NLMSG_DESC(ETHTOOL_MSG_EEE_SET, eee),
 	NLMSG_DESC(ETHTOOL_MSG_TSINFO_GET, tsinfo),
+	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_ACT, cable_test),
+	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_TDR_ACT, cable_test_tdr),
 };
 
 const unsigned int ethnl_umsg_n_desc = ARRAY_SIZE(ethnl_umsg_desc);
@@ -265,6 +345,8 @@ const struct pretty_nlmsg_desc ethnl_kmsg_desc[] = {
 	NLMSG_DESC(ETHTOOL_MSG_EEE_GET_REPLY, eee),
 	NLMSG_DESC(ETHTOOL_MSG_EEE_NTF, eee),
 	NLMSG_DESC(ETHTOOL_MSG_TSINFO_GET_REPLY, tsinfo),
+	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_NTF, cable_test_ntf),
+	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_TDR_NTF, cable_test_tdr_ntf),
 };
 
 const unsigned int ethnl_kmsg_n_desc = ARRAY_SIZE(ethnl_kmsg_desc);

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH ethtool v4 0/6] ethtool(1) cable test support
  2020-07-05  0:44 ` [PATCH ethtool v4 0/6] ethtool(1) cable test support Michal Kubecek
@ 2020-07-05 16:24   ` Andrew Lunn
  2020-07-05 23:26     ` Michal Kubecek
  2020-07-05 18:25   ` Andrew Lunn
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2020-07-05 16:24 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy

On Sun, Jul 05, 2020 at 02:44:47AM +0200, Michal Kubecek wrote:
> On Wed, Jul 01, 2020 at 03:07:37AM +0200, Andrew Lunn wrote:
> > 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.
> > 
> > v4:
> > checkpatch cleanup
> > ethtool --cable-test dev
> > Place breakout into cable_test_context
> > Remove Pair: Pair output
> 
> Hello Andrew,
> 
> could you please test this update of netlink/desc-ethtool.c on top of
> your series? The userspace messages look as expected but I'm not sure if
> I have a device with cable test support available to test pretty
> printing of kernel messages. (And even if I do, I almost certainly won't
> have physical access to it.)

Hi Michal

Currently there are three PHY drivers with support: Marvell, Atheros
at803x, and bcm54140. And you can do some amount of testing without
physical access, you can expect the test results to indicate the cable
is O.K.

However, i will give these a go.

Some sort of capture and reply would be interesting for this, and for
regression testing. The ability to do something like

ethtool --monitor -w test.cap

To dump the netlink socket data to a file,  and

ethtool --monitor -r test.cap

to read from the file and decode its contents. Maybe this is already
possible via nlmon?

	 Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH ethtool v4 0/6] ethtool(1) cable test support
  2020-07-05 16:24   ` Andrew Lunn
@ 2020-07-05 23:26     ` Michal Kubecek
  0 siblings, 0 replies; 14+ messages in thread
From: Michal Kubecek @ 2020-07-05 23:26 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Chris Healy

On Sun, Jul 05, 2020 at 06:24:21PM +0200, Andrew Lunn wrote:
> On Sun, Jul 05, 2020 at 02:44:47AM +0200, Michal Kubecek wrote:
> > On Wed, Jul 01, 2020 at 03:07:37AM +0200, Andrew Lunn wrote:
> > > 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.
> > > 
> > > v4:
> > > checkpatch cleanup
> > > ethtool --cable-test dev
> > > Place breakout into cable_test_context
> > > Remove Pair: Pair output
> > 
> > Hello Andrew,
> > 
> > could you please test this update of netlink/desc-ethtool.c on top of
> > your series? The userspace messages look as expected but I'm not sure if
> > I have a device with cable test support available to test pretty
> > printing of kernel messages. (And even if I do, I almost certainly won't
> > have physical access to it.)
> 
> Hi Michal
> 
> Currently there are three PHY drivers with support: Marvell, Atheros
> at803x, and bcm54140. And you can do some amount of testing without
> physical access, you can expect the test results to indicate the cable
> is O.K.
> 
> However, i will give these a go.
> 
> Some sort of capture and reply would be interesting for this, and for
> regression testing. The ability to do something like
> 
> ethtool --monitor -w test.cap
> 
> To dump the netlink socket data to a file,  and
> 
> ethtool --monitor -r test.cap
> 
> to read from the file and decode its contents. Maybe this is already
> possible via nlmon?

Such feature would definitely be useful. It should be possible to
capture the data using nlmon and tcpdump but whenever I tried to use
nlmon to capture netlink data, I found it rather difficult to capture
only the traffic I was interested in. For the second part (feeding the
captured data to ethtool), we would probably need some tool which would
work like tcpreplay. Having support in ethtool makes very good sense and
it shouldn't require too much extra code, perhaps a dependency on
libpcap.

In the long term, I would like to write a packetdrill-like framework for
selftests where testcases would be scripts consisting of statements
saying e.g. "run with this command line", "expect this netlink message",
"reply with this netlink message", "expect this output" etc. But that
would be more complex and ability to capture into a file and replay it
would b useful independently of that.

Michal

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH ethtool v4 0/6] ethtool(1) cable test support
  2020-07-05  0:44 ` [PATCH ethtool v4 0/6] ethtool(1) cable test support Michal Kubecek
  2020-07-05 16:24   ` Andrew Lunn
@ 2020-07-05 18:25   ` Andrew Lunn
  2020-07-05 23:27     ` Michal Kubecek
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2020-07-05 18:25 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Chris Healy

> Hello Andrew,
> 
> could you please test this update of netlink/desc-ethtool.c on top of
> your series? The userspace messages look as expected but I'm not sure if
> I have a device with cable test support available to test pretty
> printing of kernel messages. (And even if I do, I almost certainly won't
> have physical access to it.)

Tested-by: Andrew Lunn <andrew@lunn.ch>

/home/andrew/ethtool/ethtool --debug 0xff --cable-test-tdr lan2 first 10 last 12  
...
sending genetlink packet (56 bytes):
    msg length 56 ethool ETHTOOL_MSG_CABLE_TEST_TDR_ACT
    ETHTOOL_MSG_CABLE_TEST_TDR_ACT
        ETHTOOL_A_CABLE_TEST_TDR_HEADER
            ETHTOOL_A_HEADER_DEV_NAME = "lan2"
        ETHTOOL_A_CABLE_TEST_TDR_CFG
            ETHTOOL_A_CABLE_TEST_TDR_CFG_FIRST = 1000
            ETHTOOL_A_CABLE_TEST_TDR_CFG_LAST = 1200
----------------	------------------
|  0000000056  |	| message length |
| 00020 | R-A- |	|  type | flags  |
|  0000000002  |	| sequence number|
|  0000000000  |	|     port ID    |
----------------	------------------
| 1b 01 00 00  |	|  extra header  |
|00016|N-|00001|	|len |flags| type|
|00009|--|00002|	|len |flags| type|
| 6c 61 6e 32  |	|      data      |	 l a n 2
| 00 00 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00008|--|00001|	|len |flags| type|
| e8 03 00 00  |	|      data      |	        
|00008|--|00002|	|len |flags| type|
| b0 04 00 00  |	|      data      |	        
----------------	------------------
received genetlink packet (52 bytes):
    msg length 52 ethool ETHTOOL_MSG_CABLE_TEST_TDR_NTF
    ETHTOOL_MSG_CABLE_TEST_TDR_NTF
        ETHTOOL_A_CABLE_TEST_TDR_NTF_HEADER
            ETHTOOL_A_HEADER_DEV_INDEX = 7
            ETHTOOL_A_HEADER_DEV_NAME = "lan2"
        ETHTOOL_A_CABLE_TEST_TDR_NTF_STATUS = 1
----------------	------------------
|  0000000052  |	| message length |
| 00020 | ---- |	|  type | flags  |
|  0000000036  |	| sequence number|
|  0000000000  |	|     port ID    |
----------------	------------------
| 1c 01 00 00  |	|  extra header  |
|00024|N-|00001|	|len |flags| type|
|00008|--|00001|	|len |flags| type|
| 07 00 00 00  |	|      data      |	        
|00009|--|00002|	|len |flags| type|
| 6c 61 6e 32  |	|      data      |	 l a n 2
| 00 00 00 00  |	|      data      |	        
|00005|--|00002|	|len |flags| type|
| 01 00 00 00  |	|      data      |	        
----------------	------------------
Cable test TDR started for device lan2.
received genetlink packet (36 bytes):
    msg length 36 error errno=0
----------------	------------------
|  0000000036  |	| message length |
| 00002 | ---- |	|  type | flags  |
|  0000000002  |	| sequence number|
|  0000006959  |	|     port ID    |
----------------	------------------
| 00 00 00 00  |	|                |
| 38 00 00 00  |	|                |
| 14 00 05 00  |	|                |
| 02 00 00 00  |	|                |
| 00 00 00 00  |	|                |
----------------	------------------
received genetlink packet (336 bytes):
    msg length 336 ethool ETHTOOL_MSG_CABLE_TEST_TDR_NTF
    ETHTOOL_MSG_CABLE_TEST_TDR_NTF
        ETHTOOL_A_CABLE_TEST_TDR_NTF_HEADER
            ETHTOOL_A_HEADER_DEV_INDEX = 7
            ETHTOOL_A_HEADER_DEV_NAME = "lan2"
        ETHTOOL_A_CABLE_TEST_TDR_NTF_STATUS = 2
        ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST
            ETHTOOL_A_CABLE_TDR_NEST_PULSE
                ETHTOOL_A_CABLE_PULSE_mV = 1000
            ETHTOOL_A_CABLE_TDR_NEST_STEP
                ETHTOOL_A_CABLE_STEP_FIRST_DISTANCE = 966
                ETHTOOL_A_CABLE_STEP_LAST_DISTANCE = 1127
                ETHTOOL_A_CABLE_STEP_STEP_DISTANCE = 80
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 0
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 46
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 1
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 117
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 2
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 85
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 3
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 93
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 0
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 7
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 1
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 15
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 2
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 23
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 3
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 31
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 0
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 148
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 1
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 156
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 2
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 320
            ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE
                ETHTOOL_A_CABLE_AMPLITUDE_PAIR = 3
                ETHTOOL_A_CABLE_AMPLITUDE_mV = 39
----------------	------------------
|  0000000336  |	| message length |
| 00020 | ---- |	|  type | flags  |
|  0000000035  |	| sequence number|
|  0000000000  |	|     port ID    |
----------------	------------------
| 1c 01 00 00  |	|  extra header  |
|00024|N-|00001|	|len |flags| type|
|00008|--|00001|	|len |flags| type|
| 07 00 00 00  |	|      data      |	        
|00009|--|00002|	|len |flags| type|
| 6c 61 6e 32  |	|      data      |	 l a n 2
| 00 00 00 00  |	|      data      |	        
|00005|--|00002|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00284|N-|00003|	|len |flags| type|
|00012|N-|00003|	|len |flags| type|
|00006|--|00001|	|len |flags| type|
| e8 03 00 00  |	|      data      |	        
|00028|N-|00001|	|len |flags| type|
|00008|--|00001|	|len |flags| type|
| c6 03 00 00  |	|      data      |	        
|00008|--|00002|	|len |flags| type|
| 67 04 00 00  |	|      data      |	 g      
|00008|--|00003|	|len |flags| type|
| 50 00 00 00  |	|      data      |	 P      
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 00 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 2e 00 00 00  |	|      data      |	 .      
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 01 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 75 00 00 00  |	|      data      |	 u      
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 55 00 00 00  |	|      data      |	 U      
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 03 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 5d 00 00 00  |	|      data      |	 ]      
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 00 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 07 00 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 01 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 0f 00 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 17 00 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 03 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 1f 00 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 00 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 94 00 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 01 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 9c 00 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 40 01 00 00  |	|      data      |	 @      
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 03 00 00 00  |	|      data      |	        
|00006|--|00002|	|len |flags| type|
| 27 00 00 00  |	|      data      |	 '      
----------------	------------------
Cable test TDR completed for device lan2.
TDR Pulse 1000mV
Step configuration: 9.66-11.27 meters in 0.80m steps
Pair A Amplitude   46
Pair B Amplitude  117
Pair C Amplitude   85
Pair D Amplitude   93
Pair A Amplitude    7
Pair B Amplitude   15
Pair C Amplitude   23
Pair D Amplitude   31
Pair A Amplitude  148
Pair B Amplitude  156
Pair C Amplitude  320
Pair D Amplitude   39


/home/andrew/ethtool/ethtool --debug 0xff --cable-test lan2
...
sending genetlink packet (36 bytes):
    msg length 36 ethool ETHTOOL_MSG_CABLE_TEST_ACT
    ETHTOOL_MSG_CABLE_TEST_ACT
        ETHTOOL_A_CABLE_TEST_HEADER
            ETHTOOL_A_HEADER_DEV_NAME = "lan2"
----------------	------------------
|  0000000036  |	| message length |
| 00020 | R-A- |	|  type | flags  |
|  0000000002  |	| sequence number|
|  0000000000  |	|     port ID    |
----------------	------------------
| 1a 01 00 00  |	|  extra header  |
|00016|N-|00001|	|len |flags| type|
|00009|--|00002|	|len |flags| type|
| 6c 61 6e 32  |	|      data      |	 l a n 2
| 00 00 00 00  |	|      data      |	        
----------------	------------------
received genetlink packet (52 bytes):
    msg length 52 ethool ETHTOOL_MSG_CABLE_TEST_NTF
    ETHTOOL_MSG_CABLE_TEST_NTF
        ETHTOOL_A_CABLE_TEST_NTF_HEADER
            ETHTOOL_A_HEADER_DEV_INDEX = 7
            ETHTOOL_A_HEADER_DEV_NAME = "lan2"
        ETHTOOL_A_CABLE_TEST_NTF_STATUS = 1
----------------	------------------
|  0000000052  |	| message length |
| 00020 | ---- |	|  type | flags  |
|  0000000034  |	| sequence number|
|  0000000000  |	|     port ID    |
----------------	------------------
| 1b 01 00 00  |	|  extra header  |
|00024|N-|00001|	|len |flags| type|
|00008|--|00001|	|len |flags| type|
| 07 00 00 00  |	|      data      |	        
|00009|--|00002|	|len |flags| type|
| 6c 61 6e 32  |	|      data      |	 l a n 2
| 00 00 00 00  |	|      data      |	        
|00005|--|00002|	|len |flags| type|
| 01 00 00 00  |	|      data      |	        
----------------	------------------
Cable test started for device lan2.
received genetlink packet (36 bytes):
    msg length 36 error errno=0
----------------	------------------
|  0000000036  |	| message length |
| 00002 | ---- |	|  type | flags  |
|  0000000002  |	| sequence number|
|  0000006949  |	|     port ID    |
----------------	------------------
| 00 00 00 00  |	|                |
| 24 00 00 00  |	|                |
| 14 00 05 00  |	|                |
| 02 00 00 00  |	|                |
| 00 00 00 00  |	|                |
----------------	------------------
received genetlink packet (216 bytes):
    msg length 216 ethool ETHTOOL_MSG_CABLE_TEST_NTF
    ETHTOOL_MSG_CABLE_TEST_NTF
        ETHTOOL_A_CABLE_TEST_NTF_HEADER
            ETHTOOL_A_HEADER_DEV_INDEX = 7
            ETHTOOL_A_HEADER_DEV_NAME = "lan2"
        ETHTOOL_A_CABLE_TEST_NTF_STATUS = 2
        ETHTOOL_A_CABLE_TEST_NTF_NEST
            ETHTOOL_A_CABLE_NEST_RESULT
                ETHTOOL_A_CABLE_RESULT_PAIR = 0
                ETHTOOL_A_CABLE_RESULT_CODE = 2
            ETHTOOL_A_CABLE_NEST_RESULT
                ETHTOOL_A_CABLE_RESULT_PAIR = 1
                ETHTOOL_A_CABLE_RESULT_CODE = 2
            ETHTOOL_A_CABLE_NEST_RESULT
                ETHTOOL_A_CABLE_RESULT_PAIR = 2
                ETHTOOL_A_CABLE_RESULT_CODE = 2
            ETHTOOL_A_CABLE_NEST_RESULT
                ETHTOOL_A_CABLE_RESULT_PAIR = 3
                ETHTOOL_A_CABLE_RESULT_CODE = 2
            ETHTOOL_A_CABLE_NEST_FAULT_LENGTH
                ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR = 0
                ETHTOOL_A_CABLE_FAULT_LENGTH_CM = 2400
            ETHTOOL_A_CABLE_NEST_FAULT_LENGTH
                ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR = 1
                ETHTOOL_A_CABLE_FAULT_LENGTH_CM = 2480
            ETHTOOL_A_CABLE_NEST_FAULT_LENGTH
                ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR = 2
                ETHTOOL_A_CABLE_FAULT_LENGTH_CM = 2400
            ETHTOOL_A_CABLE_NEST_FAULT_LENGTH
                ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR = 3
                ETHTOOL_A_CABLE_FAULT_LENGTH_CM = 2400
----------------	------------------
|  0000000216  |	| message length |
| 00020 | ---- |	|  type | flags  |
|  0000000033  |	| sequence number|
|  0000000000  |	|     port ID    |
----------------	------------------
| 1b 01 00 00  |	|  extra header  |
|00024|N-|00001|	|len |flags| type|
|00008|--|00001|	|len |flags| type|
| 07 00 00 00  |	|      data      |	        
|00009|--|00002|	|len |flags| type|
| 6c 61 6e 32  |	|      data      |	 l a n 2
| 00 00 00 00  |	|      data      |	        
|00005|--|00002|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00164|N-|00003|	|len |flags| type|
|00020|N-|00001|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 00 00 00 00  |	|      data      |	        
|00005|--|00002|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00020|N-|00001|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 01 00 00 00  |	|      data      |	        
|00005|--|00002|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00020|N-|00001|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00005|--|00002|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00020|N-|00001|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 03 00 00 00  |	|      data      |	        
|00005|--|00002|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 00 00 00 00  |	|      data      |	        
|00008|--|00002|	|len |flags| type|
| 60 09 00 00  |	|      data      |	 `      
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 01 00 00 00  |	|      data      |	        
|00008|--|00002|	|len |flags| type|
| b0 09 00 00  |	|      data      |	        
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 02 00 00 00  |	|      data      |	        
|00008|--|00002|	|len |flags| type|
| 60 09 00 00  |	|      data      |	 `      
|00020|N-|00002|	|len |flags| type|
|00005|--|00001|	|len |flags| type|
| 03 00 00 00  |	|      data      |	        
|00008|--|00002|	|len |flags| type|
| 60 09 00 00  |	|      data      |	 `      
----------------	------------------
Cable test completed for device lan2.
Pair A code Open Circuit
Pair B code Open Circuit
Pair C code Open Circuit
Pair D code Open Circuit
Pair A, fault length: 24.00m
Pair B, fault length: 24.80m
Pair C, fault length: 24.00m
Pair D, fault length: 24.00m

    Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH ethtool v4 0/6] ethtool(1) cable test support
  2020-07-05 18:25   ` Andrew Lunn
@ 2020-07-05 23:27     ` Michal Kubecek
  0 siblings, 0 replies; 14+ messages in thread
From: Michal Kubecek @ 2020-07-05 23:27 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Chris Healy

On Sun, Jul 05, 2020 at 08:25:33PM +0200, Andrew Lunn wrote:
> > Hello Andrew,
> > 
> > could you please test this update of netlink/desc-ethtool.c on top of
> > your series? The userspace messages look as expected but I'm not sure if
> > I have a device with cable test support available to test pretty
> > printing of kernel messages. (And even if I do, I almost certainly won't
> > have physical access to it.)
> 
> Tested-by: Andrew Lunn <andrew@lunn.ch>

Thank you,
Michal

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-07-05 23:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-01  1:07 [PATCH ethtool v4 0/6] ethtool(1) cable test support Andrew Lunn
2020-07-01  1:07 ` [PATCH ethtool v4 1/6] Add " Andrew Lunn
2020-07-04 23:43   ` Michal Kubecek
2020-07-01  1:07 ` [PATCH ethtool v4 2/6] Add cable test TDR support Andrew Lunn
2020-07-04 23:58   ` Michal Kubecek
2020-07-01  1:07 ` [PATCH ethtool v4 3/6] json_writer/json_print: Import the iproute2 helper code for JSON output Andrew Lunn
2020-07-01  1:07 ` [PATCH ethtool v4 4/6] Add --json command line argument parsing Andrew Lunn
2020-07-01  1:07 ` [PATCH ethtool v4 5/6] ethtool.8.in: Document the cable test commands Andrew Lunn
2020-07-01  1:07 ` [PATCH ethtool v4 6/6] ethtool.8.in: Add --json option Andrew Lunn
2020-07-05  0:44 ` [PATCH ethtool v4 0/6] ethtool(1) cable test support Michal Kubecek
2020-07-05 16:24   ` Andrew Lunn
2020-07-05 23:26     ` Michal Kubecek
2020-07-05 18:25   ` Andrew Lunn
2020-07-05 23:27     ` Michal Kubecek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).