2010-09-27 21:07:08

by Ben Greear

[permalink] [raw]
Subject: [PATCH v4] ath5k: Allow ath5k to support virtual STA and AP interfaces.

From: Ben Greear <[email protected]>

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.

Signed-off-by: Ben Greear <[email protected]>
---
: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;
+};
+
+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);
+ 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));
}
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));
}
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++;
+ }
+ }
+
+ 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;
+
+ 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);
+
+ if (vif)
+ avf = (void *)vif->drv_priv;
+ else
+ return;
+
+ 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);
+ }
+ }
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;

+ /* Any MAC address is finee, all others are included through the
+ * 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);
+
/* 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
#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;
+ 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];
+ u16 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 */
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));

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);

/*
--
1.7.2.2



2010-09-28 10:01:10

by Bruno Randolf

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

On Tue September 28 2010 06:06:28 [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> 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 <[email protected]>
> ---
>
> :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

2010-09-28 15:42:04

by Ben Greear

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

On 09/28/2010 03:01 AM, Bruno Randolf wrote:
> On Tue September 28 2010 06:06:28 [email protected] wrote:
>> From: Ben Greear<[email protected]>
>>
>> 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

Thanks for the in-depth review!

We've just started serious testing on this, so likely problems remain.

>> +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?

When adding an interface, it isn't running yet, so the iterate would skip
it. I basically copied this code directly out of ath9k virtual.c.

>> /* 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.

I just copied this from my .31 code, and it may well have been broken there.
I'll see if I can make this better.

>> 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?

Well, it might could be done with the interface is configured UP,
at least. I'm not too sure why this code was written as it is.

>> + 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...

Ok..we never tested ad-hoc. Patches welcome if you get it working
before I get a chance to try it :)

>> @@ -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

In .31 testing, we saw cases where the system basically live-locked
trying to endlessly handle irqs. This code was to help detect and debug
this..but it can be removed from the final patch.

>> 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)?

Sounds reasonable to me...not sure why this was written as it is.

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

I'll work on addressing your comments and will post a new version when I
make some progress.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2010-09-28 16:46:28

by Ben Greear

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

On 09/28/2010 03:01 AM, Bruno Randolf wrote:
> On Tue September 28 2010 06:06:28 [email protected] wrote:

>> /* 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.

This is nasty though...you can't trust that the vif still exists.
It could have been deleted right after pushing this packet, I think.

I'd rather not have to ask mac80211 to do a lookup here.

Perhaps if I forced a flush of all tx pkts when an interface
was deleted?

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com