Search Linux Wireless

Re: [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 14, 2010 at 1:06 PM, Luis R. Rodriguez
<lrodriguez@xxxxxxxxxxx> wrote:
> On Tue, Sep 14, 2010 at 10:56:14AM -0700, Luis R. Rodriguez wrote:
>> On Tue, Sep 14, 2010 at 9:42 AM, Luis R. Rodriguez
>> <lrodriguez@xxxxxxxxxxx> wrote:
>> > On Tue, Sep 14, 2010 at 9:39 AM, Luis R. Rodriguez
>> > <lrodriguez@xxxxxxxxxxx> wrote:
>> >> On Tue, Sep 14, 2010 at 8:00 AM, Luis R. Rodriguez
>> >> <lrodriguez@xxxxxxxxxxx> wrote:
>> >>> When we return to the home channel we were never reseting our beacon
>> >>> timers, this was casued by the fact that the scanning flag was still
>> >>> on even after we returned to our home channel. There are also other
>> >>> reasons why we would get a reset and if we are not off channel
>> >>> we always need to resynch our beacon timers, because a reset will
>> >>> clear them.
>> >>>
>> >>> For more details refer to:
>> >>>
>> >>> http://code.google.com/p/chromium-os/issues/detail?id=5715
>> >>>
>> >>> This bug is a regression introduced on 2.6.36. The order of the
>> >>> changes are as follows:
>> >>>
>> >>> 5ee08656 - Sat Jul 31 - ath9k: prevent calibration during off-channel activity
>> >>> a0daa0e7 - Tue Jul 27 - Revert "mac80211: fix sw scan bracketing"
>> >>> 543708be - Fri Jun 18 - mac80211: fix sw scan bracketing
>> >>>
>> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> >>>        --contains 5ee0865615f65f84e6ee9174771a6716c29e08e1
>> >>> v2.6.36-rc1~43^2~34^2~22
>> >>>
>> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> >>>        --contains a0daa0e7592ada797d6835f11529097aabc27ad2
>> >>> v2.6.36-rc1~571^2~64^2~13
>> >>>
>> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> >>>        --contains 543708be320d7df692d24b349ca01a947b340764
>> >>> v2.6.36-rc1~571^2~107^2~187
>> >>>
>> >>> So 5ee08656 would have worked if a0daa0e7 was not committed but
>> >>> it was so this means 5ee08656 was broken since it assumed that
>> >>> when we were in the channel change routine the scan flag would
>> >>> be lifted. As it turns out the scan flag will be set when we
>> >>> are already on the home channel.
>> >>>
>> >>> These issues will need to be considered for our solution on
>> >>> reshifting the scan complete callback location on mac80211 on
>> >>> current development kernel work.
>> >>>
>> >>> This patch has stable fixes which apply down to [2.6.36+]
>> >>>
>> >>> Cc: stable@xxxxxxxxxx
>> >>> Cc: Paul Stewart <pstew@xxxxxxxxxx>
>> >>> Cc: Amod Bodas <amod.bodas@xxxxxxxxxxx>
>> >>> Signed-off-by: Luis R. Rodriguez <lrodriguez@xxxxxxxxxxx>
>> >>> ---
>> >>>
>> >>> This v2 clarifies the second to last sentence, on the v1 this
>> >>> was just jiberish on my first read.. this should clarify what
>> >>> I meant.
>> >>>
>> >>>  drivers/net/wireless/ath/ath9k/main.c |    6 ++++--
>> >>>  1 files changed, 4 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> >>> index ba029b2..e349619 100644
>> >>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> >>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> >>> @@ -258,9 +258,11 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
>> >>>        if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
>> >>>                ath_start_ani(common);
>> >>>                ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
>> >>> -               ath_beacon_config(sc, NULL);
>> >>>        }
>> >>>
>> >>> +       if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)))
>> >>> +               ath_beacon_config(sc, NULL);
>> >>> +
>> >>
>> >> Upon further thought this change should just remove the SC_OP_SCANNING
>> >> flag from above and let the code there run when we are not off
>> >> channel. Reason is that as I noted above on the commit log entry the
>> >> scan flag will be set even when we return to the home channel due to
>> >> the races in mac80211 that were actually fixed by Johannes but later
>> >> reverted due to issues. Doing the fix this way would also let us
>> >> re-arm ANI and the tx completion monitor, this was also broken! I'll
>> >> respin this patch shortly after some basic testing.
>> >
>> > I should also note that this patch fixes the issue of reseting the
>> > beacon timers upon a reset, likely when the tx monitor hits -- I
>> > suspect that when that currently would hit we'd actually be out of
>> > synch with the AP and disconnect. This patch fixes that, but I just
>> > reviewed older kernels and the offchannel flag was not available prior
>> > to 2.6.36 so we can only fix this properly on >= 2.6.36 unless we
>> > figure out a way to annotate we're on the home channel. If we do we
>> > can potentially propagate a fix down to older kernels than 2.6.36
>> > eventually.
>>
>> Fun, just noticed these would be busted too :)
>>
>>         ath9k_hw_startpcureceive(ah, (sc->sc_flags & SC_OP_SCANNING));
>>
>> Ah, joy.
>
> More joy, so I tried this as an alternative:
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 5fe570b..ecd6724 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -563,7 +563,6 @@ struct ath_ant_comb {
>  #define SC_OP_RXFLUSH                BIT(7)
>  #define SC_OP_LED_ASSOCIATED         BIT(8)
>  #define SC_OP_LED_ON                 BIT(9)
> -#define SC_OP_SCANNING               BIT(10)
>  #define SC_OP_TSF_RESET              BIT(11)
>  #define SC_OP_BT_PRIORITY_DETECTED   BIT(12)
>  #define SC_OP_BT_SCAN               BIT(13)
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index ba029b2..2207cf2 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -255,7 +255,7 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
>        ath_update_txpow(sc);
>        ath9k_hw_set_interrupts(ah, ah->imask);
>
> -       if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
> +       if (!(sc->sc_flags & SC_OP_OFFCHANNEL)) {
>                ath_start_ani(common);
>                ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
>                ath_beacon_config(sc, NULL);
> @@ -957,7 +957,7 @@ int ath_reset(struct ath_softc *sc, bool retry_tx)
>
>        ath_update_txpow(sc);
>
> -       if (sc->sc_flags & SC_OP_BEACONS)
> +       if ((sc->sc_flags & SC_OP_BEACONS) || !(sc->sc_flags & (SC_OP_OFFCHANNEL)))
>                ath_beacon_config(sc, NULL);    /* restart beacons */
>
>        ath9k_hw_set_interrupts(ah, ah->imask);
> @@ -2042,7 +2042,6 @@ static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
>
>        aphy->state = ATH_WIPHY_SCAN;
>        ath9k_wiphy_pause_all_forced(sc, aphy);
> -       sc->sc_flags |= SC_OP_SCANNING;
>        mutex_unlock(&sc->mutex);
>  }
>
> @@ -2057,7 +2056,6 @@ static void ath9k_sw_scan_complete(struct ieee80211_hw *hw)
>
>        mutex_lock(&sc->mutex);
>        aphy->state = ATH_WIPHY_ACTIVE;
> -       sc->sc_flags &= ~SC_OP_SCANNING;
>        mutex_unlock(&sc->mutex);
>  }
>
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index 87e02f5..6f7d4d1 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -301,7 +301,7 @@ static void ath_edma_start_recv(struct ath_softc *sc)
>
>        ath_opmode_init(sc);
>
> -       ath9k_hw_startpcureceive(sc->sc_ah, (sc->sc_flags & SC_OP_SCANNING));
> +       ath9k_hw_startpcureceive(sc->sc_ah, !!(sc->sc_flags & SC_OP_OFFCHANNEL));
>  }
>
>  static void ath_edma_stop_recv(struct ath_softc *sc)
> @@ -507,7 +507,7 @@ int ath_startrecv(struct ath_softc *sc)
>  start_recv:
>        spin_unlock_bh(&sc->rx.rxbuflock);
>        ath_opmode_init(sc);
> -       ath9k_hw_startpcureceive(ah, (sc->sc_flags & SC_OP_SCANNING));
> +       ath9k_hw_startpcureceive(ah, !!(sc->sc_flags & SC_OP_OFFCHANNEL));
>
>        return 0;
>  }
>
>
> But the harware goes nutty. Although technically it seems correct

I manged to figure out the issue, ANI needs to be started *after* the
beacon config. I'm going to split up these two fixes up separatey
though as one fixes the beacon loss as a regression, the other fixes a
long time issue of having left ANI completely disabled when doing a bg
scan.

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


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux