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 <[email protected]>
---
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
On Thu, Nov 26, 2009 at 1:37 PM, Kalle Valo <[email protected]> wrote:
> Vivek Natarajan <[email protected]> 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.
Vivek Natarajan <[email protected]> writes:
> On Thu, Nov 26, 2009 at 1:37 PM, Kalle Valo <[email protected]> wrote:
>> Vivek Natarajan <[email protected]> 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
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
Vivek Natarajan <[email protected]> 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
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 <[email protected]>
---
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
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 <[email protected]>
---
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