2008-11-17 15:08:33

by Larry Finger

[permalink] [raw]
Subject: [PATCH] rtl8187: Fix transmission count sent to mac80211

In the commit entitled "mac80211/drivers: rewrite the rate control API"
(commit 9ea2c74de0ec971e8ec9fc5aaea9cd5b4fec95b6), the meaning of the packet
transmit count was changed from the number of retries to the total number.
In driver rtl8187, this change was missed.

Signed-off-by: Larry Finger <[email protected]>
---

Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_dev.c
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
@@ -488,7 +488,7 @@ static void rtl8187b_status_cb(struct ur
__skb_unlink(skb, &priv->b_tx_status.queue);
if (tok)
info->flags |= IEEE80211_TX_STAT_ACK;
- info->status.rates[0].count = pkt_rc;
+ info->status.rates[0].count = pkt_rc + 1;

ieee80211_tx_status_irqsafe(hw, skb);
}


Subject: Re: [PATCH] rtl8187: Fix transmission count sent to mac80211

On Monday 17 November 2008 17:47:07 Johannes Berg wrote:
> On Mon, 2008-11-17 at 17:42 -0200, Herton Ronaldo Krzesinski wrote:
> > On Monday 17 November 2008 13:08:21 Larry Finger wrote:
> > > In the commit entitled "mac80211/drivers: rewrite the rate control API"
> > > (commit 9ea2c74de0ec971e8ec9fc5aaea9cd5b4fec95b6), the meaning of the
> > > packet transmit count was changed from the number of retries to the
> > > total number. In driver rtl8187, this change was missed.
> >
> > With this change as before, when using pid, rate never go above 1M in
> > testing here (minstrel doesn't have the same issue). May be because this
> > in rate_control_pid_tx_status (inside rc80211_pid_algo.c)?
> >
> > if (!(info->flags & IEEE80211_TX_STAT_ACK)) {
> > spinfo->tx_num_failed += 2;
> > spinfo->tx_num_xmit++;
> > } else if (info->status.rates[0].count) {
> > spinfo->tx_num_failed++;
> > spinfo->tx_num_xmit++;
> > }
> >
> > as pkt_rc + 1, looks like we always will increment tx_num_failed, and may
> > be it's causing this. Well this is only judging by a quick look, I may be
> > wrong.
>
> Larry recently fixed the above code too.

Ah nice, I didn't saw it, I'm usually lagging reading mail... I saw it now.

Acked-by: Herton Ronaldo Krzesinski <[email protected]>

>
> johannes

--
[]'s
Herton

2008-11-17 21:19:12

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Fix transmission count sent to mac80211

On Mon, Nov 17, 2008 at 8:29 PM, Herton Ronaldo Krzesinski
<[email protected]> wrote:
<sniped>
> Ah nice, I didn't saw it, I'm usually lagging reading mail... I saw it now.
>
> Acked-by: Herton Ronaldo Krzesinski <[email protected]>
Tested-by: Hin-Tak Leung <[email protected]>

Tried the one-liner change on top of wireless-testing. Works alright
- no noticeable regression.

BTW, i don't check my gmail account as often than the other account
(and the sf accounts redirects
to that other account).

Hin-Tak

Subject: Re: [PATCH] rtl8187: Fix transmission count sent to mac80211

On Monday 17 November 2008 13:08:21 Larry Finger wrote:
> In the commit entitled "mac80211/drivers: rewrite the rate control API"
> (commit 9ea2c74de0ec971e8ec9fc5aaea9cd5b4fec95b6), the meaning of the
> packet transmit count was changed from the number of retries to the total
> number. In driver rtl8187, this change was missed.

With this change as before, when using pid, rate never go above 1M in testing
here (minstrel doesn't have the same issue). May be because this in
rate_control_pid_tx_status (inside rc80211_pid_algo.c)?

if (!(info->flags & IEEE80211_TX_STAT_ACK)) {
spinfo->tx_num_failed += 2;
spinfo->tx_num_xmit++;
} else if (info->status.rates[0].count) {
spinfo->tx_num_failed++;
spinfo->tx_num_xmit++;
}

as pkt_rc + 1, looks like we always will increment tx_num_failed, and may be
it's causing this. Well this is only judging by a quick look, I may be wrong.

>
> Signed-off-by: Larry Finger <[email protected]>
> ---
>
> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_dev.c
> +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
> @@ -488,7 +488,7 @@ static void rtl8187b_status_cb(struct ur
> __skb_unlink(skb, &priv->b_tx_status.queue);
> if (tok)
> info->flags |= IEEE80211_TX_STAT_ACK;
> - info->status.rates[0].count = pkt_rc;
> + info->status.rates[0].count = pkt_rc + 1;
>
> ieee80211_tx_status_irqsafe(hw, skb);
> }

--
[]'s
Herton

2008-11-17 19:47:53

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Fix transmission count sent to mac80211

On Mon, 2008-11-17 at 17:42 -0200, Herton Ronaldo Krzesinski wrote:
> On Monday 17 November 2008 13:08:21 Larry Finger wrote:
> > In the commit entitled "mac80211/drivers: rewrite the rate control API"
> > (commit 9ea2c74de0ec971e8ec9fc5aaea9cd5b4fec95b6), the meaning of the
> > packet transmit count was changed from the number of retries to the total
> > number. In driver rtl8187, this change was missed.
>
> With this change as before, when using pid, rate never go above 1M in testing
> here (minstrel doesn't have the same issue). May be because this in
> rate_control_pid_tx_status (inside rc80211_pid_algo.c)?
>
> if (!(info->flags & IEEE80211_TX_STAT_ACK)) {
> spinfo->tx_num_failed += 2;
> spinfo->tx_num_xmit++;
> } else if (info->status.rates[0].count) {
> spinfo->tx_num_failed++;
> spinfo->tx_num_xmit++;
> }
>
> as pkt_rc + 1, looks like we always will increment tx_num_failed, and may be
> it's causing this. Well this is only judging by a quick look, I may be wrong.

Larry recently fixed the above code too.

johannes


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