We would like to include the following two patches as fixes for 2.6.29.
The first patch is of particular interest as it fixes the "rs_get_rate"
issue that is receiving a lot of attention from kerneloops.org. We will
post a patch to [email protected] when this one has been accepted into
Linus's tree.
[PATCH 1/2] iwlwifi: fix rs_get_rate WARN_ON()
[PATCH 2/2] iwlagn: fix agn rate scaling
Thank you
Reinette
On Fri, 2009-01-23 at 11:01 -0800, John W. Linville wrote:
> On Wed, Jan 21, 2009 at 10:58:03AM -0800, Reinette Chatre wrote:
> > From: Abbas, Mohamed <[email protected]>
> >
> > Sometime Tx reply rate different than what rate scale expecting
> > causing rate scale to bail out. This could cause failing to
> > commit LQ cmd. This patch will try to solve this instead of just
> > bail out. It also make sure we start with a valid rate.
> >
> > Signed-off-by: mohamed abbas <[email protected]>
> > Signed-off-by: Reinette Chatre <[email protected]>
>
> While I'm sure this would be nice to fix, I don't see the regression,
> Oops or crash?
You are right - this does not fix an oops or a crash. Having this patch
in 2.6.29 is thus not critical.
Reinette
From: Abbas, Mohamed <[email protected]>
In ieee80211_sta structure there is u64 supp_rates[IEEE80211_NUM_BANDS]
this is filled with all support rate from assoc_resp. If we associate
with G-band AP only supp_rates of G-band will be set the other band
supp_rates will be set to 0. If the user type this command
this will cause mac80211 to set to new channel, mac80211
does not disassociate in setting new channel, so the active
band is now A-band. then in handling the new essid mac80211 will
kick in the assoc steps which involve sending disassociation frame.
in this mac80211 will WARN_ON sta->supp_rates[A_BAND] == 0.
This fixes:
http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=1822
http://www.kerneloops.org/searchweek.php?search=rs_get_rate
Signed-off-by: mohamed abbas <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-3945-rs.c | 14 +++++++++++---
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 14 ++++++++++++--
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
index 9b60a0c..21c8418 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
@@ -638,12 +638,16 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
s8 scale_action = 0;
unsigned long flags;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
- u16 fc, rate_mask;
+ u16 fc;
+ u16 rate_mask = 0;
struct iwl3945_priv *priv = (struct iwl3945_priv *)priv_r;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
IWL_DEBUG_RATE("enter\n");
+ if (sta)
+ rate_mask = sta->supp_rates[sband->band];
+
/* Send management frames and broadcast/multicast data using lowest
* rate. */
fc = le16_to_cpu(hdr->frame_control);
@@ -651,11 +655,15 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
is_multicast_ether_addr(hdr->addr1) ||
!sta || !priv_sta) {
IWL_DEBUG_RATE("leave: No STA priv data to update!\n");
- info->control.rates[0].idx = rate_lowest_index(sband, sta);
+ if (!rate_mask)
+ info->control.rates[0].idx =
+ rate_lowest_index(sband, NULL);
+ else
+ info->control.rates[0].idx =
+ rate_lowest_index(sband, sta);
return;
}
- rate_mask = sta->supp_rates[sband->band];
index = min(rs_sta->last_txrate_idx & 0xffff, IWL_RATE_COUNT - 1);
if (sband->band == IEEE80211_BAND_5GHZ)
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
index f3f1792..27f5047 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
@@ -944,7 +944,8 @@ static void rs_tx_status(void *priv_r, struct ieee80211_supported_band *sband,
}
/* See if there's a better rate or modulation mode to try. */
- rs_rate_scale_perform(priv, hdr, sta, lq_sta);
+ if (sta && sta->supp_rates[sband->band])
+ rs_rate_scale_perform(priv, hdr, sta, lq_sta);
out:
return;
}
@@ -2101,14 +2102,23 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta, void *priv_sta,
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct iwl_lq_sta *lq_sta = priv_sta;
int rate_idx;
+ u64 mask_bit = 0;
IWL_DEBUG_RATE_LIMIT("rate scale calculate new rate for skb\n");
+ if (sta)
+ mask_bit = sta->supp_rates[sband->band];
+
/* Send management frames and broadcast/multicast data using lowest
* rate. */
if (!ieee80211_is_data(hdr->frame_control) ||
is_multicast_ether_addr(hdr->addr1) || !sta || !lq_sta) {
- info->control.rates[0].idx = rate_lowest_index(sband, sta);
+ if (!mask_bit)
+ info->control.rates[0].idx =
+ rate_lowest_index(sband, NULL);
+ else
+ info->control.rates[0].idx =
+ rate_lowest_index(sband, sta);
return;
}
--
1.5.4.3
On Wed, 2009-01-21 at 10:58 -0800, Reinette Chatre wrote:
> From: Abbas, Mohamed <[email protected]>
>
> In ieee80211_sta structure there is u64 supp_rates[IEEE80211_NUM_BANDS]
> this is filled with all support rate from assoc_resp. If we associate
> with G-band AP only supp_rates of G-band will be set the other band
> supp_rates will be set to 0. If the user type this command
> this will cause mac80211 to set to new channel, mac80211
> does not disassociate in setting new channel, so the active
> band is now A-band. then in handling the new essid mac80211 will
> kick in the assoc steps which involve sending disassociation frame.
> in this mac80211 will WARN_ON sta->supp_rates[A_BAND] == 0.
Sounds like we should fix mac80211 to disassociate _before_ switching
the channel? It won't help anyway if we're on another channel?
johannes
On Fri, Jan 23, 2009 at 9:28 PM, reinette chatre
<[email protected]> wrote:
> On Fri, 2009-01-23 at 11:01 -0800, John W. Linville wrote:
>> On Wed, Jan 21, 2009 at 10:58:03AM -0800, Reinette Chatre wrote:
>> > From: Abbas, Mohamed <[email protected]>
>> >
>> > Sometime Tx reply rate different than what rate scale expecting
>> > causing rate scale to bail out. This could cause failing to
>> > commit LQ cmd. This patch will try to solve this instead of just
>> > bail out. It also make sure we start with a valid rate.
>> >
>> > Signed-off-by: mohamed abbas <[email protected]>
>> > Signed-off-by: Reinette Chatre <[email protected]>
>>
>> While I'm sure this would be nice to fix, I don't see the regression,
>> Oops or crash?
>
> You are right - this does not fix an oops or a crash. Having this patch
> in 2.6.29 is thus not critical.
What is funny that we have to wait till 2.6.30 is open to get it to
stable of 2.6.27 and 28.
Tomas
On Wed, Jan 21, 2009 at 10:58:03AM -0800, Reinette Chatre wrote:
> From: Abbas, Mohamed <[email protected]>
>
> Sometime Tx reply rate different than what rate scale expecting
> causing rate scale to bail out. This could cause failing to
> commit LQ cmd. This patch will try to solve this instead of just
> bail out. It also make sure we start with a valid rate.
>
> Signed-off-by: mohamed abbas <[email protected]>
> Signed-off-by: Reinette Chatre <[email protected]>
While I'm sure this would be nice to fix, I don't see the regression,
Oops or crash?
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On Fri, Jan 23, 2009 at 09:55:34PM +0200, Tomas Winkler wrote:
> On Fri, Jan 23, 2009 at 9:28 PM, reinette chatre
> <[email protected]> wrote:
> > On Fri, 2009-01-23 at 11:01 -0800, John W. Linville wrote:
> >> On Wed, Jan 21, 2009 at 10:58:03AM -0800, Reinette Chatre wrote:
> >> > From: Abbas, Mohamed <[email protected]>
> >> >
> >> > Sometime Tx reply rate different than what rate scale expecting
> >> > causing rate scale to bail out. This could cause failing to
> >> > commit LQ cmd. This patch will try to solve this instead of just
> >> > bail out. It also make sure we start with a valid rate.
> >> >
> >> > Signed-off-by: mohamed abbas <[email protected]>
> >> > Signed-off-by: Reinette Chatre <[email protected]>
> >>
> >> While I'm sure this would be nice to fix, I don't see the regression,
> >> Oops or crash?
> >
> > You are right - this does not fix an oops or a crash. Having this patch
> > in 2.6.29 is thus not critical.
>
> What is funny that we have to wait till 2.6.30 is open to get it to
> stable of 2.6.27 and 28.
Not my rule -- ask Greg for special dispensation?
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
From: Abbas, Mohamed <[email protected]>
Sometime Tx reply rate different than what rate scale expecting
causing rate scale to bail out. This could cause failing to
commit LQ cmd. This patch will try to solve this instead of just
bail out. It also make sure we start with a valid rate.
Signed-off-by: mohamed abbas <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 31 +++++++++++++++++++++-------
1 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
index 27f5047..720a9d6 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
@@ -49,6 +49,8 @@
#define IWL_RATE_MIN_FAILURE_TH 6 /* min failures to calc tpt */
#define IWL_RATE_MIN_SUCCESS_TH 8 /* min successes to calc tpt */
+/* max allowed rate miss before sync LQ cmd */
+#define IWL_MISSED_RATE_MAX 15
/* max time to accum history 2 seconds */
#define IWL_RATE_SCALE_FLUSH_INTVL (2*HZ)
@@ -148,6 +150,7 @@ struct iwl_lq_sta {
u16 active_mimo2_rate;
u16 active_mimo3_rate;
u16 active_rate_basic;
+ u8 missed_rate_counter;
struct iwl_link_quality_cmd lq;
struct iwl_scale_tbl_info lq_info[LQ_SIZE]; /* "active", "search" */
@@ -839,10 +842,15 @@ static void rs_tx_status(void *priv_r, struct ieee80211_supported_band *sband,
/* the last LQ command could failed so the LQ in ucode not
* the same in driver sync up
*/
- iwl_send_lq_cmd(priv, &lq_sta->lq, CMD_ASYNC);
+ lq_sta->missed_rate_counter++;
+ if (lq_sta->missed_rate_counter > IWL_MISSED_RATE_MAX) {
+ lq_sta->missed_rate_counter = 0;
+ iwl_send_lq_cmd(priv, &lq_sta->lq, CMD_ASYNC);
+ }
goto out;
}
+ lq_sta->missed_rate_counter = 0;
/* Update frame history window with "failure" for each Tx retry. */
while (retries) {
/* Look up the rate and other info used for each tx attempt.
@@ -2182,6 +2190,8 @@ static void rs_rate_init(void *priv_r, struct ieee80211_supported_band *sband,
struct ieee80211_conf *conf = &priv->hw->conf;
struct iwl_lq_sta *lq_sta = priv_sta;
u16 mask_bit = 0;
+ int count;
+ int start_rate = 0;
lq_sta->flush_timer = 0;
lq_sta->supp_rates = sta->supp_rates[sband->band];
@@ -2216,6 +2226,7 @@ static void rs_rate_init(void *priv_r, struct ieee80211_supported_band *sband,
}
lq_sta->is_dup = 0;
+ lq_sta->missed_rate_counter = IWL_MISSED_RATE_MAX;
lq_sta->is_green = rs_use_green(priv, conf);
lq_sta->active_legacy_rate = priv->active_rate & ~(0x1000);
lq_sta->active_rate_basic = priv->active_rate_basic;
@@ -2254,16 +2265,20 @@ static void rs_rate_init(void *priv_r, struct ieee80211_supported_band *sband,
lq_sta->drv = priv;
/* Find highest tx rate supported by hardware and destination station */
- mask_bit = sta->supp_rates[sband->band] & lq_sta->active_legacy_rate;
- lq_sta->last_txrate_idx = 3;
- for (i = 0; i < sband->n_bitrates; i++)
+ mask_bit = sta->supp_rates[sband->band];
+ count = sband->n_bitrates;
+ if (sband->band == IEEE80211_BAND_5GHZ) {
+ count += IWL_FIRST_OFDM_RATE;
+ start_rate = IWL_FIRST_OFDM_RATE;
+ mask_bit <<= IWL_FIRST_OFDM_RATE;
+ }
+
+ mask_bit = mask_bit & lq_sta->active_legacy_rate;
+ lq_sta->last_txrate_idx = 4;
+ for (i = start_rate; i < count; i++)
if (mask_bit & BIT(i))
lq_sta->last_txrate_idx = i;
- /* For MODE_IEEE80211A, skip over cck rates in global rate table */
- if (sband->band == IEEE80211_BAND_5GHZ)
- lq_sta->last_txrate_idx += IWL_FIRST_OFDM_RATE;
-
rs_initialize_lq(priv, conf, sta, lq_sta);
}
--
1.5.4.3
-----Original Message-----
From: Johannes Berg [mailto:[email protected]]
Sent: Thursday, January 22, 2009 2:58 AM
To: Chatre, Reinette
Cc: [email protected]; [email protected]; [email protected]; Abbas, Mohamed
Subject: Re: [PATCH 1/2 v2.6.29] iwlwifi: fix rs_get_rate WARN_ON()
On Wed, 2009-01-21 at 10:58 -0800, Reinette Chatre wrote:
> From: Abbas, Mohamed <[email protected]>
>
> In ieee80211_sta structure there is u64 supp_rates[IEEE80211_NUM_BANDS]
> this is filled with all support rate from assoc_resp. If we associate
> with G-band AP only supp_rates of G-band will be set the other band
> supp_rates will be set to 0. If the user type this command
> this will cause mac80211 to set to new channel, mac80211
> does not disassociate in setting new channel, so the active
> band is now A-band. then in handling the new essid mac80211 will
> kick in the assoc steps which involve sending disassociation frame.
> in this mac80211 will WARN_ON sta->supp_rates[A_BAND] == 0.
Sounds like we should fix mac80211 to disassociate _before_ switching
the channel? It won't help anyway if we're on another channel?
[mabbas]
I totally agree, all the disassociation frame will happen in the new channel that AP wont hear.
Mohamed
johannes
On Thu, 2009-01-22 at 08:51 -0800, Abbas, Mohamed wrote:
> Sounds like we should fix mac80211 to disassociate _before_ switching
> the channel? It won't help anyway if we're on another channel?
> [mabbas]
> I totally agree, all the disassociation frame will happen in the new channel that AP wont hear.
So any idea how to fix it? I'm not all that familiar with the MLME.
johannes