[RFC] ath9k: Handle interface changes properly

[RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

The commit ""ath9k: Add change_interface callback" was failed
to update of hw opmode, ani and interrupt mask. This leads
to break p2p functionality on ath9k. And the existing add and
remove interface functions are not handling hw opmode and
ANI properly.

This patch combines the common code in interface callbacks
and also takes care of multi-vif cases.

Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |    7 +
 drivers/net/wireless/ath/ath9k/main.c  |  195 +++++++++++++------------------
 drivers/net/wireless/ath/ath9k/recv.c  |    2 +-
 3 files changed, 90 insertions(+), 114 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 3681caf5..ef00f93 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -309,6 +309,7 @@ int ath_startrecv(struct ath_softc *sc);
 bool ath_stoprecv(struct ath_softc *sc);
 void ath_flushrecv(struct ath_softc *sc);
 u32 ath_calcrxfilter(struct ath_softc *sc);
+void ath_opmode_init(struct ath_softc *sc);
 int ath_rx_init(struct ath_softc *sc, int nbufs);
 void ath_rx_cleanup(struct ath_softc *sc);
 int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp);
@@ -337,6 +338,12 @@ void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid
 /* VIFs */
 /********/
 
+enum ath_iface_optype {
+	ATH9K_ADD_IFACE,
+	ATH9K_MOD_IFACE,
+	ATH9K_DEL_IFACE,
+};
+
 struct ath_vif {
 	int av_bslot;
 	__le64 tsf_adjust; /* TSF adjustment for staggered beacons */
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index f90a6ca..b6dd7d0 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1341,64 +1341,114 @@ static void ath9k_stop(struct ieee80211_hw *hw)
 	ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
 }
 
-static int ath9k_add_interface(struct ieee80211_hw *hw,
-			       struct ieee80211_vif *vif)
+static void ath9k_reclaim_beacon(struct ath_softc *sc,
+				 struct ieee80211_vif *vif)
+{
+	struct ath_vif *avp = (void *)vif->drv_priv;
+
+	/* Disable SWBA interrupt */
+	sc->sc_ah->imask &= ~ATH9K_INT_SWBA;
+	ath9k_ps_wakeup(sc);
+	ath9k_hw_set_interrupts(sc->sc_ah, sc->sc_ah->imask);
+	ath9k_hw_stoptxdma(sc->sc_ah, sc->beacon.beaconq);
+	tasklet_kill(&sc->bcon_tasklet);
+	ath9k_ps_restore(sc);
+
+	ath_beacon_return(sc, avp);
+	sc->sc_flags &= ~SC_OP_BEACONS;
+
+	if (sc->nbcnvifs > 0) {
+		/* Re-enable beaconing */
+		sc->sc_ah->imask |= ATH9K_INT_SWBA;
+		ath9k_ps_wakeup(sc);
+		ath9k_hw_set_interrupts(sc->sc_ah, sc->sc_ah->imask);
+		ath9k_ps_restore(sc);
+	}
+}
+
+static int ath9k_iface_work(struct ieee80211_hw *hw,
+			    struct ieee80211_vif *vif,
+			    enum nl80211_iftype new_type,
+			    bool p2p,
+			    u8 optype)
 {
 	struct ath_wiphy *aphy = hw->priv;
 	struct ath_softc *sc = aphy->sc;
 	struct ath_hw *ah = sc->sc_ah;
-	struct ath_common *common = ath9k_hw_common(ah);
 	struct ath_vif *avp = (void *)vif->drv_priv;
-	enum nl80211_iftype ic_opmode = NL80211_IFTYPE_UNSPECIFIED;
+	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+	enum nl80211_iftype iftype;
 	int ret = 0;
 
 	mutex_lock(&sc->mutex);
 
-	switch (vif->type) {
+	/* Stop ANI timer */
+	del_timer_sync(&common->ani.timer);
+
+	iftype = (optype == ATH9K_MOD_IFACE) ? new_type : vif->type;
+
+	/* Remove interface */
+	if (optype == ATH9K_DEL_IFACE) {
+		if ((iftype == NL80211_IFTYPE_AP) ||
+		    (iftype == NL80211_IFTYPE_ADHOC)) {
+			ath9k_reclaim_beacon(sc, vif);
+			if (!sc->nbcnvifs)
+				sc->sc_flags &= ~SC_OP_ANI_RUN;
+		}
+		ath_dbg(common, ATH_DBG_CONFIG, "Detach Interface\n");
+		sc->nvifs--;
+		goto out;
+	}
+
+	switch (iftype) {
 	case NL80211_IFTYPE_STATION:
-		ic_opmode = NL80211_IFTYPE_STATION;
+		if ((optype == ATH9K_MOD_IFACE) &&
+		    ((vif->type == NL80211_IFTYPE_AP) ||
+		     (vif->type == NL80211_IFTYPE_ADHOC)))
+			ath9k_reclaim_beacon(sc, vif);
+		if (!sc->nbcnvifs) {
+			sc->sc_flags &= ~SC_OP_ANI_RUN;
+			ah->opmode = iftype;
+		}
 		break;
 	case NL80211_IFTYPE_WDS:
-		ic_opmode = NL80211_IFTYPE_WDS;
+		ah->opmode = iftype;
 		break;
 	case NL80211_IFTYPE_ADHOC:
-	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_MESH_POINT:
+	case NL80211_IFTYPE_AP:
 		if (sc->nbcnvifs >= ATH_BCBUF) {
 			ret = -ENOBUFS;
 			goto out;
 		}
-		ic_opmode = vif->type;
+		sc->sc_flags |= SC_OP_ANI_RUN;
+		ah->opmode = iftype;
 		break;
 	default:
 		ath_err(common, "Interface type %d not yet supported\n",
-			vif->type);
+				iftype);
 		ret = -EOPNOTSUPP;
 		goto out;
 	}
-
-	ath_dbg(common, ATH_DBG_CONFIG,
-		"Attach a VIF of type: %d\n", ic_opmode);
-
-	/* Set the VIF opmode */
-	avp->av_opmode = ic_opmode;
+	avp->av_opmode = iftype;
 	avp->av_bslot = -1;
+	vif->type = iftype;
+	vif->p2p = p2p;
 
-	sc->nvifs++;
-
-	ath9k_set_bssid_mask(hw, vif);
-
-	if (sc->nvifs > 1)
-		goto out; /* skip global settings for secondary vif */
+	if (optype == ATH9K_ADD_IFACE) {
+		ath_dbg(common, ATH_DBG_CONFIG,
+			"Attach Interface of type %d\n", vif->type);
+		sc->nvifs++;
+	} else
+		ath_dbg(common, ATH_DBG_CONFIG,
+			"Change Interface to type %d\n", vif->type);
 
-	if (ic_opmode == NL80211_IFTYPE_AP) {
+	ath_opmode_init(sc);
+	if (vif->type == NL80211_IFTYPE_AP) {
 		ath9k_hw_set_tsfadjust(ah, 1);
 		sc->sc_flags |= SC_OP_TSF_RESET;
 	}
 
-	/* Set the device opmode */
-	ah->opmode = ic_opmode;
-
 	/*
 	 * Enable MIB interrupts when there are hardware phy counters.
 	 * Note we only do this (at the moment) for station mode.
@@ -1410,43 +1460,18 @@ static int ath9k_add_interface(struct ieee80211_hw *hw,
 			ah->imask |= ATH9K_INT_MIB;
 		ah->imask |= ATH9K_INT_TSFOOR;
 	}
-
 	ath9k_hw_set_interrupts(ah, ah->imask);
 
-	if (vif->type == NL80211_IFTYPE_AP    ||
-	    vif->type == NL80211_IFTYPE_ADHOC) {
-		sc->sc_flags |= SC_OP_ANI_RUN;
-		ath_start_ani(common);
-	}
-
 out:
+	ath_start_ani(common);
 	mutex_unlock(&sc->mutex);
 	return ret;
 }
 
-static void ath9k_reclaim_beacon(struct ath_softc *sc,
-				 struct ieee80211_vif *vif)
+static int ath9k_add_interface(struct ieee80211_hw *hw,
+			       struct ieee80211_vif *vif)
 {
-	struct ath_vif *avp = (void *)vif->drv_priv;
-
-	/* Disable SWBA interrupt */
-	sc->sc_ah->imask &= ~ATH9K_INT_SWBA;
-	ath9k_ps_wakeup(sc);
-	ath9k_hw_set_interrupts(sc->sc_ah, sc->sc_ah->imask);
-	ath9k_hw_stoptxdma(sc->sc_ah, sc->beacon.beaconq);
-	tasklet_kill(&sc->bcon_tasklet);
-	ath9k_ps_restore(sc);
-
-	ath_beacon_return(sc, avp);
-	sc->sc_flags &= ~SC_OP_BEACONS;
-
-	if (sc->nbcnvifs > 0) {
-		/* Re-enable beaconing */
-		sc->sc_ah->imask |= ATH9K_INT_SWBA;
-		ath9k_ps_wakeup(sc);
-		ath9k_hw_set_interrupts(sc->sc_ah, sc->sc_ah->imask);
-		ath9k_ps_restore(sc);
-	}
+	return ath9k_iface_work(hw, vif, vif->type, vif->p2p, ATH9K_ADD_IFACE);
 }
 
 static int ath9k_change_interface(struct ieee80211_hw *hw,
@@ -1454,69 +1479,13 @@ static int ath9k_change_interface(struct ieee80211_hw *hw,
 				  enum nl80211_iftype new_type,
 				  bool p2p)
 {
-	struct ath_wiphy *aphy = hw->priv;
-	struct ath_softc *sc = aphy->sc;
-	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
-	int ret = 0;
-
-	ath_dbg(common, ATH_DBG_CONFIG, "Change Interface\n");
-	mutex_lock(&sc->mutex);
-
-	switch (new_type) {
-	case NL80211_IFTYPE_AP:
-	case NL80211_IFTYPE_ADHOC:
-		if (sc->nbcnvifs >= ATH_BCBUF) {
-			ath_err(common, "No beacon slot available\n");
-			ret = -ENOBUFS;
-			goto out;
-		}
-		break;
-	case NL80211_IFTYPE_STATION:
-		/* Stop ANI */
-		sc->sc_flags &= ~SC_OP_ANI_RUN;
-		del_timer_sync(&common->ani.timer);
-		if ((vif->type == NL80211_IFTYPE_AP) ||
-		    (vif->type == NL80211_IFTYPE_ADHOC))
-			ath9k_reclaim_beacon(sc, vif);
-		break;
-	default:
-		ath_err(common, "Interface type %d not yet supported\n",
-				vif->type);
-		ret = -ENOTSUPP;
-		goto out;
-	}
-	vif->type = new_type;
-	vif->p2p = p2p;
-
-out:
-	mutex_unlock(&sc->mutex);
-	return ret;
+	return ath9k_iface_work(hw, vif, new_type, p2p, ATH9K_MOD_IFACE);
 }
 
 static void ath9k_remove_interface(struct ieee80211_hw *hw,
 				   struct ieee80211_vif *vif)
 {
-	struct ath_wiphy *aphy = hw->priv;
-	struct ath_softc *sc = aphy->sc;
-	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
-
-	ath_dbg(common, ATH_DBG_CONFIG, "Detach Interface\n");
-
-	mutex_lock(&sc->mutex);
-
-	/* Stop ANI */
-	sc->sc_flags &= ~SC_OP_ANI_RUN;
-	del_timer_sync(&common->ani.timer);
-
-	/* Reclaim beacon resources */
-	if ((sc->sc_ah->opmode == NL80211_IFTYPE_AP) ||
-	    (sc->sc_ah->opmode == NL80211_IFTYPE_ADHOC) ||
-	    (sc->sc_ah->opmode == NL80211_IFTYPE_MESH_POINT))
-		ath9k_reclaim_beacon(sc, vif);
-
-	sc->nvifs--;
-
-	mutex_unlock(&sc->mutex);
+	ath9k_iface_work(hw, vif, vif->type, vif->p2p, ATH9K_DEL_IFACE);
 }
 
 static void ath9k_enable_ps(struct ath_softc *sc)
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index b2497b8..f02a709 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -107,7 +107,7 @@ static void ath_setdefantenna(struct ath_softc *sc, u32 antenna)
 	sc->rx.rxotherant = 0;
 }
 
-static void ath_opmode_init(struct ath_softc *sc)
+void ath_opmode_init(struct ath_softc *sc)
 {
 	struct ath_hw *ah = sc->sc_ah;
 	struct ath_common *common = ath9k_hw_common(ah);
-- 
1.7.3.5

Re: [RFC] ath9k: Handle interface changes properly

[Björn Smedman]

On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
<rmanoharan@atheros.com> wrote:
> The commit ""ath9k: Add change_interface callback" was failed
> to update of hw opmode, ani and interrupt mask. This leads
> to break p2p functionality on ath9k. And the existing add and
> remove interface functions are not handling hw opmode and
> ANI properly.
>
> This patch combines the common code in interface callbacks
> and also takes care of multi-vif cases.

How does your patch handle the race condition between the interface
change done in process context and the beacon tasklet triggered by
SWBA?

Also, perhaps more applicable to the commit log than the patch, how
can opmode be properly handled in multi-vif cases? I mean let's say I
have two AP vifs and then change one into STA, is the opmode then STA?
Compare that to the case where I have two STA vifs and change one into
AP; so again I have one AP and one STA vif but this time opmode is AP,
right? I can see how I can be wrong about these examples but I can't
really see how the opmode concept can be properly handled in multi-vif
cases.

/Björn

Re: [RFC] ath9k: Handle interface changes properly

[Sujith]

Björn Smedman wrote:
> On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
> <rmanoharan@atheros.com> wrote:
> > The commit ""ath9k: Add change_interface callback" was failed
> > to update of hw opmode, ani and interrupt mask. This leads
> > to break p2p functionality on ath9k. And the existing add and
> > remove interface functions are not handling hw opmode and
> > ANI properly.
> >
> > This patch combines the common code in interface callbacks
> > and also takes care of multi-vif cases.
> 
> How does your patch handle the race condition between the interface
> change done in process context and the beacon tasklet triggered by
> SWBA?
> 
> Also, perhaps more applicable to the commit log than the patch, how
> can opmode be properly handled in multi-vif cases? I mean let's say I
> have two AP vifs and then change one into STA, is the opmode then STA?
> Compare that to the case where I have two STA vifs and change one into
> AP; so again I have one AP and one STA vif but this time opmode is AP,
> right? I can see how I can be wrong about these examples but I can't
> really see how the opmode concept can be properly handled in multi-vif
> cases.

The opmode should be calculated every time an interface is created/destroyed.

If an AP vif is already present when a STA is added, the opmode remains as AP.
if a STA vif is changed into AP mode, then the opmode should be changed to AP,
along with disabling PS for other STA interfaces.
If an AP is present and a STA is added, the beacon interval can't be different.
And there are a few other conditions as well...

And you are right, interface management is not protected with the beacon tasklet...

Sujith

Re: [RFC] ath9k: Handle interface changes properly

[Björn Smedman]

2011/1/12 Sujith <m.sujith@gmail.com>:
> Björn Smedman wrote:
>> Also, perhaps more applicable to the commit log than the patch, how
>> can opmode be properly handled in multi-vif cases? I mean let's say I
>> have two AP vifs and then change one into STA, is the opmode then STA?
>> Compare that to the case where I have two STA vifs and change one into
>> AP; so again I have one AP and one STA vif but this time opmode is AP,
>> right? I can see how I can be wrong about these examples but I can't
>> really see how the opmode concept can be properly handled in multi-vif
>> cases.
>
> The opmode should be calculated every time an interface is created/destroyed.
>
> If an AP vif is already present when a STA is added, the opmode remains as AP.
> if a STA vif is changed into AP mode, then the opmode should be changed to AP,
> along with disabling PS for other STA interfaces.
> If an AP is present and a STA is added, the beacon interval can't be different.
> And there are a few other conditions as well...

Thank you Sujith for the clarification. When you lay out the cases it
makes more sense. But there are still border cases, like e.g. adding
first a STA and then an AP interface. If I read main.c correctly the
opmode will then be STA and the AP vif will be broken, right?

Wouldn't it be better to keep counts of number of vifs by type and
e.g. activate SWBA if (adhocvifs > 0 || apvifs > 0) and similar? Then
it wouldn't mater in which order vifs are added, removed and changed.

/Björn

Re: [RFC] ath9k: Handle interface changes properly

[Sujith]

Björn Smedman wrote:
> Thank you Sujith for the clarification. When you lay out the cases it
> makes more sense. But there are still border cases, like e.g. adding
> first a STA and then an AP interface. If I read main.c correctly the
> opmode will then be STA and the AP vif will be broken, right?
> 
> Wouldn't it be better to keep counts of number of vifs by type and
> e.g. activate SWBA if (adhocvifs > 0 || apvifs > 0) and similar? Then
> it wouldn't mater in which order vifs are added, removed and changed.

Yep, individual interface counters have to be maintained, since bringing
down one beaconing interface shouldn't disable SWBA altogether.
Also, we should allow only one IBSS interface, and deny multi-interfaces when
it is present.

Sujith

Re: [RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

On Thu, Jan 13, 2011 at 12:30:03AM +0530, Björn Smedman wrote:
> 2011/1/12 Sujith <m.sujith@gmail.com>:
> > Björn Smedman wrote:
> >> Also, perhaps more applicable to the commit log than the patch, how
> >> can opmode be properly handled in multi-vif cases? I mean let's say I
> >> have two AP vifs and then change one into STA, is the opmode then STA?
> >> Compare that to the case where I have two STA vifs and change one into
> >> AP; so again I have one AP and one STA vif but this time opmode is AP,
> >> right? I can see how I can be wrong about these examples but I can't
> >> really see how the opmode concept can be properly handled in multi-vif
> >> cases.
> >
> > The opmode should be calculated every time an interface is created/destroyed.
> >
> > If an AP vif is already present when a STA is added, the opmode remains as AP.
> > if a STA vif is changed into AP mode, then the opmode should be changed to AP,
> > along with disabling PS for other STA interfaces.
> > If an AP is present and a STA is added, the beacon interval can't be different.
> > And there are a few other conditions as well...
> 
> Thank you Sujith for the clarification. When you lay out the cases it
> makes more sense. But there are still border cases, like e.g. adding
> first a STA and then an AP interface. If I read main.c correctly the
> opmode will then be STA and the AP vif will be broken, right?

Yes. opmode is set to the type of first vif added. Thats wrong. I should
be changed on AP/IBSS type interface addtion.
> Wouldn't it be better to keep counts of number of vifs by type and
> e.g. activate SWBA if (adhocvifs > 0 || apvifs > 0) and similar? Then
> it wouldn't mater in which order vifs are added, removed and changed.
It is not necessary to keep counter for ap/adhoc vifs. instead of that,
I reused nbcnvifs counter.

--
Rajkumar

Re: [RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

On Wed, Jan 12, 2011 at 10:52:23PM +0530, Sujith wrote:
> Björn Smedman wrote:
> > On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
> > <rmanoharan@atheros.com> wrote:
> > > The commit ""ath9k: Add change_interface callback" was failed
> > > to update of hw opmode, ani and interrupt mask. This leads
> > > to break p2p functionality on ath9k. And the existing add and
> > > remove interface functions are not handling hw opmode and
> > > ANI properly.
> > >
> > > This patch combines the common code in interface callbacks
> > > and also takes care of multi-vif cases.
> > 
> > How does your patch handle the race condition between the interface
> > change done in process context and the beacon tasklet triggered by
> > SWBA?
> > 
> > Also, perhaps more applicable to the commit log than the patch, how
> > can opmode be properly handled in multi-vif cases? I mean let's say I
> > have two AP vifs and then change one into STA, is the opmode then STA?
> > Compare that to the case where I have two STA vifs and change one into
> > AP; so again I have one AP and one STA vif but this time opmode is AP,
> > right? I can see how I can be wrong about these examples but I can't
> > really see how the opmode concept can be properly handled in multi-vif
> > cases.
> 
> The opmode should be calculated every time an interface is created/destroyed.
> 
> If an AP vif is already present when a STA is added, the opmode remains as AP.
> if a STA vif is changed into AP mode, then the opmode should be changed to AP,
> along with disabling PS for other STA interfaces.
> If an AP is present and a STA is added, the beacon interval can't be different.
> And there are a few other conditions as well...
> 
> And you are right, interface management is not protected with the beacon tasklet...
I dont see a race condition here because SWBA & beacon tasklet is disabled
before accessing beacon slot.

--
Rajkumar

Re: [RFC] ath9k: Handle interface changes properly

[Felix Fietkau]

On 2011-01-12 10:06 AM, Björn Smedman wrote:
> On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
> <rmanoharan@atheros.com>  wrote:
>>  The commit ""ath9k: Add change_interface callback" was failed
>>  to update of hw opmode, ani and interrupt mask. This leads
>>  to break p2p functionality on ath9k. And the existing add and
>>  remove interface functions are not handling hw opmode and
>>  ANI properly.
>>
>>  This patch combines the common code in interface callbacks
>>  and also takes care of multi-vif cases.
>
> How does your patch handle the race condition between the interface
> change done in process context and the beacon tasklet triggered by
> SWBA?
>
> Also, perhaps more applicable to the commit log than the patch, how
> can opmode be properly handled in multi-vif cases? I mean let's say I
> have two AP vifs and then change one into STA, is the opmode then STA?
> Compare that to the case where I have two STA vifs and change one into
> AP; so again I have one AP and one STA vif but this time opmode is AP,
> right? I can see how I can be wrong about these examples but I can't
> really see how the opmode concept can be properly handled in multi-vif
> cases.
I think opmode should be handled as follows:
If there is at least one AP interface, opmode should be AP, regardless 
of what the other interfaces are set to.
If there is no AP vif, opmode can be set to the primary vif type.

- Felix

Re: [RFC] ath9k: Handle interface changes properly

[Ben Greear]

On 01/12/2011 11:51 AM, Felix Fietkau wrote:
> On 2011-01-12 10:06 AM, Björn Smedman wrote:
>> On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
>> <rmanoharan@atheros.com> wrote:
>>> The commit ""ath9k: Add change_interface callback" was failed
>>> to update of hw opmode, ani and interrupt mask. This leads
>>> to break p2p functionality on ath9k. And the existing add and
>>> remove interface functions are not handling hw opmode and
>>> ANI properly.
>>>
>>> This patch combines the common code in interface callbacks
>>> and also takes care of multi-vif cases.
>>
>> How does your patch handle the race condition between the interface
>> change done in process context and the beacon tasklet triggered by
>> SWBA?
>>
>> Also, perhaps more applicable to the commit log than the patch, how
>> can opmode be properly handled in multi-vif cases? I mean let's say I
>> have two AP vifs and then change one into STA, is the opmode then STA?
>> Compare that to the case where I have two STA vifs and change one into
>> AP; so again I have one AP and one STA vif but this time opmode is AP,
>> right? I can see how I can be wrong about these examples but I can't
>> really see how the opmode concept can be properly handled in multi-vif
>> cases.
> I think opmode should be handled as follows:
> If there is at least one AP interface, opmode should be AP, regardless of what the other interfaces are set to.
> If there is no AP vif, opmode can be set to the primary vif type.

This seems right...I think this is what we did in ath5k.

Just FYI:  With current ath9k code, I get almost zero packet throughput to the
IP of the VAP device (ie, if it's serving DHCP) if I have a few STAs also
created & associated with some other AP.  Removing the STA interfaces makes the VAP work fine.
With or without the STAs on the VAP machine, vif to vif on vifs associated with the VAP work
fine, so it's some sort of issue with passing the pkts up the stack.

I haven't debugged this further.  I think ath5k doesn't have this problem,
but I haven't tested that scenario recently.

Thanks,
Ben

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


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

On Thu, Jan 13, 2011 at 01:21:47AM +0530, Felix Fietkau wrote:
> On 2011-01-12 10:06 AM, Björn Smedman wrote:
> > On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
> > <rmanoharan@atheros.com>  wrote:
> >>  The commit ""ath9k: Add change_interface callback" was failed
> >>  to update of hw opmode, ani and interrupt mask. This leads
> >>  to break p2p functionality on ath9k. And the existing add and
> >>  remove interface functions are not handling hw opmode and
> >>  ANI properly.
> >>
> >>  This patch combines the common code in interface callbacks
> >>  and also takes care of multi-vif cases.
> >
> > How does your patch handle the race condition between the interface
> > change done in process context and the beacon tasklet triggered by
> > SWBA?
> >
> > Also, perhaps more applicable to the commit log than the patch, how
> > can opmode be properly handled in multi-vif cases? I mean let's say I
> > have two AP vifs and then change one into STA, is the opmode then STA?
> > Compare that to the case where I have two STA vifs and change one into
> > AP; so again I have one AP and one STA vif but this time opmode is AP,
> > right? I can see how I can be wrong about these examples but I can't
> > really see how the opmode concept can be properly handled in multi-vif
> > cases.
> I think opmode should be handled as follows:
> If there is at least one AP interface, opmode should be AP, regardless 
> of what the other interfaces are set to.
> If there is no AP vif, opmode can be set to the primary vif type.
>
Correct. this RFC patch does the same.

--
Rajkumar

Re: [RFC] ath9k: Handle interface changes properly

[Felix Fietkau]

On 2011-01-13 6:18 AM, Rajkumar Manoharan wrote:
> On Thu, Jan 13, 2011 at 01:21:47AM +0530, Felix Fietkau wrote:
>>  On 2011-01-12 10:06 AM, Björn Smedman wrote:
>>  >  On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
>>  >  <rmanoharan@atheros.com>   wrote:
>>  >>   The commit ""ath9k: Add change_interface callback" was failed
>>  >>   to update of hw opmode, ani and interrupt mask. This leads
>>  >>   to break p2p functionality on ath9k. And the existing add and
>>  >>   remove interface functions are not handling hw opmode and
>>  >>   ANI properly.
>>  >>
>>  >>   This patch combines the common code in interface callbacks
>>  >>   and also takes care of multi-vif cases.
>>  >
>>  >  How does your patch handle the race condition between the interface
>>  >  change done in process context and the beacon tasklet triggered by
>>  >  SWBA?
>>  >
>>  >  Also, perhaps more applicable to the commit log than the patch, how
>>  >  can opmode be properly handled in multi-vif cases? I mean let's say I
>>  >  have two AP vifs and then change one into STA, is the opmode then STA?
>>  >  Compare that to the case where I have two STA vifs and change one into
>>  >  AP; so again I have one AP and one STA vif but this time opmode is AP,
>>  >  right? I can see how I can be wrong about these examples but I can't
>>  >  really see how the opmode concept can be properly handled in multi-vif
>>  >  cases.
>>  I think opmode should be handled as follows:
>>  If there is at least one AP interface, opmode should be AP, regardless
>>  of what the other interfaces are set to.
>>  If there is no AP vif, opmode can be set to the primary vif type.
>>
> Correct. this RFC patch does the same.
Really? I don't see that being handled properly, it still seems to 
overwrite ah->opmode based on a single vif type for some types.

Maybe it would be a good idea to clean this up and first limit the 
number of different types that we pass to ath9k_hw (i.e. only AP, ADHOC, 
STA). Later we can make a separate enum for that to avoid passing the 
type as-is entirely.

I think the mesh point opmode has no place in ath9k_hw. Right now it is 
treated like ad-hoc, but I think that's completely wrong. Mesh should 
behave just like AP mode, as no ad-hoc style TSF synchronization should 
be done by the hardware, and 802.11s mesh nodes do not compete for 
beacon transmission.

Also, there is no reason to have a WDS opmode in ath9k_hw. WDS is 
typically used along with AP mode interfaces, and where it is not, the 
AP opmode should be used for ath9k_hw anyway.

- Felix

Re: [RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

On Thu, Jan 13, 2011 at 07:53:27PM +0530, Felix Fietkau wrote:
> On 2011-01-13 6:18 AM, Rajkumar Manoharan wrote:
> > On Thu, Jan 13, 2011 at 01:21:47AM +0530, Felix Fietkau wrote:
> >>  On 2011-01-12 10:06 AM, Björn Smedman wrote:
> >>  >  On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
> >>  >  <rmanoharan@atheros.com>   wrote:
> >>  >>   The commit ""ath9k: Add change_interface callback" was failed
> >>  >>   to update of hw opmode, ani and interrupt mask. This leads
> >>  >>   to break p2p functionality on ath9k. And the existing add and
> >>  >>   remove interface functions are not handling hw opmode and
> >>  >>   ANI properly.
> >>  >>
> >>  >>   This patch combines the common code in interface callbacks
> >>  >>   and also takes care of multi-vif cases.
> >>  >
> >>  >  How does your patch handle the race condition between the interface
> >>  >  change done in process context and the beacon tasklet triggered by
> >>  >  SWBA?
> >>  >
> >>  >  Also, perhaps more applicable to the commit log than the patch, how
> >>  >  can opmode be properly handled in multi-vif cases? I mean let's say I
> >>  >  have two AP vifs and then change one into STA, is the opmode then STA?
> >>  >  Compare that to the case where I have two STA vifs and change one into
> >>  >  AP; so again I have one AP and one STA vif but this time opmode is AP,
> >>  >  right? I can see how I can be wrong about these examples but I can't
> >>  >  really see how the opmode concept can be properly handled in multi-vif
> >>  >  cases.
> >>  I think opmode should be handled as follows:
> >>  If there is at least one AP interface, opmode should be AP, regardless
> >>  of what the other interfaces are set to.
> >>  If there is no AP vif, opmode can be set to the primary vif type.
> >>
> > Correct. this RFC patch does the same.
> Really? I don't see that being handled properly, it still seems to 
> overwrite ah->opmode based on a single vif type for some types.
> Also, there is no reason to have a WDS opmode in ath9k_hw. WDS is 
> typically used along with AP mode interfaces, and where it is not, the 
> AP opmode should be used for ath9k_hw anyway.
Instead of setting opmde as AP for WDS, it is better to handle WDS
case in ath9k_hw.
> Maybe it would be a good idea to clean this up and first limit the 
> number of different types that we pass to ath9k_hw (i.e. only AP, ADHOC, 
> STA). Later we can make a separate enum for that to avoid passing the 
> type as-is entirely.
Just to stick with the currently supported interfaces list, WDS also included.
> I think the mesh point opmode has no place in ath9k_hw. Right now it is 
> treated like ad-hoc, but I think that's completely wrong. Mesh should 
> behave just like AP mode, as no ad-hoc style TSF synchronization should 
> be done by the hardware, and 802.11s mesh nodes do not compete for 
> beacon transmission.
This is a different issue and it has to be addressed in separate patch.

--
Rajkumar

Re: [RFC] ath9k: Handle interface changes properly

[Felix Fietkau]

On 2011-01-13 5:35 PM, Rajkumar Manoharan wrote:
> On Thu, Jan 13, 2011 at 07:53:27PM +0530, Felix Fietkau wrote:
>>  On 2011-01-13 6:18 AM, Rajkumar Manoharan wrote:
>>  >  On Thu, Jan 13, 2011 at 01:21:47AM +0530, Felix Fietkau wrote:
>>  >>   On 2011-01-12 10:06 AM, Björn Smedman wrote:
>>  >>   >   On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
>>  >>   >   <rmanoharan@atheros.com>    wrote:
>>  >>   >>    The commit ""ath9k: Add change_interface callback" was failed
>>  >>   >>    to update of hw opmode, ani and interrupt mask. This leads
>>  >>   >>    to break p2p functionality on ath9k. And the existing add and
>>  >>   >>    remove interface functions are not handling hw opmode and
>>  >>   >>    ANI properly.
>>  >>   >>
>>  >>   >>    This patch combines the common code in interface callbacks
>>  >>   >>    and also takes care of multi-vif cases.
>>  >>   >
>>  >>   >   How does your patch handle the race condition between the interface
>>  >>   >   change done in process context and the beacon tasklet triggered by
>>  >>   >   SWBA?
>>  >>   >
>>  >>   >   Also, perhaps more applicable to the commit log than the patch, how
>>  >>   >   can opmode be properly handled in multi-vif cases? I mean let's say I
>>  >>   >   have two AP vifs and then change one into STA, is the opmode then STA?
>>  >>   >   Compare that to the case where I have two STA vifs and change one into
>>  >>   >   AP; so again I have one AP and one STA vif but this time opmode is AP,
>>  >>   >   right? I can see how I can be wrong about these examples but I can't
>>  >>   >   really see how the opmode concept can be properly handled in multi-vif
>>  >>   >   cases.
>>  >>   I think opmode should be handled as follows:
>>  >>   If there is at least one AP interface, opmode should be AP, regardless
>>  >>   of what the other interfaces are set to.
>>  >>   If there is no AP vif, opmode can be set to the primary vif type.
>>  >>
>>  >  Correct. this RFC patch does the same.
>>  Really? I don't see that being handled properly, it still seems to
>>  overwrite ah->opmode based on a single vif type for some types.
>>  Also, there is no reason to have a WDS opmode in ath9k_hw. WDS is
>>  typically used along with AP mode interfaces, and where it is not, the
>>  AP opmode should be used for ath9k_hw anyway.
> Instead of setting opmde as AP for WDS, it is better to handle WDS
> case in ath9k_hw.
Why? Right now I don't even see any NL80211_IFTYPE_WDS handling in 
ath9k_hw, and I can't think of anything that should be handled 
differently in ath9k_hw compared to the AP opmode.

>>  Maybe it would be a good idea to clean this up and first limit the
>>  number of different types that we pass to ath9k_hw (i.e. only AP, ADHOC,
>>  STA). Later we can make a separate enum for that to avoid passing the
>>  type as-is entirely.
> Just to stick with the currently supported interfaces list, WDS also included.
>>  I think the mesh point opmode has no place in ath9k_hw. Right now it is
>>  treated like ad-hoc, but I think that's completely wrong. Mesh should
>>  behave just like AP mode, as no ad-hoc style TSF synchronization should
>>  be done by the hardware, and 802.11s mesh nodes do not compete for
>>  beacon transmission.
> This is a different issue and it has to be addressed in separate patch.
I agree.

- Felix

Re: [RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

On Thu, Jan 13, 2011 at 10:19:38PM +0530, Felix Fietkau wrote:
> On 2011-01-13 5:35 PM, Rajkumar Manoharan wrote:
> > On Thu, Jan 13, 2011 at 07:53:27PM +0530, Felix Fietkau wrote:
> >>  On 2011-01-13 6:18 AM, Rajkumar Manoharan wrote:
> >>  >  On Thu, Jan 13, 2011 at 01:21:47AM +0530, Felix Fietkau wrote:
> >>  >>   On 2011-01-12 10:06 AM, Björn Smedman wrote:
> >>  >>   >   On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
> >>  >>   >   <rmanoharan@atheros.com>    wrote:
> >>  >>   >>    The commit ""ath9k: Add change_interface callback" was failed
> >>  >>   >>    to update of hw opmode, ani and interrupt mask. This leads
> >>  >>   >>    to break p2p functionality on ath9k. And the existing add and
> >>  >>   >>    remove interface functions are not handling hw opmode and
> >>  >>   >>    ANI properly.
> >>  >>   >>
> >>  >>   >>    This patch combines the common code in interface callbacks
> >>  >>   >>    and also takes care of multi-vif cases.
> >>  >>   >
> >>  >>   >   How does your patch handle the race condition between the interface
> >>  >>   >   change done in process context and the beacon tasklet triggered by
> >>  >>   >   SWBA?
> >>  >>   >
> >>  >>   >   Also, perhaps more applicable to the commit log than the patch, how
> >>  >>   >   can opmode be properly handled in multi-vif cases? I mean let's say I
> >>  >>   >   have two AP vifs and then change one into STA, is the opmode then STA?
> >>  >>   >   Compare that to the case where I have two STA vifs and change one into
> >>  >>   >   AP; so again I have one AP and one STA vif but this time opmode is AP,
> >>  >>   >   right? I can see how I can be wrong about these examples but I can't
> >>  >>   >   really see how the opmode concept can be properly handled in multi-vif
> >>  >>   >   cases.
> >>  >>   I think opmode should be handled as follows:
> >>  >>   If there is at least one AP interface, opmode should be AP, regardless
> >>  >>   of what the other interfaces are set to.
> >>  >>   If there is no AP vif, opmode can be set to the primary vif type.
> >>  >>
> >>  >  Correct. this RFC patch does the same.
> >>  Really? I don't see that being handled properly, it still seems to
> >>  overwrite ah->opmode based on a single vif type for some types.
> >>  Also, there is no reason to have a WDS opmode in ath9k_hw. WDS is
> >>  typically used along with AP mode interfaces, and where it is not, the
> >>  AP opmode should be used for ath9k_hw anyway.
> > Instead of setting opmde as AP for WDS, it is better to handle WDS
> > case in ath9k_hw.
> Why? Right now I don't even see any NL80211_IFTYPE_WDS handling in 
> ath9k_hw, and I can't think of anything that should be handled 
> differently in ath9k_hw compared to the AP opmode.
For WDS station, what should be the interface type? Forgive if I'm wrong.
> >>  Maybe it would be a good idea to clean this up and first limit the
> >>  number of different types that we pass to ath9k_hw (i.e. only AP, ADHOC,
> >>  STA). Later we can make a separate enum for that to avoid passing the
> >>  type as-is entirely.
> > Just to stick with the currently supported interfaces list, WDS also included.
> >>  I think the mesh point opmode has no place in ath9k_hw. Right now it is
> >>  treated like ad-hoc, but I think that's completely wrong. Mesh should
> >>  behave just like AP mode, as no ad-hoc style TSF synchronization should
> >>  be done by the hardware, and 802.11s mesh nodes do not compete for
> >>  beacon transmission.
> > This is a different issue and it has to be addressed in separate patch.
> I agree.
> 
> - Felix

Re: [RFC] ath9k: Handle interface changes properly

[Felix Fietkau]

On 2011-01-14 7:13 PM, Rajkumar Manoharan wrote:
> On Thu, Jan 13, 2011 at 10:19:38PM +0530, Felix Fietkau wrote:
>>  On 2011-01-13 5:35 PM, Rajkumar Manoharan wrote:
>>  >  Instead of setting opmde as AP for WDS, it is better to handle WDS
>>  >  case in ath9k_hw.
>>  Why? Right now I don't even see any NL80211_IFTYPE_WDS handling in
>>  ath9k_hw, and I can't think of anything that should be handled
>>  differently in ath9k_hw compared to the AP opmode.
> For WDS station, what should be the interface type? Forgive if I'm wrong.
There is no WDS station opmode. 'WDS station' is a regular station 
interface with 4-addr mode enabled. It needs no special handling in ath9k.

- Felix

Re: [RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

On Fri, Jan 14, 2011 at 11:52:02PM +0530, Felix Fietkau wrote:
> On 2011-01-14 7:13 PM, Rajkumar Manoharan wrote:
> > On Thu, Jan 13, 2011 at 10:19:38PM +0530, Felix Fietkau wrote:
> >>  On 2011-01-13 5:35 PM, Rajkumar Manoharan wrote:
> >>  >  Instead of setting opmde as AP for WDS, it is better to handle WDS
> >>  >  case in ath9k_hw.
> >>  Why? Right now I don't even see any NL80211_IFTYPE_WDS handling in
> >>  ath9k_hw, and I can't think of anything that should be handled
> >>  differently in ath9k_hw compared to the AP opmode.
> > For WDS station, what should be the interface type? Forgive if I'm wrong.
> There is no WDS station opmode. 'WDS station' is a regular station 
> interface with 4-addr mode enabled. It needs no special handling in ath9k.
still not convinced. Then what is the point mac80211 is informing about
WDS type to drivers. mac itself can pass it as AP type like what it 
is doing for p2p GO.

--
Rajkumar

Re: [RFC] ath9k: Handle interface changes properly

[Felix Fietkau]

On 2011-01-14 7:53 PM, Rajkumar Manoharan wrote:
> On Fri, Jan 14, 2011 at 11:52:02PM +0530, Felix Fietkau wrote:
>>  On 2011-01-14 7:13 PM, Rajkumar Manoharan wrote:
>>  >  On Thu, Jan 13, 2011 at 10:19:38PM +0530, Felix Fietkau wrote:
>>  >>   On 2011-01-13 5:35 PM, Rajkumar Manoharan wrote:
>>  >>   >   Instead of setting opmde as AP for WDS, it is better to handle WDS
>>  >>   >   case in ath9k_hw.
>>  >>   Why? Right now I don't even see any NL80211_IFTYPE_WDS handling in
>>  >>   ath9k_hw, and I can't think of anything that should be handled
>>  >>   differently in ath9k_hw compared to the AP opmode.
>>  >  For WDS station, what should be the interface type? Forgive if I'm wrong.
>>  There is no WDS station opmode. 'WDS station' is a regular station
>>  interface with 4-addr mode enabled. It needs no special handling in ath9k.
> still not convinced. Then what is the point mac80211 is informing about
> WDS type to drivers. mac itself can pass it as AP type like what it
> is doing for p2p GO.
The WDS type is something else. If you have two APs, you can link them 
together with a separate WDS vif on each side pointing at the remote MAC 
address of the other node.
I think that when we use ah->opmode, we should only use it for very 
generic operating modes:

AP: no TSF sync, beacon tx can be enabled.
ADHOC: TSF sync against IBSS cell, beacon tx can be enabled
STATION: TSF sync against one AP, only station beacon timers for PS.

Only the above distinctions are relevant for ath9k_hw, everything else 
is handled by the driver/stack. There is no reason for adding extra 
checks to ath9k_hw for mesh and WDS, since they work best with 
ah->opmode set to AP, and there is nothing extra on the *hardware* side 
that should be configured there via a different opmode. That's why I 
think leaking the mac80211 interface types to ath9k_hw is a bad idea.

- Felix

Re: [RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

On Sat, Jan 15, 2011 at 12:36:42AM +0530, Felix Fietkau wrote:
> On 2011-01-14 7:53 PM, Rajkumar Manoharan wrote:
> > On Fri, Jan 14, 2011 at 11:52:02PM +0530, Felix Fietkau wrote:
> >>  On 2011-01-14 7:13 PM, Rajkumar Manoharan wrote:
> >>  >  On Thu, Jan 13, 2011 at 10:19:38PM +0530, Felix Fietkau wrote:
> >>  >>   On 2011-01-13 5:35 PM, Rajkumar Manoharan wrote:
> >>  >>   >   Instead of setting opmde as AP for WDS, it is better to handle WDS
> >>  >>   >   case in ath9k_hw.
> >>  >>   Why? Right now I don't even see any NL80211_IFTYPE_WDS handling in
> >>  >>   ath9k_hw, and I can't think of anything that should be handled
> >>  >>   differently in ath9k_hw compared to the AP opmode.
> >>  >  For WDS station, what should be the interface type? Forgive if I'm wrong.
> >>  There is no WDS station opmode. 'WDS station' is a regular station
> >>  interface with 4-addr mode enabled. It needs no special handling in ath9k.
> > still not convinced. Then what is the point mac80211 is informing about
> > WDS type to drivers. mac itself can pass it as AP type like what it
> > is doing for p2p GO.
> The WDS type is something else. If you have two APs, you can link them 
> together with a separate WDS vif on each side pointing at the remote MAC 
> address of the other node.
> I think that when we use ah->opmode, we should only use it for very 
> generic operating modes:
> 
> AP: no TSF sync, beacon tx can be enabled.
> ADHOC: TSF sync against IBSS cell, beacon tx can be enabled
> STATION: TSF sync against one AP, only station beacon timers for PS.
> 
> Only the above distinctions are relevant for ath9k_hw, everything else 
> is handled by the driver/stack. There is no reason for adding extra 
> checks to ath9k_hw for mesh and WDS, since they work best with 
> ah->opmode set to AP, and there is nothing extra on the *hardware* side 
> that should be configured there via a different opmode. That's why I 
> think leaking the mac80211 interface types to ath9k_hw is a bad idea.
Thanks for your detailed explanation. And meanwhile Ben is also
fixing hw mode based on vif counter. It would be better if he make
use this thread and address the same.

Ben,
Any comments.

--
Rajkumar

Re: [RFC] ath9k: Handle interface changes properly

[Ben Greear]

On 01/14/2011 11:24 AM, Rajkumar Manoharan wrote:
> On Sat, Jan 15, 2011 at 12:36:42AM +0530, Felix Fietkau wrote:
>> On 2011-01-14 7:53 PM, Rajkumar Manoharan wrote:
>>> On Fri, Jan 14, 2011 at 11:52:02PM +0530, Felix Fietkau wrote:
>>>>   On 2011-01-14 7:13 PM, Rajkumar Manoharan wrote:
>>>>   >   On Thu, Jan 13, 2011 at 10:19:38PM +0530, Felix Fietkau wrote:
>>>>   >>    On 2011-01-13 5:35 PM, Rajkumar Manoharan wrote:
>>>>   >>    >    Instead of setting opmde as AP for WDS, it is better to handle WDS
>>>>   >>    >    case in ath9k_hw.
>>>>   >>    Why? Right now I don't even see any NL80211_IFTYPE_WDS handling in
>>>>   >>    ath9k_hw, and I can't think of anything that should be handled
>>>>   >>    differently in ath9k_hw compared to the AP opmode.
>>>>   >   For WDS station, what should be the interface type? Forgive if I'm wrong.
>>>>   There is no WDS station opmode. 'WDS station' is a regular station
>>>>   interface with 4-addr mode enabled. It needs no special handling in ath9k.
>>> still not convinced. Then what is the point mac80211 is informing about
>>> WDS type to drivers. mac itself can pass it as AP type like what it
>>> is doing for p2p GO.
>> The WDS type is something else. If you have two APs, you can link them
>> together with a separate WDS vif on each side pointing at the remote MAC
>> address of the other node.
>> I think that when we use ah->opmode, we should only use it for very
>> generic operating modes:
>>
>> AP: no TSF sync, beacon tx can be enabled.
>> ADHOC: TSF sync against IBSS cell, beacon tx can be enabled
>> STATION: TSF sync against one AP, only station beacon timers for PS.
>>
>> Only the above distinctions are relevant for ath9k_hw, everything else
>> is handled by the driver/stack. There is no reason for adding extra
>> checks to ath9k_hw for mesh and WDS, since they work best with
>> ah->opmode set to AP, and there is nothing extra on the *hardware* side
>> that should be configured there via a different opmode. That's why I
>> think leaking the mac80211 interface types to ath9k_hw is a bad idea.
> Thanks for your detailed explanation. And meanwhile Ben is also
> fixing hw mode based on vif counter. It would be better if he make
> use this thread and address the same.
>
> Ben,
> Any comments.

If all I need to do is to treat WDS and MESH like an AP, that shouldn't
be too hard.

Does WDS need a beacon?

Is there any limitations to the number of WDS vifs allowed, including
combinations with other VIF types?

Thanks,
Ben

>
> --
> Rajkumar


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC] ath9k: Handle interface changes properly

[Felix Fietkau]

On 2011-01-14 8:29 PM, Ben Greear wrote:
> On 01/14/2011 11:24 AM, Rajkumar Manoharan wrote:
>>  On Sat, Jan 15, 2011 at 12:36:42AM +0530, Felix Fietkau wrote:
>>>  On 2011-01-14 7:53 PM, Rajkumar Manoharan wrote:
>>>>  On Fri, Jan 14, 2011 at 11:52:02PM +0530, Felix Fietkau wrote:
>>>>>    On 2011-01-14 7:13 PM, Rajkumar Manoharan wrote:
>>>>>    >    On Thu, Jan 13, 2011 at 10:19:38PM +0530, Felix Fietkau wrote:
>>>>>    >>     On 2011-01-13 5:35 PM, Rajkumar Manoharan wrote:
>>>>>    >>     >     Instead of setting opmde as AP for WDS, it is better to handle WDS
>>>>>    >>     >     case in ath9k_hw.
>>>>>    >>     Why? Right now I don't even see any NL80211_IFTYPE_WDS handling in
>>>>>    >>     ath9k_hw, and I can't think of anything that should be handled
>>>>>    >>     differently in ath9k_hw compared to the AP opmode.
>>>>>    >    For WDS station, what should be the interface type? Forgive if I'm wrong.
>>>>>    There is no WDS station opmode. 'WDS station' is a regular station
>>>>>    interface with 4-addr mode enabled. It needs no special handling in ath9k.
>>>>  still not convinced. Then what is the point mac80211 is informing about
>>>>  WDS type to drivers. mac itself can pass it as AP type like what it
>>>>  is doing for p2p GO.
>>>  The WDS type is something else. If you have two APs, you can link them
>>>  together with a separate WDS vif on each side pointing at the remote MAC
>>>  address of the other node.
>>>  I think that when we use ah->opmode, we should only use it for very
>>>  generic operating modes:
>>>
>>>  AP: no TSF sync, beacon tx can be enabled.
>>>  ADHOC: TSF sync against IBSS cell, beacon tx can be enabled
>>>  STATION: TSF sync against one AP, only station beacon timers for PS.
>>>
>>>  Only the above distinctions are relevant for ath9k_hw, everything else
>>>  is handled by the driver/stack. There is no reason for adding extra
>>>  checks to ath9k_hw for mesh and WDS, since they work best with
>>>  ah->opmode set to AP, and there is nothing extra on the *hardware* side
>>>  that should be configured there via a different opmode. That's why I
>>>  think leaking the mac80211 interface types to ath9k_hw is a bad idea.
>>  Thanks for your detailed explanation. And meanwhile Ben is also
>>  fixing hw mode based on vif counter. It would be better if he make
>>  use this thread and address the same.
>>
>>  Ben,
>>  Any comments.
>
> If all I need to do is to treat WDS and MESH like an AP, that shouldn't
> be too hard.
>
> Does WDS need a beacon?
No

> Is there any limitations to the number of WDS vifs allowed, including
> combinations with other VIF types?
I don't think so.

- Felix

Re: [RFC] ath9k: Handle interface changes properly

[Björn Smedman]

On Fri, Jan 14, 2011 at 8:06 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> I think that when we use ah->opmode, we should only use it for very generic
> operating modes:
>
> AP: no TSF sync, beacon tx can be enabled.
> ADHOC: TSF sync against IBSS cell, beacon tx can be enabled
> STATION: TSF sync against one AP, only station beacon timers for PS.
>
> Only the above distinctions are relevant for ath9k_hw, everything else is
> handled by the driver/stack. There is no reason for adding extra checks to
> ath9k_hw for mesh and WDS, since they work best with ah->opmode set to AP,
> and there is nothing extra on the *hardware* side that should be configured
> there via a different opmode. That's why I think leaking the mac80211
> interface types to ath9k_hw is a bad idea.

Isn't that also a good argument against using nl80211_iftype in this
context? The values currently defined are:

 * @NL80211_IFTYPE_ADHOC: independent BSS member
 * @NL80211_IFTYPE_STATION: managed BSS member
 * @NL80211_IFTYPE_AP: access point
 * @NL80211_IFTYPE_AP_VLAN: VLAN interface for access points
 * @NL80211_IFTYPE_WDS: wireless distribution interface
 * @NL80211_IFTYPE_MONITOR: monitor interface receiving all frames
 * @NL80211_IFTYPE_MESH_POINT: mesh point
 * @NL80211_IFTYPE_P2P_CLIENT: P2P client
 * @NL80211_IFTYPE_P2P_GO: P2P group owner

But what we really want to do is keep track of TSF sync and beaconing.
For beacon we have already given up on opmode and now use nbcnvifs.
Perhaps we could find some similar way to handle TSF sync?

/Björn

Re: [RFC] ath9k: Handle interface changes properly

[Rajkumar Manoharan]

On Wed, Jan 12, 2011 at 10:36:28PM +0530, Björn Smedman wrote:
> On Wed, Jan 12, 2011 at 3:30 PM, Rajkumar Manoharan
> <rmanoharan@atheros.com> wrote:
> > The commit ""ath9k: Add change_interface callback" was failed
> > to update of hw opmode, ani and interrupt mask. This leads
> > to break p2p functionality on ath9k. And the existing add and
> > remove interface functions are not handling hw opmode and
> > ANI properly.
> >
> > This patch combines the common code in interface callbacks
> > and also takes care of multi-vif cases.
> 
> How does your patch handle the race condition between the interface
> change done in process context and the beacon tasklet triggered by
> SWBA?
If you look at the patch, while removing/changing AP/IBSS interface,
SWBA interrupt was disabled and beacon tasklet is killed before
releasing beacon slot. The SWBA will enabled again in the presence
of beaconing interface.
> 
> Also, perhaps more applicable to the commit log than the patch, how
> can opmode be properly handled in multi-vif cases? I mean let's say I
> have two AP vifs and then change one into STA, is the opmode then STA?
If you have any AP vifs, then opmode will always be AP type. STA mode
will be set only if there is no beaconing interface.
> Compare that to the case where I have two STA vifs and change one into
> AP; so again I have one AP and one STA vif but this time opmode is AP,
> right? I can see how I can be wrong about these examples but I can't
> really see how the opmode concept can be properly handled in multi-vif
> cases.
If you are changing STA->AP, then opmode also changed to AP type though
you have other STA interfaces.

--
Rajkumar