ath9k: Fix up hardware mode and beacons with multiple vifs.

From: Ben Greear<[email protected]>

When using a mixture of AP and Station interfaces, the hardware mode was using the type of the last VIF registered. Instead, we should keep track of the number of different types of vifs and set the mode accordingly.

In addtion, use the vif type instead of hardware opmode when dealing with beacons.

Attempt to move some of the common setup code into smaller methods so we can re-use it when changing vif mode as well as adding/deleting vifs.

This needs review.

Signed-off-by Ben Greear<[email protected]>

---
:100644 100644 3108699... a2da259... M drivers/net/wireless/ath/ath9k/ath9k.h
:100644 100644 385ba03... 8de591e... M drivers/net/wireless/ath/ath9k/beacon.c
:100644 100644 0452580... 1a65e53... M drivers/net/wireless/ath/ath9k/main.c
:100644 100644 ea2f67c... 9a2b4a8... M drivers/net/wireless/ath/ath9k/recv.c

  drivers/net/wireless/ath/ath9k/ath9k.h  |   10 +-
  drivers/net/wireless/ath/ath9k/beacon.c |   14 +-
  drivers/net/wireless/ath/ath9k/main.c   |  263 ++++++++++++++++++++++---------
  drivers/net/wireless/ath/ath9k/recv.c   |   17 ++-
  4 files changed, 214 insertions(+), 90 deletions(-)

drivers/net/wireless/ath/ath9k/ath9k.h +0 -2

index 3108699..a2da259 100644
@@ -607,10 +607,15 @@ struct ath_softc {
  	u32 sc_flags; /* SC_OP_* */
  	u16 ps_flags; /* PS_* */
  	u16 curtxpow;
-	u8 nbcnvifs;
-	u16 nvifs;
  	bool ps_enabled;
  	bool ps_idle;

drivers/net/wireless/ath/ath9k/recv.c +9 -2

index ea2f67c..9a2b4a8 100644
@@ -589,8 +589,14 @@ static void ath_rx_ps_beacon(struct ath_softc *sc, struct sk_buff *skb)
  		return;
 
  	mgmt = (struct ieee80211_mgmt *)skb->data;
-	if (memcmp(common->curbssid, mgmt->bssid, ETH_ALEN) != 0)
+	if (memcmp(common->curbssid, mgmt->bssid, ETH_ALEN) != 0) {
+		/* TODO:  This doesn't work well if you have stations
+		 * associated to two different APs because curbssid
+		 * is just the last AP that any of the stations associated
+		 * with.
+		 */
  		return; /* not from our current AP */
+	}
 
  	sc->ps_flags&= ~PS_WAIT_FOR_BEACON;
 

@@ -980,13 +986,20 @@ static void ath9k_process_rssi(struct ath_common *common,
  	int last_rssi;
  	__le16 fc;
 
+	/* TODO:  Maybe need to accept this if we have STA vifs active?? */
  	if (ah->opmode != NL80211_IFTYPE_STATION)
  		return;
 
  	fc = hdr->frame_control;
  	if (!ieee80211_is_beacon(fc) ||
-	    compare_ether_addr(hdr->addr3, common->curbssid))
+	    compare_ether_addr(hdr->addr3, common->curbssid)) {

Quoted 22 lines · greearb at candelatech.com

drivers/net/wireless/ath/ath9k/ath9k.h +0 -0

index 3108699..a2da259 100644
@@ -607,10 +607,15 @@ struct ath_softc {

u32 sc_flags; /* SC_OP_* */
u16 ps_flags; /* PS_* */
u16 curtxpow;
- u8 nbcnvifs;
- u16 nvifs;
bool ps_enabled;
bool ps_idle;
+ short nbcnvifs;
+ short nvifs;
+ short naps; /* number of APs */
+ short nmeshes;
+ short nstations;
+ short nwds;
+ short nadhocs;
unsigned long ps_usecount;

Quoted 1 line

struct ath_config config;

Do we really need all those counters? Wouldn't it be better to iterate over active interfaces instead?

Quoted 11 lines

if (!ieee80211_is_beacon(fc) ||
- compare_ether_addr(hdr->addr3, common->curbssid))
+ compare_ether_addr(hdr->addr3, common->curbssid)) {
+ /* TODO: This doesn't work well if you have stations
+ * associated to two different APs because curbssid
+ * is just the last AP that any of the stations associated
+ * with.
+ */
return;
+ }

Quoted 2 lines

if (rx_stats->rs_rssi != ATH9K_RSSI_BAD&& !rx_stats->rs_moreaggr) ATH_RSSI_LPF(aphy->last_rssi, rx_stats->rs_rssi);

Checking for the STA opmode is correct, this code should not be used for STA+AP. For the multi-STA case, it may make sense to process the RSSI average per vif and using the minimum for ANI.

Do we really need all those counters? Wouldn't it be better to iterate over active interfaces instead?

Quoted 2 lines · Felix Fietkau

Do we really need all those counters? Wouldn't it be better to iterate over active interfaces instead?

I think iterating over active interfaces is better, especially if we set up more fine-graned ah-> variables that make sense in themselves for stuff like power save and tsf sync. But that's just my two cents.

As an example of something that may be more or less impossible to do without iteration consider multiple vifs with different beacon intervals. This could be handled by computing a greatest common devisor of all beacon intervals and using this for SWBA.

From: Ben Greear <[email protected]>

When using a mixture of AP and Station interfaces, the hardware mode was using the type of the last VIF registered. Instead, we should keep track of the number of different types of vifs and set the mode accordingly.

In addtion, use the vif type instead of hardware opmode when dealing with beacons.

Attempt to move some of the common setup code into smaller methods so we can re-use it when changing vif mode as well as adding/deleting vifs.

This needs review.

Signed-off-by Ben Greear <[email protected]>

---
:100644 100644 3108699... a2da259... M drivers/net/wireless/ath/ath9k/ath9k.h
:100644 100644 385ba03... 8de591e... M drivers/net/wireless/ath/ath9k/beacon.c
:100644 100644 0452580... 1a65e53... M drivers/net/wireless/ath/ath9k/main.c
:100644 100644 ea2f67c... 9a2b4a8... M drivers/net/wireless/ath/ath9k/recv.c

 drivers/net/wireless/ath/ath9k/ath9k.h  |   10 +-
 drivers/net/wireless/ath/ath9k/beacon.c |   14 +-
 drivers/net/wireless/ath/ath9k/main.c   |  263 ++++++++++++++++++++++---------
 drivers/net/wireless/ath/ath9k/recv.c   |   17 ++-
 4 files changed, 214 insertions(+), 90 deletions(-)

Quoted 23 lines · greearb at candelatech.com

From: Ben Greear<[email protected]>

When using a mixture of AP and Station interfaces, the hardware mode was using the type of the last VIF registered. Instead, we should keep track of the number of different types of vifs and set the mode accordingly.

In addtion, use the vif type instead of hardware opmode when dealing with beacons.

Attempt to move some of the common setup code into smaller methods so we can re-use it when changing vif mode as well as adding/deleting vifs.

This needs review.

Signed-off-by Ben Greear<[email protected]>

---
:100644 100644 3108699... a2da259... M drivers/net/wireless/ath/ath9k/ath9k.h
:100644 100644 385ba03... 8de591e... M drivers/net/wireless/ath/ath9k/beacon.c
:100644 100644 0452580... 1a65e53... M drivers/net/wireless/ath/ath9k/main.c
:100644 100644 ea2f67c... 9a2b4a8... M drivers/net/wireless/ath/ath9k/recv.c

Quoted 6 lines

  drivers/net/wireless/ath/ath9k/ath9k.h  |   10 +-
  drivers/net/wireless/ath/ath9k/beacon.c |   14 +-
  drivers/net/wireless/ath/ath9k/main.c   |  263 ++++++++++++++++++++++---------
  drivers/net/wireless/ath/ath9k/recv.c   |   17 ++-
  4 files changed, 214 insertions(+), 90 deletions(-)

It would be really useful to have both an AP and MESH vif. With the mesh and ap vif's bridged, a station connected to the ap could ping outside the mesh. Currently, if you add the mesh vif and then the ap vif, it sort of works. The other way around, the mesh interval value causes beacons to be sent only for the vif associated with slot 0.

Is it possible to consider this case as you overhaul ath9k beaconing?

From: Ben Greear <[email protected]>

When using a mixture of AP and Station interfaces, the hardware mode was using the type of the last VIF registered. ?Instead, we should keep track of the number of different types of vifs and set the mode accordingly.

Quoted 7 lines · On Fri, Jan 14, 2011 at 6:27 PM,<[email protected]> wrote:

From: Ben Greear<[email protected]>

When using a mixture of AP and Station interfaces, the hardware mode was using the type of the last VIF registered. Instead, we should keep track of the number of different types of vifs and set the mode accordingly.

Thank you for your efforts to get multi-vifs working better.

This is another comment that is a little out of place but when I see the identifier beacon_interval in this context I cannot help bringing up per-vif settings as a related problem: If I read the code (or rather the lack of code) correctly the beacon interval of the last (or first?) beaconing vif added will be used to set up beacon tx, whereas the actual beacon contents will reflect the beacon intervals configured for individual vifs which may all be different... The same goes for e.g. tx queue configuration, no?

It feels like we need a general multi-vif cleanup here but it's too big for me unfortunately.

Thank you for your efforts to get multi-vifs working better.

This is another comment that is a little out of place but when I see the identifier beacon_interval in this context I cannot help bringing up per-vif settings as a related problem: If I read the code (or rather the lack of code) correctly the beacon interval of the last (or first?) beaconing vif added will be used to set up beacon tx, whereas the actual beacon contents will reflect the beacon intervals configured for individual vifs which may all be different... The same goes for e.g. tx queue configuration, no?

It feels like we need a general multi-vif cleanup here but it's too big for me unfortunately.

/Bj?rn
_______________________________________________

Quoted 8 lines

This is another comment that is a little out of place but when I see the identifier beacon_interval in this context I cannot help bringing up per-vif settings as a related problem: If I read the code (or rather the lack of code) correctly the beacon interval of the last (or first?) beaconing vif added will be used to set up beacon tx, whereas the actual beacon contents will reflect the beacon intervals configured for individual vifs which may all be different... The same goes for e.g. tx queue configuration, no?

For instance, an ap vif has a ~100ms interval and a mesh vif has a ~1000ms interval. I don't see how to avoid a per vif interval.

2011/1/15 Steve Brown <[email protected]>:

Quoted 2 lines

For instance, an ap vif has a ~100ms interval and a mesh vif has a ~1000ms interval. I don't see how to avoid a per vif interval.

I agree. We should fix ath9k so it works correctly even if beacon interval is different for different vifs (within some reasonable limits).