2009-04-14 22:34:52

by Gábor Stefanik

[permalink] [raw]
Subject: [PATCH RFC] mac80211: don't ask drivers to retry transmitting NO_ACK frames

Set the total count of TX tries to 1 if IEEE80211_TX_CTL_NO_ACK is
set. This fixes a bug where mac80211 allowed the RC algo to request
drivers to forcibly retransmit NO_ACK (e.g. multicast) frames until
the excessive-retries limit is reached.

Signed-off-by: G?bor Stefanik <[email protected]>
Cc: Johannes Berg <[email protected]>

---
Patch also available as attachment in case Gmail linewraps it.

tx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index b47435d..751934b 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -539,7 +539,8 @@ ieee80211_tx_h_rate_ctrl(struct ieee80211_tx_data *tx)
if (tx->sta)
tx->sta->last_tx_rate = txrc.reported_rate;

- if (unlikely(!info->control.rates[0].count))
+ if (unlikely(!info->control.rates[0].count) ||
+ info->flags & IEEE80211_TX_CTL_NO_ACK)
info->control.rates[0].count = 1;

if (is_multicast_ether_addr(hdr->addr1)) {


Attachments:
mac80211-fix-tx-ctl-no-ack-retry-count.patch (559.00 B)

2009-04-14 22:38:21

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: don't ask drivers to retry transmitting NO_ACK frames

On Wed, 2009-04-15 at 00:34 +0200, Gábor Stefanik wrote:
> Set the total count of TX tries to 1 if IEEE80211_TX_CTL_NO_ACK is
> set. This fixes a bug where mac80211 allowed the RC algo to request
> drivers to forcibly retransmit NO_ACK (e.g. multicast) frames until
> the excessive-retries limit is reached.

Why not fix the rate control algorithms to use the no-ack flag instead
of is_multicast?

> Signed-off-by: Gábor Stefanik <[email protected]>
> Cc: Johannes Berg <[email protected]>
>
> ---
> Patch also available as attachment in case Gmail linewraps it.

You need to get a proper MUA.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-04-14 22:48:12

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: don't ask drivers to retry transmitting NO_ACK frames

2009/4/15 Johannes Berg <[email protected]>:
> On Wed, 2009-04-15 at 00:34 +0200, G=E1bor Stefanik wrote:
>> Set the total count of TX tries to 1 if IEEE80211_TX_CTL_NO_ACK is
>> set. This fixes a bug where mac80211 allowed the RC algo to request
>> drivers to forcibly retransmit NO_ACK (e.g. multicast) frames until
>> the excessive-retries limit is reached.
>
> Why not fix the rate control algorithms to use the no-ack flag instea=
d
> of is_multicast?

I was thinking about that originally, but we have 5 RC algorithms
(minstrel, pid, iwl-3945-rs, iwl-agn-rs & ath9k-rs) right now, with
possibly more to come for future N-based devices. Fixing them all
one-by-one would be much more tedious, and we will still not be
future-proof (that is, there is always the risk of getting a new
driver released with its own, specific RC algorithm that suffers from
this bug). This way, if e.g. Whatevercom releases a driver for its
WhateWIFI UltraMIMO1337 chipset with its own, buggy "whatever-rs"
algorithm, NO_ACK will still work properly for that driver.

>
>> Signed-off-by: G=E1bor Stefanik <[email protected]>
>> Cc: Johannes Berg <[email protected]>
>>
>> ---
>> Patch also available as attachment in case Gmail linewraps it.
>
> You need to get a proper MUA.

Well, if you know any MUA that does Gmail-style conversation
threading, and can sync its conversations with the Gmail web interface
(I need access to Gmail on the go, including from public computers
where I can't install a MUA), tell me immediately. So far, I've seen
no such thing.

>
> johannes
>



--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

2009-04-15 00:01:00

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: don't ask drivers to retry transmitting NO_ACK frames

2009/4/14 G=E1bor Stefanik <[email protected]>:

>>> Patch also available as attachment in case Gmail linewraps it.
>>
>> You need to get a proper MUA.
>
> Well, if you know any MUA that does Gmail-style conversation
> threading, and can sync its conversations with the Gmail web interfac=
e
> (I need access to Gmail on the go, including from public computers
> where I can't install a MUA), tell me immediately. So far, I've seen
> no such thing.

Somewhere along the line, I discovered that in Gmail's web interface,
"Show original" under the "Reply" menu at the right-top corner of a
e-mail does what is needed - it takes a particular e-mail and open a
new tab, showing its content as text. I extracted a couple of Larry's
e-mails recently this way ("show original" then "save page as...") and
just "patch -p1" with the saved pages.

In other words, even when the recipient is using Gmail, there is no
need to send an extra attachment beyond what Linus dictates i.e. patch
in-line.

(how the sender *sends* a patch, is a different matter - cut-and-paste
with web mail or even a "proper" MUA like thunderbird just doesn't do
it without some wizardary, but I suppose most people just do it like
me, with git-send-email to send patches. The fact that git-send-mail
uses gmail for SMTP is just incidental).

2009-04-14 23:07:51

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: don't ask drivers to retry transmitting NO_ACK frames

2009/4/15 G=E1bor Stefanik <[email protected]>:
> 2009/4/15 Johannes Berg <[email protected]>:
>> On Wed, 2009-04-15 at 00:34 +0200, G=E1bor Stefanik wrote:
>>> Set the total count of TX tries to 1 if IEEE80211_TX_CTL_NO_ACK is
>>> set. This fixes a bug where mac80211 allowed the RC algo to request
>>> drivers to forcibly retransmit NO_ACK (e.g. multicast) frames until
>>> the excessive-retries limit is reached.
>>
>> Why not fix the rate control algorithms to use the no-ack flag inste=
ad
>> of is_multicast?
>
> I was thinking about that originally, but we have 5 RC algorithms
> (minstrel, pid, iwl-3945-rs, iwl-agn-rs & ath9k-rs) right now, with
> possibly more to come for future N-based devices. Fixing them all
> one-by-one would be much more tedious, and we will still not be
> future-proof (that is, there is always the risk of getting a new
> driver released with its own, specific RC algorithm that suffers from
> this bug). This way, if e.g. Whatevercom releases a driver for its
> WhateWIFI UltraMIMO1337 chipset with its own, buggy "whatever-rs"
> algorithm, NO_ACK will still work properly for that driver.

BTW, Minstrel specifically forces NO_ACK packets to be retried the
maximal allowed number of times.

rc80211_minstrel.c, line 246:
> if (!sta || !mi || use_low_rate(skb)) {
> ar[0].idx =3D rate_lowest_index(sband, sta);
> ar[0].count =3D mp->max_retry;
> return;
> }

mp->max_retry =3D hw->max_rate_tries, or 7 if hw->max_rate_tries is 0.
The use_low_rate function is basically a check for NO_ACK.

--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)