[PATCH] introduce WEXT scan capabilities

[PATCH] introduce WEXT scan capabilities

[Dan Williams]

Introduce scan capabilities to WEXT so that userspace can do intelligent
things with scan behavior, partly in an attempt to handle hidden SSIDs
more gracefully.  If the driver reports a specific scan capability, the
driver must respect the options specified in the iw_scan_req structure
when handling the SIOCSIWSCAN call, unless it's mode or state does not
allow it to do so, in which case it must return an error.

Signed-off-by: Dan Williams <dcbw@redhat.com>

diff --git a/drivers/net/wireless/hostap/hostap_ioctl.c b/drivers/net/wireless/hostap/hostap_ioctl.c
index d8f5efc..3a57d48 100644
--- a/drivers/net/wireless/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/hostap/hostap_ioctl.c
@@ -1089,6 +1089,9 @@ static int prism2_ioctl_giwrange(struct net_device *dev,
 	range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
 		IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;
 
+	if (local->sta_fw_ver >= PRISM2_FW_VER(1,3,1))
+		range->scan_capa = IW_SCAN_CAPA_ESSID;
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index 54f44e5..e30ad24 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -8901,6 +8901,8 @@ static int ipw_wx_get_range(struct net_device *dev,
 	range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
 		IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;
 
+	range->scan_capa = IW_SCAN_CAPA_ESSID | IW_SCAN_CAPA_TYPE;
+
 	IPW_DEBUG_WX("GET Range\n");
 	return 0;
 }
diff --git a/include/linux/wireless.h b/include/linux/wireless.h
index 0987aa7..07e3d01 100644
--- a/include/linux/wireless.h
+++ b/include/linux/wireless.h
@@ -541,6 +541,15 @@
 /* Maximum size of returned data */
 #define IW_SCAN_MAX_DATA	4096	/* In bytes */
 
+/* Scan capabilitiy macros - in (struct iw_range *)->scan_capa */
+#define IW_SCAN_CAPA_ESSID		0x00000001
+#define IW_SCAN_CAPA_BSSID		0x00000002
+#define IW_SCAN_CAPA_CHANNEL	0x00000004
+#define IW_SCAN_CAPA_MODE		0x00000008
+#define IW_SCAN_CAPA_RATE		0x00000010
+#define IW_SCAN_CAPA_TYPE		0x00000020
+#define IW_SCAN_CAPA_DWELL_TIME	0x00000040
+
 /* Max number of char in custom event - use multiple of them if needed */
 #define IW_CUSTOM_MAX		256	/* In bytes */
 
@@ -1040,6 +1049,16 @@ struct	iw_range
 	 * because each entry contain its channel index */
 
 	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
+
+	/* Do *NOT* use those fields, they are just used as padding to get
+	 * proper alignement with user space */
+	__s32		reserved1;
+	__s32		reserved2;
+	__u16		reserved3;
+	__s32		reserved4;
+	__u32		reserved5;
+
+	__u32		scan_capa;	/* IW_SCAN_CAPA_* bit field */
 };
 
 /*
diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index 646e2f2..0c52ed8 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -218,6 +218,8 @@ static int ieee80211_ioctl_giwrange(struct net_device *dev,
 	IW_EVENT_CAPA_SET(range->event_capa, SIOCGIWAP);
 	IW_EVENT_CAPA_SET(range->event_capa, SIOCGIWSCAN);
 
+	range->scan_capa = IW_SCAN_CAPA_ESSID;
+
 	return 0;
 }
 

Re: [PATCH] introduce WEXT scan capabilities

[David Miller]

From: Dan Williams <dcbw@redhat.com>
Date: Fri, 07 Dec 2007 19:22:46 -0500

> @@ -1040,6 +1049,16 @@ struct	iw_range
>  	 * because each entry contain its channel index */
>  
>  	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
> +
> +	/* Do *NOT* use those fields, they are just used as padding to get
> +	 * proper alignement with user space */
> +	__s32		reserved1;
> +	__s32		reserved2;
> +	__u16		reserved3;
> +	__s32		reserved4;
> +	__u32		reserved5;
> +
> +	__u32		scan_capa;	/* IW_SCAN_CAPA_* bit field */
>  };
>  
>  /*

Major NACK.  These datastructure usages are complete wrong, and
we have to stop spreading this problem instead of continuing on
with it as if it's OK.

Re: [PATCH] introduce WEXT scan capabilities

[drago01]

David Miller wrote:
> From: Dan Williams <dcbw@redhat.com>
> Date: Fri, 07 Dec 2007 19:22:46 -0500
>
>   
>> @@ -1040,6 +1049,16 @@ struct	iw_range
>>  	 * because each entry contain its channel index */
>>  
>>  	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
>> +
>> +	/* Do *NOT* use those fields, they are just used as padding to get
>> +	 * proper alignement with user space */
>> +	__s32		reserved1;
>> +	__s32		reserved2;
>> +	__u16		reserved3;
>> +	__s32		reserved4;
>> +	__u32		reserved5;
>> +
>> +	__u32		scan_capa;	/* IW_SCAN_CAPA_* bit field */
>>  };
>>  
>>  /*
>>     
>
> Major NACK.  These datastructure usages are complete wrong, and
> we have to stop spreading this problem instead of continuing on
> with it as if it's OK.
>   
But it seems like we have to deal with this api until nl80211/cfg80211 
is ready. But the situation is _worse_ without this patch (hidden ssid 
support is a huge mess).

Re: [PATCH] introduce WEXT scan capabilities

[Dan Williams]

On Fri, 2007-12-07 at 18:12 -0800, David Miller wrote:
> From: Dan Williams <dcbw@redhat.com>
> Date: Fri, 07 Dec 2007 19:22:46 -0500
> 
> > @@ -1040,6 +1049,16 @@ struct	iw_range
> >  	 * because each entry contain its channel index */
> >  
> >  	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
> > +
> > +	/* Do *NOT* use those fields, they are just used as padding to get
> > +	 * proper alignement with user space */
> > +	__s32		reserved1;
> > +	__s32		reserved2;
> > +	__u16		reserved3;
> > +	__s32		reserved4;
> > +	__u32		reserved5;
> > +
> > +	__u32		scan_capa;	/* IW_SCAN_CAPA_* bit field */
> >  };
> >  
> >  /*
> 
> Major NACK.  These datastructure usages are complete wrong, and
> we have to stop spreading this problem instead of continuing on
> with it as if it's OK.

There's not too much we can do here.  We need a better way to support
driver/card capabilities in WEXT right _now_, in parallel with
cfg80211/nl80211.  The other alternative here is to have a 64-bit
generic capabilities field-to-end-all-fields and add more bitfield
position constants to that without extending the structure any more.

Is there a better way you'd propose to do this _in_WEXT_?

I don't really forsee any more extending of this structure, since I
think scan capabilities are the last thing we really need to know about.

Dan

Re: [PATCH] introduce WEXT scan capabilities

[Dave]

Dan Williams wrote:
> On Fri, 2007-12-07 at 18:12 -0800, David Miller wrote:
>> From: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Date: Fri, 07 Dec 2007 19:22:46 -0500
>>
>>> @@ -1040,6 +1049,16 @@ struct	iw_range
>>>  	 * because each entry contain its channel index */
>>>  
>>>  	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
>>> +
>>> +	/* Do *NOT* use those fields, they are just used as padding to get
>>> +	 * proper alignement with user space */
>>> +	__s32		reserved1;
>>> +	__s32		reserved2;
>>> +	__u16		reserved3;
>>> +	__s32		reserved4;
>>> +	__u32		reserved5;
>>> +
>>> +	__u32		scan_capa;	/* IW_SCAN_CAPA_* bit field */
>>>  };
>>>  
>>>  /*
>> Major NACK.  These datastructure usages are complete wrong, and
>> we have to stop spreading this problem instead of continuing on
>> with it as if it's OK.
> 
> There's not too much we can do here.  We need a better way to support
> driver/card capabilities in WEXT right _now_, in parallel with
> cfg80211/nl80211.  The other alternative here is to have a 64-bit
> generic capabilities field-to-end-all-fields and add more bitfield
> position constants to that without extending the structure any more.
> 
> Is there a better way you'd propose to do this _in_WEXT_?

Since iw_range is not packed, there are a few locations where there is some padding. You could quite easily shoehorn an 8 bit bitmask into the existing structure without impacting backward compatibility (unless userspace is using the padding for something). For example:

--- a/include/linux/wireless.h
+++ b/include/linux/wireless.h
@@ -1035,6 +1035,7 @@ struct    iw_range
        /* Frequency */
        __u16           num_channels;   /* Number of channels [0; num - 1] */
        __u8            num_frequency;  /* Number of entry in the list */
+       __u8            scan_capa;      /* scan capabilities */
        struct iw_freq  freq[IW_MAX_FREQUENCIES];       /* list */
        /* Note : this frequency list doesn't need to fit channel numbers,
         * because each entry contain its channel index */

Other candidate blocks are Old Frequency, Rates, Encoder stuff, Transmit power.

Dave.

> I don't really forsee any more extending of this structure, since I
> think scan capabilities are the last thing we really need to know about.

Re: [PATCH] introduce WEXT scan capabilities

[Dan Williams]

On Sun, 2007-12-09 at 18:34 +0000, Dave wrote:
> 
> Dan Williams wrote:
> > On Fri, 2007-12-07 at 18:12 -0800, David Miller wrote:
> >> From: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> Date: Fri, 07 Dec 2007 19:22:46 -0500
> >>
> >>> @@ -1040,6 +1049,16 @@ struct	iw_range
> >>>  	 * because each entry contain its channel index */
> >>>  
> >>>  	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
> >>> +
> >>> +	/* Do *NOT* use those fields, they are just used as padding to get
> >>> +	 * proper alignement with user space */
> >>> +	__s32		reserved1;
> >>> +	__s32		reserved2;
> >>> +	__u16		reserved3;
> >>> +	__s32		reserved4;
> >>> +	__u32		reserved5;
> >>> +
> >>> +	__u32		scan_capa;	/* IW_SCAN_CAPA_* bit field */
> >>>  };
> >>>  
> >>>  /*
> >> Major NACK.  These datastructure usages are complete wrong, and
> >> we have to stop spreading this problem instead of continuing on
> >> with it as if it's OK.
> > 
> > There's not too much we can do here.  We need a better way to support
> > driver/card capabilities in WEXT right _now_, in parallel with
> > cfg80211/nl80211.  The other alternative here is to have a 64-bit
> > generic capabilities field-to-end-all-fields and add more bitfield
> > position constants to that without extending the structure any more.
> > 
> > Is there a better way you'd propose to do this _in_WEXT_?
> 
> Since iw_range is not packed, there are a few locations where there is some padding. You could quite easily shoehorn an 8 bit bitmask into the existing structure without impacting backward compatibility (unless userspace is using the padding for something). For example:
> 
> --- a/include/linux/wireless.h
> +++ b/include/linux/wireless.h
> @@ -1035,6 +1035,7 @@ struct    iw_range
>         /* Frequency */
>         __u16           num_channels;   /* Number of channels [0; num - 1] */
>         __u8            num_frequency;  /* Number of entry in the list */
> +       __u8            scan_capa;      /* scan capabilities */
>         struct iw_freq  freq[IW_MAX_FREQUENCIES];       /* list */
>         /* Note : this frequency list doesn't need to fit channel numbers,
>          * because each entry contain its channel index */
> 
> Other candidate blocks are Old Frequency, Rates, Encoder stuff, Transmit power.

Hmm; this could work as long as that part of the structure is guaranteed
to be 0 if it wasn't touched by the driver.  If it could be filled with
garbage bits at any point, then it's not going to work.  Interesting
thought.

Dan

Re: [PATCH] introduce WEXT scan capabilities

[Johannes Berg]

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

[what happened here with all the gmane addresses??]

> > Since iw_range is not packed, there are a few locations where there
> is some padding. You could quite easily shoehorn an 8 bit bitmask into
> the existing structure without impacting backward compatibility
> (unless userspace is using the padding for something). For example:

Ouch. But possible since we cannot do this.

> Hmm; this could work as long as that part of the structure is guaranteed
> to be 0 if it wasn't touched by the driver.  If it could be filled with
> garbage bits at any point, then it's not going to work.  Interesting
> thought.

That depends how the driver allocates it. Some drivers leak stack data
in these fields which is bad per se, and it's something we need to fix
fix in the kernel anyway.

johannes

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

Re: [PATCH] introduce WEXT scan capabilities

[Johannes Berg]

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


On Mon, 2007-12-10 at 13:23 +0100, Johannes Berg wrote:
> [what happened here with all the gmane addresses??]
> 
> > > Since iw_range is not packed, there are a few locations where there
> > is some padding. You could quite easily shoehorn an 8 bit bitmask into
> > the existing structure without impacting backward compatibility
> > (unless userspace is using the padding for something). For example:
> 
> Ouch. But possible since we cannot do this.

Eh. I mean. Possible since we cannot expand the structure.

johannes

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

Re: [PATCH] introduce WEXT scan capabilities

[Jean Tourrilhes]

On Sun, Dec 09, 2007 at 01:35:00PM -0500, Dan Williams wrote:
> 
> 
> 
> On Sun, 2007-12-09 at 18:34 +0000, Dave wrote:
> > 
> > Dan Williams wrote:
> > > On Fri, 2007-12-07 at 18:12 -0800, David Miller wrote:
> > >> From: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > >> Date: Fri, 07 Dec 2007 19:22:46 -0500
> > >>
> > >>> @@ -1040,6 +1049,16 @@ struct	iw_range
> > >>>  	 * because each entry contain its channel index */
> > >>>  
> > >>>  	__u32		enc_capa;	/* IW_ENC_CAPA_* bit field */
> > >>> +
> > >>> +	/* Do *NOT* use those fields, they are just used as padding to get
> > >>> +	 * proper alignement with user space */
> > >>> +	__s32		reserved1;
> > >>> +	__s32		reserved2;
> > >>> +	__u16		reserved3;
> > >>> +	__s32		reserved4;
> > >>> +	__u32		reserved5;
> > >>> +
> > >>> +	__u32		scan_capa;	/* IW_SCAN_CAPA_* bit field */
> > >>>  };
> > >>>  
> > >>>  /*
> > >> Major NACK.  These datastructure usages are complete wrong, and
> > >> we have to stop spreading this problem instead of continuing on
> > >> with it as if it's OK.
> > > 
> > > There's not too much we can do here.  We need a better way to support
> > > driver/card capabilities in WEXT right _now_, in parallel with
> > > cfg80211/nl80211.  The other alternative here is to have a 64-bit
> > > generic capabilities field-to-end-all-fields and add more bitfield
> > > position constants to that without extending the structure any more.
> > > 
> > > Is there a better way you'd propose to do this _in_WEXT_?
> > 
> > Since iw_range is not packed, there are a few locations where there is some padding. You could quite easily shoehorn an 8 bit bitmask into the existing structure without impacting backward compatibility (unless userspace is using the padding for something). For example:
> > 
> > --- a/include/linux/wireless.h
> > +++ b/include/linux/wireless.h
> > @@ -1035,6 +1035,7 @@ struct    iw_range
> >         /* Frequency */
> >         __u16           num_channels;   /* Number of channels [0; num - 1] */
> >         __u8            num_frequency;  /* Number of entry in the list */
> > +       __u8            scan_capa;      /* scan capabilities */
> >         struct iw_freq  freq[IW_MAX_FREQUENCIES];       /* list */
> >         /* Note : this frequency list doesn't need to fit channel numbers,
> >          * because each entry contain its channel index */
> > 
> > Other candidate blocks are Old Frequency, Rates, Encoder stuff, Transmit power.
> 
> Hmm; this could work as long as that part of the structure is guaranteed
> to be 0 if it wasn't touched by the driver.  If it could be filled with
> garbage bits at any point, then it's not going to work.  Interesting
> thought.

	You can count on zero being there in almost every case, for
this precise reason. The first thing a driver is supposed to do with
iwrange is :
----------------------------------------
	memset(range, 0, sizeof(struct iw_range));
----------------------------------------
	From what I remember, all drivers are doing it. If a driver
does not do it, it should be fixed ASAP as other things would break
(most driver only fill a few field of the struct and don't touch
others).

> Dan

	Regards,

	Jean

P.S. : What's up with all the bogus e-mail addresses in cc ?