All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH ethtool v2 0/3] Add copybreak support
@ 2014-10-06 23:12 Govindarajulu Varadarajan
  2014-10-06 23:12 ` [PATCH ethtool v2 1/3] ethtool-copy.h: Sync with net-next 3.17.0-rc7 Govindarajulu Varadarajan
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Govindarajulu Varadarajan @ 2014-10-06 23:12 UTC (permalink / raw)
  To: ben; +Cc: netdev, ogerlitz, yevgenyp, Govindarajulu Varadarajan

This series add support for setting/getting driver's copybreak value.
copybreak is set/get using ethtool tunable interface.

This was added to net-next in
commit: f0db9b073415848709dd59a6394969882f517da9

	ethtool: Add generic options for tunables

v2:
Add support for tx_copybreak
Add 'ethtool -B eth0 rx 100 tx 256' instead of 'ethtool -B eth0 100'

Govindarajulu Varadarajan (3):
  ethtool-copy.h: Sync with net-next 3.17.0-rc7
  ethtool: Add copybreak support
  ethtool.8.in: Add man page for copybreak

 ethtool-copy.h |  29 ++++++++++
 ethtool.8.in   |  20 +++++++
 ethtool.c      | 177 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 226 insertions(+)

-- 
2.1.0

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

* [PATCH ethtool v2 1/3] ethtool-copy.h: Sync with net-next 3.17.0-rc7
  2014-10-06 23:12 [PATCH ethtool v2 0/3] Add copybreak support Govindarajulu Varadarajan
@ 2014-10-06 23:12 ` Govindarajulu Varadarajan
  2014-10-06 23:12 ` [PATCH ethtool v2 2/3] ethtool: Add copybreak support Govindarajulu Varadarajan
  2014-10-06 23:12 ` [PATCH ethtool v2 3/3] ethtool.8.in: Add man page for copybreak Govindarajulu Varadarajan
  2 siblings, 0 replies; 7+ messages in thread
From: Govindarajulu Varadarajan @ 2014-10-06 23:12 UTC (permalink / raw)
  To: ben; +Cc: netdev, ogerlitz, yevgenyp, Govindarajulu Varadarajan

This covers kernel changes up to:

commit:	1255a5055449781a92076fc5429952f2b33cf309
Author:	Eric Dumazet <edumazet@google.com>
Date:	Sun Oct 5 12:35:21 2014 +0300

	ethtool: Ethtool parameter to dynamically change tx_copybreak

Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
 ethtool-copy.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 61b78fc..3c89379 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -209,6 +209,33 @@ struct ethtool_value {
 	__u32	data;
 };
 
+enum tunable_id {
+	ETHTOOL_ID_UNSPEC,
+	ETHTOOL_RX_COPYBREAK,
+	ETHTOOL_TX_COPYBREAK,
+};
+
+enum tunable_type_id {
+	ETHTOOL_TUNABLE_UNSPEC,
+	ETHTOOL_TUNABLE_U8,
+	ETHTOOL_TUNABLE_U16,
+	ETHTOOL_TUNABLE_U32,
+	ETHTOOL_TUNABLE_U64,
+	ETHTOOL_TUNABLE_STRING,
+	ETHTOOL_TUNABLE_S8,
+	ETHTOOL_TUNABLE_S16,
+	ETHTOOL_TUNABLE_S32,
+	ETHTOOL_TUNABLE_S64,
+};
+
+struct ethtool_tunable {
+	__u32	cmd;
+	__u32	id;
+	__u32	type_id;
+	__u32	len;
+	void	*data[0];
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -1152,6 +1179,8 @@ enum ethtool_sfeatures_retval_bits {
 
 #define ETHTOOL_GRSSH		0x00000046 /* Get RX flow hash configuration */
 #define ETHTOOL_SRSSH		0x00000047 /* Set RX flow hash configuration */
+#define ETHTOOL_GTUNABLE	0x00000048 /* Get tunable configuration */
+#define ETHTOOL_STUNABLE	0x00000049 /* Set tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
-- 
2.1.0

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

* [PATCH ethtool v2 2/3] ethtool: Add copybreak support
  2014-10-06 23:12 [PATCH ethtool v2 0/3] Add copybreak support Govindarajulu Varadarajan
  2014-10-06 23:12 ` [PATCH ethtool v2 1/3] ethtool-copy.h: Sync with net-next 3.17.0-rc7 Govindarajulu Varadarajan
@ 2014-10-06 23:12 ` Govindarajulu Varadarajan
  2014-12-14 17:46   ` Ben Hutchings
  2014-10-06 23:12 ` [PATCH ethtool v2 3/3] ethtool.8.in: Add man page for copybreak Govindarajulu Varadarajan
  2 siblings, 1 reply; 7+ messages in thread
From: Govindarajulu Varadarajan @ 2014-10-06 23:12 UTC (permalink / raw)
  To: ben; +Cc: netdev, ogerlitz, yevgenyp, Govindarajulu Varadarajan

This patch adds support for setting/getting driver's rx_copybreak value.
copybreak is set/get using new ethtool tunable interface.

This was added to net-next in
commit: f0db9b073415848709dd59a6394969882f517da9

	ethtool: Add generic options for tunables

Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
 ethtool.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 177 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index bf583f3..4045356 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -179,6 +179,12 @@ static const struct flag_info flags_msglvl[] = {
 	{ "wol",	NETIF_MSG_WOL },
 };
 
+static const char *tunable_name[] = {
+	[ETHTOOL_ID_UNSPEC]	= "Unspec",
+	[ETHTOOL_RX_COPYBREAK]	= "rx",
+	[ETHTOOL_TX_COPYBREAK]	= "tx",
+};
+
 struct off_flag_def {
 	const char *short_name;
 	const char *long_name;
@@ -1805,6 +1811,173 @@ static int do_gring(struct cmd_context *ctx)
 	return 0;
 }
 
+static int get_u32tunable(struct cmd_context *ctx, enum tunable_id id,
+			  __u32 *value)
+{
+	struct ethtool_tunable *etuna;
+	int ret;
+
+	etuna = calloc(sizeof(*etuna) + sizeof(__u32), 1);
+	if (!etuna)
+		return 1;
+	etuna->cmd = ETHTOOL_GTUNABLE;
+	etuna->id = id;
+	etuna->type_id = ETHTOOL_TUNABLE_U32;
+	etuna->len = sizeof(__u32);
+	ret = send_ioctl(ctx, etuna);
+	*value = *(__u32 *)((void *)etuna + sizeof(*etuna));
+	free(etuna);
+
+	return ret;
+}
+
+static int print_u32tunable(int err, enum tunable_id id, const __u32 value)
+{
+	if (err) {
+		switch (errno) {
+		/* Driver does not support this particular tunable
+		 * Usually displays 0
+		 */
+		case EINVAL:
+			goto print;
+		/* Driver does not support get tunables ops or no such device
+		 * No point in proceeding further
+		 */
+		case EOPNOTSUPP:
+		case ENODEV:
+			perror("Cannot get device settings");
+			exit(err);
+		default:
+			perror(tunable_name[id]);
+			return err;
+		}
+	}
+print:
+	fprintf(stdout, "%s: %u\n", tunable_name[id], value);
+
+	return 0;
+}
+
+static int do_gcopybreak(struct cmd_context *ctx)
+{
+	int err, anyerror = 0;
+	__u32 u32value;
+
+	if (ctx->argc != 0)
+		exit_bad_args();
+
+	fprintf(stdout, "Copybreak settings for device %s\n", ctx->devname);
+
+	err = get_u32tunable(ctx, ETHTOOL_RX_COPYBREAK, &u32value);
+	err = print_u32tunable(err, ETHTOOL_RX_COPYBREAK, u32value);
+	if (err)
+		anyerror = err;
+
+	err = get_u32tunable(ctx, ETHTOOL_TX_COPYBREAK, &u32value);
+	err = print_u32tunable(err, ETHTOOL_TX_COPYBREAK, u32value);
+	if (err)
+		anyerror = err;
+
+	if (anyerror)
+		fprintf(stderr, "Failed to get all settings. displayed partial settings\n");
+
+	return anyerror;
+}
+
+static int set_u32tunable(struct cmd_context *ctx, enum tunable_id id,
+			  const __u32 value)
+{
+	struct ethtool_tunable *etuna;
+	int ret;
+	__u32 *data;
+
+	etuna = malloc(sizeof(*etuna) + sizeof(__u32));
+	if (!etuna) {
+		perror(tunable_name[id]);
+		return 1;
+	}
+	data = (void *)etuna + sizeof(*etuna);
+	*data = value;
+	etuna->cmd = ETHTOOL_STUNABLE;
+	etuna->id = id;
+	etuna->type_id = ETHTOOL_TUNABLE_U32;
+	etuna->len = sizeof(__u32);
+	ret = send_ioctl(ctx, etuna);
+	free(etuna);
+
+	return ret;
+}
+
+static int check_set_u32tunable(int err, enum tunable_id id)
+{
+	if (err) {
+		switch (errno) {
+		/* Driver does not support get tunables ops or no such device
+		 * No point in proceeding further
+		 */
+		case EOPNOTSUPP:
+		case ENODEV:
+			perror("Cannot set device settings");
+			exit(err);
+		default:
+			perror(tunable_name[id]);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int do_scopybreak(struct cmd_context *ctx)
+{
+	int err, anyerr = 0;
+	int copybreak_changed = 0;
+	__u32 rx, tx;
+	s32 rx_seen = 0;
+	s32 tx_seen = 0;
+
+	struct cmdline_info cmdline_channels[] = {
+		{ .name = "rx",
+		  .type = CMDL_U32,
+		  .wanted_val = &rx,
+		  .seen_val = &rx_seen, },
+
+		{ .name = "tx",
+		  .type = CMDL_U32,
+		  .wanted_val = &tx,
+		  .seen_val = &tx_seen, },
+	};
+
+	parse_generic_cmdline(ctx, &copybreak_changed, cmdline_channels,
+			      ARRAY_SIZE(cmdline_channels));
+
+	if (!copybreak_changed) {
+		fprintf(stderr, "no copybreak parameters changed\n");
+		return 0;
+	}
+
+	if (rx_seen) {
+		err = set_u32tunable(ctx, ETHTOOL_RX_COPYBREAK, rx);
+		err = check_set_u32tunable(err, ETHTOOL_RX_COPYBREAK);
+		if (err)
+			anyerr = err;
+	}
+
+	if (tx_seen) {
+		err = set_u32tunable(ctx, ETHTOOL_TX_COPYBREAK, tx);
+		err = check_set_u32tunable(err, ETHTOOL_TX_COPYBREAK);
+		if (err)
+			anyerr = err;
+	}
+
+	if (anyerr) {
+		fprintf(stderr, "Failed to set all requested parameters\n");
+		return anyerr;
+	}
+
+	return 0;
+}
+
 static int do_schannels(struct cmd_context *ctx)
 {
 	struct ethtool_channels echannels;
@@ -4055,6 +4228,10 @@ static const struct option {
 	  "		[ rx-mini N ]\n"
 	  "		[ rx-jumbo N ]\n"
 	  "		[ tx N ]\n" },
+	{ "-b|--show-copybreak", 1, do_gcopybreak, "Show copybreak values" },
+	{ "-B|--set-copybreak", 1, do_scopybreak, "Set copybreak values",
+	  "		[ rx N]\n"
+	  "		[ tx N]\n" },
 	{ "-k|--show-features|--show-offload", 1, do_gfeatures,
 	  "Get state of protocol offload and other features" },
 	{ "-K|--features|--offload", 1, do_sfeatures,
-- 
2.1.0

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

* [PATCH ethtool v2 3/3] ethtool.8.in: Add man page for copybreak
  2014-10-06 23:12 [PATCH ethtool v2 0/3] Add copybreak support Govindarajulu Varadarajan
  2014-10-06 23:12 ` [PATCH ethtool v2 1/3] ethtool-copy.h: Sync with net-next 3.17.0-rc7 Govindarajulu Varadarajan
  2014-10-06 23:12 ` [PATCH ethtool v2 2/3] ethtool: Add copybreak support Govindarajulu Varadarajan
@ 2014-10-06 23:12 ` Govindarajulu Varadarajan
  2 siblings, 0 replies; 7+ messages in thread
From: Govindarajulu Varadarajan @ 2014-10-06 23:12 UTC (permalink / raw)
  To: ben; +Cc: netdev, ogerlitz, yevgenyp, Govindarajulu Varadarajan

Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
 ethtool.8.in | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index ae56293..9f0955f 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -176,6 +176,14 @@ ethtool \- query or control network driver and hardware settings
 .BN rx\-jumbo
 .BN tx
 .HP
+.B ethtool \-b|\-\-show\-copybreak
+.I devname
+.HP
+.B ethtool \-B|\-\-set\-copybreak
+.I devname
+.BN rx
+.BN tx
+.HP
 .B ethtool \-i|\-\-driver
 .I devname
 .HP
@@ -399,6 +407,18 @@ Changes the number of ring entries for the Rx Jumbo ring.
 .BI tx \ N
 Changes the number of ring entries for the Tx ring.
 .TP
+.B \-b|\-\-show\-copybreak
+Get device copybreak values
+.TP
+.B \-B|\-\-set\-copybreak
+Set device copybreak values
+.TP
+.BI rx \ N
+Change rx_copybreak
+.TP
+.BI tx \ N
+Change tx_copybreak
+.TP
 .B \-i \-\-driver
 Queries the specified network device for associated driver information.
 .TP
-- 
2.1.0

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

* Re: [PATCH ethtool v2 2/3] ethtool: Add copybreak support
  2014-10-06 23:12 ` [PATCH ethtool v2 2/3] ethtool: Add copybreak support Govindarajulu Varadarajan
@ 2014-12-14 17:46   ` Ben Hutchings
  2015-05-13  7:18     ` Hadar Hen Zion
  0 siblings, 1 reply; 7+ messages in thread
From: Ben Hutchings @ 2014-12-14 17:46 UTC (permalink / raw)
  To: Govindarajulu Varadarajan; +Cc: netdev, ogerlitz, yevgenyp

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

On Tue, 2014-10-07 at 04:42 +0530, Govindarajulu Varadarajan wrote:
> This patch adds support for setting/getting driver's rx_copybreak value.
> copybreak is set/get using new ethtool tunable interface.
> 
> This was added to net-next in
> commit: f0db9b073415848709dd59a6394969882f517da9
> 
> 	ethtool: Add generic options for tunables
> 
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
> ---
>  ethtool.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 177 insertions(+)
> 
> diff --git a/ethtool.c b/ethtool.c
> index bf583f3..4045356 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -179,6 +179,12 @@ static const struct flag_info flags_msglvl[] = {
>  	{ "wol",	NETIF_MSG_WOL },
>  };
>  
> +static const char *tunable_name[] = {
> +	[ETHTOOL_ID_UNSPEC]	= "Unspec",
> +	[ETHTOOL_RX_COPYBREAK]	= "rx",
> +	[ETHTOOL_TX_COPYBREAK]	= "tx",
> +};

Tunables should be named by a string set defined in the kernel.

[...]
> @@ -4055,6 +4228,10 @@ static const struct option {
>           "             [ rx-mini N ]\n"
>           "             [ rx-jumbo N ]\n"
>           "             [ tx N ]\n" },
> +       { "-b|--show-copybreak", 1, do_gcopybreak, "Show copybreak values" },
> +       { "-B|--set-copybreak", 1, do_scopybreak, "Set copybreak values",
> +         "             [ rx N]\n"
> +         "             [ tx N]\n" },
>         { "-k|--show-features|--show-offload", 1, do_gfeatures,
>           "Get state of protocol offload and other features" },
>         { "-K|--features|--offload", 1, do_sfeatures,
[...]

T don't think this is worth two options of its own.  You should be able
to add generic get/set-tunable optins.  You'll need to get the string
set to find out the names of tunables.  When setting a tunable, you'll
need to get it first to find out its type.

Ben.

-- 
Ben Hutchings
The two most common things in the universe are hydrogen and stupidity.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

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

* Re: [PATCH ethtool v2 2/3] ethtool: Add copybreak support
  2014-12-14 17:46   ` Ben Hutchings
@ 2015-05-13  7:18     ` Hadar Hen Zion
  2015-05-19  3:47       ` Govindarajulu Varadarajan
  0 siblings, 1 reply; 7+ messages in thread
From: Hadar Hen Zion @ 2015-05-13  7:18 UTC (permalink / raw)
  To: Govindarajulu Varadarajan, Ben Hutchings
  Cc: netdev, Or Gerlitz, yevgenyp, Amir Vadai

On Sun, Dec 14, 2014 at 8:46 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Tue, 2014-10-07 at 04:42 +0530, Govindarajulu Varadarajan wrote:
>> This patch adds support for setting/getting driver's rx_copybreak value.
>> copybreak is set/get using new ethtool tunable interface.
>>
>> This was added to net-next in
>> commit: f0db9b073415848709dd59a6394969882f517da9
>>
>>       ethtool: Add generic options for tunables
>>
>> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
>> ---
>>  ethtool.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 177 insertions(+)
>>
>> diff --git a/ethtool.c b/ethtool.c
>> index bf583f3..4045356 100644
>> --- a/ethtool.c
>> +++ b/ethtool.c
>> @@ -179,6 +179,12 @@ static const struct flag_info flags_msglvl[] = {
>>       { "wol",        NETIF_MSG_WOL },
>>  };
>>
>> +static const char *tunable_name[] = {
>> +     [ETHTOOL_ID_UNSPEC]     = "Unspec",
>> +     [ETHTOOL_RX_COPYBREAK]  = "rx",
>> +     [ETHTOOL_TX_COPYBREAK]  = "tx",
>> +};
>
> Tunables should be named by a string set defined in the kernel.
>
> [...]
>> @@ -4055,6 +4228,10 @@ static const struct option {
>>           "             [ rx-mini N ]\n"
>>           "             [ rx-jumbo N ]\n"
>>           "             [ tx N ]\n" },
>> +       { "-b|--show-copybreak", 1, do_gcopybreak, "Show copybreak values" },
>> +       { "-B|--set-copybreak", 1, do_scopybreak, "Set copybreak values",
>> +         "             [ rx N]\n"
>> +         "             [ tx N]\n" },
>>         { "-k|--show-features|--show-offload", 1, do_gfeatures,
>>           "Get state of protocol offload and other features" },
>>         { "-K|--features|--offload", 1, do_sfeatures,
> [...]
>
> T don't think this is worth two options of its own.  You should be able
> to add generic get/set-tunable optins.  You'll need to get the string
> set to find out the names of tunables.  When setting a tunable, you'll
> need to get it first to find out its type.
>
> Ben.
>
> --
> Ben Hutchings
> The two most common things in the universe are hydrogen and stupidity.

Hi Govindarajulu,

Do you have a patch that fixes Ben's comments?

I would like to add the copybreak feature so let me know if you want
me to fix the patch according to Ben's comments and resend it.

Thanks,
Hadar

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

* Re: [PATCH ethtool v2 2/3] ethtool: Add copybreak support
  2015-05-13  7:18     ` Hadar Hen Zion
@ 2015-05-19  3:47       ` Govindarajulu Varadarajan
  0 siblings, 0 replies; 7+ messages in thread
From: Govindarajulu Varadarajan @ 2015-05-19  3:47 UTC (permalink / raw)
  To: Hadar Hen Zion
  Cc: Govindarajulu Varadarajan, Ben Hutchings, netdev, Or Gerlitz,
	yevgenyp, Amir Vadai

On Wed, 13 May 2015, Hadar Hen Zion wrote:
> Hi Govindarajulu,
>
> Do you have a patch that fixes Ben's comments?
>
> I would like to add the copybreak feature so let me know if you want
> me to fix the patch according to Ben's comments and resend it.
>

Hi Hadar

I haven't looked at this for a while. Please go ahead.

Thanks

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

end of thread, other threads:[~2015-05-19  3:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-06 23:12 [PATCH ethtool v2 0/3] Add copybreak support Govindarajulu Varadarajan
2014-10-06 23:12 ` [PATCH ethtool v2 1/3] ethtool-copy.h: Sync with net-next 3.17.0-rc7 Govindarajulu Varadarajan
2014-10-06 23:12 ` [PATCH ethtool v2 2/3] ethtool: Add copybreak support Govindarajulu Varadarajan
2014-12-14 17:46   ` Ben Hutchings
2015-05-13  7:18     ` Hadar Hen Zion
2015-05-19  3:47       ` Govindarajulu Varadarajan
2014-10-06 23:12 ` [PATCH ethtool v2 3/3] ethtool.8.in: Add man page for copybreak Govindarajulu Varadarajan

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.