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 <[email protected]>
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;
}
David Miller wrote:
> From: Dan Williams <[email protected]>
> 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).
On Fri, 2007-12-07 at 18:12 -0800, David Miller wrote:
> From: Dan Williams <[email protected]>
> 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
Dan Williams wrote:
> On Fri, 2007-12-07 at 18:12 -0800, David Miller wrote:
>> From: Dan Williams <[email protected]>
>> 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.
[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
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
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 <[email protected]>
> >> 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
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 <[email protected]>
> > >> 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 ?
From: Dan Williams <[email protected]>
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.