Re: 2.6.25-rc2 regression in rt61pci wireless driver

From: Chris Clayton
Date: Sun Mar 02 2008 - 10:12:46 EST

On 02/03/2008, Stefano Brivio <stefano.brivio@xxxxxxxxx> wrote:
> On Wed, 27 Feb 2008 12:25:46 -0500
> "John W. Linville" <linville@xxxxxxxxxxxxx> wrote:
>
> > That might be easier said than done. It looks like that patch depends
> > on the big cfg80211 API change queued for 2.6.26.
> >
> > Stefano offered to rebase that on 2.6.25. Stefano, could you post
> > that as part of this thread?
>
>
> Sorry for the delay. This is based on 2.6.25-rc3. Please test.
>

I've tested this with the pid algorithm selected and my wireless
network connection is reliable. In a loop, I repeatedly ftp'd a kernel
source tarball from another box on my network for 40 minutes with no
failure, whilst at the same time, pinging my gateway. Without the
patch, that activity would have led to network failure in seconds. The
tests were with the patch applied to 2.6.25-rc3-git3, so that Ivo's
rt2x00 patches for 2.6.25 are also applied.

Thanks to everyone who has helped fix this.

> ---
>
> Merge rate_control_pid_shift_adjust() to rate_control_pid_adjust_rate()
> in order to make the learning algorithm aware of constraints on rates. Also
> add some comments and rename variables.
>
> This fixes a bug which prevented 802.11b/g non-AP STAs from working with
> 802.11b only AP STAs.
>
> Signed-off-by: Stefano Brivio <stefano.brivio@xxxxxxxxx>

Tested-by: Chris Clayton <chris2553@xxxxxxxxxxxxxx>

> ---
> Index: linux-2.6.24/net/mac80211/rc80211_pid_algo.c
> ===================================================================
> --- linux-2.6.24.orig/net/mac80211/rc80211_pid_algo.c
> +++ linux-2.6.24/net/mac80211/rc80211_pid_algo.c
> @@ -2,7 +2,7 @@
> * Copyright 2002-2005, Instant802 Networks, Inc.
> * Copyright 2005, Devicescape Software, Inc.
> * Copyright 2007, Mattias Nissler <mattias.nissler@xxxxxx>
> - * Copyright 2007, Stefano Brivio <stefano.brivio@xxxxxxxxx>
> + * Copyright 2007-2008, Stefano Brivio <stefano.brivio@xxxxxxxxx>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -63,72 +63,66 @@
> * RC_PID_ARITH_SHIFT.
> */
>
> -
> -/* Shift the adjustment so that we won't switch to a lower rate if it exhibited
> - * a worse failed frames behaviour and we'll choose the highest rate whose
> - * failed frames behaviour is not worse than the one of the original rate
> - * target. While at it, check that the adjustment is within the ranges. Then,
> - * provide the new rate index. */
> -static int rate_control_pid_shift_adjust(struct rc_pid_rateinfo *r,
> - int adj, int cur, int l)
> -{
> - int i, j, k, tmp;
> -
> - j = r[cur].rev_index;
> - i = j + adj;
> -
> - if (i < 0)
> - return r[0].index;
> - if (i >= l - 1)
> - return r[l - 1].index;
> -
> - tmp = i;
> -
> - if (adj < 0) {
> - for (k = j; k >= i; k--)
> - if (r[k].diff <= r[j].diff)
> - tmp = k;
> - } else {
> - for (k = i + 1; k + i < l; k++)
> - if (r[k].diff <= r[i].diff)
> - tmp = k;
> - }
> -
> - return r[tmp].index;
> -}
> -
> +/* Adjust the rate while ensuring that we won't switch to a lower rate if it
> + * exhibited a worse failed frames behaviour and we'll choose the highest rate
> + * whose failed frames behaviour is not worse than the one of the original rate
> + * target. While at it, check that the new rate is valid. */
> static void rate_control_pid_adjust_rate(struct ieee80211_local *local,
> struct sta_info *sta, int adj,
> struct rc_pid_rateinfo *rinfo)
> {
> struct ieee80211_sub_if_data *sdata;
> struct ieee80211_hw_mode *mode;
> - int newidx;
> - int maxrate;
> - int back = (adj > 0) ? 1 : -1;
> + int cur_sorted, new_sorted, probe, tmp, n_bitrates;
> + int cur = sta->txrate;
>
> sdata = IEEE80211_DEV_TO_SUB_IF(sta->dev);
>
> mode = local->oper_hw_mode;
> - maxrate = sdata->bss ? sdata->bss->max_ratectrl_rateidx : -1;
> + n_bitrates = mode->num_rates;
>
> - newidx = rate_control_pid_shift_adjust(rinfo, adj, sta->txrate,
> - mode->num_rates);
> + /* Map passed arguments to sorted values. */
> + cur_sorted = rinfo[cur].rev_index;
> + new_sorted = cur_sorted + adj;
> +
> + /* Check limits. */
> + if (new_sorted < 0)
> + new_sorted = rinfo[0].rev_index;
> + else if (new_sorted >= n_bitrates)
> + new_sorted = rinfo[n_bitrates - 1].rev_index;
>
> - while (newidx != sta->txrate) {
> - if (rate_supported(sta, mode, newidx) &&
> - (maxrate < 0 || newidx <= maxrate)) {
> - sta->txrate = newidx;
> - break;
> - }
> + tmp = new_sorted;
>
> - newidx += back;
> + if (adj < 0) {
> + /* Ensure that the rate decrease isn't disadvantageous. */
> + for (probe = cur_sorted; probe >= new_sorted; probe--)
> + if (rinfo[probe].diff <= rinfo[cur_sorted].diff &&
> + rate_supported(sta, mode, rinfo[probe].index))
> + tmp = probe;
> + } else {
> + /* Look for rate increase with zero (or below) cost. */
> + for (probe = new_sorted + 1; probe < n_bitrates; probe++)
> + if (rinfo[probe].diff <= rinfo[new_sorted].diff &&
> + rate_supported(sta, mode, rinfo[probe].index))
> + tmp = probe;
> }
>
> + /* Fit the rate found to the nearest supported rate. */
> + do {
> + if (rate_supported(sta, mode, rinfo[tmp].index)) {
> + sta->txrate = rinfo[tmp].index;
> + break;
> + }
> + if (adj < 0)
> + tmp--;
> + else
> + tmp++;
> + } while (tmp < n_bitrates && tmp >= 0);
> +
> #ifdef CONFIG_MAC80211_DEBUGFS
> rate_control_pid_event_rate_change(
> &((struct rc_pid_sta_info *)sta->rate_ctrl_priv)->events,
> - newidx, mode->rates[newidx].rate);
> + cur, mode->rates[cur].rate);
> #endif
> }
>
>
>
> --
> Ciao
>
> Stefano
>


--
Beauty is in the eye of the beerholder.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/