[PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[Gábor Stefanik]

Handling of multicast/NO_ACK packets by mac80211 rate scaling is a
mess. Basically, the rule of thumb is that any packet with
IEEE80211_TX_CTL_NO_ACK (including multicasts and broadcasts, which
always have this flag set) should be sent at lowest rate (unless
overridden by radiotap - this is not covered in this patchset), with
no retries.

So, the right thing to do in any rate scaling algorithm:
 * Check for IEEE80211_TX_CTL_NO_ACK. No need to check
is_multicast_ether_addr, as multicasts always have NO_ACK set.
 * If it is set, select the lowest available rate (unless overridden
by radiotap), with a retry count of 0 (total try count of 1).

However, the actual rate scaling algorithms get this wrong in various w=
ays:
 * Minstrel checks for both is_multicast_ether_addr and
IEEE80211_TX_CTL_NO_ACK (redundant, since the former implies the
latter), and sets the rate to the lowest (correct), and the retry
count to the max (wrong).
 * PID only checks is_multicast_ether_addr (completely leaving out
unicasts with IEEE80211_TX_CTL_NO_ACK set by e.g. Radiotap), setting
the rate to the lowest (correct). However, the same logic is used for
determining retry count of multicasts as for unicasts (wrong).
 * Iwlwifi's rate scaling algorithms behave like PID. In addition,
they appear to not care about retry count at all, which is handled in
the drivers themselves I guess.
 * Ath5k's algorithm correctly sets both the rate and the retry count
- but incorrectly bases its checks on is_multicast_ether_addr instead
of IEEE80211_TX_CTL_NO_ACK, again missing unicasts with this flag set.

Most PHYs can gracefully handle this error (with the help of a no-ACK
knob in the PHY TX header that disables retries), but those that have
no such knob, and instead depend on getting a correct retry count of 0
(total try count of 1), such as RTL818x, end up always transmitting
multicast/NO_ACK frames multiple times, as (incorrectly) requested by
the RC algorithm.

Signed-off-by: G=E1bor Stefanik <netrolller.3d@gmail.com>
Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Zhu Yi <yi.zhu@intel.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Nick Kossifidis <mickflemm@gmail.com>
Cc: Louis R. Rodriguez <mcgrof@gmail.com>

G=E1bor Stefanik (5):
 mac80211: Fix handling of retry count of NO_ACK frames in minstrel
 mac80211: Fix handling of retry count of NO_ACK frames in PID
 iwlwifi: Fix handling of retry count of NO_ACK frames for iwl-{3945|ag=
n}-rs
 ath5k: Fix handling of retry count of NO_ACK frames
 mac80211: Warn if the rate controller requests retries for a NO_ACK fr=
ame

 drivers/net/wireless/ath/ath9k/rc.c        |    6 +++---
 drivers/net/wireless/iwlwifi/iwl-3945-rs.c |    7 ++++---
 drivers/net/wireless/iwlwifi/iwl-agn-rs.c  |   14 +++++++-------
 net/mac80211/rc80211_minstrel.c            |    8 +++++---
 net/mac80211/rc80211_pid_algo.c            |    8 +++++---
 net/mac80211/tx.c                          |    4 ++++
 6 files changed, 28 insertions(+), 19 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[Jouni Malinen]

On Thu, Apr 23, 2009 at 07:35:55PM +0200, G=C3=A1bor Stefanik wrote:
> Handling of multicast/NO_ACK packets by mac80211 rate scaling is a
> mess. Basically, the rule of thumb is that any packet with
> IEEE80211_TX_CTL_NO_ACK (including multicasts and broadcasts, which
> always have this flag set) should be sent at lowest rate (unless
> overridden by radiotap - this is not covered in this patchset), with
> no retries.

While this may be the case with the current rate control
implementations, this is not a good assumption to make.
Multicast/broadcast frames could (and often really should) be
transmitted at higher rate, i.e., the rate control algorithm could
figure out that all associated STAs should be able to receive frames at
a higher basic rate.

As far as unicast no-ACK frames are concerned, those could (and again,
should) be sent at higher rate, too, if the receiving STA can be assume=
d
to be able to receive the frame with "large enough" probability.

> So, the right thing to do in any rate scaling algorithm:
>  * Check for IEEE80211_TX_CTL_NO_ACK. No need to check
> is_multicast_ether_addr, as multicasts always have NO_ACK set.
>  * If it is set, select the lowest available rate (unless overridden
> by radiotap), with a retry count of 0 (total try count of 1).

I would not agree with these as general rules for all rate control
algorithms.

--=20
Jouni Malinen                                            PGP id EFC895F=
A
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[Gábor Stefanik]

2009/4/23 Jouni Malinen <j@w1.fi>:
> On Thu, Apr 23, 2009 at 07:35:55PM +0200, G=E1bor Stefanik wrote:
>> Handling of multicast/NO_ACK packets by mac80211 rate scaling is a
>> mess. Basically, the rule of thumb is that any packet with
>> IEEE80211_TX_CTL_NO_ACK (including multicasts and broadcasts, which
>> always have this flag set) should be sent at lowest rate (unless
>> overridden by radiotap - this is not covered in this patchset), with
>> no retries.
>
> While this may be the case with the current rate control
> implementations, this is not a good assumption to make.
> Multicast/broadcast frames could (and often really should) be
> transmitted at higher rate, i.e., the rate control algorithm could
> figure out that all associated STAs should be able to receive frames =
at
> a higher basic rate.
>
> As far as unicast no-ACK frames are concerned, those could (and again=
,
> should) be sent at higher rate, too, if the receiving STA can be assu=
med
> to be able to receive the frame with "large enough" probability.
>
>> So, the right thing to do in any rate scaling algorithm:
>> =A0* Check for IEEE80211_TX_CTL_NO_ACK. No need to check
>> is_multicast_ether_addr, as multicasts always have NO_ACK set.
>> =A0* If it is set, select the lowest available rate (unless overridd=
en
>> by radiotap), with a retry count of 0 (total try count of 1).
>
> I would not agree with these as general rules for all rate control
> algorithms.
>
> --
> Jouni Malinen =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0PGP id EFC895FA
>

Yes, I agree what you write - but the point of the patchset is to make
sure that (1) all NO_ACK frames get correct treatment, not just
multicasts and (2) NO_ACK/multicast frames aren't requested to be
retried (which is obviously wrong, and results in always retrying till
the limit is reached). Developing an algorithm to allow NO_ACKs to be
transmitted at a higher rate is beyond the scope of this patchset.

--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[Felix Fietkau]

Jouni Malinen wrote:
> On Thu, Apr 23, 2009 at 07:35:55PM +0200, G=C3=A1bor Stefanik wrote:
>> Handling of multicast/NO_ACK packets by mac80211 rate scaling is a
>> mess. Basically, the rule of thumb is that any packet with
>> IEEE80211_TX_CTL_NO_ACK (including multicasts and broadcasts, which
>> always have this flag set) should be sent at lowest rate (unless
>> overridden by radiotap - this is not covered in this patchset), with
>> no retries.
>=20
> While this may be the case with the current rate control
> implementations, this is not a good assumption to make.
> Multicast/broadcast frames could (and often really should) be
> transmitted at higher rate, i.e., the rate control algorithm could
> figure out that all associated STAs should be able to receive frames =
at
> a higher basic rate.
Good idea. I should implement this in minstrel and use the minimum valu=
e
of max_prob_rate for all stations as multicast rate ;)

Johannes: Can you think of a good way to fit this into the API?

- Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Fri, 2009-04-24 at 19:31 +0200, Felix Fietkau wrote:

> > While this may be the case with the current rate control
> > implementations, this is not a good assumption to make.
> > Multicast/broadcast frames could (and often really should) be
> > transmitted at higher rate, i.e., the rate control algorithm could
> > figure out that all associated STAs should be able to receive frames at
> > a higher basic rate.
> Good idea. I should implement this in minstrel and use the minimum value
> of max_prob_rate for all stations as multicast rate ;)
> 
> Johannes: Can you think of a good way to fit this into the API?

Not really... We haven't exported station iterators (for a vif) yet,
which you would need (though I would recalculate only when a station is
removed, when adding you can use min(current,new))

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[Gábor Stefanik]

Did this get lost? I can't see it in wireless-testing.

2009/4/23 G=E1bor Stefanik <netrolller.3d@gmail.com>:
> Handling of multicast/NO_ACK packets by mac80211 rate scaling is a
> mess. Basically, the rule of thumb is that any packet with
> IEEE80211_TX_CTL_NO_ACK (including multicasts and broadcasts, which
> always have this flag set) should be sent at lowest rate (unless
> overridden by radiotap - this is not covered in this patchset), with
> no retries.
>
> So, the right thing to do in any rate scaling algorithm:
> =A0* Check for IEEE80211_TX_CTL_NO_ACK. No need to check
> is_multicast_ether_addr, as multicasts always have NO_ACK set.
> =A0* If it is set, select the lowest available rate (unless overridde=
n
> by radiotap), with a retry count of 0 (total try count of 1).
>
> However, the actual rate scaling algorithms get this wrong in various=
 ways:
> =A0* Minstrel checks for both is_multicast_ether_addr and
> IEEE80211_TX_CTL_NO_ACK (redundant, since the former implies the
> latter), and sets the rate to the lowest (correct), and the retry
> count to the max (wrong).
> =A0* PID only checks is_multicast_ether_addr (completely leaving out
> unicasts with IEEE80211_TX_CTL_NO_ACK set by e.g. Radiotap), setting
> the rate to the lowest (correct). However, the same logic is used for
> determining retry count of multicasts as for unicasts (wrong).
> =A0* Iwlwifi's rate scaling algorithms behave like PID. In addition,
> they appear to not care about retry count at all, which is handled in
> the drivers themselves I guess.
> =A0* Ath5k's algorithm correctly sets both the rate and the retry cou=
nt
> - but incorrectly bases its checks on is_multicast_ether_addr instead
> of IEEE80211_TX_CTL_NO_ACK, again missing unicasts with this flag set=
=2E
>
> Most PHYs can gracefully handle this error (with the help of a no-ACK
> knob in the PHY TX header that disables retries), but those that have
> no such knob, and instead depend on getting a correct retry count of =
0
> (total try count of 1), such as RTL818x, end up always transmitting
> multicast/NO_ACK frames multiple times, as (incorrectly) requested by
> the RC algorithm.
>
> Signed-off-by: G=E1bor Stefanik <netrolller.3d@gmail.com>
> Cc: Felix Fietkau <nbd@openwrt.org>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Cc: Zhu Yi <yi.zhu@intel.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Cc: Nick Kossifidis <mickflemm@gmail.com>
> Cc: Louis R. Rodriguez <mcgrof@gmail.com>
>
> G=E1bor Stefanik (5):
> =A0mac80211: Fix handling of retry count of NO_ACK frames in minstrel
> =A0mac80211: Fix handling of retry count of NO_ACK frames in PID
> =A0iwlwifi: Fix handling of retry count of NO_ACK frames for iwl-{394=
5|agn}-rs
> =A0ath5k: Fix handling of retry count of NO_ACK frames
> =A0mac80211: Warn if the rate controller requests retries for a NO_AC=
K frame
>
> =A0drivers/net/wireless/ath/ath9k/rc.c =A0 =A0 =A0 =A0| =A0 =A06 +++-=
--
> =A0drivers/net/wireless/iwlwifi/iwl-3945-rs.c | =A0 =A07 ++++---
> =A0drivers/net/wireless/iwlwifi/iwl-agn-rs.c =A0| =A0 14 +++++++-----=
--
> =A0net/mac80211/rc80211_minstrel.c =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A08 =
+++++---
> =A0net/mac80211/rc80211_pid_algo.c =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A08 =
+++++---
> =A0net/mac80211/tx.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0| =A0 =A04 ++++
> =A06 files changed, 28 insertions(+), 19 deletions(-)
>



--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[John W. Linville]

On Tue, Apr 28, 2009 at 10:53:51PM +0200, G=E1bor Stefanik wrote:
> Did this get lost? I can't see it in wireless-testing.

No, I didn't lose them.  I was waiting to process them until I had
a little extra time, because nearly every patch you send me requires
manually intervention before I can apply it.  This is quite painful
and it increases the time I need for merging patches exponentially.

Please, please, please ensure that:

	-- you are using a recent pull of wireless-testing;

	-- you're patches are based directly on wireless-testing and
	   not on top of any other patches you have not submitted;

	-- you format your patch and changelog compatibly with what
	   is described here:

		http://linux.yyz.us/patch-format.html

	-- you send the patch inline (without MIME) in email if at
	   all possible;

	-- if you simply must send a patch as an attachment then do
	   so, but...

	-- do _not_ send a patch inline _and_ attached to the same
	   email

The above are not strictly directed at you, G=E1bor.  You just were
lucky enough to be asking at the right time...

Thanks,

John

P.S.  Out of these five patches, 4 of them required manual intervention=
=2E..
--=20
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] mac80211, iwlwifi, ath9k: Fix rate scaling for IEEE80211_TX_CTL_NO_ACK packets

[John W. Linville]

On Thu, Apr 30, 2009 at 02:28:07PM -0400, John W. Linville wrote:
> On Tue, Apr 28, 2009 at 10:53:51PM +0200, G=E1bor Stefanik wrote:
> > Did this get lost? I can't see it in wireless-testing.
>=20
> No, I didn't lose them.  I was waiting to process them until I had
> a little extra time, because nearly every patch you send me requires
> manually intervention before I can apply it.  This is quite painful
> and it increases the time I need for merging patches exponentially.
>=20
> Please, please, please ensure that:
>=20
> 	-- you are using a recent pull of wireless-testing;
>=20
> 	-- you're patches are based directly on wireless-testing and
> 	   not on top of any other patches you have not submitted;
>=20
> 	-- you format your patch and changelog compatibly with what
> 	   is described here:
>=20
> 		http://linux.yyz.us/patch-format.html
>=20
> 	-- you send the patch inline (without MIME) in email if at
> 	   all possible;
>=20
> 	-- if you simply must send a patch as an attachment then do
> 	   so, but...
>=20
> 	-- do _not_ send a patch inline _and_ attached to the same
> 	   email

=46orgot one:

	-- successfully compile a current wireless-testing tree with
	   your patches applied...

>=20
> The above are not strictly directed at you, G=E1bor.

That one is...

John
--=20
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html