WE-21 support (core API)

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

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

diff -u -p linux/include/linux/wireless.20.h linux/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 - <[email protected]>
  * 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
@@ -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
@@ -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
 

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

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

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

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

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

index 50a4719..91dc36c 100644
@@ -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 */

index 1358856..7a5860f 100644
@@ -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 - <[email protected]>
  * 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 */
 };
 
 /*

index 1347276..f47f319 100644
@@ -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
 

1.4.2.GIT

-- 
John W. Linville
[email protected]

I am OK with this.

Greetings Michael.

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

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

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

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

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
[email protected]

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.

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, and have fun...

Jean

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

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

index 50a4719..91dc36c 100644
@@ -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 */

index 1358856..7a5860f 100644
@@ -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 - <[email protected]>
  * 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 */
 };
 
 /*

index 1347276..f47f319 100644
@@ -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
 

1.4.2.GIT

-- 
John W. Linville
[email protected]

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

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.

	Have fun...

	Jean

:)

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.

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

                                             ^^^^^^^

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

Looks good to me.

johannes

And I strongly disagree with your disagrement ;-)

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.

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.

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

	Have fun...

	Jean

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

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.

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.

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.

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

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

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.

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.

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.

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.

:)

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.

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

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

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

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 ?

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

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 ?

Regards,

Jean

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

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

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.

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.

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.

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.

Jean

VGER BF report: U 0.5

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

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

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

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

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

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

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.

Excuse me?

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.

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

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

Wrong answer.

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

Footprint scalability.

Jean

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.

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