2008-10-28 18:36:50

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] iwlwifi: remove implicit direct scan

When an undirected scan is requested and iwlwifi is not associated but
the user has set an SSID (and maybe was associated with that network at
some point) then iwlwifi will assume the user wanted to scan for this
SSID which seems wrong. Remove this code.

Signed-off-by: Johannes Berg <[email protected]>
---
Am I missing something? This seems very strange! What specifies that
when an SSID is set on an interface the user wants to scan for that one?
The interface might just have been associated previously to that SSID,
now disassociated and network manager is asking for a scan to get
information about networks to let the user select a new one.

drivers/net/wireless/iwlwifi/iwl-scan.c | 7 -------
drivers/net/wireless/iwlwifi/iwl3945-base.c | 8 --------
2 files changed, 15 deletions(-)

--- everything.orig/drivers/net/wireless/iwlwifi/iwl-scan.c 2008-10-28 18:12:02.000000000 +0100
+++ everything/drivers/net/wireless/iwlwifi/iwl-scan.c 2008-10-28 18:12:06.000000000 +0100
@@ -743,13 +743,6 @@ static void iwl_bg_request_scan(struct w
memcpy(scan->direct_scan[0].ssid,
priv->direct_ssid, priv->direct_ssid_len);
n_probes++;
- } else if (!iwl_is_associated(priv) && priv->essid_len) {
- IWL_DEBUG_SCAN("Start direct scan for '%s' (not associated)\n",
- print_ssid(ssid, priv->essid, priv->essid_len));
- scan->direct_scan[0].id = WLAN_EID_SSID;
- scan->direct_scan[0].len = priv->essid_len;
- memcpy(scan->direct_scan[0].ssid, priv->essid, priv->essid_len);
- n_probes++;
} else {
IWL_DEBUG_SCAN("Start indirect scan.\n");
}
--- everything.orig/drivers/net/wireless/iwlwifi/iwl3945-base.c 2008-10-28 18:12:20.000000000 +0100
+++ everything/drivers/net/wireless/iwlwifi/iwl3945-base.c 2008-10-28 18:12:27.000000000 +0100
@@ -6158,14 +6158,6 @@ static void iwl3945_bg_request_scan(stru
memcpy(scan->direct_scan[0].ssid,
priv->direct_ssid, priv->direct_ssid_len);
n_probes++;
- } else if (!iwl3945_is_associated(priv) && priv->essid_len) {
- IWL_DEBUG_SCAN
- ("Kicking off one direct scan for '%s' when not associated\n",
- print_ssid(ssid, priv->essid, priv->essid_len));
- scan->direct_scan[0].id = WLAN_EID_SSID;
- scan->direct_scan[0].len = priv->essid_len;
- memcpy(scan->direct_scan[0].ssid, priv->essid, priv->essid_len);
- n_probes++;
} else
IWL_DEBUG_SCAN("Kicking off one indirect scan.\n");





2008-10-28 23:48:59

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: remove implicit direct scan

On Wed, Oct 29, 2008 at 12:59 AM, Johannes Berg
<[email protected]> wrote:
> On Tue, 2008-10-28 at 23:54 +0200, Tomas Winkler wrote:
>
>> >> Am I missing something? This seems very strange! What specifies that
>> >> when an SSID is set on an interface the user wants to scan for that one?
>> >> The interface might just have been associated previously to that SSID,
>> >> now disassociated and network manager is asking for a scan to get
>> >> information about networks to let the user select a new one.
>> >
>> > NACK, this is a heuristic that helps connection to configured SSID. It
>> > doesn't mean that other ssid won't be scanned.
>> > This code might be revised again when multiple SSID scan will be implemented
>> > Tomas
>> >
>> Okay I've only now have seen the real reason for this. The SSID
>> removal from the driver API. It's much better argument for removing
>> this code anyway have to think how this will be substituted.
>
> :)
> Unless somebody can come up with a good argument for requiring the SSID?
> I don't see it being necessary the way beaconing and everything is
> designed in mac80211 now, and this leaves only the BSSID in the
> interface config which I'll move to BSS config and then remove the
> interface config thing completely.
>
> Also, doesn't ieee80211_sta_config_auth already do exactly this
> heuristic thing? If it doesn't know about the BSS it'll initiate a scan
> for the SSID it's looking for?

Yep, you are right.
Tomas
>
> johannes
>

2008-10-28 21:54:34

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: remove implicit direct scan

On Tue, Oct 28, 2008 at 10:53 PM, Tomas Winkler <[email protected]> wrote:
> On Tue, Oct 28, 2008 at 7:21 PM, Johannes Berg
> <[email protected]> wrote:
>> When an undirected scan is requested and iwlwifi is not associated but
>> the user has set an SSID (and maybe was associated with that network at
>> some point) then iwlwifi will assume the user wanted to scan for this
>> SSID which seems wrong. Remove this code.
>>
>> Signed-off-by: Johannes Berg <[email protected]>
>> ---
>> Am I missing something? This seems very strange! What specifies that
>> when an SSID is set on an interface the user wants to scan for that one?
>> The interface might just have been associated previously to that SSID,
>> now disassociated and network manager is asking for a scan to get
>> information about networks to let the user select a new one.
>
> NACK, this is a heuristic that helps connection to configured SSID. It
> doesn't mean that other ssid won't be scanned.
> This code might be revised again when multiple SSID scan will be implemented
> Tomas
>
Okay I've only now have seen the real reason for this. The SSID
removal from the driver API. It's much better argument for removing
this code anyway have to think how this will be substituted.
Thanks
Tomas

>
>> drivers/net/wireless/iwlwifi/iwl-scan.c | 7 -------
>> drivers/net/wireless/iwlwifi/iwl3945-base.c | 8 --------
>> 2 files changed, 15 deletions(-)
>>
>> --- everything.orig/drivers/net/wireless/iwlwifi/iwl-scan.c 2008-10-28 18:12:02.000000000 +0100
>> +++ everything/drivers/net/wireless/iwlwifi/iwl-scan.c 2008-10-28 18:12:06.000000000 +0100
>> @@ -743,13 +743,6 @@ static void iwl_bg_request_scan(struct w
>> memcpy(scan->direct_scan[0].ssid,
>> priv->direct_ssid, priv->direct_ssid_len);
>> n_probes++;
>> - } else if (!iwl_is_associated(priv) && priv->essid_len) {
>> - IWL_DEBUG_SCAN("Start direct scan for '%s' (not associated)\n",
>> - print_ssid(ssid, priv->essid, priv->essid_len));
>> - scan->direct_scan[0].id = WLAN_EID_SSID;
>> - scan->direct_scan[0].len = priv->essid_len;
>> - memcpy(scan->direct_scan[0].ssid, priv->essid, priv->essid_len);
>> - n_probes++;
>> } else {
>> IWL_DEBUG_SCAN("Start indirect scan.\n");
>> }
>> --- everything.orig/drivers/net/wireless/iwlwifi/iwl3945-base.c 2008-10-28 18:12:20.000000000 +0100
>> +++ everything/drivers/net/wireless/iwlwifi/iwl3945-base.c 2008-10-28 18:12:27.000000000 +0100
>> @@ -6158,14 +6158,6 @@ static void iwl3945_bg_request_scan(stru
>> memcpy(scan->direct_scan[0].ssid,
>> priv->direct_ssid, priv->direct_ssid_len);
>> n_probes++;
>> - } else if (!iwl3945_is_associated(priv) && priv->essid_len) {
>> - IWL_DEBUG_SCAN
>> - ("Kicking off one direct scan for '%s' when not associated\n",
>> - print_ssid(ssid, priv->essid, priv->essid_len));
>> - scan->direct_scan[0].id = WLAN_EID_SSID;
>> - scan->direct_scan[0].len = priv->essid_len;
>> - memcpy(scan->direct_scan[0].ssid, priv->essid, priv->essid_len);
>> - n_probes++;
>> } else
>> IWL_DEBUG_SCAN("Kicking off one indirect scan.\n");
>>
>>
>>
>>
>

2008-10-28 23:13:43

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: remove implicit direct scan

On Tue, 2008-10-28 at 23:54 +0200, Tomas Winkler wrote:

> >> Am I missing something? This seems very strange! What specifies that
> >> when an SSID is set on an interface the user wants to scan for that one?
> >> The interface might just have been associated previously to that SSID,
> >> now disassociated and network manager is asking for a scan to get
> >> information about networks to let the user select a new one.
> >
> > NACK, this is a heuristic that helps connection to configured SSID. It
> > doesn't mean that other ssid won't be scanned.
> > This code might be revised again when multiple SSID scan will be implemented
> > Tomas
> >
> Okay I've only now have seen the real reason for this. The SSID
> removal from the driver API. It's much better argument for removing
> this code anyway have to think how this will be substituted.

:)
Unless somebody can come up with a good argument for requiring the SSID?
I don't see it being necessary the way beaconing and everything is
designed in mac80211 now, and this leaves only the BSSID in the
interface config which I'll move to BSS config and then remove the
interface config thing completely.

Also, doesn't ieee80211_sta_config_auth already do exactly this
heuristic thing? If it doesn't know about the BSS it'll initiate a scan
for the SSID it's looking for?

johannes


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

2008-10-28 20:53:53

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH] iwlwifi: remove implicit direct scan

On Tue, Oct 28, 2008 at 7:21 PM, Johannes Berg
<[email protected]> wrote:
> When an undirected scan is requested and iwlwifi is not associated but
> the user has set an SSID (and maybe was associated with that network at
> some point) then iwlwifi will assume the user wanted to scan for this
> SSID which seems wrong. Remove this code.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> Am I missing something? This seems very strange! What specifies that
> when an SSID is set on an interface the user wants to scan for that one?
> The interface might just have been associated previously to that SSID,
> now disassociated and network manager is asking for a scan to get
> information about networks to let the user select a new one.

NACK, this is a heuristic that helps connection to configured SSID. It
doesn't mean that other ssid won't be scanned.
This code might be revised again when multiple SSID scan will be implemented
Tomas


> drivers/net/wireless/iwlwifi/iwl-scan.c | 7 -------
> drivers/net/wireless/iwlwifi/iwl3945-base.c | 8 --------
> 2 files changed, 15 deletions(-)
>
> --- everything.orig/drivers/net/wireless/iwlwifi/iwl-scan.c 2008-10-28 18:12:02.000000000 +0100
> +++ everything/drivers/net/wireless/iwlwifi/iwl-scan.c 2008-10-28 18:12:06.000000000 +0100
> @@ -743,13 +743,6 @@ static void iwl_bg_request_scan(struct w
> memcpy(scan->direct_scan[0].ssid,
> priv->direct_ssid, priv->direct_ssid_len);
> n_probes++;
> - } else if (!iwl_is_associated(priv) && priv->essid_len) {
> - IWL_DEBUG_SCAN("Start direct scan for '%s' (not associated)\n",
> - print_ssid(ssid, priv->essid, priv->essid_len));
> - scan->direct_scan[0].id = WLAN_EID_SSID;
> - scan->direct_scan[0].len = priv->essid_len;
> - memcpy(scan->direct_scan[0].ssid, priv->essid, priv->essid_len);
> - n_probes++;
> } else {
> IWL_DEBUG_SCAN("Start indirect scan.\n");
> }
> --- everything.orig/drivers/net/wireless/iwlwifi/iwl3945-base.c 2008-10-28 18:12:20.000000000 +0100
> +++ everything/drivers/net/wireless/iwlwifi/iwl3945-base.c 2008-10-28 18:12:27.000000000 +0100
> @@ -6158,14 +6158,6 @@ static void iwl3945_bg_request_scan(stru
> memcpy(scan->direct_scan[0].ssid,
> priv->direct_ssid, priv->direct_ssid_len);
> n_probes++;
> - } else if (!iwl3945_is_associated(priv) && priv->essid_len) {
> - IWL_DEBUG_SCAN
> - ("Kicking off one direct scan for '%s' when not associated\n",
> - print_ssid(ssid, priv->essid, priv->essid_len));
> - scan->direct_scan[0].id = WLAN_EID_SSID;
> - scan->direct_scan[0].len = priv->essid_len;
> - memcpy(scan->direct_scan[0].ssid, priv->essid, priv->essid_len);
> - n_probes++;
> } else
> IWL_DEBUG_SCAN("Kicking off one indirect scan.\n");
>
>
>
>