[PATCH RFC v2] mac80211: no BSS changes to driver from beacons processed during scanning

[PATCH RFC v2] mac80211: no BSS changes to driver from beacons processed during scanning

[Reinette Chatre]

There is no need to send BSS changes to driver from beacons processed
during scanning. We are more interested in beacons from an AP with whic=
h
we are associated - these will still be used to send updates to driver =
as
the beacons are received without scanning.

This change=C2=B7removes the requirement that bss_info_changed needs to=
 be atomic.
The beacons received during scanning are processed from a tasklet, but =
if we
do not call bss_info_changed for these beacons there is no need for it =
to be
atomic. This function (bss_info_changed) is called either from workqueu=
e or
ioctl in all other instances.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Acked-by: Tomas Winkler <tomas.winkler@intel.com>
---
This patch applies to the latest code after mac80211 filename changes.

 include/net/mac80211.h |    3 +--
 net/mac80211/mlme.c    |   16 +++++++++++-----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 079e7bd..4a80d74 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1020,8 +1020,7 @@ enum ieee80211_ampdu_mlme_action {
  *	level driver (e.g. assoc/disassoc status, erp parameters).
  *	This function should not be used if no BSS has been set, unless
  *	for association indication. The @changed parameter indicates which
- *	of the bss parameters has changed when a call is made. This callbac=
k
- *	has to be atomic.
+ *	of the bss parameters has changed when a call is made.
  *
  * @configure_filter: Configure the device's RX filter.
  *	See the section "Frame filtering" for more information.
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 83e8b49..e3f2cb0 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2797,6 +2797,17 @@ static void ieee80211_rx_mgmt_beacon(struct net_=
device *dev,
=20
 	ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems=
);
=20
+	if (elems.wmm_param && (ifsta->flags & IEEE80211_STA_WMM_ENABLED)) {
+		ieee80211_sta_wmm_params(dev, ifsta, elems.wmm_param,
+					 elems.wmm_param_len);
+	}
+
+	/* Do not send changes to driver if we are scanning. This removes
+	 * requirement that driver's bss_info_changed function needs to be
+	 * atomic. */
+	if (local->sta_sw_scanning || local->sta_hw_scanning)
+		return;
+
 	if (elems.erp_info && elems.erp_info_len >=3D 1)
 		changed |=3D ieee80211_handle_erp_ie(sdata, elems.erp_info[0]);
 	else {
@@ -2816,11 +2827,6 @@ static void ieee80211_rx_mgmt_beacon(struct net_=
device *dev,
 					       &bss_info);
 	}
=20
-	if (elems.wmm_param && (ifsta->flags & IEEE80211_STA_WMM_ENABLED)) {
-		ieee80211_sta_wmm_params(dev, ifsta, elems.wmm_param,
-					 elems.wmm_param_len);
-	}
-
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
=20
--=20
1.5.3.4

--
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 RFC v2] mac80211: no BSS changes to driver from beacons processed during scanning

[Johannes Berg]

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

On Wed, 2008-04-09 at 16:56 -0700, Reinette Chatre wrote:
> There is no need to send BSS changes to driver from beacons processed
> during scanning. We are more interested in beacons from an AP with which
> we are associated - these will still be used to send updates to driver as
> the beacons are received without scanning.
> 
> This change·removes the requirement that bss_info_changed needs to be atomic.
> The beacons received during scanning are processed from a tasklet, but if we
> do not call bss_info_changed for these beacons there is no need for it to be
> atomic. This function (bss_info_changed) is called either from workqueue or
> ioctl in all other instances.

Seems reasonable. Makes me itch to rewrite the scan code completely (as
I had already planned once) so that the mlme isn't full of it, we don't
do everything twice (master and regular device) and the context (tasklet
vs. workqueue) is clearer. Oh well, not right now.

> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> Acked-by: Tomas Winkler <tomas.winkler@intel.com>

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
> This patch applies to the latest code after mac80211 filename changes.

Didn't think it would be much of a problem otherwise I wouldn't have
asked for it, was it?

>  include/net/mac80211.h |    3 +--
>  net/mac80211/mlme.c    |   16 +++++++++++-----
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 079e7bd..4a80d74 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1020,8 +1020,7 @@ enum ieee80211_ampdu_mlme_action {
>   *	level driver (e.g. assoc/disassoc status, erp parameters).
>   *	This function should not be used if no BSS has been set, unless
>   *	for association indication. The @changed parameter indicates which
> - *	of the bss parameters has changed when a call is made. This callback
> - *	has to be atomic.
> + *	of the bss parameters has changed when a call is made.
>   *
>   * @configure_filter: Configure the device's RX filter.
>   *	See the section "Frame filtering" for more information.
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 83e8b49..e3f2cb0 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -2797,6 +2797,17 @@ static void ieee80211_rx_mgmt_beacon(struct net_device *dev,
>  
>  	ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems);
>  
> +	if (elems.wmm_param && (ifsta->flags & IEEE80211_STA_WMM_ENABLED)) {
> +		ieee80211_sta_wmm_params(dev, ifsta, elems.wmm_param,
> +					 elems.wmm_param_len);
> +	}
> +
> +	/* Do not send changes to driver if we are scanning. This removes
> +	 * requirement that driver's bss_info_changed function needs to be
> +	 * atomic. */
> +	if (local->sta_sw_scanning || local->sta_hw_scanning)
> +		return;
> +
>  	if (elems.erp_info && elems.erp_info_len >= 1)
>  		changed |= ieee80211_handle_erp_ie(sdata, elems.erp_info[0]);
>  	else {
> @@ -2816,11 +2827,6 @@ static void ieee80211_rx_mgmt_beacon(struct net_device *dev,
>  					       &bss_info);
>  	}
>  
> -	if (elems.wmm_param && (ifsta->flags & IEEE80211_STA_WMM_ENABLED)) {
> -		ieee80211_sta_wmm_params(dev, ifsta, elems.wmm_param,
> -					 elems.wmm_param_len);
> -	}
> -
>  	ieee80211_bss_info_change_notify(sdata, changed);
>  }
>  

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

Re: [PATCH RFC v2] mac80211: no BSS changes to driver from beacons processed during scanning

[Johannes Berg]

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

On Wed, 2008-04-09 at 16:56 -0700, Reinette Chatre wrote:
> There is no need to send BSS changes to driver from beacons processed
> during scanning. We are more interested in beacons from an AP with which
> we are associated - these will still be used to send updates to driver as
> the beacons are received without scanning.
> 
> This change·removes the requirement that bss_info_changed needs to be atomic.
> The beacons received during scanning are processed from a tasklet, but if we
> do not call bss_info_changed for these beacons there is no need for it to be
> atomic. This function (bss_info_changed) is called either from workqueue or
> ioctl in all other instances.

Should we do the same for conf_tx, i.e. ieee80211_sta_wmm_params? I'm
not entirely sure the IBSS code doesn't still call it in a non-blocking
context though.

johannes

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