LinuxLists.cc - [PATCH RFC] mac80211: Filter scan results

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, 2009-02-05 at 14:07 +0200, Jouni Malinen wrote:

> (a) Currently not, but once people realize that the length field has a
> maximum value of 65535, using it as the largest attempt sounds like the
> most logical choice. In other words, yes, this will require a small
> change in programs that care (wpa_supplicant, which I already changed;
> iwlist; etc.), but the change is minimal and not doing this does not break
> anything new.

True.

> (b) Yes, user space apps better be prepared to read the results even if
> the last entry is truncated (can happen with current kernel). There is
> no clear way for the app to know that there could be more scan results
> (well, it could guess that this is the case if the returned buffer is
> very close to 64 kB), but it is not like scanning is that reliable
> operation anyway, so applications better be prepared to not always
> find every BSS in the results.

Yeah, the truncation is a little worrying. I doubt any program but
wpa_supplicant is prepared to handle that. Should we completely remove
the entry, and add some new/custom piece to stream that indicates that
we had to truncate here?

johannes

Attachments:

signature.asc (836.00 B)
This is a digitally signed message part

2009-02-04 23:18:21

by Marcel Holtmann

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

Hi Johannes,

> > In very dense environment, the scan result buffer can get really large, mostly
> > due to the addition of proprietary IEs. iwlist fails, typically warning about:
> > "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> > reallocating the scan result buffer several times, up to 132 Kbytes:
> > [snip]
> > Scan results did not fit - trying larger buffer (131072 bytes)
> > ioctl[SIOCGIWSCAN]: Argument list too long
> >
> > By adding a mac80211 module parameter, we can filter the scan results and keep
> > only the ones userspace currently worries about, i.e. WPA1, WPA2, WMM and WPS.
> >
> > To activate this feature, 1 has to be written to
> > /sys/module/mac80211/parameters/ieee80211_scan_ie_filter
>
> Module parameter, hard-coded filter, just to work around wext crap. Eww.
> NACK.

we are perfectly aware that this is ugly and trust me, that nobody of us
wants to do it. However we do have these setups and they happen in real
life. I have been in a couple of locations where I ran into this problem
and in that cases there is no way for me to establish any kind of WiFi
connection at all, because the buffers are just too small. And that is
with a laptop. Then just think about a mobile phone running mac80211
where the memory is limited.

So WEXT is crap and everybody agrees, but until we can use cfg80211 for
scanning, we need something to make this work. So if this patch is not
acceptable, then do we get scanning support via cfg80211 for 2.6.30? You
were always marking your patches as development/RFC.

> Go for
> http://johannes.sipsolutions.net/patches/kernel/all/LATEST/028-cfg80211-scan.patch

Return a 404 btw.

Regards

Marcel

2009-02-05 22:43:10

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, 2009-02-05 at 14:26 +0200, Jouni Malinen wrote:

> It would be somewhat cleaner to truncate between BSSes instead of
> between iw_events, but it would be some more code (remember the length
> before starting to add new BSS and use it if truncations was detected).

Doesn't seem that hard? Want to post the patch for cfg80211?

> I don't think I would bother with custom iw_event stating that
> truncation happened since I do not see much use for it in
> wpa_supplicant. It could be of some use for someone running iwlist scan
> manually.

Yeah, true, no use since you can't get at the other results anyway...
Just use nl80211 :)

johannes

Attachments:

signature.asc (836.00 B)
This is a digitally signed message part

2009-02-04 23:36:24

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Wed, 2009-02-04 at 18:23 -0500, Dan Williams wrote:
> On Wed, 2009-02-04 at 16:08 +0100, Samuel Ortiz wrote:
> > From: Samuel Ortiz <[email protected]>
> >
> > In very dense environment, the scan result buffer can get really large, mostly
> > due to the addition of proprietary IEs. iwlist fails, typically warning about:
> > "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> > reallocating the scan result buffer several times, up to 132 Kbytes:
> > [snip]
> > Scan results did not fit - trying larger buffer (131072 bytes)
> > ioctl[SIOCGIWSCAN]: Argument list too long
> >
> > By adding a mac80211 module parameter, we can filter the scan results and keep
> > only the ones userspace currently worries about, i.e. WPA1, WPA2, WMM and WPS.
>
> A somewhat more acceptable, but still unacceptable hack would be to
> figure out when the returned scan results buffer would be too large, and
> automatically filter the IEs when it would be to large.

Nope, that doesn't even work. You'd filter even before the app decided
to no longer reallocate the scan buffer.

johannes

Attachments:

signature.asc (836.00 B)
This is a digitally signed message part

2009-02-05 12:27:57

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, Feb 05, 2009 at 01:10:54PM +0100, Johannes Berg wrote:

> Yeah, the truncation is a little worrying. I doubt any program but
> wpa_supplicant is prepared to handle that. Should we completely remove
> the entry, and add some new/custom piece to stream that indicates that
> we had to truncate here?

Well, the truncation is partly an artifact of the way iwe_stream_*
helpers work and I would expect all drivers to be doing this already
(should they support large enough number of BSSes to hit the limit). If
I have understood correctly, truncation happens between events, i.e., no
event (struct iw_event) itself is truncated; some events are just
missing from a BSS. As such, I would expect more or less all application
to handle that one way or another. They may assume the network does not
support WPA etc. based on some iw_events missing, but that should be
about it.

It would be somewhat cleaner to truncate between BSSes instead of
between iw_events, but it would be some more code (remember the length
before starting to add new BSS and use it if truncations was detected).
I don't think I would bother with custom iw_event stating that
truncation happened since I do not see much use for it in
wpa_supplicant. It could be of some use for someone running iwlist scan
manually.

--
Jouni Malinen PGP id EFC895FA

2009-02-05 14:59:22

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, 2009-02-05 at 15:00 +0200, Tomas Winkler wrote:
> On Thu, Feb 5, 2009 at 2:12 PM, Jouni Malinen <[email protected]> wrote:
> > On Thu, Feb 05, 2009 at 01:44:15PM +0200, Tomas Winkler wrote:
> >
> >> Just a thought If user space will be able to merge 2 scan results we
> >> can then split then in 2 separate reports....
> >
> > Theoretically speaking, yes, WEXT could be extended to do this with some
> > kind of scan parameter that indicates which 64 kB block is requested.
> > However, I would not suggest going there and I would probably object to
> > a patch that tries to do this either in the kernel or in wpa_supplicant
> > for that matter.
>
> I thin it can be done w/o extension of wext, j ust the eviction of
> old scan results is done
> in user space.IRCC the time stamp of beacon/probe is part of each
> entry. And user space state machine should be able to react on
> nl[SIOCGIWSCAN] w/o issuing ioctl[SIOCSIWSCAN]:

I still don't like this. If 177 BSSes is not enough, then get the
cfg80211 stuff working better for you. I can't think of how this scheme
would work; which half of the results get returned when something calls
SIOCGIWSCAN? Would the driver/stack internally flip to the other half
of the results on each call of SIOCGIWSCAN? That doesn't work, because
then two userspace processes requesting scan results will race with each
other for them. We had this problem before when drivers cleared the
scan list each time SIOCGIWSCAN was called.

Dan

> > If you really end up hitting a case that does not work reasonably well
> > with 64 kB fix in user space, I would suggest looking into the
> > cfg80211/nl80211 scan stuff. The only additional thing it needs in such
> > an environment is to get better optimized data structure (for which,
> > there is already a proposed implementation, too).
>
> Guess we need to proceed with cfg anyway
>
> Tomas
>
> > --
> > Jouni Malinen PGP id EFC895FA
> >

2009-02-04 23:32:29

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

> > Module parameter, hard-coded filter, just to work around wext crap. Eww.
> > NACK.
>
> we are perfectly aware that this is ugly and trust me, that nobody of us
> wants to do it. However we do have these setups and they happen in real
> life. I have been in a couple of locations where I ran into this problem
> and in that cases there is no way for me to establish any kind of WiFi
> connection at all, because the buffers are just too small. And that is
> with a laptop. Then just think about a mobile phone running mac80211
> where the memory is limited.

That's a straw-man. Your applications are _artificially_ limiting the
wext buffer size, to something well below your mobile phone's memory.

> So WEXT is crap and everybody agrees, but until we can use cfg80211 for
> scanning, we need something to make this work. So if this patch is not
> acceptable, then do we get scanning support via cfg80211 for 2.6.30? You
> were always marking your patches as development/RFC.

This patch is _certainly_ not acceptable. You know that just as well as
I do. And of course you can get scanning with cfg80211/nl80211 in
2.6.30. You just have to help make it work. The patch below has one
major flaw: it keeps all BSSes in a doubly-linked list. That gives
horrible lookup behaviour. So I set about to fix it:

http://johannes.sipsolutions.net/patches/kernel/all/LATEST/029-b%2btree.patch
http://johannes.sipsolutions.net/patches/kernel/all/LATEST/030-cfg80211-scan-btree.patch

but the b+tree was pretty much rejected, and I don't currently have time
to spend on investigating other types of trees (rbtrees, ...) or hash
tables. Without that, it's going to suck, especially on the embedded
devices you mention.

> > Go for
> > http://johannes.sipsolutions.net/patches/kernel/all/LATEST/028-cfg80211-scan.patch
>
> Return a 404 btw.

Yeah, so I rebased and it's now at
http://johannes.sipsolutions.net/patches/kernel/all/LATEST/024-cfg80211-scan.patch

johannes

Attachments:

signature.asc (836.00 B)
This is a digitally signed message part

2009-02-05 09:56:28

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, Feb 05, 2009 at 10:44:07AM +0200, Jouni Malinen wrote:
> Trying to use 131072 byte buffer is actually a bug in wpa_supplicant and
> even the previous attempt of 65536 is not valid. I did not realize
> this before, but it was pointed out to me some time ago that the length
> field in struct iw_point is only 16 bits, so there is actually a strict
> limit of 65535 bytes of data available through SIOCGIWSCAN.
>
> 65536 ends up being truncated into zero, so I would assume the largest
> buffer wpa_supplicant tries to use in practice is 32768 bytes. I'll fix
> this so that the next doubling is actually limited to 65535 (assuming it
> works with WEXT code in kernel). I would hope that this helps quite a
> bit as far as wpa_supplicant is concerned (almost doubling the maximum
> results buffers).

This moved the maximum from 88 BSSes (using IEEE 802.11g with WPA2 as an
example here; smaller number for 802.11n etc. extensions) to 177 BSSes
(or 178, but the last one was truncated, I think; the returned buffer
was 65534 bytes long). After that, things get bad, though, since there
is no way for the user space to fetch the buffer (at least with mac80211
scan reporting).

> As far as a possible workaround in kernel code is concerned,
> length=65535 could be considered a special case. Since it is clear that
> userspace cannot provide a larger buffer, there is not much point in
> returning -E2BIG. Instead, the results could be truncated after the last
> BSS that fit into the buffer and that would be returned to userspace.
> This is likely going to be much more useful than not being able to
> receive any result.

This should have helped with the test above when the number of BSSes
went past 178. Sure, all BSSes would not be reported, but should the
driver do any sorting based on BSS preference (e.g., signal strength),
this would likely work for most use cases.

--
Jouni Malinen PGP id EFC895FA

2009-02-05 11:56:01

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, 2009-02-05 at 13:10 +0200, Jouni Malinen wrote:
> On Thu, Feb 05, 2009 at 10:44:07AM +0200, Jouni Malinen wrote:
> > As far as a possible workaround in kernel code is concerned,
> > length=65535 could be considered a special case. Since it is clear that
> > userspace cannot provide a larger buffer, there is not much point in
> > returning -E2BIG. Instead, the results could be truncated after the last
> > BSS that fit into the buffer and that would be returned to userspace.
> > This is likely going to be much more useful than not being able to
> > receive any result.
>
> And here's an example patch on doing that for mac80211 (drivers that do
> not use mac80211 would need to do something similar if they want to
> support this mechanism). Would this change be acceptable for mac80211?

Seems ok to me, since wext limits us to this anyway... But can we rely
on userspace to (a) use the exactly largest buffer size and (b) be able
to interpret that?

johannes

> ---
> net/mac80211/scan.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> --- wireless-testing.orig/net/mac80211/scan.c 2009-02-05 11:33:37.000000000 +0200
> +++ wireless-testing/net/mac80211/scan.c 2009-02-05 13:07:41.000000000 +0200
> @@ -934,6 +934,19 @@ int ieee80211_scan_results(struct ieee80
> list_for_each_entry(bss, &local->bss_list, list) {
> if (buf + len - current_ev <= IW_EV_ADDR_LEN) {
> spin_unlock_bh(&local->bss_lock);
> + if (len == 65535) {
> + /*
> + * WEXT uses 16-bit field for the length which
> + * means that the largest buffer size that can
> + * be used here is 65535 bytes. There is not
> + * much help in returning -E2BIG if the user
> + * space program cannot use a larger buffer. As
> + * a workaround, return as many BSSes as
> + * possible instead of making it impossible for
> + * the application to get any results.
> + */
> + return current_ev - buf;
> + }
> return -E2BIG;
> }
> current_ev = ieee80211_scan_result(local, info, bss,
>

Attachments:

signature.asc (836.00 B)
This is a digitally signed message part

2009-02-04 23:19:50

by Marcel Holtmann

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

Hi Dan,

> > In very dense environment, the scan result buffer can get really large, mostly
> > due to the addition of proprietary IEs. iwlist fails, typically warning about:
> > "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> > reallocating the scan result buffer several times, up to 132 Kbytes:
> > [snip]
> > Scan results did not fit - trying larger buffer (131072 bytes)
> > ioctl[SIOCGIWSCAN]: Argument list too long
> >
> > By adding a mac80211 module parameter, we can filter the scan results and keep
> > only the ones userspace currently worries about, i.e. WPA1, WPA2, WMM and WPS.
> >
> > To activate this feature, 1 has to be written to
> > /sys/module/mac80211/parameters/ieee80211_scan_ie_filter
>
> NAK. Use cfg80211. Module parameters are the wrong way to work around
> API limitations; the API gets fixed or replaced instead. Furthermore,
> the stack/drivers should *NOT* be filtering scan results at all. That's
> the job of the userspace program requesting the results; otherwise
> passive listeners will get only filtered results, and not the complete
> scan list.

yes, yes and yes, but reality is a little bit different here. You are
not able to get the results out of the kernel because WEXT is just
braindead.

Regards

Marcel

2009-02-04 23:23:52

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, 2009-02-05 at 00:19 +0100, Marcel Holtmann wrote:
> Hi Dan,
>
> > > In very dense environment, the scan result buffer can get really large, mostly
> > > due to the addition of proprietary IEs. iwlist fails, typically warning about:
> > > "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> > > reallocating the scan result buffer several times, up to 132 Kbytes:
> > > [snip]
> > > Scan results did not fit - trying larger buffer (131072 bytes)
> > > ioctl[SIOCGIWSCAN]: Argument list too long
> > >
> > > By adding a mac80211 module parameter, we can filter the scan results and keep
> > > only the ones userspace currently worries about, i.e. WPA1, WPA2, WMM and WPS.
> > >
> > > To activate this feature, 1 has to be written to
> > > /sys/module/mac80211/parameters/ieee80211_scan_ie_filter
> >
> > NAK. Use cfg80211. Module parameters are the wrong way to work around
> > API limitations; the API gets fixed or replaced instead. Furthermore,
> > the stack/drivers should *NOT* be filtering scan results at all. That's
> > the job of the userspace program requesting the results; otherwise
> > passive listeners will get only filtered results, and not the complete
> > scan list.
>
> yes, yes and yes, but reality is a little bit different here. You are
> not able to get the results out of the kernel because WEXT is just
> braindead.

This hack is waaay beyond the realm of acceptable. Fix cfg80211, and
backport it to the kernels you care about since you'd be doing the same
backporting for this hack.

Dan

2009-02-05 11:14:16

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, Feb 05, 2009 at 10:44:07AM +0200, Jouni Malinen wrote:
> As far as a possible workaround in kernel code is concerned,
> length=65535 could be considered a special case. Since it is clear that
> userspace cannot provide a larger buffer, there is not much point in
> returning -E2BIG. Instead, the results could be truncated after the last
> BSS that fit into the buffer and that would be returned to userspace.
> This is likely going to be much more useful than not being able to
> receive any result.

And here's an example patch on doing that for mac80211 (drivers that do
not use mac80211 would need to do something similar if they want to
support this mechanism). Would this change be acceptable for mac80211?

---
net/mac80211/scan.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

--- wireless-testing.orig/net/mac80211/scan.c 2009-02-05 11:33:37.000000000 +0200
+++ wireless-testing/net/mac80211/scan.c 2009-02-05 13:07:41.000000000 +0200
@@ -934,6 +934,19 @@ int ieee80211_scan_results(struct ieee80
list_for_each_entry(bss, &local->bss_list, list) {
if (buf + len - current_ev <= IW_EV_ADDR_LEN) {
spin_unlock_bh(&local->bss_lock);
+ if (len == 65535) {
+ /*
+ * WEXT uses 16-bit field for the length which
+ * means that the largest buffer size that can
+ * be used here is 65535 bytes. There is not
+ * much help in returning -E2BIG if the user
+ * space program cannot use a larger buffer. As
+ * a workaround, return as many BSSes as
+ * possible instead of making it impossible for
+ * the application to get any results.
+ */
+ return current_ev - buf;
+ }
return -E2BIG;
}
current_ev = ieee80211_scan_result(local, info, bss,

--
Jouni Malinen PGP id EFC895FA

2009-02-05 12:15:29

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, Feb 05, 2009 at 01:44:15PM +0200, Tomas Winkler wrote:

> Just a thought If user space will be able to merge 2 scan results we
> can then split then in 2 separate reports....

Theoretically speaking, yes, WEXT could be extended to do this with some
kind of scan parameter that indicates which 64 kB block is requested.
However, I would not suggest going there and I would probably object to
a patch that tries to do this either in the kernel or in wpa_supplicant
for that matter.

If you really end up hitting a case that does not work reasonably well
with 64 kB fix in user space, I would suggest looking into the
cfg80211/nl80211 scan stuff. The only additional thing it needs in such
an environment is to get better optimized data structure (for which,
there is already a proposed implementation, too).

--
Jouni Malinen PGP id EFC895FA

2009-02-05 11:44:17

by Tomas Winkler

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, Feb 5, 2009 at 11:52 AM, Jouni Malinen <[email protected]> wrote:
> On Thu, Feb 05, 2009 at 10:44:07AM +0200, Jouni Malinen wrote:
>> Trying to use 131072 byte buffer is actually a bug in wpa_supplicant and
>> even the previous attempt of 65536 is not valid. I did not realize
>> this before, but it was pointed out to me some time ago that the length
>> field in struct iw_point is only 16 bits, so there is actually a strict
>> limit of 65535 bytes of data available through SIOCGIWSCAN.
>>
>> 65536 ends up being truncated into zero, so I would assume the largest
>> buffer wpa_supplicant tries to use in practice is 32768 bytes. I'll fix
>> this so that the next doubling is actually limited to 65535 (assuming it
>> works with WEXT code in kernel). I would hope that this helps quite a
>> bit as far as wpa_supplicant is concerned (almost doubling the maximum
>> results buffers).
>
> This moved the maximum from 88 BSSes (using IEEE 802.11g with WPA2 as an
> example here; smaller number for 802.11n etc. extensions) to 177 BSSes
> (or 178, but the last one was truncated, I think; the returned buffer
> was 65534 bytes long). After that, things get bad, though, since there
> is no way for the user space to fetch the buffer (at least with mac80211
> scan reporting).
>
>> As far as a possible workaround in kernel code is concerned,
>> length=65535 could be considered a special case. Since it is clear that
>> userspace cannot provide a larger buffer, there is not much point in
>> returning -E2BIG. Instead, the results could be truncated after the last
>> BSS that fit into the buffer and that would be returned to userspace.
>> This is likely going to be much more useful than not being able to
>> receive any result.
>
> This should have helped with the test above when the number of BSSes
> went past 178. Sure, all BSSes would not be reported, but should the
> driver do any sorting based on BSS preference (e.g., signal strength),
> this would likely work for most use cases.

Just a thought If user space will be able to merge 2 scan results we
can then split then in 2 separate reports....

Tomas

2009-02-05 08:45:11

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, Feb 05, 2009 at 12:35:39AM +0100, Johannes Berg wrote:
> On Wed, 2009-02-04 at 18:23 -0500, Dan Williams wrote:
> > On Wed, 2009-02-04 at 16:08 +0100, Samuel Ortiz wrote:
> > > "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> > > reallocating the scan result buffer several times, up to 132 Kbytes:
> > > [snip]
> > > Scan results did not fit - trying larger buffer (131072 bytes)
> > > ioctl[SIOCGIWSCAN]: Argument list too long

> > A somewhat more acceptable, but still unacceptable hack would be to
> > figure out when the returned scan results buffer would be too large, and
> > automatically filter the IEs when it would be to large.
>
> Nope, that doesn't even work. You'd filter even before the app decided
> to no longer reallocate the scan buffer.

There are number of issues here and I would agree that the proposed
patch is not the way to go. However, I think that there could be
something here that could be done to help this situation with WEXT
(while agreeing that the cfg80211/nl80211 scan is going to be the proper
fix for the issue).

Trying to use 131072 byte buffer is actually a bug in wpa_supplicant and
even the previous attempt of 65536 is not valid. I did not realize
this before, but it was pointed out to me some time ago that the length
field in struct iw_point is only 16 bits, so there is actually a strict
limit of 65535 bytes of data available through SIOCGIWSCAN.

65536 ends up being truncated into zero, so I would assume the largest
buffer wpa_supplicant tries to use in practice is 32768 bytes. I'll fix
this so that the next doubling is actually limited to 65535 (assuming it
works with WEXT code in kernel). I would hope that this helps quite a
bit as far as wpa_supplicant is concerned (almost doubling the maximum
results buffers).

As far as a possible workaround in kernel code is concerned,
length=65535 could be considered a special case. Since it is clear that
userspace cannot provide a larger buffer, there is not much point in
returning -E2BIG. Instead, the results could be truncated after the last
BSS that fit into the buffer and that would be returned to userspace.
This is likely going to be much more useful than not being able to
receive any result.

--
Jouni Malinen PGP id EFC895FA

2009-02-04 23:25:26

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Wed, 2009-02-04 at 16:08 +0100, Samuel Ortiz wrote:
> From: Samuel Ortiz <[email protected]>
>
> In very dense environment, the scan result buffer can get really large, mostly
> due to the addition of proprietary IEs. iwlist fails, typically warning about:
> "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> reallocating the scan result buffer several times, up to 132 Kbytes:
> [snip]
> Scan results did not fit - trying larger buffer (131072 bytes)
> ioctl[SIOCGIWSCAN]: Argument list too long
>
> By adding a mac80211 module parameter, we can filter the scan results and keep
> only the ones userspace currently worries about, i.e. WPA1, WPA2, WMM and WPS.

A somewhat more acceptable, but still unacceptable hack would be to
figure out when the returned scan results buffer would be too large, and
automatically filter the IEs when it would be to large.

The module parameter is the largest dealbreaker here.

Dan

> To activate this feature, 1 has to be written to
> /sys/module/mac80211/parameters/ieee80211_scan_ie_filter
>
> Signed-off-by: Samuel Ortiz <[email protected]>
> ---
> net/mac80211/ieee80211_i.h | 1
> net/mac80211/scan.c | 65 ++++++++++++++++++++++++++++++---------------
> net/mac80211/util.c | 40 +++++++++++++++++++++++++++
> 3 files changed, 85 insertions(+), 21 deletions(-)
>
> Index: wireless-testing/net/mac80211/scan.c
> ===================================================================
> --- wireless-testing.orig/net/mac80211/scan.c 2009-01-28 11:37:55.000000000 +0100
> +++ wireless-testing/net/mac80211/scan.c 2009-01-28 17:32:48.000000000 +0100
> @@ -31,6 +31,11 @@
> #define IEEE80211_CHANNEL_TIME (HZ / 33)
> #define IEEE80211_PASSIVE_CHANNEL_TIME (HZ / 5)
>
> +static int ieee80211_scan_ie_filter;
> +module_param(ieee80211_scan_ie_filter, bool, 0644);
> +MODULE_PARM_DESC(ieee80211_scan_ie_filter,
> + "Filter IEs from scan results");
> +
> void ieee80211_rx_bss_list_init(struct ieee80211_local *local)
> {
> spin_lock_init(&local->bss_lock);
> @@ -725,33 +730,51 @@ static void ieee80211_scan_add_ies(struc
> if (bss == NULL || bss->ies == NULL)
> return;
>
> - /*
> - * If needed, fragment the IEs buffer (at IE boundaries) into short
> - * enough fragments to fit into IW_GENERIC_IE_MAX octet messages.
> - */
> pos = bss->ies;
> end = pos + bss->ies_len;
>
> - while (end - pos > IW_GENERIC_IE_MAX) {
> - next = pos + 2 + pos[1];
> - while (next + 2 + next[1] - pos < IW_GENERIC_IE_MAX)
> - next = next + 2 + next[1];
> + if (ieee80211_scan_ie_filter) {
> + while (pos - end) {
> + next = pos + 2 + pos[1];
> + if (ieee80211_filter_ie(pos)) {
> + memset(&iwe, 0, sizeof(iwe));
> + iwe.cmd = IWEVGENIE;
> + iwe.u.data.length = next - pos;
> + *current_ev =
> + iwe_stream_add_point(info, *current_ev,
> + end_buf, &iwe,
> + pos);
> + }
> + pos = next;
> + }
> + } else {
> + /*
> + * If needed, fragment the IEs buffer (at IE boundaries) into
> + * short enough fragments to fit into IW_GENERIC_IE_MAX octet
> + * messages.
> + */
> +
> + while (end - pos > IW_GENERIC_IE_MAX) {
> + next = pos + 2 + pos[1];
> + while (next + 2 + next[1] - pos < IW_GENERIC_IE_MAX)
> + next = next + 2 + next[1];
>
> - memset(&iwe, 0, sizeof(iwe));
> - iwe.cmd = IWEVGENIE;
> - iwe.u.data.length = next - pos;
> - *current_ev = iwe_stream_add_point(info, *current_ev,
> - end_buf, &iwe, pos);
> + memset(&iwe, 0, sizeof(iwe));
> + iwe.cmd = IWEVGENIE;
> + iwe.u.data.length = next - pos;
> + *current_ev = iwe_stream_add_point(info, *current_ev,
> + end_buf, &iwe, pos);
>
> - pos = next;
> - }
> + pos = next;
> + }
>
> - if (end > pos) {
> - memset(&iwe, 0, sizeof(iwe));
> - iwe.cmd = IWEVGENIE;
> - iwe.u.data.length = end - pos;
> - *current_ev = iwe_stream_add_point(info, *current_ev,
> - end_buf, &iwe, pos);
> + if (end > pos) {
> + memset(&iwe, 0, sizeof(iwe));
> + iwe.cmd = IWEVGENIE;
> + iwe.u.data.length = end - pos;
> + *current_ev = iwe_stream_add_point(info, *current_ev,
> + end_buf, &iwe, pos);
> + }
> }
> }
>
> Index: wireless-testing/net/mac80211/util.c
> ===================================================================
> --- wireless-testing.orig/net/mac80211/util.c 2009-01-28 11:37:41.000000000 +0100
> +++ wireless-testing/net/mac80211/util.c 2009-01-28 15:57:03.000000000 +0100
> @@ -675,6 +675,46 @@ void ieee802_11_parse_elems(u8 *start, s
> }
> }
>
> +/*
> + * ieee80211_filter_ie() tries to keep only the relevant IEs for
> + * userspace (mostly hostap code).
> + */
> +int ieee80211_filter_ie(u8 *ie)
> +{
> + u8 id, ie_len, *pos;
> + u8 microsoft_oui[4] = {0x00, 0x50, 0xf2};
> + u8 wpa2_oui[3] = {0x00, 0x0f, 0xac};
> +
> + pos = ie;
> + id = *pos++;
> + ie_len = *pos++;
> +
> + switch (id) {
> + case WLAN_EID_RSN:
> + /* WPA2 */
> + if (ie_len >= 3 &&
> + !memcmp(pos, wpa2_oui, 3))
> + return 1;
> +
> + return 0;
> +
> + case WLAN_EID_VENDOR_SPECIFIC:
> + /* We're trying to catch WPA1, WMM and WPS IEs. */
> + if (ie_len >= 3 &&
> + !memcmp(pos, microsoft_oui, 3)) {
> + if ((pos[3] == 1) || /* WPA1 */
> + (pos[3] == 2) || /* WMM */
> + (pos[3] == 4)) /* WPS */
> + return 1;
> + }
> +
> + return 0;
> +
> + default:
> + return 0;
> + }
> +}
> +
> void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata)
> {
> struct ieee80211_local *local = sdata->local;
> Index: wireless-testing/net/mac80211/ieee80211_i.h
> ===================================================================
> --- wireless-testing.orig/net/mac80211/ieee80211_i.h 2009-01-28 11:37:41.000000000 +0100
> +++ wireless-testing/net/mac80211/ieee80211_i.h 2009-01-28 13:49:17.000000000 +0100
> @@ -1028,6 +1028,7 @@ void ieee80211_tx_skb(struct ieee80211_s
> int encrypt);
> void ieee802_11_parse_elems(u8 *start, size_t len,
> struct ieee802_11_elems *elems);
> +int ieee80211_filter_ie(u8 *ie);
> int ieee80211_set_freq(struct ieee80211_sub_if_data *sdata, int freq);
> u32 ieee80211_mandatory_rates(struct ieee80211_local *local,
> enum ieee80211_band band);

2009-02-05 15:36:58

by Tomas Winkler

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, Feb 5, 2009 at 4:57 PM, Dan Williams <[email protected]> wrote:
> On Thu, 2009-02-05 at 15:00 +0200, Tomas Winkler wrote:
>> On Thu, Feb 5, 2009 at 2:12 PM, Jouni Malinen <[email protected]> wrote:
>> > On Thu, Feb 05, 2009 at 01:44:15PM +0200, Tomas Winkler wrote:
>> >
>> >> Just a thought If user space will be able to merge 2 scan results we
>> >> can then split then in 2 separate reports....
>> >
>> > Theoretically speaking, yes, WEXT could be extended to do this with some
>> > kind of scan parameter that indicates which 64 kB block is requested.
>> > However, I would not suggest going there and I would probably object to
>> > a patch that tries to do this either in the kernel or in wpa_supplicant
>> > for that matter.
>>
>> I thin it can be done w/o extension of wext, j ust the eviction of
>> old scan results is done
>> in user space.IRCC the time stamp of beacon/probe is part of each
>> entry. And user space state machine should be able to react on
>> nl[SIOCGIWSCAN] w/o issuing ioctl[SIOCSIWSCAN]:
>
> I still don't like this. If 177 BSSes is not enough, then get the
> cfg80211 stuff working better for you. I can't think of how this scheme
> would work; which half of the results get returned when something calls
> SIOCGIWSCAN? Would the driver/stack internally flip to the other half
> of the results on each call of SIOCGIWSCAN? That doesn't work, because
> then two userspace processes requesting scan results will race with each
> other for them. We had this problem before when drivers cleared the
> scan list each time SIOCGIWSCAN was called.

Good point. A simple token will resolve this problem but this is again
changing wext which I wanted to avoid...no free lunch (
Anyhow I know it's possible but is this a common usecase that two
application request scan?

Thanks
Tomas

2009-02-04 15:10:38

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

Attachments:

signature.asc (836.00 B)
This is a digitally signed message part

2009-02-04 16:10:42

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Wed, 2009-02-04 at 16:08 +0100, Samuel Ortiz wrote:
> From: Samuel Ortiz <[email protected]>
>
> In very dense environment, the scan result buffer can get really large, mostly
> due to the addition of proprietary IEs. iwlist fails, typically warning about:
> "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> reallocating the scan result buffer several times, up to 132 Kbytes:
> [snip]
> Scan results did not fit - trying larger buffer (131072 bytes)
> ioctl[SIOCGIWSCAN]: Argument list too long
>
> By adding a mac80211 module parameter, we can filter the scan results and keep
> only the ones userspace currently worries about, i.e. WPA1, WPA2, WMM and WPS.
>
> To activate this feature, 1 has to be written to
> /sys/module/mac80211/parameters/ieee80211_scan_ie_filter

NAK. Use cfg80211. Module parameters are the wrong way to work around
API limitations; the API gets fixed or replaced instead. Furthermore,
the stack/drivers should *NOT* be filtering scan results at all. That's
the job of the userspace program requesting the results; otherwise
passive listeners will get only filtered results, and not the complete
scan list.

Dan

> Signed-off-by: Samuel Ortiz <[email protected]>
> ---
> net/mac80211/ieee80211_i.h | 1
> net/mac80211/scan.c | 65 ++++++++++++++++++++++++++++++---------------
> net/mac80211/util.c | 40 +++++++++++++++++++++++++++
> 3 files changed, 85 insertions(+), 21 deletions(-)
>
> Index: wireless-testing/net/mac80211/scan.c
> ===================================================================
> --- wireless-testing.orig/net/mac80211/scan.c 2009-01-28 11:37:55.000000000 +0100
> +++ wireless-testing/net/mac80211/scan.c 2009-01-28 17:32:48.000000000 +0100
> @@ -31,6 +31,11 @@
> #define IEEE80211_CHANNEL_TIME (HZ / 33)
> #define IEEE80211_PASSIVE_CHANNEL_TIME (HZ / 5)
>
> +static int ieee80211_scan_ie_filter;
> +module_param(ieee80211_scan_ie_filter, bool, 0644);
> +MODULE_PARM_DESC(ieee80211_scan_ie_filter,
> + "Filter IEs from scan results");
> +
> void ieee80211_rx_bss_list_init(struct ieee80211_local *local)
> {
> spin_lock_init(&local->bss_lock);
> @@ -725,33 +730,51 @@ static void ieee80211_scan_add_ies(struc
> if (bss == NULL || bss->ies == NULL)
> return;
>
> - /*
> - * If needed, fragment the IEs buffer (at IE boundaries) into short
> - * enough fragments to fit into IW_GENERIC_IE_MAX octet messages.
> - */
> pos = bss->ies;
> end = pos + bss->ies_len;
>
> - while (end - pos > IW_GENERIC_IE_MAX) {
> - next = pos + 2 + pos[1];
> - while (next + 2 + next[1] - pos < IW_GENERIC_IE_MAX)
> - next = next + 2 + next[1];
> + if (ieee80211_scan_ie_filter) {
> + while (pos - end) {
> + next = pos + 2 + pos[1];
> + if (ieee80211_filter_ie(pos)) {
> + memset(&iwe, 0, sizeof(iwe));
> + iwe.cmd = IWEVGENIE;
> + iwe.u.data.length = next - pos;
> + *current_ev =
> + iwe_stream_add_point(info, *current_ev,
> + end_buf, &iwe,
> + pos);
> + }
> + pos = next;
> + }
> + } else {
> + /*
> + * If needed, fragment the IEs buffer (at IE boundaries) into
> + * short enough fragments to fit into IW_GENERIC_IE_MAX octet
> + * messages.
> + */
> +
> + while (end - pos > IW_GENERIC_IE_MAX) {
> + next = pos + 2 + pos[1];
> + while (next + 2 + next[1] - pos < IW_GENERIC_IE_MAX)
> + next = next + 2 + next[1];
>
> - memset(&iwe, 0, sizeof(iwe));
> - iwe.cmd = IWEVGENIE;
> - iwe.u.data.length = next - pos;
> - *current_ev = iwe_stream_add_point(info, *current_ev,
> - end_buf, &iwe, pos);
> + memset(&iwe, 0, sizeof(iwe));
> + iwe.cmd = IWEVGENIE;
> + iwe.u.data.length = next - pos;
> + *current_ev = iwe_stream_add_point(info, *current_ev,
> + end_buf, &iwe, pos);
>
> - pos = next;
> - }
> + pos = next;
> + }
>
> - if (end > pos) {
> - memset(&iwe, 0, sizeof(iwe));
> - iwe.cmd = IWEVGENIE;
> - iwe.u.data.length = end - pos;
> - *current_ev = iwe_stream_add_point(info, *current_ev,
> - end_buf, &iwe, pos);
> + if (end > pos) {
> + memset(&iwe, 0, sizeof(iwe));
> + iwe.cmd = IWEVGENIE;
> + iwe.u.data.length = end - pos;
> + *current_ev = iwe_stream_add_point(info, *current_ev,
> + end_buf, &iwe, pos);
> + }
> }
> }
>
> Index: wireless-testing/net/mac80211/util.c
> ===================================================================
> --- wireless-testing.orig/net/mac80211/util.c 2009-01-28 11:37:41.000000000 +0100
> +++ wireless-testing/net/mac80211/util.c 2009-01-28 15:57:03.000000000 +0100
> @@ -675,6 +675,46 @@ void ieee802_11_parse_elems(u8 *start, s
> }
> }
>
> +/*
> + * ieee80211_filter_ie() tries to keep only the relevant IEs for
> + * userspace (mostly hostap code).
> + */
> +int ieee80211_filter_ie(u8 *ie)
> +{
> + u8 id, ie_len, *pos;
> + u8 microsoft_oui[4] = {0x00, 0x50, 0xf2};
> + u8 wpa2_oui[3] = {0x00, 0x0f, 0xac};
> +
> + pos = ie;
> + id = *pos++;
> + ie_len = *pos++;
> +
> + switch (id) {
> + case WLAN_EID_RSN:
> + /* WPA2 */
> + if (ie_len >= 3 &&
> + !memcmp(pos, wpa2_oui, 3))
> + return 1;
> +
> + return 0;
> +
> + case WLAN_EID_VENDOR_SPECIFIC:
> + /* We're trying to catch WPA1, WMM and WPS IEs. */
> + if (ie_len >= 3 &&
> + !memcmp(pos, microsoft_oui, 3)) {
> + if ((pos[3] == 1) || /* WPA1 */
> + (pos[3] == 2) || /* WMM */
> + (pos[3] == 4)) /* WPS */
> + return 1;
> + }
> +
> + return 0;
> +
> + default:
> + return 0;
> + }
> +}
> +
> void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata)
> {
> struct ieee80211_local *local = sdata->local;
> Index: wireless-testing/net/mac80211/ieee80211_i.h
> ===================================================================
> --- wireless-testing.orig/net/mac80211/ieee80211_i.h 2009-01-28 11:37:41.000000000 +0100
> +++ wireless-testing/net/mac80211/ieee80211_i.h 2009-01-28 13:49:17.000000000 +0100
> @@ -1028,6 +1028,7 @@ void ieee80211_tx_skb(struct ieee80211_s
> int encrypt);
> void ieee802_11_parse_elems(u8 *start, size_t len,
> struct ieee802_11_elems *elems);
> +int ieee80211_filter_ie(u8 *ie);
> int ieee80211_set_freq(struct ieee80211_sub_if_data *sdata, int freq);
> u32 ieee80211_mandatory_rates(struct ieee80211_local *local,
> enum ieee80211_band band);

2009-02-04 23:22:40

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

On Thu, 2009-02-05 at 00:18 +0100, Marcel Holtmann wrote:
> Hi Johannes,
>
> > > In very dense environment, the scan result buffer can get really large, mostly
> > > due to the addition of proprietary IEs. iwlist fails, typically warning about:
> > > "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> > > reallocating the scan result buffer several times, up to 132 Kbytes:
> > > [snip]
> > > Scan results did not fit - trying larger buffer (131072 bytes)
> > > ioctl[SIOCGIWSCAN]: Argument list too long
> > >
> > > By adding a mac80211 module parameter, we can filter the scan results and keep
> > > only the ones userspace currently worries about, i.e. WPA1, WPA2, WMM and WPS.
> > >
> > > To activate this feature, 1 has to be written to
> > > /sys/module/mac80211/parameters/ieee80211_scan_ie_filter
> >
> > Module parameter, hard-coded filter, just to work around wext crap. Eww.
> > NACK.
>
> we are perfectly aware that this is ugly and trust me, that nobody of us
> wants to do it. However we do have these setups and they happen in real
> life. I have been in a couple of locations where I ran into this problem
> and in that cases there is no way for me to establish any kind of WiFi
> connection at all, because the buffers are just too small. And that is
> with a laptop. Then just think about a mobile phone running mac80211
> where the memory is limited.
>
> So WEXT is crap and everybody agrees, but until we can use cfg80211 for
> scanning, we need something to make this work. So if this patch is not

No. This does not go into the kernel. Help make cfg80211 work for you,
and you're perfectly free to patch your kernels as you see fit. You can
certainly backport cfg80211 scanning bits to whatever kernels you care
about, just the same as you'd backport this hack to whatever kernels you
care about.

Dan

2009-02-04 23:29:28

by Marcel Holtmann

[permalink] [raw]

Subject: Re: [PATCH RFC] mac80211: Filter scan results

Hi Dan,

> > > > In very dense environment, the scan result buffer can get really large, mostly
> > > > due to the addition of proprietary IEs. iwlist fails, typically warning about:
> > > > "print_scanning_info: Allocation failed". wpa_supplicant fails as well, after
> > > > reallocating the scan result buffer several times, up to 132 Kbytes:
> > > > [snip]
> > > > Scan results did not fit - trying larger buffer (131072 bytes)
> > > > ioctl[SIOCGIWSCAN]: Argument list too long
> > > >
> > > > By adding a mac80211 module parameter, we can filter the scan results and keep
> > > > only the ones userspace currently worries about, i.e. WPA1, WPA2, WMM and WPS.
> > > >
> > > > To activate this feature, 1 has to be written to
> > > > /sys/module/mac80211/parameters/ieee80211_scan_ie_filter
> > >
> > > Module parameter, hard-coded filter, just to work around wext crap. Eww.
> > > NACK.
> >
> > we are perfectly aware that this is ugly and trust me, that nobody of us
> > wants to do it. However we do have these setups and they happen in real
> > life. I have been in a couple of locations where I ran into this problem
> > and in that cases there is no way for me to establish any kind of WiFi
> > connection at all, because the buffers are just too small. And that is
> > with a laptop. Then just think about a mobile phone running mac80211
> > where the memory is limited.
> >
> > So WEXT is crap and everybody agrees, but until we can use cfg80211 for
> > scanning, we need something to make this work. So if this patch is not
>
> No. This does not go into the kernel. Help make cfg80211 work for you,
> and you're perfectly free to patch your kernels as you see fit. You can
> certainly backport cfg80211 scanning bits to whatever kernels you care
> about, just the same as you'd backport this hack to whatever kernels you
> care about.

this is not a matter of backporting. The matter is that neither WEXT nor
cfg80211 allow scanning in dense RF environment. We are more than happy
to actually use cfg80211, but I have never seen any final patch for
adding scanning support. All of them were more or less work-in-progress.

Regards

Marcel

2009-02-05 12:10:34