[PATCH] mac80211: ratelimit few aggregated messages

[PATCH] mac80211: ratelimit few aggregated messages

[Mohammed Shafi Shajakhan]

From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>

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 <joe@perches.com>
Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
---
 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

[PATCH] mac80211: ratelimit few aggregated messages

[Sujith Manoharan]

Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
> 
> 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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Mohammed Shafi Shajakhan]

Hi Sujith,

On Thursday 07 June 2012 11:45 AM, Sujith Manoharan wrote:
> Mohammed Shafi Shajakhan wrote:
>> From: Mohammed Shafi Shajakhan<mohammed@qca.qualcomm.com>
>>
>> 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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Joe Perches]

On Thu, 2012-06-07 at 11:32 +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
> 
> 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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Mohammed Shafi Shajakhan]

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<mohammed@qca.qualcomm.com>
>>
>> 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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Joe Perches]

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<mohammed@qca.qualcomm.com>
> >>
> >> 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 <joe@perches.com>
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 <joe@perches.com>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Mohammed Shafi Shajakhan]

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<joe@perches.com>
> 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<joe@perches.com>
>      Signed-off-by: John W. Linville<linville@tuxdriver.com>
>
>

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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Joe Perches]

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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Mohammed Shafi Shajakhan]

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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Joe Perches]

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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Mohammed Shafi Shajakhan]

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 (netdev@vger.kernel.org mailing list ?) and i have to 
do any changes based on that patch right ? thank you!


-- 
thanks,
shafi

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Joe Perches]

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 (netdev@vger.kernel.org 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

Re: [PATCH] mac80211: ratelimit few aggregated messages

[Mohammed Shafi Shajakhan]

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 (netdev@vger.kernel.org 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