netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ethtool: Features added
@ 2011-09-20 10:31 Sucheta Chakraborty
  2011-09-20 10:31 ` [PATCH ethtool 1/3] ethtool: file ethtool-copy.h update Sucheta Chakraborty
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sucheta Chakraborty @ 2011-09-20 10:31 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, Dept_NX_Linux_NIC_Driver


Please apply the patches to ethtool tree.

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

* [PATCH ethtool 1/3] ethtool: file ethtool-copy.h update.
  2011-09-20 10:31 [PATCH 0/3] ethtool: Features added Sucheta Chakraborty
@ 2011-09-20 10:31 ` Sucheta Chakraborty
  2011-10-04 22:09   ` Ben Hutchings
  2011-09-20 10:31 ` [PATCH ethtool 2/3] ethtool: add support for external loopback Sucheta Chakraborty
  2011-09-20 10:31 ` [PATCH ethtool 3/3] ethtool: add ETHTOOL_{G,S}CHANNEL support Sucheta Chakraborty
  2 siblings, 1 reply; 8+ messages in thread
From: Sucheta Chakraborty @ 2011-09-20 10:31 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, Dept_NX_Linux_NIC_Driver

Used linux-3.0.1-net-next tree for the update.

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 ethtool-copy.h |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index c7a18f7..0fbbb25 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -265,7 +265,7 @@ struct ethtool_pauseparam {
 	__u32	cmd;	/* ETHTOOL_{G,S}PAUSEPARAM */
 
 	/* If the link is being auto-negotiated (via ethtool_cmd.autoneg
-	 * being true) the user may set 'autonet' here non-zero to have the
+	 * being true) the user may set 'autoneg' here non-zero to have the
 	 * pause parameters be auto-negotiated too.  In such a case, the
 	 * {rx,tx}_pause values below determine what capabilities are
 	 * advertised.
@@ -284,7 +284,7 @@ enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
 	ETH_SS_STATS,
 	ETH_SS_PRIV_FLAGS,
-	ETH_SS_NTUPLE_FILTERS,
+	ETH_SS_NTUPLE_FILTERS,	/* Do not use, GRXNTUPLE is now deprecated */
 	ETH_SS_FEATURES,
 };
 
@@ -307,9 +307,21 @@ struct ethtool_sset_info {
 				   __u32's, etc. */
 };
 
+/**
+ * enum ethtool_test_flags - flags definition of ethtool_test
+ * @ETH_TEST_FL_OFFLINE: if set perform online and offline tests, otherwise
+ *	only online tests.
+ * @ETH_TEST_FL_FAILED: Driver set this flag if test fails.
+ * @ETH_TEST_FL_EXTERNAL_LB: Application request to perform external loopback
+ *	test.
+ * @ETH_TEST_FL_EXTERNAL_LB_DONE: Driver performed the external loopback test
+ */
+
 enum ethtool_test_flags {
-	ETH_TEST_FL_OFFLINE	= (1 << 0),	/* online / offline */
-	ETH_TEST_FL_FAILED	= (1 << 1),	/* test passed / failed */
+	ETH_TEST_FL_OFFLINE	= (1 << 0),
+	ETH_TEST_FL_FAILED	= (1 << 1),
+	ETH_TEST_FL_EXTERNAL_LB	= (1 << 2),
+	ETH_TEST_FL_EXTERNAL_LB_DONE	= (1 << 3),
 };
 
 /* for requesting NIC test and getting results*/
@@ -739,7 +751,7 @@ enum ethtool_sfeatures_retval_bits {
 #define ETHTOOL_FLASHDEV	0x00000033 /* Flash firmware to device */
 #define ETHTOOL_RESET		0x00000034 /* Reset hardware */
 #define ETHTOOL_SRXNTUPLE	0x00000035 /* Add an n-tuple filter to device */
-#define ETHTOOL_GRXNTUPLE	0x00000036 /* Get n-tuple filters from device */
+#define ETHTOOL_GRXNTUPLE	0x00000036 /* deprecated */
 #define ETHTOOL_GSSET_INFO	0x00000037 /* Get string set info */
 #define ETHTOOL_GRXFHINDIR	0x00000038 /* Get RX flow hash indir'n table */
 #define ETHTOOL_SRXFHINDIR	0x00000039 /* Set RX flow hash indir'n table */
@@ -809,7 +821,7 @@ enum ethtool_sfeatures_retval_bits {
 /* The following are all involved in forcing a particular link
  * mode for the device for setting things.  When getting the
  * devices settings, these indicate the current mode and whether
- * it was foced up into this mode or autonegotiated.
+ * it was forced up into this mode or autonegotiated.
  */
 
 /* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE. */
-- 
1.6.3.3

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

* [PATCH ethtool 2/3] ethtool: add support for external loopback.
  2011-09-20 10:31 [PATCH 0/3] ethtool: Features added Sucheta Chakraborty
  2011-09-20 10:31 ` [PATCH ethtool 1/3] ethtool: file ethtool-copy.h update Sucheta Chakraborty
@ 2011-09-20 10:31 ` Sucheta Chakraborty
  2011-10-04 22:12   ` Ben Hutchings
  2011-09-20 10:31 ` [PATCH ethtool 3/3] ethtool: add ETHTOOL_{G,S}CHANNEL support Sucheta Chakraborty
  2 siblings, 1 reply; 8+ messages in thread
From: Sucheta Chakraborty @ 2011-09-20 10:31 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, Dept_NX_Linux_NIC_Driver

External loopback will be performed in addition to other offline tests.
User need to pass new parameter "external_lb" for the same.

Reqd. man page changes included.

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 ethtool.8.in |   15 +++++++++++----
 ethtool.c    |   12 ++++++++++--
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index 7a0bd43..efc6098 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -68,6 +68,10 @@
 .\"
 .ds HO \fBm\fP|\fBv\fP|\fBt\fP|\fBs\fP|\fBd\fP|\fBf\fP|\fBn\fP|\fBr\fP...
 .\"
+.\"	\(*SD - Self-diag test values
+.\"
+.ds SD \fBoffline\fP|\fBonline\fP|\fBexternal_lb\fP
+.\"
 .\"	\(*NC - Network Classifier type values
 .\"
 .ds NC \fBether\fP|\fBip4\fP|\fBtcp4\fP|\fBudp4\fP|\fBsctp4\fP|\fBah4\fP|\fBesp4\fP
@@ -227,7 +231,7 @@ ethtool \- query or control network driver and hardware settings
 .HP
 .B ethtool \-t|\-\-test
 .I ethX
-.B1 offline online
+.RI [\*(SD]
 .HP
 .B ethtool \-s
 .I ethX
@@ -457,12 +461,14 @@ statistics.
 .B \-t \-\-test
 Executes adapter selftest on the specified network device. Possible test modes are:
 .TP
-.A1 offline online
+.RI \*(SD
 defines test type: 
 .B offline
 (default) means to perform full set of tests possibly causing normal operation interruption during the tests,
 .B online
-means to perform limited set of tests do not interrupting normal adapter operation.
+means to perform limited set of tests do not interrupting normal adapter operation,
+.B external_lb
+means to perform external-loopback test in addition to other offline tests.
 .TP
 .B \-s \-\-change
 Allows changing some or all settings of the specified network device.
@@ -762,7 +768,8 @@ Andre Majorel,
 Eli Kupermann,
 Scott Feldman,
 Andi Kleen,
-Alexander Duyck.
+Alexander Duyck,
+Sucheta Chakraborty.
 .SH AVAILABILITY
 .B ethtool
 is available from
diff --git a/ethtool.c b/ethtool.c
index 943dfb7..d7d2d58 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -219,7 +219,7 @@ static struct option {
     { "-p", "--identify", MODE_PHYS_ID, "Show visible port identification (e.g. blinking)",
                 "               [ TIME-IN-SECONDS ]\n" },
     { "-t", "--test", MODE_TEST, "Execute adapter self test",
-                "               [ online | offline ]\n" },
+		"               [ online | offline | external_lb ]\n" },
     { "-S", "--statistics", MODE_GSTATS, "Show adapter statistics" },
     { "-n", "--show-nfc", MODE_GNFC, "Show Rx network flow classification "
 		"options",
@@ -408,6 +408,7 @@ static struct ethtool_rx_flow_spec rx_rule_fs;
 static enum {
 	ONLINE=0,
 	OFFLINE,
+	EXTERNAL_LB,
 } test_type = OFFLINE;
 
 typedef enum {
@@ -811,6 +812,8 @@ static void parse_cmdline(int argc, char **argp)
 					test_type = ONLINE;
 				} else if (!strcmp(argp[i], "offline")) {
 					test_type = OFFLINE;
+				} else if (!strcmp(argp[i], "external_lb")) {
+					test_type = EXTERNAL_LB;
 				} else {
 					exit_bad_args();
 				}
@@ -1689,6 +1692,9 @@ static int dump_test(struct ethtool_drvinfo *info, struct ethtool_test *test,
 
 	rc = test->flags & ETH_TEST_FL_FAILED;
 	fprintf(stdout, "The test result is %s\n", rc ? "FAIL" : "PASS");
+	fprintf(stdout, "External loopback test is %s\n",
+		test->flags & ETH_TEST_FL_EXTERNAL_LB_DONE ? "executed" :
+							"not executed");
 
 	if (info->testinfo_len)
 		fprintf(stdout, "The test extra info:\n");
@@ -2749,7 +2755,9 @@ static int do_test(int fd, struct ifreq *ifr)
 	memset (test->data, 0, drvinfo.testinfo_len * sizeof(u64));
 	test->cmd = ETHTOOL_TEST;
 	test->len = drvinfo.testinfo_len;
-	if (test_type == OFFLINE)
+	if (test_type == EXTERNAL_LB)
+		test->flags = (ETH_TEST_FL_OFFLINE | ETH_TEST_FL_EXTERNAL_LB);
+	else if (test_type == OFFLINE)
 		test->flags = ETH_TEST_FL_OFFLINE;
 	else
 		test->flags = 0;
-- 
1.6.3.3

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

* [PATCH ethtool 3/3] ethtool: add ETHTOOL_{G,S}CHANNEL support.
  2011-09-20 10:31 [PATCH 0/3] ethtool: Features added Sucheta Chakraborty
  2011-09-20 10:31 ` [PATCH ethtool 1/3] ethtool: file ethtool-copy.h update Sucheta Chakraborty
  2011-09-20 10:31 ` [PATCH ethtool 2/3] ethtool: add support for external loopback Sucheta Chakraborty
@ 2011-09-20 10:31 ` Sucheta Chakraborty
  2011-10-04 22:21   ` Ben Hutchings
  2 siblings, 1 reply; 8+ messages in thread
From: Sucheta Chakraborty @ 2011-09-20 10:31 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, Dept_NX_Linux_NIC_Driver

Used to configure number of rx and tx rings.
Reqd. man page changes are included.

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 ethtool.8.in |   28 ++++++++++++++
 ethtool.c    |  116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+), 0 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index efc6098..64e1b70 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -307,6 +307,16 @@ ethtool \- query or control network driver and hardware settings
 .BN action
 .BN loc
 .RB ]
+.HP
+.B ethtool \-l|\-\-show\-channels
+.I ethX
+.HP
+.B ethtool \-L|\-\-set\-channels
+.I ethX
+.BN rx_count
+.BN tx_count
+.BN other_count
+.BN combined_count
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -754,6 +764,24 @@ lB	l.
 Specify the location/ID to insert the rule. This will overwrite
 any rule present in that location and will not go through any
 of the rule ordering process.
+.TP
+.B \-l \-\-show\-channels
+Queries the specified network device for channel parameter information.
+.TP
+.B \-L \-\-set\-channels
+Changes the channel parameters of the specified network device.
+.TP
+.BI rx_count \ N
+Changes the number of rx channels.
+.TP
+.BI tx_count \ N
+Changes the number of tx channels.
+.TP
+.BI other_count \ N
+Changes the number of other channels. Eg: link interrupts, SRIOV co-ordination etc.
+.TP
+.BI combined_count \ N
+Changes set of channel (RX, TX or other).
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index d7d2d58..79c37e4 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -80,6 +80,8 @@ static int do_gpause(int fd, struct ifreq *ifr);
 static int do_spause(int fd, struct ifreq *ifr);
 static int do_gring(int fd, struct ifreq *ifr);
 static int do_sring(int fd, struct ifreq *ifr);
+static int do_schannels(int fd, struct ifreq *ifr);
+static int do_gchannels(int fd, struct ifreq *ifr);
 static int do_gcoalesce(int fd, struct ifreq *ifr);
 static int do_scoalesce(int fd, struct ifreq *ifr);
 static int do_goffload(int fd, struct ifreq *ifr);
@@ -133,6 +135,8 @@ static enum {
 	MODE_PERMADDR,
 	MODE_SET_DUMP,
 	MODE_GET_DUMP,
+	MODE_GCHANNELS,
+	MODE_SCHANNELS
 } mode = MODE_GSET;
 
 static struct option {
@@ -266,6 +270,12 @@ static struct option {
     { "-W", "--set-dump", MODE_SET_DUMP,
 		"Set dump flag of the device",
 		"		N\n"},
+    { "-l", "--show-channels", MODE_GCHANNELS, "Query Channels" },
+    { "-L", "--set-channels", MODE_SCHANNELS, "Set Channels",
+		"               [ rx_count N ]\n"
+		"               [ tx_count N ]\n"
+		"               [ other_count N ]\n"
+		"               [ combined_count N ]\n" },
     { "-h", "--help", MODE_HELP, "Show this help" },
     { NULL, "--version", MODE_VERSION, "Show version number" },
     {}
@@ -331,6 +341,13 @@ static s32 ring_rx_mini_wanted = -1;
 static s32 ring_rx_jumbo_wanted = -1;
 static s32 ring_tx_wanted = -1;
 
+static struct ethtool_channels echannels;
+static int gchannels_changed;
+static s32 channels_rx_wanted = -1;
+static s32 channels_tx_wanted = -1;
+static s32 channels_other_wanted = -1;
+static s32 channels_combined_wanted = -1;
+
 static struct ethtool_coalesce ecoal;
 static int gcoalesce_changed = 0;
 static s32 coal_stats_wanted = -1;
@@ -495,6 +512,13 @@ static struct cmdline_info cmdline_ring[] = {
 	{ "tx", CMDL_S32, &ring_tx_wanted, &ering.tx_pending },
 };
 
+static struct cmdline_info cmdline_channels[] = {
+	{ "rx_count", CMDL_S32, &channels_rx_wanted, &echannels.rx_count },
+	{ "tx_count", CMDL_S32, &channels_tx_wanted, &echannels.tx_count },
+	{ "other_count", CMDL_S32, &channels_other_wanted, &echannels.other_count },
+	{ "combined_count", CMDL_S32, &channels_combined_wanted, &echannels.combined_count },
+};
+
 static struct cmdline_info cmdline_coalesce[] = {
 	{ "adaptive-rx", CMDL_BOOL, &coal_adaptive_rx_wanted, &ecoal.use_adaptive_rx_coalesce },
 	{ "adaptive-tx", CMDL_BOOL, &coal_adaptive_tx_wanted, &ecoal.use_adaptive_tx_coalesce },
@@ -787,6 +811,8 @@ static void parse_cmdline(int argc, char **argp)
 			    (mode == MODE_GCOALESCE) ||
 			    (mode == MODE_SCOALESCE) ||
 			    (mode == MODE_GRING) ||
+			    (mode == MODE_GCHANNELS) ||
+			    (mode == MODE_SCHANNELS) ||
 			    (mode == MODE_SRING) ||
 			    (mode == MODE_GOFFLOAD) ||
 			    (mode == MODE_SOFFLOAD) ||
@@ -871,6 +897,14 @@ static void parse_cmdline(int argc, char **argp)
 				i = argc;
 				break;
 			}
+			if (mode == MODE_SCHANNELS) {
+				parse_generic_cmdline(argc, argp, i,
+					&gchannels_changed,
+					cmdline_channels,
+					ARRAY_SIZE(cmdline_ring));
+				i = argc;
+				break;
+			}
 			if (mode == MODE_SCOALESCE) {
 				parse_generic_cmdline(argc, argp, i,
 					&gcoalesce_changed,
@@ -1751,6 +1785,32 @@ static int dump_ring(void)
 	return 0;
 }
 
+static int dump_channels(void)
+{
+	fprintf(stdout,
+		"Re-set maximums:\n"
+		"Max RX:		%u\n"
+		"Max TX:		%u\n"
+		"Max Other:	%u\n"
+		"Max Combined:	%u\n",
+		echannels.max_rx, echannels.max_tx,
+		echannels.max_other,
+		echannels.max_combined);
+
+	fprintf(stdout,
+		"Current hardware settings:\n"
+		"RX Count:	%u\n"
+		"TX Count:	%u\n"
+		"Other Count:	%u\n"
+		"Combined Count:	%u\n",
+		echannels.rx_count, echannels.tx_count,
+		echannels.other_count,
+		echannels.combined_count);
+
+	fprintf(stdout, "\n");
+	return 0;
+}
+
 static int dump_coalesce(void)
 {
 	fprintf(stdout, "Adaptive RX: %s  TX: %s\n",
@@ -1937,6 +1997,10 @@ static int doit(void)
 		return do_gring(fd, &ifr);
 	} else if (mode == MODE_SRING) {
 		return do_sring(fd, &ifr);
+	} else if (mode == MODE_GCHANNELS) {
+		return do_gchannels(fd, &ifr);
+	} else if (mode == MODE_SCHANNELS) {
+		return do_schannels(fd, &ifr);
 	} else if (mode == MODE_GOFFLOAD) {
 		return do_goffload(fd, &ifr);
 	} else if (mode == MODE_SOFFLOAD) {
@@ -2114,6 +2178,58 @@ static int do_gring(int fd, struct ifreq *ifr)
 	return 0;
 }
 
+static int do_schannels(int fd, struct ifreq *ifr)
+{
+	int err, changed = 0;
+
+	echannels.cmd = ETHTOOL_GCHANNELS;
+	ifr->ifr_data = (caddr_t)&echannels;
+	err = send_ioctl(fd, ifr);
+	if (err) {
+		perror("Cannot get device channels settings");
+		return 1;
+	}
+
+	do_generic_set(cmdline_channels, ARRAY_SIZE(cmdline_channels),
+			&changed);
+
+	if (!changed) {
+		fprintf(stderr, "no channels parameters changed, aborting\n");
+		return 1;
+	}
+
+	echannels.cmd = ETHTOOL_SCHANNELS;
+	ifr->ifr_data = (caddr_t)&echannels;
+	err = send_ioctl(fd, ifr);
+	if (err) {
+		perror("Cannot set device channels parameters");
+		return 1;
+	}
+
+	return 0;
+}
+
+static int do_gchannels(int fd, struct ifreq *ifr)
+{
+	int err;
+
+	fprintf(stdout, "Channels parameters for %s:\n", devname);
+
+	echannels.cmd = ETHTOOL_GCHANNELS;
+	ifr->ifr_data = (caddr_t)&echannels;
+	err = send_ioctl(fd, ifr);
+	if (err == 0) {
+		err = dump_channels();
+		if (err)
+			return err;
+	} else {
+		perror("Cannot get device channels\n");
+		return 1;
+	}
+	return 0;
+
+}
+
 static int do_gcoalesce(int fd, struct ifreq *ifr)
 {
 	int err;
-- 
1.6.3.3

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

* Re: [PATCH ethtool 1/3] ethtool: file ethtool-copy.h update.
  2011-09-20 10:31 ` [PATCH ethtool 1/3] ethtool: file ethtool-copy.h update Sucheta Chakraborty
@ 2011-10-04 22:09   ` Ben Hutchings
  0 siblings, 0 replies; 8+ messages in thread
From: Ben Hutchings @ 2011-10-04 22:09 UTC (permalink / raw)
  To: Sucheta Chakraborty; +Cc: netdev, Dept_NX_Linux_NIC_Driver

On Tue, 2011-09-20 at 03:31 -0700, Sucheta Chakraborty wrote:
> Used linux-3.0.1-net-next tree for the update.
[...]

I'll update ethtool-copy.h using the current net-next.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

* Re: [PATCH ethtool 2/3] ethtool: add support for external loopback.
  2011-09-20 10:31 ` [PATCH ethtool 2/3] ethtool: add support for external loopback Sucheta Chakraborty
@ 2011-10-04 22:12   ` Ben Hutchings
  0 siblings, 0 replies; 8+ messages in thread
From: Ben Hutchings @ 2011-10-04 22:12 UTC (permalink / raw)
  To: Sucheta Chakraborty; +Cc: netdev, Dept_NX_Linux_NIC_Driver

On Tue, 2011-09-20 at 03:31 -0700, Sucheta Chakraborty wrote:
> External loopback will be performed in addition to other offline tests.
> User need to pass new parameter "external_lb" for the same.
> 
> Reqd. man page changes included.

Applied, but:

[...]
> @@ -1689,6 +1692,9 @@ static int dump_test(struct ethtool_drvinfo *info, struct ethtool_test *test,
>  
>  	rc = test->flags & ETH_TEST_FL_FAILED;
>  	fprintf(stdout, "The test result is %s\n", rc ? "FAIL" : "PASS");
> +	fprintf(stdout, "External loopback test is %s\n",
> +		test->flags & ETH_TEST_FL_EXTERNAL_LB_DONE ? "executed" :
> +							"not executed");
>  
>  	if (info->testinfo_len)
>  		fprintf(stdout, "The test extra info:\n");

1. This message should say 'was', not 'is'.

2. There are undoubtedly scripts that parse the output of ethtool -t.
We might break them if we add a line to the output of existing commands.
I've made this conditional on test_type == EXTERNAL_LB.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

* Re: [PATCH ethtool 3/3] ethtool: add ETHTOOL_{G,S}CHANNEL support.
  2011-09-20 10:31 ` [PATCH ethtool 3/3] ethtool: add ETHTOOL_{G,S}CHANNEL support Sucheta Chakraborty
@ 2011-10-04 22:21   ` Ben Hutchings
  2011-10-04 22:36     ` Ben Hutchings
  0 siblings, 1 reply; 8+ messages in thread
From: Ben Hutchings @ 2011-10-04 22:21 UTC (permalink / raw)
  To: Sucheta Chakraborty; +Cc: netdev, Dept_NX_Linux_NIC_Driver

On Tue, 2011-09-20 at 03:31 -0700, Sucheta Chakraborty wrote:
> Used to configure number of rx and tx rings.
> Reqd. man page changes are included.
[...]
> @@ -754,6 +764,24 @@ lB	l.
>  Specify the location/ID to insert the rule. This will overwrite
>  any rule present in that location and will not go through any
>  of the rule ordering process.
> +.TP
> +.B \-l \-\-show\-channels
> +Queries the specified network device for channel parameter information.
> +.TP
> +.B \-L \-\-set\-channels
> +Changes the channel parameters of the specified network device.

I think the manual page needs to explain briefly what is meant by a
channel.  (So should ethtool.h, really!)

[...]
> @@ -495,6 +512,13 @@ static struct cmdline_info cmdline_ring[] = {
>  	{ "tx", CMDL_S32, &ring_tx_wanted, &ering.tx_pending },
>  };
>  
> +static struct cmdline_info cmdline_channels[] = {
> +	{ "rx_count", CMDL_S32, &channels_rx_wanted, &echannels.rx_count },
> +	{ "tx_count", CMDL_S32, &channels_tx_wanted, &echannels.tx_count },
> +	{ "other_count", CMDL_S32, &channels_other_wanted, &echannels.other_count },
> +	{ "combined_count", CMDL_S32, &channels_combined_wanted, &echannels.combined_count },
> +};

I don't think it's necessary to include '_count' in these keywords.

[...]
> @@ -1751,6 +1785,32 @@ static int dump_ring(void)
>  	return 0;
>  }
>  
> +static int dump_channels(void)
> +{
> +	fprintf(stdout,
> +		"Re-set maximums:\n"

Should 'Re-set' be 'Pre-set'?

> +		"Max RX:		%u\n"
> +		"Max TX:		%u\n"
> +		"Max Other:	%u\n"
> +		"Max Combined:	%u\n",
> +		echannels.max_rx, echannels.max_tx,
> +		echannels.max_other,
> +		echannels.max_combined);

The heading says they are maximums, so I don't think it's necessary to
include 'Max' on each line.

> +	fprintf(stdout,
> +		"Current hardware settings:\n"
> +		"RX Count:	%u\n"
> +		"TX Count:	%u\n"
> +		"Other Count:	%u\n"
> +		"Combined Count:	%u\n",

I don't think it's necessary to include 'Count' on each line here,
either.

[...]
> @@ -2114,6 +2178,58 @@ static int do_gring(int fd, struct ifreq *ifr)
>  	return 0;
>  }
>  
> +static int do_schannels(int fd, struct ifreq *ifr)
> +{
> +	int err, changed = 0;
> +
> +	echannels.cmd = ETHTOOL_GCHANNELS;
> +	ifr->ifr_data = (caddr_t)&echannels;
> +	err = send_ioctl(fd, ifr);
> +	if (err) {
> +		perror("Cannot get device channels settings");
> +		return 1;
> +	}
> +
> +	do_generic_set(cmdline_channels, ARRAY_SIZE(cmdline_channels),
> +			&changed);
> +
> +	if (!changed) {
> +		fprintf(stderr, "no channels parameters changed, aborting\n");
> +		return 1;
> +	}
[...]

These messages (and others below) are not grammatical.  They should say
'channel settings' or 'channel parameters'.

Actually, you could be more specific about what the parameters are, and
just write 'numbers of channels'.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

* Re: [PATCH ethtool 3/3] ethtool: add ETHTOOL_{G,S}CHANNEL support.
  2011-10-04 22:21   ` Ben Hutchings
@ 2011-10-04 22:36     ` Ben Hutchings
  0 siblings, 0 replies; 8+ messages in thread
From: Ben Hutchings @ 2011-10-04 22:36 UTC (permalink / raw)
  To: Sucheta Chakraborty; +Cc: netdev, Dept_NX_Linux_NIC_Driver

On Tue, 2011-10-04 at 23:21 +0100, Ben Hutchings wrote:
> On Tue, 2011-09-20 at 03:31 -0700, Sucheta Chakraborty wrote:
> > Used to configure number of rx and tx rings.
> > Reqd. man page changes are included.
> [...]
> > @@ -754,6 +764,24 @@ lB	l.
> >  Specify the location/ID to insert the rule. This will overwrite
> >  any rule present in that location and will not go through any
> >  of the rule ordering process.
> > +.TP
> > +.B \-l \-\-show\-channels
> > +Queries the specified network device for channel parameter information.
> > +.TP
> > +.B \-L \-\-set\-channels
> > +Changes the channel parameters of the specified network device.
> 
> I think the manual page needs to explain briefly what is meant by a
> channel.  (So should ethtool.h, really!)
[...]

Perhaps something like this:

.TP
.B \-l \-\-show\-channels
Queries the specified network device for the numbers of channels it has.
A channel is an IRQ and the set of queues that can trigger that IRQ.
.TP
.B \-L \-\-set\-channels
Changes the numbers of channels of the specified network device.
.TP
.BI rx \ N
Changes the number of channels with only receive queues.
.TP
.BI tx \ N
Changes the number of channels with only transmit queues.
.TP
.BI other \ N
Changes the number of channels used only for other purposes e.g. link
interrupts or SR-IOV co-ordination.
.TP
.BI combined \ N
Changes the number of multi-purpose channels.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

end of thread, other threads:[~2011-10-04 22:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-20 10:31 [PATCH 0/3] ethtool: Features added Sucheta Chakraborty
2011-09-20 10:31 ` [PATCH ethtool 1/3] ethtool: file ethtool-copy.h update Sucheta Chakraborty
2011-10-04 22:09   ` Ben Hutchings
2011-09-20 10:31 ` [PATCH ethtool 2/3] ethtool: add support for external loopback Sucheta Chakraborty
2011-10-04 22:12   ` Ben Hutchings
2011-09-20 10:31 ` [PATCH ethtool 3/3] ethtool: add ETHTOOL_{G,S}CHANNEL support Sucheta Chakraborty
2011-10-04 22:21   ` Ben Hutchings
2011-10-04 22:36     ` Ben Hutchings

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).