| Message ID | 200908291910.14528.IvDoorn@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Ivo van Doorn a écrit : > Not all values of the TX status enumeration were covered during > updating of the TX statistics. This could lead to wrong bitrate > tuning but also wrong behavior in tools like hostapd. > > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com> I have a pending patch about TX status that changed the meaning of TXDONE_FALLBACK just a bit... just to avoid this change in fact. I should be able to send those patches in few hours. In my code, you success/failure of the packet is reported with TXDONE_SUCCESS / TXDONE_FAILURE. The TXDONE_FALLBACK is set independently to indicate that a fallback rate table has been used (and this could be the case for either success or failure). Regards, Benoit > --- drivers/net/wireless/rt2x00/rt2x00dev.c | 28 > ++++++++++++++++------------ 1 files changed, 16 insertions(+), 12 > deletions(-) > > diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c > b/drivers/net/wireless/rt2x00/rt2x00dev.c index 5db613f..71761b3 > 100644 --- a/drivers/net/wireless/rt2x00/rt2x00dev.c +++ > b/drivers/net/wireless/rt2x00/rt2x00dev.c @@ -206,6 +206,7 @@ void > rt2x00lib_txdone(struct queue_entry *entry, unsigned int > header_length = ieee80211_get_hdrlen_from_skb(entry->skb); u8 > rate_idx, rate_flags, retry_rates; unsigned int i; + bool success; > > /* * Unmap the skb. @@ -234,13 +235,18 @@ void > rt2x00lib_txdone(struct queue_entry *entry, > rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_TXDONE, entry->skb); > > /* - * Update TX statistics. + * Determine if the frame has been > successfully transmitted. */ - rt2x00dev->link.qual.tx_success += + > success = test_bit(TXDONE_SUCCESS, &txdesc->flags) || - > test_bit(TXDONE_UNKNOWN, &txdesc->flags); - > rt2x00dev->link.qual.tx_failed += - test_bit(TXDONE_FAILURE, > &txdesc->flags); + test_bit(TXDONE_UNKNOWN, &txdesc->flags) || > + test_bit(TXDONE_FALLBACK, &txdesc->flags); + + /* + * Update > TX statistics. + */ + rt2x00dev->link.qual.tx_success += success; > + rt2x00dev->link.qual.tx_failed += !success; > > rate_idx = skbdesc->tx_rate_idx; rate_flags = > skbdesc->tx_rate_flags; @@ -263,22 +269,20 @@ void > rt2x00lib_txdone(struct queue_entry *entry, > tx_info->status.rates[i].flags = rate_flags; > tx_info->status.rates[i].count = 1; } - if (i < > (IEEE80211_TX_MAX_RATES -1)) + if (i < (IEEE80211_TX_MAX_RATES - > 1)) tx_info->status.rates[i].idx = -1; /* terminate */ > > if (!(tx_info->flags & IEEE80211_TX_CTL_NO_ACK)) { - if > (test_bit(TXDONE_SUCCESS, &txdesc->flags) || - > test_bit(TXDONE_UNKNOWN, &txdesc->flags)) + if (success) > tx_info->flags |= IEEE80211_TX_STAT_ACK; - else if > (test_bit(TXDONE_FAILURE, &txdesc->flags)) + else > rt2x00dev->low_level_stats.dot11ACKFailureCount++; } > > if (rate_flags & IEEE80211_TX_RC_USE_RTS_CTS) { - if > (test_bit(TXDONE_SUCCESS, &txdesc->flags) || - > test_bit(TXDONE_UNKNOWN, &txdesc->flags)) + if (success) > rt2x00dev->low_level_stats.dot11RTSSuccessCount++; - else if > (test_bit(TXDONE_FAILURE, &txdesc->flags)) + else > rt2x00dev->low_level_stats.dot11RTSFailureCount++; } > -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkqZkB8ACgkQOR6EySwP7oKnaQCglW+lu167n88CR3Chv1i4uF1P I1kAoLrVvvXCbmzWN8M2/wHtyTSCnUZu =IJEh -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 29 August 2009, Benoit PAPILLAULT wrote: > Ivo van Doorn a écrit : > > Not all values of the TX status enumeration were covered during > > updating of the TX statistics. This could lead to wrong bitrate > > tuning but also wrong behavior in tools like hostapd. > > > > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com> > I have a pending patch about TX status that changed the meaning of > TXDONE_FALLBACK just a bit... just to avoid this change in fact. > I should be able to send those patches in few hours. > > In my code, you success/failure of the packet is reported with > TXDONE_SUCCESS / TXDONE_FAILURE. The TXDONE_FALLBACK is set > independently to indicate that a fallback rate table has been used > (and this could be the case for either success or failure). Well that shouldn't block this patch at this time since it is a bugfix which is quite important to get minstrel and hostapd working properly. Once I have reviewed your patch it can be send upstream immediately or in a single batch together with the rt2800pci driver. ;) Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c index 5db613f..71761b3 100644 --- a/drivers/net/wireless/rt2x00/rt2x00dev.c +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c @@ -206,6 +206,7 @@ void rt2x00lib_txdone(struct queue_entry *entry, unsigned int header_length = ieee80211_get_hdrlen_from_skb(entry->skb); u8 rate_idx, rate_flags, retry_rates; unsigned int i; + bool success; /* * Unmap the skb. @@ -234,13 +235,18 @@ void rt2x00lib_txdone(struct queue_entry *entry, rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_TXDONE, entry->skb); /* - * Update TX statistics. + * Determine if the frame has been successfully transmitted. */ - rt2x00dev->link.qual.tx_success += + success = test_bit(TXDONE_SUCCESS, &txdesc->flags) || - test_bit(TXDONE_UNKNOWN, &txdesc->flags); - rt2x00dev->link.qual.tx_failed += - test_bit(TXDONE_FAILURE, &txdesc->flags); + test_bit(TXDONE_UNKNOWN, &txdesc->flags) || + test_bit(TXDONE_FALLBACK, &txdesc->flags); + + /* + * Update TX statistics. + */ + rt2x00dev->link.qual.tx_success += success; + rt2x00dev->link.qual.tx_failed += !success; rate_idx = skbdesc->tx_rate_idx; rate_flags = skbdesc->tx_rate_flags; @@ -263,22 +269,20 @@ void rt2x00lib_txdone(struct queue_entry *entry, tx_info->status.rates[i].flags = rate_flags; tx_info->status.rates[i].count = 1; } - if (i < (IEEE80211_TX_MAX_RATES -1)) + if (i < (IEEE80211_TX_MAX_RATES - 1)) tx_info->status.rates[i].idx = -1; /* terminate */ if (!(tx_info->flags & IEEE80211_TX_CTL_NO_ACK)) { - if (test_bit(TXDONE_SUCCESS, &txdesc->flags) || - test_bit(TXDONE_UNKNOWN, &txdesc->flags)) + if (success) tx_info->flags |= IEEE80211_TX_STAT_ACK; - else if (test_bit(TXDONE_FAILURE, &txdesc->flags)) + else rt2x00dev->low_level_stats.dot11ACKFailureCount++; } if (rate_flags & IEEE80211_TX_RC_USE_RTS_CTS) { - if (test_bit(TXDONE_SUCCESS, &txdesc->flags) || - test_bit(TXDONE_UNKNOWN, &txdesc->flags)) + if (success) rt2x00dev->low_level_stats.dot11RTSSuccessCount++; - else if (test_bit(TXDONE_FAILURE, &txdesc->flags)) + else rt2x00dev->low_level_stats.dot11RTSFailureCount++; }
Not all values of the TX status enumeration were covered during updating of the TX statistics. This could lead to wrong bitrate tuning but also wrong behavior in tools like hostapd. Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com> --- drivers/net/wireless/rt2x00/rt2x00dev.c | 28 ++++++++++++++++------------ 1 files changed, 16 insertions(+), 12 deletions(-)