* [PATCH] mac80211: fix an issue in ieee80211_tx_info count field management
@ 2012-04-06 18:48 Lorenzo Bianconi
2012-04-12 4:03 ` Johannes Berg
0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2012-04-06 18:48 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, Johannes Berg, Felix Fietkau
Hi,
I noticed a possible issue in the status count field management of the
ieee80211_tx_info data structure. In particular, when the AGGR
processing is employed,
status.rates[].count is set just for the first frame and not for
others belonging to the same burst, leading to wrong statistic data in
the mac80211 debug file system.
Regards
Lorenzo
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
---
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -355,7 +355,13 @@
int rtap_len;
for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
- if (info->status.rates[i].idx < 0) {
+ if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
+ !(info->flags & IEEE80211_TX_STAT_AMPDU)) {
+ /* just the first aggr frame carry status info */
+ info->status.rates[i].idx = -1;
+ info->status.rates[i].count = 0;
+ break;
+ } else if (info->status.rates[i].idx < 0) {
break;
} else if (i >= hw->max_report_rates) {
/* the HW cannot have attempted that rate */
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mac80211: fix an issue in ieee80211_tx_info count field management
2012-04-06 18:48 [PATCH] mac80211: fix an issue in ieee80211_tx_info count field management Lorenzo Bianconi
@ 2012-04-12 4:03 ` Johannes Berg
2012-04-12 7:24 ` Felix Fietkau
0 siblings, 1 reply; 3+ messages in thread
From: Johannes Berg @ 2012-04-12 4:03 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: John Linville, linux-wireless, Felix Fietkau
On Fri, 2012-04-06 at 20:48 +0200, Lorenzo Bianconi wrote:
> Hi,
>
> I noticed a possible issue in the status count field management of the
> ieee80211_tx_info data structure. In particular, when the AGGR
> processing is employed,
> status.rates[].count is set just for the first frame and not for
> others belonging to the same burst, leading to wrong statistic data in
> the mac80211 debug file system.
>
> Regards
>
> Lorenzo
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
> ---
> --- a/net/mac80211/status.c
> +++ b/net/mac80211/status.c
> @@ -355,7 +355,13 @@
> int rtap_len;
>
> for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
> - if (info->status.rates[i].idx < 0) {
> + if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
> + !(info->flags & IEEE80211_TX_STAT_AMPDU)) {
> + /* just the first aggr frame carry status info */
> + info->status.rates[i].idx = -1;
> + info->status.rates[i].count = 0;
> + break;
> + } else if (info->status.rates[i].idx < 0) {
Is that really the way all drivers do it? I know ath9k seems to do it
that way, but I don't think all drivers do. ath9k is probably the
exception rather than the rule, at least today
johannes
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mac80211: fix an issue in ieee80211_tx_info count field management
2012-04-12 4:03 ` Johannes Berg
@ 2012-04-12 7:24 ` Felix Fietkau
0 siblings, 0 replies; 3+ messages in thread
From: Felix Fietkau @ 2012-04-12 7:24 UTC (permalink / raw)
To: Johannes Berg; +Cc: Lorenzo Bianconi, John Linville, linux-wireless
On 2012-04-12 6:03 AM, Johannes Berg wrote:
> On Fri, 2012-04-06 at 20:48 +0200, Lorenzo Bianconi wrote:
>> Hi,
>>
>> I noticed a possible issue in the status count field management of the
>> ieee80211_tx_info data structure. In particular, when the AGGR
>> processing is employed,
>> status.rates[].count is set just for the first frame and not for
>> others belonging to the same burst, leading to wrong statistic data in
>> the mac80211 debug file system.
>>
>> Regards
>>
>> Lorenzo
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
>> ---
>> --- a/net/mac80211/status.c
>> +++ b/net/mac80211/status.c
>> @@ -355,7 +355,13 @@
>> int rtap_len;
>>
>> for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
>> - if (info->status.rates[i].idx < 0) {
>> + if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
>> + !(info->flags & IEEE80211_TX_STAT_AMPDU)) {
>> + /* just the first aggr frame carry status info */
>> + info->status.rates[i].idx = -1;
>> + info->status.rates[i].count = 0;
>> + break;
>> + } else if (info->status.rates[i].idx < 0) {
>
> Is that really the way all drivers do it? I know ath9k seems to do it
> that way, but I don't think all drivers do. ath9k is probably the
> exception rather than the rule, at least today
At least minstrel_ht expects it this way. If I understand iwlwifi
correctly, it reports tx status in a similar way - on completion in
iwlagn_rx_reply_compressed_ba(), the first subframe carries the
information for an entire A-MPDU, the other subframes are not touched.
- Felix
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-04-12 7:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-06 18:48 [PATCH] mac80211: fix an issue in ieee80211_tx_info count field management Lorenzo Bianconi
2012-04-12 4:03 ` Johannes Berg
2012-04-12 7:24 ` Felix Fietkau
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).