[PATCH ethtool v3 0/7] pause frame stats

[PATCH ethtool v3 0/7] pause frame stats

[Jakub Kicinski]

Hi!

Sorry about the delay from v2.


This set adds support for pause frame statistics.

First pause frame info is extended to support --json.

Pause stats are first of this kind of statistics so add
support for a new flag (--include-statistics).

Next add support for dumping policy to check if the statistics
flag is supported for a given subcommand.

Last but not least - display statistics.

v3:
 - rename the ctx variable to policy_ctx
 - instead of returning the flags policy save it to a member
   of struct nl_context, for potential reuse. Also we don't
   have to worry about returning flags and negative errors
   from the read helper this way :)

Jakub Kicinski (7):
  update UAPI header copies
  pause: add --json support
  separate FLAGS out in -h
  add support for stats in subcommands
  netlink: prepare for more per-op info
  netlink: use policy dumping to check if stats flag is supported
  pause: add support for dumping statistics

 ethtool.8.in           |   7 ++
 ethtool.c              |  17 +++-
 internal.h             |   1 +
 netlink/coalesce.c     |   6 +-
 netlink/msgbuff.h      |   6 ++
 netlink/netlink.c      | 179 ++++++++++++++++++++++++++++++++++++++---
 netlink/netlink.h      |  31 +++++--
 netlink/pause.c        | 111 ++++++++++++++++++++++---
 uapi/linux/genetlink.h |  11 +++
 uapi/linux/netlink.h   |   4 +
 10 files changed, 336 insertions(+), 37 deletions(-)

-- 
2.26.2

[PATCH ethtool v3 1/7] update UAPI header copies

[Jakub Kicinski]

Update to kernel commit 9453b2d4694c.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 uapi/linux/genetlink.h | 11 +++++++++++
 uapi/linux/netlink.h   |  4 ++++
 2 files changed, 15 insertions(+)

diff --git a/uapi/linux/genetlink.h b/uapi/linux/genetlink.h
index 7c6c390c48ee..9fa720ee87ae 100644
--- a/uapi/linux/genetlink.h
+++ b/uapi/linux/genetlink.h
@@ -64,6 +64,8 @@ enum {
 	CTRL_ATTR_OPS,
 	CTRL_ATTR_MCAST_GROUPS,
 	CTRL_ATTR_POLICY,
+	CTRL_ATTR_OP_POLICY,
+	CTRL_ATTR_OP,
 	__CTRL_ATTR_MAX,
 };
 
@@ -85,6 +87,15 @@ enum {
 	__CTRL_ATTR_MCAST_GRP_MAX,
 };
 
+enum {
+	CTRL_ATTR_POLICY_UNSPEC,
+	CTRL_ATTR_POLICY_DO,
+	CTRL_ATTR_POLICY_DUMP,
+
+	__CTRL_ATTR_POLICY_DUMP_MAX,
+	CTRL_ATTR_POLICY_DUMP_MAX = __CTRL_ATTR_POLICY_DUMP_MAX - 1
+};
+
 #define CTRL_ATTR_MCAST_GRP_MAX (__CTRL_ATTR_MCAST_GRP_MAX - 1)
 
 
diff --git a/uapi/linux/netlink.h b/uapi/linux/netlink.h
index 695c88e3c29d..dfef006be9f9 100644
--- a/uapi/linux/netlink.h
+++ b/uapi/linux/netlink.h
@@ -129,6 +129,7 @@ struct nlmsgerr {
  * @NLMSGERR_ATTR_COOKIE: arbitrary subsystem specific cookie to
  *	be used - in the success case - to identify a created
  *	object or operation or similar (binary)
+ * @NLMSGERR_ATTR_POLICY: policy for a rejected attribute
  * @__NLMSGERR_ATTR_MAX: number of attributes
  * @NLMSGERR_ATTR_MAX: highest attribute number
  */
@@ -137,6 +138,7 @@ enum nlmsgerr_attrs {
 	NLMSGERR_ATTR_MSG,
 	NLMSGERR_ATTR_OFFS,
 	NLMSGERR_ATTR_COOKIE,
+	NLMSGERR_ATTR_POLICY,
 
 	__NLMSGERR_ATTR_MAX,
 	NLMSGERR_ATTR_MAX = __NLMSGERR_ATTR_MAX - 1
@@ -327,6 +329,7 @@ enum netlink_attribute_type {
  *	the index, if limited inside the nesting (U32)
  * @NL_POLICY_TYPE_ATTR_BITFIELD32_MASK: valid mask for the
  *	bitfield32 type (U32)
+ * @NL_POLICY_TYPE_ATTR_MASK: mask of valid bits for unsigned integers (U64)
  * @NL_POLICY_TYPE_ATTR_PAD: pad attribute for 64-bit alignment
  */
 enum netlink_policy_type_attr {
@@ -342,6 +345,7 @@ enum netlink_policy_type_attr {
 	NL_POLICY_TYPE_ATTR_POLICY_MAXTYPE,
 	NL_POLICY_TYPE_ATTR_BITFIELD32_MASK,
 	NL_POLICY_TYPE_ATTR_PAD,
+	NL_POLICY_TYPE_ATTR_MASK,
 
 	/* keep last */
 	__NL_POLICY_TYPE_ATTR_MAX,
-- 
2.26.2

[PATCH ethtool v3 2/7] pause: add --json support

[Jakub Kicinski]

No change in normal text output:

 # ./ethtool  -a eth0
Pause parameters for eth0:
Autonegotiate:	on
RX:		on
TX:		on
RX negotiated: on
TX negotiated: on

JSON:

 # ./ethtool --json -a eth0
[ {
        "ifname": "eth0",
        "autonegotiate": true,
        "rx": true,
        "tx": true,
        "negotiated": {
            "rx": true,
            "tx": true
        }
    } ]

v2:
 - restructure show_bool() so we can use its logic for show_bool_val()

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 netlink/coalesce.c |  6 +++---
 netlink/netlink.h  | 21 ++++++++++++++++-----
 netlink/pause.c    | 44 ++++++++++++++++++++++++++++++++------------
 3 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/netlink/coalesce.c b/netlink/coalesce.c
index 65f75cf9a8dd..07a92d04b7a1 100644
--- a/netlink/coalesce.c
+++ b/netlink/coalesce.c
@@ -36,9 +36,9 @@ int coalesce_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 	if (silent)
 		putchar('\n');
 	printf("Coalesce parameters for %s:\n", nlctx->devname);
-	printf("Adaptive RX: %s  TX: %s\n",
-	       u8_to_bool(tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_RX]),
-	       u8_to_bool(tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_TX]));
+	show_bool("rx", "Adaptive RX: %s  ",
+		  tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_RX]);
+	show_bool("tx", "TX: %s\n", tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_TX]);
 	show_u32(tb[ETHTOOL_A_COALESCE_STATS_BLOCK_USECS],
 		 "stats-block-usecs: ");
 	show_u32(tb[ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL],
diff --git a/netlink/netlink.h b/netlink/netlink.h
index dd4a02bcc916..1012e8e32cd8 100644
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -98,17 +98,28 @@ static inline void show_u32(const struct nlattr *attr, const char *label)
 		printf("%sn/a\n", label);
 }
 
-static inline const char *u8_to_bool(const struct nlattr *attr)
+static inline const char *u8_to_bool(const uint8_t *val)
 {
-	if (attr)
-		return mnl_attr_get_u8(attr) ? "on" : "off";
+	if (val)
+		return *val ? "on" : "off";
 	else
 		return "n/a";
 }
 
-static inline void show_bool(const struct nlattr *attr, const char *label)
+static inline void show_bool_val(const char *key, const char *fmt, uint8_t *val)
+{
+	if (is_json_context()) {
+		if (val)
+			print_bool(PRINT_JSON, key, NULL, val);
+	} else {
+		print_string(PRINT_FP, NULL, fmt, u8_to_bool(val));
+	}
+}
+
+static inline void show_bool(const char *key, const char *fmt,
+			     const struct nlattr *attr)
 {
-	printf("%s%s\n", label, u8_to_bool(attr));
+	show_bool_val(key, fmt, attr ? mnl_attr_get_payload(attr) : NULL);
 }
 
 /* misc */
diff --git a/netlink/pause.c b/netlink/pause.c
index 7b6b3a1d2c10..c960c82cba5f 100644
--- a/netlink/pause.c
+++ b/netlink/pause.c
@@ -40,8 +40,8 @@ static int pause_autoneg_cb(const struct nlmsghdr *nlhdr, void *data)
 	struct pause_autoneg_status ours = {};
 	struct pause_autoneg_status peer = {};
 	struct nl_context *nlctx = data;
-	bool rx_status = false;
-	bool tx_status = false;
+	uint8_t rx_status = false;
+	uint8_t tx_status = false;
 	bool silent;
 	int err_ret;
 	int ret;
@@ -72,8 +72,11 @@ static int pause_autoneg_cb(const struct nlmsghdr *nlhdr, void *data)
 		else if (peer.pause)
 			tx_status = true;
 	}
-	printf("RX negotiated: %s\nTX negotiated: %s\n",
-	       rx_status ? "on" : "off", tx_status ? "on" : "off");
+
+	open_json_object("negotiated");
+	show_bool_val("rx", "RX negotiated: %s\n", &rx_status);
+	show_bool_val("tx", "TX negotiated: %s\n", &tx_status);
+	close_json_object();
 
 	return MNL_CB_OK;
 }
@@ -121,21 +124,34 @@ int pause_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 		return err_ret;
 
 	if (silent)
-		putchar('\n');
-	printf("Pause parameters for %s:\n", nlctx->devname);
-	show_bool(tb[ETHTOOL_A_PAUSE_AUTONEG], "Autonegotiate:\t");
-	show_bool(tb[ETHTOOL_A_PAUSE_RX], "RX:\t\t");
-	show_bool(tb[ETHTOOL_A_PAUSE_TX], "TX:\t\t");
+		print_nl();
+
+	open_json_object(NULL);
+
+	print_string(PRINT_ANY, "ifname", "Pause parameters for %s:\n",
+		     nlctx->devname);
+
+	show_bool("autonegotiate", "Autonegotiate:\t%s\n",
+		  tb[ETHTOOL_A_PAUSE_AUTONEG]);
+	show_bool("rx", "RX:\t\t%s\n", tb[ETHTOOL_A_PAUSE_RX]);
+	show_bool("tx", "TX:\t\t%s\n", tb[ETHTOOL_A_PAUSE_TX]);
+
 	if (!nlctx->is_monitor && tb[ETHTOOL_A_PAUSE_AUTONEG] &&
 	    mnl_attr_get_u8(tb[ETHTOOL_A_PAUSE_AUTONEG])) {
 		ret = show_pause_autoneg_status(nlctx);
 		if (ret < 0)
-			return err_ret;
+			goto err_close_dev;
 	}
 	if (!silent)
-		putchar('\n');
+		print_nl();
+
+	close_json_object();
 
 	return MNL_CB_OK;
+
+err_close_dev:
+	close_json_object();
+	return err_ret;
 }
 
 int nl_gpause(struct cmd_context *ctx)
@@ -156,7 +172,11 @@ int nl_gpause(struct cmd_context *ctx)
 				      ETHTOOL_A_PAUSE_HEADER, 0);
 	if (ret < 0)
 		return ret;
-	return nlsock_send_get_request(nlsk, pause_reply_cb);
+
+	new_json_obj(ctx->json);
+	ret = nlsock_send_get_request(nlsk, pause_reply_cb);
+	delete_json_obj();
+	return ret;
 }
 
 /* PAUSE_SET */
-- 
2.26.2

[PATCH ethtool v3 3/7] separate FLAGS out in -h

[Jakub Kicinski]

Help output is quite crowded already with every command
being prefixed by --debug and --json options, and we're
about to add a third one.

Add an indirection.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 ethtool.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 2a7de97c51bb..403616bb7fa0 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6015,10 +6015,10 @@ static int show_usage(struct cmd_context *ctx __maybe_unused)
 	fprintf(stdout, PACKAGE " version " VERSION "\n");
 	fprintf(stdout,
 		"Usage:\n"
-		"        ethtool [ --debug MASK ][ --json ] DEVNAME\t"
+		"        ethtool [ FLAGS ] DEVNAME\t"
 		"Display standard information about device\n");
 	for (i = 0; args[i].opts; i++) {
-		fputs("        ethtool [ --debug MASK ][ --json ] ", stdout);
+		fputs("        ethtool [ FLAGS ] ", stdout);
 		fprintf(stdout, "%s %s\t%s\n",
 			args[i].opts,
 			args[i].no_dev ? "\t" : "DEVNAME",
@@ -6027,7 +6027,10 @@ 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");
+	fprintf(stdout, "\n");
+	fprintf(stdout, "FLAGS:\n");
+	fprintf(stdout, "	--debug MASK	turn on debugging messages\n");
+	fprintf(stdout, "	--json		enable JSON output format (not supported by all commands)\n");
 
 	return 0;
 }
-- 
2.26.2

[PATCH ethtool v3 4/7] add support for stats in subcommands

[Jakub Kicinski]

Add new parameters (-I | --include-statistics) which will
control requesting statistic dumps from the kernel.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 ethtool.8.in | 7 +++++++
 ethtool.c    | 8 ++++++++
 internal.h   | 1 +
 3 files changed, 16 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9beb1e5791eb..429c75f3f682 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -140,6 +140,9 @@ ethtool \- query or control network driver and hardware settings
 .B ethtool [--json]
 .I args
 .HP
+.B ethtool [-I | --include-statistics]
+.I args
+.HP
 .B ethtool \-\-monitor
 [
 .I command
@@ -499,6 +502,10 @@ 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 \-I \-\-include\-statistics
+Include command-related statistics in the output. This option allows
+displaying relevant device statistics for selected get commands.
+.TP
 .B \-a \-\-show\-pause
 Queries the specified Ethernet device for pause parameter information.
 .TP
diff --git a/ethtool.c b/ethtool.c
index 403616bb7fa0..1d9067e774af 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6031,6 +6031,7 @@ static int show_usage(struct cmd_context *ctx __maybe_unused)
 	fprintf(stdout, "FLAGS:\n");
 	fprintf(stdout, "	--debug MASK	turn on debugging messages\n");
 	fprintf(stdout, "	--json		enable JSON output format (not supported by all commands)\n");
+	fprintf(stdout, "	-I|--include-statistics		request device statistics related to the command (not supported by all commands)\n");
 
 	return 0;
 }
@@ -6291,6 +6292,13 @@ int main(int argc, char **argp)
 			argc -= 1;
 			continue;
 		}
+		if (*argp && (!strcmp(*argp, "--include-statistics") ||
+			      !strcmp(*argp, "-I"))) {
+			ctx.show_stats = true;
+			argp += 1;
+			argc -= 1;
+			continue;
+		}
 		break;
 	}
 	if (*argp && !strcmp(*argp, "--monitor")) {
diff --git a/internal.h b/internal.h
index 935ebac3ca2e..27da8eac57bb 100644
--- a/internal.h
+++ b/internal.h
@@ -225,6 +225,7 @@ struct cmd_context {
 	char **argp;		/* arguments to the sub-command */
 	unsigned long debug;	/* debugging mask */
 	bool json;		/* Output JSON, if supported */
+	bool show_stats;	/* include command-specific stats */
 #ifdef ETHTOOL_ENABLE_NETLINK
 	struct nl_context *nlctx;	/* netlink context (opaque) */
 #endif
-- 
2.26.2

[PATCH ethtool v3 5/7] netlink: prepare for more per-op info

[Jakub Kicinski]

We stored an array of op flags, to check if operations are
supported. Make that array a structure rather than plain
uint32_t in preparation for storing more state.

v3: new patch

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 netlink/netlink.c | 25 +++++++++++++------------
 netlink/netlink.h |  6 +++++-
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/netlink/netlink.c b/netlink/netlink.c
index e42d57076a4b..86dc1efdf5ce 100644
--- a/netlink/netlink.c
+++ b/netlink/netlink.c
@@ -120,19 +120,19 @@ bool netlink_cmd_check(struct cmd_context *ctx, unsigned int cmd,
 		nlctx->wildcard_unsupported = true;
 		return true;
 	}
-	if (!nlctx->ops_flags) {
+	if (!nlctx->ops_info) {
 		nlctx->ioctl_fallback = true;
 		return false;
 	}
-	if (cmd > ETHTOOL_MSG_USER_MAX || !nlctx->ops_flags[cmd]) {
+	if (cmd > ETHTOOL_MSG_USER_MAX || !nlctx->ops_info[cmd].op_flags) {
 		nlctx->ioctl_fallback = true;
 		return true;
 	}
 
-	if (is_dump && !(nlctx->ops_flags[cmd] & GENL_CMD_CAP_DUMP))
+	if (is_dump && !(nlctx->ops_info[cmd].op_flags & GENL_CMD_CAP_DUMP))
 		nlctx->wildcard_unsupported = true;
 
-	return !(nlctx->ops_flags[cmd] & cap);
+	return !(nlctx->ops_info[cmd].op_flags & cap);
 }
 
 /* initialization */
@@ -140,12 +140,12 @@ bool netlink_cmd_check(struct cmd_context *ctx, unsigned int cmd,
 static int genl_read_ops(struct nl_context *nlctx,
 			 const struct nlattr *ops_attr)
 {
+	struct nl_op_info *ops_info;
 	struct nlattr *op_attr;
-	uint32_t *ops_flags;
 	int ret;
 
-	ops_flags = calloc(__ETHTOOL_MSG_USER_CNT, sizeof(ops_flags[0]));
-	if (!ops_flags)
+	ops_info = calloc(__ETHTOOL_MSG_USER_CNT, sizeof(ops_info[0]));
+	if (!ops_info)
 		return -ENOMEM;
 
 	mnl_attr_for_each_nested(op_attr, ops_attr) {
@@ -163,13 +163,14 @@ static int genl_read_ops(struct nl_context *nlctx,
 		if (op_id >= __ETHTOOL_MSG_USER_CNT)
 			continue;
 
-		ops_flags[op_id] = mnl_attr_get_u32(tb[CTRL_ATTR_OP_FLAGS]);
+		ops_info[op_id].op_flags =
+			mnl_attr_get_u32(tb[CTRL_ATTR_OP_FLAGS]);
 	}
 
-	nlctx->ops_flags = ops_flags;
+	nlctx->ops_info = ops_info;
 	return 0;
 err:
-	free(ops_flags);
+	free(ops_info);
 	return ret;
 }
 
@@ -273,7 +274,7 @@ int netlink_init(struct cmd_context *ctx)
 out_nlsk:
 	nlsock_done(nlctx->ethnl_socket);
 out_free:
-	free(nlctx->ops_flags);
+	free(nlctx->ops_info);
 	free(nlctx);
 	return ret;
 }
@@ -283,7 +284,7 @@ static void netlink_done(struct cmd_context *ctx)
 	if (!ctx->nlctx)
 		return;
 
-	free(ctx->nlctx->ops_flags);
+	free(ctx->nlctx->ops_info);
 	free(ctx->nlctx);
 	ctx->nlctx = NULL;
 	cleanup_all_strings();
diff --git a/netlink/netlink.h b/netlink/netlink.h
index 1012e8e32cd8..e79143016bd5 100644
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -25,6 +25,10 @@ enum link_mode_class {
 	LM_CLASS_FEC,
 };
 
+struct nl_op_info {
+	uint32_t		op_flags;
+};
+
 struct nl_context {
 	struct cmd_context	*ctx;
 	void			*cmd_private;
@@ -34,7 +38,7 @@ struct nl_context {
 	unsigned int		suppress_nlerr;
 	uint16_t		ethnl_fam;
 	uint32_t		ethnl_mongrp;
-	uint32_t		*ops_flags;
+	struct nl_op_info	*ops_info;
 	struct nl_socket	*ethnl_socket;
 	struct nl_socket	*ethnl2_socket;
 	struct nl_socket	*rtnl_socket;
-- 
2.26.2

[PATCH ethtool v3 6/7] netlink: use policy dumping to check if stats flag is supported

[Jakub Kicinski]

Older kernels don't support statistics, to avoid retries
make use of netlink policy dumps to figure out which
flags kernel actually supports.

v3:
 - s/ctx/policy_ctx/
 - save the flags in nl_context to be able to reuse them,
   and not have to return errors and values from the policy
   get function

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 netlink/msgbuff.h |   6 ++
 netlink/netlink.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++
 netlink/netlink.h |   4 ++
 3 files changed, 164 insertions(+)

diff --git a/netlink/msgbuff.h b/netlink/msgbuff.h
index 24b99c5a28d7..7d6731fc24a3 100644
--- a/netlink/msgbuff.h
+++ b/netlink/msgbuff.h
@@ -81,6 +81,12 @@ static inline bool ethnla_put_u32(struct nl_msg_buff *msgbuff, uint16_t type,
 	return ethnla_put(msgbuff, type, sizeof(uint32_t), &data);
 }
 
+static inline bool ethnla_put_u16(struct nl_msg_buff *msgbuff, uint16_t type,
+				  uint16_t data)
+{
+	return ethnla_put(msgbuff, type, sizeof(uint16_t), &data);
+}
+
 static inline bool ethnla_put_u8(struct nl_msg_buff *msgbuff, uint16_t type,
 				 uint8_t data)
 {
diff --git a/netlink/netlink.c b/netlink/netlink.c
index 86dc1efdf5ce..f655f6ea25b7 100644
--- a/netlink/netlink.c
+++ b/netlink/netlink.c
@@ -135,6 +135,160 @@ bool netlink_cmd_check(struct cmd_context *ctx, unsigned int cmd,
 	return !(nlctx->ops_info[cmd].op_flags & cap);
 }
 
+struct ethtool_op_policy_query_ctx {
+	struct nl_context *nlctx;
+	unsigned int op;
+	unsigned int op_hdr_attr;
+
+	bool op_policy_found;
+	bool hdr_policy_found;
+	unsigned int op_policy_idx;
+	unsigned int hdr_policy_idx;
+	uint64_t flag_mask;
+};
+
+static int family_policy_find_op(struct ethtool_op_policy_query_ctx *policy_ctx,
+				 const struct nlattr *op_policy)
+{
+	const struct nlattr *attr;
+	unsigned int type;
+	int ret;
+
+	type = policy_ctx->nlctx->is_dump ?
+		CTRL_ATTR_POLICY_DUMP : CTRL_ATTR_POLICY_DO;
+
+	mnl_attr_for_each_nested(attr, op_policy) {
+		const struct nlattr *tb[CTRL_ATTR_POLICY_DUMP_MAX + 1] = {};
+		DECLARE_ATTR_TB_INFO(tb);
+
+		if (mnl_attr_get_type(attr) != policy_ctx->op)
+			continue;
+
+		ret = mnl_attr_parse_nested(attr, attr_cb, &tb_info);
+		if (ret < 0)
+			return ret;
+
+		if (!tb[type])
+			continue;
+
+		policy_ctx->op_policy_found = true;
+		policy_ctx->op_policy_idx = mnl_attr_get_u32(tb[type]);
+		break;
+	}
+
+	return 0;
+}
+
+static int family_policy_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	const struct nlattr *tba[NL_POLICY_TYPE_ATTR_MAX + 1] = {};
+	DECLARE_ATTR_TB_INFO(tba);
+	const struct nlattr *tb[CTRL_ATTR_MAX + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	struct ethtool_op_policy_query_ctx *policy_ctx = data;
+	const struct nlattr *policy_attr, *attr_attr, *attr;
+	unsigned int attr_idx, policy_idx;
+	int ret;
+
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return MNL_CB_ERROR;
+
+	if (!policy_ctx->op_policy_found) {
+		if (!tb[CTRL_ATTR_OP_POLICY]) {
+			fprintf(stderr, "Error: op policy map not present\n");
+			return MNL_CB_ERROR;
+		}
+		ret = family_policy_find_op(policy_ctx, tb[CTRL_ATTR_OP_POLICY]);
+		return ret < 0 ? MNL_CB_ERROR : MNL_CB_OK;
+	}
+
+	if (!tb[CTRL_ATTR_POLICY])
+		return MNL_CB_OK;
+
+	policy_attr = mnl_attr_get_payload(tb[CTRL_ATTR_POLICY]);
+	policy_idx = mnl_attr_get_type(policy_attr);
+	attr_attr = mnl_attr_get_payload(policy_attr);
+	attr_idx = mnl_attr_get_type(attr_attr);
+
+	ret = mnl_attr_parse_nested(attr_attr, attr_cb, &tba_info);
+	if (ret < 0)
+		return MNL_CB_ERROR;
+
+	if (policy_idx == policy_ctx->op_policy_idx &&
+	    attr_idx == policy_ctx->op_hdr_attr) {
+		attr = tba[NL_POLICY_TYPE_ATTR_POLICY_IDX];
+		if (!attr) {
+			fprintf(stderr,	"Error: no policy index in what was expected to be ethtool header attribute\n");
+			return MNL_CB_ERROR;
+		}
+		policy_ctx->hdr_policy_found = true;
+		policy_ctx->hdr_policy_idx = mnl_attr_get_u32(attr);
+	}
+
+	if (policy_ctx->hdr_policy_found &&
+	    policy_ctx->hdr_policy_idx == policy_idx &&
+	    attr_idx == ETHTOOL_A_HEADER_FLAGS) {
+		attr = tba[NL_POLICY_TYPE_ATTR_MASK];
+		if (!attr) {
+			fprintf(stderr,	"Error: validation mask not reported for ethtool header flags\n");
+			return MNL_CB_ERROR;
+		}
+
+		policy_ctx->flag_mask = mnl_attr_get_u64(attr);
+	}
+
+	return MNL_CB_OK;
+}
+
+static int read_flags_policy(struct nl_context *nlctx, struct nl_socket *nlsk,
+			     unsigned int nlcmd, unsigned int hdrattr)
+{
+	struct ethtool_op_policy_query_ctx policy_ctx;
+	struct nl_msg_buff *msgbuff = &nlsk->msgbuff;
+	int ret;
+
+	if (nlctx->ops_info[nlcmd].hdr_policy_loaded)
+		return 0;
+
+	memset(&policy_ctx, 0, sizeof(policy_ctx));
+	policy_ctx.nlctx = nlctx;
+	policy_ctx.op = nlcmd;
+	policy_ctx.op_hdr_attr = hdrattr;
+
+	ret = __msg_init(msgbuff, GENL_ID_CTRL, CTRL_CMD_GETPOLICY,
+			 NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP, 1);
+	if (ret < 0)
+		return ret;
+	ret = -EMSGSIZE;
+	if (ethnla_put_u16(msgbuff, CTRL_ATTR_FAMILY_ID, nlctx->ethnl_fam))
+		return ret;
+	if (ethnla_put_u32(msgbuff, CTRL_ATTR_OP, nlcmd))
+		return ret;
+
+	nlsock_sendmsg(nlsk, NULL);
+	nlsock_process_reply(nlsk, family_policy_cb, &policy_ctx);
+
+	nlctx->ops_info[nlcmd].hdr_policy_loaded = 1;
+	nlctx->ops_info[nlcmd].hdr_flags = policy_ctx.flag_mask;
+	return 0;
+}
+
+u32 get_stats_flag(struct nl_context *nlctx, unsigned int nlcmd,
+		   unsigned int hdrattr)
+{
+	if (!nlctx->ctx->show_stats)
+		return 0;
+	if (nlcmd > ETHTOOL_MSG_USER_MAX ||
+	    !(nlctx->ops_info[nlcmd].op_flags & GENL_CMD_CAP_HASPOL))
+		return 0;
+
+	if (read_flags_policy(nlctx, nlctx->ethnl_socket, nlcmd, hdrattr) < 0)
+		return 0;
+
+	return nlctx->ops_info[nlcmd].hdr_flags & ETHTOOL_FLAG_STATS;
+}
+
 /* initialization */
 
 static int genl_read_ops(struct nl_context *nlctx,
diff --git a/netlink/netlink.h b/netlink/netlink.h
index e79143016bd5..c02558540218 100644
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -27,6 +27,8 @@ enum link_mode_class {
 
 struct nl_op_info {
 	uint32_t		op_flags;
+	uint32_t		hdr_flags;
+	uint8_t			hdr_policy_loaded:1;
 };
 
 struct nl_context {
@@ -70,6 +72,8 @@ bool netlink_cmd_check(struct cmd_context *ctx, unsigned int cmd,
 		       bool allow_wildcard);
 const char *get_dev_name(const struct nlattr *nest);
 int get_dev_info(const struct nlattr *nest, int *ifindex, char *ifname);
+u32 get_stats_flag(struct nl_context *nlctx, unsigned int nlcmd,
+		   unsigned int hdrattr);
 
 int linkmodes_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int linkinfo_reply_cb(const struct nlmsghdr *nlhdr, void *data);
-- 
2.26.2

[PATCH ethtool v3 7/7] pause: add support for dumping statistics

[Jakub Kicinski]

Add support for requesting pause frame stats from the kernel.

 # ./ethtool -I -a eth0
Pause parameters for eth0:
Autonegotiate:	on
RX:		on
TX:		on
Statistics:
  tx_pause_frames: 1
  rx_pause_frames: 1

 # ./ethtool -I --json -a eth0
[ {
        "ifname": "eth0",
        "autonegotiate": true,
        "rx": true,
        "tx": true,
        "statistics": {
            "tx_pause_frames": 1,
            "rx_pause_frames": 1
        }
    } ]

v2: - correct print format for u64

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 netlink/pause.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/netlink/pause.c b/netlink/pause.c
index c960c82cba5f..9bc9a301821f 100644
--- a/netlink/pause.c
+++ b/netlink/pause.c
@@ -5,6 +5,7 @@
  */
 
 #include <errno.h>
+#include <inttypes.h>
 #include <string.h>
 #include <stdio.h>
 
@@ -105,6 +106,62 @@ static int show_pause_autoneg_status(struct nl_context *nlctx)
 	return ret;
 }
 
+static int show_pause_stats(const struct nlattr *nest)
+{
+	const struct nlattr *tb[ETHTOOL_A_PAUSE_STAT_MAX + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	static const struct {
+		unsigned int attr;
+		char *name;
+	} stats[] = {
+		{ ETHTOOL_A_PAUSE_STAT_TX_FRAMES, "tx_pause_frames" },
+		{ ETHTOOL_A_PAUSE_STAT_RX_FRAMES, "rx_pause_frames" },
+	};
+	bool header = false;
+	unsigned int i;
+	size_t n;
+	int ret;
+
+	ret = mnl_attr_parse_nested(nest, attr_cb, &tb_info);
+	if (ret < 0)
+		return ret;
+
+	open_json_object("statistics");
+	for (i = 0; i < ARRAY_SIZE(stats); i++) {
+		char fmt[32];
+
+		if (!tb[stats[i].attr])
+			continue;
+
+		if (!header && !is_json_context()) {
+			printf("Statistics:\n");
+			header = true;
+		}
+
+		if (mnl_attr_validate(tb[stats[i].attr], MNL_TYPE_U64)) {
+			fprintf(stderr, "malformed netlink message (statistic)\n");
+			goto err_close_stats;
+		}
+
+		n = snprintf(fmt, sizeof(fmt), "  %s: %%" PRIu64 "\n",
+			     stats[i].name);
+		if (n >= sizeof(fmt)) {
+			fprintf(stderr, "internal error - malformed label\n");
+			goto err_close_stats;
+		}
+
+		print_u64(PRINT_ANY, stats[i].name, fmt,
+			  mnl_attr_get_u64(tb[stats[i].attr]));
+	}
+	close_json_object();
+
+	return 0;
+
+err_close_stats:
+	close_json_object();
+	return -1;
+}
+
 int pause_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 {
 	const struct nlattr *tb[ETHTOOL_A_PAUSE_MAX + 1] = {};
@@ -142,6 +199,11 @@ int pause_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 		if (ret < 0)
 			goto err_close_dev;
 	}
+	if (tb[ETHTOOL_A_PAUSE_STATS]) {
+		ret = show_pause_stats(tb[ETHTOOL_A_PAUSE_STATS]);
+		if (ret < 0)
+			goto err_close_dev;
+	}
 	if (!silent)
 		print_nl();
 
@@ -158,6 +220,7 @@ int nl_gpause(struct cmd_context *ctx)
 {
 	struct nl_context *nlctx = ctx->nlctx;
 	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	u32 flags;
 	int ret;
 
 	if (netlink_cmd_check(ctx, ETHTOOL_MSG_PAUSE_GET, true))
@@ -168,8 +231,10 @@ int nl_gpause(struct cmd_context *ctx)
 		return 1;
 	}
 
+	flags = get_stats_flag(nlctx, ETHTOOL_MSG_PAUSE_GET,
+			       ETHTOOL_A_PAUSE_HEADER);
 	ret = nlsock_prep_get_request(nlsk, ETHTOOL_MSG_PAUSE_GET,
-				      ETHTOOL_A_PAUSE_HEADER, 0);
+				      ETHTOOL_A_PAUSE_HEADER, flags);
 	if (ret < 0)
 		return ret;
 
-- 
2.26.2

Re: [PATCH ethtool v3 0/7] pause frame stats

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 1734 bytes --]

On Sun, Oct 18, 2020 at 02:31:44PM -0700, Jakub Kicinski wrote:
> Hi!
> 
> Sorry about the delay from v2.

Actually, I'm rather surprised you were able to get back to this so
early, given the situation.

> This set adds support for pause frame statistics.
> 
> First pause frame info is extended to support --json.
> 
> Pause stats are first of this kind of statistics so add
> support for a new flag (--include-statistics).
> 
> Next add support for dumping policy to check if the statistics
> flag is supported for a given subcommand.
> 
> Last but not least - display statistics.
> 
> v3:
>  - rename the ctx variable to policy_ctx
>  - instead of returning the flags policy save it to a member
>    of struct nl_context, for potential reuse. Also we don't
>    have to worry about returning flags and negative errors
>    from the read helper this way :)
> 
> Jakub Kicinski (7):
>   update UAPI header copies
>   pause: add --json support
>   separate FLAGS out in -h
>   add support for stats in subcommands
>   netlink: prepare for more per-op info
>   netlink: use policy dumping to check if stats flag is supported
>   pause: add support for dumping statistics
> 
>  ethtool.8.in           |   7 ++
>  ethtool.c              |  17 +++-
>  internal.h             |   1 +
>  netlink/coalesce.c     |   6 +-
>  netlink/msgbuff.h      |   6 ++
>  netlink/netlink.c      | 179 ++++++++++++++++++++++++++++++++++++++---
>  netlink/netlink.h      |  31 +++++--
>  netlink/pause.c        | 111 ++++++++++++++++++++++---
>  uapi/linux/genetlink.h |  11 +++
>  uapi/linux/netlink.h   |   4 +
>  10 files changed, 336 insertions(+), 37 deletions(-)

Series applied, thank you.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]