* [ath9k-devel] [PATCH] ath9k: Configure beacons for AP vif if this has not happened yet
2015-03-13 14:27 ` Felix Fietkau
@ 2015-03-13 15:37 ` Benjamin Berg
2015-03-13 15:51 ` Ben Greear
1 sibling, 0 replies; 5+ messages in thread
From: Benjamin Berg @ 2015-03-13 15:37 UTC (permalink / raw)
To: ath9k-devel
On Fr, 2015-03-13 at 15:27 +0100, Felix Fietkau wrote:
> I sent the following patch yesterday - I think it addresses the exact
> same issue. Please test it to see if it works for you as well.
The patch looks a lot better. I just backported it and it is working
just fine for me.
Benjamin
> Subject: [PATCH 4.0] ath9k: fix tracking of enabled AP beacons
>
> sc->nbcnvifs tracks assigned beacon slots, not enabled beacons.
> Therefore, it cannot be used to decide if cur_conf->enable_beacon (bool)
> should be updated, or if beacons have been enabled already.
> With the current code (depending on the order of calls), beacons often
> do not get enabled in an AP+STA setup.
> To fix tracking of enabled beacons, convert cur_conf->enable_beacon to a
> bitmask of enabled beacon slots.
>
> Cc: stable at vger.kernel.org
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
> drivers/net/wireless/ath/ath9k/beacon.c | 20 ++++++++++++--------
> drivers/net/wireless/ath/ath9k/common.h | 2 +-
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
> index cb366ad..f50a6bc 100644
> --- a/drivers/net/wireless/ath/ath9k/beacon.c
> +++ b/drivers/net/wireless/ath/ath9k/beacon.c
> @@ -219,12 +219,15 @@ void ath9k_beacon_remove_slot(struct ath_softc *sc, struct ieee80211_vif *vif)
> struct ath_common *common = ath9k_hw_common(sc->sc_ah);
> struct ath_vif *avp = (void *)vif->drv_priv;
> struct ath_buf *bf = avp->av_bcbuf;
> + struct ath_beacon_config *cur_conf = &sc->cur_chan->beacon;
>
> ath_dbg(common, CONFIG, "Removing interface at beacon slot: %d\n",
> avp->av_bslot);
>
> tasklet_disable(&sc->bcon_tasklet);
>
> + cur_conf->enable_beacon &= ~BIT(avp->av_bslot);
> +
> if (bf && bf->bf_mpdu) {
> struct sk_buff *skb = bf->bf_mpdu;
> dma_unmap_single(sc->dev, bf->bf_buf_addr,
> @@ -521,8 +524,7 @@ static bool ath9k_allow_beacon_config(struct ath_softc *sc,
> }
>
> if (sc->sc_ah->opmode == NL80211_IFTYPE_AP) {
> - if ((vif->type != NL80211_IFTYPE_AP) ||
> - (sc->nbcnvifs > 1)) {
> + if (vif->type != NL80211_IFTYPE_AP) {
> ath_dbg(common, CONFIG,
> "An AP interface is already present !\n");
> return false;
> @@ -616,12 +618,14 @@ void ath9k_beacon_config(struct ath_softc *sc, struct ieee80211_vif *vif,
> * enabling/disabling SWBA.
> */
> if (changed & BSS_CHANGED_BEACON_ENABLED) {
> - if (!bss_conf->enable_beacon &&
> - (sc->nbcnvifs <= 1)) {
> - cur_conf->enable_beacon = false;
> - } else if (bss_conf->enable_beacon) {
> - cur_conf->enable_beacon = true;
> - ath9k_cache_beacon_config(sc, ctx, bss_conf);
> + bool enabled = cur_conf->enable_beacon;
> +
> + if (!bss_conf->enable_beacon) {
> + cur_conf->enable_beacon &= ~BIT(avp->av_bslot);
> + } else {
> + cur_conf->enable_beacon |= BIT(avp->av_bslot);
> + if (!enabled)
> + ath9k_cache_beacon_config(sc, ctx, bss_conf);
> }
> }
>
> diff --git a/drivers/net/wireless/ath/ath9k/common.h b/drivers/net/wireless/ath/ath9k/common.h
> index 2b79a56..d237373 100644
> --- a/drivers/net/wireless/ath/ath9k/common.h
> +++ b/drivers/net/wireless/ath/ath9k/common.h
> @@ -54,7 +54,7 @@ struct ath_beacon_config {
> u16 dtim_period;
> u16 bmiss_timeout;
> u8 dtim_count;
> - bool enable_beacon;
> + u8 enable_beacon;
> bool ibss_creator;
> u32 nexttbtt;
> u32 intval;
> -- 2.2.2 --
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20150313/3efb06f4/attachment-0001.pgp
^ permalink raw reply [flat|nested] 5+ messages in thread* [ath9k-devel] [PATCH] ath9k: Configure beacons for AP vif if this has not happened yet
2015-03-13 14:27 ` Felix Fietkau
2015-03-13 15:37 ` Benjamin Berg
@ 2015-03-13 15:51 ` Ben Greear
1 sibling, 0 replies; 5+ messages in thread
From: Ben Greear @ 2015-03-13 15:51 UTC (permalink / raw)
To: ath9k-devel
Any idea how far back this needs to be backported for stable?
Just to 4.0?
Thanks,
Ben
On 03/13/2015 07:27 AM, Felix Fietkau wrote:
> On 2015-03-13 14:33, Benjamin Berg wrote:
>> Right now there is a bug where beaconing might not be enabled correctly
>> if the user has configuring multiple VIFs.
>> The issue surfaces if the userspace first creates the AP devices and
>> only then configures the first VIF (ath9k_bss_info_changed is called).
>> In this case the current ath9k_allow_beacon_config implementation will
>> not allow configuration as multiple AP VIFs are already present and
>> beaconing is never configured in the driver.
>>
>> This issue was probably introduced back in 2012 by commit ef4ad6336
>> "ath9k: Cleanup beacon logic". The fix in this patch simply checks
>> whether beaconing has been configured yet (or configuration is scheduled)
>> and allows the configuration in that case. This works around the issue
>> here, but I have no idea whether it is a sane solution.
>>
>> Signed-off-by: Benjamin Berg <benjamin@sipsolutions.net>
> I sent the following patch yesterday - I think it addresses the exact
> same issue. Please test it to see if it works for you as well.
>
> Subject: [PATCH 4.0] ath9k: fix tracking of enabled AP beacons
>
> sc->nbcnvifs tracks assigned beacon slots, not enabled beacons.
> Therefore, it cannot be used to decide if cur_conf->enable_beacon (bool)
> should be updated, or if beacons have been enabled already.
> With the current code (depending on the order of calls), beacons often
> do not get enabled in an AP+STA setup.
> To fix tracking of enabled beacons, convert cur_conf->enable_beacon to a
> bitmask of enabled beacon slots.
>
> Cc: stable at vger.kernel.org
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
> drivers/net/wireless/ath/ath9k/beacon.c | 20 ++++++++++++--------
> drivers/net/wireless/ath/ath9k/common.h | 2 +-
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
> index cb366ad..f50a6bc 100644
> --- a/drivers/net/wireless/ath/ath9k/beacon.c
> +++ b/drivers/net/wireless/ath/ath9k/beacon.c
> @@ -219,12 +219,15 @@ void ath9k_beacon_remove_slot(struct ath_softc *sc, struct ieee80211_vif *vif)
> struct ath_common *common = ath9k_hw_common(sc->sc_ah);
> struct ath_vif *avp = (void *)vif->drv_priv;
> struct ath_buf *bf = avp->av_bcbuf;
> + struct ath_beacon_config *cur_conf = &sc->cur_chan->beacon;
>
> ath_dbg(common, CONFIG, "Removing interface at beacon slot: %d\n",
> avp->av_bslot);
>
> tasklet_disable(&sc->bcon_tasklet);
>
> + cur_conf->enable_beacon &= ~BIT(avp->av_bslot);
> +
> if (bf && bf->bf_mpdu) {
> struct sk_buff *skb = bf->bf_mpdu;
> dma_unmap_single(sc->dev, bf->bf_buf_addr,
> @@ -521,8 +524,7 @@ static bool ath9k_allow_beacon_config(struct ath_softc *sc,
> }
>
> if (sc->sc_ah->opmode == NL80211_IFTYPE_AP) {
> - if ((vif->type != NL80211_IFTYPE_AP) ||
> - (sc->nbcnvifs > 1)) {
> + if (vif->type != NL80211_IFTYPE_AP) {
> ath_dbg(common, CONFIG,
> "An AP interface is already present !\n");
> return false;
> @@ -616,12 +618,14 @@ void ath9k_beacon_config(struct ath_softc *sc, struct ieee80211_vif *vif,
> * enabling/disabling SWBA.
> */
> if (changed & BSS_CHANGED_BEACON_ENABLED) {
> - if (!bss_conf->enable_beacon &&
> - (sc->nbcnvifs <= 1)) {
> - cur_conf->enable_beacon = false;
> - } else if (bss_conf->enable_beacon) {
> - cur_conf->enable_beacon = true;
> - ath9k_cache_beacon_config(sc, ctx, bss_conf);
> + bool enabled = cur_conf->enable_beacon;
> +
> + if (!bss_conf->enable_beacon) {
> + cur_conf->enable_beacon &= ~BIT(avp->av_bslot);
> + } else {
> + cur_conf->enable_beacon |= BIT(avp->av_bslot);
> + if (!enabled)
> + ath9k_cache_beacon_config(sc, ctx, bss_conf);
> }
> }
>
> diff --git a/drivers/net/wireless/ath/ath9k/common.h b/drivers/net/wireless/ath/ath9k/common.h
> index 2b79a56..d237373 100644
> --- a/drivers/net/wireless/ath/ath9k/common.h
> +++ b/drivers/net/wireless/ath/ath9k/common.h
> @@ -54,7 +54,7 @@ struct ath_beacon_config {
> u16 dtim_period;
> u16 bmiss_timeout;
> u8 dtim_count;
> - bool enable_beacon;
> + u8 enable_beacon;
> bool ibss_creator;
> u32 nexttbtt;
> u32 intval;
> -- 2.2.2 --
> _______________________________________________
> ath9k-devel mailing list
> ath9k-devel at lists.ath9k.org
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 5+ messages in thread