diff mbox

mac80211: use capped prob when computing throughputs

Message ID 1384908668-27869-1-git-send-email-karl.beldan@gmail.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Karl Beldan Nov. 20, 2013, 12:51 a.m. UTC
From: Karl Beldan <karl.beldan@rivierawaves.com>

Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
it to compute the rate throughput.

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
CC: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel_ht.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Felix Fietkau Nov. 20, 2013, 7:32 a.m. UTC | #1
On 2013-11-20 01:51, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
> 
> Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
> introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
> it to compute the rate throughput.
> 
> Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> CC: Felix Fietkau <nbd@openwrt.org>
Nice catch!
Acked-by: Felix Fietkau <nbd@openwrt.org>

--
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
Karl Beldan Nov. 20, 2013, 1:56 p.m. UTC | #2
On Wed, Nov 20, 2013 at 08:32:32AM +0100, Felix Fietkau wrote:
> On 2013-11-20 01:51, Karl Beldan wrote:
> > From: Karl Beldan <karl.beldan@rivierawaves.com>
> > 
> > Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
> > introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
> > it to compute the rate throughput.
> > 
> > Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> > CC: Felix Fietkau <nbd@openwrt.org>
> Nice catch!
> Acked-by: Felix Fietkau <nbd@openwrt.org>
> 
Interestingly enough, consecutive coding rates (5/6, 3/4, 2/3) max ratio
is 9/10, did you do it on purpose ?  (e.g. (9/10) * (5/6) == 3/4,
(9/10) * (3/4) == 2/3 + 11/120).
 
Karl
--
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
Felix Fietkau Nov. 20, 2013, 2:04 p.m. UTC | #3
On 2013-11-20 14:56, Karl Beldan wrote:
> On Wed, Nov 20, 2013 at 08:32:32AM +0100, Felix Fietkau wrote:
>> On 2013-11-20 01:51, Karl Beldan wrote:
>> > From: Karl Beldan <karl.beldan@rivierawaves.com>
>> > 
>> > Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
>> > introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
>> > it to compute the rate throughput.
>> > 
>> > Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
>> > CC: Felix Fietkau <nbd@openwrt.org>
>> Nice catch!
>> Acked-by: Felix Fietkau <nbd@openwrt.org>
>> 
> Interestingly enough, consecutive coding rates (5/6, 3/4, 2/3) max ratio
> is 9/10, did you do it on purpose ?  (e.g. (9/10) * (5/6) == 3/4,
> (9/10) * (3/4) == 2/3 + 11/120).
The change has nothing to do with coding rates, it's only about
retransmissions caused by collisions under load.

- Felix

--
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
Karl Beldan Nov. 20, 2013, 2:50 p.m. UTC | #4
On Wed, Nov 20, 2013 at 03:04:34PM +0100, Felix Fietkau wrote:
> On 2013-11-20 14:56, Karl Beldan wrote:
> > On Wed, Nov 20, 2013 at 08:32:32AM +0100, Felix Fietkau wrote:
> >> On 2013-11-20 01:51, Karl Beldan wrote:
> >> > From: Karl Beldan <karl.beldan@rivierawaves.com>
> >> > 
> >> > Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
> >> > introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
> >> > it to compute the rate throughput.
> >> > 
> >> > Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> >> > CC: Felix Fietkau <nbd@openwrt.org>
> >> Nice catch!
> >> Acked-by: Felix Fietkau <nbd@openwrt.org>
> >> 
> > Interestingly enough, consecutive coding rates (5/6, 3/4, 2/3) max ratio
> > is 9/10, did you do it on purpose ?  (e.g. (9/10) * (5/6) == 3/4,
> > (9/10) * (3/4) == 2/3 + 11/120).
> The change has nothing to do with coding rates, it's only about
> retransmissions caused by collisions under load.
> 
I understand this, my point was that along with this comes the following:
let's say my SNR is just not so good to get 5/6 as good as 3/4, and e.g.
case1 htMCS7 has  91% 
      htMCS6 has 100% success 
case2 htMCS7 has  80% 
      htMCS6 has 100% success 
capping at 90% will prefer htMCS7 in case1 and htMCS6 in case2 both
achieving best real throughput.
capping at 80% will prefer htMCS7 in case1 _but_ htMCS7 in case2 the
latter being the worst real throughput(90% of 5/6 == 100% of 3/4 > 80%
of 5/6).
 
Karl
--
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
Felix Fietkau Nov. 20, 2013, 3:49 p.m. UTC | #5
On 2013-11-20 15:50, Karl Beldan wrote:
> On Wed, Nov 20, 2013 at 03:04:34PM +0100, Felix Fietkau wrote:
>> On 2013-11-20 14:56, Karl Beldan wrote:
>> > On Wed, Nov 20, 2013 at 08:32:32AM +0100, Felix Fietkau wrote:
>> >> On 2013-11-20 01:51, Karl Beldan wrote:
>> >> > From: Karl Beldan <karl.beldan@rivierawaves.com>
>> >> > 
>> >> > Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
>> >> > introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
>> >> > it to compute the rate throughput.
>> >> > 
>> >> > Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
>> >> > CC: Felix Fietkau <nbd@openwrt.org>
>> >> Nice catch!
>> >> Acked-by: Felix Fietkau <nbd@openwrt.org>
>> >> 
>> > Interestingly enough, consecutive coding rates (5/6, 3/4, 2/3) max ratio
>> > is 9/10, did you do it on purpose ?  (e.g. (9/10) * (5/6) == 3/4,
>> > (9/10) * (3/4) == 2/3 + 11/120).
>> The change has nothing to do with coding rates, it's only about
>> retransmissions caused by collisions under load.
>> 
> I understand this, my point was that along with this comes the following:
> let's say my SNR is just not so good to get 5/6 as good as 3/4, and e.g.
> case1 htMCS7 has  91% 
>       htMCS6 has 100% success 
> case2 htMCS7 has  80% 
>       htMCS6 has 100% success 
> capping at 90% will prefer htMCS7 in case1 and htMCS6 in case2 both
> achieving best real throughput.
> capping at 80% will prefer htMCS7 in case1 _but_ htMCS7 in case2 the
> latter being the worst real throughput(90% of 5/6 == 100% of 3/4 > 80%
> of 5/6).
Not sure if that's a meaningful comparison at all - you're leaving out
the per-packet overhead, which is important for the throughput
calculation as well.

- Felix

--
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
Karl Beldan Nov. 20, 2013, 4:19 p.m. UTC | #6
On Wed, Nov 20, 2013 at 04:49:55PM +0100, Felix Fietkau wrote:
> On 2013-11-20 15:50, Karl Beldan wrote:
> > On Wed, Nov 20, 2013 at 03:04:34PM +0100, Felix Fietkau wrote:
> >> On 2013-11-20 14:56, Karl Beldan wrote:
> >> > On Wed, Nov 20, 2013 at 08:32:32AM +0100, Felix Fietkau wrote:
> >> >> On 2013-11-20 01:51, Karl Beldan wrote:
> >> >> > From: Karl Beldan <karl.beldan@rivierawaves.com>
> >> >> > 
> >> >> > Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
> >> >> > introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
> >> >> > it to compute the rate throughput.
> >> >> > 
> >> >> > Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> >> >> > CC: Felix Fietkau <nbd@openwrt.org>
> >> >> Nice catch!
> >> >> Acked-by: Felix Fietkau <nbd@openwrt.org>
> >> >> 
> >> > Interestingly enough, consecutive coding rates (5/6, 3/4, 2/3) max ratio
> >> > is 9/10, did you do it on purpose ?  (e.g. (9/10) * (5/6) == 3/4,
> >> > (9/10) * (3/4) == 2/3 + 11/120).
> >> The change has nothing to do with coding rates, it's only about
> >> retransmissions caused by collisions under load.
> >> 
> > I understand this, my point was that along with this comes the following:
> > let's say my SNR is just not so good to get 5/6 as good as 3/4, and e.g.
> > case1 htMCS7 has  91% 
> >       htMCS6 has 100% success 
> > case2 htMCS7 has  80% 
> >       htMCS6 has 100% success 
> > capping at 90% will prefer htMCS7 in case1 and htMCS6 in case2 both
> > achieving best real throughput.
> > capping at 80% will prefer htMCS7 in case1 _but_ htMCS7 in case2 the
> > latter being the worst real throughput(90% of 5/6 == 100% of 3/4 > 80%
> > of 5/6).
> Not sure if that's a meaningful comparison at all - you're leaving out
> the per-packet overhead, which is important for the throughput
> calculation as well.
> 
The overhead breaks these numbers but the more we aggregate the more
this math is realistic as then the rates converge to these numbers ..
plus, IMHO using the overhead for throughput is wasteful since
throughputs are ranked and used relatively to each others and overhead
is shared by all rates.

 
Karl
--
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
Felix Fietkau Nov. 20, 2013, 4:57 p.m. UTC | #7
On 2013-11-20 08:32, Felix Fietkau wrote:
> On 2013-11-20 01:51, Karl Beldan wrote:
>> From: Karl Beldan <karl.beldan@rivierawaves.com>
>> 
>> Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
>> introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
>> it to compute the rate throughput.
>> 
>> Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
>> CC: Felix Fietkau <nbd@openwrt.org>
> Nice catch!
> Acked-by: Felix Fietkau <nbd@openwrt.org>
Sorry, I need to revoke that ACK - there's a bug in this patch:

> -	tp = 1000000 * ((mr->probability * 1000) / nsecs);
> +	tp = 1000000 * (prob * 1000) / nsecs;
Removing the outer () is wrong, it leads to overflow that makes all
throughput values 0.

- Felix
--
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
Karl Beldan Nov. 20, 2013, 5:03 p.m. UTC | #8
On Wed, Nov 20, 2013 at 05:57:06PM +0100, Felix Fietkau wrote:
> On 2013-11-20 08:32, Felix Fietkau wrote:
> > On 2013-11-20 01:51, Karl Beldan wrote:
> >> From: Karl Beldan <karl.beldan@rivierawaves.com>
> >> 
> >> Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
> >> introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
> >> it to compute the rate throughput.
> >> 
> >> Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> >> CC: Felix Fietkau <nbd@openwrt.org>
> > Nice catch!
> > Acked-by: Felix Fietkau <nbd@openwrt.org>
> Sorry, I need to revoke that ACK - there's a bug in this patch:
> 
> > -	tp = 1000000 * ((mr->probability * 1000) / nsecs);
> > +	tp = 1000000 * (prob * 1000) / nsecs;
> Removing the outer () is wrong, it leads to overflow that makes all
> throughput values 0.
> 
Arf, totally, the worst part is that I did know I had to keep them.
 
Karl
--
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
Karl Beldan Nov. 20, 2013, 5:04 p.m. UTC | #9
On Wed, Nov 20, 2013 at 05:57:06PM +0100, Felix Fietkau wrote:
> On 2013-11-20 08:32, Felix Fietkau wrote:
> > On 2013-11-20 01:51, Karl Beldan wrote:
> >> From: Karl Beldan <karl.beldan@rivierawaves.com>
> >> 
> >> Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
> >> introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
> >> it to compute the rate throughput.
> >> 
> >> Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> >> CC: Felix Fietkau <nbd@openwrt.org>
> > Nice catch!
> > Acked-by: Felix Fietkau <nbd@openwrt.org>
> Sorry, I need to revoke that ACK - there's a bug in this patch:
> 
> > -	tp = 1000000 * ((mr->probability * 1000) / nsecs);
> > +	tp = 1000000 * (prob * 1000) / nsecs;
> Removing the outer () is wrong, it leads to overflow that makes all
> throughput values 0.
> 
Do you handle this one or I resend ?
 
Karl
--
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
Felix Fietkau Nov. 20, 2013, 5:32 p.m. UTC | #10
On 2013-11-20 17:19, Karl Beldan wrote:
> On Wed, Nov 20, 2013 at 04:49:55PM +0100, Felix Fietkau wrote:
>> On 2013-11-20 15:50, Karl Beldan wrote:
>> > On Wed, Nov 20, 2013 at 03:04:34PM +0100, Felix Fietkau wrote:
>> >> On 2013-11-20 14:56, Karl Beldan wrote:
>> >> > On Wed, Nov 20, 2013 at 08:32:32AM +0100, Felix Fietkau wrote:
>> >> >> On 2013-11-20 01:51, Karl Beldan wrote:
>> >> >> > From: Karl Beldan <karl.beldan@rivierawaves.com>
>> >> >> > 
>> >> >> > Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
>> >> >> > introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
>> >> >> > it to compute the rate throughput.
>> >> >> > 
>> >> >> > Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
>> >> >> > CC: Felix Fietkau <nbd@openwrt.org>
>> >> >> Nice catch!
>> >> >> Acked-by: Felix Fietkau <nbd@openwrt.org>
>> >> >> 
>> >> > Interestingly enough, consecutive coding rates (5/6, 3/4, 2/3) max ratio
>> >> > is 9/10, did you do it on purpose ?  (e.g. (9/10) * (5/6) == 3/4,
>> >> > (9/10) * (3/4) == 2/3 + 11/120).
>> >> The change has nothing to do with coding rates, it's only about
>> >> retransmissions caused by collisions under load.
>> >> 
>> > I understand this, my point was that along with this comes the following:
>> > let's say my SNR is just not so good to get 5/6 as good as 3/4, and e.g.
>> > case1 htMCS7 has  91% 
>> >       htMCS6 has 100% success 
>> > case2 htMCS7 has  80% 
>> >       htMCS6 has 100% success 
>> > capping at 90% will prefer htMCS7 in case1 and htMCS6 in case2 both
>> > achieving best real throughput.
>> > capping at 80% will prefer htMCS7 in case1 _but_ htMCS7 in case2 the
>> > latter being the worst real throughput(90% of 5/6 == 100% of 3/4 > 80%
>> > of 5/6).
>> Not sure if that's a meaningful comparison at all - you're leaving out
>> the per-packet overhead, which is important for the throughput
>> calculation as well.
>> 
> The overhead breaks these numbers but the more we aggregate the more
> this math is realistic as then the rates converge to these numbers ..
> plus, IMHO using the overhead for throughput is wasteful since
> throughputs are ranked and used relatively to each others and overhead
> is shared by all rates.
The throughput metric (as displayed in debugfs) is calculated as:
tp = 10 ms * prob / (overhead_time / ampdu_len + packet_tx_time)

When you have two rates that are relatively close to each other, and the
faster rate is less reliable than the slower one, the throughput metric
can prefer the slower rate without aggregation, and the faster one with
aggregation.

The overhead may be shared between all rates, but that doesn't mean it
does not affect the relative comparison between rates.

- Felix

--
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
Felix Fietkau Nov. 20, 2013, 5:37 p.m. UTC | #11
On 2013-11-20 18:04, Karl Beldan wrote:
> On Wed, Nov 20, 2013 at 05:57:06PM +0100, Felix Fietkau wrote:
>> On 2013-11-20 08:32, Felix Fietkau wrote:
>> > On 2013-11-20 01:51, Karl Beldan wrote:
>> >> From: Karl Beldan <karl.beldan@rivierawaves.com>
>> >> 
>> >> Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
>> >> introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
>> >> it to compute the rate throughput.
>> >> 
>> >> Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
>> >> CC: Felix Fietkau <nbd@openwrt.org>
>> > Nice catch!
>> > Acked-by: Felix Fietkau <nbd@openwrt.org>
>> Sorry, I need to revoke that ACK - there's a bug in this patch:
>> 
>> > -	tp = 1000000 * ((mr->probability * 1000) / nsecs);
>> > +	tp = 1000000 * (prob * 1000) / nsecs;
>> Removing the outer () is wrong, it leads to overflow that makes all
>> throughput values 0.
>> 
> Do you handle this one or I resend ?
Please resend.

- Felix

--
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
Karl Beldan Nov. 20, 2013, 5:53 p.m. UTC | #12
On Wed, Nov 20, 2013 at 06:32:57PM +0100, Felix Fietkau wrote:
> On 2013-11-20 17:19, Karl Beldan wrote:
> > On Wed, Nov 20, 2013 at 04:49:55PM +0100, Felix Fietkau wrote:
> >> On 2013-11-20 15:50, Karl Beldan wrote:
> >> > On Wed, Nov 20, 2013 at 03:04:34PM +0100, Felix Fietkau wrote:
> >> >> On 2013-11-20 14:56, Karl Beldan wrote:
> >> >> > On Wed, Nov 20, 2013 at 08:32:32AM +0100, Felix Fietkau wrote:
> >> >> >> On 2013-11-20 01:51, Karl Beldan wrote:
> >> >> >> > From: Karl Beldan <karl.beldan@rivierawaves.com>
> >> >> >> > 
> >> >> >> > Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
> >> >> >> > introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
> >> >> >> > it to compute the rate throughput.
> >> >> >> > 
> >> >> >> > Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> >> >> >> > CC: Felix Fietkau <nbd@openwrt.org>
> >> >> >> Nice catch!
> >> >> >> Acked-by: Felix Fietkau <nbd@openwrt.org>
> >> >> >> 
> >> >> > Interestingly enough, consecutive coding rates (5/6, 3/4, 2/3) max ratio
> >> >> > is 9/10, did you do it on purpose ?  (e.g. (9/10) * (5/6) == 3/4,
> >> >> > (9/10) * (3/4) == 2/3 + 11/120).
> >> >> The change has nothing to do with coding rates, it's only about
> >> >> retransmissions caused by collisions under load.
> >> >> 
> >> > I understand this, my point was that along with this comes the following:
> >> > let's say my SNR is just not so good to get 5/6 as good as 3/4, and e.g.
> >> > case1 htMCS7 has  91% 
> >> >       htMCS6 has 100% success 
> >> > case2 htMCS7 has  80% 
> >> >       htMCS6 has 100% success 
> >> > capping at 90% will prefer htMCS7 in case1 and htMCS6 in case2 both
> >> > achieving best real throughput.
> >> > capping at 80% will prefer htMCS7 in case1 _but_ htMCS7 in case2 the
> >> > latter being the worst real throughput(90% of 5/6 == 100% of 3/4 > 80%
> >> > of 5/6).
> >> Not sure if that's a meaningful comparison at all - you're leaving out
> >> the per-packet overhead, which is important for the throughput
> >> calculation as well.
> >> 
> > The overhead breaks these numbers but the more we aggregate the more
> > this math is realistic as then the rates converge to these numbers ..
> > plus, IMHO using the overhead for throughput is wasteful since
> > throughputs are ranked and used relatively to each others and overhead
> > is shared by all rates.
> The throughput metric (as displayed in debugfs) is calculated as:
> tp = 10 ms * prob / (overhead_time / ampdu_len + packet_tx_time)
> 
> When you have two rates that are relatively close to each other, and the
> faster rate is less reliable than the slower one, the throughput metric
> can prefer the slower rate without aggregation, and the faster one with
> aggregation.
> 
> The overhead may be shared between all rates, but that doesn't mean it
> does not affect the relative comparison between rates.
> 
I did not say the overhead doesn't affect the relative comparison.
ampdu_len and overhead_time are shared by all the rates, what's the
purpose of computing overhead_time then ? since the rate selection is
only mere comparison of the said computed tps.
 
Karl
--
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
Felix Fietkau Nov. 20, 2013, 6:24 p.m. UTC | #13
On 2013-11-20 18:53, Karl Beldan wrote:
> On Wed, Nov 20, 2013 at 06:32:57PM +0100, Felix Fietkau wrote:
>> On 2013-11-20 17:19, Karl Beldan wrote:
>> > On Wed, Nov 20, 2013 at 04:49:55PM +0100, Felix Fietkau wrote:
>> >> On 2013-11-20 15:50, Karl Beldan wrote:
>> >> > On Wed, Nov 20, 2013 at 03:04:34PM +0100, Felix Fietkau wrote:
>> >> >> On 2013-11-20 14:56, Karl Beldan wrote:
>> >> >> > On Wed, Nov 20, 2013 at 08:32:32AM +0100, Felix Fietkau wrote:
>> >> >> >> On 2013-11-20 01:51, Karl Beldan wrote:
>> >> >> >> > From: Karl Beldan <karl.beldan@rivierawaves.com>
>> >> >> >> > 
>> >> >> >> > Commit 3e8b1eb "mac80211/minstrel_ht: improve rate selection stability"
>> >> >> >> > introduced a local capped prob in minstrel_ht_calc_tp but omitted to use
>> >> >> >> > it to compute the rate throughput.
>> >> >> >> > 
>> >> >> >> > Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
>> >> >> >> > CC: Felix Fietkau <nbd@openwrt.org>
>> >> >> >> Nice catch!
>> >> >> >> Acked-by: Felix Fietkau <nbd@openwrt.org>
>> >> >> >> 
>> >> >> > Interestingly enough, consecutive coding rates (5/6, 3/4, 2/3) max ratio
>> >> >> > is 9/10, did you do it on purpose ?  (e.g. (9/10) * (5/6) == 3/4,
>> >> >> > (9/10) * (3/4) == 2/3 + 11/120).
>> >> >> The change has nothing to do with coding rates, it's only about
>> >> >> retransmissions caused by collisions under load.
>> >> >> 
>> >> > I understand this, my point was that along with this comes the following:
>> >> > let's say my SNR is just not so good to get 5/6 as good as 3/4, and e.g.
>> >> > case1 htMCS7 has  91% 
>> >> >       htMCS6 has 100% success 
>> >> > case2 htMCS7 has  80% 
>> >> >       htMCS6 has 100% success 
>> >> > capping at 90% will prefer htMCS7 in case1 and htMCS6 in case2 both
>> >> > achieving best real throughput.
>> >> > capping at 80% will prefer htMCS7 in case1 _but_ htMCS7 in case2 the
>> >> > latter being the worst real throughput(90% of 5/6 == 100% of 3/4 > 80%
>> >> > of 5/6).
>> >> Not sure if that's a meaningful comparison at all - you're leaving out
>> >> the per-packet overhead, which is important for the throughput
>> >> calculation as well.
>> >> 
>> > The overhead breaks these numbers but the more we aggregate the more
>> > this math is realistic as then the rates converge to these numbers ..
>> > plus, IMHO using the overhead for throughput is wasteful since
>> > throughputs are ranked and used relatively to each others and overhead
>> > is shared by all rates.
>> The throughput metric (as displayed in debugfs) is calculated as:
>> tp = 10 ms * prob / (overhead_time / ampdu_len + packet_tx_time)
>> 
>> When you have two rates that are relatively close to each other, and the
>> faster rate is less reliable than the slower one, the throughput metric
>> can prefer the slower rate without aggregation, and the faster one with
>> aggregation.
>> 
>> The overhead may be shared between all rates, but that doesn't mean it
>> does not affect the relative comparison between rates.
>> 
> I did not say the overhead doesn't affect the relative comparison.
> ampdu_len and overhead_time are shared by all the rates, what's the
> purpose of computing overhead_time then ? since the rate selection is
> only mere comparison of the said computed tps.
Right, I guess we could add a mi->overhead_ampdu that gets adjusted
based on the average ampdu length before recalculating all rates.

- Felix
--
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 mbox

Patch

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 9702d88..341e6f7 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -333,7 +333,7 @@  minstrel_ht_calc_tp(struct minstrel_ht_sta *mi, int group, int rate)
 		nsecs = 1000 * mi->overhead / MINSTREL_TRUNC(mi->avg_ampdu_len);
 
 	nsecs += minstrel_mcs_groups[group].duration[rate];
-	tp = 1000000 * ((mr->probability * 1000) / nsecs);
+	tp = 1000000 * (prob * 1000) / nsecs;
 
 	mr->cur_tp = MINSTREL_TRUNC(tp);
 }