2009-02-27 16:16:41

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: Call commit() on channel setting

On Friday 27 February 2009 16:34:18 Alina Friedrichsen wrote:
> > Could you explain what commit is supposed to to after the channel switch?
> > To me it's not clear what the difference in functionality is.
> > What do you expect to happen on the commit call after the channel change.
>
> I think after all changes which affect the selected network a rejoin should be done.
>
> For example you have at channel 4 a AP with the SSID "test" und at channel 11 an other AP with the same SSID, too.
>
> If you now first say set_ssid("test") and then set_channel(11), we first join the network at channel 4 and then we switch the hardware low-level to channel 11. So we hang now in channel 4 with the BSSID of the network in channel 4 and nothing works anymore.

I think this is desired behavior. We voluntarily associated with the "test" AP on channel 4.
Who says that we want to assoc to the "test" ssid on channel 11? This is a policy decision and such policy
doesn't belong into the kernel. Userspace can initiate a reassociation after the channel change,
if the AP switch is desired.
There are good reasons to leave roaming decisions in userspace, because these decisions are simply very
complex and would put lots of policy into the kernel or require lots of kernel API.

> I think the order of the commands should not affect the result on the end and the driver should never hang in an broken/undefined state if it can be recovered.

--
Greetings, Michael.


2009-02-27 17:16:22

by Alina Friedrichsen

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: Call commit() on channel setting

Hello Michael!

> I think this is desired behavior.

Yes.

> We voluntarily associated with the
> "test" AP on channel 4.

Yes this is correct because IEEE80211_IBSS_AUTO_CHANNEL_SEL flag was se=
t.

> Who says that we want to assoc to the "test" ssid on channel 11?

Because this setting is now outdated. The channel is set to 11 and the =
IEEE80211_IBSS_AUTO_CHANNEL_SEL flag is clear.

If we now set the SSID to test again, we assoc to "test" on channel 11.=
Do not recover from the channel set automatically, is a broken behavio=
r, I think.

> This is a
> policy decision and such policy
> doesn't belong into the kernel. Userspace can initiate a reassociatio=
n
> after the channel change,
> if the AP switch is desired.
> There are good reasons to leave roaming decisions in userspace, becau=
se
> these decisions are simply very
> complex and would put lots of policy into the kernel or require lots =
of
> kernel API.

I think the basic functionality should work without any userspace daemo=
n. All the that can be implemented without real problems with a few lin=
es of code. This would better for embedded systems, if we don't need th=
e wpa_supplicant for an unencrypted network, or so.

Regards
Alina

--=20
Computer Bild Tarifsieger! GMX FreeDSL - Telefonanschluss + DSL
f=FCr nur 17,95 =BF/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T45=
69a

2009-02-27 18:25:24

by Alina Friedrichsen

[permalink] [raw]
Subject: [PATCH v3] mac80211: Call commit() on channel setting

This calls commit() on all logical interfaces of a physical interface, =
if the channel of it is changed. Before it works as it should, I must f=
ix the joining process of STA and IBSS a little, which I will make tomo=
rrow.

This version call it only in the wireless extensions, because the handl=
ing in nl80211 must discussed more, but I think in the case of wext it =
is the right way.

Signed-off-by: Alina Friedrichsen <[email protected]>
---
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index 1ac0516..dce1618 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -157,7 +157,10 @@ static int ieee80211_ioctl_siwfreq(struct net_devi=
ce *dev,
struct iw_request_info *info,
struct iw_freq *freq, char *extra)
{
+ struct ieee80211_local *local =3D wdev_priv(dev->ieee80211_ptr);
struct ieee80211_sub_if_data *sdata =3D IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_sub_if_data *nsdata;
+ int ret;
=20
if (sdata->vif.type =3D=3D NL80211_IFTYPE_ADHOC)
sdata->u.ibss.flags &=3D ~IEEE80211_IBSS_AUTO_CHANNEL_SEL;
@@ -173,19 +176,25 @@ static int ieee80211_ioctl_siwfreq(struct net_dev=
ice *dev,
else if (sdata->vif.type =3D=3D NL80211_IFTYPE_STATION)
sdata->u.mgd.flags |=3D
IEEE80211_STA_AUTO_CHANNEL_SEL;
- return 0;
+ ret =3D 0;
} else
- return ieee80211_set_freq(sdata,
+ ret =3D ieee80211_set_freq(sdata,
ieee80211_channel_to_frequency(freq->m));
} else {
int i, div =3D 1000000;
for (i =3D 0; i < freq->e; i++)
div /=3D 10;
if (div > 0)
- return ieee80211_set_freq(sdata, freq->m / div);
+ ret =3D ieee80211_set_freq(sdata, freq->m / div);
else
- return -EINVAL;
+ ret =3D -EINVAL;
}
+
+ list_for_each_entry(nsdata, &local->interfaces, list) {
+ ieee80211_commit(nsdata);
+ }
+
+ return ret;
}
=20
=20

--=20
Psssst! Schon vom neuen GMX MultiMessenger geh=F6rt? Der kann`s mit all=
en: http://www.gmx.net/de/go/multimessenger01