[PATCH 0/9] various wext cleanups

[PATCH 0/9] various wext cleanups

[Johannes Berg]

This patch series contains various cleanups to the wireless extensions code.

johannes

[PATCH 2/9] wext: clean up how wext is called

[Johannes Berg]

This patch cleans up the call paths from the core code into wext.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 include/net/iw_handler.h |   11 +----------
 include/net/wext.h       |   24 ++++++++++++++++++++++++
 net/core/dev.c           |   34 ++++------------------------------
 net/wireless/wext.c      |   28 ++++++++++++++++++++++++----
 4 files changed, 53 insertions(+), 44 deletions(-)

--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ net-2.6.22/include/net/wext.h	2007-04-24 18:51:23.568804911 +0200
@@ -0,0 +1,24 @@
+#ifndef __NET_WEXT_H
+#define __NET_WEXT_H
+
+/*
+ * wireless extensions interface to the core code
+ */
+
+#ifdef CONFIG_WIRELESS_EXT
+extern int wext_proc_init(void);
+extern int wext_handle_ioctl(struct ifreq *ifr, unsigned int cmd,
+			     void __user *arg);
+#else
+static inline int wext_proc_init()
+{
+	return 0;
+}
+static inline int wext_handle_ioctl(struct ifreq *ifr, unsigned int cmd,
+				    void __user *arg)
+{
+	return -EINVAL;
+}
+#endif
+
+#endif /* __NET_WEXT_H */
--- net-2.6.22.orig/net/core/dev.c	2007-04-24 18:50:45.888804911 +0200
+++ net-2.6.22/net/core/dev.c	2007-04-24 18:51:23.568804911 +0200
@@ -109,7 +109,7 @@
 #include <linux/netpoll.h>
 #include <linux/rcupdate.h>
 #include <linux/delay.h>
-#include <linux/wireless.h>
+#include <net/wext.h>
 #include <net/iw_handler.h>
 #include <asm/current.h>
 #include <linux/audit.h>
@@ -2348,12 +2348,6 @@ static const struct file_operations ptyp
 };
 
 
-#ifdef CONFIG_WIRELESS_EXT
-extern int wireless_proc_init(void);
-#else
-#define wireless_proc_init() 0
-#endif
-
 static int __init dev_proc_init(void)
 {
 	int rc = -ENOMEM;
@@ -2365,7 +2359,7 @@ static int __init dev_proc_init(void)
 	if (!proc_net_fops_create("ptype", S_IRUGO, &ptype_seq_fops))
 		goto out_dev2;
 
-	if (wireless_proc_init())
+	if (wext_proc_init())
 		goto out_softnet;
 	rc = 0;
 out:
@@ -2923,29 +2917,9 @@ int dev_ioctl(unsigned int cmd, void __u
 					ret = -EFAULT;
 				return ret;
 			}
-#ifdef CONFIG_WIRELESS_EXT
 			/* Take care of Wireless Extensions */
-			if (cmd >= SIOCIWFIRST && cmd <= SIOCIWLAST) {
-				/* If command is `set a parameter', or
-				 * `get the encoding parameters', check if
-				 * the user has the right to do it */
-				if (IW_IS_SET(cmd) || cmd == SIOCGIWENCODE
-				    || cmd == SIOCGIWENCODEEXT) {
-					if (!capable(CAP_NET_ADMIN))
-						return -EPERM;
-				}
-				dev_load(ifr.ifr_name);
-				rtnl_lock();
-				/* Follow me in net/wireless/wext.c */
-				ret = wireless_process_ioctl(&ifr, cmd);
-				rtnl_unlock();
-				if (IW_IS_GET(cmd) &&
-				    copy_to_user(arg, &ifr,
-						 sizeof(struct ifreq)))
-					ret = -EFAULT;
-				return ret;
-			}
-#endif	/* CONFIG_WIRELESS_EXT */
+			if (cmd >= SIOCIWFIRST && cmd <= SIOCIWLAST)
+				return wext_handle_ioctl(&ifr, cmd, arg);
 			return -EINVAL;
 	}
 }
--- net-2.6.22.orig/include/net/iw_handler.h	2007-04-24 18:54:01.678804911 +0200
+++ net-2.6.22/include/net/iw_handler.h	2007-04-24 18:56:27.328804911 +0200
@@ -431,16 +431,7 @@ struct iw_public_data {
  * Those may be called only within the kernel.
  */
 
-/* First : function strictly used inside the kernel */
-
-/* Handle /proc/net/wireless, called in net/code/dev.c */
-extern int dev_get_wireless_info(char * buffer, char **start, off_t offset,
-				 int length);
-
-/* Handle IOCTLs, called in net/core/dev.c */
-extern int wireless_process_ioctl(struct ifreq *ifr, unsigned int cmd);
-
-/* Second : functions that may be called by driver modules */
+/* functions that may be called by driver modules */
 
 /* Send a single event to user space */
 extern void wireless_send_event(struct net_device *	dev,
--- net-2.6.22.orig/net/wireless/wext.c	2007-04-24 18:51:31.288804911 +0200
+++ net-2.6.22/net/wireless/wext.c	2007-04-24 19:04:20.818804911 +0200
@@ -97,6 +97,7 @@
 #include <linux/wireless.h>		/* Pretty obvious */
 #include <net/iw_handler.h>		/* New driver API */
 #include <net/netlink.h>
+#include <net/wext.h>
 
 #include <asm/uaccess.h>		/* copy_to_user() */
 
@@ -696,7 +697,7 @@ static const struct file_operations wire
 	.release = seq_release,
 };
 
-int __init wireless_proc_init(void)
+int __init wext_proc_init(void)
 {
 	/* Create /proc/net/wireless entry */
 	if (!proc_net_fops_create("wireless", S_IRUGO, &wireless_seq_fops))
@@ -1075,11 +1076,10 @@ static inline int ioctl_private_call(str
 
 /* ---------------------------------------------------------------- */
 /*
- * Main IOCTl dispatcher. Called from the main networking code
- * (dev_ioctl() in net/core/dev.c).
+ * Main IOCTl dispatcher.
  * Check the type of IOCTL and call the appropriate wrapper...
  */
-int wireless_process_ioctl(struct ifreq *ifr, unsigned int cmd)
+static int wireless_process_ioctl(struct ifreq *ifr, unsigned int cmd)
 {
 	struct net_device *dev;
 	iw_handler	handler;
@@ -1143,6 +1143,26 @@ int wireless_process_ioctl(struct ifreq 
 	return -EINVAL;
 }
 
+/* entry point from dev ioctl */
+int wext_handle_ioctl(struct ifreq *ifr, unsigned int cmd,
+		      void __user *arg)
+{
+	int ret;
+
+	/* If command is `set a parameter', or
+	 * `get the encoding parameters', check if
+	 * the user has the right to do it */
+	if (IW_IS_SET(cmd) || cmd == SIOCGIWENCODE || cmd == SIOCGIWENCODEEXT)
+		if (!capable(CAP_NET_ADMIN))
+			return -EPERM;
+	dev_load(ifr->ifr_name);
+	rtnl_lock();
+	ret = wireless_process_ioctl(ifr, cmd);
+	rtnl_unlock();
+	if (IW_IS_GET(cmd) && copy_to_user(arg, ifr, sizeof(struct ifreq)))
+		return -EFAULT;
+	return ret;
+}
 
 /************************* EVENT PROCESSING *************************/
 /*

--

[PATCH 3/9] wext: remove dead debug code

[Johannes Berg]

This patch kills a whole bunch of code that can only ever be used by
defining some things in wext.c. Also, the things that are printed are
mostly useless since the API is fairly well-tested.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 net/wireless/wext.c |   92 ----------------------------------------------------
 1 file changed, 92 deletions(-)

--- net-2.6.22.orig/net/wireless/wext.c	2007-04-24 18:58:07.738804911 +0200
+++ net-2.6.22/net/wireless/wext.c	2007-04-24 18:59:00.998804911 +0200
@@ -103,11 +103,6 @@
 
 /**************************** CONSTANTS ****************************/
 
-/* Debugging stuff */
-#undef WE_IOCTL_DEBUG		/* Debug IOCTL API */
-#undef WE_EVENT_DEBUG		/* Debug Event dispatcher */
-#undef WE_SPY_DEBUG		/* Debug enhanced spy support */
-
 /* Options */
 #define WE_EVENT_RTNETLINK	/* Propagate events using RtNetlink */
 #define WE_SET_EVENT		/* Generate an event on some set commands */
@@ -736,12 +731,6 @@ static int ioctl_standard_call(struct ne
 		return -EOPNOTSUPP;
 	descr = &(standard_ioctl[cmd - SIOCIWFIRST]);
 
-#ifdef WE_IOCTL_DEBUG
-	printk(KERN_DEBUG "%s (WE) : Found standard handler for 0x%04X\n",
-	       ifr->ifr_name, cmd);
-	printk(KERN_DEBUG "%s (WE) : Header type : %d, Token type : %d, size : %d, token : %d\n", dev->name, descr->header_type, descr->token_type, descr->token_size, descr->max_tokens);
-#endif	/* WE_IOCTL_DEBUG */
-
 	/* Prepare the call */
 	info.cmd = cmd;
 	info.flags = 0;
@@ -832,11 +821,6 @@ static int ioctl_standard_call(struct ne
 			}
 		}
 
-#ifdef WE_IOCTL_DEBUG
-		printk(KERN_DEBUG "%s (WE) : Malloc %d bytes\n",
-		       dev->name, extra_size);
-#endif	/* WE_IOCTL_DEBUG */
-
 		/* Create the kernel buffer */
 		/*    kzalloc ensures NULL-termination for essid_compat */
 		extra = kzalloc(extra_size, GFP_KERNEL);
@@ -853,11 +837,6 @@ static int ioctl_standard_call(struct ne
 				kfree(extra);
 				return -EFAULT;
 			}
-#ifdef WE_IOCTL_DEBUG
-			printk(KERN_DEBUG "%s (WE) : Got %d bytes\n",
-			       dev->name,
-			       iwr->u.data.length * descr->token_size);
-#endif	/* WE_IOCTL_DEBUG */
 		}
 
 		/* Call the handler */
@@ -878,11 +857,6 @@ static int ioctl_standard_call(struct ne
 					   descr->token_size);
 			if (err)
 				ret =  -EFAULT;
-#ifdef WE_IOCTL_DEBUG
-			printk(KERN_DEBUG "%s (WE) : Wrote %d bytes\n",
-			       dev->name,
-			       iwr->u.data.length * descr->token_size);
-#endif	/* WE_IOCTL_DEBUG */
 		}
 
 #ifdef WE_SET_EVENT
@@ -947,16 +921,6 @@ static inline int ioctl_private_call(str
 			break;
 		}
 
-#ifdef WE_IOCTL_DEBUG
-	printk(KERN_DEBUG "%s (WE) : Found private handler for 0x%04X\n",
-	       ifr->ifr_name, cmd);
-	if (descr) {
-		printk(KERN_DEBUG "%s (WE) : Name %s, set %X, get %X\n",
-		       dev->name, descr->name,
-		       descr->set_args, descr->get_args);
-	}
-#endif	/* WE_IOCTL_DEBUG */
-
 	/* Compute the size of the set/get arguments */
 	if (descr != NULL) {
 		if (IW_IS_SET(cmd)) {
@@ -1013,11 +977,6 @@ static inline int ioctl_private_call(str
 				return -EFAULT;
 		}
 
-#ifdef WE_IOCTL_DEBUG
-		printk(KERN_DEBUG "%s (WE) : Malloc %d bytes\n",
-		       dev->name, extra_size);
-#endif	/* WE_IOCTL_DEBUG */
-
 		/* Always allocate for max space. Easier, and won't last
 		 * long... */
 		extra = kmalloc(extra_size, GFP_KERNEL);
@@ -1033,10 +992,6 @@ static inline int ioctl_private_call(str
 				kfree(extra);
 				return -EFAULT;
 			}
-#ifdef WE_IOCTL_DEBUG
-			printk(KERN_DEBUG "%s (WE) : Got %d elem\n",
-			       dev->name, iwr->u.data.length);
-#endif	/* WE_IOCTL_DEBUG */
 		}
 
 		/* Call the handler */
@@ -1056,10 +1011,6 @@ static inline int ioctl_private_call(str
 					   extra_size);
 			if (err)
 				ret =  -EFAULT;
-#ifdef WE_IOCTL_DEBUG
-			printk(KERN_DEBUG "%s (WE) : Wrote %d elem\n",
-			       dev->name, iwr->u.data.length);
-#endif	/* WE_IOCTL_DEBUG */
 		}
 
 		/* Cleanup - I told you it wasn't that long ;-) */
@@ -1319,11 +1270,6 @@ void wireless_send_event(struct net_devi
 		       dev->name, cmd);
 		return;
 	}
-#ifdef WE_EVENT_DEBUG
-	printk(KERN_DEBUG "%s (WE) : Got event 0x%04X\n",
-	       dev->name, cmd);
-	printk(KERN_DEBUG "%s (WE) : Header type : %d, Token type : %d, size : %d, token : %d\n", dev->name, descr->header_type, descr->token_type, descr->token_size, descr->max_tokens);
-#endif	/* WE_EVENT_DEBUG */
 
 	/* Check extra parameters and set extra_len */
 	if (descr->header_type == IW_HEADER_TYPE_POINT) {
@@ -1341,19 +1287,12 @@ void wireless_send_event(struct net_devi
 			extra_len = wrqu->data.length * descr->token_size;
 		/* Always at an offset in wrqu */
 		wrqu_off = IW_EV_POINT_OFF;
-#ifdef WE_EVENT_DEBUG
-		printk(KERN_DEBUG "%s (WE) : Event 0x%04X, tokens %d, extra_len %d\n", dev->name, cmd, wrqu->data.length, extra_len);
-#endif	/* WE_EVENT_DEBUG */
 	}
 
 	/* Total length of the event */
 	hdr_len = event_type_size[descr->header_type];
 	event_len = hdr_len + extra_len;
 
-#ifdef WE_EVENT_DEBUG
-	printk(KERN_DEBUG "%s (WE) : Event 0x%04X, hdr_len %d, wrqu_off %d, event_len %d\n", dev->name, cmd, hdr_len, wrqu_off, event_len);
-#endif	/* WE_EVENT_DEBUG */
-
 	/* Create temporary buffer to hold the event */
 	event = kmalloc(event_len, GFP_ATOMIC);
 	if (event == NULL)
@@ -1443,19 +1382,6 @@ int iw_handler_set_spy(struct net_device
 		/* Reset stats */
 		memset(spydata->spy_stat, 0,
 		       sizeof(struct iw_quality) * IW_MAX_SPY);
-
-#ifdef WE_SPY_DEBUG
-		printk(KERN_DEBUG "iw_handler_set_spy() :  wireless_data %p, spydata %p, num %d\n", dev->wireless_data, spydata, wrqu->data.length);
-		for (i = 0; i < wrqu->data.length; i++)
-			printk(KERN_DEBUG
-			       "%02X:%02X:%02X:%02X:%02X:%02X \n",
-			       spydata->spy_address[i][0],
-			       spydata->spy_address[i][1],
-			       spydata->spy_address[i][2],
-			       spydata->spy_address[i][3],
-			       spydata->spy_address[i][4],
-			       spydata->spy_address[i][5]);
-#endif	/* WE_SPY_DEBUG */
 	}
 
 	/* Make sure above is updated before re-enabling */
@@ -1525,10 +1451,6 @@ int iw_handler_set_thrspy(struct net_dev
 	/* Clear flag */
 	memset(spydata->spy_thr_under, '\0', sizeof(spydata->spy_thr_under));
 
-#ifdef WE_SPY_DEBUG
-	printk(KERN_DEBUG "iw_handler_set_thrspy() :  low %d ; high %d\n", spydata->spy_thr_low.level, spydata->spy_thr_high.level);
-#endif	/* WE_SPY_DEBUG */
-
 	return 0;
 }
 
@@ -1579,16 +1501,6 @@ static void iw_send_thrspy_event(struct 
 	memcpy(&(threshold.low), &(spydata->spy_thr_low),
 	       2 * sizeof(struct iw_quality));
 
-#ifdef WE_SPY_DEBUG
-	printk(KERN_DEBUG "iw_send_thrspy_event() : address %02X:%02X:%02X:%02X:%02X:%02X, level %d, up = %d\n",
-	       threshold.addr.sa_data[0],
-	       threshold.addr.sa_data[1],
-	       threshold.addr.sa_data[2],
-	       threshold.addr.sa_data[3],
-	       threshold.addr.sa_data[4],
-	       threshold.addr.sa_data[5], threshold.qual.level);
-#endif	/* WE_SPY_DEBUG */
-
 	/* Send event to user space */
 	wireless_send_event(dev, SIOCGIWTHRSPY, &wrqu, (char *) &threshold);
 }
@@ -1612,10 +1524,6 @@ void wireless_spy_update(struct net_devi
 	if (!spydata)
 		return;
 
-#ifdef WE_SPY_DEBUG
-	printk(KERN_DEBUG "wireless_spy_update() :  wireless_data %p, spydata %p, address %02X:%02X:%02X:%02X:%02X:%02X\n", dev->wireless_data, spydata, address[0], address[1], address[2], address[3], address[4], address[5]);
-#endif	/* WE_SPY_DEBUG */
-
 	/* Update all records that match */
 	for (i = 0; i < spydata->spy_number; i++)
 		if (!compare_ether_addr(address, spydata->spy_address[i])) {

--

Re: [PATCH 3/9] wext: remove dead debug code

[Jean Tourrilhes]

On Tue, Apr 24, 2007 at 08:07:35PM +0200, Johannes Berg wrote:
> This patch kills a whole bunch of code that can only ever be used by
> defining some things in wext.c. Also, the things that are printed are
> mostly useless since the API is fairly well-tested.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

	Hi,

	Overall ok with the patches. Comments on a few of them...
	I personally would not remove this DEBUG code, because it
decreases the overall maitainability of the code.
	First, it does not only help to debug the API, but it can be
used to debug userspace and drivers in some extreme cases.
	Second, this debugging act as documentation. He points out
which are the important variables and how they are used at critical
points of the code.
	As this debug is localised, and compiled out, it does not
bother anybody and should stay.

	Regards,

	Jean

Re: [PATCH 3/9] wext: remove dead debug code

[John W. Linville]

On Thu, Apr 26, 2007 at 09:46:32AM -0700, Jean Tourrilhes wrote:

> 	Overall ok with the patches. Comments on a few of them...
> 	I personally would not remove this DEBUG code, because it
> decreases the overall maitainability of the code.
> 	First, it does not only help to debug the API, but it can be
> used to debug userspace and drivers in some extreme cases.
> 	Second, this debugging act as documentation. He points out
> which are the important variables and how they are used at critical
> points of the code.
> 	As this debug is localised, and compiled out, it does not
> bother anybody and should stay.

Jean,

I think the code looks cleaner with the patch applied.  If the
debugging is valuable to you, I recommend that you save this patch
and reverse apply it before future work.

Thanks,

John
-- 
John W. Linville
linville@tuxdriver.com

[PATCH 4/9] wext: remove options

[Johannes Berg]

This patch kills the two options in wext that are required to be enabled
anyway because they influence the userspace API.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 net/wireless/wext.c |   15 ---------------
 1 file changed, 15 deletions(-)

--- net-2.6.22.orig/net/wireless/wext.c	2007-04-24 18:59:13.258804911 +0200
+++ net-2.6.22/net/wireless/wext.c	2007-04-24 18:59:34.138804911 +0200
@@ -101,12 +101,6 @@
 
 #include <asm/uaccess.h>		/* copy_to_user() */
 
-/**************************** CONSTANTS ****************************/
-
-/* Options */
-#define WE_EVENT_RTNETLINK	/* Propagate events using RtNetlink */
-#define WE_SET_EVENT		/* Generate an event on some set commands */
-
 /************************* GLOBAL VARIABLES *************************/
 /*
  * You should not use global variables, because of re-entrancy.
@@ -741,12 +735,10 @@ static int ioctl_standard_call(struct ne
 		/* No extra arguments. Trivial to handle */
 		ret = handler(dev, &info, &(iwr->u), NULL);
 
-#ifdef WE_SET_EVENT
 		/* Generate an event to notify listeners of the change */
 		if ((descr->flags & IW_DESCR_FLAG_EVENT) &&
 		   ((ret == 0) || (ret == -EIWCOMMIT)))
 			wireless_send_event(dev, cmd, &(iwr->u), NULL);
-#endif	/* WE_SET_EVENT */
 	} else {
 		char *	extra;
 		int	extra_size;
@@ -859,7 +851,6 @@ static int ioctl_standard_call(struct ne
 				ret =  -EFAULT;
 		}
 
-#ifdef WE_SET_EVENT
 		/* Generate an event to notify listeners of the change */
 		if ((descr->flags & IW_DESCR_FLAG_EVENT) &&
 		   ((ret == 0) || (ret == -EIWCOMMIT))) {
@@ -871,7 +862,6 @@ static int ioctl_standard_call(struct ne
 				wireless_send_event(dev, cmd, &(iwr->u),
 						    extra);
 		}
-#endif	/* WE_SET_EVENT */
 
 		/* Cleanup - I told you it wasn't that long ;-) */
 		kfree(extra);
@@ -1121,7 +1111,6 @@ int wext_handle_ioctl(struct ifreq *ifr,
  * Most often, the event will be propagated through rtnetlink
  */
 
-#ifdef WE_EVENT_RTNETLINK
 /* ---------------------------------------------------------------- */
 /*
  * Locking...
@@ -1225,8 +1214,6 @@ static inline void rtmsg_iwinfo(struct n
 	tasklet_schedule(&wireless_nlevent_tasklet);
 }
 
-#endif	/* WE_EVENT_RTNETLINK */
-
 /* ---------------------------------------------------------------- */
 /*
  * Main event dispatcher. Called from other parts and drivers.
@@ -1305,10 +1292,8 @@ void wireless_send_event(struct net_devi
 	if (extra != NULL)
 		memcpy(((char *) event) + hdr_len, extra, extra_len);
 
-#ifdef WE_EVENT_RTNETLINK
 	/* Send via the RtNetlink event channel */
 	rtmsg_iwinfo(dev, (char *) event, event_len);
-#endif	/* WE_EVENT_RTNETLINK */
 
 	/* Cleanup */
 	kfree(event);

--

[PATCH 5/9] wext: cleanup early ioctl call path

[Johannes Berg]

This patch makes the code in wireless_process_ioctl somewhat more readable.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 net/wireless/wext.c |   73 ++++++++++++++++++----------------------------------
 1 file changed, 26 insertions(+), 47 deletions(-)

--- net-2.6.22.orig/net/wireless/wext.c	2007-04-24 19:05:57.648804911 +0200
+++ net-2.6.22/net/wireless/wext.c	2007-04-24 19:10:51.038804911 +0200
@@ -1035,53 +1035,31 @@ static int wireless_process_ioctl(struct
 	/* A bunch of special cases, then the generic case...
 	 * Note that 'cmd' is already filtered in dev_ioctl() with
 	 * (cmd >= SIOCIWFIRST && cmd <= SIOCIWLAST) */
-	switch (cmd) {
-	case SIOCGIWSTATS:
-		/* Get Wireless Stats */
-		return ioctl_standard_call(dev,
-					   ifr,
-					   cmd,
+	if (cmd == SIOCGIWSTATS)
+		return ioctl_standard_call(dev, ifr, cmd,
 					   &iw_handler_get_iwstats);
 
-	case SIOCGIWPRIV:
-		/* Check if we have some wireless handlers defined */
-		if (dev->wireless_handlers != NULL) {
-			/* We export to user space the definition of
-			 * the private handler ourselves */
-			return ioctl_standard_call(dev,
-						   ifr,
-						   cmd,
-						   &iw_handler_get_private);
-		}
-		// ## Fall-through for old API ##
-	default:
-		/* Generic IOCTL */
-		/* Basic check */
-		if (!netif_device_present(dev))
-			return -ENODEV;
-		/* New driver API : try to find the handler */
-		handler = get_handler(dev, cmd);
-		if (handler != NULL) {
-			/* Standard and private are not the same */
-			if (cmd < SIOCIWFIRSTPRIV)
-				return ioctl_standard_call(dev,
-							   ifr,
-							   cmd,
-							   handler);
-			else
-				return ioctl_private_call(dev,
-							  ifr,
-							  cmd,
-							  handler);
-		}
-		/* Old driver API : call driver ioctl handler */
-		if (dev->do_ioctl) {
-			return dev->do_ioctl(dev, ifr, cmd);
-		}
-		return -EOPNOTSUPP;
+	if (cmd == SIOCGIWPRIV && dev->wireless_handlers)
+		return ioctl_standard_call(dev, ifr, cmd,
+					   &iw_handler_get_private);
+
+	/* Basic check */
+	if (!netif_device_present(dev))
+		return -ENODEV;
+
+	/* New driver API : try to find the handler */
+	handler = get_handler(dev, cmd);
+	if (handler) {
+		/* Standard and private are not the same */
+		if (cmd < SIOCIWFIRSTPRIV)
+			return ioctl_standard_call(dev, ifr, cmd, handler);
+		else
+			return ioctl_private_call(dev, ifr, cmd, handler);
 	}
-	/* Not reached */
-	return -EINVAL;
+	/* Old driver API : call driver ioctl handler */
+	if (dev->do_ioctl)
+		return dev->do_ioctl(dev, ifr, cmd);
+	return -EOPNOTSUPP;
 }
 
 /* entry point from dev ioctl */
@@ -1093,9 +1071,10 @@ int wext_handle_ioctl(struct ifreq *ifr,
 	/* If command is `set a parameter', or
 	 * `get the encoding parameters', check if
 	 * the user has the right to do it */
-	if (IW_IS_SET(cmd) || cmd == SIOCGIWENCODE || cmd == SIOCGIWENCODEEXT)
-		if (!capable(CAP_NET_ADMIN))
-			return -EPERM;
+	if ((IW_IS_SET(cmd) || cmd == SIOCGIWENCODE || cmd == SIOCGIWENCODEEXT)
+	    && !capable(CAP_NET_ADMIN))
+		return -EPERM;
+
 	dev_load(ifr->ifr_name);
 	rtnl_lock();
 	ret = wireless_process_ioctl(ifr, cmd);

--

[PATCH 6/9] wext: move EXPORT_SYMBOL statements where they belong

[Johannes Berg]

EXPORT_SYMBOL statements are supposed to go together with the symbol
they're exporting. This patch moves them accordingly.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 net/wireless/wext.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

--- net-2.6.22.orig/net/wireless/wext.c	2007-04-24 19:11:11.888804911 +0200
+++ net-2.6.22/net/wireless/wext.c	2007-04-24 19:12:44.288804911 +0200
@@ -1279,6 +1279,7 @@ void wireless_send_event(struct net_devi
 
 	return;		/* Always success, I guess ;-) */
 }
+EXPORT_SYMBOL(wireless_send_event);
 
 /********************** ENHANCED IWSPY SUPPORT **********************/
 /*
@@ -1356,6 +1357,7 @@ int iw_handler_set_spy(struct net_device
 
 	return 0;
 }
+EXPORT_SYMBOL(iw_handler_set_spy);
 
 /*------------------------------------------------------------------*/
 /*
@@ -1391,6 +1393,7 @@ int iw_handler_get_spy(struct net_device
 		spydata->spy_stat[i].updated &= ~IW_QUAL_ALL_UPDATED;
 	return 0;
 }
+EXPORT_SYMBOL(iw_handler_get_spy);
 
 /*------------------------------------------------------------------*/
 /*
@@ -1417,6 +1420,7 @@ int iw_handler_set_thrspy(struct net_dev
 
 	return 0;
 }
+EXPORT_SYMBOL(iw_handler_set_thrspy);
 
 /*------------------------------------------------------------------*/
 /*
@@ -1440,6 +1444,7 @@ int iw_handler_get_thrspy(struct net_dev
 
 	return 0;
 }
+EXPORT_SYMBOL(iw_handler_get_thrspy);
 
 /*------------------------------------------------------------------*/
 /*
@@ -1516,10 +1521,4 @@ void wireless_spy_update(struct net_devi
 		}
 	}
 }
-
-EXPORT_SYMBOL(iw_handler_get_spy);
-EXPORT_SYMBOL(iw_handler_get_thrspy);
-EXPORT_SYMBOL(iw_handler_set_spy);
-EXPORT_SYMBOL(iw_handler_set_thrspy);
-EXPORT_SYMBOL(wireless_send_event);
 EXPORT_SYMBOL(wireless_spy_update);

--

[PATCH 7/9] wext: reduce inline abuse

[Johannes Berg]

This patch removes a bunch of inline abuse from wext. Most functions
that were marked inline are only used once so the compiler will inline
them anyway, others are used multiple times but there's no requirement
for them to be inline since they aren't in any fast paths.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 net/wireless/wext.c |   29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

--- net-2.6.22.orig/net/wireless/wext.c	2007-04-24 19:13:50.288804911 +0200
+++ net-2.6.22/net/wireless/wext.c	2007-04-24 19:18:49.198804911 +0200
@@ -440,10 +440,8 @@ static const int event_type_pk_size[] = 
 /* ---------------------------------------------------------------- */
 /*
  * Return the driver handler associated with a specific Wireless Extension.
- * Called from various place, so make sure it remains efficient.
  */
-static inline iw_handler get_handler(struct net_device *dev,
-				     unsigned int cmd)
+static iw_handler get_handler(struct net_device *dev, unsigned int cmd)
 {
 	/* Don't "optimise" the following variable, it will crash */
 	unsigned int	index;		/* *MUST* be unsigned */
@@ -470,7 +468,7 @@ static inline iw_handler get_handler(str
 /*
  * Get statistics out of the driver
  */
-static inline struct iw_statistics *get_wireless_stats(struct net_device *dev)
+static struct iw_statistics *get_wireless_stats(struct net_device *dev)
 {
 	/* New location */
 	if ((dev->wireless_handlers != NULL) &&
@@ -500,7 +498,7 @@ static inline struct iw_statistics *get_
  * netif_running(dev) test. I'm open on that one...
  * Hopefully, the driver will remember to do a commit in "open()" ;-)
  */
-static inline int call_commit_handler(struct net_device *	dev)
+static int call_commit_handler(struct net_device *dev)
 {
 	if ((netif_running(dev)) &&
 	   (dev->wireless_handlers->standard[0] != NULL)) {
@@ -622,8 +620,8 @@ static int iw_handler_get_private(struct
 /*
  * Print one entry (line) of /proc/net/wireless
  */
-static __inline__ void wireless_seq_printf_stats(struct seq_file *seq,
-						 struct net_device *dev)
+static void wireless_seq_printf_stats(struct seq_file *seq,
+				      struct net_device *dev)
 {
 	/* Get stats from the driver */
 	struct iw_statistics *stats = get_wireless_stats(dev);
@@ -892,10 +890,8 @@ static int ioctl_standard_call(struct ne
  * a iw_handler but process it in your ioctl handler (i.e. use the
  * old driver API).
  */
-static inline int ioctl_private_call(struct net_device *	dev,
-				     struct ifreq *		ifr,
-				     unsigned int		cmd,
-				     iw_handler		handler)
+static int ioctl_private_call(struct net_device *dev, struct ifreq *ifr,
+			      unsigned int cmd, iw_handler handler)
 {
 	struct iwreq *			iwr = (struct iwreq *) ifr;
 	const struct iw_priv_args *	descr = NULL;
@@ -1134,11 +1130,8 @@ static DECLARE_TASKLET(wireless_nlevent_
  * current wireless config. Dumping the wireless config is far too
  * expensive (for each parameter, the driver need to query the hardware).
  */
-static inline int rtnetlink_fill_iwinfo(struct sk_buff *	skb,
-					struct net_device *	dev,
-					int			type,
-					char *			event,
-					int			event_len)
+static int rtnetlink_fill_iwinfo(struct sk_buff *skb, struct net_device *dev,
+				 int type, char *event, int event_len)
 {
 	struct ifinfomsg *r;
 	struct nlmsghdr  *nlh;
@@ -1172,9 +1165,7 @@ rtattr_failure:
  * Andrzej Krzysztofowicz mandated that I used a IFLA_XXX field
  * within a RTM_NEWLINK event.
  */
-static inline void rtmsg_iwinfo(struct net_device *	dev,
-				char *			event,
-				int			event_len)
+static void rtmsg_iwinfo(struct net_device *dev, char *event, int event_len)
 {
 	struct sk_buff *skb;
 	int size = NLMSG_GOODSIZE;

--

Re: [PATCH 7/9] wext: reduce inline abuse

[Jean Tourrilhes]

On Tue, Apr 24, 2007 at 08:07:39PM +0200, Johannes Berg wrote:
> This patch removes a bunch of inline abuse from wext. Most functions
> that were marked inline are only used once so the compiler will inline
> them anyway, others are used multiple times but there's no requirement
> for them to be inline since they aren't in any fast paths.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

	That's clearly not true of all compilers. All gcc versions
before 4.0 need serious help to inline functions used only once. Our
current minimal requirement for the kernel is gcc 3.2, therefore this
code is still useful.
	Note that this is a legitimate use of inline (tell the
compiler to inline the function), not an abuse.

	Jean

Re: [PATCH 7/9] wext: reduce inline abuse

[Michael Buesch]

On Thursday 26 April 2007 18:50:32 Jean Tourrilhes wrote:
> On Tue, Apr 24, 2007 at 08:07:39PM +0200, Johannes Berg wrote:
> > This patch removes a bunch of inline abuse from wext. Most functions
> > that were marked inline are only used once so the compiler will inline
> > them anyway, others are used multiple times but there's no requirement
> > for them to be inline since they aren't in any fast paths.
> > 
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> 
> 	That's clearly not true of all compilers. All gcc versions
> before 4.0 need serious help to inline functions used only once. Our
> current minimal requirement for the kernel is gcc 3.2, therefore this
> code is still useful.
> 	Note that this is a legitimate use of inline (tell the
> compiler to inline the function), not an abuse.

By my personal definition _every_ use of inline is abuse, if it's not
in an absolute fastpath and applied to a really tiny function.
Sure, other people have different opinions on that, but I think
with my approach we get smallest code with good speed.
In general I try to avoid inline whereever possible.
I think this patch is OK and should go in.

Often it's even desired to have out of line functions in fastpaths.
See spinlocks.

-- 
Greetings Michael.

Re: [PATCH 7/9] wext: reduce inline abuse

[Jean Tourrilhes]

On Thu, Apr 26, 2007 at 07:03:27PM +0200, Michael Buesch wrote:
> 
> Sure, other people have different opinions on that, but I think
> with my approach we get smallest code with good speed.

	Try with gcc-3.3 if you don't trust me. Your patch will
produce bigger and slower code. Thanks.

	Jean

Re: [PATCH 7/9] wext: reduce inline abuse

[David Miller]

From: Jean Tourrilhes <jt@hpl.hp.com>
Date: Thu, 26 Apr 2007 10:15:17 -0700

> On Thu, Apr 26, 2007 at 07:03:27PM +0200, Michael Buesch wrote:
> > 
> > Sure, other people have different opinions on that, but I think
> > with my approach we get smallest code with good speed.
> 
> 	Try with gcc-3.3 if you don't trust me. Your patch will
> produce bigger and slower code. Thanks.

A broken compiler is not an argument for keeping all of these
bogus inlines around.

It is the compilers job to optimize things correctly, and current
gcc's do.

We've always had this approach to dealing with compiler "issues".

Re: [PATCH 7/9] wext: reduce inline abuse

[Johannes Berg]

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

On Thu, 2007-04-26 at 09:50 -0700, Jean Tourrilhes wrote:

> 	That's clearly not true of all compilers. All gcc versions
> before 4.0 need serious help to inline functions used only once. Our
> current minimal requirement for the kernel is gcc 3.2, therefore this
> code is still useful.
> 	Note that this is a legitimate use of inline (tell the
> compiler to inline the function), not an abuse.

No, the abuse is using inline in the first place because it doesn't
matter, none of this has an actual reason to be inlined.

johannes

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

[PATCH 8/9] wext: misc code cleanups

[Johannes Berg]

Just a few things that didn't fit in with the other patches.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 net/wireless/wext.c |   28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

--- net-2.6.22.orig/net/wireless/wext.c	2007-04-24 19:18:49.198804911 +0200
+++ net-2.6.22/net/wireless/wext.c	2007-04-24 19:25:46.768804911 +0200
@@ -476,7 +476,7 @@ static struct iw_statistics *get_wireles
 		return dev->wireless_handlers->get_wireless_stats(dev);
 
 	/* Not found */
-	return (struct iw_statistics *) NULL;
+	return NULL;
 }
 
 /* ---------------------------------------------------------------- */
@@ -501,11 +501,11 @@ static struct iw_statistics *get_wireles
 static int call_commit_handler(struct net_device *dev)
 {
 	if ((netif_running(dev)) &&
-	   (dev->wireless_handlers->standard[0] != NULL)) {
+	   (dev->wireless_handlers->standard[0] != NULL))
 		/* Call the commit handler on the driver */
 		return dev->wireless_handlers->standard[0](dev, NULL,
 							   NULL, NULL);
-	} else
+	else
 		return 0;		/* Command completed successfully */
 }
 
@@ -554,8 +554,7 @@ static int iw_handler_get_iwstats(struct
 	struct iw_statistics *stats;
 
 	stats = get_wireless_stats(dev);
-	if (stats != (struct iw_statistics *) NULL) {
-
+	if (stats) {
 		/* Copy statistics to extra */
 		memcpy(extra, stats, sizeof(struct iw_statistics));
 		wrqu->data.length = sizeof(struct iw_statistics);
@@ -814,9 +813,8 @@ static int ioctl_standard_call(struct ne
 		/* Create the kernel buffer */
 		/*    kzalloc ensures NULL-termination for essid_compat */
 		extra = kzalloc(extra_size, GFP_KERNEL);
-		if (extra == NULL) {
+		if (extra == NULL)
 			return -ENOMEM;
-		}
 
 		/* If it is a SET, get all the extra data in here */
 		if (IW_IS_SET(cmd) && (iwr->u.data.length != 0)) {
@@ -957,18 +955,14 @@ static int ioctl_private_call(struct net
 			if (iwr->u.data.length > (descr->set_args &
 						 IW_PRIV_SIZE_MASK))
 				return -E2BIG;
-		} else {
-			/* Check NULL pointer */
-			if (iwr->u.data.pointer == NULL)
-				return -EFAULT;
-		}
+		} else if (iwr->u.data.pointer == NULL)
+			return -EFAULT;
 
 		/* Always allocate for max space. Easier, and won't last
 		 * long... */
 		extra = kmalloc(extra_size, GFP_KERNEL);
-		if (extra == NULL) {
+		if (extra == NULL)
 			return -ENOMEM;
-		}
 
 		/* If it is a SET, get all the extra data in here */
 		if (IW_IS_SET(cmd) && (iwr->u.data.length != 0)) {
@@ -1259,7 +1253,7 @@ void wireless_send_event(struct net_devi
 	event->len = event_len;
 	event->cmd = cmd;
 	memcpy(&event->u, ((char *) wrqu) + wrqu_off, hdr_len - IW_EV_LCP_LEN);
-	if (extra != NULL)
+	if (extra)
 		memcpy(((char *) event) + hdr_len, extra, extra_len);
 
 	/* Send via the RtNetlink event channel */
@@ -1290,11 +1284,11 @@ EXPORT_SYMBOL(wireless_send_event);
  * Because this is called on the Rx path via wireless_spy_update(),
  * we want it to be efficient...
  */
-static inline struct iw_spy_data * get_spydata(struct net_device *dev)
+static inline struct iw_spy_data *get_spydata(struct net_device *dev)
 {
 	/* This is the new way */
 	if (dev->wireless_data)
-		return(dev->wireless_data->spy_data);
+		return dev->wireless_data->spy_data;
 	return NULL;
 }
 

--

[PATCH 9/9] net_device: dont include wext bits if not required

[Johannes Berg]

This patch makes the wext bits in struct net_device depend on
CONFIG_WIRELESS_EXT.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
 include/linux/netdevice.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- net-2.6.22.orig/include/linux/netdevice.h	2007-04-24 19:36:59.248804911 +0200
+++ net-2.6.22/include/linux/netdevice.h	2007-04-24 19:37:31.608804911 +0200
@@ -352,12 +352,13 @@ struct net_device
 	struct net_device_stats* (*get_stats)(struct net_device *dev);
 	struct net_device_stats	stats;
 
+#ifdef CONFIG_WIRELESS_EXT
 	/* List of functions to handle Wireless Extensions (instead of ioctl).
 	 * See <net/iw_handler.h> for details. Jean II */
 	const struct iw_handler_def *	wireless_handlers;
 	/* Instance data managed by the core of Wireless Extensions. */
 	struct iw_public_data *	wireless_data;
-
+#endif
 	const struct ethtool_ops *ethtool_ops;
 
 	/*

--

Re: [PATCH 9/9] net_device: dont include wext bits if not required

[Jean Tourrilhes]

On Tue, Apr 24, 2007 at 08:07:41PM +0200, Johannes Berg wrote:
> This patch makes the wext bits in struct net_device depend on
> CONFIG_WIRELESS_EXT.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

	I personally would not do that. Having conditional fields in
"struct net_device" is very bad, it's a sure way to crash on modules
in some cases. If I remember well, Jeff Garzik has been fighting those
over the years.

	Jean

Re: [PATCH 9/9] net_device: dont include wext bits if not required

[Johannes Berg]

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

On Thu, 2007-04-26 at 09:53 -0700, Jean Tourrilhes wrote:

> 	I personally would not do that. Having conditional fields in
> "struct net_device" is very bad, it's a sure way to crash on modules
> in some cases. If I remember well, Jeff Garzik has been fighting those
> over the years.

I'm fine with posting a patch the other way around (i.e. removing the
conditional from wireless-dev) just wanted to float this. It originally
made much more sense anyway when I had wanted to do cfg80211/wext
compatibility in a different way. Now it looks like it'll just depend on
wext.

johannes

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

Re: [PATCH 9/9] net_device: dont include wext bits if not required

[John W. Linville]

On Thu, Apr 26, 2007 at 09:53:04AM -0700, Jean Tourrilhes wrote:
> On Tue, Apr 24, 2007 at 08:07:41PM +0200, Johannes Berg wrote:
> > This patch makes the wext bits in struct net_device depend on
> > CONFIG_WIRELESS_EXT.
> > 
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> 
> 	I personally would not do that. Having conditional fields in
> "struct net_device" is very bad, it's a sure way to crash on modules
> in some cases. If I remember well, Jeff Garzik has been fighting those
> over the years.

I'm not sure I understand the reasoning here.  Without a doubt kernel
modules need to be built against the same configuration as the kernels
that are loading them.  So, conditional component of net_device should
not cause crashes.

FWIW, there are other conditionally compiled portions of net_device
(NETPOLL, NET_POLL_CONTROLLER).  I don't see the harm here.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH 0/9] various wext cleanups

[Johannes Berg]

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

On Tue, 2007-04-24 at 20:07 +0200, Johannes Berg wrote:
> This patch series contains various cleanups to the wireless extensions code.

Patch 1/9 got lost because it's too large, but it's only moving
net/core/wireless.c to net/wireless/wext.c and doing some tiny Makefile
updates.

johannes

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

Re: [PATCH 0/9] various wext cleanups

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 26 Apr 2007 12:15:13 +0200

> On Tue, 2007-04-24 at 20:07 +0200, Johannes Berg wrote:
> > This patch series contains various cleanups to the wireless extensions code.
> 
> Patch 1/9 got lost because it's too large, but it's only moving
> net/core/wireless.c to net/wireless/wext.c and doing some tiny Makefile
> updates.

I got the bounce so I have the changelog too :-)

Re: [PATCH 0/9] various wext cleanups

[Johannes Berg]

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

On Thu, 2007-04-26 at 03:18 -0700, David Miller wrote:

> I got the bounce so I have the changelog too :-)

Sorry. I should've known and just posted git instructions. You should
have gotten a copy directly to your inbox too.

johannes

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

Re: [PATCH 0/9] various wext cleanups

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 24 Apr 2007 20:07:32 +0200

> This patch series contains various cleanups to the wireless extensions code.

I'll wait for Linville to ACK this stuff, then I'll apply it.

Thanks.

Re: [PATCH 0/9] various wext cleanups

[John W. Linville]

On Thu, Apr 26, 2007 at 03:19:21AM -0700, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Tue, 24 Apr 2007 20:07:32 +0200
> 
> > This patch series contains various cleanups to the wireless extensions code.
> 
> I'll wait for Linville to ACK this stuff, then I'll apply it.

I'm OK with the whole series.

Signed-off-by: John W. Linville <linville@tuxdriver.com>

Good enough?  Or would you prefer a git pull request?

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH 0/9] various wext cleanups

[David Miller]

From: "John W. Linville" <linville@tuxdriver.com>
Date: Thu, 26 Apr 2007 21:12:51 -0400

> I'm OK with the whole series.
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 
> Good enough?  Or would you prefer a git pull request?

Good enough for me.

Thanks John.

Re: [PATCH 0/9] various wext cleanups

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 24 Apr 2007 20:07:32 +0200

> This patch series contains various cleanups to the wireless extensions code.

All applied, thanks Johannes.