Hi,
The commit "mac80211: stop tx before doing hw config and rate
update" (and a later commit fixing it) introduced a queue stop & flush
on switching 20/40.
I'm working on not calling hw_config() for switching between 20/40 but
instead only calling rate_control_rate_update() and
ieee80211_bss_info_change_notify() to tell the rate control and driver.
If we do that, is it still necessary to stop the queues for it? It seems
like it shouldn't.
johannes
On Mon, 2012-03-19 at 15:34 +0530, Sujith Manoharan wrote:
> Johannes Berg wrote:
> > Actually, no -- that's how we got into this mess to start with :-)
> > Bandwidth here is a per-peer property, BSS_CHANGED_HT is changing the
> > operation mode (things like non-HT protection).
> >
> > We used to change the channel bandwidth when the AP wanted to only
> > receive 20 MHz, but that seems like a bug because that doesn't even
> > necessarily mean it wants to TX 20 MHz.
>
> 11.14.4.3 specifies that if the operating bandwidth has been changed by the AP
> to 20 MHz, then transmitting frames in an extension channel is not allowed ?
Ok, good point. Paul's case was regulatory related, where we
(voluntarily) ceased 40 MHz transmissions.
Still though, I think reconfiguring the channel type rather than the
rate control algorithm doesn't make a lot of sense in this case since
it'd make this very special and unlike the other cases (e.g. AP mode).
johannes
On 18 March 2012 03:19, Johannes Berg <[email protected]> wrote:
> I'm not sure I can believe this -- surely the firmware has to have a way
> of dealing with stations that can't do 40 MHz and stations that can, so
> switching between the two doesn't seem like a major proposal? We also
> don't currently handle the action frames for that, but it seems like we
> should.
>
> Note, however, that we're now discussing something already done --
> Paul's patch already removed the channel type update when all we wanted
> was update the RC.
When are stations being notified of a CWM change?
I know FreeBSD handles it with a big stick at the moment - it just
does a full interface reset and channel change. I don't necessarily
like it, but I haven't yet sat down to look at when and why this is
occuring.
Adrian
On Mon, 2012-03-19 at 15:38 +0530, Sujith Manoharan wrote:
> Johannes Berg wrote:
> > > As far as ath9k/ath9k_htc is concerned, I think we have to reset/reconfigure the HW
> > > when a channel switch happens, since we reprogram various MAC/PHY registers based on
> > > the HT bandwidth. Both ath9k and ath9k_htc update their rate control modules
> > > correctly - ath9k via the rate_update() callback and ath9k_htc using BSS_CHANGED_HT
> > > in bss_info_changed().
> >
> > I'm not sure ath9k_htc is behaving correctly. Ok I should say that
> > differently -- it's probably behaving "correctly" wrt. whatever mac80211
> > did before, but we're changing the way mac80211 behaves and I believe
> > that to be (more) correct, see my other email.
>
> Well, we do a bunch of HW configuration based on the channel bandwidth, like
> TX power, program MAC/PHY registers, various calibration routines at the baseband level.
> So when the bandwidth changes, doing a full HW reset is required, I think.
I guess I'm arguing that the channel bandwidth never changed, only the
inputs to rate control changed in a way that means you'll now transmit
only 20 MHz frames on that 40 MHz channel. After all, at least in AP
mode you have to deal with that anyway if a station joins that can't
receive 40 MHz for some reason, unless you want to penalise all others
as well?
johannes
Johannes Berg wrote:
> > 11.14.4.3 specifies that if the operating bandwidth has been changed by the AP
> > to 20 MHz, then transmitting frames in an extension channel is not allowed ?
>
> Ok, good point. Paul's case was regulatory related, where we
> (voluntarily) ceased 40 MHz transmissions.
>
> Still though, I think reconfiguring the channel type rather than the
> rate control algorithm doesn't make a lot of sense in this case since
> it'd make this very special and unlike the other cases (e.g. AP mode).
The regulatory issue could have been fixed by sending out the action frame
specified in 7.4.10.2, no ?
Sujith
On Mon, 2012-03-19 at 16:02 +0530, Sujith Manoharan wrote:
> Johannes Berg wrote:
> > > 11.14.4.3 specifies that if the operating bandwidth has been changed by the AP
> > > to 20 MHz, then transmitting frames in an extension channel is not allowed ?
> >
> > Ok, good point. Paul's case was regulatory related, where we
> > (voluntarily) ceased 40 MHz transmissions.
> >
> > Still though, I think reconfiguring the channel type rather than the
> > rate control algorithm doesn't make a lot of sense in this case since
> > it'd make this very special and unlike the other cases (e.g. AP mode).
>
> The regulatory issue could have been fixed by sending out the action frame
> specified in 7.4.10.2, no ?
Technically, but that would be assuming it's implemented everywhere ...
if it was tested by the WFA we'd implement it :-)
johannes
On Mon, 2012-03-19 at 08:25 +0530, Sujith Manoharan wrote:
> Johannes Berg wrote:
> > I'm not sure I can believe this -- surely the firmware has to have a way
> > of dealing with stations that can't do 40 MHz and stations that can, so
> > switching between the two doesn't seem like a major proposal? We also
> > don't currently handle the action frames for that, but it seems like we
> > should.
>
> We have BSS_CHANGED_HT for doing this.
Actually, no -- that's how we got into this mess to start with :-)
Bandwidth here is a per-peer property, BSS_CHANGED_HT is changing the
operation mode (things like non-HT protection).
We used to change the channel bandwidth when the AP wanted to only
receive 20 MHz, but that seems like a bug because that doesn't even
necessarily mean it wants to TX 20 MHz.
So I think what Paul and I have changed/are changing it to makes more
sense:
* the channel bandwidth never changes
* the RX bandwidth for a peer has its own notification
* BSS_CHANGED_HT changes protection mode etc.
johannes
Johannes Berg wrote:
> I'm not sure I can believe this -- surely the firmware has to have a way
> of dealing with stations that can't do 40 MHz and stations that can, so
> switching between the two doesn't seem like a major proposal? We also
> don't currently handle the action frames for that, but it seems like we
> should.
We have BSS_CHANGED_HT for doing this.
Sujith
Adrian Chadd wrote:
> On 18 March 2012 03:19, Johannes Berg <[email protected]> wrote:
>
> > I'm not sure I can believe this -- surely the firmware has to have a way
> > of dealing with stations that can't do 40 MHz and stations that can, so
> > switching between the two doesn't seem like a major proposal? We also
> > don't currently handle the action frames for that, but it seems like we
> > should.
> >
> > Note, however, that we're now discussing something already done --
> > Paul's patch already removed the channel type update when all we wanted
> > was update the RC.
>
> When are stations being notified of a CWM change?
>
> I know FreeBSD handles it with a big stick at the moment - it just
> does a full interface reset and channel change. I don't necessarily
> like it, but I haven't yet sat down to look at when and why this is
> occuring.
For AP mode, we currently don't have dynamic CWM - and if it is ever implemented,
it would probably be in hostapd.
For station mode, HT bandwidth changes can be notified via beacons, probe responses
or action frames. mac80211 currently processes beacons and the HT operation element.
Also, only legacy CSA announcements are handled by mac80211 right now, the 11n
extensions to the CSA action frame are yet to be implemented.
As far as ath9k/ath9k_htc is concerned, I think we have to reset/reconfigure the HW
when a channel switch happens, since we reprogram various MAC/PHY registers based on
the HT bandwidth. Both ath9k and ath9k_htc update their rate control modules
correctly - ath9k via the rate_update() callback and ath9k_htc using BSS_CHANGED_HT
in bss_info_changed().
Sujith
Hi,
> > Yeah that's actually an interesting point -- but we have it wrong right
> > now I think. Did you see Paul's recent patch? We really shouldn't be
> > reconfiguring the channel if all we want is to stop transmitting 40 MHz,
> > we should just update rate control only. Now for in-device rate control
> > that isn't really possible today since they can't get rate_update(), but
> > that means we should add a separate callback, I think?
>
> Yes. I missed that. I am not sure about the new callback alone is
> sufficient. We have to issue a WMI command
> to reconfigure the rc in firmware. If so, it needs firmware changes
> also. May be Sujith is the right person to answer
> about ath9k_htc.
I'm not sure I can believe this -- surely the firmware has to have a way
of dealing with stations that can't do 40 MHz and stations that can, so
switching between the two doesn't seem like a major proposal? We also
don't currently handle the action frames for that, but it seems like we
should.
Note, however, that we're now discussing something already done --
Paul's patch already removed the channel type update when all we wanted
was update the RC.
johannes
On Mon, 2012-03-19 at 12:12 +0100, Johannes Berg wrote:
> On Mon, 2012-03-19 at 16:02 +0530, Sujith Manoharan wrote:
> > Johannes Berg wrote:
> > > > 11.14.4.3 specifies that if the operating bandwidth has been changed by the AP
> > > > to 20 MHz, then transmitting frames in an extension channel is not allowed ?
> > >
> > > Ok, good point. Paul's case was regulatory related, where we
> > > (voluntarily) ceased 40 MHz transmissions.
> > >
> > > Still though, I think reconfiguring the channel type rather than the
> > > rate control algorithm doesn't make a lot of sense in this case since
> > > it'd make this very special and unlike the other cases (e.g. AP mode).
> >
> > The regulatory issue could have been fixed by sending out the action frame
> > specified in 7.4.10.2, no ?
>
> Technically, but that would be assuming it's implemented everywhere ...
> if it was tested by the WFA we'd implement it :-)
Also, in order to actually implement it in AP mode we need to use the
"RC update only".
I think it makes more sense to unify the handling here -- change rate
control (and also tell the driver, like one of my patches did) rather
than treating 20/40 changes in managed mode differently than in AP mode.
johannes
On Mon, 2012-03-19 at 09:19 +0530, Sujith Manoharan wrote:
> Adrian Chadd wrote:
> > On 18 March 2012 03:19, Johannes Berg <[email protected]> wrote:
> >
> > > I'm not sure I can believe this -- surely the firmware has to have a way
> > > of dealing with stations that can't do 40 MHz and stations that can, so
> > > switching between the two doesn't seem like a major proposal? We also
> > > don't currently handle the action frames for that, but it seems like we
> > > should.
> > >
> > > Note, however, that we're now discussing something already done --
> > > Paul's patch already removed the channel type update when all we wanted
> > > was update the RC.
> >
> > When are stations being notified of a CWM change?
> >
> > I know FreeBSD handles it with a big stick at the moment - it just
> > does a full interface reset and channel change. I don't necessarily
> > like it, but I haven't yet sat down to look at when and why this is
> > occuring.
>
> For AP mode, we currently don't have dynamic CWM - and if it is ever implemented,
> it would probably be in hostapd.
Yeah, though I think if we implement the action frame in mac80211 we
could handle that in AP mode too? Might be easier not to though.
> For station mode, HT bandwidth changes can be notified via beacons, probe responses
> or action frames. mac80211 currently processes beacons and the HT operation element.
>
> Also, only legacy CSA announcements are handled by mac80211 right now, the 11n
> extensions to the CSA action frame are yet to be implemented.
Yeah, at least for VHT I'd guess those will become necessary.
> As far as ath9k/ath9k_htc is concerned, I think we have to reset/reconfigure the HW
> when a channel switch happens, since we reprogram various MAC/PHY registers based on
> the HT bandwidth. Both ath9k and ath9k_htc update their rate control modules
> correctly - ath9k via the rate_update() callback and ath9k_htc using BSS_CHANGED_HT
> in bss_info_changed().
I'm not sure ath9k_htc is behaving correctly. Ok I should say that
differently -- it's probably behaving "correctly" wrt. whatever mac80211
did before, but we're changing the way mac80211 behaves and I believe
that to be (more) correct, see my other email.
johannes
Adrian Chadd wrote:
> Right, but for AP mode what's involved?
>
> * Does it send out a CSA to the same channel but different operational
> mode stuff?
> * are all current aggregation sessions maintained, or are they reset
> when a CSA to the same channel is done?
> * What about stuff to do with power saving?
All of the above probably have to be handled. :)
(along with virtual interface management).
> I'm ignoring CWM for now until I've finished off these SMP TX
> aggregation bugs, then I'll sort out channel scanning and power saving
> stuff. That sets all the right "bits" for CWM (ie, being able to queue
> all the frames without flushing them, then restarting the software TX
> queue as needed.) Once that's done, CWM should be:
>
> * signal stop TX;
> * wait for TX threads, TX completion and TX DMA to complete;
> * signal a rate control change;
> * recalculate the rate control selection for everything in the software queues;
> * restart..
You missed the MAC/PHY update based on the new channel information.
Which is where it gets tricky, since we end up doing a full reset.
Sujith
On Tue, 2012-03-20 at 10:09 +0530, Sujith Manoharan wrote:
> > Also, in order to actually implement it in AP mode we need to use the
> > "RC update only".
> >
> > I think it makes more sense to unify the handling here -- change rate
> > control (and also tell the driver, like one of my patches did) rather
> > than treating 20/40 changes in managed mode differently than in AP mode.
>
> Agreed.
So what should we do with ath9k_htc? As I understood you, it relies on
the no-longer updated global channel type?
IOW -- I don't think the patches we were discussing break it, Paul's
patch would already have broken it. However my patch with the new
callback could help you fix it?
johannes
Johannes Berg wrote:
> > As far as ath9k/ath9k_htc is concerned, I think we have to reset/reconfigure the HW
> > when a channel switch happens, since we reprogram various MAC/PHY registers based on
> > the HT bandwidth. Both ath9k and ath9k_htc update their rate control modules
> > correctly - ath9k via the rate_update() callback and ath9k_htc using BSS_CHANGED_HT
> > in bss_info_changed().
>
> I'm not sure ath9k_htc is behaving correctly. Ok I should say that
> differently -- it's probably behaving "correctly" wrt. whatever mac80211
> did before, but we're changing the way mac80211 behaves and I believe
> that to be (more) correct, see my other email.
Well, we do a bunch of HW configuration based on the channel bandwidth, like
TX power, program MAC/PHY registers, various calibration routines at the baseband level.
So when the bandwidth changes, doing a full HW reset is required, I think.
Sujith
On Fri, 2012-03-16 at 14:01 +0100, Johannes Berg wrote:
> > Not calling hw_config could affect drivers like ath9k_htc where the rate control runs in the firmware.
> > Those drivers will update the rate control based on CHANGED_HT flag.
>
> Yeah that's actually an interesting point -- but we have it wrong right
> now I think. Did you see Paul's recent patch? We really shouldn't be
> reconfiguring the channel if all we want is to stop transmitting 40 MHz,
> we should just update rate control only. Now for in-device rate control
> that isn't really possible today since they can't get rate_update(), but
> that means we should add a separate callback, I think?
Below patch is what I've just added to my patchset. This will allow
drivers like ath9k_htc to update their internal rate control as needed
without requiring that we actually change the channel type, which we
don't want to do for the reasons we discussed with Paul in the thread
around his patch (see the thread at
http://mid.gmane.org/[email protected] and go to my later replies)
I think this is the right thing to do -- we leave the channel type set
to whatever we wanted to have to start with, and just update the 20/40
bandwidth as needed.
johannes
mac80211: notify driver of rate control updates
From: Johannes Berg <[email protected]>
Devices that have internal rate control need to be
notified when the bandwidth or SMPS state changes
just like external rate control algorithms get a
notification now.
Add this notification and clarify the change bits
while at it, the HT_CHANGED bit really meant only
bandwidth changed.
Signed-off-by: Johannes Berg <[email protected]>
---
Documentation/DocBook/80211.tmpl | 2 -
drivers/net/wireless/ath/ath9k/rc.c | 2 -
include/net/mac80211.h | 37 +++++++++++++++++++++++-------------
net/mac80211/driver-ops.h | 17 ++++++++++++++++
net/mac80211/driver-trace.h | 28 +++++++++++++++++++++++++++
net/mac80211/mlme.c | 2 -
net/mac80211/rate.h | 2 +
7 files changed, 74 insertions(+), 16 deletions(-)
--- a/Documentation/DocBook/80211.tmpl 2012-03-16 14:13:10.000000000 +0100
+++ b/Documentation/DocBook/80211.tmpl 2012-03-16 14:25:57.000000000 +0100
@@ -533,7 +533,7 @@ MISSING
!Finclude/net/mac80211.h ieee80211_start_tx_ba_cb_irqsafe
!Finclude/net/mac80211.h ieee80211_stop_tx_ba_session
!Finclude/net/mac80211.h ieee80211_stop_tx_ba_cb_irqsafe
-!Finclude/net/mac80211.h rate_control_changed
+!Finclude/net/mac80211.h ieee80211_rate_control_changed
!Finclude/net/mac80211.h ieee80211_tx_rate_control
!Finclude/net/mac80211.h rate_control_send_low
</chapter>
--- a/include/net/mac80211.h 2012-03-16 14:14:31.000000000 +0100
+++ b/include/net/mac80211.h 2012-03-16 14:36:25.000000000 +0100
@@ -1778,6 +1778,18 @@ enum ieee80211_frame_release_type {
};
/**
+ * enum ieee80211_rate_control_changed - flags to indicate what changed
+ *
+ * @IEEE80211_RC_BW_CHANGED: The bandwidth that can be used to transmit
+ * to this station changed.
+ * @IEEE80211_RC_SMPS_CHANGED: The SMPS state of the station changed.
+ */
+enum ieee80211_rate_control_changed {
+ IEEE80211_RC_BW_CHANGED = BIT(0),
+ IEEE80211_RC_SMPS_CHANGED = BIT(1),
+};
+
+/**
* struct ieee80211_ops - callbacks from mac80211 to the driver
*
* This structure contains various callbacks that the driver may
@@ -1978,6 +1990,14 @@ enum ieee80211_frame_release_type {
* up the list of states.
* The callback can sleep.
*
+ * @sta_rc_update: Notifies the driver of changes to the bitrates that can be
+ * used to transmit to the station. The changes are advertised with bits
+ * from &enum ieee80211_rate_control_changed and the values are reflected
+ * in the station data. This callback should only be used when the driver
+ * uses hardware rate control (%IEEE80211_HW_HAS_RATE_CONTROL) since
+ * otherwise the rate control algorithm is notified directly.
+ * Must be atomic.
+ *
* @conf_tx: Configure TX queue parameters (EDCF (aifs, cw_min, cw_max),
* bursting) for a hardware TX queue.
* Returns a negative error code on failure.
@@ -2194,6 +2214,10 @@ struct ieee80211_ops {
struct ieee80211_sta *sta,
enum ieee80211_sta_state old_state,
enum ieee80211_sta_state new_state);
+ void (*sta_rc_update)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta,
+ u32 changed);
int (*conf_tx)(struct ieee80211_hw *hw,
struct ieee80211_vif *vif, u16 queue,
const struct ieee80211_tx_queue_params *params);
@@ -3510,19 +3534,6 @@ void ieee80211_send_bar(struct ieee80211
/* Rate control API */
/**
- * enum rate_control_changed - flags to indicate which parameter changed
- *
- * @IEEE80211_RC_HT_CHANGED: The HT parameters of the operating channel have
- * changed, rate control algorithm can update its internal state if needed.
- * @IEEE80211_RC_SMPS_CHANGED: The SMPS state of the station changed, the rate
- * control algorithm needs to adjust accordingly.
- */
-enum rate_control_changed {
- IEEE80211_RC_HT_CHANGED = BIT(0),
- IEEE80211_RC_SMPS_CHANGED = BIT(1),
-};
-
-/**
* struct ieee80211_tx_rate_control - rate control information for/from RC algo
*
* @hw: The hardware the algorithm is invoked for.
--- a/drivers/net/wireless/ath/ath9k/rc.c 2012-03-16 14:14:31.000000000 +0100
+++ b/drivers/net/wireless/ath/ath9k/rc.c 2012-03-16 14:28:34.000000000 +0100
@@ -1447,7 +1447,7 @@ static void ath_rate_update(void *priv,
/* FIXME: Handle AP mode later when we support CWM */
- if (changed & IEEE80211_RC_HT_CHANGED) {
+ if (changed & IEEE80211_RC_BW_CHANGED) {
if (sc->sc_ah->opmode != NL80211_IFTYPE_STATION)
return;
--- a/net/mac80211/mlme.c 2012-03-16 14:23:40.000000000 +0100
+++ b/net/mac80211/mlme.c 2012-03-16 14:28:30.000000000 +0100
@@ -219,7 +219,7 @@ static u32 ieee80211_config_ht_tx(struct
sta->sta.ht_cap.cap |= IEEE80211_HT_CAP_SUP_WIDTH_20_40;
rate_control_rate_update(local, sband, sta,
- IEEE80211_RC_HT_CHANGED);
+ IEEE80211_RC_BW_CHANGED);
}
mutex_unlock(&local->sta_mtx);
--- a/net/mac80211/driver-trace.h 2012-03-15 20:43:13.000000000 +0100
+++ b/net/mac80211/driver-trace.h 2012-03-16 14:37:24.000000000 +0100
@@ -624,6 +624,34 @@ TRACE_EVENT(drv_sta_state,
)
);
+TRACE_EVENT(drv_sta_rc_update,
+ TP_PROTO(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_sta *sta,
+ u32 changed),
+
+ TP_ARGS(local, sdata, sta, changed),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ VIF_ENTRY
+ STA_ENTRY
+ __field(u32, changed)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ VIF_ASSIGN;
+ STA_ASSIGN;
+ __entry->changed = changed;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT VIF_PR_FMT STA_PR_FMT " changed: 0x%x",
+ LOCAL_PR_ARG, VIF_PR_ARG, STA_PR_ARG, __entry->changed
+ )
+);
+
TRACE_EVENT(drv_sta_add,
TP_PROTO(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata,
--- a/net/mac80211/driver-ops.h 2012-03-15 20:43:13.000000000 +0100
+++ b/net/mac80211/driver-ops.h 2012-03-16 14:35:54.000000000 +0100
@@ -474,6 +474,23 @@ int drv_sta_state(struct ieee80211_local
return ret;
}
+static inline void drv_sta_rc_update(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_sta *sta, u32 changed)
+{
+ might_sleep();
+
+ sdata = get_bss_sdata(sdata);
+ check_sdata_in_driver(sdata);
+
+ trace_drv_sta_rc_update(local, sdata, sta, changed);
+ if (local->ops->sta_rc_update)
+ local->ops->sta_rc_update(&local->hw, &sdata->vif,
+ sta, changed);
+
+ trace_drv_return_void(local);
+}
+
static inline int drv_conf_tx(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata, u16 queue,
const struct ieee80211_tx_queue_params *params)
--- a/net/mac80211/rate.h 2012-03-16 14:14:31.000000000 +0100
+++ b/net/mac80211/rate.h 2012-03-16 14:38:20.000000000 +0100
@@ -17,6 +17,7 @@
#include <net/mac80211.h>
#include "ieee80211_i.h"
#include "sta_info.h"
+#include "driver-ops.h"
struct rate_control_ref {
struct ieee80211_local *local;
@@ -72,6 +73,7 @@ static inline void rate_control_rate_upd
if (ref && ref->ops->rate_update)
ref->ops->rate_update(ref->priv, sband, ista,
priv_sta, changed);
+ drv_sta_rc_update(local, sta->sdata, &sta->sta, changed);
}
static inline void *rate_control_alloc_sta(struct rate_control_ref *ref,
Johannes Berg wrote:
> Actually, no -- that's how we got into this mess to start with :-)
> Bandwidth here is a per-peer property, BSS_CHANGED_HT is changing the
> operation mode (things like non-HT protection).
>
> We used to change the channel bandwidth when the AP wanted to only
> receive 20 MHz, but that seems like a bug because that doesn't even
> necessarily mean it wants to TX 20 MHz.
11.14.4.3 specifies that if the operating bandwidth has been changed by the AP
to 20 MHz, then transmitting frames in an extension channel is not allowed ?
Sujith
Johannes Berg wrote:
> Technically, but that would be assuming it's implemented everywhere ...
> if it was tested by the WFA we'd implement it :-)
Heh :)
> Also, in order to actually implement it in AP mode we need to use the
> "RC update only".
>
> I think it makes more sense to unify the handling here -- change rate
> control (and also tell the driver, like one of my patches did) rather
> than treating 20/40 changes in managed mode differently than in AP mode.
Agreed.
Sujith
Hi,
[I'll also fold in your other email]
> Not calling hw_config could affect drivers like ath9k_htc where the rate control runs in the firmware.
> Those drivers will update the rate control based on CHANGED_HT flag.
Yeah that's actually an interesting point -- but we have it wrong right
now I think. Did you see Paul's recent patch? We really shouldn't be
reconfiguring the channel if all we want is to stop transmitting 40 MHz,
we should just update rate control only. Now for in-device rate control
that isn't really possible today since they can't get rate_update(), but
that means we should add a separate callback, I think?
> > If we do that, is it still necessary to stop the queues for it? It seems
> > like it shouldn't.
>
> Actually the queue stop was added to avoid mismatches b/w hw and rate control. But If you dont stop the queues
> before the rate control change, the station might send the frames in different ht mode (for example ht40) wrt AP's ht mode (ht20).
Yes, but that's probably OK. It's going to do that anyway since there
are still frames in the hardware queues, and those will go out without
updated rate information. There can be no guarantee (without having some
form of hardware assist) that from the point we receive a 20/40
notification we'll send the right bandwidth.
In the other mail you said almost the same thing:
> The queue stop was added to avoid sending frames while the hw
> reconfigure is in progress.
> So that we can prevent differences b/w hw and rate control ht mode.
> But I doubt that hw_reconfig
> removal could not affect split drivers where the rc is offloaded.
Like above, I think we need a new callback for rc offloaded drivers so
that they can update when the bandwidth changes -- I do remove the
bandwidth update after all, i.e. we no longer reconfigure the channel
when it's desired to no longer TX 40 MHz.
I'll think about offloaded rate control a bit more.
johannes
> The commit "mac80211: stop tx before doing hw config and rate
> update" (and a later commit fixing it) introduced a queue stop & flush
> on switching 20/40.
>
> I'm working on not calling hw_config() for switching between 20/40 but
> instead only calling rate_control_rate_update() and
> ieee80211_bss_info_change_notify() to tell the rate control and driver.
Johannes,
Not calling hw_config could affect drivers like ath9k_htc where the rate control runs in the firmware.
Those drivers will update the rate control based on CHANGED_HT flag.
>
> If we do that, is it still necessary to stop the queues for it? It seems
> like it shouldn't.
Actually the queue stop was added to avoid mismatches b/w hw and rate control. But If you dont stop the queues
before the rate control change, the station might send the frames in different ht mode (for example ht40) wrt AP's ht mode (ht20).
-Rajkumar
Johannes Berg wrote:
> So what should we do with ath9k_htc? As I understood you, it relies on
> the no-longer updated global channel type?
>
> IOW -- I don't think the patches we were discussing break it, Paul's
> patch would already have broken it. However my patch with the new
> callback could help you fix it?
I'll test ath9k_htc, see if it's broken and then wander off toward a red
horizon, er.. I mean, fix it or something. :)
Sujith
> Hi,
>
> [I'll also fold in your other email]
>
> > Not calling hw_config could affect drivers like ath9k_htc where the rate control runs in the firmware.
> > Those drivers will update the rate control based on CHANGED_HT flag.
>
> Yeah that's actually an interesting point -- but we have it wrong right
> now I think. Did you see Paul's recent patch? We really shouldn't be
> reconfiguring the channel if all we want is to stop transmitting 40 MHz,
> we should just update rate control only. Now for in-device rate control
> that isn't really possible today since they can't get rate_update(), but
> that means we should add a separate callback, I think?
Yes. I missed that. I am not sure about the new callback alone is sufficient. We have to issue a WMI command
to reconfigure the rc in firmware. If so, it needs firmware changes also. May be Sujith is the right person to answer
about ath9k_htc.
> > > If we do that, is it still necessary to stop the queues for it? It seems
> > > like it shouldn't.
> >
> > Actually the queue stop was added to avoid mismatches b/w hw and rate control. But If you dont stop the queues
> > before the rate control change, the station might send the frames in different ht mode (for example ht40) wrt AP's ht mode (ht20).
>
> Yes, but that's probably OK. It's going to do that anyway since there
> are still frames in the hardware queues, and those will go out without
> updated rate information. There can be no guarantee (without having some
> form of hardware assist) that from the point we receive a 20/40
> notification we'll send the right bandwidth.
Yes. Sounds fine.
-Rajkumar
On Sun, 2012-03-18 at 12:19 -0700, Adrian Chadd wrote:
> On 18 March 2012 03:19, Johannes Berg <[email protected]> wrote:
>
> > I'm not sure I can believe this -- surely the firmware has to have a way
> > of dealing with stations that can't do 40 MHz and stations that can, so
> > switching between the two doesn't seem like a major proposal? We also
> > don't currently handle the action frames for that, but it seems like we
> > should.
> >
> > Note, however, that we're now discussing something already done --
> > Paul's patch already removed the channel type update when all we wanted
> > was update the RC.
>
> When are stations being notified of a CWM change?
> I know FreeBSD handles it with a big stick at the moment - it just
> does a full interface reset and channel change. I don't necessarily
> like it, but I haven't yet sat down to look at when and why this is
> occuring.
I'm not convinced this is the right thing to do -- we kinda do/did the
same thing but we want to change TX bandwidth for the AP peer, not RX
(channel) bandwidth.
johannes
On 18 March 2012 20:49, Sujith Manoharan <[email protected]> wrote:
>> When are stations being notified of a CWM change?
>>
>> I know FreeBSD handles it with a big stick at the moment - it just
>> does a full interface reset and channel change. I don't necessarily
>> like it, but I haven't yet sat down to look at when and why this is
>> occuring.
>
> For AP mode, we currently don't have dynamic CWM - and if it is ever implemented,
> it would probably be in hostapd.
Right, but for AP mode what's involved?
* Does it send out a CSA to the same channel but different operational
mode stuff?
* are all current aggregation sessions maintained, or are they reset
when a CSA to the same channel is done?
* What about stuff to do with power saving?
>
> For station mode, HT bandwidth changes can be notified via beacons, probe responses
> or action frames. mac80211 currently processes beacons and the HT operation element.
Same deal as above.
I'm ignoring CWM for now until I've finished off these SMP TX
aggregation bugs, then I'll sort out channel scanning and power saving
stuff. That sets all the right "bits" for CWM (ie, being able to queue
all the frames without flushing them, then restarting the software TX
queue as needed.) Once that's done, CWM should be:
* signal stop TX;
* wait for TX threads, TX completion and TX DMA to complete;
* signal a rate control change;
* recalculate the rate control selection for everything in the software queues;
* restart..
Adrian