[PATCH 2.6.18] WE-21 support (core API)

[PATCH 2.6.18] WE-21 support (core API)

[Jean Tourrilhes]

	Hi John,

	This is version 21 of the Wireless Extensions. Changelog :
		o finishes migrating the ESSID API (remove the +1)
		o netdev->get_wireless_stats is no more
		o modulation
		o long/short retry
		o relative power saving.
	Only the first item is controversial, but it was agreed on
this list a few months ago. Actually, those patches have been on my
web page for a few months, waiting for users-space chages to propagate
to the distros.

	I've rediffed and retested with 2.6.18-rc5, and also made the
patches for the latest wireless-2.6 git. Core API patch included here,
mandatory driver patches to follow (to reflect API changes), extra
driver patches to be sent to the respective maintainers later. Of
course, out-of-tree drivers need #ifdef not included here...
	Would you mind sending that into Linus's kernel at the next
opportunity, for example for 2.6.19-pre.
        Have fun...

        Jean

Signed-off-by: Jean Tourrilhes <jt@hpl.hp.com>

-----------------------------------------------------------

diff -u -p linux/include/linux/wireless.20.h linux/include/linux/wireless.h
--- linux/include/linux/wireless.20.h	2006-08-28 15:00:30.000000000 -0700
+++ linux/include/linux/wireless.h	2006-08-28 15:09:59.000000000 -0700
@@ -1,7 +1,7 @@
 /*
  * This file define a set of standard wireless extensions
  *
- * Version :	20	17.2.06
+ * Version :	21	14.3.06
  *
  * Authors :	Jean Tourrilhes - HPL - <jt@hpl.hp.com>
  * Copyright (c) 1997-2006 Jean Tourrilhes, All Rights Reserved.
@@ -69,9 +69,14 @@
 
 /***************************** INCLUDES *****************************/
 
+/* This header is used in user-space, therefore need to be sanitised
+ * for that purpose. Those includes are usually not compatible with glibc.
+ * To know which includes to use in user-space, check iwlib.h. */
+#ifdef __KERNEL__
 #include <linux/types.h>		/* for "caddr_t" et al		*/
 #include <linux/socket.h>		/* for "struct sockaddr" et al	*/
 #include <linux/if.h>			/* for IFNAMSIZ and co... */
+#endif	/* __KERNEL__ */
 
 /***************************** VERSION *****************************/
 /*
@@ -80,7 +85,7 @@
  * (there is some stuff that will be added in the future...)
  * I just plan to increment with each new version.
  */
-#define WIRELESS_EXT	20
+#define WIRELESS_EXT	21
 
 /*
  * Changes :
@@ -208,6 +213,17 @@
  * V19 to V20
  * ----------
  *	- RtNetlink requests support (SET/GET)
+ *
+ * V20 to V21
+ * ----------
+ *	- Remove (struct net_device *)->get_wireless_stats()
+ *	- Change length in ESSID and NICK to strlen() instead of strlen()+1
+ *	- Add SIOCSIWMODUL/SIOCGIWMODUL for modulation setting
+ *	- Add IW_RETRY_SHORT/IW_RETRY_LONG retry modifiers
+ *	- Add IW_POWER_SAVING power type
+ *	- Power/Retry relative values no longer * 100000
+ *	- Add bitrate flags for unicast/broadcast
+ *	- Add explicit flag to tell stats are in 802.11k RCPI : IW_QUAL_RCPI
  */
 
 /**************************** CONSTANTS ****************************/
@@ -281,6 +297,9 @@
 /* Power saving stuff (power management, unicast and multicast) */
 #define SIOCSIWPOWER	0x8B2C		/* set Power Management settings */
 #define SIOCGIWPOWER	0x8B2D		/* get Power Management settings */
+/* Modulation bitmask */
+#define SIOCSIWMODUL	0x8B2E		/* set Modulations settings */
+#define SIOCGIWMODUL	0x8B2F		/* get Modulations settings */
 
 /* WPA : Generic IEEE 802.11 informatiom element (e.g., for WPA/RSN/WMM).
  * This ioctl uses struct iw_point and data buffer that includes IE id and len
@@ -448,6 +467,7 @@
 #define IW_QUAL_QUAL_INVALID	0x10	/* Driver doesn't provide value */
 #define IW_QUAL_LEVEL_INVALID	0x20
 #define IW_QUAL_NOISE_INVALID	0x40
+#define IW_QUAL_RCPI		0x80	/* Level + Noise are 802.11k RCPI */
 #define IW_QUAL_ALL_INVALID	0x70
 
 /* Frequency flags */
@@ -477,6 +497,7 @@
 #define IW_POWER_TYPE		0xF000	/* Type of parameter */
 #define IW_POWER_PERIOD		0x1000	/* Value is a period/duration of  */
 #define IW_POWER_TIMEOUT	0x2000	/* Value is a timeout (to go asleep) */
+#define IW_POWER_SAVING		0x4000	/* Value is relative (how aggressive)*/
 #define IW_POWER_MODE		0x0F00	/* Power Management mode */
 #define IW_POWER_UNICAST_R	0x0100	/* Receive only unicast messages */
 #define IW_POWER_MULTICAST_R	0x0200	/* Receive only multicast messages */
@@ -500,10 +521,12 @@
 #define IW_RETRY_TYPE		0xF000	/* Type of parameter */
 #define IW_RETRY_LIMIT		0x1000	/* Maximum number of retries*/
 #define IW_RETRY_LIFETIME	0x2000	/* Maximum duration of retries in us */
-#define IW_RETRY_MODIFIER	0x000F	/* Modify a parameter */
+#define IW_RETRY_MODIFIER	0x00FF	/* Modify a parameter */
 #define IW_RETRY_MIN		0x0001	/* Value is a minimum  */
 #define IW_RETRY_MAX		0x0002	/* Value is a maximum */
 #define IW_RETRY_RELATIVE	0x0004	/* Value is not in seconds/ms/us */
+#define IW_RETRY_SHORT		0x0010	/* Value is for short packets  */
+#define IW_RETRY_LONG		0x0020	/* Value is for long packets */
 
 /* Scanning request flags */
 #define IW_SCAN_DEFAULT		0x0000	/* Default scan of the driver */
@@ -621,6 +644,27 @@
 #define IW_EVENT_CAPA_SET(event_capa, cmd) (event_capa[IW_EVENT_CAPA_INDEX(cmd)] |= IW_EVENT_CAPA_MASK(cmd))
 #define IW_EVENT_CAPA_SET_KERNEL(event_capa) {event_capa[0] |= IW_EVENT_CAPA_K_0; event_capa[1] |= IW_EVENT_CAPA_K_1; }
 
+/* Modulations bitmasks */
+#define IW_MODUL_ALL		0x00000000	/* Everything supported */
+#define IW_MODUL_FH		0x00000001	/* Frequency Hopping */
+#define IW_MODUL_DS		0x00000002	/* Original Direct Sequence */
+#define IW_MODUL_CCK		0x00000004	/* 802.11b : 5.5 + 11 Mb/s */
+#define IW_MODUL_11B		(IW_MODUL_DS | IW_MODUL_CCK)
+#define IW_MODUL_PBCC		0x00000008	/* TI : 5.5 + 11 + 22 Mb/s */
+#define IW_MODUL_OFDM_A		0x00000010	/* 802.11a : 54 Mb/s */
+#define IW_MODUL_11A		(IW_MODUL_OFDM_A)
+#define IW_MODUL_11AB		(IW_MODUL_11B | IW_MODUL_11A)
+#define IW_MODUL_OFDM_G		0x00000020	/* 802.11g : 54 Mb/s */
+#define IW_MODUL_11G		(IW_MODUL_11B | IW_MODUL_OFDM_G)
+#define IW_MODUL_11AG		(IW_MODUL_11G | IW_MODUL_11A)
+#define IW_MODUL_TURBO		0x00000040	/* ATH : bonding, 108 Mb/s */
+/* In here we should define MIMO stuff. Later... */
+#define IW_MODUL_CUSTOM		0x40000000	/* Driver specific */
+
+/* Bitrate flags available */
+#define IW_BITRATE_TYPE		0x00FF	/* Type of value */
+#define IW_BITRATE_UNICAST	0x0001	/* Maximum/Fixed unicast bitrate */
+#define IW_BITRATE_BROADCAST	0x0002	/* Fixed broadcast bitrate */
 
 /****************************** TYPES ******************************/
 
@@ -1017,7 +1061,18 @@ struct	iw_range
 	/* Note : this frequency list doesn't need to fit channel numbers,
 	 * because each entry contain its channel index */
 
-	__u32		enc_capa; /* IW_ENC_CAPA_* bit field */
+	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
+
+	/* More power management stuff */
+	__s32		min_pms;	/* Minimal PM saving */
+	__s32		max_pms;	/* Maximal PM saving */
+	__u16		pms_flags;	/* How to decode max/min PM saving */
+
+	/* All available modulations for driver (hw may support less) */
+	__s32		modul_capa;	/* IW_MODUL_* bit field */
+
+	/* More bitrate stuff */
+	__u32		bitrate_capa;	/* Types of bitrates supported */
 };
 
 /*
diff -u -p linux/include/linux/netdevice.20.h linux/include/linux/netdevice.h
--- linux/include/linux/netdevice.20.h	2006-08-28 15:00:45.000000000 -0700
+++ linux/include/linux/netdevice.h	2006-08-28 15:01:21.000000000 -0700
@@ -334,7 +334,6 @@ struct net_device
 
 
 	struct net_device_stats* (*get_stats)(struct net_device *dev);
-	struct iw_statistics*	(*get_wireless_stats)(struct net_device *dev);
 
 	/* List of functions to handle Wireless Extensions (instead of ioctl).
 	 * See <net/iw_handler.h> for details. Jean II */
diff -u -p linux/net/core/net-sysfs.20.c linux/net/core/net-sysfs.c
--- linux/net/core/net-sysfs.20.c	2006-08-28 15:00:55.000000000 -0700
+++ linux/net/core/net-sysfs.c	2006-08-28 15:04:15.000000000 -0700
@@ -344,8 +344,6 @@ static ssize_t wireless_show(struct clas
 		if(dev->wireless_handlers &&
 		   dev->wireless_handlers->get_wireless_stats)
 			iw = dev->wireless_handlers->get_wireless_stats(dev);
-		else if (dev->get_wireless_stats)
-			iw = dev->get_wireless_stats(dev);
 		if (iw != NULL)
 			ret = (*format)(iw, buf);
 	}
@@ -465,8 +463,7 @@ int netdev_register_sysfs(struct net_dev
 		*groups++ = &netstat_group;
 
 #ifdef WIRELESS_EXT
-	if (net->get_wireless_stats
-	    || (net->wireless_handlers && net->wireless_handlers->get_wireless_stats))
+	if (net->wireless_handlers && net->wireless_handlers->get_wireless_stats)
 		*groups++ = &wireless_group;
 #endif
 
diff -u -p linux/net/core/wireless.20.c linux/net/core/wireless.c
--- linux/net/core/wireless.20.c	2006-08-28 14:31:33.000000000 -0700
+++ linux/net/core/wireless.c	2006-08-28 15:02:30.000000000 -0700
@@ -68,6 +68,15 @@
  *
  * v8 - 17.02.06 - Jean II
  *	o RtNetlink requests support (SET/GET)
+ *
+ * v8b - 03.08.06 - Herbert Xu
+ *	o Fix Wireless Event locking issues.
+ *
+ * v9 - 14.3.06 - Jean II
+ *	o Change length in ESSID and NICK to strlen() instead of strlen()+1
+ *	o Add SIOCSIWMODUL/SIOCGIWMODUL for modulation setting
+ *	o Make standard_ioctl_num and standard_event_num unsigned
+ *	o Remove (struct net_device *)->get_wireless_stats()
  */
 
 /***************************** INCLUDES *****************************/
@@ -234,24 +243,24 @@ static const struct iw_ioctl_description
 	[SIOCSIWESSID	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 		.flags		= IW_DESCR_FLAG_EVENT,
 	},
 	[SIOCGIWESSID	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 		.flags		= IW_DESCR_FLAG_DUMP,
 	},
 	[SIOCSIWNICKN	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 	},
 	[SIOCGIWNICKN	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 	},
 	[SIOCSIWRATE	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_PARAM,
@@ -301,6 +310,12 @@ static const struct iw_ioctl_description
 	[SIOCGIWPOWER	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_PARAM,
 	},
+	[SIOCSIWMODUL	- SIOCIWFIRST] = {
+		.header_type	= IW_HEADER_TYPE_PARAM,
+	},
+	[SIOCGIWMODUL	- SIOCIWFIRST] = {
+		.header_type	= IW_HEADER_TYPE_PARAM,
+	},
 	[SIOCSIWGENIE	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
@@ -338,8 +353,8 @@ static const struct iw_ioctl_description
 		.max_tokens	= sizeof(struct iw_pmksa),
 	},
 };
-static const int standard_ioctl_num = (sizeof(standard_ioctl) /
-				       sizeof(struct iw_ioctl_description));
+static const unsigned standard_ioctl_num = (sizeof(standard_ioctl) /
+					    sizeof(struct iw_ioctl_description));
 
 /*
  * Meta-data about all the additional standard Wireless Extension events
@@ -389,8 +404,8 @@ static const struct iw_ioctl_description
 		.max_tokens	= sizeof(struct iw_pmkid_cand),
 	},
 };
-static const int standard_event_num = (sizeof(standard_event) /
-				       sizeof(struct iw_ioctl_description));
+static const unsigned standard_event_num = (sizeof(standard_event) /
+					    sizeof(struct iw_ioctl_description));
 
 /* Size (in bytes) of the various private data types */
 static const char iw_priv_type_size[] = {
@@ -465,17 +480,6 @@ static inline struct iw_statistics *get_
 	   (dev->wireless_handlers->get_wireless_stats != NULL))
 		return dev->wireless_handlers->get_wireless_stats(dev);
 
-	/* Old location, field to be removed in next WE */
-	if(dev->get_wireless_stats) {
-		static int printed_message;
-
-		if (!printed_message++)
-			printk(KERN_DEBUG "%s (WE) : Driver using old /proc/net/wireless support, please fix driver !\n",
-				dev->name);
-
-		return dev->get_wireless_stats(dev);
-	}
-
 	/* Not found */
 	return (struct iw_statistics *) NULL;
 }
@@ -1843,8 +1847,33 @@ int wireless_rtnetlink_set(struct net_de
  */
 
 #ifdef WE_EVENT_RTNETLINK
+/* ---------------------------------------------------------------- */
+/*
+ * Locking...
+ * ----------
+ *
+ * Thanks to Herbert Xu <herbert@gondor.apana.org.au> for fixing
+ * the locking issue in here and implementing this code !
+ *
+ * The issue : wireless_send_event() is often called in interrupt context,
+ * while the Netlink layer can never be called in interrupt context.
+ * The fully formed RtNetlink events are queued, and then a tasklet is run
+ * to feed those to Netlink.
+ * The skb_queue is interrupt safe, and its lock is not held while calling
+ * Netlink, so there is no possibility of dealock.
+ * Jean II
+ */
+
 static struct sk_buff_head wireless_nlevent_queue;
 
+static int __init wireless_nlevent_init(void)
+{
+	skb_queue_head_init(&wireless_nlevent_queue);
+	return 0;
+}
+
+subsys_initcall(wireless_nlevent_init);
+
 static void wireless_nlevent_process(unsigned long data)
 {
 	struct sk_buff *skb;
@@ -1921,13 +1950,6 @@ static inline void rtmsg_iwinfo(struct n
 	tasklet_schedule(&wireless_nlevent_tasklet);
 }
 
-static int __init wireless_nlevent_init(void)
-{
-	skb_queue_head_init(&wireless_nlevent_queue);
-	return 0;
-}
-
-subsys_initcall(wireless_nlevent_init);
 #endif	/* WE_EVENT_RTNETLINK */
 
 /* ---------------------------------------------------------------- */

Re: [PATCH 2.6.18] WE-21 support (core API)

[Johannes Berg]

On Tue, 2006-08-29 at 17:56 -0700, Jean Tourrilhes wrote:
> 		o modulation
> 		o long/short retry
> 		o relative power saving.

I strongly disagree to these.

What's the point of adding more ioctls that we'll be implementing them
as wrappers around nl80211? Right now, those new ioctls/options aren't
implemented in *any* driver at all so they're completely useless, and
just add more to the pile of historic baggage we end up carrying around.
If we add these to mainline now, it's another thing we'll have to carry
for a long time even though it currently has no users...

I'd much prefer merging nl80211 and putting any really *new* stuff into
it directly. Of course this fragments the user space API for a while
since you need to use two APIs then for the time being to configure all
things, but we can move over all the rest of the configuration gradually
and before we end up in mainline with the new API we can have that
finished.

Or putting it the other way round, I'm ok with
 * cleaning up the ssid mess
 * adding RCPI (we'll probably be using it in nl80211 anyway, so it's
   easier if we add it now and declare no support for other things)
 * getting rid of get_wireless_stats (good one!)
   [actually, I plan to get rid of wireless_handlers too]

johannes

Re: [PATCH 2.6.18] WE-21 support (core API)

[Jouni Malinen]

On Thu, Aug 31, 2006 at 03:32:18PM +0200, Johannes Berg wrote:
> On Tue, 2006-08-29 at 17:56 -0700, Jean Tourrilhes wrote:
> > 		o modulation
> > 		o long/short retry
> > 		o relative power saving.

> What's the point of adding more ioctls that we'll be implementing them
> as wrappers around nl80211? Right now, those new ioctls/options aren't
> implemented in *any* driver at all so they're completely useless, and
> just add more to the pile of historic baggage we end up carrying around.
> If we add these to mainline now, it's another thing we'll have to carry
> for a long time even though it currently has no users...

I don't know about the others, but long/short retry limits have users
(e.g., Host AP driver) and these drivers are currently forced to use a
hack to do this without this cleanup. Furthermore, this part does not
add a new ioctl.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 2.6.18] WE-21 support (core API)

[Johannes Berg]

On Thu, 2006-08-31 at 06:51 -0700, Jouni Malinen wrote:

> I don't know about the others, but long/short retry limits have users
> (e.g., Host AP driver) and these drivers are currently forced to use a
> hack to do this without this cleanup. Furthermore, this part does not
> add a new ioctl.

It does, however, add new parameters and things that'd need to be
translated in the compat layer later. Hence, even there, I'd prefer to
add them directly into nl80211. However, the compat code for that
shouldn't be that bad, so I can see that as a softer target :) But I
don't want to see new ioctls for sure.

johannes

[RFC] Alternate WE-21 support (core API)

[John W. Linville]

On Thu, Aug 31, 2006 at 04:00:05PM +0200, Johannes Berg wrote:
> On Thu, 2006-08-31 at 06:51 -0700, Jouni Malinen wrote:
> 
> > I don't know about the others, but long/short retry limits have users
> > (e.g., Host AP driver) and these drivers are currently forced to use a
> > hack to do this without this cleanup. Furthermore, this part does not
> > add a new ioctl.
> 
> It does, however, add new parameters and things that'd need to be
> translated in the compat layer later. Hence, even there, I'd prefer to
> add them directly into nl80211. However, the compat code for that
> shouldn't be that bad, so I can see that as a softer target :) But I
> don't want to see new ioctls for sure.

OK, I think we all agree that there are good parts to Jean's WE-21
patch.  Below I've made an attempt to separate the wheat from the chaff
(or to cut the baby in half)...

Is this patch acceptable to the group?  Does it make things better?
Or worse?  Did I leave-out anything that should still go in?  Did I
take too much?

Let me know what you think...?

John

---

This is version 21 of the Wireless Extensions. Changelog :
	o finishes migrating the ESSID API (remove the +1)
	o netdev->get_wireless_stats is no more
	o long/short retry

This is a redacted version of a patch originally submitted by Jean
Tourrilhes.  I removed most of the additions, in order to minimize
future support requirements for nl80211 (or other WE successor).

CC: Jean Tourrilhes <jt@hpl.hp.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 include/linux/netdevice.h |    1 -
 include/linux/wireless.h  |   23 +++++++++++++--
 net/core/net-sysfs.c      |    5 +--
 net/core/wireless.c       |   67 ++++++++++++++++++++++++++++-----------------
 4 files changed, 61 insertions(+), 35 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 50a4719..91dc36c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -334,7 +334,6 @@ #define NETIF_F_ALL_CSUM	(NETIF_F_IP_CSU
 
 
 	struct net_device_stats* (*get_stats)(struct net_device *dev);
-	struct iw_statistics*	(*get_wireless_stats)(struct net_device *dev);
 
 	/* List of functions to handle Wireless Extensions (instead of ioctl).
 	 * See <net/iw_handler.h> for details. Jean II */
diff --git a/include/linux/wireless.h b/include/linux/wireless.h
index 1358856..7a5860f 100644
--- a/include/linux/wireless.h
+++ b/include/linux/wireless.h
@@ -1,7 +1,7 @@
 /*
  * This file define a set of standard wireless extensions
  *
- * Version :	20	17.2.06
+ * Version :	21	14.3.06
  *
  * Authors :	Jean Tourrilhes - HPL - <jt@hpl.hp.com>
  * Copyright (c) 1997-2006 Jean Tourrilhes, All Rights Reserved.
@@ -69,9 +69,14 @@ #define _LINUX_WIRELESS_H
 
 /***************************** INCLUDES *****************************/
 
+/* This header is used in user-space, therefore need to be sanitised
+ * for that purpose. Those includes are usually not compatible with glibc.
+ * To know which includes to use in user-space, check iwlib.h. */
+#ifdef __KERNEL__
 #include <linux/types.h>		/* for "caddr_t" et al		*/
 #include <linux/socket.h>		/* for "struct sockaddr" et al	*/
 #include <linux/if.h>			/* for IFNAMSIZ and co... */
+#endif	/* __KERNEL__ */
 
 /***************************** VERSION *****************************/
 /*
@@ -80,7 +85,7 @@ #include <linux/if.h>			/* for IFNAMSIZ 
  * (there is some stuff that will be added in the future...)
  * I just plan to increment with each new version.
  */
-#define WIRELESS_EXT	20
+#define WIRELESS_EXT	21
 
 /*
  * Changes :
@@ -208,6 +213,13 @@ #define WIRELESS_EXT	20
  * V19 to V20
  * ----------
  *	- RtNetlink requests support (SET/GET)
+ *
+ * V20 to V21
+ * ----------
+ *	- Remove (struct net_device *)->get_wireless_stats()
+ *	- Change length in ESSID and NICK to strlen() instead of strlen()+1
+ *	- Add IW_RETRY_SHORT/IW_RETRY_LONG retry modifiers
+ *	- Add explicit flag to tell stats are in 802.11k RCPI : IW_QUAL_RCPI
  */
 
 /**************************** CONSTANTS ****************************/
@@ -448,6 +460,7 @@ #define IW_QUAL_DBM		0x08	/* Level + Noi
 #define IW_QUAL_QUAL_INVALID	0x10	/* Driver doesn't provide value */
 #define IW_QUAL_LEVEL_INVALID	0x20
 #define IW_QUAL_NOISE_INVALID	0x40
+#define IW_QUAL_RCPI		0x80	/* Level + Noise are 802.11k RCPI */
 #define IW_QUAL_ALL_INVALID	0x70
 
 /* Frequency flags */
@@ -500,10 +513,12 @@ #define IW_RETRY_ON		0x0000	/* No detail
 #define IW_RETRY_TYPE		0xF000	/* Type of parameter */
 #define IW_RETRY_LIMIT		0x1000	/* Maximum number of retries*/
 #define IW_RETRY_LIFETIME	0x2000	/* Maximum duration of retries in us */
-#define IW_RETRY_MODIFIER	0x000F	/* Modify a parameter */
+#define IW_RETRY_MODIFIER	0x00FF	/* Modify a parameter */
 #define IW_RETRY_MIN		0x0001	/* Value is a minimum  */
 #define IW_RETRY_MAX		0x0002	/* Value is a maximum */
 #define IW_RETRY_RELATIVE	0x0004	/* Value is not in seconds/ms/us */
+#define IW_RETRY_SHORT		0x0010	/* Value is for short packets  */
+#define IW_RETRY_LONG		0x0020	/* Value is for long packets */
 
 /* Scanning request flags */
 #define IW_SCAN_DEFAULT		0x0000	/* Default scan of the driver */
@@ -1017,7 +1032,7 @@ struct	iw_range
 	/* Note : this frequency list doesn't need to fit channel numbers,
 	 * because each entry contain its channel index */
 
-	__u32		enc_capa; /* IW_ENC_CAPA_* bit field */
+	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
 };
 
 /*
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 1347276..f47f319 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -344,8 +344,6 @@ static ssize_t wireless_show(struct clas
 		if(dev->wireless_handlers &&
 		   dev->wireless_handlers->get_wireless_stats)
 			iw = dev->wireless_handlers->get_wireless_stats(dev);
-		else if (dev->get_wireless_stats)
-			iw = dev->get_wireless_stats(dev);
 		if (iw != NULL)
 			ret = (*format)(iw, buf);
 	}
@@ -465,8 +463,7 @@ int netdev_register_sysfs(struct net_dev
 		*groups++ = &netstat_group;
 
 #ifdef WIRELESS_EXT
-	if (net->get_wireless_stats
-	    || (net->wireless_handlers && net->wireless_handlers->get_wireless_stats))
+	if (net->wireless_handlers && net->wireless_handlers->get_wireless_stats)
 		*groups++ = &wireless_group;
 #endif
 
diff --git a/net/core/wireless.c b/net/core/wireless.c
index de0bde4..61457d8 100644
--- a/net/core/wireless.c
+++ b/net/core/wireless.c
@@ -68,6 +68,14 @@
  *
  * v8 - 17.02.06 - Jean II
  *	o RtNetlink requests support (SET/GET)
+ *
+ * v8b - 03.08.06 - Herbert Xu
+ *	o Fix Wireless Event locking issues.
+ *
+ * v9 - 14.3.06 - Jean II
+ *	o Change length in ESSID and NICK to strlen() instead of strlen()+1
+ *	o Make standard_ioctl_num and standard_event_num unsigned
+ *	o Remove (struct net_device *)->get_wireless_stats()
  */
 
 /***************************** INCLUDES *****************************/
@@ -234,24 +242,24 @@ static const struct iw_ioctl_description
 	[SIOCSIWESSID	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 		.flags		= IW_DESCR_FLAG_EVENT,
 	},
 	[SIOCGIWESSID	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 		.flags		= IW_DESCR_FLAG_DUMP,
 	},
 	[SIOCSIWNICKN	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 	},
 	[SIOCGIWNICKN	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 	},
 	[SIOCSIWRATE	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_PARAM,
@@ -338,8 +346,8 @@ static const struct iw_ioctl_description
 		.max_tokens	= sizeof(struct iw_pmksa),
 	},
 };
-static const int standard_ioctl_num = (sizeof(standard_ioctl) /
-				       sizeof(struct iw_ioctl_description));
+static const unsigned standard_ioctl_num = (sizeof(standard_ioctl) /
+					    sizeof(struct iw_ioctl_description));
 
 /*
  * Meta-data about all the additional standard Wireless Extension events
@@ -389,8 +397,8 @@ static const struct iw_ioctl_description
 		.max_tokens	= sizeof(struct iw_pmkid_cand),
 	},
 };
-static const int standard_event_num = (sizeof(standard_event) /
-				       sizeof(struct iw_ioctl_description));
+static const unsigned standard_event_num = (sizeof(standard_event) /
+					    sizeof(struct iw_ioctl_description));
 
 /* Size (in bytes) of the various private data types */
 static const char iw_priv_type_size[] = {
@@ -465,17 +473,6 @@ static inline struct iw_statistics *get_
 	   (dev->wireless_handlers->get_wireless_stats != NULL))
 		return dev->wireless_handlers->get_wireless_stats(dev);
 
-	/* Old location, field to be removed in next WE */
-	if(dev->get_wireless_stats) {
-		static int printed_message;
-
-		if (!printed_message++)
-			printk(KERN_DEBUG "%s (WE) : Driver using old /proc/net/wireless support, please fix driver !\n",
-				dev->name);
-
-		return dev->get_wireless_stats(dev);
-	}
-
 	/* Not found */
 	return (struct iw_statistics *) NULL;
 }
@@ -1843,8 +1840,33 @@ #endif	/* CONFIG_NET_WIRELESS_RTNETLINK 
  */
 
 #ifdef WE_EVENT_RTNETLINK
+/* ---------------------------------------------------------------- */
+/*
+ * Locking...
+ * ----------
+ *
+ * Thanks to Herbert Xu <herbert@gondor.apana.org.au> for fixing
+ * the locking issue in here and implementing this code !
+ *
+ * The issue : wireless_send_event() is often called in interrupt context,
+ * while the Netlink layer can never be called in interrupt context.
+ * The fully formed RtNetlink events are queued, and then a tasklet is run
+ * to feed those to Netlink.
+ * The skb_queue is interrupt safe, and its lock is not held while calling
+ * Netlink, so there is no possibility of dealock.
+ * Jean II
+ */
+
 static struct sk_buff_head wireless_nlevent_queue;
 
+static int __init wireless_nlevent_init(void)
+{
+	skb_queue_head_init(&wireless_nlevent_queue);
+	return 0;
+}
+
+subsys_initcall(wireless_nlevent_init);
+
 static void wireless_nlevent_process(unsigned long data)
 {
 	struct sk_buff *skb;
@@ -1921,13 +1943,6 @@ static inline void rtmsg_iwinfo(struct n
 	tasklet_schedule(&wireless_nlevent_tasklet);
 }
 
-static int __init wireless_nlevent_init(void)
-{
-	skb_queue_head_init(&wireless_nlevent_queue);
-	return 0;
-}
-
-subsys_initcall(wireless_nlevent_init);
 #endif	/* WE_EVENT_RTNETLINK */
 
 /* ---------------------------------------------------------------- */
-- 
1.4.2.GIT

-- 
John W. Linville
linville@tuxdriver.com

Re: [RFC] Alternate WE-21 support (core API)

[Michael Buesch]

On Wednesday 06 September 2006 22:55, John W. Linville wrote:
> On Thu, Aug 31, 2006 at 04:00:05PM +0200, Johannes Berg wrote:
> > On Thu, 2006-08-31 at 06:51 -0700, Jouni Malinen wrote:
> > 
> > > I don't know about the others, but long/short retry limits have users
> > > (e.g., Host AP driver) and these drivers are currently forced to use a
> > > hack to do this without this cleanup. Furthermore, this part does not
> > > add a new ioctl.
> > 
> > It does, however, add new parameters and things that'd need to be
> > translated in the compat layer later. Hence, even there, I'd prefer to
> > add them directly into nl80211. However, the compat code for that
> > shouldn't be that bad, so I can see that as a softer target :) But I
> > don't want to see new ioctls for sure.
> 
> OK, I think we all agree that there are good parts to Jean's WE-21
> patch.  Below I've made an attempt to separate the wheat from the chaff
> (or to cut the baby in half)...
> 
> Is this patch acceptable to the group?  Does it make things better?
> Or worse?  Did I leave-out anything that should still go in?  Did I
> take too much?
> 
> Let me know what you think...?

I am OK with this.

-- 
Greetings Michael.

Re: [RFC] Alternate WE-21 support (core API)

[Jean Tourrilhes]

On Wed, Sep 06, 2006 at 04:55:44PM -0400, John W. Linville wrote:
> On Thu, Aug 31, 2006 at 04:00:05PM +0200, Johannes Berg wrote:
> > On Thu, 2006-08-31 at 06:51 -0700, Jouni Malinen wrote:
> > 
> > > I don't know about the others, but long/short retry limits have users
> > > (e.g., Host AP driver) and these drivers are currently forced to use a
> > > hack to do this without this cleanup. Furthermore, this part does not
> > > add a new ioctl.
> > 
> > It does, however, add new parameters and things that'd need to be
> > translated in the compat layer later. Hence, even there, I'd prefer to
> > add them directly into nl80211. However, the compat code for that
> > shouldn't be that bad, so I can see that as a softer target :) But I
> > don't want to see new ioctls for sure.
> 
> OK, I think we all agree that there are good parts to Jean's WE-21
> patch.  Below I've made an attempt to separate the wheat from the chaff
> (or to cut the baby in half)...

	Wow, some progress instead of empty talks ! I fully appreciate
your effort in moving forward on this issue...
	Is there anything I can do to help you ? The driver patches
should be straighforward as they don't use any of the features you
dropped (those patches using the new features were not sent to you to
be more precise).

> Is this patch acceptable to the group?  Does it make things better?
> Or worse?  Did I leave-out anything that should still go in?  Did I
> take too much?
> 
> Let me know what you think...?

	As far as I can see, your patch looks technically correct.

> John

> + * V20 to V21
> + * ----------
> + *	- Remove (struct net_device *)->get_wireless_stats()
> + *	- Change length in ESSID and NICK to strlen() instead of strlen()+1
> + *	- Add IW_RETRY_SHORT/IW_RETRY_LONG retry modifiers
> + *	- Add explicit flag to tell stats are in 802.11k RCPI : IW_QUAL_RCPI

	Personally, I would also add this :

+ *	- Power/Retry relative values no longer * 100000

	Three reason :
		1) It's a cleanup and does not add any new feature
		2) It does not change the rest of the patches
		3) Userspace part has already gone in distro, not
including this bit would mean breaking userspace.

	The other bits can be included at a later time ;-)
	Thanks, and have fun...

	Jean

Re: [RFC] Alternate WE-21 support (core API)

[John W. Linville]

On Wed, Sep 06, 2006 at 02:30:53PM -0700, Jean Tourrilhes wrote:
> On Wed, Sep 06, 2006 at 04:55:44PM -0400, John W. Linville wrote:

> > + * V20 to V21
> > + * ----------
> > + *	- Remove (struct net_device *)->get_wireless_stats()
> > + *	- Change length in ESSID and NICK to strlen() instead of strlen()+1
> > + *	- Add IW_RETRY_SHORT/IW_RETRY_LONG retry modifiers
> > + *	- Add explicit flag to tell stats are in 802.11k RCPI : IW_QUAL_RCPI
> 
> 	Personally, I would also add this :
> 
> + *	- Power/Retry relative values no longer * 100000
>
> 	Three reason :
> 		1) It's a cleanup and does not add any new feature
> 		2) It does not change the rest of the patches
> 		3) Userspace part has already gone in distro, not
> including this bit would mean breaking userspace.

Is there any code that corresponds to that?  Or does the comment
simply indicate policy?

> 	The other bits can be included at a later time ;-)

Well, maybe.  But, I think we should now consider WE to be in
maintenance mode.  I think nl80211 is the future, as long as Johannes
delivers.

Thanks,

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

Re: [RFC] Alternate WE-21 support (core API)

[Jean Tourrilhes]

On Fri, Sep 08, 2006 at 10:29:23AM -0400, John W. Linville wrote:
> On Wed, Sep 06, 2006 at 02:30:53PM -0700, Jean Tourrilhes wrote:
> > On Wed, Sep 06, 2006 at 04:55:44PM -0400, John W. Linville wrote:
> 
> > > + * V20 to V21
> > > + * ----------
> > > + *	- Remove (struct net_device *)->get_wireless_stats()
> > > + *	- Change length in ESSID and NICK to strlen() instead of strlen()+1
> > > + *	- Add IW_RETRY_SHORT/IW_RETRY_LONG retry modifiers
> > > + *	- Add explicit flag to tell stats are in 802.11k RCPI : IW_QUAL_RCPI
> > 
> > 	Personally, I would also add this :
> > 
> > + *	- Power/Retry relative values no longer * 100000
> >
> > 	Three reason :
> > 		1) It's a cleanup and does not add any new feature
> > 		2) It does not change the rest of the patches
> > 		3) Userspace part has already gone in distro, not
> > including this bit would mean breaking userspace.
> 
> Is there any code that corresponds to that?  Or does the comment
> simply indicate policy?

	There is no code in the core of the WE, so it only indicates
policy. But, I believe policy change need to be documented.
	On the other hand you will find code in the tiacx patch.

> > 	The other bits can be included at a later time ;-)
> 
> Well, maybe.  But, I think we should now consider WE to be in
> maintenance mode.  I think nl80211 is the future, as long as Johannes
> delivers.

	We'll see. WE has been supposed to be replaced any time soon
for the last 3 years. And I don't believe nl80211 will address legacy
driver and non-802.11 hardware.

> Thanks,
> 
> John

	Thanks, and have fun...

	Jean

Re: [RFC] Alternate WE-21 support (core API)

[John W. Linville]

On Fri, Sep 08, 2006 at 09:13:45AM -0700, Jean Tourrilhes wrote:
> On Fri, Sep 08, 2006 at 10:29:23AM -0400, John W. Linville wrote:
> > On Wed, Sep 06, 2006 at 02:30:53PM -0700, Jean Tourrilhes wrote:
> > > On Wed, Sep 06, 2006 at 04:55:44PM -0400, John W. Linville wrote:
> > 
> > > > + * V20 to V21
> > > > + * ----------
> > > > + *	- Remove (struct net_device *)->get_wireless_stats()
> > > > + *	- Change length in ESSID and NICK to strlen() instead of strlen()+1
> > > > + *	- Add IW_RETRY_SHORT/IW_RETRY_LONG retry modifiers
> > > > + *	- Add explicit flag to tell stats are in 802.11k RCPI : IW_QUAL_RCPI
> > > 
> > > 	Personally, I would also add this :
> > > 
> > > + *	- Power/Retry relative values no longer * 100000
> > >
> > > 	Three reason :
> > > 		1) It's a cleanup and does not add any new feature
> > > 		2) It does not change the rest of the patches
> > > 		3) Userspace part has already gone in distro, not
> > > including this bit would mean breaking userspace.
> > 
> > Is there any code that corresponds to that?  Or does the comment
> > simply indicate policy?
> 
> 	There is no code in the core of the WE, so it only indicates
> policy. But, I believe policy change need to be documented.
> 	On the other hand you will find code in the tiacx patch.

Fair enough...

Any objections?

---


This is version 21 of the Wireless Extensions. Changelog :
	o finishes migrating the ESSID API (remove the +1)
	o netdev->get_wireless_stats is no more
	o long/short retry

This is a redacted version of a patch originally submitted by Jean
Tourrilhes.  I removed most of the additions, in order to minimize
future support requirements for nl80211 (or other WE successor).

CC: Jean Tourrilhes <jt@hpl.hp.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 include/linux/netdevice.h |    1 -
 include/linux/wireless.h  |   23 +++++++++++++--
 net/core/net-sysfs.c      |    5 +--
 net/core/wireless.c       |   67 ++++++++++++++++++++++++++++-----------------
 4 files changed, 61 insertions(+), 35 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 50a4719..91dc36c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -334,7 +334,6 @@ #define NETIF_F_ALL_CSUM	(NETIF_F_IP_CSU
 
 
 	struct net_device_stats* (*get_stats)(struct net_device *dev);
-	struct iw_statistics*	(*get_wireless_stats)(struct net_device *dev);
 
 	/* List of functions to handle Wireless Extensions (instead of ioctl).
 	 * See <net/iw_handler.h> for details. Jean II */
diff --git a/include/linux/wireless.h b/include/linux/wireless.h
index 1358856..7a5860f 100644
--- a/include/linux/wireless.h
+++ b/include/linux/wireless.h
@@ -1,7 +1,7 @@
 /*
  * This file define a set of standard wireless extensions
  *
- * Version :	20	17.2.06
+ * Version :	21	14.3.06
  *
  * Authors :	Jean Tourrilhes - HPL - <jt@hpl.hp.com>
  * Copyright (c) 1997-2006 Jean Tourrilhes, All Rights Reserved.
@@ -69,9 +69,14 @@ #define _LINUX_WIRELESS_H
 
 /***************************** INCLUDES *****************************/
 
+/* This header is used in user-space, therefore need to be sanitised
+ * for that purpose. Those includes are usually not compatible with glibc.
+ * To know which includes to use in user-space, check iwlib.h. */
+#ifdef __KERNEL__
 #include <linux/types.h>		/* for "caddr_t" et al		*/
 #include <linux/socket.h>		/* for "struct sockaddr" et al	*/
 #include <linux/if.h>			/* for IFNAMSIZ and co... */
+#endif	/* __KERNEL__ */
 
 /***************************** VERSION *****************************/
 /*
@@ -80,7 +85,7 @@ #include <linux/if.h>			/* for IFNAMSIZ 
  * (there is some stuff that will be added in the future...)
  * I just plan to increment with each new version.
  */
-#define WIRELESS_EXT	20
+#define WIRELESS_EXT	21
 
 /*
  * Changes :
@@ -208,6 +213,14 @@ #define WIRELESS_EXT	20
  * V19 to V20
  * ----------
  *	- RtNetlink requests support (SET/GET)
+ *
+ * V20 to V21
+ * ----------
+ *	- Remove (struct net_device *)->get_wireless_stats()
+ *	- Change length in ESSID and NICK to strlen() instead of strlen()+1
+ *	- Add IW_RETRY_SHORT/IW_RETRY_LONG retry modifiers
+ *	- Power/Retry relative values no longer * 100000
+ *	- Add explicit flag to tell stats are in 802.11k RCPI : IW_QUAL_RCPI
  */
 
 /**************************** CONSTANTS ****************************/
@@ -448,6 +460,7 @@ #define IW_QUAL_DBM		0x08	/* Level + Noi
 #define IW_QUAL_QUAL_INVALID	0x10	/* Driver doesn't provide value */
 #define IW_QUAL_LEVEL_INVALID	0x20
 #define IW_QUAL_NOISE_INVALID	0x40
+#define IW_QUAL_RCPI		0x80	/* Level + Noise are 802.11k RCPI */
 #define IW_QUAL_ALL_INVALID	0x70
 
 /* Frequency flags */
@@ -500,10 +513,12 @@ #define IW_RETRY_ON		0x0000	/* No detail
 #define IW_RETRY_TYPE		0xF000	/* Type of parameter */
 #define IW_RETRY_LIMIT		0x1000	/* Maximum number of retries*/
 #define IW_RETRY_LIFETIME	0x2000	/* Maximum duration of retries in us */
-#define IW_RETRY_MODIFIER	0x000F	/* Modify a parameter */
+#define IW_RETRY_MODIFIER	0x00FF	/* Modify a parameter */
 #define IW_RETRY_MIN		0x0001	/* Value is a minimum  */
 #define IW_RETRY_MAX		0x0002	/* Value is a maximum */
 #define IW_RETRY_RELATIVE	0x0004	/* Value is not in seconds/ms/us */
+#define IW_RETRY_SHORT		0x0010	/* Value is for short packets  */
+#define IW_RETRY_LONG		0x0020	/* Value is for long packets */
 
 /* Scanning request flags */
 #define IW_SCAN_DEFAULT		0x0000	/* Default scan of the driver */
@@ -1017,7 +1032,7 @@ struct	iw_range
 	/* Note : this frequency list doesn't need to fit channel numbers,
 	 * because each entry contain its channel index */
 
-	__u32		enc_capa; /* IW_ENC_CAPA_* bit field */
+	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
 };
 
 /*
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 1347276..f47f319 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -344,8 +344,6 @@ static ssize_t wireless_show(struct clas
 		if(dev->wireless_handlers &&
 		   dev->wireless_handlers->get_wireless_stats)
 			iw = dev->wireless_handlers->get_wireless_stats(dev);
-		else if (dev->get_wireless_stats)
-			iw = dev->get_wireless_stats(dev);
 		if (iw != NULL)
 			ret = (*format)(iw, buf);
 	}
@@ -465,8 +463,7 @@ int netdev_register_sysfs(struct net_dev
 		*groups++ = &netstat_group;
 
 #ifdef WIRELESS_EXT
-	if (net->get_wireless_stats
-	    || (net->wireless_handlers && net->wireless_handlers->get_wireless_stats))
+	if (net->wireless_handlers && net->wireless_handlers->get_wireless_stats)
 		*groups++ = &wireless_group;
 #endif
 
diff --git a/net/core/wireless.c b/net/core/wireless.c
index de0bde4..61457d8 100644
--- a/net/core/wireless.c
+++ b/net/core/wireless.c
@@ -68,6 +68,14 @@
  *
  * v8 - 17.02.06 - Jean II
  *	o RtNetlink requests support (SET/GET)
+ *
+ * v8b - 03.08.06 - Herbert Xu
+ *	o Fix Wireless Event locking issues.
+ *
+ * v9 - 14.3.06 - Jean II
+ *	o Change length in ESSID and NICK to strlen() instead of strlen()+1
+ *	o Make standard_ioctl_num and standard_event_num unsigned
+ *	o Remove (struct net_device *)->get_wireless_stats()
  */
 
 /***************************** INCLUDES *****************************/
@@ -234,24 +242,24 @@ static const struct iw_ioctl_description
 	[SIOCSIWESSID	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 		.flags		= IW_DESCR_FLAG_EVENT,
 	},
 	[SIOCGIWESSID	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 		.flags		= IW_DESCR_FLAG_DUMP,
 	},
 	[SIOCSIWNICKN	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 	},
 	[SIOCGIWNICKN	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_POINT,
 		.token_size	= 1,
-		.max_tokens	= IW_ESSID_MAX_SIZE + 1,
+		.max_tokens	= IW_ESSID_MAX_SIZE,
 	},
 	[SIOCSIWRATE	- SIOCIWFIRST] = {
 		.header_type	= IW_HEADER_TYPE_PARAM,
@@ -338,8 +346,8 @@ static const struct iw_ioctl_description
 		.max_tokens	= sizeof(struct iw_pmksa),
 	},
 };
-static const int standard_ioctl_num = (sizeof(standard_ioctl) /
-				       sizeof(struct iw_ioctl_description));
+static const unsigned standard_ioctl_num = (sizeof(standard_ioctl) /
+					    sizeof(struct iw_ioctl_description));
 
 /*
  * Meta-data about all the additional standard Wireless Extension events
@@ -389,8 +397,8 @@ static const struct iw_ioctl_description
 		.max_tokens	= sizeof(struct iw_pmkid_cand),
 	},
 };
-static const int standard_event_num = (sizeof(standard_event) /
-				       sizeof(struct iw_ioctl_description));
+static const unsigned standard_event_num = (sizeof(standard_event) /
+					    sizeof(struct iw_ioctl_description));
 
 /* Size (in bytes) of the various private data types */
 static const char iw_priv_type_size[] = {
@@ -465,17 +473,6 @@ static inline struct iw_statistics *get_
 	   (dev->wireless_handlers->get_wireless_stats != NULL))
 		return dev->wireless_handlers->get_wireless_stats(dev);
 
-	/* Old location, field to be removed in next WE */
-	if(dev->get_wireless_stats) {
-		static int printed_message;
-
-		if (!printed_message++)
-			printk(KERN_DEBUG "%s (WE) : Driver using old /proc/net/wireless support, please fix driver !\n",
-				dev->name);
-
-		return dev->get_wireless_stats(dev);
-	}
-
 	/* Not found */
 	return (struct iw_statistics *) NULL;
 }
@@ -1843,8 +1840,33 @@ #endif	/* CONFIG_NET_WIRELESS_RTNETLINK 
  */
 
 #ifdef WE_EVENT_RTNETLINK
+/* ---------------------------------------------------------------- */
+/*
+ * Locking...
+ * ----------
+ *
+ * Thanks to Herbert Xu <herbert@gondor.apana.org.au> for fixing
+ * the locking issue in here and implementing this code !
+ *
+ * The issue : wireless_send_event() is often called in interrupt context,
+ * while the Netlink layer can never be called in interrupt context.
+ * The fully formed RtNetlink events are queued, and then a tasklet is run
+ * to feed those to Netlink.
+ * The skb_queue is interrupt safe, and its lock is not held while calling
+ * Netlink, so there is no possibility of dealock.
+ * Jean II
+ */
+
 static struct sk_buff_head wireless_nlevent_queue;
 
+static int __init wireless_nlevent_init(void)
+{
+	skb_queue_head_init(&wireless_nlevent_queue);
+	return 0;
+}
+
+subsys_initcall(wireless_nlevent_init);
+
 static void wireless_nlevent_process(unsigned long data)
 {
 	struct sk_buff *skb;
@@ -1921,13 +1943,6 @@ static inline void rtmsg_iwinfo(struct n
 	tasklet_schedule(&wireless_nlevent_tasklet);
 }
 
-static int __init wireless_nlevent_init(void)
-{
-	skb_queue_head_init(&wireless_nlevent_queue);
-	return 0;
-}
-
-subsys_initcall(wireless_nlevent_init);
 #endif	/* WE_EVENT_RTNETLINK */
 
 /* ---------------------------------------------------------------- */
-- 
1.4.2.GIT

-- 
John W. Linville
linville@tuxdriver.com

Re: [RFC] Alternate WE-21 support (core API)

[Johannes Berg]

On Fri, 2006-09-08 at 09:13 -0700, Jean Tourrilhes wrote:
> And I don't believe nl80211 will address legacy
> driver and non-802.11 hardware.

It'd help if you'd tell me what in WE specifically addresses non-802.11
legacy hardware. I don't really see anything that is completely
orthogonal.

johannes

Re: [RFC] Alternate WE-21 support (core API)

[Jean Tourrilhes]

On Tue, Sep 12, 2006 at 10:43:57AM +0200, Johannes Berg wrote:
> 
> > But there are bits left and right, so
> > more inspection of drivers would be needed. For example, wavelan and
> > netwave are using encryption key sizes which are not compatible with
> > 802.11.
> 
> Hmm, interesting. Will need to think about that for a bit. The whole key
> management stuff is fairly hairy, even more so in wpa/wpa2.

	I was initially very negative towards the WPA API (WPA +
extended scan), because it's so complex. I went back and forth with
Jouni trying to simplify it, but we did not manage to gain much. I
trust that Jouni did the best he could, that's just the nature of the
beast.

	I was thinking of the WE -> nl80211 compatibility. It's pretty
trivial to do as far as WE is concerned, you just need to hack
get_handler() to return the nl80211 handler. Or, if you don't like a
single handler, you could have one wrapper per ioctl, which is even
easier.
	Do you have a recent version of your code so that I can see
how it can hook on your side ? I think we could put the generic
mechanism in place early so that people can add specifics as they need
them.

> johannes

	Have fun...

	Jean

Re: [RFC] Alternate WE-21 support (core API)

[Johannes Berg]

On Tue, 2006-09-12 at 09:17 -0700, Jean Tourrilhes wrote:

> 	I was initially very negative towards the WPA API (WPA +
> extended scan), because it's so complex. I went back and forth with
> Jouni trying to simplify it, but we did not manage to gain much. I
> trust that Jouni did the best he could, that's just the nature of the
> beast.

:)

> 	I was thinking of the WE -> nl80211 compatibility. It's pretty
> trivial to do as far as WE is concerned, you just need to hack
> get_handler() to return the nl80211 handler. Or, if you don't like a
> single handler, you could have one wrapper per ioctl, which is even
> easier.

Currently, I don't rely on that at all, nl80211 relies on the
driver/stack assigning ieee80211_ptr in the netdevice field and uses
that as a cookie. IOW, get_handler() gets to die.

My current plan for compatibility was to make all drivers cfg80211-away
by making them register themselves with cfg80211 with a callback struct
with entries for each thing they need to handle, and then introduce a
translation layer that translates incoming WE requests into the
appropriate callbacks just like nl80211 translates the incoming
genetlink messages.

> 	Do you have a recent version of your code so that I can see
> how it can hook on your side ? I think we could put the generic
> mechanism in place early so that people can add specifics as they need
> them.

The latest patches were posted to netdev quite a while ago, I haven't
done anything with them recently.

One thing I'd like to do is rename the include from net/nl80211.h to
net/cfg80211.h so that it's more obvious that it isn't purely netlink
(include/nl80211.h is the userspace netlink interface so that stays).

johannes

Re: [RFC] Alternate WE-21 support (core API)

[Larry Finger]

John W. Linville wrote:
> On Thu, Aug 31, 2006 at 04:00:05PM +0200, Johannes Berg wrote:
>> On Thu, 2006-08-31 at 06:51 -0700, Jouni Malinen wrote:
>>
>>> I don't know about the others, but long/short retry limits have users
>>> (e.g., Host AP driver) and these drivers are currently forced to use a
>>> hack to do this without this cleanup. Furthermore, this part does not
>>> add a new ioctl.
>> It does, however, add new parameters and things that'd need to be
>> translated in the compat layer later. Hence, even there, I'd prefer to
>> add them directly into nl80211. However, the compat code for that
>> shouldn't be that bad, so I can see that as a softer target :) But I
>> don't want to see new ioctls for sure.
> 
> OK, I think we all agree that there are good parts to Jean's WE-21
> patch.  Below I've made an attempt to separate the wheat from the chaff
> (or to cut the baby in half)...
> 
> Is this patch acceptable to the group?  Does it make things better?
> Or worse?  Did I leave-out anything that should still go in?  Did I
> take too much?
> 
> Let me know what you think...?

> + *
> + * The issue : wireless_send_event() is often called in interrupt context,
> + * while the Netlink layer can never be called in interrupt context.
> + * The fully formed RtNetlink events are queued, and then a tasklet is run
> + * to feed those to Netlink.
> + * The skb_queue is interrupt safe, and its lock is not held while calling
> + * Netlink, so there is no possibility of dealock.
                                              ^^^^^^^
You might as well fix the typo now. Otherwise, it looks OK to me. I've been running the original 
patch for several days with no problems. I don't think you removed anything essential.

Larry

Re: [RFC] Alternate WE-21 support (core API)

[Johannes Berg]

On Wed, 2006-09-06 at 16:55 -0400, John W. Linville wrote:

> Is this patch acceptable to the group?  Does it make things better?
> Or worse?  Did I leave-out anything that should still go in?  Did I
> take too much?

Looks good to me.

johannes

Re: [PATCH 2.6.18] WE-21 support (core API)

[Jean Tourrilhes]

On Thu, Aug 31, 2006 at 03:32:18PM +0200, Johannes Berg wrote:
> On Tue, 2006-08-29 at 17:56 -0700, Jean Tourrilhes wrote:
> > 		o modulation
> > 		o long/short retry
> > 		o relative power saving.
> 
> I strongly disagree to these.

	And I strongly disagree with your disagrement ;-)

> What's the point of adding more ioctls that we'll be implementing them
> as wrappers around nl80211? Right now, those new ioctls/options aren't
> implemented in *any* driver at all so they're completely useless, and
> just add more to the pile of historic baggage we end up carrying around.
> If we add these to mainline now, it's another thing we'll have to carry
> for a long time even though it currently has no users...

	I'm sorry to say it like this, but I hope my work will not be
impacted by vaporware. How many drivers are currently converted to
nl80211 ?
	I hope you realised that we are all trying to make 802.11
support progress, each through our own ways. And that all those
efforts are not conflicting with each other.
	The reason why those new features are good for you :
		o They give you a template on how you could implement
those features in nl80211, saving you design time.
		o The goal is to replace private ioctls (driver
specific) with standard ioctls. So, in other words, they actually
*reduce* the historic baggage and make it easier to wrap around those
functions if you want (I won't expect you to wrap around *every* WE).

	Personally, I still have not seen the point of nl80211, as the
full Wireless Extension is already available over NetLink (have you
tried it ?). But, I shut up my big mouth, and let you work freely on
it, as a mark of respect for your work and intentions, and also
because something good may come out of it.

> I'd much prefer merging nl80211 and putting any really *new* stuff into
> it directly. Of course this fragments the user space API for a while
> since you need to use two APIs then for the time being to configure all
> things, but we can move over all the rest of the configuration gradually
> and before we end up in mainline with the new API we can have that
> finished.

	Good luck with that plan, user-space is notorious to not like
entropy...
	Personally, I though that the plan that was more or less the
consensus that the "new API" would be targeted mostly at the
DeviceScape stack, as it seems difficult to offer the full feature set
of that stack through WE. So, I would have expected the development of
the "new API" to be somewhat in sync with the integration of the
DeviceScape stack.

> Or putting it the other way round, I'm ok with
>  * cleaning up the ssid mess
>  * adding RCPI (we'll probably be using it in nl80211 anyway, so it's
>    easier if we add it now and declare no support for other things)
>  * getting rid of get_wireless_stats (good one!)
>    [actually, I plan to get rid of wireless_handlers too]

	And if you look closely, you will realise the other features
are also good for you ;-)

> johannes

	Have fun...

	Jean

Re: [PATCH 2.6.18] WE-21 support (core API)

[Michael Buesch]

On Thursday 31 August 2006 19:12, Jean Tourrilhes wrote:
> On Thu, Aug 31, 2006 at 03:32:18PM +0200, Johannes Berg wrote:
> > On Tue, 2006-08-29 at 17:56 -0700, Jean Tourrilhes wrote:
> > > 		o modulation
> > > 		o long/short retry
> > > 		o relative power saving.
> > 
> > I strongly disagree to these.
> 
> 	And I strongly disagree with your disagrement ;-)
> 
> > What's the point of adding more ioctls that we'll be implementing them
> > as wrappers around nl80211? Right now, those new ioctls/options aren't
> > implemented in *any* driver at all so they're completely useless, and
> > just add more to the pile of historic baggage we end up carrying around.
> > If we add these to mainline now, it's another thing we'll have to carry
> > for a long time even though it currently has no users...
> 
> 	I'm sorry to say it like this, but I hope my work will not be
> impacted by vaporware. How many drivers are currently converted to
> nl80211 ?
> 	I hope you realised that we are all trying to make 802.11
> support progress, each through our own ways. And that all those
> efforts are not conflicting with each other.

Yes. Why don't all people pull at the same rope end?

> 	The reason why those new features are good for you :
> 		o They give you a template on how you could implement
> those features in nl80211, saving you design time.
> 		o The goal is to replace private ioctls (driver
> specific) with standard ioctls. So, in other words, they actually
> *reduce* the historic baggage and make it easier to wrap around those
> functions if you want (I won't expect you to wrap around *every* WE).

I can't see how adding API reduces historic baggage.
I don't think it's possible to reduce something by adding stuff
to it.

> 	Personally, I still have not seen the point of nl80211, as the
> full Wireless Extension is already available over NetLink (have you
> tried it ?). But, I shut up my big mouth, and let you work freely on
> it, as a mark of respect for your work and intentions, and also
> because something good may come out of it.

Wireless Extensions are _horrible_ to implement and, most important,
to get right. Yes, you say it's easy. But you implemented it. I also
say understanding bcm43xx code is very easy. But I am sure that most
people will strongly disagree here.
I don't say you misdesigned WE. It was a good solution back when you
designed it (1996?).
WE is simply showing its age and should be replaced by nl80211.

> > I'd much prefer merging nl80211 and putting any really *new* stuff into
> > it directly. Of course this fragments the user space API for a while
> > since you need to use two APIs then for the time being to configure all
> > things, but we can move over all the rest of the configuration gradually
> > and before we end up in mainline with the new API we can have that
> > finished.
> 
> 	Good luck with that plan, user-space is notorious to not like
> entropy...
> 	Personally, I though that the plan that was more or less the
> consensus that the "new API" would be targeted mostly at the
> DeviceScape stack, as it seems difficult to offer the full feature set
> of that stack through WE. So, I would have expected the development of
> the "new API" to be somewhat in sync with the integration of the
> DeviceScape stack.

It is. Nobody says different. I think with "mainline" Johannes meant
the wireless-dev tree. Merging nl80211 with softmac would indeed not
make sense to me, too.

-- 
Greetings Michael.

Re: [PATCH 2.6.18] WE-21 support (core API)

[Johannes Berg]

On Thu, 2006-08-31 at 19:57 +0200, Michael Buesch wrote:

> It is. Nobody says different. I think with "mainline" Johannes meant
> the wireless-dev tree. Merging nl80211 with softmac would indeed not
> make sense to me, too.

Actually, I do say different. I want softmac to be a consumer of nl80211
too, as well as all the fullmac drivers. I want to put all the ioctl
handling and all the mess with implicit semantics and special cases into
some common shim layer that simply calls the same methods you gave to
nl80211, so that drivers never ever need to bother with WE again.

johannes

Re: [PATCH 2.6.18] WE-21 support (core API)

[Johannes Berg]

On Thu, 2006-08-31 at 10:12 -0700, Jean Tourrilhes wrote:

> 	And I strongly disagree with your disagrement ;-)

You're of course free to do that :) But let me explain.

> 	I'm sorry to say it like this, but I hope my work will not be
> impacted by vaporware. How many drivers are currently converted to
> nl80211 ?

None, I have to admit, of course. But I hope you realise that I can
impossibly convert all drivers at once and all by myself. I merely set
out a framework in which it is easy to add new features, new calls and
make things behave.

> 	The reason why those new features are good for you :
> 		o They give you a template on how you could implement
> those features in nl80211, saving you design time.

Gack. I'm inclined to not even respond to this, but I am convinced that
having all the unions and ill-defined semantics with one field having
multiple meanings is in no way saving me design time because it's part
of the problem I intend to completely avoid.

> 		o The goal is to replace private ioctls (driver
> specific) with standard ioctls. So, in other words, they actually
> *reduce* the historic baggage and make it easier to wrap around those
> functions if you want (I won't expect you to wrap around *every* WE).

Actually, trust me, you will. Once drivers can no longer handle WEs by
themselves because it's done centrally and over a shim layer in
cfg80211, you will.

> 	Personally, I still have not seen the point of nl80211, as the
> full Wireless Extension is already available over NetLink (have you
> tried it ?). 

Actually, no. But that's beside the point anyway. Actually, I think that
publishing WE over netlink was a mistake. See, my gripes with WE aren't
how the interface works. I could live with an ioctl based interface
forever, we have many of those and that's not really a problem. The real
problem with WE is, as I previously said, the ill-defined semantics of
both the user-space API and the in-kernel API. There are too many
special cases. Setting an ESSID to all-zeroes to disable something (or
was it all-ones to disable??) might have seemed like a good idea 10
years ago, but I certainly think that putting this implicit behaviour
into a modern interface is just asking for the kind of trouble that WE
has.

> But, I shut up my big mouth, and let you work freely on
> it, as a mark of respect for your work and intentions, and also
> because something good may come out of it.

:)

> 	Personally, I though that the plan that was more or less the
> consensus that the "new API" would be targeted mostly at the
> DeviceScape stack, as it seems difficult to offer the full feature set
> of that stack through WE. So, I would have expected the development of
> the "new API" to be somewhat in sync with the integration of the
> DeviceScape stack.

Not at all. I plan to support all drivers over nl80211, even those that
make no use of d80211. In fact, my current plan is to convert all
drivers to nl80211 and put WE as a shim layer. Yes, that'll rip out a
good part of your code, sorry about that.

> 	And if you look closely, you will realise the other features
> are also good for you ;-)

Not really. More ioctls aren't good. Even if they replace private ones.
Private ones were never really guaranteed anyway, so all the private
ones that generically make sense (and shouldn't just be in debugfs
instead...) shall be in nl80211 as well.

johannes

Re: [PATCH 2.6.18] WE-21 support (core API)

[Jean Tourrilhes]

On Fri, Sep 01, 2006 at 08:54:00AM +0200, Johannes Berg wrote:
> On Thu, 2006-08-31 at 10:12 -0700, Jean Tourrilhes wrote:
> 
> > 	And I strongly disagree with your disagrement ;-)
> 
> You're of course free to do that :) But let me explain.

	And my explanation is even more simple : let's not throw the
baby with the bathwater. If things are broken in WE, let's just fix
it. I've always worked with people working with me.
	Note that one thing that worry me with your approach is
footprint. I've used various embedded devices over the years, such as
the Gumstix (4MB Flash), and this is why WE was optimised for
footprint.

> > 	I'm sorry to say it like this, but I hope my work will not be
> > impacted by vaporware. How many drivers are currently converted to
> > nl80211 ?
> 
> None, I have to admit, of course. But I hope you realise that I can
> impossibly convert all drivers at once and all by myself. I merely set
> out a framework in which it is easy to add new features, new calls and
> make things behave.

	I hope you realised that you can't expect driver authors to
convert their driver, it won't happen. I've implemented WE in half the
driver in the kernel myself, and I convert WE in those drivers myself.

> > 	The reason why those new features are good for you :
> > 		o They give you a template on how you could implement
> > those features in nl80211, saving you design time.
> 
> Gack. I'm inclined to not even respond to this, but I am convinced that
> having all the unions and ill-defined semantics with one field having
> multiple meanings is in no way saving me design time because it's part
> of the problem I intend to completely avoid.

	Can we have specifics, instead of generalities ? Every time
somebody pointed out a specific thing that was broken in WE, I've
fixed it (with a few exceptions).
	Case in point : the ESSID change (which was a pain to
coordinate in user space).

> Actually, no. But that's beside the point anyway. Actually, I think that
> publishing WE over netlink was a mistake.

	I don't understand, in the last few years everybody was
clamoring for a Netlink interface, and now that it is done, people say
it is a stupid thing. Any specifics ?

> The real
> problem with WE is, as I previously said, the ill-defined semantics of
> both the user-space API and the in-kernel API.

	I don't understand why you say it's ill defined, it 100%
documented in the iwconfig man page.

> Setting an ESSID to all-zeroes to disable something (or
> was it all-ones to disable??)

	This has *never* existed, there has always been a separate
flag to indicate that since the beggining. I don't know where you got
that.
	Any other misconception I need to straighten ?

> johannes

	Regards,

	Jean

Re: [PATCH 2.6.18] WE-21 support (core API)

[Michael Buesch]

On Friday 01 September 2006 18:35, Jean Tourrilhes wrote:
> On Fri, Sep 01, 2006 at 08:54:00AM +0200, Johannes Berg wrote:
> > On Thu, 2006-08-31 at 10:12 -0700, Jean Tourrilhes wrote:
> > 
> > > 	And I strongly disagree with your disagrement ;-)
> > 
> > You're of course free to do that :) But let me explain.
> 
> 	And my explanation is even more simple : let's not throw the
> baby with the bathwater. If things are broken in WE, let's just fix
> it. I've always worked with people working with me.

Fixing would require redesign of some parts, so incompatible ABI
changes. nl80211 actually _does_ fix it by rewriting the stuff.

> 	Note that one thing that worry me with your approach is
> footprint. I've used various embedded devices over the years, such as
> the Gumstix (4MB Flash), and this is why WE was optimised for
> footprint.

Can you please explain in more detail, how WE + the WE-netlink wrapper
has lower footprint than this netlink-only layer?

> > Actually, no. But that's beside the point anyway. Actually, I think that
> > publishing WE over netlink was a mistake.
> 
> 	I don't understand, in the last few years everybody was
> clamoring for a Netlink interface, and now that it is done, people say
> it is a stupid thing. Any specifics ?
> 
> > The real
> > problem with WE is, as I previously said, the ill-defined semantics of
> > both the user-space API and the in-kernel API.
> 
> 	I don't understand why you say it's ill defined, it 100%
> documented in the iwconfig man page.

It is horribly documented.
There is one big union and one magic "extra" parameter.
You have to guess (or look at other implementations) to find
out which element of the union or even if and how to use the extra
parameter. That's a real pain.
And after you found out which element to use, you have to figure
out somehow how to actually use that element. That's nontrivial,
escpecially because some flags (that are not documented) may
magically change the whole semantics of the contents.

In my opinion this
"One function signature fits all" design used in WE is simply
broken by design. It leads to exactly this big union, the
magic extra field and all the other magic flags here and there.
It is in no way clear from looking at the function signature
what to do. But it should be. nl80211 is designed much better here.

-- 
Greetings Michael.

Re: [PATCH 2.6.18] WE-21 support (core API)

[Jean Tourrilhes]

On Fri, Sep 01, 2006 at 08:55:48PM +0200, Michael Buesch wrote:
> 
> > 	Note that one thing that worry me with your approach is
> > footprint. I've used various embedded devices over the years, such as
> > the Gumstix (4MB Flash), and this is why WE was optimised for
> > footprint.
> 
> Can you please explain in more detail, how WE + the WE-netlink wrapper
> has lower footprint than this netlink-only layer?

	WE-netlink is optional. And WE-ioctl could be made optional
(still on the todo list). You can also disable WE-event and WE-iwspy
for further footprint reduction.

> > > The real
> > > problem with WE is, as I previously said, the ill-defined semantics of
> > > both the user-space API and the in-kernel API.
> > 
> > 	I don't understand why you say it's ill defined, it 100%
> > documented in the iwconfig man page.
> 
> It is horribly documented.
> There is one big union and one magic "extra" parameter.
> You have to guess (or look at other implementations) to find
> out which element of the union or even if and how to use the extra
> parameter. That's a real pain.
> And after you found out which element to use, you have to figure
> out somehow how to actually use that element. That's nontrivial,
> escpecially because some flags (that are not documented) may
> magically change the whole semantics of the contents.

	If you are trying to write WE without reading any other code,
that's true. But that's not the way sane people work. Sane people
cut'n'paste from other drivers, and then check the source code of
iwconfig (which is fully commented) in case of doubt.
	It's strange, many driver authors are not afraid of asking me
questions, but some can't manage to do that.

> In my opinion this
> "One function signature fits all" design used in WE is simply
> broken by design.

	So, are you saying that the 'syscal' design is broken by
nature ? I've never seen the kernel and glibc people complaining about
it.
	It was designed this way on purpose, because you get low
footprint and very good scalability. And I've yet to see anyone
tripped by it.

> Greetings Michael.

	Jean

-- 
VGER BF report: U 0.5

Re: [PATCH 2.6.18] WE-21 support (core API)

[Michael Buesch]

On Saturday 02 September 2006 00:10, Jean Tourrilhes wrote:
> On Fri, Sep 01, 2006 at 08:55:48PM +0200, Michael Buesch wrote:
> > 
> > > 	Note that one thing that worry me with your approach is
> > > footprint. I've used various embedded devices over the years, such as
> > > the Gumstix (4MB Flash), and this is why WE was optimised for
> > > footprint.
> > 
> > Can you please explain in more detail, how WE + the WE-netlink wrapper
> > has lower footprint than this netlink-only layer?
> 
> 	WE-netlink is optional. And WE-ioctl could be made optional
> (still on the todo list). You can also disable WE-event and WE-iwspy
> for further footprint reduction.

And we don't need all this stuff on these devices? OK, nl80211
can easily be made optional, too.

> > > > The real
> > > > problem with WE is, as I previously said, the ill-defined semantics of
> > > > both the user-space API and the in-kernel API.
> > > 
> > > 	I don't understand why you say it's ill defined, it 100%
> > > documented in the iwconfig man page.
> > 
> > It is horribly documented.
> > There is one big union and one magic "extra" parameter.
> > You have to guess (or look at other implementations) to find
> > out which element of the union or even if and how to use the extra
> > parameter. That's a real pain.
> > And after you found out which element to use, you have to figure
> > out somehow how to actually use that element. That's nontrivial,
> > escpecially because some flags (that are not documented) may
> > magically change the whole semantics of the contents.
> 
> 	If you are trying to write WE without reading any other code,
> that's true. But that's not the way sane people work. Sane people
> cut'n'paste from other drivers, and then check the source code of
> iwconfig (which is fully commented) in case of doubt.
> 	It's strange, many driver authors are not afraid of asking me
> questions, but some can't manage to do that.

Heh, well. I would say sane code should not raise the questions
in the first place.

> > In my opinion this
> > "One function signature fits all" design used in WE is simply
> > broken by design.
> 
> 	So, are you saying that the 'syscal' design is broken by
> nature ? I've never seen the kernel and glibc people complaining about
> it.

?? All syscalls have the same function signature? I doubt that.

> 	It was designed this way on purpose, because you get low
> footprint and very good scalability. And I've yet to see anyone
> tripped by it.

I don't see how this is lower footprint.
A function pointer is always the same size. Regardless of how
the function looks like.

-- 
Greetings Michael.

-- 
VGER BF report: U 0.5

Re: [PATCH 2.6.18] WE-21 support (core API)

[Johannes Berg]

On Sat, 2006-09-02 at 02:47 +0200, Michael Buesch wrote:

> And we don't need all this stuff on these devices? OK, nl80211
> can easily be made optional, too.

Not the whole of nl80211, but I guess some parts for event reporting
etc. could be made optional and the functions tiny do-nothing inlines.

johannes

-- 
VGER BF report: U 0.510304

Re: [PATCH 2.6.18] WE-21 support (core API)

[Johannes Berg]

Uh, please don't strip me from the CC list :)

> 	WE-netlink is optional. And WE-ioctl could be made optional
> (still on the todo list). You can also disable WE-event and WE-iwspy
> for further footprint reduction.

The real question is: Why does removing WE-event reduce footprint? I
guess the answer is that there's a lot of non-generic code needed to
pack/unpack all the data. Which is not really something you want.

wireless.c has about 2.3k lines of code. But, for example airo.c
contains another 15 lines of code just for the trivial *parameter
checking* in airo_set_essid. This is duplicated all over. Did it never
occur to you that things like
                /* Check the size of the string */
                if(dwrq->length > IW_ESSID_MAX_SIZE+1) {
                        return -E2BIG ;
                }
can be checked generically? Maybe you're actually checking this
generically. But if I did it your way, I'd copy and paste this all
over...

genetlink puts this all into the generic code reducing code-size in the
actual handler code a lot. No more checking for nul-termination or foo
like that, it's automatically enforced and if the string isn't
nul-terminated but you want it, it gets rejected much earlier, nl80211
doesn't even get to see it. Same with length restrictions.

> 	If you are trying to write WE without reading any other code,
> that's true. But that's not the way sane people work. 

Excuse me?

> Sane people
> cut'n'paste from other drivers, and then check the source code of
> iwconfig (which is fully commented) in case of doubt.

No no no, you have this all the wrong way around. *Sane* people *DON'T*
just copy and paste random code into their drivers without knowing what
it does. Also, *sane* people *DON'T* check the userspace tools for what
they should send out of the kernel since userspace doesn't define the
ABI, the kernel does.
And this attitudes gets you what you can see: a proliferation of large
amounts of code between all drivers that is ever so slightly different
everywhere and you can't really know whether the differences are on
purpose or just errors.

Note: This is another case point against WE. Why is this code
duplication needed in the first place? Because WE itself doesn't give
the driver authors proper info, it requires them to know about all the
specialties.

> 	It was designed this way on purpose, because you get low
> footprint and very good scalability.

Wtf does scalability have to do with it? One thing you don't seem to
realise that developer time in the open source community can be a much
more precious resource than 1k object code. And an API that isn't buggy,
well-defined, and behaves the same across all drivers (you can't tell me
that softmac behaves the same as airo.c even where it could... I can't
possibly be that lucky) is worth even more.

I, for one, have spent hours on understanding all the magic WE. You seem
to claim I don't need to understand it and I can just copy/paste code.
But as I said above, it doesn't really work that way. Also, I probably
spent more time understanding and writing the WE part of softmac than
writing the rest.

Ideally, we'd have a set of well-defined callbacks that a driver author
assigns (not more memory overhead than the ioctl array) with
well-defined structures that are passed in (admittedly a bit overhead to
deserialize netlink messages into those structures, but a large part of
that is done by genetlink) checked by the generic netlink code that's
present anyway (getting rid of code, most importantly in the drivers
itself, they are complicated enough anyway).

This is what I'm aiming for with nl80211.

And since you don't seem to want to work with me on nl80211 I'll just
have to work against you. I'll do my best to stop proliferation of more
WE stuff because I believe it to be dead as soon as I can manage to
write enough code. I will provide a compatibility layer since I believe
in stable userspace interfaces, but this will simply translate the WE
ioctls calls to appropriate cfg80211 backend calls similar to what
nl80211 does.

wireless.c and driver WE support in its current form must die.

johannes

-- 
VGER BF report: U 0.903988

Re: [PATCH 2.6.18] WE-21 support (core API)

[Stuffed Crust]

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

On Mon, Sep 04, 2006 at 10:35:09AM +0200, Johannes Berg wrote:
> wireless.c and driver WE support in its current form must die.

I doubt you'll have anyone argue this point; not even JT.  I doubt he 
cares how WE is ultimately implemented, only that things continue 
to "just work".

The problems you've just enumerated with WE aren't actually the fault of
WE per se, but rather an internal kernel API problem -- Wireless drivers
currently handle the WE ioctls directly, and thus all have their own
quirks. (Or as you put it, a userspace API dictated internal APIs.  It 
may have been GoodEnough(tm) then, but it isn't any longer, eh?)

While a new userspace API will eventually open up more possibilities,
the real benefit of d80211/nl80211 is its middle/thunk layer, simplifying
driver development considerably through its shared code (and 802.11
stack), and generally enforcing a separation of userspace from device
drivers and the current mess that's a result.  But then you already
explained all of this (and I completely agree with your enumeration of
the benefits)

I won't shed any tears to see WE (the userspace API) superceded (as long
as its replacement is genuinely better rather than simply different) and
I'll dance for joy when the internal wireless APIs get replaced and
wireless drivers no longer directly interact with userspace -- but keep
in mind that these are two independent scenarios.

Treat WE as a userspace API, not an internal API.  From d80211/nl80211's
perspective, the code necessary to support the latest WE-21 proposal is
trivial[1], and in the mean time, WE-21 fixes several real, current
problems that affect users *now*, necessitating a solution *now*.

Meanwhile.

I look at this and can't but help think about ALSA.  How long did it
take to get ALSA merged into the kernel, despite it being that much
MoreBetter(tm) than the in-kernel OSSFree drivers/APIs, each with their
own userspace interactions and thus behaivoral quirks and bugs?  And how
many in-kernel OSS drivers still remain because of problems with (or
missing) ALSA drivers?  (And how many quirks still remain with the 
alsa-oss emulation?)

 - Solomon

[1] Generic WE support is 2K/37K [mid-layer] lines in one of the 
    codebases I maintain, and that includes full compatibility with 
    WE-8 through WE-21.
-- 
Solomon Peachy        		       pizza at shaftnet dot org	 
Melbourne, FL                          ^^ (mail/jabber/gtalk) ^^
Quidquid latine dictum sit, altum viditur.          ICQ: 1318344


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2.6.18] WE-21 support (core API)

[Jean Tourrilhes]

On Mon, Sep 04, 2006 at 10:35:09AM +0200, Johannes Berg wrote:
> Uh, please don't strip me from the CC list :)
> 
> > 	WE-netlink is optional. And WE-ioctl could be made optional
> > (still on the todo list). You can also disable WE-event and WE-iwspy
> > for further footprint reduction.
> 
> The real question is: Why does removing WE-event reduce footprint? I
> guess the answer is that there's a lot of non-generic code needed to
> pack/unpack all the data. Which is not really something you want.

	Wrong answer.

> wireless.c has about 2.3k lines of code. But, for example airo.c
> contains another 15 lines of code just for the trivial *parameter
> checking* in airo_set_essid. This is duplicated all over. Did it never
> occur to you that things like
>                 /* Check the size of the string */
>                 if(dwrq->length > IW_ESSID_MAX_SIZE+1) {
>                         return -E2BIG ;
>                 }
> can be checked generically? Maybe you're actually checking this
> generically. But if I did it your way, I'd copy and paste this all
> over...

	It is actually checked generically, that's the whole point of
the code in wireless.c. But, driver authors don't trust generic
checks.

> > 	It was designed this way on purpose, because you get low
> > footprint and very good scalability.
> 
> Wtf does scalability have to do with it?

	Footprint scalability.

> johannes

	Jean

Re: [PATCH 2.6.18] WE-21 support (core API)

[Ulrich Kunitz]

On 06-09-01 20:55 Michael Buesch wrote:

> > > The real
> > > problem with WE is, as I previously said, the ill-defined semantics of
> > > both the user-space API and the in-kernel API.
> > 
> > 	I don't understand why you say it's ill defined, it 100%
> > documented in the iwconfig man page.
> 
> It is horribly documented.

Jean, you have certainly done more for wireless support in Linux
than most of us and definitely for a much longer time. However I
have to admit that Michael has a point here. The iwconfig manual
page doesn't reference a single IO control macro, the big union or
a single flag. Reading source code doesn't make things a lot
easier, because the drivers do things quite differently or not at
all. I had to check by trial and error, what the right semantics
of controls might be. For example I spent hours for the controls
SIOCGIWFREQ and SIOCSIWFREQ, because I had no idea what the exact
semantics of IW_FREQ_FIXED and IW_FREQ_AUTO are and whether it's
only a SIOCGIWFREQ issue or not.

I fear that user space programmers are facing the same issues.
This might be the reason, why we don't have a larger number of
graphical widgets that show signal strength and support AP
selection.

Jean, you could do us all a favour, if you could start to write
down, what you know about the wireless extensions. This would help
us folks, who do not have your long experience, a lot.

> In my opinion this
> "One function signature fits all" design used in WE is simply
> broken by design. It leads to exactly this big union, the
> magic extra field and all the other magic flags here and there.

I second this. Simple special structs for the control pairs would
have made the task of understanding the interface much more easier
and even simpler to document. From my point of view it's a
fundamental issue, which could be mitigated by documentation a
little bit. But in my opinion it might be about time to put the
wireless extension API to rest and replace it by something better.
Johannes actually tries to do this. I have also trouble to
understand, how adding new controls to the old API should help
here. It actually increases the footprint for the compatibility
layer and there has not exactly been a lot of pressure to
introduce these controls right now.

Ciao,

Uli

-- 
VGER BF report: U 0.5