linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ath9k: Ensure a fair beacon distribution in IBSS mode.
@ 2009-11-24 11:53 Vivek Natarajan
  2009-11-24 11:53 ` [PATCH] mac80211: Fix dynamic power save for scanning Vivek Natarajan
  2009-11-25  4:42 ` [PATCH] ath9k: Ensure a fair beacon distribution in IBSS mode Sujith
  0 siblings, 2 replies; 7+ messages in thread
From: Vivek Natarajan @ 2009-11-24 11:53 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless

Update the beacon queue parameters with best effort queue parameters for
IBSS mode. This reduces the number of beacons generated by ath9k and
ensures a fair beacon distribution when there are multiple IBSS stations.
Also CWmin is quadrupled to achieve the expected percentage of
distribution.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h  |    1 +
 drivers/net/wireless/ath/ath9k/beacon.c |   14 +++++++++-----
 drivers/net/wireless/ath/ath9k/main.c   |    4 ++++
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index bc81401..c093928 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -330,6 +330,7 @@ void ath_beacon_tasklet(unsigned long data);
 void ath_beacon_config(struct ath_softc *sc, struct ieee80211_vif *vif);
 int ath_beacon_alloc(struct ath_wiphy *aphy, struct ieee80211_vif *vif);
 void ath_beacon_return(struct ath_softc *sc, struct ath_vif *avp);
+int ath_beaconq_config(struct ath_softc *sc);
 
 /*******/
 /* ANI */
diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index b10c884..0a9f9a1 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -23,11 +23,12 @@
  *  the operating mode of the station (AP or AdHoc).  Parameters are AIFS
  *  settings and channel width min/max
 */
-static int ath_beaconq_config(struct ath_softc *sc)
+int ath_beaconq_config(struct ath_softc *sc)
 {
 	struct ath_hw *ah = sc->sc_ah;
 	struct ath_common *common = ath9k_hw_common(ah);
-	struct ath9k_tx_queue_info qi;
+	struct ath9k_tx_queue_info qi, qi_be;
+	int qnum;
 
 	ath9k_hw_get_txq_props(ah, sc->beacon.beaconq, &qi);
 	if (sc->sc_ah->opmode == NL80211_IFTYPE_AP) {
@@ -37,9 +38,12 @@ static int ath_beaconq_config(struct ath_softc *sc)
 		qi.tqi_cwmax = 0;
 	} else {
 		/* Adhoc mode; important thing is to use 2x cwmin. */
-		qi.tqi_aifs = sc->beacon.beacon_qi.tqi_aifs;
-		qi.tqi_cwmin = 2*sc->beacon.beacon_qi.tqi_cwmin;
-		qi.tqi_cwmax = sc->beacon.beacon_qi.tqi_cwmax;
+		qnum = ath_tx_get_qnum(sc, ATH9K_TX_QUEUE_DATA,
+				       ATH9K_WME_AC_BE);
+		ath9k_hw_get_txq_props(ah, qnum, &qi_be);
+		qi.tqi_aifs = qi_be.tqi_aifs;
+		qi.tqi_cwmin = 4*qi_be.tqi_cwmin;
+		qi.tqi_cwmax = qi_be.tqi_cwmax;
 	}
 
 	if (!ath9k_hw_set_txq_props(ah, sc->beacon.beaconq, &qi)) {
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 68f9788..e95c645 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2914,6 +2914,10 @@ static int ath9k_conf_tx(struct ieee80211_hw *hw, u16 queue,
 	if (ret)
 		ath_print(common, ATH_DBG_FATAL, "TXQ Update failed\n");
 
+	if (sc->sc_ah->opmode == NL80211_IFTYPE_ADHOC)
+		if (qnum == sc->tx.hwq_map[ATH9K_WME_AC_BE])
+			ath_beaconq_config(sc);
+
 	mutex_unlock(&sc->mutex);
 
 	return ret;
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] mac80211: Fix dynamic power save for scanning.
  2009-11-24 11:53 [PATCH] ath9k: Ensure a fair beacon distribution in IBSS mode Vivek Natarajan
@ 2009-11-24 11:53 ` Vivek Natarajan
  2009-11-24 11:53   ` [PATCH] cfg80211: Clear encryption privacy when key off is done Vivek Natarajan
  2009-11-26  8:07   ` [PATCH] mac80211: Fix dynamic power save for scanning Kalle Valo
  2009-11-25  4:42 ` [PATCH] ath9k: Ensure a fair beacon distribution in IBSS mode Sujith
  1 sibling, 2 replies; 7+ messages in thread
From: Vivek Natarajan @ 2009-11-24 11:53 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless

Not only ps_sdata but also IEEE80211_CONF_PS is to be considered
before restoring PS in scan_ps_disable(). For instance, when ps_sdata
is set but CONF_PS is not set just because the dynamic timer is still
running, a sw scan leads to setting of CONF_PS in scan_ps_disable
instead of starting the dynamic PS timer.
Also for the above case, a null data frame is to be sent after
returning to operating channel which was not happening with the
current implementation. This patch fixes this too.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/scan.c        |   14 ++++++++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ab28942..57bef7c 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -746,6 +746,7 @@ struct ieee80211_local {
 	unsigned int wmm_acm; /* bit field of ACM bits (BIT(802.1D tag)) */
 
 	bool pspolling;
+	bool scan_ps_enabled;
 	/*
 	 * PS can only be enabled when we have exactly one managed
 	 * interface (and monitors) in PS, this then points there.
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 4cf387c..41076e4 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -227,7 +227,8 @@ static bool ieee80211_prep_hw_scan(struct ieee80211_local *local)
 static void ieee80211_scan_ps_enable(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_local *local = sdata->local;
-	bool ps = false;
+
+	local->scan_ps_enabled = false;
 
 	/* FIXME: what to do when local->pspolling is true? */
 
@@ -235,12 +236,13 @@ static void ieee80211_scan_ps_enable(struct ieee80211_sub_if_data *sdata)
 	cancel_work_sync(&local->dynamic_ps_enable_work);
 
 	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
-		ps = true;
+		local->scan_ps_enabled = true;
 		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
 		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
 	}
 
-	if (!ps || !(local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK))
+	if (!(local->scan_ps_enabled) ||
+	    !(local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK))
 		/*
 		 * If power save was enabled, no need to send a nullfunc
 		 * frame because AP knows that we are sleeping. But if the
@@ -261,7 +263,7 @@ static void ieee80211_scan_ps_disable(struct ieee80211_sub_if_data *sdata)
 
 	if (!local->ps_sdata)
 		ieee80211_send_nullfunc(local, sdata, 0);
-	else {
+	else if (local->scan_ps_enabled) {
 		/*
 		 * In !IEEE80211_HW_PS_NULLFUNC_STACK case the hardware
 		 * will send a nullfunc frame with the powersave bit set
@@ -277,6 +279,10 @@ static void ieee80211_scan_ps_disable(struct ieee80211_sub_if_data *sdata)
 		 */
 		local->hw.conf.flags |= IEEE80211_CONF_PS;
 		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	} else {
+		ieee80211_send_nullfunc(local, sdata, 0);
+		mod_timer(&local->dynamic_ps_timer, jiffies +
+			  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
 	}
 }
 
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] cfg80211: Clear encryption privacy when key off is done.
  2009-11-24 11:53 ` [PATCH] mac80211: Fix dynamic power save for scanning Vivek Natarajan
@ 2009-11-24 11:53   ` Vivek Natarajan
  2009-11-26  8:07   ` [PATCH] mac80211: Fix dynamic power save for scanning Kalle Valo
  1 sibling, 0 replies; 7+ messages in thread
From: Vivek Natarajan @ 2009-11-24 11:53 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless

When the current_bss is not set, 'iwconfig <iface> key off' does not
clear the private flag. Hence after we connect with WEP to an AP and
then try to connect with another non-WEP AP, it does not work.
This issue will not be seen if supplicant is used.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
 net/wireless/wext-compat.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 29091ac..000df52 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -479,6 +479,7 @@ static int __cfg80211_set_encryption(struct cfg80211_registered_device *rdev,
 			}
 			err = rdev->ops->del_key(&rdev->wiphy, dev, idx, addr);
 		}
+		wdev->wext.connect.privacy = false;
 		/*
 		 * Applications using wireless extensions expect to be
 		 * able to delete keys that don't exist, so allow that.
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] ath9k: Ensure a fair beacon distribution in IBSS mode.
  2009-11-24 11:53 [PATCH] ath9k: Ensure a fair beacon distribution in IBSS mode Vivek Natarajan
  2009-11-24 11:53 ` [PATCH] mac80211: Fix dynamic power save for scanning Vivek Natarajan
@ 2009-11-25  4:42 ` Sujith
  1 sibling, 0 replies; 7+ messages in thread
From: Sujith @ 2009-11-25  4:42 UTC (permalink / raw)
  To: Vivek Natarajan; +Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org

Vivek Natarajan wrote:
> @@ -2914,6 +2914,10 @@ static int ath9k_conf_tx(struct ieee80211_hw *hw, u16 queue,
>         if (ret)
>                 ath_print(common, ATH_DBG_FATAL, "TXQ Update failed\n");
> 
> +       if (sc->sc_ah->opmode == NL80211_IFTYPE_ADHOC)
> +               if (qnum == sc->tx.hwq_map[ATH9K_WME_AC_BE])
> +                       ath_beaconq_config(sc);
> +

If queue parameter updation has failed earlier, this code chunk would be
redundant. Handling the special case for IBSS should probably be done only if
the earlier ath_txq_update() is successful.

Sujith

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mac80211: Fix dynamic power save for scanning.
  2009-11-24 11:53 ` [PATCH] mac80211: Fix dynamic power save for scanning Vivek Natarajan
  2009-11-24 11:53   ` [PATCH] cfg80211: Clear encryption privacy when key off is done Vivek Natarajan
@ 2009-11-26  8:07   ` Kalle Valo
  2009-11-26 11:09     ` Vivek Natarajan
  1 sibling, 1 reply; 7+ messages in thread
From: Kalle Valo @ 2009-11-26  8:07 UTC (permalink / raw)
  To: Vivek Natarajan; +Cc: linville, linux-wireless

Vivek Natarajan <vnatarajan@atheros.com> writes:

> Not only ps_sdata but also IEEE80211_CONF_PS is to be considered
> before restoring PS in scan_ps_disable(). For instance, when ps_sdata
> is set but CONF_PS is not set just because the dynamic timer is still
> running, a sw scan leads to setting of CONF_PS in scan_ps_disable
> instead of starting the dynamic PS timer.

Good that you found this. Some comments about the fix, though.

> @@ -746,6 +746,7 @@ struct ieee80211_local {
>  	unsigned int wmm_acm; /* bit field of ACM bits (BIT(802.1D tag)) */
>  
>  	bool pspolling;
> +	bool scan_ps_enabled;

I don't like the idea of adding more state variables to
ieee80211_local. We should be removing them instead because the code
is getting quite complex. Is there any way to fix this but not add a
new variable?

> @@ -277,6 +279,10 @@ static void ieee80211_scan_ps_disable(struct ieee80211_sub_if_data *sdata)
>  		 */
>  		local->hw.conf.flags |= IEEE80211_CONF_PS;
>  		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
> +	} else {
> +		ieee80211_send_nullfunc(local, sdata, 0);
> +		mod_timer(&local->dynamic_ps_timer, jiffies +
> +			  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
>  	}
>  }

You need to take into account that dynamic_ps_timeout can be zero.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mac80211: Fix dynamic power save for scanning.
  2009-11-26  8:07   ` [PATCH] mac80211: Fix dynamic power save for scanning Kalle Valo
@ 2009-11-26 11:09     ` Vivek Natarajan
  2009-11-27 11:20       ` Kalle Valo
  0 siblings, 1 reply; 7+ messages in thread
From: Vivek Natarajan @ 2009-11-26 11:09 UTC (permalink / raw)
  To: Kalle Valo; +Cc: Vivek Natarajan, linville, linux-wireless

On Thu, Nov 26, 2009 at 1:37 PM, Kalle Valo <kalle.valo@iki.fi> wrote:
> Vivek Natarajan <vnatarajan@atheros.com> writes:
>> @@ -746,6 +746,7 @@ struct ieee80211_local {
>>       unsigned int wmm_acm; /* bit field of ACM bits (BIT(802.1D tag)) */
>>
>>       bool pspolling;
>> +     bool scan_ps_enabled;
>
> I don't like the idea of adding more state variables to
> ieee80211_local. We should be removing them instead because the code
> is getting quite complex. Is there any way to fix this but not add a
> new variable?
>

In this case, I need a variable to store the PS state before scan so
that I can restore the same state when we come back to home channel.

There are only two variables relating to this context:
the flag IEEE80211_CONF_PS and ps_sdata
CONF_PS flag is getting changed in scan_ps_enable.
After association, ps_sdata is always set since power save is enabled
by default.
This will be changed if the power save is disabled through iwconfig
which may happen during scan too. Hence I cannot use this too.
So, I had to introduce a new variable.

>> @@ -277,6 +279,10 @@ static void ieee80211_scan_ps_disable(struct ieee80211_sub_if_data *sdata)
>>                */
>>               local->hw.conf.flags |= IEEE80211_CONF_PS;
>>               ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
>> +     } else {
>> +             ieee80211_send_nullfunc(local, sdata, 0);
>> +             mod_timer(&local->dynamic_ps_timer, jiffies +
>> +                       msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
>>       }
>>  }
>
> You need to take into account that dynamic_ps_timeout can be zero.

When dynamic_ps_timeout is zero, as soon as the station is associated,
CONF_PS will be set and communicated to the driver.
If a scan request comes in this period, there will be a issue with my
fix. I think this is the case you are pointing at. If so, I think the
following chunk will work:

@@ -277,6 +279,10 @@ static void ieee80211_scan_ps_disable(struct
ieee80211_sub_if_data *sdata)
                */
               local->hw.conf.flags |= IEEE80211_CONF_PS;
               ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+       } else if (local->hw.conf.dynamic_ps_timeout > 0){
+               ieee80211_send_nullfunc(local, sdata, 0);
+               mod_timer(&local->dynamic_ps_timer, jiffies +
+                         msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
       }
 }


Vivek.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mac80211: Fix dynamic power save for scanning.
  2009-11-26 11:09     ` Vivek Natarajan
@ 2009-11-27 11:20       ` Kalle Valo
  0 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2009-11-27 11:20 UTC (permalink / raw)
  To: Vivek Natarajan; +Cc: Vivek Natarajan, linville, linux-wireless

Vivek Natarajan <vivek.natraj@gmail.com> writes:

> On Thu, Nov 26, 2009 at 1:37 PM, Kalle Valo <kalle.valo@iki.fi> wrote:
>> Vivek Natarajan <vnatarajan@atheros.com> writes:
>>
>> I don't like the idea of adding more state variables to
>> ieee80211_local. We should be removing them instead because the code
>> is getting quite complex. Is there any way to fix this but not add a
>> new variable?
>
> In this case, I need a variable to store the PS state before scan so
> that I can restore the same state when we come back to home channel.
>
> There are only two variables relating to this context:
> the flag IEEE80211_CONF_PS and ps_sdata
> CONF_PS flag is getting changed in scan_ps_enable.
> After association, ps_sdata is always set since power save is enabled
> by default.
> This will be changed if the power save is disabled through iwconfig
> which may happen during scan too. Hence I cannot use this too.
> So, I had to introduce a new variable.

Ok, we have to live with this then.

>> You need to take into account that dynamic_ps_timeout can be zero.
>
> When dynamic_ps_timeout is zero, as soon as the station is associated,
> CONF_PS will be set and communicated to the driver.
> If a scan request comes in this period, there will be a issue with my
> fix. I think this is the case you are pointing at. If so, I think the
> following chunk will work:
>
> @@ -277,6 +279,10 @@ static void ieee80211_scan_ps_disable(struct
> ieee80211_sub_if_data *sdata)
>                 */
>                local->hw.conf.flags |= IEEE80211_CONF_PS;
>                ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
> +       } else if (local->hw.conf.dynamic_ps_timeout > 0){
> +               ieee80211_send_nullfunc(local, sdata, 0);
> +               mod_timer(&local->dynamic_ps_timer, jiffies +
> +                         msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));

Yes, this should be enough. Please also add a comment why we send a
nullfunc frame.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-11-27 11:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-24 11:53 [PATCH] ath9k: Ensure a fair beacon distribution in IBSS mode Vivek Natarajan
2009-11-24 11:53 ` [PATCH] mac80211: Fix dynamic power save for scanning Vivek Natarajan
2009-11-24 11:53   ` [PATCH] cfg80211: Clear encryption privacy when key off is done Vivek Natarajan
2009-11-26  8:07   ` [PATCH] mac80211: Fix dynamic power save for scanning Kalle Valo
2009-11-26 11:09     ` Vivek Natarajan
2009-11-27 11:20       ` Kalle Valo
2009-11-25  4:42 ` [PATCH] ath9k: Ensure a fair beacon distribution in IBSS mode Sujith

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).