Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next-2.6 0/4] More sfc changes for 2.6.36
From: Ben Hutchings @ 2010-06-25 17:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers

Dave,

You appear to have applied all my previous patches to net-next-2.6, so
here are the fix-ups for hash insertion.  Also, an improvement in
hardware monitoring which has been in the the out-of-tree sfc driver for
a while but which I failed to carry over.

Ben.

Ben Hutchings (4):
  sfc: Fix reading of inserted hash
  sfc: Move siena_nic_data::ipv6_rss_key to efx_nic::rx_hash_key
  sfc: Use Toeplitz IPv4 hash for RSS and hash insertion
  sfc: Log clearer error messages for hardware monitor

 drivers/net/sfc/efx.c           |    3 +++
 drivers/net/sfc/falcon.c        |   14 ++++++++++++--
 drivers/net/sfc/falcon_boards.c |   11 ++++++++---
 drivers/net/sfc/net_driver.h    |    1 +
 drivers/net/sfc/nic.h           |    1 -
 drivers/net/sfc/rx.c            |   11 ++++++-----
 drivers/net/sfc/selftest.c      |    3 ---
 drivers/net/sfc/siena.c         |   23 ++++++++++++++---------
 8 files changed, 44 insertions(+), 23 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.


^ permalink raw reply

* dhclient, checksum and tap
From: Michael S. Tsirkin @ 2010-06-25 15:10 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev

Hi!
I've been looking at the issue of checksum on
dhcp packets: to recap, 8dc4194474159660d7f37c495e3fc3f10d0db8cc
added a way for af_packet sockets to get the packet status.
Unfortunately not all dhcp clients caught up with
this development, so they are still broken
when both server and client run on the same host,
e.g. with bridge+tap.

And of course virtualization adds another way to run
old dhcp clients, so userspace virtio net in qemu has
a hack to detect DHCP and fill in the checksum.
I guess we could add this in vhost, as well.

However, one wonders whether the tap driver is a better place
for this work-around, that would help all users.
Any objections against putting such code in tap?

-- 
MST

^ permalink raw reply

* [PATCH ethtool 5/5] ethtool: Use named flag support to update extended offload flags
From: Ben Hutchings @ 2010-06-25 14:50 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, linux-net-drivers
In-Reply-To: <1277477027.2094.2.camel@achroite.uk.solarflarecom.com>

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) |
+			     off_flags_wanted);
 
 		err = ioctl(fd, SIOCETHTOOL, ifr);
 		if (err) {
-			perror("Cannot set large receive offload settings");
+			perror("Cannot set device flag settings");
 			return 92;
 		}
 	}
@@ -2225,52 +2225,6 @@ static int do_soffload(int fd, struct ifreq *ifr)
 			return 93;
 		}
 	}
-	if (off_ntuple_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_GFLAGS;
-		eval.data = 0;
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot get device flag settings");
-			return 91;
-		}
-
-		eval.cmd = ETHTOOL_SFLAGS;
-		if (off_ntuple_wanted == 1)
-			eval.data |= ETH_FLAG_NTUPLE;
-		else
-			eval.data &= ~ETH_FLAG_NTUPLE;
-
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set n-tuple filter settings");
-			return 93;
-		}
-	}
-	if (off_rxhash_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_GFLAGS;
-		eval.data = 0;
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot get device flag settings");
-			return 91;
-		}
-
-		eval.cmd = ETHTOOL_SFLAGS;
-		if (off_rxhash_wanted)
-			eval.data |= ETH_FLAG_RXHASH;
-		else
-			eval.data &= ~ETH_FLAG_RXHASH;
-
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set receive hash settings");
-			return 93;
-		}
-	}
 
 	if (!changed) {
 		fprintf(stdout, "no offload settings changed\n");
-- 
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.


^ permalink raw reply related

* [PATCH ethtool 4/5] ethtool: Implement named message type flags
From: Ben Hutchings @ 2010-06-25 14:49 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, linux-net-drivers
In-Reply-To: <1277477027.2094.2.camel@achroite.uk.solarflarecom.com>

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 status
+.PP
+The precise meanings of these type flags differ between drivers.
+.PD
+.RE
 .TP
 .B \-n \-\-show-nfc
 Retrieves the receive network flow classification configurations.
diff --git a/ethtool.c b/ethtool.c
index 4adab4b..a70cd03 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -20,7 +20,6 @@
  *   * better man page (steal from mii-tool?)
  *   * fall back on SIOCMII* ioctl()s and possibly SIOCDEVPRIVATE*
  *   * abstract ioctls to allow for fallback modes of data gathering
- *   * symbolic names for msglvl bitmask
  */
 
 #ifdef HAVE_CONFIG_H
@@ -39,6 +38,7 @@
 #include <net/if.h>
 #include <sys/utsname.h>
 #include <limits.h>
+#include <ctype.h>
 
 #include <linux/sockios.h>
 #include "ethtool-util.h"
@@ -51,6 +51,26 @@
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 #endif
 
+#ifndef HAVE_NETIF_MSG
+enum {
+	NETIF_MSG_DRV		= 0x0001,
+	NETIF_MSG_PROBE		= 0x0002,
+	NETIF_MSG_LINK		= 0x0004,
+	NETIF_MSG_TIMER		= 0x0008,
+	NETIF_MSG_IFDOWN	= 0x0010,
+	NETIF_MSG_IFUP		= 0x0020,
+	NETIF_MSG_RX_ERR	= 0x0040,
+	NETIF_MSG_TX_ERR	= 0x0080,
+	NETIF_MSG_TX_QUEUED	= 0x0100,
+	NETIF_MSG_INTR		= 0x0200,
+	NETIF_MSG_TX_DONE	= 0x0400,
+	NETIF_MSG_RX_STATUS	= 0x0800,
+	NETIF_MSG_PKTDATA	= 0x1000,
+	NETIF_MSG_HW		= 0x2000,
+	NETIF_MSG_WOL		= 0x4000,
+};
+#endif
+
 static int parse_wolopts(char *optstr, u32 *data);
 static char *unparse_wolopts(int wolopts);
 static int parse_sopass(char *src, unsigned char *dest);
@@ -126,7 +146,7 @@ static struct option {
 		"		[ xcvr internal|external ]\n"
 		"		[ wol p|u|m|b|a|g|s|d... ]\n"
 		"		[ sopass %x:%x:%x:%x:%x:%x ]\n"
-		"		[ msglvl %d ] \n" },
+		"		[ msglvl %d | msglvl type on|off ... ]\n" },
     { "-a", "--show-pause", MODE_GPAUSE, "Show pause options" },
     { "-A", "--pause", MODE_SPAUSE, "Set pause options",
       "		[ autoneg on|off ]\n"
@@ -313,7 +333,6 @@ static int wol_change = 0;
 static u8 sopass_wanted[SOPASS_MAX];
 static int sopass_change = 0;
 static int gwol_changed = 0; /* did anything in GWOL change? */
-static int msglvl_wanted = -1;
 static int phys_id_time = 0;
 static int gregs_changed = 0;
 static int gregs_dump_raw = 0;
@@ -337,6 +356,11 @@ static struct ethtool_rx_ntuple_flow_spec ntuple_fs;
 static char *flash_file = NULL;
 static int flash = -1;
 static int flash_region = -1;
+
+static int msglvl_changed;
+static u32 msglvl_wanted = 0;
+static u32 msglvl_unwanted =0;
+
 static enum {
 	ONLINE=0,
 	OFFLINE,
@@ -455,6 +479,39 @@ static struct cmdline_info cmdline_ntuple[] = {
 	{ "action", CMDL_S32, &ntuple_fs.action, NULL },
 };
 
+static struct cmdline_info cmdline_msglvl[] = {
+	{ "drv", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_DRV, &msglvl_unwanted },
+	{ "probe", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_PROBE, &msglvl_unwanted },
+	{ "link", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_LINK, &msglvl_unwanted },
+	{ "timer", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_TIMER, &msglvl_unwanted },
+	{ "ifdown", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_IFDOWN, &msglvl_unwanted },
+	{ "ifup", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_IFUP, &msglvl_unwanted },
+	{ "rx_err", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_RX_ERR, &msglvl_unwanted },
+	{ "tx_err", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_TX_ERR, &msglvl_unwanted },
+	{ "tx_queued", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_TX_QUEUED, &msglvl_unwanted },
+	{ "intr", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_INTR, &msglvl_unwanted },
+	{ "tx_done", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_TX_DONE, &msglvl_unwanted },
+	{ "rx_status", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_RX_STATUS, &msglvl_unwanted },
+	{ "pktdata", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_PKTDATA, &msglvl_unwanted },
+	{ "hw", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_HW, &msglvl_unwanted },
+	{ "wol", CMDL_FLAG, &msglvl_wanted, NULL,
+	  NETIF_MSG_WOL, &msglvl_unwanted },
+};
+
 static long long
 get_int_range(char *str, int base, long long min, long long max)
 {
@@ -920,7 +977,20 @@ static void parse_cmdline(int argc, char **argp)
 				i++;
 				if (i >= argc)
 					show_usage(1);
-				msglvl_wanted = get_int(argp[i], 0);
+				if (isdigit((unsigned char)argp[i][0])) {
+					msglvl_changed = 1;
+					msglvl_unwanted = ~0;
+					msglvl_wanted =
+						get_uint_range(argp[i], 0,
+							       0xffffffff);
+				} else {
+					parse_generic_cmdline(
+						argc, argp, i,
+						&msglvl_changed,
+						cmdline_msglvl,
+						ARRAY_SIZE(cmdline_msglvl));
+					i = argc;
+				}
 				break;
 			}
 			show_usage(1);
@@ -2247,8 +2317,12 @@ static int do_gset(int fd, struct ifreq *ifr)
 	ifr->ifr_data = (caddr_t)&edata;
 	err = send_ioctl(fd, ifr);
 	if (err == 0) {
-		fprintf(stdout, "	Current message level: 0x%08x (%d)\n",
+		fprintf(stdout, "	Current message level: 0x%08x (%d)\n"
+			"			       ",
 			edata.data, edata.data);
+		print_flags(cmdline_msglvl, ARRAY_SIZE(cmdline_msglvl),
+			    edata.data);
+		fprintf(stdout, "\n");
 		allfail = 0;
 	} else if (errno != EOPNOTSUPP) {
 		perror("Cannot get message level");
@@ -2371,15 +2445,23 @@ static int do_sset(int fd, struct ifreq *ifr)
 		}
 	}
 
-	if (msglvl_wanted != -1) {
+	if (msglvl_changed) {
 		struct ethtool_value edata;
 
-		edata.cmd = ETHTOOL_SMSGLVL;
-		edata.data = msglvl_wanted;
-		ifr->ifr_data = (caddr_t)&edata;;
+		edata.cmd = ETHTOOL_GMSGLVL;
+		ifr->ifr_data = (caddr_t)&edata;
 		err = send_ioctl(fd, ifr);
-		if (err < 0)
-			perror("Cannot set new msglvl");
+		if (err < 0) {
+			perror("Cannot get msglvl");
+		} else {
+			edata.cmd = ETHTOOL_SMSGLVL;
+			edata.data = ((edata.data & ~msglvl_unwanted) |
+				      msglvl_wanted);
+			ifr->ifr_data = (caddr_t)&edata;
+			err = send_ioctl(fd, ifr);
+			if (err < 0)
+				perror("Cannot set new msglvl");
+		}
 	}
 
 	return 0;
-- 
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.


^ permalink raw reply related

* [PATCH ethtool 3/5] ethtool: Add support for named flags
From: Ben Hutchings @ 2010-06-25 14:49 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, linux-net-drivers
In-Reply-To: <1277477027.2094.2.camel@achroite.uk.solarflarecom.com>

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 == CMDL_FLAG && value & info->flag_val) {
+			printf("%s%s", sep, info->name);
+			sep = " ";
+			value &= ~info->flag_val;
+		}
+		++info;
+		--n_info;
+	}
+
+	/* Print any unrecognised flags in hex */
+	if (value)
+		printf("%s%#x", sep, value);
+}
+
 static int rxflow_str_to_type(const char *str)
 {
 	int flow_type = 0;
-- 
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.


^ permalink raw reply related

* [PATCH ethtool 2/5] ethtool: Mark show_usage() as noreturn
From: Ben Hutchings @ 2010-06-25 14:48 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, linux-net-drivers
In-Reply-To: <1277477027.2094.2.camel@achroite.uk.solarflarecom.com>


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.


^ permalink raw reply related

* [PATCH ethtool 1/5] ethtool: Parse integers into variables of different sizes and byte orders
From: Ben Hutchings @ 2010-06-25 14:46 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, linux-net-drivers, Peter Waskiewicz
In-Reply-To: <1277477027.2094.2.camel@achroite.uk.solarflarecom.com>

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 *regs);
diff --git a/ethtool.c b/ethtool.c
index 5d61439..8969390 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -268,35 +268,35 @@ static int pause_tx_wanted = -1;
 
 static struct ethtool_ringparam ering;
 static int gring_changed = 0;
-static int ring_rx_wanted = -1;
-static int ring_rx_mini_wanted = -1;
-static int ring_rx_jumbo_wanted = -1;
-static int ring_tx_wanted = -1;
+static s32 ring_rx_wanted = -1;
+static s32 ring_rx_mini_wanted = -1;
+static s32 ring_rx_jumbo_wanted = -1;
+static s32 ring_tx_wanted = -1;
 
 static struct ethtool_coalesce ecoal;
 static int gcoalesce_changed = 0;
-static int coal_stats_wanted = -1;
+static s32 coal_stats_wanted = -1;
 static int coal_adaptive_rx_wanted = -1;
 static int coal_adaptive_tx_wanted = -1;
-static int coal_sample_rate_wanted = -1;
-static int coal_pkt_rate_low_wanted = -1;
-static int coal_pkt_rate_high_wanted = -1;
-static int coal_rx_usec_wanted = -1;
-static int coal_rx_frames_wanted = -1;
-static int coal_rx_usec_irq_wanted = -1;
-static int coal_rx_frames_irq_wanted = -1;
-static int coal_tx_usec_wanted = -1;
-static int coal_tx_frames_wanted = -1;
-static int coal_tx_usec_irq_wanted = -1;
-static int coal_tx_frames_irq_wanted = -1;
-static int coal_rx_usec_low_wanted = -1;
-static int coal_rx_frames_low_wanted = -1;
-static int coal_tx_usec_low_wanted = -1;
-static int coal_tx_frames_low_wanted = -1;
-static int coal_rx_usec_high_wanted = -1;
-static int coal_rx_frames_high_wanted = -1;
-static int coal_tx_usec_high_wanted = -1;
-static int coal_tx_frames_high_wanted = -1;
+static s32 coal_sample_rate_wanted = -1;
+static s32 coal_pkt_rate_low_wanted = -1;
+static s32 coal_pkt_rate_high_wanted = -1;
+static s32 coal_rx_usec_wanted = -1;
+static s32 coal_rx_frames_wanted = -1;
+static s32 coal_rx_usec_irq_wanted = -1;
+static s32 coal_rx_frames_irq_wanted = -1;
+static s32 coal_tx_usec_wanted = -1;
+static s32 coal_tx_frames_wanted = -1;
+static s32 coal_tx_usec_irq_wanted = -1;
+static s32 coal_tx_frames_irq_wanted = -1;
+static s32 coal_rx_usec_low_wanted = -1;
+static s32 coal_rx_frames_low_wanted = -1;
+static s32 coal_tx_usec_low_wanted = -1;
+static s32 coal_tx_frames_low_wanted = -1;
+static s32 coal_rx_usec_high_wanted = -1;
+static s32 coal_rx_frames_high_wanted = -1;
+static s32 coal_tx_usec_high_wanted = -1;
+static s32 coal_tx_frames_high_wanted = -1;
 
 static int speed_wanted = -1;
 static int duplex_wanted = -1;
@@ -319,13 +319,13 @@ static int gregs_dump_hex = 0;
 static char *gregs_dump_file = NULL;
 static int geeprom_changed = 0;
 static int geeprom_dump_raw = 0;
-static int geeprom_offset = 0;
-static int geeprom_length = -1;
+static s32 geeprom_offset = 0;
+static s32 geeprom_length = -1;
 static int seeprom_changed = 0;
-static int seeprom_magic = 0;
-static int seeprom_length = -1;
-static int seeprom_offset = 0;
-static int seeprom_value = EOF;
+static s32 seeprom_magic = 0;
+static s32 seeprom_length = -1;
+static s32 seeprom_offset = 0;
+static s32 seeprom_value = EOF;
 static int rx_fhash_get = 0;
 static int rx_fhash_set = 0;
 static u32 rx_fhash_val = 0;
@@ -343,14 +343,19 @@ static enum {
 typedef enum {
 	CMDL_NONE,
 	CMDL_BOOL,
-	CMDL_INT,
-	CMDL_UINT,
+	CMDL_S32,
+	CMDL_U16,
+	CMDL_U32,
+	CMDL_U64,
+	CMDL_BE16,
+	CMDL_BE32,
 	CMDL_STR,
 } cmdline_type_t;
 
 struct cmdline_info {
 	const char *name;
 	cmdline_type_t type;
+	/* Points to int (BOOL), s32, u16, u32, u64 or char * (STR) */
 	void *wanted_val;
 	void *ioctl_val;
 };
@@ -362,16 +367,16 @@ static struct cmdline_info cmdline_gregs[] = {
 };
 
 static struct cmdline_info cmdline_geeprom[] = {
-	{ "offset", CMDL_INT, &geeprom_offset, NULL },
-	{ "length", CMDL_INT, &geeprom_length, NULL },
+	{ "offset", CMDL_S32, &geeprom_offset, NULL },
+	{ "length", CMDL_S32, &geeprom_length, NULL },
 	{ "raw", CMDL_BOOL, &geeprom_dump_raw, NULL },
 };
 
 static struct cmdline_info cmdline_seeprom[] = {
-	{ "magic", CMDL_INT, &seeprom_magic, NULL },
-	{ "offset", CMDL_INT, &seeprom_offset, NULL },
-	{ "length", CMDL_INT, &seeprom_length, NULL },
-	{ "value", CMDL_INT, &seeprom_value, NULL },
+	{ "magic", CMDL_S32, &seeprom_magic, NULL },
+	{ "offset", CMDL_S32, &seeprom_offset, NULL },
+	{ "length", CMDL_S32, &seeprom_length, NULL },
+	{ "value", CMDL_S32, &seeprom_value, NULL },
 };
 
 static struct cmdline_info cmdline_offload[] = {
@@ -394,87 +399,94 @@ static struct cmdline_info cmdline_pause[] = {
 };
 
 static struct cmdline_info cmdline_ring[] = {
-	{ "rx", CMDL_INT, &ring_rx_wanted, &ering.rx_pending },
-	{ "rx-mini", CMDL_INT, &ring_rx_mini_wanted, &ering.rx_mini_pending },
-	{ "rx-jumbo", CMDL_INT, &ring_rx_jumbo_wanted, &ering.rx_jumbo_pending },
-	{ "tx", CMDL_INT, &ring_tx_wanted, &ering.tx_pending },
+	{ "rx", CMDL_S32, &ring_rx_wanted, &ering.rx_pending },
+	{ "rx-mini", CMDL_S32, &ring_rx_mini_wanted, &ering.rx_mini_pending },
+	{ "rx-jumbo", CMDL_S32, &ring_rx_jumbo_wanted, &ering.rx_jumbo_pending },
+	{ "tx", CMDL_S32, &ring_tx_wanted, &ering.tx_pending },
 };
 
 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 },
-	{ "sample-interval", CMDL_INT, &coal_sample_rate_wanted, &ecoal.rate_sample_interval },
-	{ "stats-block-usecs", CMDL_INT, &coal_stats_wanted, &ecoal.stats_block_coalesce_usecs },
-	{ "pkt-rate-low", CMDL_INT, &coal_pkt_rate_low_wanted, &ecoal.pkt_rate_low },
-	{ "pkt-rate-high", CMDL_INT, &coal_pkt_rate_high_wanted, &ecoal.pkt_rate_high },
-	{ "rx-usecs", CMDL_INT, &coal_rx_usec_wanted, &ecoal.rx_coalesce_usecs },
-	{ "rx-frames", CMDL_INT, &coal_rx_frames_wanted, &ecoal.rx_max_coalesced_frames },
-	{ "rx-usecs-irq", CMDL_INT, &coal_rx_usec_irq_wanted, &ecoal.rx_coalesce_usecs_irq },
-	{ "rx-frames-irq", CMDL_INT, &coal_rx_frames_irq_wanted, &ecoal.rx_max_coalesced_frames_irq },
-	{ "tx-usecs", CMDL_INT, &coal_tx_usec_wanted, &ecoal.tx_coalesce_usecs },
-	{ "tx-frames", CMDL_INT, &coal_tx_frames_wanted, &ecoal.tx_max_coalesced_frames },
-	{ "tx-usecs-irq", CMDL_INT, &coal_tx_usec_irq_wanted, &ecoal.tx_coalesce_usecs_irq },
-	{ "tx-frames-irq", CMDL_INT, &coal_tx_frames_irq_wanted, &ecoal.tx_max_coalesced_frames_irq },
-	{ "rx-usecs-low", CMDL_INT, &coal_rx_usec_low_wanted, &ecoal.rx_coalesce_usecs_low },
-	{ "rx-frames-low", CMDL_INT, &coal_rx_frames_low_wanted, &ecoal.rx_max_coalesced_frames_low },
-	{ "tx-usecs-low", CMDL_INT, &coal_tx_usec_low_wanted, &ecoal.tx_coalesce_usecs_low },
-	{ "tx-frames-low", CMDL_INT, &coal_tx_frames_low_wanted, &ecoal.tx_max_coalesced_frames_low },
-	{ "rx-usecs-high", CMDL_INT, &coal_rx_usec_high_wanted, &ecoal.rx_coalesce_usecs_high },
-	{ "rx-frames-high", CMDL_INT, &coal_rx_frames_high_wanted, &ecoal.rx_max_coalesced_frames_high },
-	{ "tx-usecs-high", CMDL_INT, &coal_tx_usec_high_wanted, &ecoal.tx_coalesce_usecs_high },
-	{ "tx-frames-high", CMDL_INT, &coal_tx_frames_high_wanted, &ecoal.tx_max_coalesced_frames_high },
+	{ "sample-interval", CMDL_S32, &coal_sample_rate_wanted, &ecoal.rate_sample_interval },
+	{ "stats-block-usecs", CMDL_S32, &coal_stats_wanted, &ecoal.stats_block_coalesce_usecs },
+	{ "pkt-rate-low", CMDL_S32, &coal_pkt_rate_low_wanted, &ecoal.pkt_rate_low },
+	{ "pkt-rate-high", CMDL_S32, &coal_pkt_rate_high_wanted, &ecoal.pkt_rate_high },
+	{ "rx-usecs", CMDL_S32, &coal_rx_usec_wanted, &ecoal.rx_coalesce_usecs },
+	{ "rx-frames", CMDL_S32, &coal_rx_frames_wanted, &ecoal.rx_max_coalesced_frames },
+	{ "rx-usecs-irq", CMDL_S32, &coal_rx_usec_irq_wanted, &ecoal.rx_coalesce_usecs_irq },
+	{ "rx-frames-irq", CMDL_S32, &coal_rx_frames_irq_wanted, &ecoal.rx_max_coalesced_frames_irq },
+	{ "tx-usecs", CMDL_S32, &coal_tx_usec_wanted, &ecoal.tx_coalesce_usecs },
+	{ "tx-frames", CMDL_S32, &coal_tx_frames_wanted, &ecoal.tx_max_coalesced_frames },
+	{ "tx-usecs-irq", CMDL_S32, &coal_tx_usec_irq_wanted, &ecoal.tx_coalesce_usecs_irq },
+	{ "tx-frames-irq", CMDL_S32, &coal_tx_frames_irq_wanted, &ecoal.tx_max_coalesced_frames_irq },
+	{ "rx-usecs-low", CMDL_S32, &coal_rx_usec_low_wanted, &ecoal.rx_coalesce_usecs_low },
+	{ "rx-frames-low", CMDL_S32, &coal_rx_frames_low_wanted, &ecoal.rx_max_coalesced_frames_low },
+	{ "tx-usecs-low", CMDL_S32, &coal_tx_usec_low_wanted, &ecoal.tx_coalesce_usecs_low },
+	{ "tx-frames-low", CMDL_S32, &coal_tx_frames_low_wanted, &ecoal.tx_max_coalesced_frames_low },
+	{ "rx-usecs-high", CMDL_S32, &coal_rx_usec_high_wanted, &ecoal.rx_coalesce_usecs_high },
+	{ "rx-frames-high", CMDL_S32, &coal_rx_frames_high_wanted, &ecoal.rx_max_coalesced_frames_high },
+	{ "tx-usecs-high", CMDL_S32, &coal_tx_usec_high_wanted, &ecoal.tx_coalesce_usecs_high },
+	{ "tx-frames-high", CMDL_S32, &coal_tx_frames_high_wanted, &ecoal.tx_max_coalesced_frames_high },
 };
 
 static struct cmdline_info cmdline_ntuple[] = {
-	{ "src-ip", CMDL_INT, &ntuple_fs.h_u.tcp_ip4_spec.ip4src, NULL },
-	{ "src-ip-mask", CMDL_UINT, &ntuple_fs.m_u.tcp_ip4_spec.ip4src, NULL },
-	{ "dst-ip", CMDL_INT, &ntuple_fs.h_u.tcp_ip4_spec.ip4dst, NULL },
-	{ "dst-ip-mask", CMDL_UINT, &ntuple_fs.m_u.tcp_ip4_spec.ip4dst, NULL },
-	{ "src-port", CMDL_INT, &ntuple_fs.h_u.tcp_ip4_spec.psrc, NULL },
-	{ "src-port-mask", CMDL_UINT, &ntuple_fs.m_u.tcp_ip4_spec.psrc, NULL },
-	{ "dst-port", CMDL_INT, &ntuple_fs.h_u.tcp_ip4_spec.pdst, NULL },
-	{ "dst-port-mask", CMDL_UINT, &ntuple_fs.m_u.tcp_ip4_spec.pdst, NULL },
-	{ "vlan", CMDL_INT, &ntuple_fs.vlan_tag, NULL },
-	{ "vlan-mask", CMDL_UINT, &ntuple_fs.vlan_tag_mask, NULL },
-	{ "user-def", CMDL_INT, &ntuple_fs.data, NULL },
-	{ "user-def-mask", CMDL_UINT, &ntuple_fs.data_mask, NULL },
-	{ "action", CMDL_INT, &ntuple_fs.action, NULL },
+	{ "src-ip", CMDL_BE32, &ntuple_fs.h_u.tcp_ip4_spec.ip4src, NULL },
+	{ "src-ip-mask", CMDL_BE32, &ntuple_fs.m_u.tcp_ip4_spec.ip4src, NULL },
+	{ "dst-ip", CMDL_BE32, &ntuple_fs.h_u.tcp_ip4_spec.ip4dst, NULL },
+	{ "dst-ip-mask", CMDL_BE32, &ntuple_fs.m_u.tcp_ip4_spec.ip4dst, NULL },
+	{ "src-port", CMDL_BE16, &ntuple_fs.h_u.tcp_ip4_spec.psrc, NULL },
+	{ "src-port-mask", CMDL_BE16, &ntuple_fs.m_u.tcp_ip4_spec.psrc, NULL },
+	{ "dst-port", CMDL_BE16, &ntuple_fs.h_u.tcp_ip4_spec.pdst, NULL },
+	{ "dst-port-mask", CMDL_BE16, &ntuple_fs.m_u.tcp_ip4_spec.pdst, NULL },
+	{ "vlan", CMDL_U16, &ntuple_fs.vlan_tag, NULL },
+	{ "vlan-mask", CMDL_U16, &ntuple_fs.vlan_tag_mask, NULL },
+	{ "user-def", CMDL_U64, &ntuple_fs.data, NULL },
+	{ "user-def-mask", CMDL_U64, &ntuple_fs.data_mask, NULL },
+	{ "action", CMDL_S32, &ntuple_fs.action, NULL },
 };
 
-static int get_int(char *str, int base)
+static long long
+get_int_range(char *str, int base, long long min, long long max)
 {
-	long v;
+	long long v;
 	char *endp;
 
 	if (!str)
 		show_usage(1);
 	errno = 0;
-	v = strtol(str, &endp, base);
-	if ( errno || *endp || v > INT_MAX)
+	v = strtoll(str, &endp, base);
+	if (errno || *endp || v < min || v > max)
 		show_usage(1);
-	return (int)v;
+	return v;
 }
 
-static int get_uint(char *str, int base)
+static unsigned long long
+get_uint_range(char *str, int base, unsigned long long max)
 {
-	unsigned long v;
+	unsigned long long v;
 	char *endp;
 
 	if (!str)
 		show_usage(1);
 	errno = 0;
-	v = strtoul(str, &endp, base);
-	if ( errno || *endp || v > UINT_MAX)
+	v = strtoull(str, &endp, base);
+	if ( errno || *endp || v > max)
 		show_usage(1);
 	return v;
 }
 
+static int get_int(char *str, int base)
+{
+	return get_int_range(str, base, INT_MIN, INT_MAX);
+}
+
 static void parse_generic_cmdline(int argc, char **argp,
 				  int first_arg, int *changed,
 				  struct cmdline_info *info,
 				  unsigned int n_info)
 {
-	int i, idx, *p;
+	int i, idx;
 	int found;
 
 	for (i = first_arg; i < argc; i++) {
@@ -486,9 +498,9 @@ static void parse_generic_cmdline(int argc, char **argp,
 				i += 1;
 				if (i >= argc)
 					show_usage(1);
-				p = info[idx].wanted_val;
 				switch (info[idx].type) {
-				case CMDL_BOOL:
+				case CMDL_BOOL: {
+					int *p = info[idx].wanted_val;
 					if (!strcmp(argp[i], "on"))
 						*p = 1;
 					else if (!strcmp(argp[i], "off"))
@@ -496,12 +508,44 @@ static void parse_generic_cmdline(int argc, char **argp,
 					else
 						show_usage(1);
 					break;
-				case CMDL_INT: {
-					*p = get_int(argp[i],0);
+				}
+				case CMDL_S32: {
+					s32 *p = info[idx].wanted_val;
+					*p = get_int_range(argp[i], 0,
+							   -0x80000000LL,
+							   0x7fffffff);
+					break;
+				}
+				case CMDL_U16: {
+					u16 *p = info[idx].wanted_val;
+					*p = get_uint_range(argp[i], 0, 0xffff);
+					break;
+				}
+				case CMDL_U32: {
+					u32 *p = info[idx].wanted_val;
+					*p = get_uint_range(argp[i], 0,
+							    0xffffffff);
+					break;
+				}
+				case CMDL_U64: {
+					u64 *p = info[idx].wanted_val;
+					*p = get_uint_range(
+						argp[i], 0,
+						0xffffffffffffffffLL);
+					break;
+				}
+				case CMDL_BE16: {
+					u16 *p = info[idx].wanted_val;
+					*p = cpu_to_be16(
+						get_uint_range(argp[i], 0,
+							       0xffff));
 					break;
 				}
-				case CMDL_UINT: {
-					*p = get_uint(argp[i],0);
+				case CMDL_BE32: {
+					u32 *p = info[idx].wanted_val;
+					*p = cpu_to_be32(
+						get_uint_range(argp[i], 0,
+							       0xffffffff));
 					break;
 				}
 				case CMDL_STR: {
-- 
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.


^ permalink raw reply related

* [PATCH ethtool 0/5] Fix and enhance command-line parsing
From: Ben Hutchings @ 2010-06-25 14:43 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, linux-net-drivers

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.


^ permalink raw reply

* Re: [PATCH] smsc95xx: Add module parameter to override MAC address
From: Sebastien Jan @ 2010-06-25 14:20 UTC (permalink / raw)
  To: Steve.Glendinning@smsc.com
  Cc: Simon Horman, linux-omap@vger.kernel.org, netdev@vger.kernel.org,
	davem@davemloft.net, Ian.Saturley@smsc.com
In-Reply-To: <OF60E8D688.255F8F16-ON8025774D.002E17AA-8025774D.002FEC33@smsc.com>

Hi Steve,

Thanks for your answer.

On 06/25/2010 10:43 AM, Steve.Glendinning@smsc.com wrote:
[...]
> I can see you have a different use case, but I don't think this specific
> driver is the place for this logic.  I'd rather see it added to either
> the usbnet framework or (preferably) the netdev framework so *all*
> ethernet drivers can do this the same way.  otherwise we could end up
> with slight variations of this code in every single driver!

I perfectly understand your concerns. Unfortunately, I will probably not be
able to implement these changes into the netdev framework. However, I'd be 
happy to make some tests if someone proposes such changes.


^ permalink raw reply

* [PATCH] ip: correctly report 802.15.4 link type
From: Jan Engelhardt @ 2010-06-25 14:01 UTC (permalink / raw)
  To: stephen.hemminger; +Cc: netdev

Up until now, the "hardwpan" devices were displayed as link/[804].

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 lib/ll_types.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/lib/ll_types.c b/lib/ll_types.c
index 846cdb0..1cc46b6 100644
--- a/lib/ll_types.c
+++ b/lib/ll_types.c
@@ -125,6 +125,9 @@ __PF(IEEE80211_PRISM,ieee802.11/prism)
 #ifdef ARPHRD_IEEE80211_RADIOTAP
 __PF(IEEE80211_RADIOTAP,ieee802.11/radiotap)
 #endif
+#ifdef ARPHRD_IEEE802154
+__PF(IEEE802154, ieee802.15.4)
+#endif
 #ifdef ARPHRD_NONE
 __PF(NONE, none)
 #endif
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH] b43: Add SDIO_DEVICE() for EW-CG1102GC
From: Michael Büsch @ 2010-06-25 13:29 UTC (permalink / raw)
  To: Magnus Damm
  Cc: linux-kernel, Tejun Heo, Stefano Brivio, netdev, linux-wireless,
	John W. Linville, Albert Herranz
In-Reply-To: <20100625093254.9484.69877.sendpatchset@t400s>

On 06/25/2010 11:32 AM, Magnus Damm wrote:
> From: Magnus Damm<damm@opensource.se>
>
> This patch enables the EW-CG1102GC SDIO card in the b43 driver.
>
> b43-sdio mmc0:0001:1: Chip ID 14e4:4318
> ssb: Core 0 found: ChipCommon (cc 0x800, rev 0x0D, vendor 0x4243)
> ssb: Core 1 found: IEEE 802.11 (cc 0x812, rev 0x09, vendor 0x4243)
> ssb: Core 2 found: PCI (cc 0x804, rev 0x0C, vendor 0x4243)
> ssb: Core 3 found: PCMCIA (cc 0x80D, rev 0x07, vendor 0x4243)
> b43-phy0: Broadcom 4318 WLAN found (core revision 9)
> b43-phy0 debug: Found PHY: Analog 3, Type 2, Revision 7
> b43-phy0 debug: Found Radio: Manuf 0x17F, Version 0x2050, Revision 8
>
> Tested with openfwwf-5.2 using a SuperH SDHI host controller.
>
> Signed-off-by: Magnus Damm<damm@opensource.se>
> ---
>
>   drivers/net/wireless/b43/sdio.c |    1 +
>   1 file changed, 1 insertion(+)
>
> --- 0001/drivers/net/wireless/b43/sdio.c
> +++ work/drivers/net/wireless/b43/sdio.c	2010-06-25 18:20:46.000000000 +0900
> @@ -182,6 +182,7 @@ static void b43_sdio_remove(struct sdio_
>
>   static const struct sdio_device_id b43_sdio_ids[] = {
>   	{ SDIO_DEVICE(0x02d0, 0x044b) }, /* Nintendo Wii WLAN daughter card */
> +	{ SDIO_DEVICE(0x0092, 0x0004) }, /* C-guys, Inc. EW-CG1102GC */
>   	{ },
>   };
>
>

ack

-- 
Greetings Michael.

^ permalink raw reply

* Re: [PATCH 0/3] b43: logging cleanups
From: Michael Büsch @ 2010-06-25 13:28 UTC (permalink / raw)
  To: Joe Perches
  Cc: John W. Linville, Stefano Brivio, linux-wireless, netdev,
	linux-kernel, Larry.Finger, zajec5
In-Reply-To: <1277408432.1654.80.camel@Joe-Laptop.home>

On 06/24/2010 09:40 PM, Joe Perches wrote:
> On Thu, 2010-06-24 at 14:53 -0400, John W. Linville wrote:
>> On Sat, Jun 19, 2010 at 04:30:08PM -0700, Joe Perches wrote:
>>> Just some small cleanups
>>> Joe Perches (3):
>>>    drivers/net/wireless/b43: Use local ratelimit_state
>>>    drivers/net/wireless/b43: Logging cleanups
>>>    drivers/net/wireless/b43: Rename b43_debug to b43_debugging
>> Any of the b43 guys want to express an opinion on these?

The cleanups, well, exchange one name for the other. I don't have a
strong opinion on one or the other. So in the end I don't see why
that stuff should be renamed. However, in the end I really do not care.

I'm not sure why we need an additional ratelimit here. Is it a policy
that every kernel driver should have local ratelimiting? If not, why
do you think b43 needs its own ratelimiting? Note that b43 does not
output a single line to dmesg in normal operation, after init has
finished (and ratelimit is disabled in init).

> Stefano, are you active here?
> Your last ack for b43 was Feb 2008.
> There have been 400+ commits to b43 without your ack.
>
> Should your name be moved from MAINTAINERS to CREDITS?
>
> $ ./scripts/get_maintainer.pl --rolestats -f drivers/net/wireless/b43/
> Stefano Brivio<stefano.brivio@polimi.it>  (maintainer:B43 WIRELESS DRIVER)
> "John W. Linville"<linville@tuxdriver.com>  (maintainer:NETWORKING [WIREL...,commit_signer:204/240=85%)
> "Rafał Miłecki"<zajec5@gmail.com>  (commit_signer:83/240=35%)
> "Gábor Stefanik"<netrolller.3d@gmail.com>  (commit_signer:44/240=18%)
> Michael Buesch<mb@bu3sch.de>  (commit_signer:39/240=16%)
> Larry Finger<Larry.Finger@lwfinger.net>  (commit_signer:13/240=5%)
> linux-wireless@vger.kernel.org (open list:B43 WIRELESS DRIVER)
> netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
> linux-kernel@vger.kernel.org (open list)

I think these statistics are almost worthless for b43.

-- 
Greetings Michael.

^ permalink raw reply

* Re: [PATCH net-next] MAINTAINERS: b43 updates
From: Michael Büsch @ 2010-06-25 13:22 UTC (permalink / raw)
  To: Larry Finger
  Cc: Joe Perches, John W. Linville, Stefano Brivio,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	zajec5-Re5JQEeQqe8AvxtiuMwx3w, Gábor Stefanik
In-Reply-To: <4C241770.6030701-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>

On 06/25/2010 04:41 AM, Larry Finger wrote:
> On 06/24/2010 06:57 PM, Joe Perches wrote:
>> On Thu, 2010-06-24 at 18:53 -0500, Larry Finger wrote:
>>> I never said that b43 is unmaintained. It is definitely getting much
>>> more than "odd fixes".
>>
>> If Stefano isn't maintaining it, who is?
>
> We are working on that.

Well, there are some developers working on the driver and there
are some developers that do not actively work on the driver but used
to work on it in the past (that includes me). Neither of them is an
official maintainer (yet). I used to be a maintainer, but I dropped
my official MAINTAINERS file entry, because I do not actively work
on b43 anymore. However, I still read the patches going into the driver
and comment on them as appropriate.
So well. I think b43 does not have a maintainer. But it's
not unmaintained. There are lots of people working on it. I don't think
an official maintainer is needed. Patches are sent directly to John
(the wireless maintainer) and the b43 development crew reviews them.
There's no problem with that.

-- 
Greetings Michael.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Question about xfrm by MARK feature
From: jamal @ 2010-06-25 12:43 UTC (permalink / raw)
  To: Gerd v. Egidy; +Cc: timo.teras, kaber, herbert, netdev
In-Reply-To: <201006250935.30967.lists@egidy.de>

Hi Gerd,

On Fri, 2010-06-25 at 09:35 +0200, Gerd v. Egidy wrote:

> I planned to avoid looking at the remote gateway ip (to even allow two 
> different remote gateways hiding natted behind the same ip) but that would be 
> a good fallback solution if my other ideas don't work out.
> 

Doesnt have to be a remote IP...
If you can somehow even map a remote to some MAC address or incoming
virtual interface (such as a VLAN) that would do it as well.
Alternatively, you should probably look at namespaces  - i just find it
more usable when you have overlapping/conflicting ip addresses. You
of course will have to run strongswan per namespace if you dont want to
hack strongswan.

cheers,
jamal


^ permalink raw reply

* Re: [PATCH v3] act_mirred: don't clone skb when skb isn't shared
From: jamal @ 2010-06-25 12:39 UTC (permalink / raw)
  To: Changli Gao; +Cc: David S. Miller, netdev
In-Reply-To: <1277432712-2869-1-git-send-email-xiaosuo@gmail.com>

On Fri, 2010-06-25 at 10:25 +0800, Changli Gao wrote:
> don't clone skb when skb isn't shared
> 
> When the tcf_action is TC_ACT_STOLEN, and the skb isn't shared, we don't need
> to clone a new skb. As the skb will be freed after this function returns, we
> can use it freely once we get a reference to it.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>


cheers,
jamal


^ permalink raw reply

* Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Michael S. Tsirkin @ 2010-06-25 11:06 UTC (permalink / raw)
  To: Dong, Eddie
  Cc: Herbert Xu, Xin, Xiaohui, Stephen Hemminger,
	netdev@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, mingo@elte.hu, davem@davemloft.net,
	jdike@linux.intel.com
In-Reply-To: <1A42CE6F5F474C41B63392A5F80372B21F58D666@shsmsx501.ccr.corp.intel.com>

On Fri, Jun 25, 2010 at 09:03:46AM +0800, Dong, Eddie wrote:
> Herbert Xu wrote:
> > On Wed, Jun 23, 2010 at 06:05:41PM +0800, Dong, Eddie wrote:
> >> 
> >> I mean once the frontend side driver post the buffers to the backend
> >> driver, the backend driver will "immediately" use that buffers to
> >> compose skb or gro_frags and post them to the assigned host NIC
> >> driver as receive buffers. In that case, if the backend driver
> >> recieves a packet from the NIC that requires to do copy, it may be
> >> unable to find additional free guest buffer because all of them are
> >> already used by the NIC driver. We have to reserve some guest
> >> buffers for the possible copy even if the buffer address is not
> >> identified by original skb :(        
> > 
> > OK I see what you mean.  Can you tell me how does Xiaohui's
> > previous patch-set deal with this problem?
> > 
> > Thanks,
> 
> In current patch, each SKB for the assigned device (SRIOV VF or NIC or a complete queue pairs) uses the buffer from guest, so it eliminates copy completely in software and requires hardware to do so. If we can have an additonal place to store the buffer per skb (may cause copy later on), we can do copy later on or re-post the buffer to assigned NIC driver later on. But that may be not very clean either :(
> BTW, some hardware may require certain level of packet copy such as for broadcast packets in very old VMDq device, which is not addressed in previous Xiaohui's patch yet. We may address this by implementing an additional virtqueue between guest and host for slow path (broadcast packets only here) with additinal complexity in FE/BE driver. 
> 
> Thx, Eddie

guest posts a large number of buffers to the host.
Host can use them any way it wants to, and in any order,
for example reserve half the buffers for the copy.

This might waste some memory if buffers are used
only partially, but let's worry about this later.

-- 
MST

^ permalink raw reply

* [PATCH] sched: export sched_set/getaffinity (was Re: [PATCH 3/3] vhost: apply cpumask and cgroup to vhost pollers)
From: Michael S. Tsirkin @ 2010-06-25 10:10 UTC (permalink / raw)
  To: Sridhar Samudrala
  Cc: Tejun Heo, Oleg Nesterov, netdev, lkml, kvm@vger.kernel.org,
	Andrew Morton, Dmitri Vorobiev, Jiri Kosina, Thomas Gleixner,
	Ingo Molnar, Andi Kleen
In-Reply-To: <1277419551.27868.27.camel@w-sridhar.beaverton.ibm.com>

On Thu, Jun 24, 2010 at 03:45:51PM -0700, Sridhar Samudrala wrote:
> On Thu, 2010-06-24 at 11:11 +0300, Michael S. Tsirkin wrote:
> > On Sun, May 30, 2010 at 10:25:01PM +0200, Tejun Heo wrote:
> > > Apply the cpumask and cgroup of the initializing task to the created
> > > vhost poller.
> > > 
> > > Based on Sridhar Samudrala's patch.
> > > 
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: Sridhar Samudrala <samudrala.sridhar@gmail.com>
> > 
> > 
> > I wanted to apply this, but modpost fails:
> > ERROR: "sched_setaffinity" [drivers/vhost/vhost_net.ko] undefined!
> > ERROR: "sched_getaffinity" [drivers/vhost/vhost_net.ko] undefined!
> > 
> > Did you try building as a module?
> 
> In my original implementation, i had these calls in workqueue.c.
> Now that these are moved to vhost.c which can be built as a module,
> these symbols need to be exported.
> The following patch fixes the build issue with vhost as a module.
> 
> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Works for me. To simplify dependencies, I'd like to queue this
together with the chost patches through net-next.
Ack to this?

> diff --git a/kernel/sched.c b/kernel/sched.c
> index 3c2a54f..15a0c6f 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -4837,6 +4837,7 @@ out_put_task:
>  	put_online_cpus();
>  	return retval;
>  }
> +EXPORT_SYMBOL_GPL(sched_setaffinity);
>  
>  static int get_user_cpu_mask(unsigned long __user *user_mask_ptr,
> unsigned len,
>  			     struct cpumask *new_mask)
> @@ -4900,6 +4901,7 @@ out_unlock:
>  
>  	return retval;
>  }
> +EXPORT_SYMBOL_GPL(sched_getaffinity);
>  
>  /**
>   * sys_sched_getaffinity - get the cpu affinity of a process
> 
> 
> > > ---
> > >  drivers/vhost/vhost.c |   36 +++++++++++++++++++++++++++++++-----
> > >  1 file changed, 31 insertions(+), 5 deletions(-)
> > > 
> > > Index: work/drivers/vhost/vhost.c
> > > ===================================================================
> > > --- work.orig/drivers/vhost/vhost.c
> > > +++ work/drivers/vhost/vhost.c
> > > @@ -23,6 +23,7 @@
> > >  #include <linux/highmem.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/kthread.h>
> > > +#include <linux/cgroup.h>
> > > 
> > >  #include <linux/net.h>
> > >  #include <linux/if_packet.h>
> > > @@ -176,12 +177,30 @@ repeat:
> > >  long vhost_dev_init(struct vhost_dev *dev,
> > >  		    struct vhost_virtqueue *vqs, int nvqs)
> > >  {
> > > -	struct task_struct *poller;
> > > -	int i;
> > > +	struct task_struct *poller = NULL;
> > > +	cpumask_var_t mask;
> > > +	int i, ret = -ENOMEM;
> > > +
> > > +	if (!alloc_cpumask_var(&mask, GFP_KERNEL))
> > > +		goto out;
> > > 
> > >  	poller = kthread_create(vhost_poller, dev, "vhost-%d", current->pid);
> > > -	if (IS_ERR(poller))
> > > -		return PTR_ERR(poller);
> > > +	if (IS_ERR(poller)) {
> > > +		ret = PTR_ERR(poller);
> > > +		goto out;
> > > +	}
> > > +
> > > +	ret = sched_getaffinity(current->pid, mask);
> > > +	if (ret)
> > > +		goto out;
> > > +
> > > +	ret = sched_setaffinity(poller->pid, mask);
> > > +	if (ret)
> > > +		goto out;
> > > +
> > > +	ret = cgroup_attach_task_current_cg(poller);
> > > +	if (ret)
> > > +		goto out;
> > > 
> > >  	dev->vqs = vqs;
> > >  	dev->nvqs = nvqs;
> > > @@ -202,7 +221,14 @@ long vhost_dev_init(struct vhost_dev *de
> > >  			vhost_poll_init(&dev->vqs[i].poll,
> > >  					dev->vqs[i].handle_kick, POLLIN, dev);
> > >  	}
> > > -	return 0;
> > > +
> > > +	wake_up_process(poller);	/* avoid contributing to loadavg */
> > > +	ret = 0;
> > > +out:
> > > +	if (ret)
> > > +		kthread_stop(poller);
> > > +	free_cpumask_var(mask);
> > > +	return ret;
> > >  }
> > > 
> > >  /* Caller should have device mutex */
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 6/8] netpoll: Allow netpoll_setup/cleanup recursion
From: Nick Piggin @ 2010-06-25 10:08 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Miller, herbert, mst, frzhang, netdev, amwang, shemminger,
	mpm, paulmck, a.p.zijlstra, mingo
In-Reply-To: <20100624214204.a85c8ba2.akpm@linux-foundation.org>

On Thu, Jun 24, 2010 at 09:42:04PM -0700, Andrew Morton wrote:
> On Thu, 24 Jun 2010 21:27:13 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
> 
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Date: Thu, 24 Jun 2010 20:50:59 -0700
> > 
> > > What happens if you want to actually *drop* a patch from net-next? 
> > > Surely that happens?
> > 
> > I've only respun the tree on two or three occasions and that was
> > because I made some significant error myself and screwed up the
> > GIT tree somehow.
> > 
> > We've fixed much worse bugs than this one weeks after the changes
> > causing them went in, life goes on.
> 
> Still sucks - this is a quite ugly drawback to how we're using git. 
> I've hit bisection holes several times which held up the show. 
> Sometimes you can make them go away by fiddling the .config, other
> times I've hunted down the fix and manually applied it for each
> iteration.  It makes me feel all guilty each time I ask some poor sap
> to bisect a bug for us.

This might be somewhat improved by tagging a fix with the id of the
patch causing the regression, so that bisect could go through and try
to apply a fix out of order.

This could be done with a specific note format in the changelog, but
I don't know whether it's realistic to support in git format or if
you'd have to have a tool that builds up a mapping going the other
way...

Could also be useful for distros backporting things.


^ permalink raw reply

* Re: [PATCH 6/8] netpoll: Allow netpoll_setup/cleanup recursion
From: Peter Zijlstra @ 2010-06-25  9:45 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Miller, herbert, mst, frzhang, netdev, amwang, shemminger,
	mpm, paulmck, mingo
In-Reply-To: <20100625014253.698d9ff5.akpm@linux-foundation.org>

On Fri, 2010-06-25 at 01:42 -0700, Andrew Morton wrote:
> On Fri, 25 Jun 2010 10:08:56 +0200 Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Thu, 2010-06-24 at 21:42 -0700, Andrew Morton wrote:
> > > That being said, I wonder why Herbert didn't hit this in his testing. 
> > > I suspect that he'd enabled lockdep, which hid the bug.  I haven't
> > > worked out _why_ lockdep hides the double-mutex_unlock bug, but it's a
> > > pretty bad thing to do. 
> > 
> > Most weird indeed, lockdep is supposed so shout its lungs out when
> > someone wants to unlock a lock that isn't actually owned by him (and it
> > not being locked at all certainly implies you're not the owner).
> > 
> > In fact, the below patch results in the below splat -- its also
> > something that's tested by the locking self-test:
> 
> When I enabled lockdep, the bug actually went away.  Is it possible
> that when lockdep detects this bug, it prevents mutex.count from going
> from 1 to 2?

Not lockdep itself but the DEBUG_MUTEXES code (forced by lockdep).

The difference between the normal and the debug code is that the debug
code disables all fast-path code.

The x86 fast-path code does:

 LOCK incl &lock->count
 jg done:
 call slowpath
done:

Since 1++ is >0 it will complete without calling the slow-path, would
do:

 if (__mutex_slowpath_needs_to_unlock()) /* 1 regardless of DEBUG_MUTEX */
   atomic_set(&lock->count, 1);

The question I guess is, do we want double unlocks to go silently
unnoticed? In that case we need to touch the fastpath asm.

> It could be that lockdep _did_ detect (and correct!) the bug.  But
> because I had no usable console output at the time, I didn't see it.
> 
> I did notice that the taint output was "G W".  So something warned
> about something, but I don't know what.  But that was happening with
> lockdep disabled.

Hrmm,. yeah without console output lockdep isn't going to help much,
should we maybe use the speaker to read out the dmesg :-)

> It'd be interesting to add
> 
> 	printk("%d:%d\n", __LINE__, atomic_read(&foo.count));
> 
> after the mutex_unlock()s.

1352:1



^ permalink raw reply

* Re: [PATCH] s2io: add dynamic LRO disable support
From: Cong Wang @ 2010-06-25  9:09 UTC (permalink / raw)
  To: Jon Mason
  Cc: netdev@vger.kernel.org, nhorman@redhat.com, sgruszka@redhat.com,
	herbert.xu@redhat.com, bhutchings@solarflare.com, Ramkrishna Vepa,
	davem@davemloft.net
In-Reply-To: <20100625044510.GC2739@exar.com>

On 06/25/10 12:45, Jon Mason wrote:
> This patch adds dynamic LRO disable support for s2io net driver,
> enables LRO by default, increases the driver version number, and
> corrects the name of the LRO modparm.
>

Very good work, Jon! I believe this version is the best we have.

Thanks again!

^ permalink raw reply

* Re: [PATCH] smsc95xx: Add module parameter to override MAC address
From: Steve.Glendinning @ 2010-06-25  8:43 UTC (permalink / raw)
  To: Sebastien Jan
  Cc: Simon Horman, linux-omap@vger.kernel.org, netdev@vger.kernel.org,
	davem, Ian.Saturley
In-Reply-To: <4C2452ED.8020307@ti.com>

Hi Sebastien,

> > I'm confused as to why this is desirable when the mac address
> > can already be configured after module insertion via
> > smsc95xx_netdev_ops.eth_mac_addr().
> 
> For example for booting over NFS using a pre-defined MAC address, with 
> a minimal setup (no initrd). Or is there another way to force a MAC
> address for this use-case?

I can't see an existing way of specifying this as a kernel parameter
in Documentation/kernel-parameters.txt. netdev= doesn't have a mac
address parameter.

During development I initially had smsc95xx driver using this logic to
select a MAC address:

1. If net->dev_addr has already been set to a valid mac address (i.e. by
an administrator before bringing the device up) then use that address.

2. If the device is already currently set to a valid mac address then
use that address.  This could have been set by either the device's
EEPROM or by a previously running bootloader.

3. Generate a random mac address.

Unfortunately, this doesn't work so well as the usbnet framework sets
net->dev_addr to the USB node_id before calling our bind function, so
we usually matched at step 1 (and not with the desired outcome).  So
I removed step 1 because, as Simon mentioned, it's possible to change
the mac address after the device is brought up.

I can see you have a different use case, but I don't think this specific
driver is the place for this logic.  I'd rather see it added to either
the usbnet framework or (preferably) the netdev framework so *all*
ethernet drivers can do this the same way.  otherwise we could end up
with slight variations of this code in every single driver!

Steve

^ permalink raw reply

* Re: [v4 Patch 1/2] s2io: add dynamic LRO disable support
From: Cong Wang @ 2010-06-25  9:01 UTC (permalink / raw)
  To: Jon Mason
  Cc: netdev@vger.kernel.org, nhorman@redhat.com, sgruszka@redhat.com,
	herbert.xu@redhat.com, bhutchings@solarflare.com, Ramkrishna Vepa,
	davem@davemloft.net
In-Reply-To: <20100625043343.GB2739@exar.com>

On 06/25/10 12:33, Jon Mason wrote:
> On Tue, Jun 22, 2010 at 01:50:07AM -0700, Amerigo Wang wrote:
>>
>> This patch adds dynamic LRO diable support for s2io net driver.
>>
>> (I don't have s2io card, so only did compiling test. Anyone who wants
>> to test this is more than welcome.)
>
> Unfortunately the patch did not quite work, mostly due to NETIF_F_LRO
> needing to be added at probe time to the device features.  I was able
> to modify the patch to get it working, and will respond seperately
> with the working patch.
>

Thanks very much, Jon!

I am very pleased to see this patch gets tested. :)

^ permalink raw reply

* Re: [PATCH net-next-2.6] netfilter: allow nf_tproxy_core module to be removed
From: KOVACS Krisztian @ 2010-06-25  7:56 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, fw, jpirko, netdev, Balazs Scheidler
In-Reply-To: <4C2379D9.2080608@trash.net>

Hi,

On 06/24/2010 05:29 PM, Patrick McHardy wrote:
> David Miller wrote:
>> From: Florian Westphal <fw@strlen.de>
>> Date: Wed, 23 Jun 2010 20:46:11 +0200
>>
>>> tproxy assigns skb->destructor, what prevents module unload while
>>> such skbs may
>>> still be around?
>>
>> The only reference to nf_tproxy_core.ko is for the symbol,
>> "nf_tproxy_assign_sock".
>> xt_TPROXY.c, which references this symbol, thus creates a symbol
>> dependency on this
>> module, so xt_TPROXY.o needs to unload before nf_tproxy_core.ko can
>> unload, and
>> xt_TPROXY.o has it's own manner for handling module references properly.
>
> I don't see anything waiting for skbs in flight using the tproxy
> destructor in either xt_TPROXY or nf_tproxy_core though, so I think
> Florian is correct.

Yes, I think Florian and Patrick's right. Right now there's nothing 
preventing xt_TPROXY and nf_tproxy_core from being removed while there 
are skbs in flight with the tproxy destructor set.

-- 
KOVACS Krisztian

^ permalink raw reply

* Re: [v4 Patch 1/2] s2io: add dynamic LRO disable support
From: Cong Wang @ 2010-06-25  8:59 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, nhorman, sgruszka, herbert.xu, Ramkrishna.Vepa, davem
In-Reply-To: <1277207080.2091.2.camel@achroite.uk.solarflarecom.com>

On 06/22/10 19:44, Ben Hutchings wrote:
> On Tue, 2010-06-22 at 04:50 -0400, Amerigo Wang wrote:
>> This patch adds dynamic LRO diable support for s2io net driver.
>>
>> (I don't have s2io card, so only did compiling test. Anyone who wants
>> to test this is more than welcome.)
>>
>> This is based on Neil's initial work, and heavily modified
>> based on Ramkrishna's suggestions.
> [...]
>> +static int s2io_ethtool_set_flags(struct net_device *dev, u32 data)
>> +{
>> +	struct s2io_nic *sp = netdev_priv(dev);
>> +	int rc = 0;
>> +	int changed = 0;
>> +
>> +	if (data&  ~ETH_FLAG_LRO)
>> +		return -EOPNOTSUPP;
>> +
>> +	if (data&  ETH_FLAG_LRO) {
>> +		if (lro_enable) {
>> +			if (!(dev->features&  NETIF_F_LRO)) {
>> +				dev->features |= NETIF_F_LRO;
>> +				changed = 1;
>> +			}
>> +		} else
>> +			rc = -EOPNOTSUPP;
>
> Should lro_enable=0 really prevent enabling it later?  This seems
> unusual.
>

I agree with Stanislaw on this point.


>> +	} else if (dev->features&  NETIF_F_LRO) {
>> +		dev->features&= ~NETIF_F_LRO;
>> +		changed = 1;
>> +	}
>> +
>> +	if (changed&&  netif_running(dev)) {
>> +		s2io_stop_all_tx_queue(sp);
>> +		s2io_card_down(sp);
>> +		sp->lro = dev->features&  NETIF_F_LRO;
> [...]
>
> This means s2io_nic::lro and ring_info::lro can have the value
> NETIF_F_LRO, where previously they would only have the value 0 or 1.  I
> don't know whether this could be a problem, but the safe thing to do is
> to coerce the value by writing !!(dev->features&  NETIF_F_LRO).
>

Yeah, agreed.
It seems that Jon already fixed this when he updated the patch.

Thanks!

^ permalink raw reply

* Re: [PATCH 6/8] netpoll: Allow netpoll_setup/cleanup recursion
From: Ingo Molnar @ 2010-06-25  8:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Miller, herbert, mst, frzhang, netdev, amwang, shemminger,
	mpm, paulmck, a.p.zijlstra, Linus Torvalds, linux-kernel
In-Reply-To: <20100624214204.a85c8ba2.akpm@linux-foundation.org>


* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 24 Jun 2010 21:27:13 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
> 
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Date: Thu, 24 Jun 2010 20:50:59 -0700
> > 
> > > What happens if you want to actually *drop* a patch from net-next? 
> > > Surely that happens?
> > 
> > I've only respun the tree on two or three occasions and that was
> > because I made some significant error myself and screwed up the
> > GIT tree somehow.
> > 
> > We've fixed much worse bugs than this one weeks after the changes
> > causing them went in, life goes on.
> 
> Still sucks - this is a quite ugly drawback to how we're using git. 
> I've hit bisection holes several times which held up the show. 
> Sometimes you can make them go away by fiddling the .config, other
> times I've hunted down the fix and manually applied it for each
> iteration.  It makes me feel all guilty each time I ask some poor sap
> to bisect a bug for us.

One solution would be, for really grave bugs that introduce a significant 
window of bisection breakage, to add a special tag:

  Fixes-Bug: SHA1

Which could then be parsed by a special variant of git bisect and which would 
try to cherry-pick all Fixes-Bug commits into the bisection point.

But there are numerous disadvantages that:

 - The bisection itself would be slower (maybe not a big issue for most 
   bisection sessions)

 - Such cherry-picked trees wouldnt be precisely the same thing as the tree 
   as-it-was-there.

 - Awkward combination bugs could ensue

 - The cherry-pick may fail or may mis-apply things.

 - If the Fixes-Bug tag is typoed or misconstrued then that might not be found 
   until someone tries a bisection and fails ... leading to awkward 
   add-the-tag-again-to-some-commit scenarios.

My gut feeling is that we really dont want to go there. As painful as 
bisection breakages are, they are relatively rare (compared to regular 
regressions), and the value of testing _that precise_ sha1 is a fundamental 
thing - git bisect shouldnt interact and reorder fixes.

If a bug wasnt found sooner then it either didnt matter that much, or the tree 
QA was bad. Neither of those factors forces a change in the development model.

	Ingo

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox