2008-10-14 15:21:08

by Tim Gardner

[permalink] [raw]
Subject: [PATCH] ipw2200: Change driver default policies

>From 41804698de6f2ce121c1452943b9ad2b8a54988b Mon Sep 17 00:00:00 2001
From: Tim Gardner <[email protected]>
Date: Tue, 14 Oct 2008 08:16:09 -0600
Subject: [PATCH] ipw2200: change default policies for auto-associate, auto-create, and auto-roaming

We now have applications available to set policy. This patch changes the driver defaults to:

1) associate=0 - do not automatically associate to an SSID.
2) auto_create=0 - do not automatically create an ad-hoc network
3) roaming=0 - do not automatically roam to another AP with the same SSID.

Signed-off-by: Tim Gardner <[email protected]>
---
Documentation/networking/README.ipw2200 | 6 +++---
drivers/net/wireless/ipw2200.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/README.ipw2200 b/Documentation/networking/README.ipw2200
index 4f2a40f..8f3e40d 100644
--- a/Documentation/networking/README.ipw2200
+++ b/Documentation/networking/README.ipw2200
@@ -147,14 +147,14 @@ Where the supported parameter are:
driver. If disabled, the driver will not attempt to scan
for and associate to a network until it has been configured with
one or more properties for the target network, for example configuring
- the network SSID. Default is 1 (auto-associate)
+ the network SSID. Default is 0 (do not auto-associate)

- Example: % modprobe ipw2200 associate=0
+ Example: % modprobe ipw2200 associate=1

auto_create
Set to 0 to disable the auto creation of an Ad-Hoc network
matching the channel and network name parameters provided.
- Default is 1.
+ Default is 0.

channel
channel number for association. The normal method for setting
diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index dcce354..31f396b 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -87,13 +87,13 @@ static int channel = 0;
static int mode = 0;

static u32 ipw_debug_level;
-static int associate = 1;
-static int auto_create = 1;
+static int associate;
+static int auto_create;
static int led = 0;
static int disable = 0;
static int bt_coexist = 0;
static int hwcrypto = 0;
-static int roaming = 1;
+static int roaming;
static const char ipw_modes[] = {
'a', 'b', 'g', '?'
};
--
1.5.6.3



2008-10-14 15:44:17

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] ipw2200: Change driver default policies

On Tue, 2008-10-14 at 08:57 -0600, Tim Gardner wrote:
> From 41804698de6f2ce121c1452943b9ad2b8a54988b Mon Sep 17 00:00:00 2001
> From: Tim Gardner <[email protected]>
> Date: Tue, 14 Oct 2008 08:16:09 -0600
> Subject: [PATCH] ipw2200: change default policies for auto-associate, auto-create, and auto-roaming
>
> We now have applications available to set policy. This patch changes the driver defaults to:
>
> 1) associate=0 - do not automatically associate to an SSID.

This one is fine.

> 2) auto_create=0 - do not automatically create an ad-hoc network

This will break adhoc network creation, thus NAK. I think we should
just remove the module parameter and default to 1 anyway. This is the
way adhoc _should_ work in the first place, if the adhoc network you're
attempting to join does not yet exist, the driver should create that
network. This is the way it works for all other drivers as well.

Disabling this effectively means you can only join existing adhoc
networks, not start your own.

> 3) roaming=0 - do not automatically roam to another AP with the same SSID.

Not really sure why this should be changed? Were there reported issues
with this behavior?

Dan

> Signed-off-by: Tim Gardner <[email protected]>
> ---
> Documentation/networking/README.ipw2200 | 6 +++---
> drivers/net/wireless/ipw2200.c | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/networking/README.ipw2200 b/Documentation/networking/README.ipw2200
> index 4f2a40f..8f3e40d 100644
> --- a/Documentation/networking/README.ipw2200
> +++ b/Documentation/networking/README.ipw2200
> @@ -147,14 +147,14 @@ Where the supported parameter are:
> driver. If disabled, the driver will not attempt to scan
> for and associate to a network until it has been configured with
> one or more properties for the target network, for example configuring
> - the network SSID. Default is 1 (auto-associate)
> + the network SSID. Default is 0 (do not auto-associate)
>
> - Example: % modprobe ipw2200 associate=0
> + Example: % modprobe ipw2200 associate=1
>
> auto_create
> Set to 0 to disable the auto creation of an Ad-Hoc network
> matching the channel and network name parameters provided.
> - Default is 1.
> + Default is 0.
>
> channel
> channel number for association. The normal method for setting
> diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
> index dcce354..31f396b 100644
> --- a/drivers/net/wireless/ipw2200.c
> +++ b/drivers/net/wireless/ipw2200.c
> @@ -87,13 +87,13 @@ static int channel = 0;
> static int mode = 0;
>
> static u32 ipw_debug_level;
> -static int associate = 1;
> -static int auto_create = 1;
> +static int associate;
> +static int auto_create;
> static int led = 0;
> static int disable = 0;
> static int bt_coexist = 0;
> static int hwcrypto = 0;
> -static int roaming = 1;
> +static int roaming;
> static const char ipw_modes[] = {
> 'a', 'b', 'g', '?'
> };


2008-10-14 16:36:43

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] ipw2200: Change driver default policies

On Tue, 2008-10-14 at 10:25 -0600, Tim Gardner wrote:
> Dan Williams wrote:
> > On Tue, 2008-10-14 at 08:57 -0600, Tim Gardner wrote:
> >> From 41804698de6f2ce121c1452943b9ad2b8a54988b Mon Sep 17 00:00:00 2001
> >> From: Tim Gardner <[email protected]>
> >> Date: Tue, 14 Oct 2008 08:16:09 -0600
> >> Subject: [PATCH] ipw2200: change default policies for auto-associate, auto-create, and auto-roaming
> >>
> >> We now have applications available to set policy. This patch changes the driver defaults to:
> >>
> >> 1) associate=0 - do not automatically associate to an SSID.
> >
> > This one is fine.
> >
> >> 2) auto_create=0 - do not automatically create an ad-hoc network
> >
> > This will break adhoc network creation, thus NAK. I think we should
> > just remove the module parameter and default to 1 anyway. This is the
> > way adhoc _should_ work in the first place, if the adhoc network you're
> > attempting to join does not yet exist, the driver should create that
> > network. This is the way it works for all other drivers as well.
> >
> > Disabling this effectively means you can only join existing adhoc
> > networks, not start your own.
> >
>
> Agreed - I missed the part where its qualified by IW_MODE_ADHOC.
>
> >> 3) roaming=0 - do not automatically roam to another AP with the same SSID.
> >
> > Not really sure why this should be changed? Were there reported issues
> > with this behavior?
> >
>
> Its been my expereince that wireless drivers rarely make the correct
> decision about when or where to roam. It also seems like a policy
> decision that ought to be made elsewhere.
>
> Frankly, I'm not that concerned about the roaming option. The default
> for 'associate' was the one I _really_ wanted changed. I'll submit a v2
> patch with just that change.

Sure. That's something I've wanted to turn off for quite a while, it's
sort of a security problem. Nothing should be auto-associating until
told to do so to a specific SSID or SSID/BSSID combination.

Dan


2008-10-14 16:44:52

by Tim Gardner

[permalink] [raw]
Subject: Re: [PATCH] ipw2200: Change driver default policies

Johannes Berg wrote:
> On Tue, 2008-10-14 at 10:25 -0600, Tim Gardner wrote:
>
>>>> 3) roaming=0 - do not automatically roam to another AP with the same SSID.
>>> Not really sure why this should be changed? Were there reported issues
>>> with this behavior?
>>>
>> Its been my expereince that wireless drivers rarely make the correct
>> decision about when or where to roam. It also seems like a policy
>> decision that ought to be made elsewhere.
>>
>> Frankly, I'm not that concerned about the roaming option. The default
>> for 'associate' was the one I _really_ wanted changed. I'll submit a v2
>> patch with just that change.
>
> Would that also turn off the spectrum-polluting continuous scanning
> these cards do? :)
>
> johannes

I've always thought that background scanning was a bad idea, especially
when coupled with active probing. Its not practical to switch channels
for a reasonable dwell time without having some impact on connectivity.

rtg
--
Tim Gardner [email protected] http://www.tpi.com
OR 503-601-0234 x102 MT 406-443-5357

2008-10-14 16:35:43

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] ipw2200: Change driver default policies

On Tue, 2008-10-14 at 10:25 -0600, Tim Gardner wrote:

> >> 3) roaming=0 - do not automatically roam to another AP with the same SSID.
> >
> > Not really sure why this should be changed? Were there reported issues
> > with this behavior?
> >
>
> Its been my expereince that wireless drivers rarely make the correct
> decision about when or where to roam. It also seems like a policy
> decision that ought to be made elsewhere.
>
> Frankly, I'm not that concerned about the roaming option. The default
> for 'associate' was the one I _really_ wanted changed. I'll submit a v2
> patch with just that change.

Would that also turn off the spectrum-polluting continuous scanning
these cards do? :)

johannes


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

2008-10-14 16:25:12

by Tim Gardner

[permalink] [raw]
Subject: Re: [PATCH] ipw2200: Change driver default policies

Dan Williams wrote:
> On Tue, 2008-10-14 at 08:57 -0600, Tim Gardner wrote:
>> From 41804698de6f2ce121c1452943b9ad2b8a54988b Mon Sep 17 00:00:00 2001
>> From: Tim Gardner <[email protected]>
>> Date: Tue, 14 Oct 2008 08:16:09 -0600
>> Subject: [PATCH] ipw2200: change default policies for auto-associate, auto-create, and auto-roaming
>>
>> We now have applications available to set policy. This patch changes the driver defaults to:
>>
>> 1) associate=0 - do not automatically associate to an SSID.
>
> This one is fine.
>
>> 2) auto_create=0 - do not automatically create an ad-hoc network
>
> This will break adhoc network creation, thus NAK. I think we should
> just remove the module parameter and default to 1 anyway. This is the
> way adhoc _should_ work in the first place, if the adhoc network you're
> attempting to join does not yet exist, the driver should create that
> network. This is the way it works for all other drivers as well.
>
> Disabling this effectively means you can only join existing adhoc
> networks, not start your own.
>

Agreed - I missed the part where its qualified by IW_MODE_ADHOC.

>> 3) roaming=0 - do not automatically roam to another AP with the same SSID.
>
> Not really sure why this should be changed? Were there reported issues
> with this behavior?
>

Its been my expereince that wireless drivers rarely make the correct
decision about when or where to roam. It also seems like a policy
decision that ought to be made elsewhere.

Frankly, I'm not that concerned about the roaming option. The default
for 'associate' was the one I _really_ wanted changed. I'll submit a v2
patch with just that change.

rtg
--
Tim Gardner [email protected] http://www.tpi.com
OR 503-601-0234 x102 MT 406-443-5357

2008-10-14 16:48:17

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] ipw2200: Change driver default policies

On Tue, 2008-10-14 at 18:35 +0200, Johannes Berg wrote:
> On Tue, 2008-10-14 at 10:25 -0600, Tim Gardner wrote:
>
> > >> 3) roaming=0 - do not automatically roam to another AP with the same SSID.
> > >
> > > Not really sure why this should be changed? Were there reported issues
> > > with this behavior?
> > >
> >
> > Its been my expereince that wireless drivers rarely make the correct
> > decision about when or where to roam. It also seems like a policy
> > decision that ought to be made elsewhere.
> >
> > Frankly, I'm not that concerned about the roaming option. The default
> > for 'associate' was the one I _really_ wanted changed. I'll submit a v2
> > patch with just that change.
>
> Would that also turn off the spectrum-polluting continuous scanning
> these cards do? :)

No, that would be a separate patch to disable the background scanning
behavior or at least tone it down to once every few seconds or
something.

Dan