On 09/28/2010 03:01 AM, Bruno Randolf wrote:
On Tue September 28 2010 06:06:28 greearb@xxxxxxxxxxxxxxx wrote:
From: Ben Greear<greearb@xxxxxxxxxxxxxxx>
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 <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
--
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
