[PATCH ethtool 3/5] ethtool: Add support for named flags

Previous thread: [PATCH] ip: correctly report 802.15.4 link type by Jan Engelhardt on Friday, June 25, 2010 - 7:01 am. (1 message)

Next thread: dhclient, checksum and tap by Michael S. Tsirkin on Friday, June 25, 2010 - 8:10 am. (6 messages)
From: Ben Hutchings
Date: Friday, June 25, 2010 - 7:43 am

This patch series fixes the initialisation of RX n-tuple specs, adds
general support for named flags, and then uses that for message types
and extended offload flags.

Ben.

Ben Hutchings (5):
  ethtool: Parse integers into variables of different sizes and byte
    orders
  ethtool: Mark show_usage() as noreturn
  ethtool: Add support for named flags
  ethtool: Implement named message type flags
  ethtool: Use named flag support to update extended offload flags

 ethtool-util.h |   29 ++++-
 ethtool.8      |   66 ++++++++-
 ethtool.c      |  440 +++++++++++++++++++++++++++++++++++--------------------
 3 files changed, 368 insertions(+), 167 deletions(-)

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

--

From: Ben Hutchings
Date: Friday, June 25, 2010 - 7:46 am

The arguments for RX n-tuple traffic direction are filled into
structure fields of varying size, some of which are in big-endian
rather than native byte order.  Currently parse_generic_cmdline()
only supports 32-bit integers in native byte order, so this does
not work correctly.

Replace CMDL_INT and CMDL_UINT with the more explicit CMDL_S32 and
CMDL_U32.  Add CMDL_U16 and CMDL_U64 for narrower and wider integers,
and CMDL_BE16 and CMDL_BE32 for big-endian unsigned integers.  Use
them for RX n-tuple argument parsing.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
I've removed the 'historical' comment from the kernel-style named types
because I see no sign of a transition away from them.

Ben.

 ethtool-util.h |   29 ++++++-
 ethtool.c      |  226 +++++++++++++++++++++++++++++++++-----------------------
 2 files changed, 160 insertions(+), 95 deletions(-)

diff --git a/ethtool-util.h b/ethtool-util.h
index e9a998a..01b1d03 100644
--- a/ethtool-util.h
+++ b/ethtool-util.h
@@ -4,14 +4,35 @@
 #define ETHTOOL_UTIL_H__
 
 #include <sys/types.h>
+#include <endian.h>
 
 #include "ethtool-copy.h"
 
-/* historical: we used to use kernel-like types; remove these once cleaned */
 typedef unsigned long long u64;
-typedef __uint32_t u32;         /* ditto */
-typedef __uint16_t u16;         /* ditto */
-typedef __uint8_t u8;           /* ditto */
+typedef __uint32_t u32;
+typedef __uint16_t u16;
+typedef __uint8_t u8;
+typedef __int32_t s32;
+
+#if __BYTE_ORDER == __BIG_ENDIAN
+static inline u16 cpu_to_be16(u16 value)
+{
+    return value;
+}
+static inline u32 cpu_to_be32(u32 value)
+{
+    return value;
+}
+#else
+static inline u16 cpu_to_be16(u16 value)
+{
+	return (value >> 8) | (value << 8);
+}
+static inline u32 cpu_to_be32(u32 value)
+{
+	return cpu_to_be16(value >> 16) | (cpu_to_be16(value) << 16);
+}
+#endif
 
 /* National Semiconductor DP83815, DP83816 */
 int natsemi_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs ...
From: Ben Hutchings
Date: Friday, June 25, 2010 - 7:48 am

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
This prevents the next patch from provoking an incorrect warning about
an uninitialised variable.

Ben.

 ethtool.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 8969390..d4bf5a8 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -220,6 +220,8 @@ static struct option {
 };
 

+static void show_usage(int badarg) __attribute__((noreturn));
+
 static void show_usage(int badarg)
 {
 	int i;
-- 
1.6.2.5


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

--

From: Ben Hutchings
Date: Friday, June 25, 2010 - 7:49 am

Define an argument type CMDL_FLAG, which can be used to set and
clear flags.  For each setting that can be modified in this way,
the flags to be set and cleared are accumulated in two variables.

Add support for CMDL_FLAGS parse_generic_cmdline().

Add utility function print_flags().

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
print_flags() is unused here, but will be used in the next patch.

Ben.

 ethtool.c |   40 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index d4bf5a8..4adab4b 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -352,14 +352,21 @@ typedef enum {
 	CMDL_BE16,
 	CMDL_BE32,
 	CMDL_STR,
+	CMDL_FLAG,
 } cmdline_type_t;
 
 struct cmdline_info {
 	const char *name;
 	cmdline_type_t type;
-	/* Points to int (BOOL), s32, u16, u32, u64 or char * (STR) */
+	/* Points to int (BOOL), s32, u16, u32 (U32/FLAG), u64 or
+	 * char * (STR).  For FLAG, the value accumulates all flags
+	 * to be set. */
 	void *wanted_val;
 	void *ioctl_val;
+	/* For FLAG, the flag value to be set/cleared */
+	u32 flag_val;
+	/* For FLAG, accumulates all flags to be cleared */
+	u32 *unwanted_val;
 };
 
 static struct cmdline_info cmdline_gregs[] = {
@@ -550,6 +557,17 @@ static void parse_generic_cmdline(int argc, char **argp,
 							       0xffffffff));
 					break;
 				}
+				case CMDL_FLAG: {
+					u32 *p;
+					if (!strcmp(argp[i], "on"))
+						p = info[idx].wanted_val;
+					else if (!strcmp(argp[i], "off"))
+						p = info[idx].unwanted_val;
+					else
+						show_usage(1);
+					*p |= info[idx].flag_val;
+					break;
+				}
 				case CMDL_STR: {
 					char **s = info[idx].wanted_val;
 					*s = strdup(argp[i]);
@@ -566,6 +584,26 @@ static void parse_generic_cmdline(int argc, char **argp,
 	}
 }
 
+static void
+print_flags(const struct cmdline_info *info, unsigned int n_info, u32 value)
+{
+	const char *sep = "";
+
+	while (n_info) {
+		if (info->type == ...
From: Ben Hutchings
Date: Friday, June 25, 2010 - 7:49 am

Allow message type flags to be turned on and off by name.
Print the names of the currently set flags below the numeric value.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool.8 |   66 +++++++++++++++++++++++++++++++++++++--
 ethtool.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 156 insertions(+), 14 deletions(-)

diff --git a/ethtool.8 b/ethtool.8
index a7b43d5..5983d0e 100644
--- a/ethtool.8
+++ b/ethtool.8
@@ -200,7 +200,10 @@ ethtool \- Display or change ethernet card settings
 .RB [ wol \ \*(WO]
 .RB [ sopass \ \*(MA]
 .RB [ msglvl
-.IR N ]
+.IR N \ |
+.BI msglvl \ type
+.A1 on off
+.RB ...]
 
 .B ethtool \-n
 .I ethX
@@ -482,9 +485,66 @@ Disable (wake on nothing).  This option clears all previous options.
 .B sopass \*(MA\c
 Sets the SecureOn(tm) password.  The argument to this option must be 6
 bytes in ethernet MAC hex format (\*(MA).
-.TP
+.PP
 .BI msglvl \ N
-Sets the driver message level. Meanings differ per driver.
+.br
+.BI msglvl \ type
+.A1 on off
+.RB ...
+.RS
+Sets the driver message type flags by name or number. \fItype\fR
+names the type of message to enable or disable; \fIN\fR specifies the
+new flags numerically. The defined type names and numbers are:
+.PD 0
+.TP 12
+.B drv
+0x0001  General driver status
+.TP 12
+.B probe
+0x0002  Hardware probing
+.TP 12
+.B link
+0x0004  Link state
+.TP 12
+.B timer
+0x0008  Periodic status check
+.TP 12
+.B ifdown
+0x0010  Interface being brought down
+.TP 12
+.B ifup
+0x0020  Interface being brought up
+.TP 12
+.B rx_err
+0x0040  Receive error
+.TP 12
+.B tx_err
+0x0080  Transmit error
+.TP 12
+.B tx_queued
+0x0100  Transmit queueing
+.TP 12
+.B intr
+0x0200  Interrupt handling
+.TP 12
+.B tx_done
+0x0400  Transmit completion
+.TP 12
+.B rx_status
+0x0800  Receive completion
+.TP 12
+.B pktdata
+0x1000  Packet contents
+.TP 12
+.B hw
+0x2000  Hardware status
+.TP 12
+.B wol
+0x4000  Wake-on-LAN ...
From: Jeff Garzik
Date: Friday, June 25, 2010 - 10:11 am

A nice improvement over one-variable-per-bit, I think.

applied patches 1-5, thanks!

	Jeff



P.S.  If you could use my open source email address (jgarzik@pobox.com 
or jeff@garzik.org), that would be greatly appreciated.  @redhat.com is 
only used for sign-offs, as it's less portable than !@redhat.com addresses.

--

From: Ben Hutchings
Date: Friday, June 25, 2010 - 7:50 am

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool.c |   70 ++++++++++--------------------------------------------------
 1 files changed, 12 insertions(+), 58 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index a70cd03..4073745 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -277,10 +277,9 @@ static int off_sg_wanted = -1;
 static int off_tso_wanted = -1;
 static int off_ufo_wanted = -1;
 static int off_gso_wanted = -1;
-static int off_lro_wanted = -1;
+static u32 off_flags_wanted = 0;
+static u32 off_flags_unwanted = 0;
 static int off_gro_wanted = -1;
-static int off_ntuple_wanted = -1;
-static int off_rxhash_wanted = -1;
 
 static struct ethtool_pauseparam epause;
 static int gpause_changed = 0;
@@ -419,10 +418,13 @@ static struct cmdline_info cmdline_offload[] = {
 	{ "tso", CMDL_BOOL, &off_tso_wanted, NULL },
 	{ "ufo", CMDL_BOOL, &off_ufo_wanted, NULL },
 	{ "gso", CMDL_BOOL, &off_gso_wanted, NULL },
-	{ "lro", CMDL_BOOL, &off_lro_wanted, NULL },
+	{ "lro", CMDL_FLAG, &off_flags_wanted, NULL,
+	  ETH_FLAG_LRO, &off_flags_unwanted },
 	{ "gro", CMDL_BOOL, &off_gro_wanted, NULL },
-	{ "ntuple", CMDL_BOOL, &off_ntuple_wanted, NULL },
-	{ "rxhash", CMDL_BOOL, &off_rxhash_wanted, NULL },
+	{ "ntuple", CMDL_FLAG, &off_flags_wanted, NULL,
+	  ETH_FLAG_NTUPLE, &off_flags_unwanted },
+	{ "rxhash", CMDL_FLAG, &off_flags_wanted, NULL,
+	  ETH_FLAG_RXHASH, &off_flags_unwanted },
 };
 
 static struct cmdline_info cmdline_pause[] = {
@@ -2191,7 +2193,7 @@ static int do_soffload(int fd, struct ifreq *ifr)
 			return 90;
 		}
 	}
-	if (off_lro_wanted >= 0) {
+	if (off_flags_wanted || off_flags_unwanted) {
 		changed = 1;
 		eval.cmd = ETHTOOL_GFLAGS;
 		eval.data = 0;
@@ -2203,14 +2205,12 @@ static int do_soffload(int fd, struct ifreq *ifr)
 		}
 
 		eval.cmd = ETHTOOL_SFLAGS;
-		if (off_lro_wanted == 1)
-			eval.data |= ETH_FLAG_LRO;
-		else
-			eval.data &= ~ETH_FLAG_LRO;
+		eval.data = ((eval.data & ~off_flags_unwanted) |
+			     ...
Previous thread: [PATCH] ip: correctly report 802.15.4 link type by Jan Engelhardt on Friday, June 25, 2010 - 7:01 am. (1 message)

Next thread: dhclient, checksum and tap by Michael S. Tsirkin on Friday, June 25, 2010 - 8:10 am. (6 messages)