2012-06-07 06:07:45

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: [PATCH] mac80211: ratelimit few aggregated messages

From: Mohammed Shafi Shajakhan <[email protected]>

ratelimit few aggregation related messages, these messages
floods the log when aggregation is disabled in the AP(for some
wifi testcases) and we run traffic between STA and AP.
mac80211 gives up after 15 addba requests, but
ieee80211_start_tx_ba_session will be repeatedly called
by drivers rate control tx_status callback. These net_dbg_ratelimited
messages will be visible only when dynamic debug is enabled for
mac80211 module.

Cc: Joe Perches <[email protected]>
Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
net/mac80211/agg-tx.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index da07f01..c2dc766 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -449,8 +449,11 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
(local->hw.flags & IEEE80211_HW_TX_AMPDU_SETUP_IN_HW))
return -EINVAL;

- ht_vdbg("Open BA session requested for %pM tid %u\n",
- pubsta->addr, tid);
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ net_dbg_ratelimited("Open BA session requested for %pM tid %u\n",
+ pubsta->addr, tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+

if (sdata->vif.type != NL80211_IFTYPE_STATION &&
sdata->vif.type != NL80211_IFTYPE_MESH_POINT &&
@@ -485,8 +488,11 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,

spin_lock_bh(&sta->lock);

- /* we have tried too many times, receiver does not want A-MPDU */
if (sta->ampdu_mlme.addba_req_num[tid] > HT_AGG_MAX_RETRIES) {
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ net_dbg_ratelimited("receiver does not wants A-MPDU, maximum addba requests tried for tid %u\n",
+ tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
ret = -EBUSY;
goto err_unlock_sta;
}
@@ -499,8 +505,10 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
if (sta->ampdu_mlme.addba_req_num[tid] > HT_AGG_BURST_RETRIES &&
time_before(jiffies, sta->ampdu_mlme.last_addba_req_time[tid] +
HT_AGG_RETRIES_PERIOD)) {
- ht_vdbg("BA request denied - waiting a grace period after %d failed requests on tid %u\n",
- sta->ampdu_mlme.addba_req_num[tid], tid);
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ net_dbg_ratelimited("BA request denied - waiting a grace period after %d failed requests on tid %u\n",
+ sta->ampdu_mlme.addba_req_num[tid], tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
ret = -EBUSY;
goto err_unlock_sta;
}
--
1.7.0.4



2012-06-07 07:11:14

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

Hi Joe,

On Thursday 07 June 2012 11:55 AM, Joe Perches wrote:
> On Thu, 2012-06-07 at 11:32 +0530, Mohammed Shafi Shajakhan wrote:
>> From: Mohammed Shafi Shajakhan<[email protected]>
>>
>> ratelimit few aggregation related messages, these messages
>> floods the log when aggregation is disabled in the AP(for some
>> wifi testcases) and we run traffic between STA and AP.
>
> I looked at all of the #ifdef CONFIG_MAC80211_HT_DEBUG uses.
>
> I think it's better simply to define a mac80211_ht_dbg
> macro for each of the !CONFIG and CONFIG cases and convert
> all of the other #ifdef CONFIG_MAC80211_HT_DEBUG logging
> messages.

oh, ok.

>
> #ifdef CONFIG_MAC80211_HT_DEBUG
> #define mac80211_ht_dbg(fmt, ...) \
> net_dbg_ratelimited(fmt, ##__VA_ARGS__)
> #else
> #define mac80211_ht_dbg(fmt, ...) \
> do { \
> if (0) \
> net_dbg_ratelimited(fmt, ##__VA_ARGS__); \
> } while (0)
>
> etc...
>
> I think there's one use of wiphy_dbg that could be a
> mac80211_ht_dbg without much loss.
>
> Maybe add "%s", wiphy_name() to the args there.
>

sorry, i could not understand with wiphy_debug, with mac80211_ht_debug.
the later is for HT and aggregation related messages.

--
thanks,
shafi

2012-06-13 04:59:30

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

Hi Joe,

On Friday 08 June 2012 01:22 AM, Joe Perches wrote:
> On Thu, 2012-06-07 at 14:08 +0530, Mohammed Shafi Shajakhan wrote:
>> On Thursday 07 June 2012 01:27 PM, Joe Perches wrote:
>>> On Thu, 2012-06-07 at 13:07 +0530, Mohammed Shafi Shajakhan wrote:
>>>> what about the other wiphy_debug messages, we would
>>>> still retain them know
>>>
>>> Yes, and...
>>>
>>> No, you know those should now still be wiphy_debug
>>> unless the message is HT related and need a new change.
>>
>> sure. so lets drop this patch and do that generic stuff.
>
> I did a little while ago.
> Please modify the patch based on wireless-next commit d63e9ae3b12

sorry for the delayed reply. so you had already posted the change you
had suggested in ([email protected] mailing list ?) and i have to
do any changes based on that patch right ? thank you!


--
thanks,
shafi

2012-06-07 06:16:14

by Sujith Manoharan

[permalink] [raw]
Subject: [PATCH] mac80211: ratelimit few aggregated messages

Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <[email protected]>
>
> ratelimit few aggregation related messages, these messages
> floods the log when aggregation is disabled in the AP(for some
> wifi testcases) and we run traffic between STA and AP.
> mac80211 gives up after 15 addba requests, but
> ieee80211_start_tx_ba_session will be repeatedly called
> by drivers rate control tx_status callback. These net_dbg_ratelimited
> messages will be visible only when dynamic debug is enabled for
> mac80211 module.

I think the drivers need to be fixed. For ath9k, we probably need
an intermediate state to track whether an ADDBA request is pending.

Sujith

2012-06-07 08:38:46

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

On Thursday 07 June 2012 01:27 PM, Joe Perches wrote:
> On Thu, 2012-06-07 at 13:07 +0530, Mohammed Shafi Shajakhan wrote:
>> what about the other wiphy_debug messages, we would
>> still retain them know
>
> Yes, and...
>
> No, you know those should now still be wiphy_debug
> unless the message is HT related and need a new change.

sure. so lets drop this patch and do that generic stuff.

>
> All sorts of kneed/need, knitting/nitting bits are in
> my head right now, but I'll spare all of you any more.
>
> English is still a spelt-silly language.


:)

>
> cheers, Joe
>


--
thanks,
shafi

2012-06-07 19:52:55

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

On Thu, 2012-06-07 at 14:08 +0530, Mohammed Shafi Shajakhan wrote:
> On Thursday 07 June 2012 01:27 PM, Joe Perches wrote:
> > On Thu, 2012-06-07 at 13:07 +0530, Mohammed Shafi Shajakhan wrote:
> >> what about the other wiphy_debug messages, we would
> >> still retain them know
> >
> > Yes, and...
> >
> > No, you know those should now still be wiphy_debug
> > unless the message is HT related and need a new change.
>
> sure. so lets drop this patch and do that generic stuff.

I did a little while ago.
Please modify the patch based on wireless-next commit d63e9ae3b12

cheers, Joe


2012-06-14 04:38:02

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

On Thursday 14 June 2012 06:43 AM, Joe Perches wrote:
> On Wed, 2012-06-13 at 10:29 +0530, Mohammed Shafi Shajakhan wrote:
>> On Friday 08 June 2012 01:22 AM, Joe Perches wrote:
>>> Please modify the patch based on wireless-next commit d63e9ae3b12
>> sorry for the delayed reply. so you had already posted the change you
>> had suggested in ([email protected] mailing list ?) and i have to
>> do any changes based on that patch right ? thank you!
>
> Yes, pull wireless-next and start from there.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next.git
> http://git.kernel.org/?p=linux/kernel/git/linville/wireless-next.git;a=summary
>

thanks Joe!

--
thanks,
shafi

2012-06-14 01:13:46

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

On Wed, 2012-06-13 at 10:29 +0530, Mohammed Shafi Shajakhan wrote:
> On Friday 08 June 2012 01:22 AM, Joe Perches wrote:
> > Please modify the patch based on wireless-next commit d63e9ae3b12
> sorry for the delayed reply. so you had already posted the change you
> had suggested in ([email protected] mailing list ?) and i have to
> do any changes based on that patch right ? thank you!

Yes, pull wireless-next and start from there.

git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next.git
http://git.kernel.org/?p=linux/kernel/git/linville/wireless-next.git;a=summary

cheers, Joe


2012-06-07 07:57:17

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

On Thu, 2012-06-07 at 13:07 +0530, Mohammed Shafi Shajakhan wrote:
> what about the other wiphy_debug messages, we would
> still retain them know

Yes, and...

No, you know those should now still be wiphy_debug
unless the message is HT related and need a new change.

All sorts of kneed/need, knitting/nitting bits are in
my head right now, but I'll spare all of you any more.

English is still a spelt-silly language.

cheers, Joe


2012-06-07 07:37:19

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

Hi Joe,

>>>
>>> I think there's one use of wiphy_dbg that could be a
>>> mac80211_ht_dbg without much loss.
>>>
>>> Maybe add "%s", wiphy_name() to the args there.
>>>
>>
>> sorry, i could not understand with wiphy_debug, with mac80211_ht_debug.
>> the later is for HT and aggregation related messages.
>
> It was an automatic conversion.
>
> Might as well unconvert it back too.
>
> $ git grep -E -A4 -n "ifdef CONFIG_MAC80211_HT_DEBUG" net/mac80211/rx.c
> net/mac80211/rx.c:635:#ifdef CONFIG_MAC80211_HT_DEBUG
> net/mac80211/rx.c-636- if (net_ratelimit())
> net/mac80211/rx.c-637- wiphy_debug(hw->wiphy,
> net/mac80211/rx.c-638- "release an RX reorder frame due to timeout on earlier frames\n");
> net/mac80211/rx.c-639-#endif
>
> $ git blame -L635,+4 net/mac80211/rx.c
> aa0c8636 (Christian Lamparter 2010-08-05 01:36:04 +0200 635) #ifdef CONFIG_MAC80211_HT_DEBUG
> aa0c8636 (Christian Lamparter 2010-08-05 01:36:04 +0200 636) if (net_ratelimit())
> 0fb9a9ec (Joe Perches 2010-08-20 16:25:38 -0700 637) wiphy_debug(hw->wiphy,
> 0fb9a9ec (Joe Perches 2010-08-20 16:25:38 -0700 638) "release an RX reorder fram
>
> $ git log -1 0fb9a9ec
> commit 0fb9a9ec27718fbf7fa3153bc94becefb716ceeb
> Author: Joe Perches<[email protected]>
> Date: Fri Aug 20 16:25:38 2010 -0700
>
> net/mac80211: Use wiphy_<level>
>
> Standardize logging messages from
> printk(KERN_<level> "%s: " fmt , wiphy_name(foo), args);
> to
> wiphy_<level>(foo, fmt, args);
>
> Signed-off-by: Joe Perches<[email protected]>
> Signed-off-by: John W. Linville<[email protected]>
>
>

thanks(and brilliant!), in that rx.c we can convert it to
mac80211_ht_debug. what about the other wiphy_debug messages, we would
still retain them know

--
thanks,
shafi

2012-06-07 07:24:12

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

On Thu, 2012-06-07 at 12:41 +0530, Mohammed Shafi Shajakhan wrote:
> Hi Joe,

Hello Mohammed.

> On Thursday 07 June 2012 11:55 AM, Joe Perches wrote:
> > On Thu, 2012-06-07 at 11:32 +0530, Mohammed Shafi Shajakhan wrote:
> >> From: Mohammed Shafi Shajakhan<[email protected]>
> >>
> >> ratelimit few aggregation related messages, these messages
> >> floods the log when aggregation is disabled in the AP(for some
> >> wifi testcases) and we run traffic between STA and AP.
> >
> > I looked at all of the #ifdef CONFIG_MAC80211_HT_DEBUG uses.
> >
> > I think it's better simply to define a mac80211_ht_dbg
> > macro for each of the !CONFIG and CONFIG cases and convert
> > all of the other #ifdef CONFIG_MAC80211_HT_DEBUG logging
> > messages.
>
> oh, ok.
>
> >
> > #ifdef CONFIG_MAC80211_HT_DEBUG
> > #define mac80211_ht_dbg(fmt, ...) \
> > net_dbg_ratelimited(fmt, ##__VA_ARGS__)
> > #else
> > #define mac80211_ht_dbg(fmt, ...) \
> > do { \
> > if (0) \
> > net_dbg_ratelimited(fmt, ##__VA_ARGS__); \
> > } while (0)
> >
> > etc...
> >
> > I think there's one use of wiphy_dbg that could be a
> > mac80211_ht_dbg without much loss.
> >
> > Maybe add "%s", wiphy_name() to the args there.
> >
>
> sorry, i could not understand with wiphy_debug, with mac80211_ht_debug.
> the later is for HT and aggregation related messages.

It was an automatic conversion.

Might as well unconvert it back too.

$ git grep -E -A4 -n "ifdef CONFIG_MAC80211_HT_DEBUG" net/mac80211/rx.c
net/mac80211/rx.c:635:#ifdef CONFIG_MAC80211_HT_DEBUG
net/mac80211/rx.c-636- if (net_ratelimit())
net/mac80211/rx.c-637- wiphy_debug(hw->wiphy,
net/mac80211/rx.c-638- "release an RX reorder frame due to timeout on earlier frames\n");
net/mac80211/rx.c-639-#endif

$ git blame -L635,+4 net/mac80211/rx.c
aa0c8636 (Christian Lamparter 2010-08-05 01:36:04 +0200 635) #ifdef CONFIG_MAC80211_HT_DEBUG
aa0c8636 (Christian Lamparter 2010-08-05 01:36:04 +0200 636) if (net_ratelimit())
0fb9a9ec (Joe Perches 2010-08-20 16:25:38 -0700 637) wiphy_debug(hw->wiphy,
0fb9a9ec (Joe Perches 2010-08-20 16:25:38 -0700 638) "release an RX reorder fram

$ git log -1 0fb9a9ec
commit 0fb9a9ec27718fbf7fa3153bc94becefb716ceeb
Author: Joe Perches <[email protected]>
Date: Fri Aug 20 16:25:38 2010 -0700

net/mac80211: Use wiphy_<level>

Standardize logging messages from
printk(KERN_<level> "%s: " fmt , wiphy_name(foo), args);
to
wiphy_<level>(foo, fmt, args);

Signed-off-by: Joe Perches <[email protected]>
Signed-off-by: John W. Linville <[email protected]>



2012-06-07 06:25:10

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

On Thu, 2012-06-07 at 11:32 +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <[email protected]>
>
> ratelimit few aggregation related messages, these messages
> floods the log when aggregation is disabled in the AP(for some
> wifi testcases) and we run traffic between STA and AP.

I looked at all of the #ifdef CONFIG_MAC80211_HT_DEBUG uses.

I think it's better simply to define a mac80211_ht_dbg
macro for each of the !CONFIG and CONFIG cases and convert
all of the other #ifdef CONFIG_MAC80211_HT_DEBUG logging
messages.

#ifdef CONFIG_MAC80211_HT_DEBUG
#define mac80211_ht_dbg(fmt, ...) \
net_dbg_ratelimited(fmt, ##__VA_ARGS__)
#else
#define mac80211_ht_dbg(fmt, ...) \
do { \
if (0) \
net_dbg_ratelimited(fmt, ##__VA_ARGS__); \
} while (0)

etc...

I think there's one use of wiphy_dbg that could be a
mac80211_ht_dbg without much loss.

Maybe add "%s", wiphy_name() to the args there.

cheers, Joe


2012-06-07 06:27:59

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [PATCH] mac80211: ratelimit few aggregated messages

Hi Sujith,

On Thursday 07 June 2012 11:45 AM, Sujith Manoharan wrote:
> Mohammed Shafi Shajakhan wrote:
>> From: Mohammed Shafi Shajakhan<[email protected]>
>>
>> ratelimit few aggregation related messages, these messages
>> floods the log when aggregation is disabled in the AP(for some
>> wifi testcases) and we run traffic between STA and AP.
>> mac80211 gives up after 15 addba requests, but
>> ieee80211_start_tx_ba_session will be repeatedly called
>> by drivers rate control tx_status callback. These net_dbg_ratelimited
>> messages will be visible only when dynamic debug is enabled for
>> mac80211 module.
>
> I think the drivers need to be fixed. For ath9k, we probably need
> an intermediate state to track whether an ADDBA request is pending.
>

the timeout till the addba requests can be sent out is completely taken
care by mac80211 and we stop trying after a particular amount of time
which is also tracked by mac80211, these are just logs after sometime

--
thanks,
shafi