[PATCH v2] mac80211: Call commit() on channel setting

[PATCH v2] mac80211: Call commit() on channel setting

[Alina Friedrichsen]

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.

Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>
---
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index c43129e..015062e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1173,11 +1173,19 @@ static int ieee80211_set_channel(struct wiphy *=
wiphy,
 				 enum nl80211_channel_type channel_type)
 {
 	struct ieee80211_local *local =3D wiphy_priv(wiphy);
+	struct ieee80211_sub_if_data *sdata;
+	int ret;
=20
 	local->oper_channel =3D chan;
 	local->oper_channel_type =3D channel_type;
=20
-	return ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+	ret =3D ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		ieee80211_commit(sdata);
+	}
+
+	return ret;
 }
=20
 static int set_mgmt_extra_ie_sta(struct ieee80211_sub_if_data *sdata,
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
Computer Bild Tarifsieger! GMX FreeDSL - Telefonanschluss + DSL
f=FCr nur 17,95 =BF/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Michael Buesch]

On Friday 27 February 2009 00:43:07 Alina Friedrichsen wrote:
> 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 fix the joining process of STA and IBSS a little, which I will make tomorrow.


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.

> Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>
> ---
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index c43129e..015062e 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1173,11 +1173,19 @@ static int ieee80211_set_channel(struct wiphy *wiphy,
>  				 enum nl80211_channel_type channel_type)
>  {
>  	struct ieee80211_local *local = wiphy_priv(wiphy);
> +	struct ieee80211_sub_if_data *sdata;
> +	int ret;
>  
>  	local->oper_channel = chan;
>  	local->oper_channel_type = channel_type;
>  
> -	return ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
> +	ret = ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
> +
> +	list_for_each_entry(sdata, &local->interfaces, list) {
> +		ieee80211_commit(sdata);
> +	}

Why do you commit, even if channel change failed?

> +
> +	return ret;
>  }
>  
>  static int set_mgmt_extra_ie_sta(struct ieee80211_sub_if_data *sdata,
> 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_device *dev,
>  				   struct iw_request_info *info,
>  				   struct iw_freq *freq, char *extra)
>  {
> +	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
>  	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> +	struct ieee80211_sub_if_data *nsdata;
> +	int ret;
>  
>  	if (sdata->vif.type == NL80211_IFTYPE_ADHOC)
>  		sdata->u.ibss.flags &= ~IEEE80211_IBSS_AUTO_CHANNEL_SEL;
> @@ -173,19 +176,25 @@ static int ieee80211_ioctl_siwfreq(struct net_device *dev,
>  			else if (sdata->vif.type == NL80211_IFTYPE_STATION)
>  				sdata->u.mgd.flags |=
>  					IEEE80211_STA_AUTO_CHANNEL_SEL;
> -			return 0;
> +			ret = 0;
>  		} else
> -			return ieee80211_set_freq(sdata,
> +			ret = ieee80211_set_freq(sdata,
>  				ieee80211_channel_to_frequency(freq->m));
>  	} else {
>  		int i, div = 1000000;
>  		for (i = 0; i < freq->e; i++)
>  			div /= 10;
>  		if (div > 0)
> -			return ieee80211_set_freq(sdata, freq->m / div);
> +			ret = ieee80211_set_freq(sdata, freq->m / div);
>  		else
> -			return -EINVAL;
> +			ret = -EINVAL;
>  	}
> +
> +	list_for_each_entry(nsdata, &local->interfaces, list) {
> +		ieee80211_commit(nsdata);
> +	}

dito.

> +
> +	return ret;
>  }
>  
>  
> 



-- 
Greetings, Michael.

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

[Alina Friedrichsen]

Hello Michael!

> Could you explain what commit is supposed to to after the channel swi=
tch?
> 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 cha=
nge.

I think after all changes which affect the selected network a rejoin sh=
ould be done.

=46or example you have at channel 4 a AP with the SSID "test" und at ch=
annel 11 an other AP with the same SSID, too.

If you now first say set_ssid("test") and then set_channel(11), we firs=
t join the network at channel 4 and then we switch the hardware low-lev=
el to channel 11. So we hang now in channel 4 with the BSSID of the net=
work in channel 4 and nothing works anymore.

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Alina Friedrichsen]

> I think after all changes which affect the selected network a rejoin
> should be done.
>=20
> For example you have at channel 4 a AP with the SSID "test" und at ch=
annel
> 11 an other AP with the same SSID, too.
>=20
> If you now first say set_ssid("test") and then set_channel(11), we fi=
rst
> join the network at channel 4 and then we switch the hardware low-lev=
el to
> channel 11. So we hang now in channel 4 with the BSSID of the network=
 in
> channel 4 and nothing works anymore.

So we hang now in channel _11_ with the BSSID of the network in
channel 4 and nothing works anymore.

> 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 c=
an be
> recovered.

--=20
Computer Bild Tarifsieger! GMX FreeDSL - Telefonanschluss + DSL
f=FCr nur 17,95 =BF/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Michael Buesch]

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.

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

[Alina Friedrichsen]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Alina Friedrichsen]

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 <x-alina@gmx.net>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

On Fri, 2009-02-27 at 00:43 +0100, Alina Friedrichsen wrote:
> 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 fix the joining process of STA and IBSS a little, which I will make tomorrow.

You really should remember to linewrap your descriptions.

> +	list_for_each_entry(sdata, &local->interfaces, list) {
> +		ieee80211_commit(sdata);
> +	}

That doesn't look like it can work? sta_commit/ibss_commit? Separate by
interface type?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Alina Friedrichsen]

Hello Johannes!

> You really should remember to linewrap your descriptions.

Sorry, I'm accustomed that the software wrap it
automatically to the screen width. (XHTML and so...)

Can you please describe how a IBSS join should done with
nl80211? "Das sich wext und nl80211 nicht gegenseitig
auf die Fuesse treten." (Sorry I can't translate this to
English - locution.)

Regards
Alina

P.S.: I have received this mail 5 hours to late.

--=20
Psssst! Schon vom neuen GMX MultiMessenger geh=F6rt? Der kann`s mit all=
en: http://www.gmx.net/de/go/multimessenger01
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Berg]

On Fri, 2009-02-27 at 22:26 +0100, Alina Friedrichsen wrote:
> Hello Johannes!
> 
> > You really should remember to linewrap your descriptions.
> 
> Sorry, I'm accustomed that the software wrap it
> automatically to the screen width. (XHTML and so...)
> 
> Can you please describe how a IBSS join should done with
> nl80211? "Das sich wext und nl80211 nicht gegenseitig
> auf die Fuesse treten." (Sorry I can't translate this to
> English - locution.)

on the todo list

> Regards
> Alina
> 
> P.S.: I have received this mail 5 hours to late.

well I\m travelling and wrote that on plane -- off for a week now

johannes