RFC: v3: Support multiple STA on same AP with ath9k

ath9k-devel Ben Greear 3 participants 10 messages Sep 10, 2010 – Sep 14, 2010 mac80211

Share & tools

Ben Greear

author

2010-09-10 20:07 UTC #0 lore raw

The attached patch lets mac80111 create multiple STA on the same AP, and the changes to ath9k let it function properly, at least for un-encrypted traffic. WPA does not work..no idea why yet.

It also consolidates some of the util.c iterator logic in mac80211.

Please let me know if this is moving in the right direction.

Thanks,
Ben

Signature · 8 lines

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


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: wt-36-vsta-ok-c.patch
Url: http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20100910/2bb9cea1/attachment-0001.txt

Johannes Berg

maintainer

2010-09-10 21:12 UTC #1 lore raw

Quoted 2 lines · Ben Greear

It also consolidates some of the util.c iterator logic in mac80211.

That really ought to be a different patch.

And I don't think I like the counting in mac80211 -- it's just overhead (EXPORT_SYMBOL etc) and a very simple implementation in the driver itself.

johannes

Ben Greear

author

2010-09-10 21:23 UTC #2 lore raw

Quoted 10 lines · Johannes Berg

Quoted 2 lines · Ben Greear
It also consolidates some of the util.c iterator logic in mac80211.
That really ought to be a different patch.
And I don't think I like the counting in mac80211 -- it's just overhead (EXPORT_SYMBOL etc) and a very simple implementation in the driver itself.

I can just use the helper method by itself if you don't want another new method exposed to explicitly count interfaces.

As for counting, as far as I can tell, ath9k doesn't keep a list of interfaces, but rather calls those iterator methods when it wants to know them, so one way or another I need to call back into mac80211.

I'll break the patches apart for the next posting.

Did you get a chance to think more about this part of the patch?

net/mac80211/sta_info.c +1 -1

index 687077e..db751f1 100644
@@ -440,7 +440,7 @@ int sta_info_insert_rcu(struct sta_info *sta) __acquires(RCU)
 
         spin_lock_irqsave(&local->sta_lock, flags);
         /* check if STA exists already */
-       if (sta_info_get_bss(sdata, sta->sta.addr)) {
+       if (sta_info_get(sdata, sta->sta.addr)) {
                 spin_unlock_irqrestore(&local->sta_lock, flags);
                 mutex_unlock(&local->sta_mtx);
                 rcu_read_lock();

Thanks,
Ben

Signature · 2 lines

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

Johannes Berg

maintainer

2010-09-10 21:27 UTC #3 lore raw

Quoted 3 lines · Ben Greear

As for counting, as far as I can tell, ath9k doesn't keep a list of interfaces, but rather calls those iterator methods when it wants to know them, so one way or another I need to call back into mac80211.

But it can just call the atomic iterator with a callback that keeps the count.

Quoted 3 lines

I'll break the patches apart for the next posting.
Did you get a chance to think more about this part of the patch?

No, I didn't, and since I'm at the wireless summit now it seems unlikely that I'll get to it before I get/go back home on Monday (US time morning).

johannes

Vasanthakumar Thiagarajan

2010-09-13 12:14 UTC #4 lore raw

In reply to Ben Greear

Quoted 16 lines

(quoted diff) +0 -0

 	/* configure operational mode */

drivers/net/wireless/ath/ath9k/recv.c +0 -1

index b32c8f0..4ce4029 100644
@@ -110,7 +110,6 @@ static void ath_setdefantenna(struct ath_softc *sc, u32 antenna)
 static void ath_opmode_init(struct ath_softc *sc)
 {
 	struct ath_hw *ah = sc->sc_ah;
-	struct ath_common *common = ath9k_hw_common(ah);
 
 	u32 rfilt, mfilt[2];

@@ -118,9 +117,15 @@ static void ath_opmode_init(struct ath_softc *sc)
 	rfilt = ath_calcrxfilter(sc);
 	ath9k_hw_setrxfilter(ah, rfilt);

drivers/net/wireless/ath/ath9k/recv.c static void ath_opmode_init(struct ath_softc *sc) · line 125

Quoted 10 lines

drivers/net/wireless/ath/ath9k/recv.c +6 -3

 
-	/* configure bssid mask */
-	if (ah->caps.hw_caps & ATH9K_HW_CAP_BSSIDMASK)
-		ath_hw_setbssidmask(common);
+	/* configure bssid mask, if ah->hw is configured.
+	 * it is NOT configured when mac80211 is calling
+	 * ieee80211_do_open, but probably just as well since
+	 * this STA isn't in the list yet.
+	 */
+	if (ah->hw) {

This seems bogus. why a check for hw at this point??. IIRS hw of ah is not initialized at all, in that case this would fail always. Did you test with these code?

drivers/net/wireless/ath/ath9k/recv.c static void ath_opmode_init(struct ath_softc *sc) · line 126

Quoted 1 line

drivers/net/wireless/ath/ath9k/recv.c +1 -0
+	int avifs = ieee80211_count_sta_atomic(sc->hw);

If your aim here is to get the number of virtual interfaces associated to a hw, sc->nvifs should work.

Ben Greear

author

2010-09-13 14:36 UTC #5 lore raw

Quoted 17 lines · Vasanthakumar Thiagarajan

Quoted 17 lines

/* configure operational mode */

drivers/net/wireless/ath/ath9k/recv.c +0 -1

index b32c8f0..4ce4029 100644
@@ -110,7 +110,6 @@ static void ath_setdefantenna(struct ath_softc *sc, u32 antenna)
  static void ath_opmode_init(struct ath_softc *sc)
  {
  	struct ath_hw *ah = sc->sc_ah;
-	struct ath_common *common = ath9k_hw_common(ah);
 
  	u32 rfilt, mfilt[2];

@@ -118,9 +117,15 @@ static void ath_opmode_init(struct ath_softc *sc)
  	rfilt = ath_calcrxfilter(sc);
  	ath9k_hw_setrxfilter(ah, rfilt);

Quoted 13 lines

drivers/net/wireless/ath/ath9k/recv.c static void ath_opmode_init(struct ath_softc *sc) · line 125
Quoted 9 lines
drivers/net/wireless/ath/ath9k/recv.c +6 -3
-	/* configure bssid mask */
-	if (ah->caps.hw_caps&  ATH9K_HW_CAP_BSSIDMASK)
-		ath_hw_setbssidmask(common);
+	/* configure bssid mask, if ah->hw is configured.
+	 * it is NOT configured when mac80211 is calling
+	 * ieee80211_do_open, but probably just as well since
+	 * this STA isn't in the list yet.
+	 */
+	if (ah->hw) {
This seems bogus. why a check for hw at this point??. IIRS hw of ah is not initialized at all, in that case this would fail always. Did you test with these code?

I did test it, and it worked for me. Maybe we just shouldn't configure the bssid mask in that method? When using multiple VIFs, we need to do the mask properly, and from what I can tell, when this method is called, we do not have enough information to do that properly.

Quoted 5 lines

drivers/net/wireless/ath/ath9k/recv.c static void ath_opmode_init(struct ath_softc *sc) · line 126
Quoted 2 lines
drivers/net/wireless/ath/ath9k/recv.c +1 -0
+	int avifs = ieee80211_count_sta_atomic(sc->hw);
If your aim here is to get the number of virtual interfaces associated to a hw, sc->nvifs should work.

I'll check to see if this will work.

Thanks,
Ben

Quoted 4 lines

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

Signature · 2 lines

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

Ben Greear

author

2010-09-13 19:23 UTC #6 lore raw

In reply to Vasanthakumar Thiagarajan

Quoted 14 lines · Vasanthakumar Thiagarajan

Review
Quoted 9 lines
(quoted diff) +6 -3
-	/* configure bssid mask */
-	if (ah->caps.hw_caps&  ATH9K_HW_CAP_BSSIDMASK)
-		ath_hw_setbssidmask(common);
+	/* configure bssid mask, if ah->hw is configured.
+	 * it is NOT configured when mac80211 is calling
+	 * ieee80211_do_open, but probably just as well since
+	 * this STA isn't in the list yet.
+	 */
+	if (ah->hw) {
This seems bogus. why a check for hw at this point??. IIRS hw of ah is not initialized at all, in that case this would fail always. Did you test with these code?

Quoted 4 lines

Review
Quoted 1 line
(quoted diff) +1 -0
+	int avifs = ieee80211_count_sta_atomic(sc->hw);
If your aim here is to get the number of virtual interfaces associated to a hw, sc->nvifs should work.

Using nvifs seems to work fine. I can respin that patch using nvifs.

Any suggestions for fixing the bssid mask setting in ath_opmode_init? My change, appears to work because it's set in ath9k_add_interface, even if not in the ath_opmode_init method.

Thanks,
Ben

Signature · 2 lines

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

Vasanthakumar Thiagarajan

2010-09-14 13:46 UTC #7 lore raw

Quoted 3 lines · Ben Greear

Any suggestions for fixing the bssid mask setting in ath_opmode_init? My change, appears to work because it's set in ath9k_add_interface, even if not in the ath_opmode_init method.

checking for ah->hw in ath_opmode_init() is buggy. bssid mask configuration from ath9k_add_interface() may not work well when some of the other vifs are in down state, may be this needs to be in ath_opmode_init() without that incorrect check.

Vasanth

Ben Greear

author

2010-09-14 14:39 UTC #8 lore raw

Quoted 9 lines · Vasanthakumar Thiagarajan

Quoted 3 lines · Ben Greear
Any suggestions for fixing the bssid mask setting in ath_opmode_init? My change, appears to work because it's set in ath9k_add_interface, even if not in the ath_opmode_init method.
checking for ah->hw in ath_opmode_init() is buggy. bssid mask configuration from ath9k_add_interface() may not work well when some of the other vifs are in down state, may be this needs to be in ath_opmode_init() without that incorrect check.

It seems we need the ah->hw object to get the list of interfaces, but ah->hw is NULL in ath_opmode_init. Is there another way to get the list of interfaces so we can iterate over them to build the ssid mask?

Thanks,
Ben

Signature · 2 lines

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

Vasanthakumar Thiagarajan

2010-09-14 14:44 UTC #9 lore raw

Quoted 6 lines · Ben Greear

Quoted 5 lines · Vasanthakumar Thiagarajan
Quoted 3 lines · Ben Greear
Any suggestions for fixing the bssid mask setting in ath_opmode_init? My change, appears to work because it's set in ath9k_add_interface, even if not in the ath_opmode_init method.

Quoted 8 lines

Quoted 4 lines
checking for ah->hw in ath_opmode_init() is buggy. bssid mask configuration from ath9k_add_interface() may not work well when some of the other vifs are in down state, may be this needs to be in ath_opmode_init() without that incorrect check.
It seems we need the ah->hw object to get the list of interfaces, but ah->hw is NULL in ath_opmode_init. Is there another way to get the list of interfaces so we can iterate over them to build the ssid mask?

common->hw.