[RFC] Wireless extensions rethink

[RFC] Wireless extensions rethink

[Feldman, Scott]

Jeff suggested in an earlier post that there is an opportunity to
totally rethink the wireless extensions now that we have wireless-2.6.

Let's get rid of the iotcl and /proc interfaces as Jeff suggests:

1) iw_handler API goes away and is replaced by struct 
   net_device::wireless_ops (ala ethtool_ops).
2) sysfs get/set mapping for wireless_ops.
3) iw_statistics just becomes one of the wireless_ops.
4) Remove /proc/net/wireless support from wireless.c.  (Already
   have sysfs support for the same :)
5) No private handler support.  If you need private support,
   pass it in some other way (custom sysfs of modparam).  Or, 
   better yet, make a case that others could benefit and move
   into wireless_ops as standard.
6) Convert drivers from iw_handler and iw_statistics to
   wireless_ops.
7) Rewrite iw* tools to use sysfs interface rather than ioctl.
   (scriptable tools?)
8) [Optional] Remove iotcl interface.  May want to keep for
   backward compat with legacy tools?  Easy to map between
   ioctl and wireless_ops in wireless.c.
9) [Open] What to do about wireless events?  Any ideas?

Proposed sysfs layout:

	class/
	`-- net
	    |-- eth[x]
	        |-- wireless
	            |-- statistics
	            |   |-- beacon
			|   |-- crypt
	            |   `-- ...
	            `-- control
	                |-- commit
	                |-- name
	                |-- network_id
	                |-- freq
	                `-- ...

Is someone already working on this???

Comments?

-scott

Re: [RFC] Wireless extensions rethink

[Stephen Hemminger]

On Mon, 7 Jun 2004 11:33:10 -0700
"Feldman, Scott" <scott.feldman@intel.com> wrote:

> Jeff suggested in an earlier post that there is an opportunity to
> totally rethink the wireless extensions now that we have wireless-2.6.
> 
> Let's get rid of the iotcl and /proc interfaces as Jeff suggests:
> 
> 1) iw_handler API goes away and is replaced by struct 
>    net_device::wireless_ops (ala ethtool_ops).
> 2) sysfs get/set mapping for wireless_ops.
> 3) iw_statistics just becomes one of the wireless_ops.
> 4) Remove /proc/net/wireless support from wireless.c.  (Already
>    have sysfs support for the same :)
> 5) No private handler support.  If you need private support,
>    pass it in some other way (custom sysfs of modparam).  Or, 
>    better yet, make a case that others could benefit and move
>    into wireless_ops as standard.
> 6) Convert drivers from iw_handler and iw_statistics to
>    wireless_ops.
> 7) Rewrite iw* tools to use sysfs interface rather than ioctl.
>    (scriptable tools?)
> 8) [Optional] Remove iotcl interface.  May want to keep for
>    backward compat with legacy tools?  Easy to map between
>    ioctl and wireless_ops in wireless.c.
> 9) [Open] What to do about wireless events?  Any ideas?
> 
> Proposed sysfs layout:
> 
> 	class/
> 	`-- net
> 	    |-- eth[x]
> 	        |-- wireless
> 	            |-- statistics
> 	            |   |-- beacon
> 			|   |-- crypt
> 	            |   `-- ...
> 	            `-- control
> 	                |-- commit
> 	                |-- name
> 	                |-- network_id
> 	                |-- freq
> 	                `-- ...

That layout would mean that wireless needs to be a separate object
(allocation/structure/kobject).  Not bad, just one more issue to deal with.

Go ahead and view existing sysfs wireless stuff as a prototype since no tools
are using it (that I know of) yet.

Re: [RFC] Wireless extensions rethink

[Gertjan van Wingerde]

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

Hi Scott,

I was thinking along the same lines, however I was taking the ethtool
interface as the starting point (using a single ioctl for all wireless
operations). The private handlers would just have to be converted to
plain ioctls handled by the driver itself.

The attached patch can be used as a starting point for this.
It is not complete (not by far), but it shows the basic structure.
I've called the structure wlantool_ops, again using the example set by
ethtool.

Comments?

--- Gertjan.

On Mon, 7 Jun 2004 11:33:10 -0700 "Feldman, Scott" <scott.feldman@intel.com> wrote:
 > Jeff suggested in an earlier post that there is an opportunity to
 > totally rethink the wireless extensions now that we have wireless-2.6.
 >
 > Let's get rid of the iotcl and /proc interfaces as Jeff suggests:
 >
 > 1) iw_handler API goes away and is replaced by struct
 >    net_device::wireless_ops (ala ethtool_ops).
 > 2) sysfs get/set mapping for wireless_ops.
 > 3) iw_statistics just becomes one of the wireless_ops.
 > 4) Remove /proc/net/wireless support from wireless.c.  (Already
 >    have sysfs support for the same :)
 > 5) No private handler support.  If you need private support,
 >    pass it in some other way (custom sysfs of modparam).  Or,
 >    better yet, make a case that others could benefit and move
 >    into wireless_ops as standard.
 > 6) Convert drivers from iw_handler and iw_statistics to
 >    wireless_ops.
 > 7) Rewrite iw* tools to use sysfs interface rather than ioctl.
 >    (scriptable tools?)
 > 8) [Optional] Remove iotcl interface.  May want to keep for
 >    backward compat with legacy tools?  Easy to map between
 >    ioctl and wireless_ops in wireless.c.
 > 9) [Open] What to do about wireless events?  Any ideas?
 >
 > Proposed sysfs layout:
 >
 > 	class/
 > 	`-- net
 > 	    |-- eth[x]
 > 	        |-- wireless
 > 	            |-- statistics
 > 	            |   |-- beacon
 > 			|   |-- crypt
 > 	            |   `-- ...
 > 	            `-- control
 > 	                |-- commit
 > 	                |-- name
 > 	                |-- network_id
 > 	                |-- freq
 > 	                `-- ...
 >
 >Is someone already working on this???
 >
 >Comments?
 >
 >-scott




[-- Attachment #2: wlantool.diff --]
[-- Type: text/plain, Size: 10145 bytes --]

diff -Nru a/include/linux/netdevice.h b/include/linux/netdevice.h
--- a/include/linux/netdevice.h	2004-06-07 21:29:13 +02:00
+++ b/include/linux/netdevice.h	2004-06-07 21:29:13 +02:00
@@ -41,6 +41,7 @@
 struct divert_blk;
 struct vlan_group;
 struct ethtool_ops;
+struct wlantool_ops;
 
 					/* source back-compat hooks */
 #define SET_ETHTOOL_OPS(netdev,ops) \
@@ -308,6 +309,8 @@
 
 	struct ethtool_ops *ethtool_ops;
 
+    struct wlantool_ops *wlantool_ops;
+
 	/*
 	 * This marks the end of the "visible" part of the structure. All
 	 * fields hereafter are internal to the system, and may change at
@@ -678,6 +681,7 @@
 extern int		netif_receive_skb(struct sk_buff *skb);
 extern int		dev_ioctl(unsigned int cmd, void __user *);
 extern int		dev_ethtool(struct ifreq *);
+extern int      dev_wlantool(struct ifreq *);
 extern unsigned		dev_get_flags(const struct net_device *);
 extern int		dev_change_flags(struct net_device *, unsigned);
 extern int		dev_set_mtu(struct net_device *, int);
diff -Nru a/include/linux/sockios.h b/include/linux/sockios.h
--- a/include/linux/sockios.h	2004-06-07 21:29:13 +02:00
+++ b/include/linux/sockios.h	2004-06-07 21:29:13 +02:00
@@ -83,6 +83,8 @@
 
 #define SIOCWANDEV	0x894A		/* get/set netdev parameters	*/
 
+#define SIOCWLANTOOL	0x894B		/* WLANtool interface		*/
+
 /* ARP cache control calls. */
 		    /*  0x8950 - 0x8952  * obsolete calls, don't re-use */
 #define SIOCDARP	0x8953		/* delete ARP table entry	*/
diff -Nru a/include/linux/wlantool.h b/include/linux/wlantool.h
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/include/linux/wlantool.h	2004-06-07 21:29:13 +02:00
@@ -0,0 +1,72 @@
+/*
+ * wlantool.h: Defines for Linux WLANtool.
+ *
+ * Copyright (C) 2004 Gertjan van Wingerde (gwingerde@home.nl)
+ */
+
+#ifndef _LINUX_WLANTOOL_H
+#define _LINUX_WLANTOOL_H
+
+#include <linux/if.h>       /* For IFNAMSIZ, etc. */
+
+/* This should work for both 32 and 64 bit userland. */
+struct wlantool_name {
+	u32	 cmd;
+    char name[IFNAMSIZ];
+};
+
+struct wlantool_param {
+    u32  cmd;
+    s32  value;
+    u8   fixed;
+    u8   disabled;
+    u16  flags;
+};
+
+struct wlantool_freq {
+    u32  cmd;
+    s32  mantissa;
+    s16  exponent;
+    u8   index;
+    u8   reserved;
+};
+
+struct wlantool_mode {
+    u32 cmd;
+    u32 mode;
+};
+
+struct net_device;
+
+/**
+ * &wlantool_ops - Alter and report network device settings
+ *
+ * Description:
+ *
+ */
+struct wlantool_ops {
+    int  (*commit)(struct net_device *);
+    void (*get_name)(struct net_device *, struct wlantool_name *);
+    int  (*get_nwid)(struct net_device *, struct wlantool_param *);
+    int  (*set_nwid)(struct net_device *, struct wlantool_param *);
+    int  (*get_freq)(struct net_device *, struct wlantool_freq *);
+    int  (*set_freq)(struct net_device *, struct wlantool_freq *);
+    int  (*get_mode)(struct net_device *, struct wlantool_mode *);
+    int  (*set_mode)(struct net_device *, struct wlantool_mode *);
+    int  (*get_sens)(struct net_device *, struct wlantool_param *);
+    int  (*set_sens)(struct net_device *, struct wlantool_param *);
+};
+
+/* CMDs currently supported */
+#define WLANTOOL_COMMIT		0x00000001 /* Commit pending changes. */
+#define WLANTOOL_GNAME		0x00000002 /* Get name (=wireless protocol). */
+#define WLANTOOL_GNWID  	0x00000003 /* Get network ID (the cell). */
+#define WLANTOOL_SNWID		0x00000004 /* Set network ID (pre-802.11). */
+#define WLANTOOL_GFREQ		0x00000005 /* Get channel/frequency (Hz). */
+#define WLANTOOL_SFREQ		0x00000006 /* Set channel/frequency (Hz). */
+#define WLANTOOL_GMODE		0x00000007 /* Get operation mode. */
+#define WLANTOOL_SMODE		0x00000008 /* Set operation mode. */
+#define WLANTOOL_GSENS   	0x00000009 /* Get sensitivity (dBm). */
+#define WLANTOOL_SSENS		0x0000000a /* Set sensitivity (dBm). */
+
+#endif /* _LINUX_WLANTOOL_H */
diff -Nru a/net/core/Makefile b/net/core/Makefile
--- a/net/core/Makefile	2004-06-07 21:29:13 +02:00
+++ b/net/core/Makefile	2004-06-07 21:29:13 +02:00
@@ -7,7 +7,8 @@
 obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
 
 obj-y		     += flow.o dev.o ethtool.o dev_mcast.o dst.o \
-			neighbour.o rtnetlink.o utils.o link_watch.o filter.o
+			neighbour.o rtnetlink.o utils.o link_watch.o filter.o \
+            wlantool.o
 
 obj-$(CONFIG_SYSFS) += net-sysfs.o
 obj-$(CONFIG_NETFILTER) += netfilter.o
diff -Nru a/net/core/dev.c b/net/core/dev.c
--- a/net/core/dev.c	2004-06-07 21:29:13 +02:00
+++ b/net/core/dev.c	2004-06-07 21:29:13 +02:00
@@ -2700,6 +2700,20 @@
 		case SIOCSIFLINK:
 			return -EINVAL;
 
+		case SIOCWLANTOOL:
+			dev_load(ifr.ifr_name);
+			rtnl_lock();
+			ret = dev_wlantool(&ifr);
+			rtnl_unlock();
+			if (!ret) {
+				if (colon)
+					*colon = ':';
+				if (copy_to_user(arg, &ifr,
+						 sizeof(struct ifreq)))
+					ret = -EFAULT;
+			}
+			return ret;
+
 		/*
 		 *	Unknown or private ioctl.
 		 */
diff -Nru a/net/core/wlantool.c b/net/core/wlantool.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/net/core/wlantool.c	2004-06-07 21:29:13 +02:00
@@ -0,0 +1,221 @@
+/*
+ * net/core/wlantool.c - WLANtool ioctl handler
+ * Copyright (c) 2004 Gertjan van Wingerde <gwingerde@home.nL>
+ *
+ * This file is where we call all the wlantool_ops commands to get
+ * the information wlantool needs.  We fall back to calling do_ioctl()
+ * for drivers which haven't been converted to wlantool_ops yet.
+ *
+ * It's GPL, stupid.
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/wlantool.h>
+#include <linux/netdevice.h>
+#include <asm/uaccess.h>
+
+/* Handlers for each wlantool command */
+
+static int wlantool_commit(struct net_device *dev)
+{
+	int err;
+
+	if (!dev->wlantool_ops->commit)
+		return -EOPNOTSUPP;
+
+	err = dev->wlantool_ops->commit(dev);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int wlantool_get_name(struct net_device *dev, void __user *useraddr)
+{
+	struct wlantool_name name;
+
+	if (!dev->wlantool_ops->get_name)
+		return -EOPNOTSUPP;
+
+	dev->wlantool_ops->get_name(dev, &name);
+
+	if (copy_to_user(useraddr, &name, sizeof(name)))
+		return -EFAULT;
+
+    return 0;
+}
+
+static int wlantool_get_nwid(struct net_device *dev, void __user *useraddr)
+{
+	struct wlantool_param param;
+    int err;
+
+	if (!dev->wlantool_ops->get_nwid)
+		return -EOPNOTSUPP;
+
+	err = dev->wlantool_ops->get_nwid(dev, &param);
+
+	if (copy_to_user(useraddr, &param, sizeof(param)))
+		return -EFAULT;
+
+    return 0;
+}
+
+static int wlantool_set_nwid(struct net_device *dev, char __user *useraddr)
+{
+	struct wlantool_param param;
+
+	if (!dev->wlantool_ops->set_nwid)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&param, useraddr, sizeof(param)))
+		return -EFAULT;
+
+	return dev->wlantool_ops->set_nwid(dev, &param);
+}
+
+static int wlantool_get_freq(struct net_device *dev, void __user *useraddr)
+{
+	struct wlantool_freq freq;
+    int err;
+
+	if (!dev->wlantool_ops->get_freq)
+		return -EOPNOTSUPP;
+
+	err = dev->wlantool_ops->get_freq(dev, &freq);
+
+	if (copy_to_user(useraddr, &freq, sizeof(freq)))
+		return -EFAULT;
+
+    return 0;
+}
+
+static int wlantool_set_freq(struct net_device *dev, char __user *useraddr)
+{
+	struct wlantool_freq freq;
+
+	if (!dev->wlantool_ops->set_freq)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&freq, useraddr, sizeof(freq)))
+		return -EFAULT;
+
+	return dev->wlantool_ops->set_freq(dev, &freq);
+}
+
+static int wlantool_get_mode(struct net_device *dev, void __user *useraddr)
+{
+	struct wlantool_mode mode;
+    int err;
+
+	if (!dev->wlantool_ops->get_mode)
+		return -EOPNOTSUPP;
+
+	err = dev->wlantool_ops->get_mode(dev, &mode);
+
+	if (copy_to_user(useraddr, &mode, sizeof(mode)))
+		return -EFAULT;
+
+    return 0;
+}
+
+static int wlantool_set_mode(struct net_device *dev, char __user *useraddr)
+{
+	struct wlantool_mode mode;
+
+	if (!dev->wlantool_ops->set_mode)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&mode, useraddr, sizeof(mode)))
+		return -EFAULT;
+
+	return dev->wlantool_ops->set_mode(dev, &mode);
+}
+
+static int wlantool_get_sens(struct net_device *dev, void __user *useraddr)
+{
+	struct wlantool_param param;
+    int err;
+
+	if (!dev->wlantool_ops->get_sens)
+		return -EOPNOTSUPP;
+
+	err = dev->wlantool_ops->get_sens(dev, &param);
+
+	if (copy_to_user(useraddr, &param, sizeof(param)))
+		return -EFAULT;
+
+    return 0;
+}
+
+static int wlantool_set_sens(struct net_device *dev, char __user *useraddr)
+{
+	struct wlantool_param param;
+
+	if (!dev->wlantool_ops->set_sens)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&param, useraddr, sizeof(param)))
+		return -EFAULT;
+
+	return dev->wlantool_ops->set_sens(dev, &param);
+}
+
+/* The main entry point in this file.  Called from net/core/dev.c */
+
+int dev_wlantool(struct ifreq *ifr)
+{
+	struct net_device *dev = __dev_get_by_name(ifr->ifr_name);
+	void __user *useraddr = (void __user *) ifr->ifr_data;
+	u32 wlancmd;
+
+	/*
+	 * XXX: This can be pushed down into the wlantool_* handlers that
+	 * need it.  Keep existing behaviour for the moment.
+	 */
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
+	if (!dev || !netif_device_present(dev))
+		return -ENODEV;
+
+	if (!dev->wlantool_ops)
+		goto ioctl;
+
+	if (copy_from_user(&wlancmd, useraddr, sizeof (wlancmd)))
+		return -EFAULT;
+
+	switch (wlancmd) {
+	case WLANTOOL_COMMIT:
+		return wlantool_commit(dev);
+	case WLANTOOL_GNAME:
+		return wlantool_get_name(dev, useraddr);
+	case WLANTOOL_GNWID:
+		return wlantool_get_nwid(dev, useraddr);
+	case WLANTOOL_SNWID:
+		return wlantool_set_nwid(dev, useraddr);
+	case WLANTOOL_GFREQ:
+		return wlantool_get_freq(dev, useraddr);
+	case WLANTOOL_SFREQ:
+		return wlantool_set_freq(dev, useraddr);
+	case WLANTOOL_GMODE:
+		return wlantool_get_mode(dev, useraddr);
+	case WLANTOOL_SMODE:
+		return wlantool_set_mode(dev, useraddr);
+	case WLANTOOL_GSENS:
+		return wlantool_get_sens(dev, useraddr);
+	case WLANTOOL_SSENS:
+		return wlantool_set_sens(dev, useraddr);
+	default:
+		return -EOPNOTSUPP;
+	}
+
+ ioctl:
+	if (dev->do_ioctl)
+		return dev->do_ioctl(dev, ifr, SIOCWLANTOOL);
+	return -EOPNOTSUPP;
+}
+
+EXPORT_SYMBOL(dev_wlantool);

Re: [RFC] Wireless extensions rethink

[Ben Greear]

Gertjan van Wingerde wrote:
> Hi Scott,
> 
> I was thinking along the same lines, however I was taking the ethtool
> interface as the starting point (using a single ioctl for all wireless
> operations). The private handlers would just have to be converted to
> plain ioctls handled by the driver itself.
> 
> The attached patch can be used as a starting point for this.
> It is not complete (not by far), but it shows the basic structure.
> I've called the structure wlantool_ops, again using the example set by
> ethtool.
> 
> Comments?

For what it's worth, I like the idea of using a single IOCTL similar
to ethtool...

Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC] Wireless extensions rethink

[Herbert Xu]

Feldman, Scott <scott.feldman@intel.com> wrote:
>
> 9) [Open] What to do about wireless events?  Any ideas?

netlink
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RE: [RFC] Wireless extensions rethink

[Feldman, Scott]

> I was thinking along the same lines, however I was taking the 
> ethtool interface as the starting point (using a single ioctl 
> for all wireless operations). The private handlers would just 
> have to be converted to plain ioctls handled by the driver itself.
> 
> The attached patch can be used as a starting point for this.
> It is not complete (not by far), but it shows the basic structure.
> I've called the structure wlantool_ops, again using the 
> example set by ethtool.
> 
> Comments?

What if we just use the ethtool ioctl that's already defined, and extend
ethtool with a wireless option:

ethtool -w DEVNAME \
	[ nwid N|off|on} ] \
	[ freq x.xx ] \
	[ mode ad-hoc|managed|master|repeater|... ] \
	[ sens N ] \
	[ ... ]

Each one of the sub-options to -w would have it's own ETHTOOL_[G|S]W...
command as well as a type-safe ethtool_op.

Running ethtool DEVNAME dumps ETHTOOL_GW... :

Wireless settings for eth0:
        nwid: AB34
        freq: 2.422G
        mode: managed
        sens: -80

Good/bad idea?

-scott        

Re: [RFC] Wireless extensions rethink

[Gertjan van Wingerde]

Scott,

Sounds like a good idea. I'll start refactoring my work towards that approach. Please bear
with me for a couple of days and I'll post a draft patch for this.

--- Gertjan.


Feldman, Scott wrote:
>>I was thinking along the same lines, however I was taking the 
>>ethtool interface as the starting point (using a single ioctl 
>>for all wireless operations). The private handlers would just 
>>have to be converted to plain ioctls handled by the driver itself.
>>
>>The attached patch can be used as a starting point for this.
>>It is not complete (not by far), but it shows the basic structure.
>>I've called the structure wlantool_ops, again using the 
>>example set by ethtool.
>>
>>Comments?
> 
> 
> What if we just use the ethtool ioctl that's already defined, and extend
> ethtool with a wireless option:
> 
> ethtool -w DEVNAME \
> 	[ nwid N|off|on} ] \
> 	[ freq x.xx ] \
> 	[ mode ad-hoc|managed|master|repeater|... ] \
> 	[ sens N ] \
> 	[ ... ]
> 
> Each one of the sub-options to -w would have it's own ETHTOOL_[G|S]W...
> command as well as a type-safe ethtool_op.
> 
> Running ethtool DEVNAME dumps ETHTOOL_GW... :
> 
> Wireless settings for eth0:
>         nwid: AB34
>         freq: 2.422G
>         mode: managed
>         sens: -80
> 
> Good/bad idea?
> 
> -scott        
> 
> 

Re: [RFC] Wireless extensions rethink

[Vladimir Kondratiev]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I'd like this idea too. When patches will be ready, I am willing to convert my 
driver to use this interface, and I'll provide feedback.
Vladimir.

On Tuesday 15 June 2004 19:39, Gertjan van Wingerde wrote:
> Scott,
>
> Sounds like a good idea. I'll start refactoring my work towards that
> approach. Please bear with me for a couple of days and I'll post a draft
> patch for this.
>
> --- Gertjan.
>
> Feldman, Scott wrote:
> >>I was thinking along the same lines, however I was taking the
> >>ethtool interface as the starting point (using a single ioctl
> >>for all wireless operations). The private handlers would just
> >>have to be converted to plain ioctls handled by the driver itself.
> >>
> >>The attached patch can be used as a starting point for this.
> >>It is not complete (not by far), but it shows the basic structure.
> >>I've called the structure wlantool_ops, again using the
> >>example set by ethtool.
> >>
> >>Comments?
> >
> > What if we just use the ethtool ioctl that's already defined, and extend
> > ethtool with a wireless option:
> >
> > ethtool -w DEVNAME \
> > 	[ nwid N|off|on} ] \
> > 	[ freq x.xx ] \
> > 	[ mode ad-hoc|managed|master|repeater|... ] \
> > 	[ sens N ] \
> > 	[ ... ]
> >
> > Each one of the sub-options to -w would have it's own ETHTOOL_[G|S]W...
> > command as well as a type-safe ethtool_op.
> >
> > Running ethtool DEVNAME dumps ETHTOOL_GW... :
> >
> > Wireless settings for eth0:
> >         nwid: AB34
> >         freq: 2.422G
> >         mode: managed
> >         sens: -80
> >
> > Good/bad idea?
> >
> > -scott
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFAzzBaqxdj7mhC6o0RAjveAJ94RhRFBBYxewaSYuELds8HJXkntACcCQDW
7SxvO7j/88hGxeAL3g8zQ8k=
=iYxo
-----END PGP SIGNATURE-----

Re: [RFC] Wireless extensions rethink

[Scott Feldman]

On Tue, 2004-06-15 at 09:39, Gertjan van Wingerde wrote:
> Sounds like a good idea. I'll start refactoring my work towards that approach. Please bear
> with me for a couple of days and I'll post a draft patch for this.

Gertjan,

24 clock: see how this patch looks.  

It adds just two more ETHTOOL_[G|S]WSET commands to handle the get/set
of the wireless settings.  This works just like ETHTOOL_[G|S]SET by
passing in/out a struct ethtool_wx_cmd that holds all of the wireless
settings.  The idea is the application would get the settings from the
driver, let the user change one or more of the settings, and send all of
the settings back to the driver.  The driver just resets all settings in
one shot.  

I tried to use the correct type for the various settings based on what
the drivers did (unwinding the iw_param and iw_point members).  I
probably missed some details; please help.

Some observations/opens:

1) This is not compatible with the current wireless extensions ioctl, so
iwconfig must be re-written to use ethtool -w. :(
2) Note that wireless stats can just be moved into ETHTOOL_GSTATS.
3) iw_range needs another ETHTOOL_GWRANGE, but it looks like there is
some duplication with ETHTOOL_GWSET, so I'm not sure what's left - would
you look into this?
4) nick is cosmetic, so it's not included here.
5) Individual events are not generated for things like setting ESSID and
NWID.  Since all of these settings are now set in batch, there really is
just one event: wireless settings changed.  Event needs to come from the
ethtool_ops->get_wx_settings() so it's generated regardless of UI.  On
the other hand, is there any practical use for the event in the first
place?  We don't have events for LAN drivers when speed/duplex change,
for example, do we?
6) The whole commit strategy is useless to us now because settings are
always set in batch with ETHTOOL_GWSET.  The driver does an implicit
commit after applying settings.
7) We need a driver to prototype this against - I'm using ipw2100, but
it would be better to use something that's already in the kernel.
8) We need ethtool app updates to add a -w option to set wireless
settings.  
9) ethtool DEVNAME would use either get_settings or get_wx_settings,
depending on which one was implemented in the driver.
10) Not sure what to do with get/set scan.

See if you can make some progress on these open issues.

-scott

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

diff -Naurp linux-2.6.7-rc3-bk1/include/linux/ethtool.h linux-2.6.7-rc3-bk1-ethtool/include/linux/ethtool.h
--- linux-2.6.7-rc3-bk1/include/linux/ethtool.h	2004-05-09 19:32:26.000000000 -0700
+++ linux-2.6.7-rc3-bk1-ethtool/include/linux/ethtool.h	2004-06-16 01:09:44.018094360 -0700
@@ -250,6 +250,49 @@ struct ethtool_stats {
 	u64	data[0];
 };
 
+enum ethtool_wx_mode {
+	ETH_WX_MODE_AUTO		= 0,
+	ETH_WX_MODE_ADHOC,
+	ETH_WX_MODE_INFRA,
+	ETH_WX_MODE_MASTER,
+	ETH_WX_MODE_REPEATER,
+	ETH_WX_MODE_SECOND,
+	ETH_WX_MODE_MONITOR,
+};
+
+enum ethtool_wx_sec_mode {
+	ETH_WX_SEC_MODE_OPEN		= 0,
+	ETH_WX_SEC_MODE_RESTRICTED,
+};
+
+/* for getting/setting wireless settings */
+struct ethtool_wx_cmd {
+	u32	cmd;
+	u16	nwid;		/* Wireless network ID; 0 = disabled */
+	struct {
+		u32	mantissa;
+		u16	exponent;
+	} freq;			/* Operating frequency */
+	u32	mode;		/* Operating mode (ETH_WX_MODE_xxx) */
+	u16	sens;		/* Sensitivity threshold (-dBm) */
+	struct sockaddr wap;	/* Register with access point */
+				/*   auto = 00:00:00:00:00:00 */
+	char	essid[32];	/* ESSID; any = NULL string */
+	u32	rate;		/* Bit rate b/s; 0 = auto */
+	u16	rts;		/* Smallest packet size for which */
+				/* the node sends RTS; 0 = off */
+	u16	frag;		/* Maximum fragment size; 0 = no frag */
+	u16	tx_power;	/* Transmit power in dBm */
+	struct { /* TODO: thit needs work */
+		u16	limit;
+		u32	lifetime;	/* usec */
+	} retry;		/* MAC retransmission */
+	u32	sec_mode;	/* Security mode (ETH_WX_SEC_MODE_xxx) */
+	char	sec_key[32];	/* Security mode encryption key */
+	/* TODO: add struct power */
+	u32	reserved[16];
+};
+
 struct net_device;
 
 /* Some generic methods drivers may use in their ethtool_ops */
@@ -293,6 +336,8 @@ int ethtool_op_set_tso(struct net_device
  * get_strings: Return a set of strings that describe the requested objects 
  * phys_id: Identify the device
  * get_stats: Return statistics about the device
+ * get_wx_settings: Get device-specific wireless settings
+ * set_wx_settings: Set device-specific wireless settings
  *
  * Description:
  *
@@ -351,6 +396,8 @@ struct ethtool_ops {
 	int	(*phys_id)(struct net_device *, u32);
 	int	(*get_stats_count)(struct net_device *);
 	void	(*get_ethtool_stats)(struct net_device *, struct ethtool_stats *, u64 *);
+	int	(*get_wx_settings)(struct net_device *, struct ethtool_wx_cmd *);
+	int	(*set_wx_settings)(struct net_device *, struct ethtool_wx_cmd *);
 };
 
 /* CMDs currently supported */
@@ -386,6 +433,8 @@ struct ethtool_ops {
 #define ETHTOOL_GSTATS		0x0000001d /* get NIC-specific statistics */
 #define ETHTOOL_GTSO		0x0000001e /* Get TSO enable (ethtool_value) */
 #define ETHTOOL_STSO		0x0000001f /* Set TSO enable (ethtool_value) */
+#define ETHTOOL_GWSET		0x00000020 /* Get wireless settings. */
+#define ETHTOOL_SWSET		0x00000021 /* Set wireless settings. */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff -Naurp linux-2.6.7-rc3-bk1/net/core/ethtool.c linux-2.6.7-rc3-bk1-ethtool/net/core/ethtool.c
--- linux-2.6.7-rc3-bk1/net/core/ethtool.c	2004-06-08 11:13:53.000000000 -0700
+++ linux-2.6.7-rc3-bk1-ethtool/net/core/ethtool.c	2004-06-16 01:13:52.615301840 -0700
@@ -645,6 +645,36 @@ static int ethtool_get_stats(struct net_
 	return ret;
 }
 
+static int ethtool_get_wx_settings(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_wx_cmd cmd = { ETHTOOL_GWSET };
+	int err;
+
+	if (!dev->ethtool_ops->get_wx_settings)
+		return -EOPNOTSUPP;
+
+	err = dev->ethtool_ops->get_wx_settings(dev, &cmd);
+	if (err < 0)
+		return err;
+
+	if (copy_to_user(useraddr, &cmd, sizeof(cmd)))
+		return -EFAULT;
+	return 0;
+}
+
+static int ethtool_set_wx_settings(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_wx_cmd cmd;
+
+	if (!dev->ethtool_ops->set_wx_settings)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&cmd, useraddr, sizeof(cmd)))
+		return -EFAULT;
+
+	return dev->ethtool_ops->set_wx_settings(dev, &cmd);
+}
+
 /* The main entry point in this file.  Called from net/core/dev.c */
 
 int dev_ethtool(struct ifreq *ifr)
@@ -730,6 +760,10 @@ int dev_ethtool(struct ifreq *ifr)
 		return ethtool_phys_id(dev, useraddr);
 	case ETHTOOL_GSTATS:
 		return ethtool_get_stats(dev, useraddr);
+	case ETHTOOL_GWSET:
+		return ethtool_get_wx_settings(dev, useraddr);
+	case ETHTOOL_SWSET:
+		return ethtool_set_wx_settings(dev, useraddr);
 	default:
 		return -EOPNOTSUPP;
 	}

Re: [RFC] Wireless extensions rethink

[Gerald Britton]

A few comments below.

On Wed, Jun 16, 2004 at 02:13:18AM -0700, Scott Feldman wrote:
> +/* for getting/setting wireless settings */
> +struct ethtool_wx_cmd {
> +	u32	cmd;
> +	u16	nwid;		/* Wireless network ID; 0 = disabled */
> +	struct {
> +		u32	mantissa;
> +		u16	exponent;
> +	} freq;			/* Operating frequency */
> +	u32	mode;		/* Operating mode (ETH_WX_MODE_xxx) */
> +	u16	sens;		/* Sensitivity threshold (-dBm) */
> +	struct sockaddr wap;	/* Register with access point */
> +				/*   auto = 00:00:00:00:00:00 */
> +	char	essid[32];	/* ESSID; any = NULL string */

This isn't sufficient as you can have \0 bytes in the ESSID so treating it
as a null-terminated string is probably not ideal.  Also the spec specifies
32 characters as a max, but the 802.11 management IE's could support upto
255 character essid's, this probably needs a little extra thought.

> +	u32	rate;		/* Bit rate b/s; 0 = auto */
> +	u16	rts;		/* Smallest packet size for which */
> +				/* the node sends RTS; 0 = off */
> +	u16	frag;		/* Maximum fragment size; 0 = no frag */
> +	u16	tx_power;	/* Transmit power in dBm */
> +	struct { /* TODO: thit needs work */
> +		u16	limit;
> +		u32	lifetime;	/* usec */
> +	} retry;		/* MAC retransmission */
> +	u32	sec_mode;	/* Security mode (ETH_WX_SEC_MODE_xxx) */
> +	char	sec_key[32];	/* Security mode encryption key */

Similar here, is 32 characters worth of "key" enough here.

> +	/* TODO: add struct power */
> +	u32	reserved[16];
> +};

Also a quick thought to settings many drivers have in their iwpriv
commands such as operating modes .11b/.11a/.11g/auto.  A survey of a bunch
of drivers is probably worth doing to collect where the previous wireless
extentions didn't really fit their needs.

				-- Gerald

Re: [RFC] Wireless extensions rethink

[Vladimir Kondratiev]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wednesday 16 June 2004 18:28, Gerald Britton wrote:
> A few comments below.
>
> On Wed, Jun 16, 2004 at 02:13:18AM -0700, Scott Feldman wrote:
> > +/* for getting/setting wireless settings */
> > +struct ethtool_wx_cmd {
> > +	u32	cmd;
> > +	u16	nwid;		/* Wireless network ID; 0 = disabled */
> > +	struct {
> > +		u32	mantissa;
> > +		u16	exponent;
> > +	} freq;			/* Operating frequency */
> > +	u32	mode;		/* Operating mode (ETH_WX_MODE_xxx) */
> > +	u16	sens;		/* Sensitivity threshold (-dBm) */
> > +	struct sockaddr wap;	/* Register with access point */
> > +				/*   auto = 00:00:00:00:00:00 */
> > +	char	essid[32];	/* ESSID; any = NULL string */
>
> This isn't sufficient as you can have \0 bytes in the ESSID so treating it
> as a null-terminated string is probably not ideal.  Also the spec specifies
> 32 characters as a max, but the 802.11 management IE's could support upto
> 255 character essid's, this probably needs a little extra thought.

... And WiFi compliance tests include testing for ESSID with \0 and other 
non-printable characters in all places.

Sure, ESSID length should be added. One byte is sufficient. In all places 
within MAC, it is used as "info element" like
struct ie {
  u8 id;
  u8 length;
  u8 content[0];
};
Is it worth to keep it in this format?

Other candidates to include:
- - supported rates
- - Channel width. In Japan, in addition to 20Mhz channels, there are 10Mzh 
ones.
- - country information. Country code and Tx power limit per channel group
- - there are 2 independent keys: unicast and mcast, each may belong to 
different suite (AES, TKIP, WEP...)
- - TGe data: air access parameters per access category, list of TSPECs, whether 
STA uses aggregate schedule, whether STA uses APSD (automatic power save 
delivery)

>
> > +	u32	rate;		/* Bit rate b/s; 0 = auto */
> > +	u16	rts;		/* Smallest packet size for which */
> > +				/* the node sends RTS; 0 = off */
> > +	u16	frag;		/* Maximum fragment size; 0 = no frag */
> > +	u16	tx_power;	/* Transmit power in dBm */
> > +	struct { /* TODO: thit needs work */
> > +		u16	limit;
> > +		u32	lifetime;	/* usec */
> > +	} retry;		/* MAC retransmission */
> > +	u32	sec_mode;	/* Security mode (ETH_WX_SEC_MODE_xxx) */
> > +	char	sec_key[32];	/* Security mode encryption key */
>
> Similar here, is 32 characters worth of "key" enough here.
>
> > +	/* TODO: add struct power */
> > +	u32	reserved[16];
> > +};
>
> Also a quick thought to settings many drivers have in their iwpriv
> commands such as operating modes .11b/.11a/.11g/auto.  A survey of a bunch
> of drivers is probably worth doing to collect where the previous wireless
> extentions didn't really fit their needs.
>
> 				-- Gerald
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFA0IX8qxdj7mhC6o0RAu6pAJ9gZsba1Fv7PE4ELFZTr6Tj7KkgfQCgrwIw
/H6eBtA6jETvx2N/YzhAvTw=
=pOUh
-----END PGP SIGNATURE-----

Re: [RFC] Wireless extensions rethink

[Gertjan van Wingerde]

Scott,

I'm afraid that this isn't enough. I think we have to split up the various settings over
multiple commands. If we add just two commands (one for GET and one for SET) we have to
change the binary ioctl-interface every time a new setting arrives (e.g. see how new
settings will be added due to WPA support. That solution just isn't future-proof.

Jean, Jouni, Jeff: What are your thoughts about this?

Therefore I suggest we combine some of the basic settings in one command, and try to
keep other settings in separate commands (again logically coherent settings should
be combined in 1 command). I'll work on that, but unfortunately I can't hardly do anything
until the weekend. I agree that we should be able to remove the commit command if we are
able to set multiple settings that belong together at the same time.
BTW I agree with most of the points your making. We just have to make sure that we define
an interface that is sane and is future-proof with respect to future changes.

--- Gertjan.

Scott Feldman wrote:
> On Tue, 2004-06-15 at 09:39, Gertjan van Wingerde wrote:
> 
>>Sounds like a good idea. I'll start refactoring my work towards that approach. Please bear
>>with me for a couple of days and I'll post a draft patch for this.
> 
> 
> Gertjan,
> 
> 24 clock: see how this patch looks.  
> 
> It adds just two more ETHTOOL_[G|S]WSET commands to handle the get/set
> of the wireless settings.  This works just like ETHTOOL_[G|S]SET by
> passing in/out a struct ethtool_wx_cmd that holds all of the wireless
> settings.  The idea is the application would get the settings from the
> driver, let the user change one or more of the settings, and send all of
> the settings back to the driver.  The driver just resets all settings in
> one shot.  
> 
> I tried to use the correct type for the various settings based on what
> the drivers did (unwinding the iw_param and iw_point members).  I
> probably missed some details; please help.
> 
> Some observations/opens:
> 
> 1) This is not compatible with the current wireless extensions ioctl, so
> iwconfig must be re-written to use ethtool -w. :(
> 2) Note that wireless stats can just be moved into ETHTOOL_GSTATS.
> 3) iw_range needs another ETHTOOL_GWRANGE, but it looks like there is
> some duplication with ETHTOOL_GWSET, so I'm not sure what's left - would
> you look into this?
> 4) nick is cosmetic, so it's not included here.
> 5) Individual events are not generated for things like setting ESSID and
> NWID.  Since all of these settings are now set in batch, there really is
> just one event: wireless settings changed.  Event needs to come from the
> ethtool_ops->get_wx_settings() so it's generated regardless of UI.  On
> the other hand, is there any practical use for the event in the first
> place?  We don't have events for LAN drivers when speed/duplex change,
> for example, do we?
> 6) The whole commit strategy is useless to us now because settings are
> always set in batch with ETHTOOL_GWSET.  The driver does an implicit
> commit after applying settings.
> 7) We need a driver to prototype this against - I'm using ipw2100, but
> it would be better to use something that's already in the kernel.
> 8) We need ethtool app updates to add a -w option to set wireless
> settings.  
> 9) ethtool DEVNAME would use either get_settings or get_wx_settings,
> depending on which one was implemented in the driver.
> 10) Not sure what to do with get/set scan.
> 
> See if you can make some progress on these open issues.
> 
> -scott
> 
> ----------------
> 
> diff -Naurp linux-2.6.7-rc3-bk1/include/linux/ethtool.h linux-2.6.7-rc3-bk1-ethtool/include/linux/ethtool.h
> --- linux-2.6.7-rc3-bk1/include/linux/ethtool.h	2004-05-09 19:32:26.000000000 -0700
> +++ linux-2.6.7-rc3-bk1-ethtool/include/linux/ethtool.h	2004-06-16 01:09:44.018094360 -0700
> @@ -250,6 +250,49 @@ struct ethtool_stats {
>  	u64	data[0];
>  };
>  
> +enum ethtool_wx_mode {
> +	ETH_WX_MODE_AUTO		= 0,
> +	ETH_WX_MODE_ADHOC,
> +	ETH_WX_MODE_INFRA,
> +	ETH_WX_MODE_MASTER,
> +	ETH_WX_MODE_REPEATER,
> +	ETH_WX_MODE_SECOND,
> +	ETH_WX_MODE_MONITOR,
> +};
> +
> +enum ethtool_wx_sec_mode {
> +	ETH_WX_SEC_MODE_OPEN		= 0,
> +	ETH_WX_SEC_MODE_RESTRICTED,
> +};
> +
> +/* for getting/setting wireless settings */
> +struct ethtool_wx_cmd {
> +	u32	cmd;
> +	u16	nwid;		/* Wireless network ID; 0 = disabled */
> +	struct {
> +		u32	mantissa;
> +		u16	exponent;
> +	} freq;			/* Operating frequency */
> +	u32	mode;		/* Operating mode (ETH_WX_MODE_xxx) */
> +	u16	sens;		/* Sensitivity threshold (-dBm) */
> +	struct sockaddr wap;	/* Register with access point */
> +				/*   auto = 00:00:00:00:00:00 */
> +	char	essid[32];	/* ESSID; any = NULL string */
> +	u32	rate;		/* Bit rate b/s; 0 = auto */
> +	u16	rts;		/* Smallest packet size for which */
> +				/* the node sends RTS; 0 = off */
> +	u16	frag;		/* Maximum fragment size; 0 = no frag */
> +	u16	tx_power;	/* Transmit power in dBm */
> +	struct { /* TODO: thit needs work */
> +		u16	limit;
> +		u32	lifetime;	/* usec */
> +	} retry;		/* MAC retransmission */
> +	u32	sec_mode;	/* Security mode (ETH_WX_SEC_MODE_xxx) */
> +	char	sec_key[32];	/* Security mode encryption key */
> +	/* TODO: add struct power */
> +	u32	reserved[16];
> +};
> +
>  struct net_device;
>  
>  /* Some generic methods drivers may use in their ethtool_ops */
> @@ -293,6 +336,8 @@ int ethtool_op_set_tso(struct net_device
>   * get_strings: Return a set of strings that describe the requested objects 
>   * phys_id: Identify the device
>   * get_stats: Return statistics about the device
> + * get_wx_settings: Get device-specific wireless settings
> + * set_wx_settings: Set device-specific wireless settings
>   *
>   * Description:
>   *
> @@ -351,6 +396,8 @@ struct ethtool_ops {
>  	int	(*phys_id)(struct net_device *, u32);
>  	int	(*get_stats_count)(struct net_device *);
>  	void	(*get_ethtool_stats)(struct net_device *, struct ethtool_stats *, u64 *);
> +	int	(*get_wx_settings)(struct net_device *, struct ethtool_wx_cmd *);
> +	int	(*set_wx_settings)(struct net_device *, struct ethtool_wx_cmd *);
>  };
>  
>  /* CMDs currently supported */
> @@ -386,6 +433,8 @@ struct ethtool_ops {
>  #define ETHTOOL_GSTATS		0x0000001d /* get NIC-specific statistics */
>  #define ETHTOOL_GTSO		0x0000001e /* Get TSO enable (ethtool_value) */
>  #define ETHTOOL_STSO		0x0000001f /* Set TSO enable (ethtool_value) */
> +#define ETHTOOL_GWSET		0x00000020 /* Get wireless settings. */
> +#define ETHTOOL_SWSET		0x00000021 /* Set wireless settings. */
>  
>  /* compatibility with older code */
>  #define SPARC_ETH_GSET		ETHTOOL_GSET
> diff -Naurp linux-2.6.7-rc3-bk1/net/core/ethtool.c linux-2.6.7-rc3-bk1-ethtool/net/core/ethtool.c
> --- linux-2.6.7-rc3-bk1/net/core/ethtool.c	2004-06-08 11:13:53.000000000 -0700
> +++ linux-2.6.7-rc3-bk1-ethtool/net/core/ethtool.c	2004-06-16 01:13:52.615301840 -0700
> @@ -645,6 +645,36 @@ static int ethtool_get_stats(struct net_
>  	return ret;
>  }
>  
> +static int ethtool_get_wx_settings(struct net_device *dev, void __user *useraddr)
> +{
> +	struct ethtool_wx_cmd cmd = { ETHTOOL_GWSET };
> +	int err;
> +
> +	if (!dev->ethtool_ops->get_wx_settings)
> +		return -EOPNOTSUPP;
> +
> +	err = dev->ethtool_ops->get_wx_settings(dev, &cmd);
> +	if (err < 0)
> +		return err;
> +
> +	if (copy_to_user(useraddr, &cmd, sizeof(cmd)))
> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static int ethtool_set_wx_settings(struct net_device *dev, void __user *useraddr)
> +{
> +	struct ethtool_wx_cmd cmd;
> +
> +	if (!dev->ethtool_ops->set_wx_settings)
> +		return -EOPNOTSUPP;
> +
> +	if (copy_from_user(&cmd, useraddr, sizeof(cmd)))
> +		return -EFAULT;
> +
> +	return dev->ethtool_ops->set_wx_settings(dev, &cmd);
> +}
> +
>  /* The main entry point in this file.  Called from net/core/dev.c */
>  
>  int dev_ethtool(struct ifreq *ifr)
> @@ -730,6 +760,10 @@ int dev_ethtool(struct ifreq *ifr)
>  		return ethtool_phys_id(dev, useraddr);
>  	case ETHTOOL_GSTATS:
>  		return ethtool_get_stats(dev, useraddr);
> +	case ETHTOOL_GWSET:
> +		return ethtool_get_wx_settings(dev, useraddr);
> +	case ETHTOOL_SWSET:
> +		return ethtool_set_wx_settings(dev, useraddr);
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> 
> 
> 

Re: [RFC] Wireless extensions rethink

[Scott Feldman]

On Wed, 2004-06-16 at 08:28, Gerald Britton wrote:
> A few comments below.
> [snip]
> > +	char	essid[32];	/* ESSID; any = NULL string */
> 
> This isn't sufficient as you can have \0 bytes in the ESSID so treating it
> as a null-terminated string is probably not ideal.  Also the spec specifies
> 32 characters as a max, but the 802.11 management IE's could support upto
> 255 character essid's, this probably needs a little extra thought.

Ok, how about

	u8	essid[32];	/* ESSID; any = all zeros */

On the size, include/linux/wireless.h has it defined as 32:

/* Maximum size of the ESSID and NICKN strings */
#define IW_ESSID_MAX_SIZE       32
                                                                            > [snip]
> > +	char	sec_key[32];	/* Security mode encryption key */
> 
> Similar here, is 32 characters worth of "key" enough here.

include/linux/wireless.h has it defined as 32:

/* Maximum size of the encoding token in bytes */
#define IW_ENCODING_TOKEN_MAX   32      /* 256 bits (for now) */
                                                                        
> Also a quick thought to settings many drivers have in their iwpriv
> commands such as operating modes .11b/.11a/.11g/auto.  A survey of a bunch
> of drivers is probably worth doing to collect where the previous wireless
> extentions didn't really fit their needs.

So far we've only looked at the standard settings, but you're right, we
should move commonly used private settings over to the standard set. 
There are some drivers (i.e. prism54) that use module params that are
duplicates of the standard settings.  These need cleaning up as well.

-scott

Re: [RFC] Wireless extensions rethink

[Gerald Britton]

On Wed, Jun 16, 2004 at 10:53:38AM -0700, Scott Feldman wrote:
> On Wed, 2004-06-16 at 08:28, Gerald Britton wrote:
> > A few comments below.
> > [snip]
> > > +	char	essid[32];	/* ESSID; any = NULL string */
> > 
> > This isn't sufficient as you can have \0 bytes in the ESSID so treating it
> > as a null-terminated string is probably not ideal.  Also the spec specifies
> > 32 characters as a max, but the 802.11 management IE's could support upto
> > 255 character essid's, this probably needs a little extra thought.
> 
> Ok, how about
> 
> 	u8	essid[32];	/* ESSID; any = all zeros */
> 
> On the size, include/linux/wireless.h has it defined as 32:
> 
> /* Maximum size of the ESSID and NICKN strings */
> #define IW_ESSID_MAX_SIZE       32

IMHO, this is a flaw in the current wireless extensions.  Your idea doesn't
provide any way to determine the actual size of the data.  ssids of \0\0 and
\0\0\0 differ.  I like Vladimir Kondratiev's suggestion of defining things
in terms of info elements (IE's).  This is how they get encoded in the frames
sent over the air.

> > > +	char	sec_key[32];	/* Security mode encryption key */
> > 
> > Similar here, is 32 characters worth of "key" enough here.
> 
> include/linux/wireless.h has it defined as 32:
> 
> /* Maximum size of the encoding token in bytes */
> #define IW_ENCODING_TOKEN_MAX   32      /* 256 bits (for now) */

IMO, simply following the wireless extentions is probably the wrong way to
approach this, there is the opportunity to redesign things to better fit
the actual needs of a generic ieee 802.11 stack in the kernel, and of the
existing and likely future drivers. The current wireless extentions in many
areas attempt to be over generic with interfaces which I've never seen a
device actually implement to others whre the API ends up limiting you (like
the ssid and key setting here).

> > Also a quick thought to settings many drivers have in their iwpriv
> > commands such as operating modes .11b/.11a/.11g/auto.  A survey of a bunch
> > of drivers is probably worth doing to collect where the previous wireless
> > extentions didn't really fit their needs.
> 
> So far we've only looked at the standard settings, but you're right, we
> should move commonly used private settings over to the standard set. 
> There are some drivers (i.e. prism54) that use module params that are
> duplicates of the standard settings.  These need cleaning up as well.
> 
> -scott

Yeah.  a handful of examples i can think of off the top of my head:
antenna selection (diversity), beacon intervals, roaming settings,
promiscuous mode within a bssid vs. fully promiscous receive (this
one is probably highly dependant on generic 802.11 stack support).

				-- Gerald

Re: [RFC] Wireless extensions rethink

[Scott Feldman]

On Wed, 2004-06-16 at 10:46, Gertjan van Wingerde wrote:

> I'm afraid that this isn't enough. I think we have to split up the various settings over
> multiple commands. If we add just two commands (one for GET and one for SET) we have to
> change the binary ioctl-interface every time a new setting arrives (e.g. see how new
> settings will be added due to WPA support. That solution just isn't future-proof.

There is some reserved space at the bottom of the struct to add new
commands.  The question is: how many new commands are needed for the
future?  I think we need to be very careful in keeping the standard
settings for wireless bounded.  In fact, we should take a careful look
at the standard settings already defined and really question their need.

For example, do we really want to burden the user with the decision of
whether they need to set a maximum fragment size?  Or what the
sensitivity threshold should be?  The point is, we need to find the
minimal set of settings to get the job done.

So let's have this discussion first before getting back to the
implementation details.

What is the minimal set of settings to expose to the user[1]?  I would
group them into basic, advanced, and authentication.  Which ones are
read-only?  What's new in the near future?

Once we agree on that set, the implementation falls out.

-scott

[1] By user, I mean someone that's just trying to get their wireless
device to attach to a network, but doesn't know the difference between
11b and 11g or the meaning of RTS.

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

I apologize for not responding earlier...

ethtool is the wrong direction for a few reasons,
* I really should have done ethtool via netlink
* We want to move away from pretending that 802.3 == 802.11, and using 
ethtool only serves to reinforce an association we don't want to make. 
"everything is ethernet" isn't really true :)


About overall API design...

When designing interfaces that the low-level drivers must implement, 
keep two things in mind:

* locking, security checks, and ioctl marshalling/thunking should be 
done in common kernel code, not each hardware driver.
* given that, each wireless hook (callback) should be purpose-specific 
-- which means each callback's arguments and return value vary, 
depending on the callback's purpose.  iw_handler must be killed, because 
that forces more code than necessary to be in each low-level driver.

The example to look at here is struct ethtool_ops, and net/core/ethtool.c.


Low-level drivers should be implementing small, fine-grained hooks NOT 
raw ioctl handlers.  In addition to fostering smaller hardware drivers, 
this insulates the driver interface from ABI changes to a certain degree.

	Jeff

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Wed, Jun 16, 2004 at 07:46:17PM +0200, Gertjan van Wingerde wrote:
> 
> Scott,
> 
> I'm afraid that this isn't enough. I think we have to split up the various 
> settings over
> multiple commands. If we add just two commands (one for GET and one for 
> SET) we have to
> change the binary ioctl-interface every time a new setting arrives (e.g. 
> see how new
> settings will be added due to WPA support. That solution just isn't 
> future-proof.
> 
> Jean, Jouni, Jeff: What are your thoughts about this?

	You are asking me what I think about throwing away the API I
designed and restarting from scratch. You must be joking ;-)
	Furthermore, if you really want to redesign the API, then you
don't want to ask the guy responsible of the first one to avoid its
influence and avoid doing the same mistakes ;-)

	If you really want to have more background on my thoughts,
please look at this thread :
		http://marc.theaimsgroup.com/?t=107835339400003&r=1&w=2

	Seriously, these are my thoughts :
	o Backward/forward compatibility has always be my number one
priority since 1996. There is many wireless drivers *NOT* in the
kernel (see my web page) and many utilities using the current API. For
example, both the Red-Hat configurator and the Debian installer link
with libiw. So, it's not just about rewritting iwconfig.
	o Designing an API is not about choosing if you use multiple
ioctls or a single ioctl, or another mechanism. I don't care about
those details. Making choices will always make people unhappy.
	o I'm slow and lazy, but not opposed to changes, as long as
they are well motivated and designed. Yeah, for WPA, I totally suck,
but Jouni is going to save the day. I'm not opposed to fixing my
mistakes, as long as it's painless for people using the API.
	o If you really want to improve Linux-802.11 support, we
should focus first on the missing parts (in-kernel 802.11 framing and
management), not redo the stuff that already works.
	o I don't want to get in your way, so don't expect much from
me. If you can do a better job than me, then we all win.

	I also asked Jeff why he want to redesign the API, and in the
end there was only two issues :
	1) type-safe handler versus generic handler. I personally
disagree with Jeff on that one. But, you can easilly fix it by
offering wrapper to the current API :
	http://marc.theaimsgroup.com/?l=linux-netdev&m=107896289630224&w=2
	2) The use of ioctls. I've created a patch to add RtNetlink
support to the Wireless Extension API, and so far nobody has commented
on this :
	http://marc.theaimsgroup.com/?l=linux-netdev&m=107846135617655&w=2
	A more recent/functional version of this patch is on my web
page. I'm not going to waste time on this if nobody cares.

> I agree that we should be able to remove the commit 
> command if we are
> able to set multiple settings that belong together at the same time.

	Setting that belong together is highly dependant on how the
firware is written. With most cards, *all* setting belong together
(check the Raylink driver if you have time).
	The commit command in current WE is optional. You don't like
it, you don't use it.

	Are those the answer you were looking for, or did I miss
something ?

	Have fun...

	Jean

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Wed, Jun 16, 2004 at 12:06:21PM -0700, Scott Feldman wrote:
> 
> For example, do we really want to burden the user with the decision of
> whether they need to set a maximum fragment size?  Or what the
> sensitivity threshold should be?  The point is, we need to find the
> minimal set of settings to get the job done.

	For the record, those commands were added in 1997 for
sensitivity and 1999 for frag/rts. For the card of those ancient time,
setting those parameters was important, as the Access Point was not
pushing them to the cards.
	Have fun...

	Jean

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jean Tourrilhes wrote:
> 	Seriously, these are my thoughts :
> 	o Backward/forward compatibility has always be my number one
> priority since 1996. There is many wireless drivers *NOT* in the
> kernel (see my web page) and many utilities using the current API. For
> example, both the Red-Hat configurator and the Debian installer link
> with libiw. So, it's not just about rewritting iwconfig.

This is something on which we'll have to agree to disagree.

In Linux we are free to improve the APIs :)


> 	I also asked Jeff why he want to redesign the API, and in the
> end there was only two issues :
> 	1) type-safe handler versus generic handler. I personally
> disagree with Jeff on that one. But, you can easilly fix it by
> offering wrapper to the current API :
> 	http://marc.theaimsgroup.com/?l=linux-netdev&m=107896289630224&w=2

I think I explained myself better, in the recent post in this thread. 
Such a wrapper does nothing to move the locking, ioctl marshalling, and 
security checks out of the drivers.

ethtool_ops and net/core/ethtool.c were quantum-leap improvements over 
the ethtool handling code that existed to that point.  Wireless needs to 
make the same quantum leap.


> 	2) The use of ioctls. I've created a patch to add RtNetlink
> support to the Wireless Extension API, and so far nobody has commented
> on this :
> 	http://marc.theaimsgroup.com/?l=linux-netdev&m=107846135617655&w=2
> 	A more recent/functional version of this patch is on my web
> page. I'm not going to waste time on this if nobody cares.

Well, I liked the patch at least :)

Take your patch, add new 'struct wireless_ops', convert existing 
wireless handlers, and we're good to go :)

	Jeff

Re: [RFC] Wireless extensions rethink

[Scott Feldman]

On Wed, 2004-06-16 at 12:49, Jeff Garzik wrote:
> I apologize for not responding earlier...
> 
> ethtool is the wrong direction for a few reasons,
> * I really should have done ethtool via netlink
> * We want to move away from pretending that 802.3 == 802.11, and using 
> ethtool only serves to reinforce an association we don't want to make. 
> "everything is ethernet" isn't really true :)

Ok, then Gertjan was on track with his earlier post introducing
wlantool_ops.  Scratch the ethtool idea, my bad.

Gertjan, can we make the wlantool_ops more purpose-specific as Jeff
suggests?

 struct wlantool_ops {
     int  (*commit)(struct net_device *);
     void (*get_name)(struct net_device *, struct wlantool_name *);
-    int  (*get_nwid)(struct net_device *, struct wlantool_param *);
-    int  (*set_nwid)(struct net_device *, struct wlantool_param *);
+    u16  (*get_nwid)(struct net_device *);
+    void (*set_nwid)(struct net_device *, u16);
     int  (*get_freq)(struct net_device *, struct wlantool_freq *);
     int  (*set_freq)(struct net_device *, struct wlantool_freq *);
-    int  (*get_mode)(struct net_device *, struct wlantool_mode *);
-    int  (*set_mode)(struct net_device *, struct wlantool_mode *);
-    int  (*get_sens)(struct net_device *, struct wlantool_param *); 
-    int  (*set_sens)(struct net_device *, struct wlantool_param *);
+    u32  (*get_mode)(struct net_device *);
+    void (*set_mode)(struct net_device *, u32);
+    u16  (*get_sens)(struct net_device *); 
+    void (*set_sens)(struct net_device *, u16);
     <etc>
 };

-scott

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Wed, Jun 16, 2004 at 05:36:27PM -0400, Jeff Garzik wrote:
> 
> This is something on which we'll have to agree to disagree.
> 
> In Linux we are free to improve the APIs :)

	Well, this is where I wish system apps were strongly linked
with the kernel as it is the case in *BSD. They don't have the
headacke of backward/forward compatibility that we have.

> >	I also asked Jeff why he want to redesign the API, and in the
> >end there was only two issues :
> >	1) type-safe handler versus generic handler. I personally
> >disagree with Jeff on that one. But, you can easilly fix it by
> >offering wrapper to the current API :
> >	http://marc.theaimsgroup.com/?l=linux-netdev&m=107896289630224&w=2
> 
> I think I explained myself better, in the recent post in this thread. 
> Such a wrapper does nothing to move the locking, ioctl marshalling, and 
> security checks out of the drivers.

	The iw_handler already take care of the net locking, security
check, and most of the ioctl marshalling. What the driver has to take
care is only the remaining of marshalling and driver locking.

	I think you did not understood my idea at all.
	If you build a set of generic wrappers and hook them in the
iw_handler table, you could trivially add the features you feel are
missing into those wrappers. Those wrappers would expose whatever API
you choose to the driver, call your struct wireless_ops for example.

> >	2) The use of ioctls. I've created a patch to add RtNetlink
> >support to the Wireless Extension API, and so far nobody has commented
> >on this :
> >	http://marc.theaimsgroup.com/?l=linux-netdev&m=107846135617655&w=2
> >	A more recent/functional version of this patch is on my web
> >page. I'm not going to waste time on this if nobody cares.
> 
> Well, I liked the patch at least :)

	Ok, I'll try to resume work on that, after I manage WPA. I
still have issues with the RtNetlink API which was not designed to
perform individual GET requests, and this make the code a bit
tricky. I also want to totally remove the useless pointer from this
API, but need a way to do that transparently.

> 	Jeff

	Have fun...

	Jean

Re: [RFC] Wireless extensions rethink

[Scott Feldman]

On Wed, 2004-06-16 at 13:42, Jean Tourrilhes wrote:
> 	You are asking me what I think about throwing away the API I
> designed and restarting from scratch. You must be joking ;-)
> 	Furthermore, if you really want to redesign the API, then you
> don't want to ask the guy responsible of the first one to avoid its
> influence and avoid doing the same mistakes ;-)

This is more about bringing the wireless extensions in-line with other
APIs in the kernel, not trying to fix mistakes.  Your advise and
guidance would be much appreciated in this process.

-scott

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jean Tourrilhes wrote:
> 	I think you did not understood my idea at all.
> 	If you build a set of generic wrappers and hook them in the
> iw_handler table, you could trivially add the features you feel are
> missing into those wrappers. Those wrappers would expose whatever API
> you choose to the driver, call your struct wireless_ops for example.

That doesn't help anything.  Wrappers add code, while not solving the 
following problems...

You want to expose a _specific_, fine-grained API to the driver.  That 
is by definition the smallest interface one can design for the low-level 
driver.  Anything else requires "additional work" -- from simple 
type-casting to real code.

The three major problems of the current WE iw_handler interface 
specifically are,

1) uses array, rather than a structure of function pointers.  This makes 
assignment of values _very_ easy to screw up.

2) type-opaque interfaces break C compiler pointer analysis.  This 
prevents alias optimization from working well, and also breaks checkers 
like the Stanford checker and sparse.  C gave the world a type system, 
we should use it.  :)

3) Uses and stores offsets to driver-private structures in generic code 
(iw_handler_def).  See #2 for pointer chasing/aliasing problems related 
to doing stuff like this.  Further, this is a massive layer / object 
lifetime violation.  The generic/core code should never need to know 
about a structure declared in a low-level driver.  That's backwards from 
the way information should flow.

It's not Jeff's weird personal preference that iw_handler be killed... 
type-opaque interfaces cause real problems.  A good C programmer should 
very, very rarely use type-casting.

	Jeff

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Wed, Jun 16, 2004 at 07:06:13PM -0400, Jeff Garzik wrote:
> Jean Tourrilhes wrote:
> >	I think you did not understood my idea at all.
> >	If you build a set of generic wrappers and hook them in the
> >iw_handler table, you could trivially add the features you feel are
> >missing into those wrappers. Those wrappers would expose whatever API
> >you choose to the driver, call your struct wireless_ops for example.
> 
> That doesn't help anything.  Wrappers add code, while not solving the 
> following problems...

	You need to start somewhere. This allow you to experiment with
the API simply until you get it to the point you like it.

	Jean

Re: [RFC] Wireless extensions rethink

[Luis R. Rodriguez]

On Wed, Jun 16, 2004 at 10:53:38AM -0700, Scott Feldman wrote:
> There are some drivers (i.e. prism54) that use module params that are
> duplicates of the standard settings.  These need cleaning up as well.

I've removed those already in my current WPA work.

	Luis

-- 
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84  A34A 6ADD 4937 E20A 525E

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Wed, Jun 16, 2004 at 07:06:13PM -0400, Jeff Garzik wrote:
> 
> It's not Jeff's weird personal preference that iw_handler be killed... 
> type-opaque interfaces cause real problems.  A good C programmer should 
> very, very rarely use type-casting.

	Yes, I know that I am really a perverse mind and I did
designed the API this way only to make your life miserable and to
sabotage the Linux kernel.

	As a matter of fact, I tried the strongly type approach you
advocate and find its kernel overhead not acceptable. Note that people
not using wireless have to suffer from this bloat, and wireless
extensions are used in embeeded platforms such as OpenAP, iPaq and
Zaurus where footprint matters.
	There was additional benefits, such as the ability to use a
single handler for multiple commands (horror !), and I know of a few
driver using this feature.

	Now, it seems that you clearly have not understood my
proposal, so I made a little patch, hopping that it will clear the
obvious miscommunication. Patch is below.
	I hope you will notice that the changes are fairly trivial and
clean. And it's backward compatible.
	I also want you to notice that the code is quasi optimal,
compared to rewritting the API, there is only one additional function
call (plus the backward compatibility). I know you won't believe me,
so verify that yourself. I think this is a small price to pay for
keeping backward compatibility with all those drivers outside the
kernel (see my web page).

> 	Jeff

	Now, I will have to excuse myself, and let you guys argue over
my patch. I have a full time job and a new baby that require my
dedicated attention, plus a huge backlog of Linux stuff (WTool
release, WPA, RtNetlink stuff, IrDA patches...). I don't have the
luxury of having Linux as my primary occupation.
	Have fun...

	Jean

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

--- linux/include/net/iw_handler.j1.h	Thu Jun 17 10:16:22 2004
+++ linux/include/net/iw_handler.h	Thu Jun 17 10:17:24 2004
@@ -301,6 +301,9 @@ typedef int (*iw_handler)(struct net_dev
  */
 struct iw_handler_def
 {
+	/* Wireless Ops definitions */
+	struct wireless_ops *	wireless_ops;
+
 	/* Number of handlers defined (more precisely, index of the
 	 * last defined handler + 1) */
 	__u16			num_standard;
--- linux/net/core/wireless.j1.c	Thu Jun 17 10:08:45 2004
+++ linux/net/core/wireless.c	Thu Jun 17 10:28:59 2004
@@ -343,8 +343,14 @@ static inline iw_handler get_handler(str
 	if(dev->wireless_handlers == NULL)
 		return NULL;
 
-	/* Try as a standard command */
 	index = cmd - SIOCIWFIRST;
+
+	/* Try as wireless_ops */
+	if((index < (SIOCIWFIRSTPRIV - SIOCIWFIRST)) &&
+	   (dev->wireless_handlers->wireless_ops != NULL))
+		return &iw_process_wireless_ops;
+
+	/* Try as a standard command */
 	if(index < dev->wireless_handlers->num_standard)
 		return dev->wireless_handlers->standard[index];
 
@@ -1372,3 +1378,41 @@ EXPORT_SYMBOL(iw_handler_set_spy);
 EXPORT_SYMBOL(iw_handler_set_thrspy);
 EXPORT_SYMBOL(wireless_send_event);
 EXPORT_SYMBOL(wireless_spy_update);
+
+/*********************** WIRELESS OPS SUPPORT ***********************/
+/*
+ * Documentation to be added by Jeff ;-)
+ */
+
+/*------------------------------------------------------------------*/
+/*
+ * Generic Wireless Handler to process Wireless Ops
+ */
+int iw_process_wireless_ops(struct net_device *	dev,
+			    struct iw_request_info *info,
+			    union iwreq_data *	wrqu,
+			    char *		extra)
+{
+	struct wireless_ops *	ops;
+
+	/* Already verified != NULL in get_handler() */
+	ops = dev->wireless_handlers->wireless_ops;
+
+	/* Just do it */
+	switch(info->cmd)
+	{
+	case SIOCSIWESSID:
+		/* Set ESSID */
+		if(ops->set_essid != NULL)
+			return ops->set_essid(dev, extra, wrqu->essid.length,
+					      wrqu->essid.flags);
+	case SIOCGIWESSID:
+		/* Get ESSID */
+		if(ops->get_essid != NULL)
+			return ops->get_essid(dev, extra, &wrqu->essid.length,
+					      &wrqu->essid.flags);
+	default:
+	}
+	return -EOPNOTSUPP;
+}
+

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

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

Jean Tourrilhes wrote:
> On Wed, Jun 16, 2004 at 07:06:13PM -0400, Jeff Garzik wrote:
> 
>>It's not Jeff's weird personal preference that iw_handler be killed... 
>>type-opaque interfaces cause real problems.  A good C programmer should 
>>very, very rarely use type-casting.
> 
> 
> 	Yes, I know that I am really a perverse mind and I did
> designed the API this way only to make your life miserable and to
> sabotage the Linux kernel.
> 
> 	As a matter of fact, I tried the strongly type approach you
> advocate and find its kernel overhead not acceptable. Note that people
> not using wireless have to suffer from this bloat, and wireless
> extensions are used in embeeded platforms such as OpenAP, iPaq and
> Zaurus where footprint matters.

As you can see from the patch and header I have attached, there is 
_zero_ change to storage.  No additional bloat.


> 	Now, it seems that you clearly have not understood my
> proposal, so I made a little patch, hopping that it will clear the
> obvious miscommunication. Patch is below.
> 	I hope you will notice that the changes are fairly trivial and
> clean. And it's backward compatible.
> 	I also want you to notice that the code is quasi optimal,
> compared to rewritting the API, there is only one additional function
> call (plus the backward compatibility). I know you won't believe me,
> so verify that yourself. I think this is a small price to pay for
> keeping backward compatibility with all those drivers outside the
> kernel (see my web page).

Sorry, keeping compatibility with drivers outside the kernel is _not_ a 
priority here.  Backward compatibility is how this cruft accumulated in 
the first place.

Go ahead and assume that drivers outside the kernel will break.  This is 
no different from vendor drivers -- if the driver is not in the kernel, 
it doesn't exist.

	Jeff



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4008 bytes --]

===== drivers/net/wireless/wl3501_cs.c 1.77 vs edited =====
--- 1.77/drivers/net/wireless/wl3501_cs.c	2004-06-03 21:38:07 -04:00
+++ edited/drivers/net/wireless/wl3501_cs.c	2004-06-16 19:42:10 -04:00
@@ -46,6 +46,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/wireless.h>
+#include <linux/wifi.h>
 
 #include <net/iw_handler.h>
 
@@ -1959,38 +1960,37 @@
 	return rc;
 }
 
-static const iw_handler	wl3501_handler[] = {
-	[SIOCGIWNAME	- SIOCIWFIRST] = wl3501_get_name,
-	[SIOCSIWFREQ	- SIOCIWFIRST] = wl3501_set_freq,
-	[SIOCGIWFREQ	- SIOCIWFIRST] = wl3501_get_freq,
-	[SIOCSIWMODE	- SIOCIWFIRST] = wl3501_set_mode,
-	[SIOCGIWMODE	- SIOCIWFIRST] = wl3501_get_mode,
-	[SIOCGIWSENS	- SIOCIWFIRST] = wl3501_get_sens,
-	[SIOCGIWRANGE	- SIOCIWFIRST] = wl3501_get_range,
-	[SIOCSIWSPY	- SIOCIWFIRST] = iw_handler_set_spy,
-	[SIOCGIWSPY	- SIOCIWFIRST] = iw_handler_get_spy,
-	[SIOCSIWTHRSPY	- SIOCIWFIRST] = iw_handler_set_thrspy,
-	[SIOCGIWTHRSPY	- SIOCIWFIRST] = iw_handler_get_thrspy,
-	[SIOCSIWAP	- SIOCIWFIRST] = wl3501_set_wap,
-	[SIOCGIWAP	- SIOCIWFIRST] = wl3501_get_wap,
-	[SIOCSIWSCAN	- SIOCIWFIRST] = wl3501_set_scan,
-	[SIOCGIWSCAN	- SIOCIWFIRST] = wl3501_get_scan,
-	[SIOCSIWESSID	- SIOCIWFIRST] = wl3501_set_essid,
-	[SIOCGIWESSID	- SIOCIWFIRST] = wl3501_get_essid,
-	[SIOCSIWNICKN	- SIOCIWFIRST] = wl3501_set_nick,
-	[SIOCGIWNICKN	- SIOCIWFIRST] = wl3501_get_nick,
-	[SIOCGIWRATE	- SIOCIWFIRST] = wl3501_get_rate,
-	[SIOCGIWRTS	- SIOCIWFIRST] = wl3501_get_rts_threshold,
-	[SIOCGIWFRAG	- SIOCIWFIRST] = wl3501_get_frag_threshold,
-	[SIOCGIWTXPOW	- SIOCIWFIRST] = wl3501_get_txpow,
-	[SIOCGIWRETRY	- SIOCIWFIRST] = wl3501_get_retry,
-	[SIOCGIWENCODE	- SIOCIWFIRST] = wl3501_get_encode,
-	[SIOCGIWPOWER	- SIOCIWFIRST] = wl3501_get_power,
+static const struct wireless_ops wl3501_ops = {
+	.get_name		= wl3501_get_name,
+	.get_freq		= wl3501_get_freq,
+	.set_freq		= wl3501_set_freq,
+	.set_mode		= wl3501_set_mode,
+	.get_mode		= wl3501_get_mode,
+	.get_sens		= wl3501_get_sens,
+	.get_range		= wl3501_get_range,
+	.set_spy		= iw_handler_set_spy,
+	.get_spy		= iw_handler_get_spy,
+	.set_thrspy		= iw_handler_set_thrspy,
+	.get_thrspy		= iw_handler_get_thrspy,
+	.set_wap		= wl3501_set_wap,
+	.get_wap		= wl3501_get_wap,
+	.set_scan		= wl3501_set_scan,
+	.get_scan		= wl3501_get_scan,
+	.set_essid		= wl3501_set_essid,
+	.get_essid		= wl3501_get_essid,
+	.set_nick		= wl3501_set_nick,
+	.get_nick		= wl3501_get_nick,
+	.get_rate		= wl3501_get_rate,
+	.get_rts_threshold	= wl3501_get_rts_threshold,
+	.get_frag_threshold	= wl3501_get_frag_threshold,
+	.get_txpow		= wl3501_get_txpow,
+	.get_retry		= wl3501_get_retry,
+	.get_encode		= wl3501_get_encode,
+	.get_power		= wl3501_get_power,
 };
 
-static const struct iw_handler_def wl3501_handler_def = {
-	.num_standard	= sizeof(wl3501_handler) / sizeof(iw_handler),
-	.standard	= (iw_handler *)wl3501_handler,
+static const struct wireless_handler wl3501_handler = {
+	.ops		= &wl3501_ops,
 	.spy_offset	= offsetof(struct wl3501_card, spy_data),
 };
 
@@ -2048,7 +2048,7 @@
 	dev->get_stats		= wl3501_get_stats;
 	dev->get_wireless_stats = wl3501_get_wireless_stats;
 	dev->do_ioctl		= wl3501_ioctl;
-	dev->wireless_handlers	= (struct iw_handler_def *)&wl3501_handler_def;
+	dev->wireless_handler	= &wl3501_handler;
 	netif_stop_queue(dev);
 	link->priv = link->irq.Instance = dev;
 
===== include/linux/netdevice.h 1.80 vs edited =====
--- 1.80/include/linux/netdevice.h	2004-06-04 01:02:47 -04:00
+++ edited/include/linux/netdevice.h	2004-06-16 19:42:38 -04:00
@@ -41,6 +41,7 @@
 struct divert_blk;
 struct vlan_group;
 struct ethtool_ops;
+struct wireless_handler;
 
 					/* source back-compat hooks */
 #define SET_ETHTOOL_OPS(netdev,ops) \
@@ -304,7 +305,7 @@
 
 	/* List of functions to handle Wireless Extensions (instead of ioctl).
 	 * See <net/iw_handler.h> for details. Jean II */
-	struct iw_handler_def *	wireless_handlers;
+	const struct wireless_handler *wireless_handler;
 
 	struct ethtool_ops *ethtool_ops;
 

[-- Attachment #3: wifi.h --]
[-- Type: text/x-chdr, Size: 3813 bytes --]

#ifndef __LINUX_WIFI_H__
#define __LINUX_WIFI_H__

#include <net/iw_handler.h>

struct wireless_ops {
	int (*get_name) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_freq) (struct net_device *dev, struct iw_request_info *info,
	                           union iwreq_data *wrqu, char *extra);
	int (*set_freq) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_mode) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*set_mode) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_sens) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_range) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_spy) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*set_spy) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_thrspy) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*set_thrspy) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_wap) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*set_wap) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_scan) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*set_scan) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_essid) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*set_essid) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_nick) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*set_nick) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_rate) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_rts_threshold) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_frag_threshold) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_txpow) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_retry) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_encode) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);
	int (*get_power) (struct net_device *dev, struct iw_request_info *info,
                           union iwreq_data *wrqu, char *extra);

};

struct wireless_handler {
	const struct wireless_ops	*ops;
	size_t				spy_offset;
};

#endif /* __LINUX_WIFI_H__ */

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jeff Garzik wrote:
> struct wireless_ops {
> 	int (*get_name) (struct net_device *dev, struct iw_request_info *info,
>                            union iwreq_data *wrqu, char *extra);
> 	int (*get_freq) (struct net_device *dev, struct iw_request_info *info,
> 	                           union iwreq_data *wrqu, char *extra);
> 	int (*set_freq) (struct net_device *dev, struct iw_request_info *info,
>                            union iwreq_data *wrqu, char *extra);
> 	int (*get_mode) (struct net_device *dev, struct iw_request_info *info,
>                            union iwreq_data *wrqu, char *extra);
> 	int (*set_mode) (struct net_device *dev, struct iw_request_info *info,
>                            union iwreq_data *wrqu, char *extra);


Note that the above is only a first step.  Through the standard Linux 
development process -- evolution -- each hook can be pared down to 
precisely what each call needs.  The above allows for a quick transition 
of drivers, while keeping them working.

	Jeff

Re: [RFC] Wireless extensions rethink

[Gertjan van Wingerde]

Jeff,

I think I get the gist of it. I'll start working on this and produce some results in a few days.
We can then review the results start discussing further directions/modifications.

I'll leave netlink out of it right now (see the problems Jean had earlier), but I think that the
basic approach (wireless_ops, etc.) can be reused within a netlink implementation anyway.

--- Gertjan.


Jeff Garzik wrote:
> Jeff Garzik wrote:
> 
>> struct wireless_ops {
>>     int (*get_name) (struct net_device *dev, struct iw_request_info 
>> *info,
>>                            union iwreq_data *wrqu, char *extra);
>>     int (*get_freq) (struct net_device *dev, struct iw_request_info 
>> *info,
>>                                union iwreq_data *wrqu, char *extra);
>>     int (*set_freq) (struct net_device *dev, struct iw_request_info 
>> *info,
>>                            union iwreq_data *wrqu, char *extra);
>>     int (*get_mode) (struct net_device *dev, struct iw_request_info 
>> *info,
>>                            union iwreq_data *wrqu, char *extra);
>>     int (*set_mode) (struct net_device *dev, struct iw_request_info 
>> *info,
>>                            union iwreq_data *wrqu, char *extra);
> 
> 
> 
> Note that the above is only a first step.  Through the standard Linux 
> development process -- evolution -- each hook can be pared down to 
> precisely what each call needs.  The above allows for a quick transition 
> of drivers, while keeping them working.
> 
>     Jeff
> 
> 
> 

Re: [RFC] Wireless extensions rethink

[Stephen Hemminger]

On Thu, 17 Jun 2004 14:26:20 -0400
Jeff Garzik <jgarzik@pobox.com> wrote:

> Jeff Garzik wrote:
> > struct wireless_ops {
> > 	int (*get_name) (struct net_device *dev, struct iw_request_info *info,
> >                            union iwreq_data *wrqu, char *extra);
> > 	int (*get_freq) (struct net_device *dev, struct iw_request_info *info,
> > 	                           union iwreq_data *wrqu, char *extra);
> > 	int (*set_freq) (struct net_device *dev, struct iw_request_info *info,
> >                            union iwreq_data *wrqu, char *extra);
> > 	int (*get_mode) (struct net_device *dev, struct iw_request_info *info,
> >                            union iwreq_data *wrqu, char *extra);
> > 	int (*set_mode) (struct net_device *dev, struct iw_request_info *info,
> >                            union iwreq_data *wrqu, char *extra);
> 
> 
> Note that the above is only a first step.  Through the standard Linux 
> development process -- evolution -- each hook can be pared down to 
> precisely what each call needs.  The above allows for a quick transition 
> of drivers, while keeping them working.

Remember the API for applications (netlink) doesn't need to be the same as
the driver interface (wireless_ops).  That is the whole point of having
a common core layer.

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Thu, Jun 17, 2004 at 02:23:01PM -0400, Jeff Garzik wrote:
> Jean Tourrilhes wrote:
> >
> >	As a matter of fact, I tried the strongly type approach you
> >advocate and find its kernel overhead not acceptable. Note that people
> >not using wireless have to suffer from this bloat, and wireless
> >extensions are used in embeeded platforms such as OpenAP, iPaq and
> >Zaurus where footprint matters.
> 
> As you can see from the patch and header I have attached, there is 
> _zero_ change to storage.  No additional bloat.

	I've never talked of driver bloat, which I don't really care
about. I'm talking of *kernel* bloat. And not about storage bloat, but
code bloat.
	When I designed the API, I did verify this carefully :
	http://marc.theaimsgroup.com/?l=linux-kernel&m=100829443600986&w=2

> Sorry, keeping compatibility with drivers outside the kernel is _not_ a 
> priority here.  Backward compatibility is how this cruft accumulated in 
> the first place.
> 
> Go ahead and assume that drivers outside the kernel will break.  This is 
> no different from vendor drivers -- if the driver is not in the kernel, 
> it doesn't exist.

	Jeff, this is not the way I work. For example, there are good
reasons why the Atheros driver is outside the kernel.

> 	Jeff

	Jean

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Thu, Jun 17, 2004 at 02:26:20PM -0400, Jeff Garzik wrote:
> 
> Note that the above is only a first step.  Through the standard Linux 
> development process -- evolution -- each hook can be pared down to 
> precisely what each call needs.  The above allows for a quick transition 
> of drivers, while keeping them working.
> 
> 	Jeff

	Have you looked at the patch I sent you ? In which way does it
fails to meet your need ?

	Jean

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Thu, Jun 17, 2004 at 11:51:56AM -0700, Stephen Hemminger wrote:
> 
> Remember the API for applications (netlink) doesn't need to be the same as
> the driver interface (wireless_ops).  That is the whole point of having
> a common core layer.

	Stephen : we have been there since 2001. Check the RtNetlink
patch on my web page, it allows full access to all wireless extensions
through netlink without having to modify a single driver.
	Have fun...

	Jean

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jean Tourrilhes wrote:
> On Thu, Jun 17, 2004 at 02:26:20PM -0400, Jeff Garzik wrote:
> 
>>Note that the above is only a first step.  Through the standard Linux 
>>development process -- evolution -- each hook can be pared down to 
>>precisely what each call needs.  The above allows for a quick transition 
>>of drivers, while keeping them working.
>>
>>	Jeff
> 
> 
> 	Have you looked at the patch I sent you ? In which way does it
> fails to meet your need ?


The three major problems I listed in a previous email are still 
present...  Also there is a fourth -- WE doesn't work 100% when you have 
a 32-bit userland and a 64-bit kernel.

	Jeff

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jean Tourrilhes wrote:
> On Thu, Jun 17, 2004 at 02:23:01PM -0400, Jeff Garzik wrote:
> 
>>Jean Tourrilhes wrote:
>>
>>>	As a matter of fact, I tried the strongly type approach you
>>>advocate and find its kernel overhead not acceptable. Note that people
>>>not using wireless have to suffer from this bloat, and wireless
>>>extensions are used in embeeded platforms such as OpenAP, iPaq and
>>>Zaurus where footprint matters.
>>
>>As you can see from the patch and header I have attached, there is 
>>_zero_ change to storage.  No additional bloat.
> 
> 
> 	I've never talked of driver bloat, which I don't really care
> about. I'm talking of *kernel* bloat. And not about storage bloat, but
> code bloat.

Yes, I was referring to driver bloat not core kernel bloat.

Nonetheless, the type-safe interface would not be larger, and very 
likely will be smaller.


> 	When I designed the API, I did verify this carefully :
> 	http://marc.theaimsgroup.com/?l=linux-kernel&m=100829443600986&w=2

This WAS a step forward, and this will help greatly in the 
implementation of wireless_ops.  It's good stuff, but we are moving 
forward yet again :)


>>Sorry, keeping compatibility with drivers outside the kernel is _not_ a 
>>priority here.  Backward compatibility is how this cruft accumulated in 
>>the first place.
>>
>>Go ahead and assume that drivers outside the kernel will break.  This is 
>>no different from vendor drivers -- if the driver is not in the kernel, 
>>it doesn't exist.
> 
> 
> 	Jeff, this is not the way I work. For example, there are good
> reasons why the Atheros driver is outside the kernel.

Yes, but...  that's the way the Linux kernel works.

If a driver isn't in the kernel, it's the responsibility of the vendor 
to follow the changes in the kernel.  It's not the kernel developer's 
responsibility to track random stuff posted on web pages.  That's simply 
not scalable.

I imagine this is another area where we must agree to disagree.  Linux 
kernel development has always focused on in-tree drivers.

Wireless traditionally has had a lot of drivers out-of-tree -- and being 
out of tree, we see what happens:  vendors are encouraged to mixed 
binary-only drivers, multiple wireless stacks appears, and confusion 
reigned.

It's now time for convergence :)

	Jeff

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jean Tourrilhes wrote:
> On Thu, Jun 17, 2004 at 11:51:56AM -0700, Stephen Hemminger wrote:
> 
>>Remember the API for applications (netlink) doesn't need to be the same as
>>the driver interface (wireless_ops).  That is the whole point of having
>>a common core layer.
> 
> 
> 	Stephen : we have been there since 2001. Check the RtNetlink
> patch on my web page, it allows full access to all wireless extensions
> through netlink without having to modify a single driver.


Nod...  here's hoping others are checking out out too.

As Stephen points out, though, the driver interface redesign is separate 
from the use of netlink.

	Jeff

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jeff Garzik wrote:
> Yes, but...  that's the way the Linux kernel works.

Er, I meant to say, that's NOT the way the kernel works.

Talk about a Freudian slip :)

	Jeff

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Thu, Jun 17, 2004 at 03:02:49PM -0400, Jeff Garzik wrote:
> Jean Tourrilhes wrote:
> >On Thu, Jun 17, 2004 at 02:26:20PM -0400, Jeff Garzik wrote:
> >
> >>Note that the above is only a first step.  Through the standard Linux 
> >>development process -- evolution -- each hook can be pared down to 
> >>precisely what each call needs.  The above allows for a quick transition 
> >>of drivers, while keeping them working.
> >>
> >>	Jeff
> >
> >
> >	Have you looked at the patch I sent you ? In which way does it
> >fails to meet your need ?
> 
> 
> The three major problems I listed in a previous email are still 
> present...

	Are we talking of the same patch ? I'm talking of this patch :
http://marc.theaimsgroup.com/?l=linux-netdev&m=108749580004668&w=2
	I reattached the patch below. It's short enough.

>  Also there is a fourth -- WE doesn't work 100% when you have 
> a 32-bit userland and a 64-bit kernel.

	Since when ? What made you change your mind ?
	Please check :
http://marc.theaimsgroup.com/?l=linux-netdev&m=107894322418086&w=2

> 	Jeff

	Jean

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

--- linux/include/net/iw_handler.j1.h	Thu Jun 17 10:16:22 2004
+++ linux/include/net/iw_handler.h	Thu Jun 17 10:17:24 2004
@@ -301,6 +301,9 @@ typedef int (*iw_handler)(struct net_dev
  */
 struct iw_handler_def
 {
+	/* Wireless Ops definitions */
+	struct wireless_ops *	wireless_ops;
+
 	/* Number of handlers defined (more precisely, index of the
 	 * last defined handler + 1) */
 	__u16			num_standard;
--- linux/net/core/wireless.j1.c	Thu Jun 17 10:08:45 2004
+++ linux/net/core/wireless.c	Thu Jun 17 10:28:59 2004
@@ -343,8 +343,14 @@ static inline iw_handler get_handler(str
 	if(dev->wireless_handlers == NULL)
 		return NULL;
 
-	/* Try as a standard command */
 	index = cmd - SIOCIWFIRST;
+
+	/* Try as wireless_ops */
+	if((index < (SIOCIWFIRSTPRIV - SIOCIWFIRST)) &&
+	   (dev->wireless_handlers->wireless_ops != NULL))
+		return &iw_process_wireless_ops;
+
+	/* Try as a standard command */
 	if(index < dev->wireless_handlers->num_standard)
 		return dev->wireless_handlers->standard[index];
 
@@ -1372,3 +1378,41 @@ EXPORT_SYMBOL(iw_handler_set_spy);
 EXPORT_SYMBOL(iw_handler_set_thrspy);
 EXPORT_SYMBOL(wireless_send_event);
 EXPORT_SYMBOL(wireless_spy_update);
+
+/*********************** WIRELESS OPS SUPPORT ***********************/
+/*
+ * Documentation to be added by Jeff ;-)
+ */
+
+/*------------------------------------------------------------------*/
+/*
+ * Generic Wireless Handler to process Wireless Ops
+ */
+int iw_process_wireless_ops(struct net_device *	dev,
+			    struct iw_request_info *info,
+			    union iwreq_data *	wrqu,
+			    char *		extra)
+{
+	struct wireless_ops *	ops;
+
+	/* Already verified != NULL in get_handler() */
+	ops = dev->wireless_handlers->wireless_ops;
+
+	/* Just do it */
+	switch(info->cmd)
+	{
+	case SIOCSIWESSID:
+		/* Set ESSID */
+		if(ops->set_essid != NULL)
+			return ops->set_essid(dev, extra, wrqu->essid.length,
+					      wrqu->essid.flags);
+	case SIOCGIWESSID:
+		/* Get ESSID */
+		if(ops->get_essid != NULL)
+			return ops->get_essid(dev, extra, &wrqu->essid.length,
+					      &wrqu->essid.flags);
+	default:
+	}
+	return -EOPNOTSUPP;
+}
+

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Thu, Jun 17, 2004 at 03:09:08PM -0400, Jeff Garzik wrote:
>
> >	When I designed the API, I did verify this carefully :
> >	http://marc.theaimsgroup.com/?l=linux-kernel&m=100829443600986&w=2
> 
> This WAS a step forward, and this will help greatly in the 
> implementation of wireless_ops.  It's good stuff, but we are moving 
> forward yet again :)

	And guess what, I'm helping you in the process. Look back at
all the e-mail I sent to the various thread on the subject, and you
will clearly see that I'm constructive and giving suggestion on how to
do best in this process. I even provide patches.
	I don't understand why you are so opposed to my suggestions,
and what more you expect from me.

> If a driver isn't in the kernel, it's the responsibility of the vendor 
> to follow the changes in the kernel.  It's not the kernel developer's 
> responsibility to track random stuff posted on web pages.  That's simply 
> not scalable.

	The wireless extension has remained backward compatible over
almost 8 years, while tremendously improving and adding new
features. And I believe that moving forward, the price of keeping
backward compatibility is small, as you can see from my patch.
	It's possible. It's not difficult. Breaking backward
compatibility is not a design goal.

> I imagine this is another area where we must agree to disagree.  Linux 
> kernel development has always focused on in-tree drivers.
> 
> Wireless traditionally has had a lot of drivers out-of-tree -- and being 
> out of tree, we see what happens:  vendors are encouraged to mixed 
> binary-only drivers, multiple wireless stacks appears, and confusion 
> reigned.

	Jeff, as you know it, I personally added more wireless driver
to the Linux kernel than any other person, for all those reasons. Look
at how Luis R. Rodriguez blame me to have pushed his driver in the
kernel. I've given you plenty of advices on how to move forward with
the in-kernel 802.11 stack, and some of those advices may even have
been helpful.
	So, what are you blaming me of ?

> It's now time for convergence :)

	Converging wireless into Linux since 1996. Welcome to the club ;-)

> 	Jeff

	Jean

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jean Tourrilhes wrote:
> On Thu, Jun 17, 2004 at 03:02:49PM -0400, Jeff Garzik wrote:
> 
>>Jean Tourrilhes wrote:
>>
>>>On Thu, Jun 17, 2004 at 02:26:20PM -0400, Jeff Garzik wrote:
>>>
>>>
>>>>Note that the above is only a first step.  Through the standard Linux 
>>>>development process -- evolution -- each hook can be pared down to 
>>>>precisely what each call needs.  The above allows for a quick transition 
>>>>of drivers, while keeping them working.
>>>>
>>>>	Jeff
>>>
>>>
>>>	Have you looked at the patch I sent you ? In which way does it
>>>fails to meet your need ?
>>
>>
>>The three major problems I listed in a previous email are still 
>>present...
> 
> 
> 	Are we talking of the same patch ? I'm talking of this patch :
> http://marc.theaimsgroup.com/?l=linux-netdev&m=108749580004668&w=2
> 	I reattached the patch below. It's short enough.

Your patch is half the job -- it allows development of a type-specific 
interface...  but it does nothing to address the problems with the 
underlying type-opaque interface.

The creation of the type-specific interface replaces the type-opaque 
interface, not layers on top of it.

So while this patch may be useful in early development, it does not 
allow the direct exposure of core wireless code to the type-specific 
interfaces, and as such, it can paper over problems that would be 
immediately obviously if the type-specific interface were the only one 
to exist.


>> Also there is a fourth -- WE doesn't work 100% when you have 
>>a 32-bit userland and a 64-bit kernel.
> 
> 
> 	Since when ? What made you change your mind ?
> 	Please check :
> http://marc.theaimsgroup.com/?l=linux-netdev&m=107894322418086&w=2

The general API, yes.  But most driver-private interfaces will fail 
miserably through 32/64-bit translation.

	Jeff

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Thu, Jun 17, 2004 at 03:34:22PM -0400, Jeff Garzik wrote:
> 
> Your patch is half the job -- it allows development of a type-specific 
> interface...

	Which is exactly what you want. Good.

> So while this patch may be useful in early development, it does not 
> allow the direct exposure of core wireless code to the type-specific 
> interfaces

	What is the core wireless code ?

> and as such, it can paper over problems that would be 
> immediately obviously if the type-specific interface were the only one 
> to exist.

	Any new code in the kernel is free to use only the new
API. That's a big enoug incentive to migrate drivers over to the new
API.

> >>Also there is a fourth -- WE doesn't work 100% when you have 
> >>a 32-bit userland and a 64-bit kernel.
> >
> >
> >	Since when ? What made you change your mind ?
> >	Please check :
> >http://marc.theaimsgroup.com/?l=linux-netdev&m=107894322418086&w=2
> 
> The general API, yes.  But most driver-private interfaces will fail 
> miserably through 32/64-bit translation.

	That's fixable, and easy to fix, if needed. You have all the
data you need in the kernel.

> 	Jeff

	Jean

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jean Tourrilhes wrote:
> 	And guess what, I'm helping you in the process. Look back at
> all the e-mail I sent to the various thread on the subject, and you
> will clearly see that I'm constructive and giving suggestion on how to
> do best in this process. I even provide patches.
> 	I don't understand why you are so opposed to my suggestions,
> and what more you expect from me.
[...]
> 	Converging wireless into Linux since 1996. Welcome to the club ;-)

hehe  :)

I'm _not_ blaming you for anything.  You have certainly contributed a 
lot.  I've enjoyed working with you in the past, today, and hopefully 
into the future as well.  I _am_ listening.  But I think we have a 
fundamental disagreement:

I feel strongly (as you see :)) that the type-opaque interface has got 
to go, and that means breaking backwards compatibility in the driver API.

The iw_handler interface breaks rules of C that we shouldn't be 
breaking.  Today's kernel includes boatloads of reference-counted 
kobjects, with strict definitions of lifetime rules.  This has exposed 
endless bugs and caused a lot of pain, but overall it's a good thing to 
clean all that up.  Interfaces that store offsets into driver-local 
structures (iw_handler_def) violate these lifetime rules by simply 
assuming you always have access to the structure of choice.

We want to design driver interfaces that make it tough for the driver 
writer to screw up.  Excluding yourself, myself, and others on this 
list, I think we all know that driver writers can't code their way out 
of a paper bag.  A properly designed interface lets the compiler flag 
incorrect code at the first possible opportunity.  Current WE __does__ 
get the job done (kudos), but it simply doesn't afford that kind of 
protection.

	Jeff

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Jean Tourrilhes wrote:
> On Thu, Jun 17, 2004 at 03:34:22PM -0400, Jeff Garzik wrote:
> 
>>Your patch is half the job -- it allows development of a type-specific 
>>interface...
> 
> 
> 	Which is exactly what you want. Good.

The patch doesn't kill the type-opaque interface, so only 50% of what I 
want.


>>So while this patch may be useful in early development, it does not 
>>allow the direct exposure of core wireless code to the type-specific 
>>interfaces
> 
> 
> 	What is the core wireless code ?

At the moment, "the stuff that calls the driver-local iw_handlers", but 
hopefully more generic wireless core soon with the merge of HostAP.


>>and as such, it can paper over problems that would be 
>>immediately obviously if the type-specific interface were the only one 
>>to exist.
> 
> 
> 	Any new code in the kernel is free to use only the new
> API. That's a big enoug incentive to migrate drivers over to the new
> API.

With the type-opaque interface gone (key design goal), drivers not using 
the new API will not function...


>>>>Also there is a fourth -- WE doesn't work 100% when you have 
>>>>a 32-bit userland and a 64-bit kernel.
>>>
>>>
>>>	Since when ? What made you change your mind ?
>>>	Please check :
>>>http://marc.theaimsgroup.com/?l=linux-netdev&m=107894322418086&w=2
>>
>>The general API, yes.  But most driver-private interfaces will fail 
>>miserably through 32/64-bit translation.
> 
> 
> 	That's fixable, and easy to fix, if needed. You have all the
> data you need in the kernel.

Not really -- it's the same problem as SIOCDEVPRIVATE.  Driver-private 
interfaces by definition change for each driver.  Translation of the 
same ioctl differs on a per-driver basis.  Consider what happens when 
passing pointers from userland, for example...

It was for this reason that we created the MII ioctls, which were 
previously SIOCDEVPRIVATE.

	Jeff

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Thu, Jun 17, 2004 at 04:06:25PM -0400, Jeff Garzik wrote:
> 
> >>The general API, yes.  But most driver-private interfaces will fail 
> >>miserably through 32/64-bit translation.
> >
> >
> >	That's fixable, and easy to fix, if needed. You have all the
> >data you need in the kernel.
> 
> Not really -- it's the same problem as SIOCDEVPRIVATE.  Driver-private 
> interfaces by definition change for each driver.  Translation of the 
> same ioctl differs on a per-driver basis.  Consider what happens when 
> passing pointers from userland, for example...

	I can assure you that I thought about that precise issue when
I re-designed the API in 91. I confirm that all the necessary data in
the kernel to do this properly.
	Proof : the "copy_to/from_user" is done in the generic code,
not in the driver. The driver export to the generic code all the data
needed to know where are those user pointers. What else is needed ?

> It was for this reason that we created the MII ioctls, which were 
> previously SIOCDEVPRIVATE.

	Totally different business. Those ioctls were totally
anonymous, with no meta-data.

> 	Jeff

	Jean

Re: [RFC] Wireless extensions rethink

[Jean Tourrilhes]

On Thu, Jun 17, 2004 at 03:52:39PM -0400, Jeff Garzik wrote:
> 
> I'm _not_ blaming you for anything.  You have certainly contributed a 
> lot.  I've enjoyed working with you in the past, today, and hopefully 
> into the future as well.  I _am_ listening.  But I think we have a 
> fundamental disagreement:
> 
> I feel strongly (as you see :)) that the type-opaque interface has got 
> to go, and that means breaking backwards compatibility in the driver API.

	Jeff, the main disagreement is not about type-opaque versus
strongly-typed interfaces. I gave you a patch to do that, and I told
you you were free to do it the way you want, I would not get in the
way.
	The main disagreement is about backward compatibility. I've
been fighting for 8 years to maintain backward compatibility. I
believe in it, and I not accept gutting it for no good reasons.

> 	Jeff

	Jean

Re: [RFC] Wireless extensions rethink

[Andrew Morton]

Coming into this with my lateness exceeded only by my lack of context, I'd
say that I share Jean's concern over making incompatible changes to the
wireless user<->kernel interface at this time.  If we can retain _both_
interfaces in 2.6, remove the old one in 2.7 then maybe that'll be OK.

But I do wonder whether this API is the uppermost issue with the wireless
drivers.  There seem to be a lot of bug reports, and these drivers are
*really* complex, and there are lots of out-of-tree drivers.  Aren't these
the things which we should be devoting cycles to?

Re: [RFC] Wireless extensions rethink

[Jeff Garzik]

Andrew Morton wrote:
> Coming into this with my lateness exceeded only by my lack of context, I'd
> say that I share Jean's concern over making incompatible changes to the
> wireless user<->kernel interface at this time.  If we can retain _both_
> interfaces in 2.6, remove the old one in 2.7 then maybe that'll be OK.

Two points:
1) This is about the _driver_ API.  The userland interface is a 
different issue.  In Linux the userland ABI is a holy grail that 
shouldn't be broken without warnings across major kernel versions.  We 
can easily add netlink support (as Jean demonstrated) without

2) I won't break the stable kernel driver API, so you worries here are 
unfounded.


> But I do wonder whether this API is the uppermost issue with the wireless
> drivers.

The driver API has got to go.  Period.  Just look at what a sample 
driver exports:


> static const struct iw_handler_def      wavelan_handler_def =
> {
>         .num_standard   = sizeof(wavelan_handler)/sizeof(iw_handler),
>         .num_private    = sizeof(wavelan_private_handler)/sizeof(iw_handler),
>         .num_private_args = sizeof(wavelan_private_args)/sizeof(struct iw_priv_args),
>         .standard       = (iw_handler *) wavelan_handler,
>         .private        = (iw_handler *) wavelan_private_handler,
>         .private_args   = (struct iw_priv_args *) wavelan_private_args,
>         .spy_offset     = ((void *) (&((net_local *) NULL)->spy_data) -
>                            (void *) NULL),
> };


It flat out doesn't work with object lifetime rules, taking _offsets_ in 
driver-local structs into more generic code.

As for the wireless drivers themselves, they will change as the HostAP 
stuff gets integrated more closely into the kernel.


> There seem to be a lot of bug reports, and these drivers are
> *really* complex, and there are lots of out-of-tree drivers.  Aren't these
> the things which we should be devoting cycles to?

The driver API is one of the causes of complexity.  Lack of direct 
integration into the net stack (see 
http://www.kernel.org/pub/linux/kernel/people/jgarzik/patchkits/2.6/davem-p80211.tar.bz2 
for an example direction) another cause of the complexity.

As for out of tree drivers, just look at the web page.  Most either (a) 
have binary-only BLOBs associated or (b) duplicate existing drivers N 
times.  Outside the kernel tree there is no unification, but 3-4 drivers 
for _every_ wireless chipset.

	Jeff