Re: [PATCH v4] ath5k: Allow ath5k to support virtual STA and AP interfaces.

[Bruno Randolf <br1@einfach.org>]

On Tue September 28 2010 06:06:28 greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> Support up to 4 virtual APs and as many virtual STA interfaces
> as desired.
> 
> This patch is ported forward from a patch that Patrick McHardy
> did for me against 2.6.31.

hi ben!

it's great to see this! :) i tested it today, but only using ping, and at it 
seemed to work fine with two WPA and two non-encrypted access points! but 
unfortunately, i have also seen some instablility, and problems like:

hostapd:
AP-STA-DISCONNECTED 00:0e:8e:25:92:7d
Station 00:0e:8e:25:92:7d trying to deauthenticate, but it is not 
authenticated.
handle_auth_cb: STA 00:0e:8e:25:92:7d not found
handle_assoc_cb: STA 00:0e:8e:25:92:7d not found

and:

Could not set station 00:0e:8e:25:92:7d flags for kernel driver (errno=97).
handle_auth_cb: STA 00:0e:8e:25:92:7d not found

that might just have been some problems in my setup, but i think we need to 
test this further before it can get merged...

also your patch breaks beaconing in ad-hoc mode, so that needs to be fixed as 
well.

below are a few comments on the code:

> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> 
> :100644 100644 95072db... 23a693d...
> :M	drivers/net/wireless/ath/ath5k/base.c 100644 100644 7f9d0d3...
> :7f91032... M	drivers/net/wireless/ath/ath5k/base.h 100644 100644
> :58912cd... 5b179d0... M	drivers/net/wireless/ath/ath5k/reset.c
> 
>  drivers/net/wireless/ath/ath5k/base.c  |  271
> ++++++++++++++++++++++++++----- drivers/net/wireless/ath/ath5k/base.h  |  
> 22 ++-
>  drivers/net/wireless/ath/ath5k/reset.c |    4 +-
>  3 files changed, 246 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath5k/base.c
> b/drivers/net/wireless/ath/ath5k/base.c index 95072db..23a693d 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -52,6 +52,7 @@
>  #include <linux/ethtool.h>
>  #include <linux/uaccess.h>
>  #include <linux/slab.h>
> +#include <linux/etherdevice.h>
> 
>  #include <net/ieee80211_radiotap.h>
> 
> @@ -509,8 +510,69 @@ ath5k_setcurmode(struct ath5k_softc *sc, unsigned int
> mode) }
>  }
> 
> +struct ath_vif_iter_data {
> +	const u8 *hw_macaddr;
> +	u8 mask[ETH_ALEN];
> +	u8 active_mac[ETH_ALEN]; /* first active MAC */
> +	bool need_set_hw_addr;
> +	bool found_active;
> +	bool any_assoc;
> +};

please tabs...

> +static void ath_vif_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
> +{
> +	struct ath_vif_iter_data *iter_data = data;
> +	int i;
> +
> +	for (i = 0; i < ETH_ALEN; i++)
> +		iter_data->mask[i] &= ~(iter_data->hw_macaddr[i] ^ mac[i]);
> +
> +	if (!iter_data->found_active) {
> +		iter_data->found_active = true;
> +		memcpy(iter_data->active_mac, mac, ETH_ALEN);
> +	}
> +
> +	if (iter_data->need_set_hw_addr)
> +		if (compare_ether_addr(iter_data->hw_macaddr, mac) == 0)
> +			iter_data->need_set_hw_addr = false;
> +
> +	if (!iter_data->any_assoc) {
> +		struct ath5k_vif *avf = (void *)vif->drv_priv;
> +		if (avf->assoc)
> +			iter_data->any_assoc = true;
> +	}
> +}
> +
> +void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct ieee80211_vif
> *vif) +{
> +	struct ath_common *common = ath5k_hw_common(sc->ah);
> +	struct ath_vif_iter_data iter_data;
> +
> +	/*
> +	 * Use the hardware MAC address as reference, the hardware uses it
> +	 * together with the BSSID mask when matching addresses.
> +	 */
> +	iter_data.hw_macaddr = common->macaddr;
> +	memset(&iter_data.mask, 0xff, ETH_ALEN);
> +	iter_data.found_active = false;
> +	iter_data.need_set_hw_addr = true;
> +
> +	if (vif)
> +		ath_vif_iter(&iter_data, vif->addr, vif);
> +
> +	/* Get list of all active MAC addresses */
> +	ieee80211_iterate_active_interfaces_atomic(sc->hw, ath_vif_iter,
> +						   &iter_data);

maybe i misunderstand something here, but why call ath_vif_iter()? doesn't 
ieee80211_iterate_active_interfaces_atomic() iterate over it anyways?

> +	memcpy(sc->bssidmask, iter_data.mask, ETH_ALEN);
> +
> +	if (iter_data.need_set_hw_addr && iter_data.found_active)
> +		ath5k_hw_set_lladdr(sc->ah, iter_data.active_mac);
> +
> +	ath5k_hw_set_bssid_mask(sc->ah, sc->bssidmask);
> +}
> +
>  static void
> -ath5k_mode_setup(struct ath5k_softc *sc)
> +ath5k_mode_setup(struct ath5k_softc *sc, struct ieee80211_vif *vif)
>  {
>  	struct ath5k_hw *ah = sc->ah;
>  	u32 rfilt;
> @@ -520,7 +582,7 @@ ath5k_mode_setup(struct ath5k_softc *sc)
>  	ath5k_hw_set_rx_filter(ah, rfilt);
> 
>  	if (ath5k_hw_hasbssidmask(ah))
> -		ath5k_hw_set_bssid_mask(ah, sc->bssidmask);
> +		ath5k_update_bssid_mask(sc, vif);
> 
>  	/* configure operational mode */
>  	ath5k_hw_set_opmode(ah, sc->opmode);
> @@ -698,13 +760,13 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct
> ath5k_buf *bf, flags |= AR5K_TXDESC_RTSENA;
>  		cts_rate = ieee80211_get_rts_cts_rate(sc->hw, info)->hw_value;
>  		duration = le16_to_cpu(ieee80211_rts_duration(sc->hw,
> -			sc->vif, pktlen, info));
> +			NULL, pktlen, info));

hmm, this NULL means we don't handle short preamble and erp correctly. i don't 
know if we did before, but it would be better to use the corresponding vif - i 
think it can be found in ieee80211_tx_info *info.

>  	}
>  	if (rc_flags & IEEE80211_TX_RC_USE_CTS_PROTECT) {
>  		flags |= AR5K_TXDESC_CTSENA;
>  		cts_rate = ieee80211_get_rts_cts_rate(sc->hw, info)->hw_value;
>  		duration = le16_to_cpu(ieee80211_ctstoself_duration(sc->hw,
> -			sc->vif, pktlen, info));
> +			NULL, pktlen, info));

same here

>  	}
>  	ret = ah->ah_setup_tx_desc(ah, ds, pktlen,
>  		ieee80211_get_hdrlen_from_skb(skb), padsize,
> @@ -806,10 +868,13 @@ ath5k_desc_alloc(struct ath5k_softc *sc, struct
> pci_dev *pdev) list_add_tail(&bf->list, &sc->txbuf);
>  	}
> 
> -	/* beacon buffer */
> -	bf->desc = ds;
> -	bf->daddr = da;
> -	sc->bbuf = bf;
> +	/* beacon buffers */
> +	INIT_LIST_HEAD(&sc->bcbuf);
> +	for (i = 0; i < ATH_BCBUF; i++, bf++, ds++, da += sizeof(*ds)) {
> +		bf->desc = ds;
> +		bf->daddr = da;
> +		list_add_tail(&bf->list, &sc->bcbuf);
> +	}
> 
>  	return 0;
>  err_free:
> @@ -824,11 +889,12 @@ ath5k_desc_free(struct ath5k_softc *sc, struct
> pci_dev *pdev) {
>  	struct ath5k_buf *bf;
> 
> -	ath5k_txbuf_free_skb(sc, sc->bbuf);
>  	list_for_each_entry(bf, &sc->txbuf, list)
>  		ath5k_txbuf_free_skb(sc, bf);
>  	list_for_each_entry(bf, &sc->rxbuf, list)
>  		ath5k_rxbuf_free_skb(sc, bf);
> +	list_for_each_entry(bf, &sc->bcbuf, list)
> +		ath5k_txbuf_free_skb(sc, bf);
> 
>  	/* Free memory associated with all descriptors */
>  	pci_free_consistent(pdev, sc->desc_len, sc->desc, sc->desc_daddr);
> @@ -837,7 +903,6 @@ ath5k_desc_free(struct ath5k_softc *sc, struct pci_dev
> *pdev)
> 
>  	kfree(sc->bufptr);
>  	sc->bufptr = NULL;
> -	sc->bbuf = NULL;
>  }
> 
> 
> @@ -1083,7 +1148,7 @@ ath5k_rx_start(struct ath5k_softc *sc)
>  	spin_unlock_bh(&sc->rxbuflock);
> 
>  	ath5k_hw_start_rx_dma(ah);	/* enable recv descriptors */
> -	ath5k_mode_setup(sc);		/* set filters, etc. */
> +	ath5k_mode_setup(sc, NULL);		/* set filters, etc. */
>  	ath5k_hw_start_rx_pcu(ah);	/* re-enable PCU/DMA engine */
> 
>  	return 0;
> @@ -1741,6 +1806,7 @@ ath5k_beacon_update(struct ieee80211_hw *hw, struct
> ieee80211_vif *vif) {
>  	int ret;
>  	struct ath5k_softc *sc = hw->priv;
> +	struct ath5k_vif *avf = (void *)vif->drv_priv;
>  	struct sk_buff *skb;
> 
>  	if (WARN_ON(!vif)) {
> @@ -1757,11 +1823,34 @@ ath5k_beacon_update(struct ieee80211_hw *hw, struct
> ieee80211_vif *vif)
> 
>  	ath5k_debug_dump_skb(sc, skb, "BC  ", 1);
> 
> -	ath5k_txbuf_free_skb(sc, sc->bbuf);
> -	sc->bbuf->skb = skb;
> -	ret = ath5k_beacon_setup(sc, sc->bbuf);
> +	if (!avf->bbuf) {
> +		WARN_ON(list_empty(&sc->bcbuf));
> +		avf->bbuf = list_first_entry(&sc->bcbuf, struct ath5k_buf,
> +					     list);
> +		list_del(&avf->bbuf->list);
> +
> +		/* Assign the vap to a beacon xmit slot. */
> +		if (avf->opmode == NL80211_IFTYPE_AP) {
> +			int slot;
> +
> +			avf->bslot = 0;
> +			for (slot = 0; slot < ATH_BCBUF; slot++) {
> +				if (!sc->bslot[slot]) {
> +					avf->bslot = slot;
> +					break;
> +				}
> +			}
> +			BUG_ON(sc->bslot[avf->bslot] != NULL);
> +			sc->bslot[avf->bslot] = vif;
> +			sc->nbcnvifs++;
> +		}
> +	}

hmm, do we really have to do that here? couldn't we assign the beacon slot 
when the interface is added?

> +	ath5k_txbuf_free_skb(sc, avf->bbuf);
> +	avf->bbuf->skb = skb;
> +	ret = ath5k_beacon_setup(sc, avf->bbuf);
>  	if (ret)
> -		sc->bbuf->skb = NULL;
> +		avf->bbuf->skb = NULL;
>  out:
>  	return ret;
>  }
> @@ -1777,16 +1866,18 @@ out:
>  static void
>  ath5k_beacon_send(struct ath5k_softc *sc)
>  {
> -	struct ath5k_buf *bf = sc->bbuf;
>  	struct ath5k_hw *ah = sc->ah;
> +	struct ieee80211_vif *vif;
> +	struct ath5k_vif *avf;
> +	struct ath5k_buf *bf;
>  	struct sk_buff *skb;
> +	u64 tsf;
> +	u32 tsftu;
> +	u16 intval;
> +	int slot;
> 
>  	ATH5K_DBG_UNLIMIT(sc, ATH5K_DEBUG_BEACON, "in beacon_send\n");
> 
> -	if (unlikely(bf->skb == NULL || sc->opmode == NL80211_IFTYPE_STATION)) {
> -		ATH5K_WARN(sc, "bf=%p bf_skb=%p\n", bf, bf ? bf->skb : NULL);
> -		return;
> -	}
>  	/*
>  	 * Check if the previous beacon has gone out.  If
>  	 * not, don't don't try to post another: skip this
> @@ -1815,6 +1906,28 @@ ath5k_beacon_send(struct ath5k_softc *sc)
>  		sc->bmisscount = 0;
>  	}
> 
> +	intval = sc->bintval ? sc->bintval : ATH5K_DEFAULT_BINTVAL;

bintval is already initialized in ath5k_pci_probe.

> +	tsf = ath5k_hw_get_tsf64(ah);
> +	tsftu = TSF_TO_TU(tsf);
> +	slot = ((tsftu % intval) * ATH_BCBUF) / intval;
> +	vif = sc->bslot[(slot + 1) % ATH_BCBUF];
> +
> +	pr_debug("tsf %llx tsftu %x intval %u slot %u vif %p\n",
> +		 (unsigned long long)tsf, tsftu, intval, slot, vif);

please use ATH5K_DBG.

> +	if (vif)
> +		avf = (void *)vif->drv_priv;
> +	else
> +		return;

i think this is part of what breaks ad-hoc beacons... we also need to assign a 
slot in adhoc mode...

> +	bf = avf->bbuf;
> +	if (unlikely(bf->skb == NULL || sc->opmode == NL80211_IFTYPE_STATION ||
> +			sc->opmode == NL80211_IFTYPE_MONITOR)) {
> +		ATH5K_WARN(sc, "bf=%p bf_skb=%p\n", bf, bf ? bf->skb : NULL);
> +		return;
> +	}
> +
>  	/*
>  	 * Stop any current dma and put the new frame on the queue.
>  	 * This should never fail since we check above that no frames
> @@ -1827,17 +1940,17 @@ ath5k_beacon_send(struct ath5k_softc *sc)
> 
>  	/* refresh the beacon for AP mode */
>  	if (sc->opmode == NL80211_IFTYPE_AP)
> -		ath5k_beacon_update(sc->hw, sc->vif);
> +		ath5k_beacon_update(sc->hw, vif);
> 
>  	ath5k_hw_set_txdp(ah, sc->bhalq, bf->daddr);
>  	ath5k_hw_start_tx_dma(ah, sc->bhalq);
>  	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON, "TXDP[%u] = %llx (%p)\n",
>  		sc->bhalq, (unsigned long long)bf->daddr, bf->desc);
> 
> -	skb = ieee80211_get_buffered_bc(sc->hw, sc->vif);
> +	skb = ieee80211_get_buffered_bc(sc->hw, vif);
>  	while (skb) {
>  		ath5k_tx_queue(sc->hw, skb, sc->cabq);
> -		skb = ieee80211_get_buffered_bc(sc->hw, sc->vif);
> +		skb = ieee80211_get_buffered_bc(sc->hw, vif);
>  	}
> 
>  	sc->bsent++;
> @@ -1867,6 +1980,12 @@ ath5k_beacon_update_timers(struct ath5k_softc *sc,
> u64 bc_tsf) u64 hw_tsf;
> 
>  	intval = sc->bintval & AR5K_BEACON_PERIOD;
> +	if (sc->opmode == NL80211_IFTYPE_AP) {
> +		intval /= ATH_BCBUF;	/* staggered multi-bss beacons */
> +		if (intval < 15)
> +			ATH5K_WARN(sc, "intval %u is too low, min 15\n",
> +				   intval);
> +	}
>  	if (WARN_ON(!intval))
>  		return;
> 
> @@ -2056,6 +2175,15 @@ ath5k_intr(int irq, void *dev_id)
> 
>  	do {
>  		ath5k_hw_get_isr(ah, &status);		/* NB: clears IRQ too */
> +		{
> +			static unsigned int irq_counter;
> +			if ((++irq_counter % 10000) == 9999) {
> +				ATH5K_WARN(sc, "status 0x%x/0x%x  dev_id: %p"
> +					   "  counter: %i  irq_counter: %i\n",
> +					   status, sc->imask, dev_id, counter,
> +					   irq_counter);
> +			}
> +		}

why did you add this code?

i see these warnings:
ath5k phy0: status 0x1/0x800814b5  dev_id: c7e30d20  counter: 1000  
irq_counter: 9999
ath5k phy0: status 0x1/0x800814b5  dev_id: c7e30d20  counter: 1000  
irq_counter: 19999
ath5k phy0: status 0x1/0x800814b5  dev_id: c7e30d20  counter: 1000  
irq_counter: 29999
ath5k phy0: status 0x1/0x800814b5  dev_id: c7e30d20  counter: 1000  
irq_counter: 39999

>  		ATH5K_DBG(sc, ATH5K_DEBUG_INTR, "status 0x%x/0x%x\n",
>  				status, sc->imask);
>  		if (unlikely(status & AR5K_INT_FATAL)) {
> @@ -2311,6 +2439,10 @@ ath5k_init(struct ath5k_softc *sc)
>  		ath_hw_keyreset(common, (u16) i);
> 
>  	ath5k_hw_set_ack_bitrate_high(ah, true);
> +
> +	for (i = 0; i < ARRAY_SIZE(sc->bslot); i++)
> +		sc->bslot[i] = NULL;
> +
>  	ret = 0;
>  done:
>  	mmiowb();
> @@ -2370,7 +2502,6 @@ ath5k_stop_hw(struct ath5k_softc *sc)
>  		ATH5K_DBG(sc, ATH5K_DEBUG_RESET,
>  				"putting device to sleep\n");
>  	}
> -	ath5k_txbuf_free_skb(sc, sc->bbuf);
> 
>  	mmiowb();
>  	mutex_unlock(&sc->lock);
> @@ -2575,9 +2706,9 @@ ath5k_attach(struct pci_dev *pdev, struct
> ieee80211_hw *hw) }
> 
>  	SET_IEEE80211_PERM_ADDR(hw, mac);
> +	memcpy(&sc->lladdr, mac, ETH_ALEN);
>  	/* All MAC address bits matter for ACKs */
> -	memcpy(sc->bssidmask, ath_bcast_mac, ETH_ALEN);
> -	ath5k_hw_set_bssid_mask(sc->ah, sc->bssidmask);
> +	ath5k_update_bssid_mask(sc, NULL);
> 
>  	regulatory->current_rd = ah->ah_capabilities.cap_eeprom.ee_regdomain;
>  	ret = ath_regd_init(regulatory, hw->wiphy, ath5k_reg_notifier);
> @@ -2675,31 +2806,55 @@ static int ath5k_add_interface(struct ieee80211_hw
> *hw, {
>  	struct ath5k_softc *sc = hw->priv;
>  	int ret;
> +	struct ath5k_hw *ah = sc->ah;
> +	struct ath5k_vif *avf = (void *)vif->drv_priv;
> 
>  	mutex_lock(&sc->lock);
> -	if (sc->vif) {
> -		ret = 0;
> +
> +	if (vif->type == NL80211_IFTYPE_AP && sc->nbcnvifs >= ATH_BCBUF) {
> +		ret = -ELNRNG;
>  		goto end;
>  	}
> -
> -	sc->vif = vif;
> +	sc->nvifs++;
> 
>  	switch (vif->type) {
>  	case NL80211_IFTYPE_AP:
>  	case NL80211_IFTYPE_STATION:
>  	case NL80211_IFTYPE_ADHOC:
>  	case NL80211_IFTYPE_MESH_POINT:
> -		sc->opmode = vif->type;
> +		avf->opmode = vif->type;
>  		break;
>  	default:
>  		ret = -EOPNOTSUPP;
>  		goto end;
>  	}
> 
> -	ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "add interface mode %d\n", sc->opmode);
> +	ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "add interface mode %d\n", avf->opmode);
> +
> +	/* Set combined mode - when APs are configured, operate in AP mode.
> +	 * Otherwise use the mode of the new interface. This can currently
> +	 * only deal with combinations of APs and STAs I think ...
> +	 */
> +	if (sc->nbcnvifs)
> +		sc->opmode = NL80211_IFTYPE_AP;
> +	else
> +		sc->opmode = vif->type;
> +
> +	ath5k_hw_set_opmode(ah, sc->opmode);
> +
> +	/* Set to a reasonable value. Note that this will
> +	 * be set to mac80211's value at ath5k_config(). */
> +	sc->bintval = ATH5K_DEFAULT_BINTVAL;

sc->bintval is already initialized in ath5k_pci_probe. we should do it either 
there or here, and anyhow it's going to be set by mac80211 later.

> +	/* Any MAC address is finee, all others are included through the

typo: "finee"

> +	 * filter.
> +	 */
> +	memcpy(&sc->lladdr, vif->addr, ETH_ALEN);
>  	ath5k_hw_set_lladdr(sc->ah, vif->addr);
> -	ath5k_mode_setup(sc);
> +
> +	memcpy(&avf->lladdr, vif->addr, ETH_ALEN);
> +
> +	ath5k_mode_setup(sc, vif);
> 
>  	ret = 0;
>  end:
> @@ -2712,15 +2867,26 @@ ath5k_remove_interface(struct ieee80211_hw *hw,
>  			struct ieee80211_vif *vif)
>  {
>  	struct ath5k_softc *sc = hw->priv;
> -	u8 mac[ETH_ALEN] = {};
> +	struct ath5k_vif *avf = (void *)vif->drv_priv;
> +	unsigned int i;
> 
>  	mutex_lock(&sc->lock);
> -	if (sc->vif != vif)
> -		goto end;
> +	sc->nvifs--;
> +
> +	if (avf->bbuf) {
> +		ath5k_txbuf_free_skb(sc, avf->bbuf);
> +		list_add_tail(&avf->bbuf->list, &sc->bcbuf);
> +		for (i = 0; i < ATH_BCBUF; i++) {
> +			if (sc->bslot[i] == vif) {
> +				sc->bslot[i] = NULL;
> +				break;
> +			}
> +		}
> +		sc->nbcnvifs--;
> +		avf->bbuf = NULL;
> +	}
>
> -	ath5k_hw_set_lladdr(sc->ah, mac);
> -	sc->vif = NULL;
> -end:
> +	ath5k_update_bssid_mask(sc, NULL);
>  	mutex_unlock(&sc->lock);
>  }
> 
> @@ -2803,6 +2969,18 @@ static u64 ath5k_prepare_multicast(struct
> ieee80211_hw *hw, return ((u64)(mfilt[1]) << 32) | mfilt[0];
>  }
> 
> +static bool ath_any_vif_assoc(struct ath5k_softc *sc)
> +{
> +	struct ath_vif_iter_data iter_data;
> +	iter_data.any_assoc = false;
> +	iter_data.need_set_hw_addr = false;
> +	iter_data.found_active = true;
> +
> +	ieee80211_iterate_active_interfaces_atomic(sc->hw, ath_vif_iter,
> +						   &iter_data);
> +	return iter_data.any_assoc;
> +}
> +
>  #define SUPPORTED_FIF_FLAGS \
>  	FIF_PROMISC_IN_BSS |  FIF_ALLMULTI | FIF_FCSFAIL | \
>  	FIF_PLCPFAIL | FIF_CONTROL | FIF_OTHER_BSS | \
> @@ -2873,7 +3051,7 @@ static void ath5k_configure_filter(struct
> ieee80211_hw *hw,
> 
>  	/* FIF_BCN_PRBRESP_PROMISC really means to enable beacons
>  	* and probes for any BSSID */
> -	if (*new_flags & FIF_BCN_PRBRESP_PROMISC)
> +	if ((*new_flags & FIF_BCN_PRBRESP_PROMISC) || (sc->nvifs > 1))
>  		rfilt |= AR5K_RX_FILTER_BEACON;
> 
>  	/* FIF_CONTROL doc says that if FIF_PROMISC_IN_BSS is not
> @@ -3058,14 +3236,13 @@ static void ath5k_bss_info_changed(struct
> ieee80211_hw *hw, struct ieee80211_bss_conf *bss_conf,
>  				    u32 changes)
>  {
> +	struct ath5k_vif *avf = (void *)vif->drv_priv;
>  	struct ath5k_softc *sc = hw->priv;
>  	struct ath5k_hw *ah = sc->ah;
>  	struct ath_common *common = ath5k_hw_common(ah);
>  	unsigned long flags;
> 
>  	mutex_lock(&sc->lock);
> -	if (WARN_ON(sc->vif != vif))
> -		goto unlock;
> 
>  	if (changes & BSS_CHANGED_BSSID) {
>  		/* Cache for later use during resets */
> @@ -3079,7 +3256,12 @@ static void ath5k_bss_info_changed(struct
> ieee80211_hw *hw, sc->bintval = bss_conf->beacon_int;
> 
>  	if (changes & BSS_CHANGED_ASSOC) {
> -		sc->assoc = bss_conf->assoc;
> +		avf->assoc = bss_conf->assoc;
> +		if (bss_conf->assoc)
> +			sc->assoc = bss_conf->assoc;
> +		else
> +			sc->assoc = ath_any_vif_assoc(sc);
> +
>  		if (sc->opmode == NL80211_IFTYPE_STATION)
>  			set_beacon_filter(hw, sc->assoc);
>  		ath5k_hw_set_ledstate(sc->ah, sc->assoc ?
> @@ -3107,7 +3289,6 @@ static void ath5k_bss_info_changed(struct
> ieee80211_hw *hw, BSS_CHANGED_BEACON_INT))
>  		ath5k_beacon_config(sc);
> 
> - unlock:
>  	mutex_unlock(&sc->lock);
>  }
> 
> @@ -3382,6 +3563,8 @@ ath5k_pci_probe(struct pci_dev *pdev,
>  		hw->max_rate_tries = 11;
>  	}
> 
> +	hw->vif_data_size = sizeof(struct ath5k_vif);
> +

this is not used and unnecessary.

>  	/* Finish private driver data initialization */
>  	ret = ath5k_attach(pdev, hw);
>  	if (ret)
> diff --git a/drivers/net/wireless/ath/ath5k/base.h
> b/drivers/net/wireless/ath/ath5k/base.h index 7f9d0d3..7f91032 100644
> --- a/drivers/net/wireless/ath/ath5k/base.h
> +++ b/drivers/net/wireless/ath/ath5k/base.h
> @@ -58,8 +58,8 @@
> 
>  #define	ATH_RXBUF	40		/* number of RX buffers */
>  #define	ATH_TXBUF	200		/* number of TX buffers */
> -#define ATH_BCBUF	1		/* number of beacon buffers */
> -
> +#define ATH_BCBUF	4		/* number of beacon buffers */
> +#define ATH5K_DEFAULT_BINTVAL	1000

bintval is initialized in ath5k_pci_probe, if you want to use this define, use 
it there. anyhow please put it in a different place, to keep the buffer 
related things together.

>  #define ATH5K_TXQ_LEN_MAX	(ATH_TXBUF / 4)		/* bufs per queue */
>  #define ATH5K_TXQ_LEN_LOW	(ATH5K_TXQ_LEN_MAX / 2)	/* low mark */
> 
> @@ -152,6 +152,15 @@ struct ath5k_statistics {
>  #define ATH_CHAN_MAX	(14+14+14+252+20)
>  #endif
> 
> +struct ath5k_vif {
> +	bool			assoc; /* are we associated or not */
> +	int			if_id;

if_id is unused.

> +	enum nl80211_iftype	opmode;
> +	int			bslot;
> +	struct ath5k_buf	*bbuf;		/* beacon buffer */
> +	u8			lladdr[ETH_ALEN];
> +};
> +
>  /* Software Carrier, keeps track of the driver state
>   * associated with an instance of a device */
>  struct ath5k_softc {
> @@ -188,10 +197,11 @@ struct ath5k_softc {
>  	unsigned int		curmode;	/* current phy mode */
>  	struct ieee80211_channel *curchan;	/* current h/w channel */
> 
> -	struct ieee80211_vif *vif;
> +	u16			nvifs;
> 
>  	enum ath5k_int		imask;		/* interrupt mask copy */
> 
> +	u8			lladdr[ETH_ALEN];
>  	u8			bssidmask[ETH_ALEN];
> 
>  	unsigned int		led_pin,	/* GPIO pin for driving LED */
> @@ -219,7 +229,9 @@ struct ath5k_softc {
> 
>  	spinlock_t		block;		/* protects beacon */
>  	struct tasklet_struct	beacontq;	/* beacon intr tasklet */
> -	struct ath5k_buf	*bbuf;		/* beacon buffer */
> +	struct list_head	bcbuf;		/* beacon buffer */
> +	struct ieee80211_vif	*bslot[ATH_BCBUF];

it seems a bit pointless to use a linked list to keep the beacon buffers, 
since they are more or less statically assigned to the slots / vifs anyways. 
couldn't we pre-assign one buffer per slot (skb would be NULL if unused, or we 
keep track of that some other way)?

> +	u16			nbcnvifs;

can we have a more descriptive name than "nbcnvifs"?

>  	unsigned int		bhalq,		/* SW q for outgoing beacons */
>  				bmisscount,	/* missed beacon transmits */
>  				bintval,	/* beacon interval in TU */
> @@ -228,7 +240,7 @@ struct ath5k_softc {
>  	struct ath5k_txq	*cabq;		/* content after beacon */
> 
>  	int 			power_level;	/* Requested tx power in dbm */
> -	bool			assoc;		/* associate state */
> +	bool                    assoc;          /* associate state */

whitespace messed up.

>  	bool			enable_beacon;	/* true if beacons are on */
> 
>  	struct ath5k_statistics	stats;
> diff --git a/drivers/net/wireless/ath/ath5k/reset.c
> b/drivers/net/wireless/ath/ath5k/reset.c index 58912cd..5b179d0 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -167,7 +167,7 @@ static inline void ath5k_hw_write_rate_duration(struct
> ath5k_hw *ah, * ieee80211_duration() for a brief description of
>  		 * what rate we should choose to TX ACKs. */
>  		tx_time = le16_to_cpu(ieee80211_generic_frame_duration(sc->hw,
> -							sc->vif, 10, rate));
> +							NULL, 10, rate));

same like above, we loose the erp settings, but here it does not make sense to 
use vif. a similar thing is commented in the code, so it's probably ok.

>  		ath5k_hw_reg_write(ah, tx_time, reg);
> 
> @@ -1060,7 +1060,7 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum
> nl80211_iftype op_mode, * XXX: rethink this after new mode changes to
>  		 * mac80211 are integrated */
>  		if (ah->ah_version == AR5K_AR5212 &&
> -			ah->ah_sc->vif != NULL)
> +			ah->ah_sc->nvifs)
>  			ath5k_hw_write_rate_duration(ah, mode);
> 
>  		/*

i will continue to test and i will look at the adhoc beacon problem tomorrow.

greetings,
bruno

CC: ath5-devel list too