All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH ethtool v2 1/2] ethtool: add support for get/set ethtool_tunable
@ 2020-07-17 14:59 Govindarajulu Varadarajan
  2020-07-17 14:59 ` [PATCH ethtool v2 2/2] man: add man page for ETHTOOL_GTUNABLE and ETHTOOL_STUNABLE Govindarajulu Varadarajan
  2020-07-19 23:47 ` [PATCH ethtool v2 1/2] ethtool: add support for get/set ethtool_tunable Michal Kubecek
  0 siblings, 2 replies; 4+ messages in thread
From: Govindarajulu Varadarajan @ 2020-07-17 14:59 UTC (permalink / raw)
  To: netdev, edumazet, linville, mkubecek
  Cc: govind.varadar, benve, Govindarajulu Varadarajan

Add support for ETHTOOL_GTUNABLE and ETHTOOL_STUNABLE options.

Tested rx-copybreak on enic driver. Tested ETHTOOL_TUNNABLE_STRING
options with test/debug changes in kernel.

Signed-off-by: Govindarajulu Varadarajan <gvaradar@cisco.com>
---
v2:
* Fix alignments and braces.
* Move union definition outside struct.
* Make seen type int.
* Use uniform C90 types in union.
* Remove NULL assignment and memset to 0.
* Change variable name from tinfo to tunables_info.
* Use ethtool_tunable_info_val in print_tunable()
* Remove one-letter command line option.
* Use PRI* for int type in print_tunable().

 ethtool.c | 208 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 208 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index 021f528..0a12699 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -40,6 +40,7 @@
 #include <sys/utsname.h>
 #include <limits.h>
 #include <ctype.h>
+#include <inttypes.h>
 
 #include <sys/socket.h>
 #include <netinet/in.h>
@@ -4685,6 +4686,197 @@ static int do_seee(struct cmd_context *ctx)
 	return 0;
 }
 
+/* copy of net/ethtool/common.c */
+char
+tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
+	[ETHTOOL_ID_UNSPEC]		= "Unspec",
+	[ETHTOOL_RX_COPYBREAK]		= "rx-copybreak",
+	[ETHTOOL_TX_COPYBREAK]		= "tx-copybreak",
+	[ETHTOOL_PFC_PREVENTION_TOUT]	= "pfc-prevention-tout",
+};
+
+union ethtool_tunable_info_val {
+	uint8_t u8;
+	uint16_t u16;
+	uint32_t u32;
+	uint64_t u64;
+	int8_t s8;
+	int16_t s16;
+	int32_t s32;
+	int64_t s64;
+	char *str;
+};
+
+struct ethtool_tunable_info {
+	enum tunable_id t_id;
+	enum tunable_type_id t_type_id;
+	size_t size;
+	cmdline_type_t type;
+	union ethtool_tunable_info_val wanted;
+	int seen;
+};
+
+static struct ethtool_tunable_info tunables_info[] = {
+	{ .t_id		= ETHTOOL_RX_COPYBREAK,
+	  .t_type_id	= ETHTOOL_TUNABLE_U32,
+	  .size		= sizeof(u32),
+	  .type		= CMDL_U32,
+	},
+	{ .t_id		= ETHTOOL_TX_COPYBREAK,
+	  .t_type_id	= ETHTOOL_TUNABLE_U32,
+	  .size		= sizeof(u32),
+	  .type		= CMDL_U32,
+	},
+	{ .t_id		= ETHTOOL_PFC_PREVENTION_TOUT,
+	  .t_type_id	= ETHTOOL_TUNABLE_U16,
+	  .size		= sizeof(u16),
+	  .type		= CMDL_U16,
+	},
+};
+#define TUNABLES_INFO_SIZE	ARRAY_SIZE(tunables_info)
+
+static int do_stunable(struct cmd_context *ctx)
+{
+	struct cmdline_info cmdline_tunable[TUNABLES_INFO_SIZE];
+	struct ethtool_tunable_info *tinfo = tunables_info;
+	int changed = 0;
+	int i;
+
+	for (i = 0; i < TUNABLES_INFO_SIZE; i++) {
+		cmdline_tunable[i].name = tunable_strings[tinfo[i].t_id];
+		cmdline_tunable[i].type = tinfo[i].type;
+		cmdline_tunable[i].wanted_val = &tinfo[i].wanted;
+		cmdline_tunable[i].seen_val = &tinfo[i].seen;
+	}
+
+	parse_generic_cmdline(ctx, &changed, cmdline_tunable, TUNABLES_INFO_SIZE);
+	if (!changed)
+		exit_bad_args();
+
+	for (i = 0; i < TUNABLES_INFO_SIZE; i++) {
+		char **val = (char **)&tinfo[i].wanted;
+		struct ethtool_tunable *tuna;
+		size_t size;
+		int ret;
+
+		if (!tinfo[i].seen)
+			continue;
+
+		size = sizeof(*tuna);
+		if (tinfo[i].type == CMDL_STR)
+			size +=  strlen(*val) + 1;
+		else
+			size += tinfo[i].size;
+		tuna = calloc(1, size);
+		if (!tuna) {
+			perror(tunable_strings[tinfo[i].t_id]);
+			return 1;
+		}
+		tuna->cmd = ETHTOOL_STUNABLE;
+		tuna->id = tinfo[i].t_id;
+		tuna->type_id = tinfo[i].t_type_id;
+		if (tinfo[i].type == CMDL_STR) {
+			tuna->len = strlen(*val) + 1;
+			memcpy(tuna->data, *val, strlen(*val) + 1);
+		} else {
+			tuna->len = tinfo[i].size;
+			memcpy(tuna->data, &tinfo[i].wanted, tuna->len);
+		}
+		ret = send_ioctl(ctx, tuna);
+		if (ret) {
+			perror(tunable_strings[tuna->id]);
+			return ret;
+		}
+		free(tuna);
+	}
+	return 0;
+}
+
+static void print_tunable(struct ethtool_tunable *tuna)
+{
+	char *name = tunable_strings[tuna->id];
+	union ethtool_tunable_info_val *val;
+
+	val = (union ethtool_tunable_info_val *)tuna->data;
+	switch (tuna->type_id) {
+	case ETHTOOL_TUNABLE_U8:
+		fprintf(stdout, "%s: %" PRIu8 "\n", name, val->u8);
+		break;
+	case ETHTOOL_TUNABLE_U16:
+		fprintf(stdout, "%s: %" PRIu16 "\n", name, val->u16);
+		break;
+	case ETHTOOL_TUNABLE_U32:
+		fprintf(stdout, "%s: %" PRIu32 "\n", name, val->u32);
+		break;
+	case ETHTOOL_TUNABLE_U64:
+		fprintf(stdout, "%s: %" PRIu64 "\n", name, val->u64);
+		break;
+	case ETHTOOL_TUNABLE_S8:
+		fprintf(stdout, "%s: %" PRId8 "\n", name, val->s8);
+		break;
+	case ETHTOOL_TUNABLE_S16:
+		fprintf(stdout, "%s: %" PRId16 "\n", name, val->s16);
+		break;
+	case ETHTOOL_TUNABLE_S32:
+		fprintf(stdout, "%s: %" PRId32 "\n", name, val->s32);
+		break;
+	case ETHTOOL_TUNABLE_S64:
+		fprintf(stdout, "%s: %" PRId64 "\n", name, val->s64);
+		break;
+	case ETHTOOL_TUNABLE_STRING:
+		fprintf(stdout, "%s: %s\n", name, val->str);
+		break;
+	default:
+		fprintf(stdout, "%s: Unknown format\n", name);
+	}
+}
+
+static int do_gtunable(struct cmd_context *ctx)
+{
+	struct ethtool_tunable_info *tinfo = tunables_info;
+	char **argp = ctx->argp;
+	int argc = ctx->argc;
+	int i;
+	int j;
+
+	if (argc < 1)
+		exit_bad_args();
+
+	for (i = 0; i < argc; i++) {
+		int valid = 0;
+
+		for (j = 0; j < TUNABLES_INFO_SIZE; j++) {
+			char *ts = tunable_strings[tinfo[j].t_id];
+			struct ethtool_tunable *tuna;
+			int ret;
+
+			if (strcmp(argp[i], ts))
+				continue;
+			valid = 1;
+
+			tuna = calloc(1, sizeof(*tuna) + tinfo[j].size);
+			if (!tuna) {
+				perror(ts);
+				return 1;
+			}
+			tuna->cmd = ETHTOOL_GTUNABLE;
+			tuna->id = tinfo[j].t_id;
+			tuna->type_id = tinfo[j].t_type_id;
+			tuna->len = tinfo[j].size;
+			ret = send_ioctl(ctx, tuna);
+			if (ret) {
+				fprintf(stderr, "%s: Cannot get tunable\n", ts);
+				return ret;
+			}
+			print_tunable(tuna);
+			free(tuna);
+		}
+		if (!valid)
+			exit_bad_args();
+	}
+	return 0;
+}
+
 static int do_get_phy_tunable(struct cmd_context *ctx)
 {
 	int argc = ctx->argc;
@@ -5438,6 +5630,22 @@ static const struct option args[] = {
 			  "		[ fast-link-down ]\n"
 			  "		[ energy-detect-power-down ]\n"
 	},
+	{
+		.opts	= "--get-tunable",
+		.func	= do_gtunable,
+		.help	= "Get tunable",
+		.xhelp	= "		[ rx-copybreak ]\n"
+			  "		[ tx-copybreak ]\n"
+			  "		[ pfc-precention-tout ]\n"
+	},
+	{
+		.opts	= "--set-tunable",
+		.func	= do_stunable,
+		.help	= "Set tunable",
+		.xhelp	= "		[ rx-copybreak N]\n"
+			  "		[ tx-copybreak N]\n"
+			  "		[ pfc-precention-tout N]\n"
+	},
 	{
 		.opts	= "--reset",
 		.func	= do_reset,
-- 
2.27.0


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

* [PATCH ethtool v2 2/2] man: add man page for ETHTOOL_GTUNABLE and ETHTOOL_STUNABLE
  2020-07-17 14:59 [PATCH ethtool v2 1/2] ethtool: add support for get/set ethtool_tunable Govindarajulu Varadarajan
@ 2020-07-17 14:59 ` Govindarajulu Varadarajan
  2020-07-19 23:51   ` Michal Kubecek
  2020-07-19 23:47 ` [PATCH ethtool v2 1/2] ethtool: add support for get/set ethtool_tunable Michal Kubecek
  1 sibling, 1 reply; 4+ messages in thread
From: Govindarajulu Varadarajan @ 2020-07-17 14:59 UTC (permalink / raw)
  To: netdev, edumazet, linville, mkubecek
  Cc: govind.varadar, benve, Govindarajulu Varadarajan

Signed-off-by: Govindarajulu Varadarajan <gvaradar@cisco.com>
---
v2:
Add description

 ethtool.8.in | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 689822e..9a3e9a7 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -398,6 +398,18 @@ ethtool \- query or control network driver and hardware settings
 .RB [ fast-link-down ]
 .RB [ energy-detect-power-down ]
 .HP
+.B ethtool \-\-get\-tunable
+.I devname
+.RB [ rx-copybreak ]
+.RB [ tx-copybreak ]
+.RB [ pfc-prevention-tout ]
+.HP
+.B ethtool \-\-set\-tunable
+.I devname
+.BN rx\-copybreak
+.BN tx\-copybreak
+.BN pfc\-prevention\-tout
+.HP
 .B ethtool \-\-reset
 .I devname
 .BN flags
@@ -1211,6 +1223,34 @@ Gets the PHY Fast Link Down status / period.
 .B energy\-detect\-power\-down
 Gets the current configured setting for Energy Detect Power Down (if supported).
 
+.RE
+.TP
+.B \-\-get\-tunable
+Get the tunable parameters.
+.RS 4
+.TP
+.B rx\-copybreak
+Get the current rx copybreak value in bytes.
+.TP
+.B tx\-copybreak
+Get the current tx copybreak value in bytes.
+.TP
+.B pfc\-prevention\-tout
+Get the current pfc prevention timeout value in msecs.
+.RE
+.TP
+.B \-\-set\-tunable
+Set driver's tunable parameters.
+.RS 4
+.TP
+.BI rx\-copybreak \ N
+Set the rx copybreak value in bytes.
+.TP
+.BI tx\-copybreak \ N
+Set the tx copybreak value in bytes.
+.TP
+.BI pfc\-prevention\-tout \ N
+Set pfc prevention timeout in msecs.
 .RE
 .TP
 .B \-\-reset
-- 
2.27.0


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

* Re: [PATCH ethtool v2 1/2] ethtool: add support for get/set ethtool_tunable
  2020-07-17 14:59 [PATCH ethtool v2 1/2] ethtool: add support for get/set ethtool_tunable Govindarajulu Varadarajan
  2020-07-17 14:59 ` [PATCH ethtool v2 2/2] man: add man page for ETHTOOL_GTUNABLE and ETHTOOL_STUNABLE Govindarajulu Varadarajan
@ 2020-07-19 23:47 ` Michal Kubecek
  1 sibling, 0 replies; 4+ messages in thread
From: Michal Kubecek @ 2020-07-19 23:47 UTC (permalink / raw)
  To: Govindarajulu Varadarajan
  Cc: netdev, edumazet, linville, govind.varadar, benve

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

On Fri, Jul 17, 2020 at 07:59:49AM -0700, Govindarajulu Varadarajan wrote:
> Add support for ETHTOOL_GTUNABLE and ETHTOOL_STUNABLE options.
> 
> Tested rx-copybreak on enic driver. Tested ETHTOOL_TUNNABLE_STRING

A typo: TUNNABLE -> TUNABLE

> options with test/debug changes in kernel.
> 
> Signed-off-by: Govindarajulu Varadarajan <gvaradar@cisco.com>
> ---

This looks good to me but I'm still not happy with the string tunables
handling. The reason I asked about it was to find out if I missed some
important piece of information. But it doesn't seem to be the case so
that the situation looks like this:

  - there is no documentation telling us how they should work
  - there is no kernel or userspace code yet (except this patch)
  - there is no string tunable yet
  - we don't even know if there is ever going to be any
  - proposed code is inconsistent (it allows passing value to kernel
    which it would not be able to receive back from kernel)
  - it adds extra complexity to do_gtunable() and do_stunable()
    (special handling and allocating a new buffer in each iteration)
  - it's dead code anyway: the way the interface is designed, current
    ethtool cannot get/set future tunables it does not recognize)

Therefore I suggest to drop handling of string tunables until there is
actually a string tunable and we get (preferrably documented) consensus
on how the interface should behave. (Which may very well never happen.)

Michal

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

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

* Re: [PATCH ethtool v2 2/2] man: add man page for ETHTOOL_GTUNABLE and ETHTOOL_STUNABLE
  2020-07-17 14:59 ` [PATCH ethtool v2 2/2] man: add man page for ETHTOOL_GTUNABLE and ETHTOOL_STUNABLE Govindarajulu Varadarajan
@ 2020-07-19 23:51   ` Michal Kubecek
  0 siblings, 0 replies; 4+ messages in thread
From: Michal Kubecek @ 2020-07-19 23:51 UTC (permalink / raw)
  To: Govindarajulu Varadarajan
  Cc: netdev, edumazet, linville, govind.varadar, benve

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

On Fri, Jul 17, 2020 at 07:59:50AM -0700, Govindarajulu Varadarajan wrote:
> Signed-off-by: Govindarajulu Varadarajan <gvaradar@cisco.com>
> ---
> v2:
> Add description
> 
>  ethtool.8.in | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 689822e..9a3e9a7 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -398,6 +398,18 @@ ethtool \- query or control network driver and hardware settings
>  .RB [ fast-link-down ]
>  .RB [ energy-detect-power-down ]
>  .HP
> +.B ethtool \-\-get\-tunable
> +.I devname
> +.RB [ rx-copybreak ]
> +.RB [ tx-copybreak ]
> +.RB [ pfc-prevention-tout ]
> +.HP
> +.B ethtool \-\-set\-tunable
> +.I devname
> +.BN rx\-copybreak
> +.BN tx\-copybreak
> +.BN pfc\-prevention\-tout
> +.HP
>  .B ethtool \-\-reset
>  .I devname
>  .BN flags
> @@ -1211,6 +1223,34 @@ Gets the PHY Fast Link Down status / period.
>  .B energy\-detect\-power\-down
>  Gets the current configured setting for Energy Detect Power Down (if supported).
>  
> +.RE
> +.TP
> +.B \-\-get\-tunable
> +Get the tunable parameters.
> +.RS 4
> +.TP
> +.B rx\-copybreak
> +Get the current rx copybreak value in bytes.
> +.TP
> +.B tx\-copybreak
> +Get the current tx copybreak value in bytes.
> +.TP
> +.B pfc\-prevention\-tout
> +Get the current pfc prevention timeout value in msecs.

Please document also the special values 0 (disabled) and 65535 (auto).

Michal

> +.RE
> +.TP
> +.B \-\-set\-tunable
> +Set driver's tunable parameters.
> +.RS 4
> +.TP
> +.BI rx\-copybreak \ N
> +Set the rx copybreak value in bytes.
> +.TP
> +.BI tx\-copybreak \ N
> +Set the tx copybreak value in bytes.
> +.TP
> +.BI pfc\-prevention\-tout \ N
> +Set pfc prevention timeout in msecs.
>  .RE
>  .TP
>  .B \-\-reset
> -- 
> 2.27.0
> 

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

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

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

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 14:59 [PATCH ethtool v2 1/2] ethtool: add support for get/set ethtool_tunable Govindarajulu Varadarajan
2020-07-17 14:59 ` [PATCH ethtool v2 2/2] man: add man page for ETHTOOL_GTUNABLE and ETHTOOL_STUNABLE Govindarajulu Varadarajan
2020-07-19 23:51   ` Michal Kubecek
2020-07-19 23:47 ` [PATCH ethtool v2 1/2] ethtool: add support for get/set ethtool_tunable Michal Kubecek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.