[RFC PATCH 0/10] mac80211/iwlwifi: A-MPDU Tx support

[RFC PATCH 0/10] mac80211/iwlwifi: A-MPDU Tx support

[Ron Rindjunsky]

Hi folks

This series is published as RFC, as I would like to address the design
for A-MPDU TX, reflected in the API and in implementation, since the
initiator side (Tx) is a bit more complicated then the recipient side (Rx).

Recipient (Rx) flow:
====================
regular Rx -> addBA request arrives -> addBA response send -> Rx switches to
A-MPDU from starting sequence number published in addBA request -> A-MPDU Rx
-> delBA arrives.

What we would expect from Initiator (Tx) flow:
================================================ 
regular Tx -> addBA request send -> addBA response arrives -> Tx switches to
A-MPDU from starting sequence number published in addBA request -> A-MPDU Tx
-> delBA send

What are the pitfalls in Tx flow:
=================================
1 - Session "handshake" idle time - As addBA request/response is being done
during regular data flow the initiator can not just stop Tx and wait for
recipient answer because it will cause a serious drop in performance,
especially considering the fact that recipient can decline the request,
so during this period Tx should continue, much the same like recipient
does not stops Rx after addBA request arrives...

2 - Starting sequence number (ssn) - Because we don't want to stop Tx flow,
starting sequence number for first possible A-MPDU frame should be forecasted,
either based on mac80211 book keeping, but more likely on low-level driver
estimation upon current HW queues status that are likely buffered with frames
for Tx in regular mode, and in that case this forecast should be supplied to
mac80211 by low level driver so a correct addBA request with the ssn will be
issued to recipient.

3 - Finish Tx of regular MPDUs - If recipient does accept the addBA, and
A-MPDU session should start, low level driver should first check that all
queued frames for regular Tx on same TID prior to published ssn were delivered
to recipient, and only then allow A-MPDU Tx. When stopping A-MPDU the opposite
should happen, finish sending all A-MPDU, and only then send delBA and
do regular Tx.

4 - Queue policy - regular Tx is based on QoS AC granularity, while A-MPDU Tx
is based on TID/Receiving Address granularity, so already upon A-MPDU
_intention_ (before addBA request is issued), already a classification change
should occur, and return to QoS granularity only if aggregation stopped or
declined.

5 - Queues drain (HW queues and Qdiscs) - so, derived from the above,
either when starting or stopping A-MPDU session, HW queues and Qdiscs should
first be examined and drained, so no packet loss due to wrong Tx mode,
delay in Tx, or leftovers that were buffered in the queues will occur.

So, considering these constraints, low-level driver needs to handle next issues:

1 - Starting sequence number (deliver to mac80211).
    Done through the ampdu_action ops interface.
2 - Make sure HW queue are clear to Tx (TID criteria) and ready to
    A-MPDU when starting aggregation (and report to mac80211 MLME),
    and clear and ready to regular Tx when stopping aggregation
    (and report to mac80211 MLME), This readiness will be reported
    to mac80211 through the supplied call backs.

While mac80211 will have to control:

1 - Session level (MLME), you can review these changes inside the patches.
2 - AC/TIDxRA queue policy (Qdisc). I have a functioning solution here,
    but will be glad to hear comments and suggestions, as I not sure about
    this implementation.

Thanks everyone
Ron


---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[RFC PATCH 01/10] mac80211: A-MPDU Tx add session's and low level driver's API

[Ron Rindjunsky]

This patch adds the API for 3 stages in A-MPDU Tx session flow:
- request mac80211 to start/stop A-MPDU Tx session for specific TID. such a
  request should be issued by a load aware element, either mac80211 itself
  or external element.
- requests by mac80211 to low-level driver to start/stop Tx aggregation.
  notice that low level driver responds now with Starting Sequence Number.
- async feedback by low-level to mac80211 to inform that HW is ready for
  next A-MPDU Tx state.

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 include/net/mac80211.h       |   62 +++++++++++++++++++++++++++++++++++++++++-
 net/mac80211/ieee80211_sta.c |    4 +-
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 3b7bfc6..3151fcf 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -943,10 +943,14 @@ enum ieee80211_erp_change_flags {
  * &struct ieee80211_ops to indicate which action is needed.
  * @IEEE80211_AMPDU_RX_START: start Rx aggregation
  * @IEEE80211_AMPDU_RX_STOP: stop Rx aggregation
+ * @IEEE80211_AMPDU_TX_START: start Tx aggregation
+ * @IEEE80211_AMPDU_TX_STOP: stop Tx aggregation
  */
 enum ieee80211_ampdu_mlme_action {
 	IEEE80211_AMPDU_RX_START,
 	IEEE80211_AMPDU_RX_STOP,
+	IEEE80211_AMPDU_TX_START,
+	IEEE80211_AMPDU_TX_STOP,
 };
 
 /**
@@ -1130,7 +1134,7 @@ struct ieee80211_ops {
 	int (*conf_ht)(struct ieee80211_hw *hw, struct ieee80211_conf *conf);
 	int (*ampdu_action)(struct ieee80211_hw *hw,
 			    enum ieee80211_ampdu_mlme_action action,
-			    const u8 *ra, u16 tid, u16 ssn);
+			    u8 *addr, u16 tid, u16 *ssn);
 };
 
 /**
@@ -1513,4 +1517,60 @@ void ieee80211_iterate_active_interfaces(struct ieee80211_hw *hw,
 						struct ieee80211_vif *vif),
 					 void *data);
 
+/**
+ * ieee80211_start_tx_ba_session - Start a tx Block Ack session.
+ * @hw: pointer as obtained from ieee80211_alloc_hw().
+ * @ra: destination address of the BA session recipient
+ * @tid: the TID to BA on.
+ * @return: success if addBA request was sent, failure otherwise
+ *
+ * Although mac80211/low level driver/user space application can estimate
+ * the need to start aggregation on a certain RA/TID, the session level
+ * will be managed by the mac80211.
+ */
+int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid);
+
+/**
+ * ieee80211_start_tx_ba_cb/_irqsafe - low level driver is ready to aggregate.
+ * @hw: pointer as obtained from ieee80211_alloc_hw().
+ * @ra: destination address of the BA session recipient.
+ * @tid: the TID to BA on.
+ *
+ * This function must be called by low level driver once it has
+ * finished with preparations for the BA session.
+ * Two versions of the function are provided for low-deriver's convinience,
+ * a regular and irqsafe version.
+ */
+void ieee80211_start_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u16 tid);
+void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, u8 *ra,
+				      u16 tid);
+/**
+ * ieee80211_stop_tx_ba_session - Stop a Block Ack session.
+ * @hw: pointer as obtained from ieee80211_alloc_hw().
+ * @ra: destination address of the BA session recipient
+ * @tid: the TID to stop BA.
+ * @initiator: if indicates initiator DELBA frame will be sent.
+ * @return: error if no sta with matching da found, success otherwise
+ *
+ * Although mac80211/low level driver/user space application can estimate
+ * the need to stop aggregation on a certain RA/TID, the session level
+ * will be managed by the mac80211.
+ */
+int ieee80211_stop_tx_ba_session(struct ieee80211_hw *hw, u8 *ra,
+				 u16 tid, u8 initiator);
+
+/**
+ * ieee80211_stop_BA_cb - low level driver is ready to stop aggregation.
+ * @hw: pointer as obtained from ieee80211_alloc_hw().
+ * @ra: destination address of the BA session recipient.
+ * @tid: the desired TID to BA on.
+ *
+ * This function must be called by low level driver once it has
+ * finished with preparations for the BA session tear down.
+ * Two versions of the function are provided for low-deriver's convinience,
+ * a regular and an irqsafe version.
+ */
+void ieee80211_stop_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u8 tid);
+void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, u8 *ra, u16 tid);
+
 #endif /* MAC80211_H */
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index b1e7d17..ec62938 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -1132,7 +1132,7 @@ static void ieee80211_sta_process_addba_request(struct net_device *dev,
 
 	if (local->ops->ampdu_action)
 		ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_RX_START,
-					       sta->addr, tid, start_seq_num);
+					       sta->addr, tid, &start_seq_num);
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Rx A-MPDU on tid %d result %d", tid, ret);
 #endif /* CONFIG_MAC80211_HT_DEBUG */
@@ -1237,7 +1237,7 @@ void ieee80211_sta_stop_rx_ba_session(struct net_device *dev, u8 *ra, u16 tid,
 	BUG_ON(!local->ops->ampdu_action);
 
 	ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_RX_STOP,
-					ra, tid, EINVAL);
+					ra, tid, NULL);
 	if (ret)
 		printk(KERN_DEBUG "HW problem - can not stop rx "
 				"aggergation for tid %d\n", tid);
-- 
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [RFC PATCH 01/10] mac80211: A-MPDU Tx add session's and low level driver's API

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 3697 bytes --]

Hi Ron,

Thanks for the explaining comments in your 0/10 mail, that really helps.

A few small comments.

> +/**
> + * ieee80211_start_tx_ba_session - Start a tx Block Ack session.
> + * @hw: pointer as obtained from ieee80211_alloc_hw().
> + * @ra: destination address of the BA session recipient
> + * @tid: the TID to BA on.
> + * @return: success if addBA request was sent, failure otherwise
> + *
> + * Although mac80211/low level driver/user space application can estimate
> + * the need to start aggregation on a certain RA/TID, the session level
> + * will be managed by the mac80211.
> + */
> +int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid);

It seems to me that this should be per-virtual interface instead of
per-hardware? I guess ultimately it won't make a difference because the
RA will be unique but I think we should still keep it per-vif where it
makes sense.

> +/**
> + * ieee80211_start_tx_ba_cb/_irqsafe - low level driver is ready to aggregate.
> + * @hw: pointer as obtained from ieee80211_alloc_hw().
> + * @ra: destination address of the BA session recipient.
> + * @tid: the TID to BA on.
> + *
> + * This function must be called by low level driver once it has
> + * finished with preparations for the BA session.
> + * Two versions of the function are provided for low-deriver's convinience,
> + * a regular and irqsafe version.
> + */
> +void ieee80211_start_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u16 tid);
> +void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, u8 *ra,
> +				      u16 tid);

You actually need to write kerneldoc in two different comments so it can
be embedded into output properly. I know, I should be writing the
mac80211 book... I'll get around to it again, I promise :) Could you
help with an 11n part of the book that explains how all this session
stuff works? Just some text, possibly within the header file as

/**
 * DOC: Aggregation
 *
 *...
 */

Also, should we just call it _irq instead of _irqsafe? I personally
dislike the rx_irqsafe naming too.

> +/**
> + * ieee80211_stop_tx_ba_session - Stop a Block Ack session.
> + * @hw: pointer as obtained from ieee80211_alloc_hw().
> + * @ra: destination address of the BA session recipient
> + * @tid: the TID to stop BA.
> + * @initiator: if indicates initiator DELBA frame will be sent.
> + * @return: error if no sta with matching da found, success otherwise

DA? The variable is called RA though.

> + * Although mac80211/low level driver/user space application can estimate
> + * the need to stop aggregation on a certain RA/TID, the session level
> + * will be managed by the mac80211.
> + */
> +int ieee80211_stop_tx_ba_session(struct ieee80211_hw *hw, u8 *ra,
> +				 u16 tid, u8 initiator);

Shouldn't the "initiator" argument get a proper enum type?

> +/**
> + * ieee80211_stop_BA_cb - low level driver is ready to stop aggregation.
> + * @hw: pointer as obtained from ieee80211_alloc_hw().
> + * @ra: destination address of the BA session recipient.
> + * @tid: the desired TID to BA on.
> + *
> + * This function must be called by low level driver once it has
> + * finished with preparations for the BA session tear down.
> + * Two versions of the function are provided for low-deriver's convinience,

Typo, should be "convenience". Same as with the other one though, you
need to copy the kerneldoc and explain them separately.

>  	ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_RX_STOP,
> -					ra, tid, EINVAL);
> +					ra, tid, NULL);

I think the kerneldoc should mention that ssn can be NULL for the
RX_STOP notification.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC PATCH 01/10] mac80211: A-MPDU Tx add session's and low level driver's API

[Ron Rindjunsky]

> > +int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid);
>
> It seems to me that this should be per-virtual interface instead of
> per-hardware? I guess ultimately it won't make a difference because the
> RA will be unique but I think we should still keep it per-vif where it
> makes sense.
>
I can pass only vif in all the API i give, and that's something that i
am not sure about since you introduced vif - generally all functions
can get hw just from having the vif, but still there are plenty of
places in the code that i see hw being passed back and forth. is it
just because API compatibility or because something else i need to
consider?

> > +void ieee80211_start_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u16 tid);
> > +void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, u8 *ra,
> > +                                   u16 tid);
>
> You actually need to write kerneldoc in two different comments so it can
> be embedded into output properly

code duplication in comments :-)
i'll separate them, no problem.

>. I know, I should be writing the
> mac80211 book... I'll get around to it again, I promise :) Could you
> help with an 11n part of the book that explains how all this session
> stuff works? Just some text, possibly within the header file as
>

Gladly! the really basic functioanlity I have already showed in patch
0/10. I guess you are thinking about something more then that so lets
talk about it in a diffrent thread. i also didn't understand what
header file you mean.

> /**
>  * DOC: Aggregation
>  *
>  *...
>  */
>
> Also, should we just call it _irq instead of _irqsafe? I personally
> dislike the rx_irqsafe naming too.

it is the same as the naming of tx_status_irqsafe and rx_irqsafe. you
want to change this convention?

>
> > +/**
> > + * ieee80211_stop_tx_ba_session - Stop a Block Ack session.
> > + * @hw: pointer as obtained from ieee80211_alloc_hw().
> > + * @ra: destination address of the BA session recipient
> > + * @tid: the TID to stop BA.
> > + * @initiator: if indicates initiator DELBA frame will be sent.
> > + * @return: error if no sta with matching da found, success otherwise
>
> DA? The variable is called RA though.

will change, thanks

> > +int ieee80211_stop_tx_ba_session(struct ieee80211_hw *hw, u8 *ra,
> > +                              u16 tid, u8 initiator);
>
> Shouldn't the "initiator" argument get a proper enum type?

agree, will change

> > +/**
> > + * ieee80211_stop_BA_cb - low level driver is ready to stop aggregation.
> > + * @hw: pointer as obtained from ieee80211_alloc_hw().
> > + * @ra: destination address of the BA session recipient.
> > + * @tid: the desired TID to BA on.
> > + *
> > + * This function must be called by low level driver once it has
> > + * finished with preparations for the BA session tear down.
> > + * Two versions of the function are provided for low-deriver's convinience,
>
> Typo, should be "convenience". Same as with the other one though, you
> need to copy the kerneldoc and explain them separately.
>

ok

> >       ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_RX_STOP,
> > -                                     ra, tid, EINVAL);
> > +                                     ra, tid, NULL);
>
> I think the kerneldoc should mention that ssn can be NULL for the
> RX_STOP notification.
>

I'll add it

> johannes
>
>

Re: [RFC PATCH 01/10] mac80211: A-MPDU Tx add session's and low level driver's API

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]


> > It seems to me that this should be per-virtual interface instead of
> > per-hardware? I guess ultimately it won't make a difference because the
> > RA will be unique but I think we should still keep it per-vif where it
> > makes sense.
> >
> I can pass only vif in all the API i give, and that's something that i
> am not sure about since you introduced vif - generally all functions
> can get hw just from having the vif, but still there are plenty of
> places in the code that i see hw being passed back and forth. is it
> just because API compatibility or because something else i need to
> consider?

Basically I'm thinking that everything that could in theory be
per-interface should be getting a vif. Say you have a multi-BSS AP. I
don't think it's really legal for a STA to associate to multiple APs at
the same time but want to avoid the code falling over when a STA
actually does associate to multiple of our virtual BSSes... Maybe that's
just unnecessary.

> > You actually need to write kerneldoc in two different comments so it can
> > be embedded into output properly
> 
> code duplication in comments :-)

Yeah, heh, I know.

> Gladly! the really basic functioanlity I have already showed in patch
> 0/10. I guess you are thinking about something more then that so lets
> talk about it in a diffrent thread. i also didn't understand what
> header file you mean.

Right. I'll try to find some time to resurrect the mac80211 book. Then
when I post that we can talk about an 11n chapter.

> > Also, should we just call it _irq instead of _irqsafe? I personally
> > dislike the rx_irqsafe naming too.
> 
> it is the same as the naming of tx_status_irqsafe and rx_irqsafe. you
> want to change this convention?

Well I dislike it only because it implies that you can call it from
non-irq contexts too, which you shouldn't. But no, I don't feel strongly
about it, stick with the _irqsafe you have.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC PATCH 01/10] mac80211: A-MPDU Tx add session's and low level driver's API

[Ron Rindjunsky]

> > > It seems to me that this should be per-virtual interface instead of
> > > per-hardware? I guess ultimately it won't make a difference because the
> > > RA will be unique but I think we should still keep it per-vif where it
> > > makes sense.
> > >
> > I can pass only vif in all the API i give, and that's something that i
> > am not sure about since you introduced vif - generally all functions
> > can get hw just from having the vif, but still there are plenty of
> > places in the code that i see hw being passed back and forth. is it
> > just because API compatibility or because something else i need to
> > consider?
>
> Basically I'm thinking that everything that could in theory be
> per-interface should be getting a vif. Say you have a multi-BSS AP. I
> don't think it's really legal for a STA to associate to multiple APs at
> the same time but want to avoid the code falling over when a STA
> actually does associate to multiple of our virtual BSSes... Maybe that's
> just unnecessary.
>

I am not sure about the necessity of it myself, as once A-MPDU is
active all traffic for the RA/TID (no matter what interface usese it)
should be aggregated.
I will currently leave it this way, and if you see any problem  please
describe it specifically and i will change it either on this patch or
on top of it.

[RFC PATCH 02/10] mac80211: A-MPDU Tx add MLME structures

[Ron Rindjunsky]

This patch adds the needed structures to describe the Tx aggregation MLME per STA
new:
 - struct tid_ampdu_tx: TID aggregation information (Tx)
changed:
 - struct sta_ampdu_mlme: Tx aggregation information per TID and
			  dialog token creator were added
 - struct sta_info: tid_to_tx_q added for tid<->tx queue mapping

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 net/mac80211/sta_info.h |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 96fe3ed..48f25f2 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -33,15 +33,36 @@
 
 #define STA_TID_NUM 16
 #define ADDBA_RESP_INTERVAL HZ
+#define HT_AGG_MAX_RETRIES		(0x3)
 
 #define HT_AGG_STATE_INITIATOR_SHIFT	(4)
 
+#define HT_ADDBA_REQUESTED_MSK		BIT(0)
+#define HT_ADDBA_DRV_READY_MSK		BIT(1)
+#define HT_ADDBA_RECEIVED_MSK		BIT(2)
 #define HT_AGG_STATE_REQ_STOP_BA_MSK	BIT(3)
-
+#define HT_AGG_STATE_INITIATOR_MSK      BIT(HT_AGG_STATE_INITIATOR_SHIFT)
 #define HT_AGG_STATE_IDLE		(0x0)
 #define HT_AGG_STATE_OPERATIONAL	(0x7)
 
 /**
+ * struct tid_ampdu_tx - TID aggregation information (Tx).
+ *
+ * @state: TID's state in session state machine.
+ * @dialog_token: dialog token for aggregation session
+ * @ssn: Starting Sequence Number expected to be aggregated.
+ * @addba_resp_timer: timer for peer's response to addba request
+ * @addba_req_num: number of times addBA request has been sent.
+ */
+struct tid_ampdu_tx {
+	u8 state;
+	u8 dialog_token;
+	u16 ssn;
+	struct timer_list addba_resp_timer;
+	u8 addba_req_num;
+};
+
+/**
  * struct tid_ampdu_rx - TID aggregation information (Rx).
  *
  * @state: TID's state in session state machine.
@@ -69,12 +90,18 @@ struct tid_ampdu_rx {
 /**
  * struct sta_ampdu_mlme - STA aggregation information.
  *
- * @tid_agg_info_rx: aggregation info for Rx per TID
+ * @tid_rx: aggregation info for Rx per TID
+ * @tid_tx: aggregation info for Tx per TID
  * @ampdu_rx: for locking sections in aggregation Rx flow
+ * @ampdu_tx: for locking sectionsi in aggregation Tx flow
+ * @dialog_token_allocator: dialog token enumerator for each new session;
  */
 struct sta_ampdu_mlme {
 	struct tid_ampdu_rx tid_rx[STA_TID_NUM];
+	struct tid_ampdu_tx tid_tx[STA_TID_NUM];
 	spinlock_t ampdu_rx;
+	spinlock_t ampdu_tx;
+	u8 dialog_token_allocator;
 };
 
 struct sta_info {
@@ -148,6 +175,7 @@ struct sta_info {
 					     of this STA */
 	struct sta_ampdu_mlme ampdu_mlme;
 	u8 timer_to_tid[STA_TID_NUM];	/* convert timer id to tid */
+	u8 tid_to_tx_q[STA_TID_NUM];	/* map tid to tx queue */
 
 #ifdef CONFIG_MAC80211_DEBUGFS
 	struct sta_info_debugfsdentries {
-- 
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [RFC PATCH 02/10] mac80211: A-MPDU Tx add MLME structures

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]



> +#define HT_AGG_STATE_INITIATOR_MSK      BIT(HT_AGG_STATE_INITIATOR_SHIFT)

I think you should use a tab here instead of spaces ;)

>  struct sta_ampdu_mlme {
>  	struct tid_ampdu_rx tid_rx[STA_TID_NUM];
> +	struct tid_ampdu_tx tid_tx[STA_TID_NUM];
>  	spinlock_t ampdu_rx;
> +	spinlock_t ampdu_tx;
> +	u8 dialog_token_allocator;
>  };
>  
>  struct sta_info {
> @@ -148,6 +175,7 @@ struct sta_info {
>  					     of this STA */
>  	struct sta_ampdu_mlme ampdu_mlme;
>  	u8 timer_to_tid[STA_TID_NUM];	/* convert timer id to tid */
> +	u8 tid_to_tx_q[STA_TID_NUM];	/* map tid to tx queue */

I wonder if we're reaching a point where this is blowing up the sta info
stuff enough to warrant putting in ifdefs for the aggregation stuff...
When you have a small AP where you know it won't be able to do
aggregation due to the hardware, you could turn it off and save a
significant amount of memory per sta-info. Not sure though, and we
should definitely do it *after* this patch series if at all.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[RFC PATCH 03/10] mac80211: A-MPDU Tx adding basic functionality

[Ron Rindjunsky]

This patch adds the following abilities to mac80211:
 - start A-MPDU Tx session
 - stop A-MPDU Tx session
 - call backs to start/stop A-MPDU Tx session
 - sending addBA request
 - processing addBA response

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 net/mac80211/ieee80211.c     |  343 ++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h   |    8 +
 net/mac80211/ieee80211_sta.c |  178 ++++++++++++++++++++++
 3 files changed, 529 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c
index 5093a1a..6342fc6 100644
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -406,6 +406,331 @@ static int ieee80211_stop(struct net_device *dev)
 	return 0;
 }
 
+int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct sta_info *sta;
+	struct ieee80211_sub_if_data *sdata;
+	u16 start_seq_num = 0;
+	u8 *state;
+	int ret;
+	DECLARE_MAC_BUF(mac);
+
+	if (tid >= STA_TID_NUM)
+		return -EINVAL;
+
+#ifdef CONFIG_MAC80211_HT_DEBUG
+	printk(KERN_DEBUG "Open BA session requested for %s tid %u\n",
+				print_mac(mac, ra), tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+
+	sta = sta_info_get(local, ra);
+	if (!sta) {
+		printk(KERN_ERR "Could not find the station\n");
+		return -ENOENT;
+	}
+
+	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
+
+	/* we have tried too many times, reciever does not want A-MPDU */
+	if (sta->ampdu_mlme.tid_tx[tid].addba_req_num >	HT_AGG_MAX_RETRIES) {
+		ret = -EBUSY;
+		goto start_ba_exit;
+	}
+
+	state = &sta->ampdu_mlme.tid_tx[tid].state;
+	/* check if the TID is not in aggregation flow already */
+	if (*state != HT_AGG_STATE_IDLE) {
+#ifdef CONFIG_MAC80211_HT_DEBUG
+		printk(KERN_DEBUG "BA request denied - session is not "
+				 "idle on tid %u\n", tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+		ret = -EAGAIN;
+		goto start_ba_exit;
+	}
+
+	/* FIXME: need a better way to ensure that TX flow won't interrupt us*/
+	/* until the end of the call to requeue function */
+	spin_lock_bh(&local->mdev->queue_lock);
+
+	/* create a new queue for this aggregation */
+	ret = ieee80211_ht_agg_queue_add(local, sta, tid);
+
+	/* case no queue is available to aggregation */
+	/* don't switch to aggregation */
+	if (ret) {
+#ifdef CONFIG_MAC80211_HT_DEBUG
+		printk(KERN_DEBUG "BA request denied - no queue available for"
+					" tid %d\n", tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+		spin_unlock_bh(&local->mdev->queue_lock);
+		goto start_ba_exit;
+	}
+	sdata = IEEE80211_DEV_TO_SUB_IF(sta->dev);
+
+	/* Ok, the Addba frame hasn't been sent yet, but if the driver calls the
+	 * call back right away, it must see that the flow has begun */
+	*state |= HT_ADDBA_REQUESTED_MSK;
+
+	if (local->ops->ampdu_action)
+		ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_TX_START,
+						ra, tid, &start_seq_num);
+
+	if (ret) {
+		/* No need to requeue the packets in the agg queue, since we
+		 * held the tx lock: no packet could be enqueued to the newly
+		 * allocated queue */
+		ieee80211_ht_agg_queue_remove(local, sta, tid, 0);
+#ifdef CONFIG_MAC80211_HT_DEBUG
+		printk(KERN_DEBUG "BA request denied - HW or queue unavailable"
+				" for tid %d\n", tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+		spin_unlock_bh(&local->mdev->queue_lock);
+		*state = HT_AGG_STATE_IDLE;
+		goto start_ba_exit;
+	}
+
+	/* Will put all the packets in the new SW queue */
+	ieee80211_requeue(local, ieee802_1d_to_ac[tid]);
+	spin_unlock_bh(&local->mdev->queue_lock);
+
+	/* We have most probably almost emptied the legacy queue */
+	ieee80211_wake_queue(local_to_hw(local), ieee802_1d_to_ac[tid]);
+
+	/* send an addBA request */
+	sta->ampdu_mlme.dialog_token_allocator++;
+	sta->ampdu_mlme.tid_tx[tid].dialog_token =
+			sta->ampdu_mlme.dialog_token_allocator;
+	sta->ampdu_mlme.tid_tx[tid].ssn = start_seq_num;
+
+	ieee80211_send_addba_request(sta->dev, ra, tid,
+			 sta->ampdu_mlme.tid_tx[tid].dialog_token,
+			 sta->ampdu_mlme.tid_tx[tid].ssn,
+			 0x40, 5000);
+
+	/* activate the timer for the recipient's addBA response */
+	sta->ampdu_mlme.tid_tx[tid].addba_resp_timer.expires =
+				jiffies + ADDBA_RESP_INTERVAL;
+	add_timer(&sta->ampdu_mlme.tid_tx[tid].addba_resp_timer);
+	printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid);
+
+start_ba_exit:
+	spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+	sta_info_put(sta);
+	return ret;
+}
+EXPORT_SYMBOL(ieee80211_start_tx_ba_session);
+
+int ieee80211_stop_tx_ba_session(struct ieee80211_hw *hw, u8 *ra,
+				 u16 tid, u8 initiator)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct sta_info *sta;
+	u8 *state;
+	int ret = 0;
+	DECLARE_MAC_BUF(mac);
+
+	if (tid >= STA_TID_NUM)
+		return -EINVAL;
+
+#ifdef CONFIG_MAC80211_HT_DEBUG
+	printk(KERN_ERR "Stop a BA session requested for %s tid %u\n",
+				print_mac(mac, ra), tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+
+	sta = sta_info_get(local, ra);
+	if (!sta)
+		return -ENOENT;
+
+	/* check if the TID is in aggregation */
+	state = &sta->ampdu_mlme.tid_tx[tid].state;
+	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
+
+	if (*state != HT_AGG_STATE_OPERATIONAL) {
+		printk(KERN_ERR "Call to %s, whith AGG non operational\n",
+								__func__);
+		ret = -ENOENT;
+		goto stop_BA_exit;
+	}
+
+	ieee80211_stop_queue(hw, sta->tid_to_tx_q[tid]);
+
+	*state = HT_AGG_STATE_REQ_STOP_BA_MSK |
+		(initiator << HT_AGG_STATE_INITIATOR_SHIFT);
+
+	/* Tell the driver to stop its HW queue */
+	if (local->ops->ampdu_action)
+		ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_TX_STOP,
+						ra, tid, NULL);
+
+	/* case HW denied going back to legacy */
+	if (ret) {
+		printk(KERN_ERR "Driver could not stop aggregations\n");
+		*state = HT_AGG_STATE_OPERATIONAL;
+		ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
+		goto stop_BA_exit;
+	}
+
+stop_BA_exit:
+	spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+	sta_info_put(sta);
+	return ret;
+}
+EXPORT_SYMBOL(ieee80211_stop_tx_ba_session);
+
+void ieee80211_start_tx_ba_cb(struct ieee80211_hw *hw, u8 *da, u16 tid)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct sta_info *sta;
+	u8 *state;
+	DECLARE_MAC_BUF(mac);
+
+	if (tid >= STA_TID_NUM) {
+		printk(KERN_ERR "Bad TID value: tid = %d >= %d = STA_TID_NUM\n",
+				tid, STA_TID_NUM);
+		return;
+	}
+
+	printk(KERN_ERR "Start a BA session requested on DA %s tid %d\n",
+				print_mac(mac, da), tid);
+
+	sta = sta_info_get(local, da);
+	if (!sta) {
+		printk(KERN_ERR "Could not find station: %s\n",
+				print_mac(mac, da));
+		return;
+	}
+
+	state = &sta->ampdu_mlme.tid_tx[tid].state;
+	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
+
+	if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
+		printk(KERN_ERR "%s, state not HT_ADDBA_REQUESTED_MSK: %d\n",
+			__func__, *state);
+		spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+		sta_info_put(sta);
+		return;
+	}
+
+	if (*state & HT_ADDBA_DRV_READY_MSK)
+		printk(KERN_ERR "driver already prepared for aggregation\n");
+
+	*state |= HT_ADDBA_DRV_READY_MSK;
+
+	if (*state == HT_AGG_STATE_OPERATIONAL) {
+		printk(KERN_ERR "%s. OPERATIONNAL\n", __func__);
+		ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
+	}
+	spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+	sta_info_put(sta);
+}
+EXPORT_SYMBOL(ieee80211_start_tx_ba_cb);
+
+void ieee80211_stop_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u8 tid)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct sta_info *sta;
+	u8 *state;
+	int agg_queue;
+	DECLARE_MAC_BUF(mac);
+
+	if (tid >= STA_TID_NUM) {
+		printk(KERN_ERR "Bad TID value: tid = %d >= %d = STA_TID_NUM\n",
+				tid, STA_TID_NUM);
+		return;
+	}
+
+	printk(KERN_DEBUG "Stop a BA session requested on DA %s tid %d\n",
+				print_mac(mac, ra), tid);
+
+	sta = sta_info_get(local, ra);
+	if (!sta) {
+		printk(KERN_ERR "Could not find station: %s\n",
+				print_mac(mac, ra));
+		return;
+	}
+	state = &sta->ampdu_mlme.tid_tx[tid].state;
+
+	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
+	if ((*state & HT_AGG_STATE_REQ_STOP_BA_MSK) == 0) {
+		printk(KERN_ERR "Call to %s while not waiting for driver\n"
+				, __func__);
+		sta_info_put(sta);
+		spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+		return;
+	}
+
+	if (*state & HT_AGG_STATE_INITIATOR_MSK)
+		ieee80211_send_delba(sta->dev, ra, tid,
+			WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE);
+
+	agg_queue = sta->tid_to_tx_q[tid];
+
+	/* avoid ordering issues: we are the only one that can modify
+	 * the content of the qdiscs */
+	spin_lock_bh(&local->mdev->queue_lock);
+	/* remove the queue for this aggregation */
+	ieee80211_ht_agg_queue_remove(local, sta, tid, 1);
+	spin_unlock_bh(&local->mdev->queue_lock);
+
+	/* Not scheduling the device leads to a stall in the TX when qdisc */
+	/* contains more than ~220 packets... */
+	netif_schedule(local->mdev);
+	*state = HT_AGG_STATE_IDLE;
+	spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+
+	sta_info_put(sta);
+}
+EXPORT_SYMBOL(ieee80211_stop_tx_ba_cb);
+
+void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_hw *hw,
+				      u8 *ra, u16 tid)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_hdr *hdr;
+	struct sk_buff *skb = dev_alloc_skb(sizeof(struct ieee80211_hdr));
+
+	if (unlikely(!skb)) {
+		if (net_ratelimit())
+			printk(KERN_WARNING "%s: Not enough memory, "
+			       "dropping start BA session", skb->dev->name);
+		return;
+	}
+	hdr = (struct ieee80211_hdr *) skb_put(skb,
+					sizeof(struct ieee80211_hdr));
+	memcpy(&hdr->addr1, ra, ETH_ALEN);
+	hdr->frame_control = tid;
+
+	skb->pkt_type = IEEE80211_ADDBA_MSG;
+	skb_queue_tail(&local->skb_queue, skb);
+	tasklet_schedule(&local->tasklet);
+}
+EXPORT_SYMBOL(ieee80211_start_tx_ba_cb_irqsafe);
+
+void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_hw *hw,
+				     u8 *ra, u16 tid)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_hdr *hdr;
+	struct sk_buff *skb = dev_alloc_skb(sizeof(struct ieee80211_hdr));
+
+	if (unlikely(!skb)) {
+		if (net_ratelimit())
+			printk(KERN_WARNING "%s: Not enough memory, "
+			       "dropping stop BA session", skb->dev->name);
+		return;
+	}
+	hdr = (struct ieee80211_hdr *) skb_put(skb,
+					sizeof(struct ieee80211_hdr));
+	memcpy(&hdr->addr1, ra, ETH_ALEN);
+	hdr->frame_control = tid;
+
+	skb->pkt_type = IEEE80211_DELBA_MSG;
+	skb_queue_tail(&local->skb_queue, skb);
+	tasklet_schedule(&local->tasklet);
+}
+EXPORT_SYMBOL(ieee80211_stop_tx_ba_cb_irqsafe);
+
 static void ieee80211_set_multicast_list(struct net_device *dev)
 {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
@@ -700,6 +1025,8 @@ static void ieee80211_tasklet_handler(unsigned long data)
 	struct sk_buff *skb;
 	struct ieee80211_rx_status rx_status;
 	struct ieee80211_tx_status *tx_status;
+	struct ieee80211_hdr *hdr;
+	u8 tid;
 
 	while ((skb = skb_dequeue(&local->skb_queue)) ||
 	       (skb = skb_dequeue(&local->skb_queue_unreliable))) {
@@ -720,6 +1047,22 @@ static void ieee80211_tasklet_handler(unsigned long data)
 					    skb, tx_status);
 			kfree(tx_status);
 			break;
+		case IEEE80211_DELBA_MSG:
+			hdr = (struct ieee80211_hdr *) skb->data;
+			/* the frame control contains the tid */
+			tid = hdr->frame_control & 0xf;
+			ieee80211_stop_tx_ba_cb(local_to_hw(local),
+						hdr->addr1, tid);
+			dev_kfree_skb(skb);
+			break;
+		case IEEE80211_ADDBA_MSG:
+			hdr = (struct ieee80211_hdr *) skb->data;
+			/* the frame control contains the tid */
+			tid = hdr->frame_control & 0xf;
+			ieee80211_start_tx_ba_cb(local_to_hw(local),
+						 hdr->addr1, tid);
+			dev_kfree_skb(skb);
+			break ;
 		default: /* should never get here! */
 			printk(KERN_ERR "%s: Unknown message type (%d)\n",
 			       wiphy_name(local->hw.wiphy), skb->pkt_type);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 383d8d0..b8b246c 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -403,6 +403,8 @@ struct ieee80211_sub_if_data *vif_to_sdata(struct ieee80211_vif *p)
 enum {
 	IEEE80211_RX_MSG	= 1,
 	IEEE80211_TX_STATUS_MSG	= 2,
+	IEEE80211_DELBA_MSG	= 3,
+	IEEE80211_ADDBA_MSG	= 4,
 };
 
 struct ieee80211_local {
@@ -776,9 +778,15 @@ int ieee80211_ht_cap_ie_to_ht_info(struct ieee80211_ht_cap *ht_cap_ie,
 int ieee80211_ht_addt_info_ie_to_ht_bss_info(
 			struct ieee80211_ht_addt_info *ht_add_info_ie,
 			struct ieee80211_ht_bss_info *bss_info);
+void ieee80211_send_addba_request(struct net_device *dev, const u8 *da,
+				  u16 tid, u8 dialog_token, u16 start_seq_num,
+				  u16 agg_size, u16 timeout);
+void ieee80211_send_delba(struct net_device *dev, const u8 *da, u16 tid,
+				u16 initiator, u16 reason_code);
 void ieee80211_sta_stop_rx_ba_session(struct net_device *dev, u8 *da,
 				u16 tid, u16 initiator, u16 reason);
 void sta_rx_agg_session_timer_expired(unsigned long data);
+void sta_addba_resp_timer_expired(unsigned long data);
 /* ieee80211_iface.c */
 int ieee80211_if_add(struct net_device *dev, const char *name,
 		     struct net_device **new_dev, int type);
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index ec62938..551a4d3 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -1050,6 +1050,58 @@ static void ieee80211_send_addba_resp(struct net_device *dev, u8 *da, u16 tid,
 	return;
 }
 
+void ieee80211_send_addba_request(struct net_device *dev, const u8 *da,
+				u16 tid, u8 dialog_token, u16 start_seq_num,
+				u16 agg_size, u16 timeout)
+{
+	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_if_sta *ifsta = &sdata->u.sta;
+	struct sk_buff *skb;
+	struct ieee80211_mgmt *mgmt;
+	u16 capab;
+
+	skb = dev_alloc_skb(sizeof(*mgmt) + local->hw.extra_tx_headroom + 1 +
+				sizeof(mgmt->u.action.u.addba_req));
+
+
+	if (!skb) {
+		printk(KERN_ERR "%s: failed to allocate buffer "
+				"for addba request frame\n", dev->name);
+		return;
+	}
+	skb_reserve(skb, local->hw.extra_tx_headroom);
+	mgmt = (struct ieee80211_mgmt *) skb_put(skb, 24);
+	memset(mgmt, 0, 24);
+	memcpy(mgmt->da, da, ETH_ALEN);
+	memcpy(mgmt->sa, dev->dev_addr, ETH_ALEN);
+	if (sdata->vif.type == IEEE80211_IF_TYPE_AP)
+		memcpy(mgmt->bssid, dev->dev_addr, ETH_ALEN);
+	else
+		memcpy(mgmt->bssid, ifsta->bssid, ETH_ALEN);
+
+	mgmt->frame_control = IEEE80211_FC(IEEE80211_FTYPE_MGMT,
+					IEEE80211_STYPE_ACTION);
+
+	skb_put(skb, 1 + sizeof(mgmt->u.action.u.addba_req));
+
+	mgmt->u.action.category = WLAN_CATEGORY_BACK;
+	mgmt->u.action.u.addba_req.action_code = WLAN_ACTION_ADDBA_REQ;
+
+	mgmt->u.action.u.addba_req.dialog_token = dialog_token;
+	capab = (u16)(1 << 1);		/* bit 1 aggregation policy */
+	capab |= (u16)(tid << 2); 	/* bit 5:2 TID number */
+	capab |= (u16)(agg_size << 6);	/* bit 15:6 max size of aggergation */
+
+	mgmt->u.action.u.addba_req.capab = cpu_to_le16(capab);
+
+	mgmt->u.action.u.addba_req.timeout = cpu_to_le16(timeout);
+	mgmt->u.action.u.addba_req.start_seq_num =
+					cpu_to_le16(start_seq_num << 4);
+
+	ieee80211_sta_tx(dev, skb, 0);
+}
+
 static void ieee80211_sta_process_addba_request(struct net_device *dev,
 						struct ieee80211_mgmt *mgmt,
 						size_t len)
@@ -1160,6 +1212,78 @@ end_no_lock:
 	sta_info_put(sta);
 }
 
+static void ieee80211_sta_process_addba_resp(struct net_device *dev,
+					     struct ieee80211_mgmt *mgmt,
+					     size_t len)
+{
+	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	struct ieee80211_hw *hw = &local->hw;
+	struct sta_info *sta;
+	u16 capab;
+	u16 tid;
+	u8 *state;
+
+	sta = sta_info_get(local, mgmt->sa);
+	if (!sta)
+		return;
+
+	capab = le16_to_cpu(mgmt->u.action.u.addba_resp.capab);
+	tid = (capab & IEEE80211_ADDBA_PARAM_TID_MASK) >> 2;
+
+	state = &sta->ampdu_mlme.tid_tx[tid].state;
+
+	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
+
+	if (mgmt->u.action.u.addba_resp.dialog_token !=
+		sta->ampdu_mlme.tid_tx[tid].dialog_token) {
+		spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+#ifdef CONFIG_MAC80211_HT_DEBUG
+		printk(KERN_DEBUG "wrong addBA response token, tid %d\n", tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+		sta_info_put(sta);
+		return;
+	}
+
+	del_timer_sync(&sta->ampdu_mlme.tid_tx[tid].addba_resp_timer);
+#ifdef CONFIG_MAC80211_HT_DEBUG
+	printk(KERN_DEBUG "switched off addBA timer for tid %d \n", tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+	if (le16_to_cpu(mgmt->u.action.u.addba_resp.status)
+			== WLAN_STATUS_SUCCESS) {
+		if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
+			spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+			printk(KERN_DEBUG "state not HT_ADDBA_REQUESTED_MSK:"
+				"%d\n", *state);
+			sta_info_put(sta);
+			return;
+		}
+
+		if (*state & HT_ADDBA_RECEIVED_MSK)
+			printk(KERN_DEBUG "double addBA response\n");
+
+		*state |= HT_ADDBA_RECEIVED_MSK;
+		sta->ampdu_mlme.tid_tx[tid].addba_req_num = 0;
+
+		if (*state == HT_AGG_STATE_OPERATIONAL) {
+			printk(KERN_ERR "%s. OPERATIONNAL\n", __func__);
+			ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
+		}
+
+		spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+		printk(KERN_DEBUG "recipient accepted agg: tid %d \n", tid);
+	} else {
+		printk(KERN_DEBUG "recipient rejected agg: tid %d \n", tid);
+
+		sta->ampdu_mlme.tid_tx[tid].addba_req_num++;
+		/* this will allow the state check in stop_BA_session */
+		*state = HT_AGG_STATE_OPERATIONAL;
+		spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+		ieee80211_stop_tx_ba_session(hw, sta->addr, tid,
+					     WLAN_BACK_INITIATOR);
+	}
+	sta_info_put(sta);
+}
+
 void ieee80211_send_delba(struct net_device *dev, const u8 *da, u16 tid,
 				u16 initiator, u16 reason_code)
 {
@@ -1266,6 +1390,7 @@ void ieee80211_sta_stop_rx_ba_session(struct net_device *dev, u8 *ra, u16 tid,
 	sta_info_put(sta);
 }
 
+
 static void ieee80211_sta_process_delba(struct net_device *dev,
 			struct ieee80211_mgmt *mgmt, size_t len)
 {
@@ -1297,6 +1422,53 @@ static void ieee80211_sta_process_delba(struct net_device *dev,
 }
 
 /*
+ * After sending add Block Ack request we activated a timer until
+ * add Block Ack response will arrive from the recipient.
+ * If this timer expires sta_addba_resp_timer_expired will be executed.
+ */
+void sta_addba_resp_timer_expired(unsigned long data)
+{
+	/* not an elegant detour, but there is no choice as the timer passes
+	 * only one argument, and verious sta_info are needed here, so init
+	 * flow in sta_info_add gives the TID as data, while the timer_to_id
+	 * array gives the sta through container_of */
+	u16 tid = *(int *)data;
+	struct sta_info *temp_sta = container_of((void *)data,
+		struct sta_info, timer_to_tid[tid]);
+
+	struct ieee80211_local *local = temp_sta->local;
+	struct ieee80211_hw *hw = &local->hw;
+	struct sta_info *sta;
+	u8 *state;
+
+	sta = sta_info_get(local, temp_sta->addr);
+	if (!sta)
+		return;
+
+	state = &sta->ampdu_mlme.tid_tx[tid].state;
+	/* check if the TID waits for addBA response */
+	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
+	if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
+		spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+		*state = HT_AGG_STATE_IDLE;
+		printk(KERN_DEBUG "timer expired on tid %d but we are not "
+				"expecting addBA response there", tid);
+		goto timer_expired_exit;
+	}
+
+	printk(KERN_DEBUG "addBA response timer expired on tid %d\n", tid);
+
+	/* go through the state check in stop_BA_session */
+	*state = HT_AGG_STATE_OPERATIONAL;
+	spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
+	ieee80211_stop_tx_ba_session(hw, temp_sta->addr, tid,
+				     WLAN_BACK_INITIATOR);
+
+timer_expired_exit:
+	sta_info_put(sta);
+}
+
+/*
  * After receiving Block Ack Request (BAR) we activated a
  * timer after each frame arrives from the originator.
  * if this timer expires ieee80211_sta_stop_rx_ba_session will be executed.
@@ -2236,6 +2408,12 @@ static void ieee80211_rx_mgmt_action(struct net_device *dev,
 				break;
 			ieee80211_sta_process_addba_request(dev, mgmt, len);
 			break;
+		case WLAN_ACTION_ADDBA_RESP:
+			if (len < (IEEE80211_MIN_ACTION_SIZE +
+				   sizeof(mgmt->u.action.u.addba_resp)))
+				break;
+			ieee80211_sta_process_addba_resp(dev, mgmt, len);
+			break;
 		case WLAN_ACTION_DELBA:
 			if (len < (IEEE80211_MIN_ACTION_SIZE +
 				   sizeof(mgmt->u.action.u.delba)))
-- 
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [RFC PATCH 03/10] mac80211: A-MPDU Tx adding basic functionality

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 6624 bytes --]


> +	sta = sta_info_get(local, ra);
> +	if (!sta) {
> +		printk(KERN_ERR "Could not find the station\n");
> +		return -ENOENT;
> +	}
> +
> +	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
> +
> +	/* we have tried too many times, reciever does not want A-MPDU */

typo: "receiver"

> +	if (sta->ampdu_mlme.tid_tx[tid].addba_req_num >	HT_AGG_MAX_RETRIES) {
> +		ret = -EBUSY;
> +		goto start_ba_exit;

Can this counter reset somehow? Or do we only implicitly reset it when
the station disassociates and associates again?

> +	/* FIXME: need a better way to ensure that TX flow won't interrupt us*/
> +	/* until the end of the call to requeue function */
> +	spin_lock_bh(&local->mdev->queue_lock);

Hm. I think the queue lock here is pretty much the only way. On the
other hand, maybe here's the point where we should talk about
multi-queue ethernet devices?

> +	if (local->ops->ampdu_action)
> +		ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_TX_START,
> +						ra, tid, &start_seq_num);
> +
> +	if (ret) {
> +		/* No need to requeue the packets in the agg queue, since we
> +		 * held the tx lock: no packet could be enqueued to the newly
> +		 * allocated queue */
> +		ieee80211_ht_agg_queue_remove(local, sta, tid, 0);
> +#ifdef CONFIG_MAC80211_HT_DEBUG
> +		printk(KERN_DEBUG "BA request denied - HW or queue unavailable"
> +				" for tid %d\n", tid);
> +#endif /* CONFIG_MAC80211_HT_DEBUG */
> +		spin_unlock_bh(&local->mdev->queue_lock);
> +		*state = HT_AGG_STATE_IDLE;
> +		goto start_ba_exit;
> +	}
> +
> +	/* Will put all the packets in the new SW queue */
> +	ieee80211_requeue(local, ieee802_1d_to_ac[tid]);
> +	spin_unlock_bh(&local->mdev->queue_lock);
> +
> +	/* We have most probably almost emptied the legacy queue */
> +	ieee80211_wake_queue(local_to_hw(local), ieee802_1d_to_ac[tid]);
> +
> +	/* send an addBA request */
> +	sta->ampdu_mlme.dialog_token_allocator++;
> +	sta->ampdu_mlme.tid_tx[tid].dialog_token =
> +			sta->ampdu_mlme.dialog_token_allocator;
> +	sta->ampdu_mlme.tid_tx[tid].ssn = start_seq_num;
> +
> +	ieee80211_send_addba_request(sta->dev, ra, tid,
> +			 sta->ampdu_mlme.tid_tx[tid].dialog_token,
> +			 sta->ampdu_mlme.tid_tx[tid].ssn,
> +			 0x40, 5000);
> +
> +	/* activate the timer for the recipient's addBA response */
> +	sta->ampdu_mlme.tid_tx[tid].addba_resp_timer.expires =
> +				jiffies + ADDBA_RESP_INTERVAL;
> +	add_timer(&sta->ampdu_mlme.tid_tx[tid].addba_resp_timer);
> +	printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid);
> +
> +start_ba_exit:
> +	spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
> +	sta_info_put(sta);
> +	return ret;
> +}
> +EXPORT_SYMBOL(ieee80211_start_tx_ba_session);

Why is this exported? I thought this is the first step of enabling
aggregation?


> +	/* check if the TID is in aggregation */
> +	state = &sta->ampdu_mlme.tid_tx[tid].state;
> +	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
> +
> +	if (*state != HT_AGG_STATE_OPERATIONAL) {
> +		printk(KERN_ERR "Call to %s, whith AGG non operational\n",
> +								__func__);
> +		ret = -ENOENT;
> +		goto stop_BA_exit;

I dislike the __func__ usage here, why not just say "tried to disable TX
aggregation while not operational" or something? Typo: "with".

> +	/* Tell the driver to stop its HW queue */
> +	if (local->ops->ampdu_action)
> +		ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_TX_STOP,
> +						ra, tid, NULL);

Does it really stop the hw queue? Isn't that more like "stop
aggregation"? Does the hardware actually have queues for each of these?
Hm. I'm familiar enough with this I think.

> +	/* case HW denied going back to legacy */
> +	if (ret) {
> +		printk(KERN_ERR "Driver could not stop aggregations\n");
> +		*state = HT_AGG_STATE_OPERATIONAL;
> +		ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
> +		goto stop_BA_exit;

Is there really a use case for a driver denying this? IOW, can you think
of a good reason why a driver would deny it?

> +EXPORT_SYMBOL(ieee80211_stop_tx_ba_session);

Again, why export this?

> +	if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
> +		printk(KERN_ERR "%s, state not HT_ADDBA_REQUESTED_MSK: %d\n",
> +			__func__, *state);

Please just write the message w/o the function name.

> +	if (*state & HT_ADDBA_DRV_READY_MSK)
> +		printk(KERN_ERR "driver already prepared for aggregation\n");

That's a bit odd. Shouldn't it rather be a WARN_ON() because the driver
did something weird?

> +	*state |= HT_ADDBA_DRV_READY_MSK;
> +
> +	if (*state == HT_AGG_STATE_OPERATIONAL) {
> +		printk(KERN_ERR "%s. OPERATIONNAL\n", __func__);

typo, weird message :)

> +	/* avoid ordering issues: we are the only one that can modify
> +	 * the content of the qdiscs */
> +	spin_lock_bh(&local->mdev->queue_lock);
> +	/* remove the queue for this aggregation */
> +	ieee80211_ht_agg_queue_remove(local, sta, tid, 1);
> +	spin_unlock_bh(&local->mdev->queue_lock);
> +
> +	/* Not scheduling the device leads to a stall in the TX when qdisc */
> +	/* contains more than ~220 packets... */
> +	netif_schedule(local->mdev);

This seems a bit weird, can you explain a bit more?

> +	hdr = (struct ieee80211_hdr *) skb_put(skb,
> +					sizeof(struct ieee80211_hdr));
> +	memcpy(&hdr->addr1, ra, ETH_ALEN);
> +	hdr->frame_control = tid;

Hrm. This is just strange and hard to understand imho. Can't we just use
some other structure instead of and ieee80211_hdr and push that onto the
queue? Then we shouldn't be using an skb queue any more I guess if we're
going to use it like this. For the skbs we can use skb->cb for our own
list structure, and here we just use another structure that embeds our
list structure...

> +	/* not an elegant detour, but there is no choice as the timer passes
> +	 * only one argument, and verious sta_info are needed here, so init

typo: various, but I think it should read something like "both the
sta_info and the TID are needed here"?

> +	/* check if the TID waits for addBA response */
> +	spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
> +	if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
> +		spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
> +		*state = HT_AGG_STATE_IDLE;
> +		printk(KERN_DEBUG "timer expired on tid %d but we are not "
> +				"expecting addBA response there", tid);
> +		goto timer_expired_exit;
> +	}

Couldn't this also happen when the response was just received while the
timer expires but the response grabs the ampdu_tx spinlock before the
timer does? Seems that should be in a comment and not worry about a
printk.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC PATCH 03/10] mac80211: A-MPDU Tx adding basic functionality

[Ron Rindjunsky]

> > +
> > +     /* we have tried too many times, reciever does not want A-MPDU */
>
> typo: "receiver"

thanks

>
> > +     if (sta->ampdu_mlme.tid_tx[tid].addba_req_num > HT_AGG_MAX_RETRIES) {
> > +             ret = -EBUSY;
> > +             goto start_ba_exit;
>
> Can this counter reset somehow? Or do we only implicitly reset it when
> the station disassociates and associates again?
>

agree. I'll handle this value.

> > +     /* FIXME: need a better way to ensure that TX flow won't interrupt us*/
> > +     /* until the end of the call to requeue function */
> > +     spin_lock_bh(&local->mdev->queue_lock);
>
> Hm. I think the queue lock here is pretty much the only way. On the
> other hand, maybe here's the point where we should talk about
> multi-queue ethernet devices?



> > +EXPORT_SYMBOL(ieee80211_start_tx_ba_session);
>
> Why is this exported? I thought this is the first step of enabling
> aggregation?

this is the trigger to aggregation, but a load aware entity should
call it. in my next series of patches i'll work on this issue, but
basically i would like to give the option to start/stop aggregations
to other modules (e.g. rate scaling module)

> > +     if (*state != HT_AGG_STATE_OPERATIONAL) {
> > +             printk(KERN_ERR "Call to %s, whith AGG non operational\n",
> > +                                                             __func__);
>
> I dislike the __func__ usage here, why not just say "tried to disable TX
> aggregation while not operational" or something? Typo: "with".
>

will change

> > +     /* Tell the driver to stop its HW queue */
> > +     if (local->ops->ampdu_action)
> > +             ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_TX_STOP,
> > +                                             ra, tid, NULL);
>
> Does it really stop the hw queue? Isn't that more like "stop
> aggregation"? Does the hardware actually have queues for each of these?
> Hm. I'm familiar enough with this I think.
>

I'll remove this comment anyhow, as it is misleading, but the basic
idea is what i wrote in 0/10 - first we need to drain all A-MPDUs,
only then we can delBA, otherwise we are confusing the receiver.

> > +     /* case HW denied going back to legacy */
> > +     if (ret) {
> > +             printk(KERN_ERR "Driver could not stop aggregations\n");
> > +             *state = HT_AGG_STATE_OPERATIONAL;
> > +             ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
> > +             goto stop_BA_exit;
>
> Is there really a use case for a driver denying this? IOW, can you think
> of a good reason why a driver would deny it?

can't think about one right now, but i am not familiar with other HWs.
maybe a self contained HW decision based on frames load can lead to
that. in any case i would like to give the driver this option.

>
> > +EXPORT_SYMBOL(ieee80211_stop_tx_ba_session);
>
> Again, why export this?

please see above.

>
> > +     if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
> > +             printk(KERN_ERR "%s, state not HT_ADDBA_REQUESTED_MSK: %d\n",
> > +                     __func__, *state);
>
> Please just write the message w/o the function name.

will do

>
> > +     if (*state & HT_ADDBA_DRV_READY_MSK)
> > +             printk(KERN_ERR "driver already prepared for aggregation\n");
>
> That's a bit odd. Shouldn't it rather be a WARN_ON() because the driver
> did something weird?

that's a good option, i'll put it.

> > +             printk(KERN_ERR "%s. OPERATIONNAL\n", __func__);
>
> typo, weird message :)

changed it, thanks

>
> > +     /* avoid ordering issues: we are the only one that can modify
> > +      * the content of the qdiscs */
> > +     spin_lock_bh(&local->mdev->queue_lock);
> > +     /* remove the queue for this aggregation */
> > +     ieee80211_ht_agg_queue_remove(local, sta, tid, 1);
> > +     spin_unlock_bh(&local->mdev->queue_lock);
> > +
> > +     /* Not scheduling the device leads to a stall in the TX when qdisc */
> > +     /* contains more than ~220 packets... */
> > +     netif_schedule(local->mdev);
>
> This seems a bit weird, can you explain a bit more?

i'll change the comment here to a clearer one. since the qdisc has
just been requeued then heavy load (~220 frames queued) might lead us
to miss a softirq for this qdisc. we can't use ieee80211_wake_queue as
the qdisc is not necessarily stopped, so this is a solution for it.

>
> > +     hdr = (struct ieee80211_hdr *) skb_put(skb,
> > +                                     sizeof(struct ieee80211_hdr));
> > +     memcpy(&hdr->addr1, ra, ETH_ALEN);
> > +     hdr->frame_control = tid;
>
> Hrm. This is just strange and hard to understand imho. Can't we just use
> some other structure instead of and ieee80211_hdr and push that onto the
> queue? Then we shouldn't be using an skb queue any more I guess if we're
> going to use it like this. For the skbs we can use skb->cb for our own
> list structure, and here we just use another structure that embeds our
> list structure...
>

this is a use of an existing mechanism, I wouldn't like another queue
just for this purpose, though a misuse, i agree. i can either stay
with this code, use cb for this purpose, or create a new struct in
mac80211.h, somehting like ieee80211_ra_tid and pass it. what do you
think os the best?


> > +     /* not an elegant detour, but there is no choice as the timer passes
> > +      * only one argument, and verious sta_info are needed here, so init
>
> typo: various, but I think it should read something like "both the
> sta_info and the TID are needed here"?

thanks

>
> > +     /* check if the TID waits for addBA response */
> > +     spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
> > +     if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
> > +             spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
> > +             *state = HT_AGG_STATE_IDLE;
> > +             printk(KERN_DEBUG "timer expired on tid %d but we are not "
> > +                             "expecting addBA response there", tid);
> > +             goto timer_expired_exit;
> > +     }
>
> Couldn't this also happen when the response was just received while the
> timer expires but the response grabs the ampdu_tx spinlock before the
> timer does? Seems that should be in a comment and not worry about a
> printk.

the condition is on whether addBA request was sent or not for this
address and TID, not on the addBA request.

>
> johannes
>
>

Re: [RFC PATCH 03/10] mac80211: A-MPDU Tx adding basic functionality

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 4955 bytes --]


> > > +EXPORT_SYMBOL(ieee80211_start_tx_ba_session);
> >
> > Why is this exported? I thought this is the first step of enabling
> > aggregation?
> 
> this is the trigger to aggregation, but a load aware entity should
> call it. in my next series of patches i'll work on this issue, but
> basically i would like to give the option to start/stop aggregations
> to other modules (e.g. rate scaling module)

Ah. Let only export it then though, please put the export into that
patch series. Maybe a rate control algorithm should indicate this in
some other way via the callbacks anyway?

> > > +     /* Tell the driver to stop its HW queue */
> > > +     if (local->ops->ampdu_action)
> > > +             ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_TX_STOP,
> > > +                                             ra, tid, NULL);
> >
> > Does it really stop the hw queue? Isn't that more like "stop
> > aggregation"? Does the hardware actually have queues for each of these?
> > Hm. I'm familiar enough with this I think.
> >
> 
> I'll remove this comment anyhow, as it is misleading, but the basic
> idea is what i wrote in 0/10 - first we need to drain all A-MPDUs,
> only then we can delBA, otherwise we are confusing the receiver.

Ok, I'm trying to learn 11n here too, does the hardware need extra
queues for the A-MPDU feature?

> > > +     /* case HW denied going back to legacy */
> > > +     if (ret) {
> > > +             printk(KERN_ERR "Driver could not stop aggregations\n");
> > > +             *state = HT_AGG_STATE_OPERATIONAL;
> > > +             ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
> > > +             goto stop_BA_exit;
> >
> > Is there really a use case for a driver denying this? IOW, can you think
> > of a good reason why a driver would deny it?
> 
> can't think about one right now, but i am not familiar with other HWs.
> maybe a self contained HW decision based on frames load can lead to
> that. in any case i would like to give the driver this option.

Ok. Let's put

WARN_ON(ret != -EBUSY)

into that if (ret) { block though to force the driver to give exactly
that error code (and document that) so that callers of
ieee80211_stop_tx_ba_session() can discover that the driver failed.

> > > +     /* avoid ordering issues: we are the only one that can modify
> > > +      * the content of the qdiscs */
> > > +     spin_lock_bh(&local->mdev->queue_lock);
> > > +     /* remove the queue for this aggregation */
> > > +     ieee80211_ht_agg_queue_remove(local, sta, tid, 1);
> > > +     spin_unlock_bh(&local->mdev->queue_lock);
> > > +
> > > +     /* Not scheduling the device leads to a stall in the TX when qdisc */
> > > +     /* contains more than ~220 packets... */
> > > +     netif_schedule(local->mdev);
> >
> > This seems a bit weird, can you explain a bit more?
> 
> i'll change the comment here to a clearer one. since the qdisc has
> just been requeued then heavy load (~220 frames queued) might lead us
> to miss a softirq for this qdisc. we can't use ieee80211_wake_queue as
> the qdisc is not necessarily stopped, so this is a solution for it.

Ok. I don't think I've fully understood it yet, but I haven't really
looked into the qdisc stuff at all yet.

> this is a use of an existing mechanism, I wouldn't like another queue
> just for this purpose, though a misuse, i agree. i can either stay
> with this code, use cb for this purpose, or create a new struct in
> mac80211.h, somehting like ieee80211_ra_tid and pass it. what do you
> think os the best?

I think we should use something other than an skb queue here with some
other struct, that other struct would be in skb->cb for the existing skb
cases and would just sit in some new struct ieee80211_ra_tid too for
this case. It's a bit of rework here but I think it'd help the code to
be more readable. Not sure if we should also do it for the unreliable
queue.

> > > +     /* check if the TID waits for addBA response */
> > > +     spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
> > > +     if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
> > > +             spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
> > > +             *state = HT_AGG_STATE_IDLE;
> > > +             printk(KERN_DEBUG "timer expired on tid %d but we are not "
> > > +                             "expecting addBA response there", tid);
> > > +             goto timer_expired_exit;
> > > +     }
> >
> > Couldn't this also happen when the response was just received while the
> > timer expires but the response grabs the ampdu_tx spinlock before the
> > timer does? Seems that should be in a comment and not worry about a
> > printk.
> 
> the condition is on whether addBA request was sent or not for this
> address and TID, not on the addBA request.

Oh, hmm, ok. But that actually leads me to another problem,
HT_ADDBA_REQUESTED_MSK is never cleared anywhere.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC PATCH 03/10] mac80211: A-MPDU Tx adding basic functionality

[Ron Rindjunsky]

> > > > +EXPORT_SYMBOL(ieee80211_start_tx_ba_session);
> > >
> > this is the trigger to aggregation, but a load aware entity should
> > call it. in my next series of patches i'll work on this issue, but
> > basically i would like to give the option to start/stop aggregations
> > to other modules (e.g. rate scaling module)
>
> Ah. Let only export it then though, please put the export into that
> patch series. Maybe a rate control algorithm should indicate this in
> some other way via the callbacks anyway?

ok, i'll check several ways for the A-MPDU trigger and edit the
patches accordingly

> > > > +     /* Tell the driver to stop its HW queue */
> > > > +     if (local->ops->ampdu_action)
> > > > +             ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_TX_STOP,
> > > > +                                             ra, tid, NULL);
> > >
> > > Does it really stop the hw queue? Isn't that more like "stop
> > > aggregation"? Does the hardware actually have queues for each of these?
> > > Hm. I'm familiar enough with this I think.
> > >
> >
> > I'll remove this comment anyhow, as it is misleading, but the basic
> > idea is what i wrote in 0/10 - first we need to drain all A-MPDUs,
> > only then we can delBA, otherwise we are confusing the receiver.
>
> Ok, I'm trying to learn 11n here too, does the hardware need extra
> queues for the A-MPDU feature?

Yes, it should have queues for A-MPDU, as regular QoS queues should
not be eliminated during A-MPDU sesion. moreover, RA/TID combination
defines uniquely A-MPDU queue, so it is up to a specific HW to decide
how to maintain the queues.

> > > > +     /* case HW denied going back to legacy */
> > > > +     if (ret) {
> > > > +             printk(KERN_ERR "Driver could not stop aggregations\n");
> > > > +             *state = HT_AGG_STATE_OPERATIONAL;
> > > > +             ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
> > > > +             goto stop_BA_exit;
> > >
> > > Is there really a use case for a driver denying this? IOW, can you think
> > > of a good reason why a driver would deny it?
> >
> > can't think about one right now, but i am not familiar with other HWs.
> > maybe a self contained HW decision based on frames load can lead to
> > that. in any case i would like to give the driver this option.
>
> Ok. Let's put
>
> WARN_ON(ret != -EBUSY)
>
> into that if (ret) { block though to force the driver to give exactly
> that error code (and document that) so that callers of
> ieee80211_stop_tx_ba_session() can discover that the driver failed.
>

ok, i'll add this check

> > > +     hdr = (struct ieee80211_hdr *) skb_put(skb,
> > > +                                     sizeof(struct ieee80211_hdr));
> > > +     memcpy(&hdr->addr1, ra, ETH_ALEN);
> > > +     hdr->frame_control = tid;
> >
> > this is a use of an existing mechanism, I wouldn't like another queue
> > just for this purpose, though a misuse, i agree. i can either stay
> > with this code, use cb for this purpose, or create a new struct in
> > mac80211.h, somehting like ieee80211_ra_tid and pass it. what do you
> > think os the best?
>
> I think we should use something other than an skb queue here with some
> other struct, that other struct would be in skb->cb for the existing skb
> cases and would just sit in some new struct ieee80211_ra_tid too for
> this case. It's a bit of rework here but I think it'd help the code to
> be more readable. Not sure if we should also do it for the unreliable
> queue.

ok, i'll add this struct (ieee80211_ra_tid), and pass it in cb. this
struct can be useful in other cases too.

>
> > > > +     /* check if the TID waits for addBA response */
> > > > +     spin_lock_bh(&sta->ampdu_mlme.ampdu_tx);
> > > > +     if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
> > > > +             spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx);
> > > > +             *state = HT_AGG_STATE_IDLE;
> > > > +             printk(KERN_DEBUG "timer expired on tid %d but we are not "
> > > > +                             "expecting addBA response there", tid);
> > > > +             goto timer_expired_exit;
> > > > +     }
> > >
> > > Couldn't this also happen when the response was just received while the
> > > timer expires but the response grabs the ampdu_tx spinlock before the
> > > timer does? Seems that should be in a comment and not worry about a
> > > printk.
> >
> > the condition is on whether addBA request was sent or not for this
> > address and TID, not on the addBA request.
>
> Oh, hmm, ok. But that actually leads me to another problem,
> HT_ADDBA_REQUESTED_MSK is never cleared anywhere.
>

to go into A-MPDU active state we need 3 conditions:
- addBA request was sent (HT_ADDBA_REQUESTED_MSK)
- addBA response arrived (HT_ADDBA_RECEIVED_MSK)
- all legacy frames drained up to ssn (HT_ADDBA_DRV_READY_MSK)

their combination gives us HT_AGG_STATE_OPERATIONAL. so when we go to
HT_AGG_STATE_IDLE automatically we reset all the above.

Re: [RFC PATCH 03/10] mac80211: A-MPDU Tx adding basic functionality

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]


> to go into A-MPDU active state we need 3 conditions:
> - addBA request was sent (HT_ADDBA_REQUESTED_MSK)
> - addBA response arrived (HT_ADDBA_RECEIVED_MSK)
> - all legacy frames drained up to ssn (HT_ADDBA_DRV_READY_MSK)
> 
> their combination gives us HT_AGG_STATE_OPERATIONAL. so when we go to
> HT_AGG_STATE_IDLE automatically we reset all the above.

Ah, I see, sorry. I was looking for a flag unsetting.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[RFC PATCH 04/10] mac80211: A-MPDU Tx MLME data initialization

[Ron Rindjunsky]

This patch initialize A-MPDU MLME data for Tx sessions.

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 net/mac80211/sta_info.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index c964082..7c65849 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -117,8 +117,10 @@ static void sta_info_release(struct kref *kref)
 	while ((skb = skb_dequeue(&sta->tx_filtered)) != NULL) {
 		dev_kfree_skb_any(skb);
 	}
-	for (i = 0; i <  STA_TID_NUM; i++)
+	for (i = 0; i <  STA_TID_NUM; i++) {
 		del_timer_sync(&sta->ampdu_mlme.tid_rx[i].session_timer);
+		del_timer_sync(&sta->ampdu_mlme.tid_tx[i].addba_resp_timer);
+	}
 	rate_control_free_sta(sta->rate_ctrl, sta->rate_ctrl_priv);
 	rate_control_put(sta->rate_ctrl);
 	kfree(sta);
@@ -157,17 +159,26 @@ struct sta_info * sta_info_add(struct ieee80211_local *local,
 	sta->local = local;
 	sta->dev = dev;
 	spin_lock_init(&sta->ampdu_mlme.ampdu_rx);
+	spin_lock_init(&sta->ampdu_mlme.ampdu_tx);
 	for (i = 0; i < STA_TID_NUM; i++) {
 		/* timer_to_tid must be initialized with identity mapping to
 		 * enable session_timer's data differentiation. refer to
 		 * sta_rx_agg_session_timer_expired for useage */
 		sta->timer_to_tid[i] = i;
+		/* tid to tx queue: initialize according to HW (0 is valid) */
+		sta->tid_to_tx_q[i] = local->hw.queues;
 		/* rx timers */
 		sta->ampdu_mlme.tid_rx[i].session_timer.function =
 			sta_rx_agg_session_timer_expired;
 		sta->ampdu_mlme.tid_rx[i].session_timer.data =
 			(unsigned long)&sta->timer_to_tid[i];
 		init_timer(&sta->ampdu_mlme.tid_rx[i].session_timer);
+		/* tx timers */
+		sta->ampdu_mlme.tid_tx[i].addba_resp_timer.function =
+			sta_addba_resp_timer_expired;
+		sta->ampdu_mlme.tid_tx[i].addba_resp_timer.data =
+			(unsigned long)&sta->timer_to_tid[i];
+		init_timer(&sta->ampdu_mlme.tid_tx[i].addba_resp_timer);
 	}
 	skb_queue_head_init(&sta->ps_tx_buf);
 	skb_queue_head_init(&sta->tx_filtered);
-- 
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[RFC PATCH 05/10] mac80211: A-MPDU add debugfs support

[Ron Rindjunsky]

This patch adds A-MPDU status report per STA to the debugfs.
Remark:
=======
The option to activate A-MPDU through debugfs is temporary, only
until A-MPDU manager will be added to the code, and for RFC purposes.

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 net/mac80211/debugfs_sta.c |  116 ++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/sta_info.h    |    1 +
 2 files changed, 117 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 8f5944c..0878146 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -55,6 +55,13 @@ static const struct file_operations sta_ ##name## _ops = {		\
 	.open = mac80211_open_file_generic,				\
 }
 
+#define STA_OPS_WR(name)						\
+static const struct file_operations sta_ ##name## _ops = {		\
+	.read = sta_##name##_read,					\
+	.write = sta_##name##_write,					\
+	.open = mac80211_open_file_generic,				\
+}
+
 #define STA_FILE(name, field, format)					\
 		STA_READ_##format(name, field)				\
 		STA_OPS(name)
@@ -191,6 +198,113 @@ static ssize_t sta_wme_tx_queue_read(struct file *file, char __user *userbuf,
 STA_OPS(wme_tx_queue);
 #endif
 
+static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
+					size_t count, loff_t *ppos)
+{
+	char buf[768], *p = buf;
+	int i;
+	struct sta_info *sta = file->private_data;
+	p += scnprintf(p, sizeof(buf)+buf-p, "Agg state for STA is:\n");
+	p += scnprintf(p, sizeof(buf)+buf-p, " STA next dialog_token is %d \n "
+			"TIDs info is: \n TID :",
+			(sta->ampdu_mlme.dialog_token_allocator + 1));
+	for (i = 0; i < STA_TID_NUM; i++)
+		p += scnprintf(p, sizeof(buf)+buf-p, "%5d", i);
+
+	p += scnprintf(p, sizeof(buf)+buf-p, "\n RX  :");
+	for (i = 0; i < STA_TID_NUM; i++)
+		p += scnprintf(p, sizeof(buf)+buf-p, "%5d",
+	sta->ampdu_mlme.tid_rx[i].state);
+
+	p += scnprintf(p, sizeof(buf)+buf-p, "\n DTKN:");
+	for (i = 0; i < STA_TID_NUM; i++)
+		p += scnprintf(p, sizeof(buf)+buf-p, "%5d",
+			sta->ampdu_mlme.tid_rx[i].dialog_token);
+
+	p += scnprintf(p, sizeof(buf)+buf-p, "\n TX  :");
+	for (i = 0; i < STA_TID_NUM; i++)
+		p += scnprintf(p, sizeof(buf)+buf-p, "%5d",
+			sta->ampdu_mlme.tid_tx[i].state);
+
+	p += scnprintf(p, sizeof(buf)+buf-p, "\n DTKN:");
+	for (i = 0; i < STA_TID_NUM; i++)
+		p += scnprintf(p, sizeof(buf)+buf-p, "%5d",
+			sta->ampdu_mlme.tid_tx[i].dialog_token);
+
+	p += scnprintf(p, sizeof(buf)+buf-p, "\n SSN :");
+	for (i = 0; i < STA_TID_NUM; i++)
+		p += scnprintf(p, sizeof(buf)+buf-p, "%5d",
+			sta->ampdu_mlme.tid_tx[i].ssn);
+
+	p += scnprintf(p, sizeof(buf)+buf-p, "\n");
+
+	return simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
+}
+
+static ssize_t sta_agg_status_write(struct file *file,
+		const char __user *user_buf, size_t count, loff_t *ppos)
+{
+	struct sta_info *sta = file->private_data;
+	struct net_device *dev = sta->dev;
+	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	struct ieee80211_hw *hw = &local->hw;
+	u8 *da = sta->addr;
+	static int tid_static_tx[16] = {0, 0, 0, 0, 0, 0, 0, 0,
+					0, 0, 0, 0, 0, 0, 0, 0};
+	static int tid_static_rx[16] = {1, 1, 1, 1, 1, 1, 1, 1,
+					1, 1, 1, 1, 1, 1, 1, 1};
+	char *endp;
+	char buf[32];
+	int buf_size, rs;
+	unsigned int tid_num;
+	char state[4];
+
+	memset(buf, 0x00, sizeof(buf));
+	buf_size = min(count, (sizeof(buf)-1));
+	if (copy_from_user(buf, user_buf, buf_size))
+		return -EFAULT;
+
+	tid_num = simple_strtoul(buf, &endp, 0);
+	if (endp == buf)
+		return -EINVAL;
+
+	if ((tid_num >= 100) && (tid_num <= 115)) {
+		/* toggle Rx aggregation command */
+		tid_num = tid_num - 100;
+		if (tid_static_rx[tid_num] == 1) {
+			strcpy(state, "off ");
+			ieee80211_sta_stop_rx_ba_session(dev, da, tid_num, 0,
+					WLAN_REASON_QSTA_REQUIRE_SETUP);
+			sta->ampdu_mlme.tid_rx[tid_num].buf_size = 0xFF;
+			tid_static_rx[tid_num] = 0;
+		} else {
+			strcpy(state, "on ");
+			sta->ampdu_mlme.tid_rx[tid_num].buf_size = 0x00;
+			tid_static_rx[tid_num] = 1;
+		}
+		printk(KERN_ERR "%s tried switching tid=%u %s\n", __func__,
+				tid_num, state);
+	} else if ((tid_num >= 0) && (tid_num <= 15)) {
+		/* toggle Tx aggregation command */
+		if (tid_static_tx[tid_num] == 0) {
+			strcpy(state, "on ");
+			rs =  ieee80211_start_tx_ba_session(hw, da, tid_num);
+			if (rs == 0)
+				tid_static_tx[tid_num] = 1;
+		} else {
+			strcpy(state, "off");
+			rs =  ieee80211_stop_tx_ba_session(hw, da, tid_num, 1);
+			if (rs == 0)
+				tid_static_tx[tid_num] = 0;
+		}
+		printk(KERN_ERR "%s tried switching tid=%u %s, return=%d\n",
+				__func__, tid_num, state, rs);
+	}
+
+	return count;
+}
+STA_OPS_WR(agg_status);
+
 #define DEBUGFS_ADD(name) \
 	sta->debugfs.name = debugfs_create_file(#name, 0444, \
 		sta->debugfs.dir, sta, &sta_ ##name## _ops);
@@ -224,6 +338,7 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
 	DEBUGFS_ADD(wme_rx_queue);
 	DEBUGFS_ADD(wme_tx_queue);
 #endif
+	DEBUGFS_ADD(agg_status);
 }
 
 void ieee80211_sta_debugfs_remove(struct sta_info *sta)
@@ -238,6 +353,7 @@ void ieee80211_sta_debugfs_remove(struct sta_info *sta)
 	DEBUGFS_DEL(wme_rx_queue);
 	DEBUGFS_DEL(wme_tx_queue);
 #endif
+	DEBUGFS_DEL(agg_status);
 
 	debugfs_remove(sta->debugfs.dir);
 	sta->debugfs.dir = NULL;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 48f25f2..1e4410f 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -190,6 +190,7 @@ struct sta_info {
 		struct dentry *wme_rx_queue;
 		struct dentry *wme_tx_queue;
 #endif
+		struct dentry *agg_status;
 	} debugfs;
 #endif
 };
-- 
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[RFC PATCH 06/10] mac80211: A-MPDU Tx adding qdisc support

[Ron Rindjunsky]

This patch allows qdisc support in A-MPDU Tx. This is not a complete
solution, but a general direction to handle QoS <-> TID switches

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 include/net/mac80211.h     |    8 ++-
 net/mac80211/ieee80211_i.h |    5 +-
 net/mac80211/tx.c          |    2 +
 net/mac80211/wme.c         |  134 ++++++++++++++++++++++++++++++++++++++++---
 net/mac80211/wme.h         |   23 +++++++-
 5 files changed, 156 insertions(+), 16 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 3151fcf..335f506 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -246,6 +246,7 @@ struct ieee80211_tx_queue_stats_data {
  * @IEEE80211_TX_QUEUE_AFTER_BEACON: transmit queue for frames to be
  *	sent after a beacon
  * @IEEE80211_TX_QUEUE_BEACON: transmit queue for beacon frames
+ * @NUM_TX_DATA_QUEUES_AMPDU: adding more queues for A-MPDU
  */
 enum ieee80211_tx_queue {
 	IEEE80211_TX_QUEUE_DATA0,
@@ -261,11 +262,12 @@ enum ieee80211_tx_queue {
  * this struct need to have fixed values. As soon as it is removed, we can
  * fix these entries. */
 	IEEE80211_TX_QUEUE_AFTER_BEACON = 6,
-	IEEE80211_TX_QUEUE_BEACON = 7
+	IEEE80211_TX_QUEUE_BEACON = 7,
+	NUM_TX_DATA_QUEUES_AMPDU = 16
 };
 
 struct ieee80211_tx_queue_stats {
-	struct ieee80211_tx_queue_stats_data data[NUM_TX_DATA_QUEUES];
+	struct ieee80211_tx_queue_stats_data data[NUM_TX_DATA_QUEUES_AMPDU];
 };
 
 struct ieee80211_low_level_stats {
@@ -311,6 +313,8 @@ struct ieee80211_tx_control {
 #define IEEE80211_TXCTL_EAPOL_FRAME	(1<<11) /* internal to mac80211 */
 #define IEEE80211_TXCTL_SEND_AFTER_DTIM	(1<<12) /* send this frame after DTIM
 						 * beacon */
+#define IEEE80211_TXCTL_AMPDU		(1<<13) /* this frame should be sent
+						 * as part of an A-MPDU */
 	u32 flags;			       /* tx control flags defined
 						* above */
 	u8 key_idx;		/* keyidx from hw->set_key(), undefined if
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index b8b246c..9220295 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -165,6 +165,7 @@ struct ieee80211_txrx_data {
 #define IEEE80211_TXPD_DO_NOT_ENCRYPT	BIT(1)
 #define IEEE80211_TXPD_REQUEUE		BIT(2)
 #define IEEE80211_TXPD_EAPOL_FRAME	BIT(3)
+#define IEEE80211_TXPD_AMPDU		BIT(4)
 /* Stored in sk_buff->cb */
 struct ieee80211_tx_packet_data {
 	int ifindex;
@@ -448,8 +449,8 @@ struct ieee80211_local {
 	struct sta_info *sta_hash[STA_HASH_SIZE];
 	struct timer_list sta_cleanup;
 
-	unsigned long state[NUM_TX_DATA_QUEUES];
-	struct ieee80211_tx_stored_packet pending_packet[NUM_TX_DATA_QUEUES];
+	unsigned long state[NUM_TX_DATA_QUEUES_AMPDU];
+	struct ieee80211_tx_stored_packet pending_packet[NUM_TX_DATA_QUEUES_AMPDU];
 	struct tasklet_struct tx_pending_tasklet;
 
 	/* number of interfaces with corresponding IFF_ flags */
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index f079676..cb5b582 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1262,6 +1262,8 @@ int ieee80211_master_start_xmit(struct sk_buff *skb,
 		control.flags |= IEEE80211_TXCTL_REQUEUE;
 	if (pkt_data->flags & IEEE80211_TXPD_EAPOL_FRAME)
 		control.flags |= IEEE80211_TXCTL_EAPOL_FRAME;
+	if (pkt_data->flags & IEEE80211_TXPD_AMPDU)
+		control.flags |= IEEE80211_TXCTL_AMPDU;
 	control.queue = pkt_data->queue;
 
 	ret = ieee80211_tx(odev, skb, &control);
diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index 5b8a157..b9801f5 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -19,10 +19,13 @@
 #include "wme.h"
 
 /* maximum number of hardware queues we support. */
-#define TC_80211_MAX_QUEUES 8
+#define TC_80211_MAX_QUEUES 16
+
+const int ieee802_1d_to_ac[8] = { 2, 3, 3, 2, 1, 1, 0, 0 };
 
 struct ieee80211_sched_data
 {
+	unsigned long qdisc_pool;
 	struct tcf_proto *filter_list;
 	struct Qdisc *queues[TC_80211_MAX_QUEUES];
 	struct sk_buff_head requeued[TC_80211_MAX_QUEUES];
@@ -97,7 +100,6 @@ static inline int classify80211(struct sk_buff *skb, struct Qdisc *qd)
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	unsigned short fc = le16_to_cpu(hdr->frame_control);
 	int qos;
-	const int ieee802_1d_to_ac[8] = { 2, 3, 3, 2, 1, 1, 0, 0 };
 
 	/* see if frame is data or non data frame */
 	if (unlikely((fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA)) {
@@ -145,9 +147,25 @@ static int wme_qdiscop_enqueue(struct sk_buff *skb, struct Qdisc* qd)
 	unsigned short fc = le16_to_cpu(hdr->frame_control);
 	struct Qdisc *qdisc;
 	int err, queue;
+	struct sta_info *sta;
+	u8 tid;
 
 	if (pkt_data->flags & IEEE80211_TXPD_REQUEUE) {
-		skb_queue_tail(&q->requeued[pkt_data->queue], skb);
+		queue = pkt_data->queue;
+		sta = sta_info_get(local, hdr->addr1);
+		tid = skb->priority & QOS_CONTROL_TAG1D_MASK;
+		if (sta) {
+			int ampdu_queue = sta->tid_to_tx_q[tid];
+			if ((ampdu_queue < local->hw.queues) &&
+			    test_bit(ampdu_queue, &q->qdisc_pool)) {
+				queue = ampdu_queue;
+				pkt_data->flags |= IEEE80211_TXPD_AMPDU;
+			} else {
+				pkt_data->flags &= ~IEEE80211_TXPD_AMPDU;
+			}
+			sta_info_put(sta);
+		}
+		skb_queue_tail(&q->requeued[queue], skb);
 		qd->q.qlen++;
 		return 0;
 	}
@@ -158,14 +176,28 @@ static int wme_qdiscop_enqueue(struct sk_buff *skb, struct Qdisc* qd)
 	 */
 	if (WLAN_FC_IS_QOS_DATA(fc)) {
 		u8 *p = skb->data + ieee80211_get_hdrlen(fc) - 2;
-		u8 qos_hdr = skb->priority & QOS_CONTROL_TAG1D_MASK;
+		u8 ack_policy = 0;
+		tid = skb->priority & QOS_CONTROL_TAG1D_MASK;
 		if (local->wifi_wme_noack_test)
-			qos_hdr |= QOS_CONTROL_ACK_POLICY_NOACK <<
+			ack_policy |= QOS_CONTROL_ACK_POLICY_NOACK <<
 					QOS_CONTROL_ACK_POLICY_SHIFT;
 		/* qos header is 2 bytes, second reserved */
-		*p = qos_hdr;
+		*p = ack_policy | tid;
 		p++;
 		*p = 0;
+
+		sta = sta_info_get(local, hdr->addr1);
+		if (sta) {
+			int ampdu_queue = sta->tid_to_tx_q[tid];
+			if ((ampdu_queue < local->hw.queues) &&
+				test_bit(ampdu_queue, &q->qdisc_pool)) {
+				queue = ampdu_queue;
+				pkt_data->flags |= IEEE80211_TXPD_AMPDU;
+			} else {
+				pkt_data->flags &= ~IEEE80211_TXPD_AMPDU;
+			}
+			sta_info_put(sta);
+		}
 	}
 
 	if (unlikely(queue >= local->hw.queues)) {
@@ -183,6 +215,7 @@ static int wme_qdiscop_enqueue(struct sk_buff *skb, struct Qdisc* qd)
 			kfree_skb(skb);
 			err = NET_XMIT_DROP;
 	} else {
+		tid = skb->priority & QOS_CONTROL_TAG1D_MASK;
 		pkt_data->queue = (unsigned int) queue;
 		qdisc = q->queues[queue];
 		err = qdisc->enqueue(skb, qdisc);
@@ -234,10 +267,11 @@ static struct sk_buff *wme_qdiscop_dequeue(struct Qdisc* qd)
 	/* check all the h/w queues in numeric/priority order */
 	for (queue = 0; queue < hw->queues; queue++) {
 		/* see if there is room in this hardware queue */
-		if (test_bit(IEEE80211_LINK_STATE_XOFF,
-			     &local->state[queue]) ||
-		    test_bit(IEEE80211_LINK_STATE_PENDING,
-			     &local->state[queue]))
+		if ((test_bit(IEEE80211_LINK_STATE_XOFF,
+				&local->state[queue])) ||
+		    (test_bit(IEEE80211_LINK_STATE_PENDING,
+				&local->state[queue])) ||
+			 (!test_bit(queue, &q->qdisc_pool)))
 			continue;
 
 		/* there is space - try and get a frame */
@@ -359,6 +393,9 @@ static int wme_qdiscop_init(struct Qdisc *qd, struct rtattr *opt)
 		}
 	}
 
+	for (i = 0; i < min(4, queues); i++)
+		set_bit(i, &q->qdisc_pool);
+
 	return err;
 }
 
@@ -604,3 +641,80 @@ void ieee80211_wme_unregister(void)
 {
 	unregister_qdisc(&wme_qdisc_ops);
 }
+
+int ieee80211_ht_agg_queue_add(struct ieee80211_local *local,
+			struct sta_info *sta, u16 tid)
+{
+	int i;
+	struct ieee80211_sched_data *q =
+			qdisc_priv(local->mdev->qdisc_sleeping);
+	DECLARE_MAC_BUF(mac);
+
+	/* prepare the filter and save it for the SW queue
+	 * matching the recieved HW queue */
+
+	/* try to get a Qdisc from a pool */
+	for (i = 7; i < local->hw.queues; i++)
+		if (!test_and_set_bit(i, &q->qdisc_pool)) {
+			ieee80211_stop_queue(local_to_hw(local), i);
+			sta->tid_to_tx_q[tid] = i;
+
+			/* IF there are already pending packets
+			 * on this tid first we need to drain them
+			 * on the previous queue
+			 * since HT is strict in order */
+#ifdef CONFIG_MAC80211_HT_DEBUG
+			if (net_ratelimit())
+				printk(KERN_DEBUG "allocated aggregation queue"
+					" %d tid %d addr %s pool=0x%lX\n",
+					i, tid, print_mac(mac, sta->addr),
+					q->qdisc_pool);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+			return 0;
+		}
+
+	return -EAGAIN;
+}
+
+/**
+ * the caller needs to hold local->mdev->queue_lock
+ */
+void ieee80211_ht_agg_queue_remove(struct ieee80211_local *local,
+				   struct sta_info *sta, u16 tid,
+				   u8 requeue)
+{
+	struct ieee80211_sched_data *q =
+		qdisc_priv(local->mdev->qdisc_sleeping);
+	int agg_queue = sta->tid_to_tx_q[tid];
+
+	/* return the qdisc to the pool */
+	clear_bit(agg_queue, &q->qdisc_pool);
+	sta->tid_to_tx_q[tid] = local->hw.queues;
+
+	if (requeue)
+		ieee80211_requeue(local, agg_queue);
+	else
+		q->queues[agg_queue]->ops->reset(q->queues[agg_queue]);
+}
+
+void ieee80211_requeue(struct ieee80211_local *local, int queue)
+{
+	struct Qdisc *root_qd = local->mdev->qdisc_sleeping;
+	struct ieee80211_sched_data *q = qdisc_priv(root_qd);
+	struct Qdisc *qdisc = q->queues[queue];
+	struct sk_buff *skb = NULL;
+	u32 len = qdisc->q.qlen;
+
+	if (!qdisc || !qdisc->dequeue)
+		return;
+
+	printk(KERN_DEBUG "requeue: qlen = %d\n", qdisc->q.qlen);
+	for (len = qdisc->q.qlen; len > 0; len--) {
+		skb = qdisc->dequeue(qdisc);
+		root_qd->q.qlen--;
+		/* packet will be classified again and */
+		/* skb->packet_data->queue will be overridden if needed */
+		if (skb)
+			wme_qdiscop_enqueue(skb, root_qd);
+	}
+}
diff --git a/net/mac80211/wme.h b/net/mac80211/wme.h
index 76c713a..fcc6b05 100644
--- a/net/mac80211/wme.h
+++ b/net/mac80211/wme.h
@@ -24,6 +24,8 @@
 
 #define QOS_CONTROL_TAG1D_MASK 0x07
 
+extern const int ieee802_1d_to_ac[8];
+
 static inline int WLAN_FC_IS_QOS_DATA(u16 fc)
 {
 	return (fc & 0x8C) == 0x88;
@@ -32,7 +34,12 @@ static inline int WLAN_FC_IS_QOS_DATA(u16 fc)
 #ifdef CONFIG_NET_SCHED
 void ieee80211_install_qdisc(struct net_device *dev);
 int ieee80211_qdisc_installed(struct net_device *dev);
-
+int ieee80211_ht_agg_queue_add(struct ieee80211_local *local,
+			       struct sta_info *sta, u16 tid);
+void ieee80211_ht_agg_queue_remove(struct ieee80211_local *local,
+				   struct sta_info *sta, u16 tid,
+				   u8 requeue);
+void ieee80211_requeue(struct ieee80211_local *local, int queue);
 int ieee80211_wme_register(void);
 void ieee80211_wme_unregister(void);
 #else
@@ -43,7 +50,19 @@ static inline int ieee80211_qdisc_installed(struct net_device *dev)
 {
 	return 0;
 }
-
+static inline int ieee80211_ht_agg_queue_add(struct ieee80211_local *local,
+					     struct sta_info *sta, u16 tid)
+{
+	return -EAGAIN;
+}
+static inline void ieee80211_ht_agg_queue_remove(struct ieee80211_local *local,
+						 struct sta_info *sta, u16 tid,
+						 u8 requeue)
+{
+}
+static inline void ieee80211_requeue(struct ieee80211_local *local, int queue)
+{
+}
 static inline int ieee80211_wme_register(void)
 {
 	return 0;
-- 
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [RFC PATCH 06/10] mac80211: A-MPDU Tx adding qdisc support

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]


On Thu, 2008-01-10 at 19:22 +0200, Ron Rindjunsky wrote:
> This patch allows qdisc support in A-MPDU Tx. This is not a complete
> solution, but a general direction to handle QoS <-> TID switches

Ok, I'll have to think a lot more about this, my train is about to
arrive :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC PATCH 06/10] mac80211: A-MPDU Tx adding qdisc support

[Patrick McHardy]

Ron Rindjunsky wrote:
> This patch allows qdisc support in A-MPDU Tx. This is not a complete
> solution, but a general direction to handle QoS <-> TID switches
>
> Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
> ---

> --- a/net/mac80211/wme.c
> +++ b/net/mac80211/wme.c
> @@ -19,10 +19,13 @@
>  #include "wme.h"
>  
>  /* maximum number of hardware queues we support. */
> -#define TC_80211_MAX_QUEUES 8
> +#define TC_80211_MAX_QUEUES 16

Is there a reason to add new features to this instead of using
the generic multiqueue support, moving classification to a
classifier and killing wme?

Re: [RFC PATCH 06/10] mac80211: A-MPDU Tx adding qdisc support

[Tomas Winkler]

On Jan 27, 2008 7:55 PM, Patrick McHardy <kaber@trash.net> wrote:
> Ron Rindjunsky wrote:
> > This patch allows qdisc support in A-MPDU Tx. This is not a complete
> > solution, but a general direction to handle QoS <-> TID switches
> >
> > Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
> > ---
>
> > --- a/net/mac80211/wme.c
> > +++ b/net/mac80211/wme.c
> > @@ -19,10 +19,13 @@
> >  #include "wme.h"
> >
> >  /* maximum number of hardware queues we support. */
> > -#define TC_80211_MAX_QUEUES 8
> > +#define TC_80211_MAX_QUEUES 16
>
> Is there a reason to add new features to this instead of using
> the generic multiqueue support, moving classification to a
> classifier and killing wme?
>
Multiqueue is where we are heading to. We have developed this solution
before multiqueue was accepted and it worked well for few months
already so it can provide enough usability for 11n users for now.
We still have too many opens to close so we prefer to stage them
rather then choke on them. It will take few more weeks till we start
to work on it unless someone else take the lead of course.
Thanks
Tomas

>
> -
> 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
>

Re: [RFC PATCH 06/10] mac80211: A-MPDU Tx adding qdisc support

[Patrick McHardy]

Tomas Winkler wrote:
> On Jan 27, 2008 7:55 PM, Patrick McHardy <kaber@trash.net> wrote:
>
>> Is there a reason to add new features to this instead of using
>> the generic multiqueue support, moving classification to a
>> classifier and killing wme?
>>
> Multiqueue is where we are heading to. We have developed this solution
> before multiqueue was accepted and it worked well for few months
> already so it can provide enough usability for 11n users for now.
> We still have too many opens to close so we prefer to stage them
> rather then choke on them. It will take few more weeks till we start
> to work on it unless someone else take the lead of course.


Thanks for the explanation.

[RFC PATCH 07/10] mac80211: A-MPDU Tx change tx_status to support Block Ack data

[Ron Rindjunsky]

This patch adds fields to ieee80211_tx_status in order to allow block ack
information exchange between low-level driver,mac80211 and rate scaling
module.

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 include/net/mac80211.h |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 335f506..dd9a761 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -408,12 +408,14 @@ struct ieee80211_rx_status {
  *
  * @IEEE80211_TX_STATUS_TX_FILTERED: The frame was not transmitted
  *	because the destination STA was in powersave mode.
- *
  * @IEEE80211_TX_STATUS_ACK: Frame was acknowledged
+ * @IEEE80211_TX_STATUS_AMPDU: The frame was aggregated, so status
+ * 	is for the whole aggregation.
  */
 enum ieee80211_tx_status_flags {
 	IEEE80211_TX_STATUS_TX_FILTERED	= 1<<0,
 	IEEE80211_TX_STATUS_ACK		= 1<<1,
+	IEEE80211_TX_STATUS_AMPDU	= 1<<2,
 };
 
 /**
@@ -424,24 +426,25 @@ enum ieee80211_tx_status_flags {
  *
  * @control: a copy of the &struct ieee80211_tx_control passed to the driver
  *	in the tx() callback.
- *
  * @flags: transmit status flags, defined above
- *
- * @ack_signal: signal strength of the ACK frame
- *
+ * @retry_count: number of retries
  * @excessive_retries: set to 1 if the frame was retried many times
  *	but not acknowledged
- *
- * @retry_count: number of retries
- *
+ * @ampdu_ack_len: number of aggregated frames.
+ * 	relevant only if IEEE80211_TX_STATUS_AMPDU was set.
+ * @ampdu_ack_map: block ack bit map for the aggregation.
+ * 	relevant only if IEEE80211_TX_STATUS_AMPDU was set.
+ * @ack_signal: signal strength of the ACK frame
  * @queue_length: ?? REMOVE
  * @queue_number: ?? REMOVE
  */
 struct ieee80211_tx_status {
 	struct ieee80211_tx_control control;
 	u8 flags;
-	bool excessive_retries;
 	u8 retry_count;
+	bool excessive_retries;
+	u8 ampdu_ack_len;
+	u64 ampdu_ack_map;
 	int ack_signal;
 	int queue_length;
 	int queue_number;
-- 
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [RFC PATCH 07/10] mac80211: A-MPDU Tx change tx_status to support Block Ack data

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]


> + * @ampdu_ack_len: number of aggregated frames.
> + * 	relevant only if IEEE80211_TX_STATUS_AMPDU was set.
> + * @ampdu_ack_map: block ack bit map for the aggregation.
> + * 	relevant only if IEEE80211_TX_STATUS_AMPDU was set.

Can you explain what that bitmap contains?

Broadcom hardware is able to give intermediate status notifications,
would there be any use in those?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC PATCH 07/10] mac80211: A-MPDU Tx change tx_status to support Block Ack data

[Tomas Winkler]

On Jan 11, 2008 6:56 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> > + * @ampdu_ack_len: number of aggregated frames.
> > + *   relevant only if IEEE80211_TX_STATUS_AMPDU was set.
> > + * @ampdu_ack_map: block ack bit map for the aggregation.
> > + *   relevant only if IEEE80211_TX_STATUS_AMPDU was set.
>
> Can you explain what that bitmap contains?
>

This actually represents Block Ack frame. This is bitmap of received
and failed frames in the aggregation.
Rate scale algorithm should not be concern with parsing BACK frame so
the info is summarized to something useful to rate scale algorithm
at least to what we believe is. We don't have much information as
other vendor view this.
The bitmap is useful to keep statistic window accurate.
All the frames are sent in same rate (currently we didn't see other
implementation)

> Broadcom hardware is able to give intermediate status notifications,
> would there be any use in those?

Can you explain?

Tomas
> johannes
>

RE: [RFC PATCH 07/10] mac80211: A-MPDU Tx change tx_status to support Block Ack data

[Rindjunsky, Ron]

>>> Broadcom hardware is able to give intermediate status notifications,
>>> would there be any use in those?
>>
>> Can you explain?

> Actually, I don't really know what the microcode there provides. I
guess
> it tells you something like "I sent this frame, no TX errors occurred"
or
> so...

> Johannes

>From what you wrote, I would also guess that these notifications
indicate that the Tx itself was OK, as we have the same mechanism. Yet,
Block Ack handling is a must if we want good rate scaling during A-MPDU,
so the driver will have to communicate this data to the rate scaling
module with no connection to HW conformations.

Ron
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [RFC PATCH 07/10] mac80211: A-MPDU Tx change tx_status to support Block Ack data

[Tomas Winkler]

> > This actually represents Block Ack frame. This is bitmap of received
> > and failed frames in the aggregation.
>
> Ok so each bit just represents "this frame of the block ack was received"?

I would rather word it as  'the 'frame of this aggregation was received/acked.'

> Might be worth elaborating the comment a bit.

Absolutely

> >> Broadcom hardware is able to give intermediate status notifications,
> >> would there be any use in those?
> >
> > Can you explain?
>
> Actually, I don't really know what the microcode there provides. I guess
> it tells you something like "I sent this frame, no TX errors occurred" or
> so...

Is this response to aggregation or just general response to TX?

Thanks
Tomas

[RFC PATCH 08/10] iwlwifi: A-MPDU Tx conform API to mac80211

[Ron Rindjunsky]

This patch alters the current API in order to fit the new
API mac80211 gives for A-MPDU Tx

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-4965.c     |   13 +++++++++----
 drivers/net/wireless/iwlwifi/iwl-4965.h     |    6 +-----
 drivers/net/wireless/iwlwifi/iwl4965-base.c |    4 ----
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 7aab60a..4f262a2 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -4721,7 +4721,7 @@ static void iwl4965_sta_modify_del_ba_tid(struct iwl4965_priv *priv,
 
 int iwl4965_mac_ampdu_action(struct ieee80211_hw *hw,
 			     enum ieee80211_ampdu_mlme_action action,
-			     const u8 *addr, u16 tid, u16 ssn)
+			     u8 *addr, u16 tid, u16 *ssn)
 {
 	struct iwl4965_priv *priv = hw->priv;
 	int sta_id;
@@ -4733,12 +4733,18 @@ int iwl4965_mac_ampdu_action(struct ieee80211_hw *hw,
 	switch (action) {
 	case IEEE80211_AMPDU_RX_START:
 		IWL_DEBUG_HT("start Rx\n");
-		iwl4965_sta_modify_add_ba_tid(priv, sta_id, tid, ssn);
+		iwl4965_sta_modify_add_ba_tid(priv, sta_id, tid, *ssn);
 		break;
 	case IEEE80211_AMPDU_RX_STOP:
 		IWL_DEBUG_HT("stop Rx\n");
 		iwl4965_sta_modify_del_ba_tid(priv, sta_id, tid);
 		break;
+	case IEEE80211_AMPDU_TX_START:
+		IWL_DEBUG_HT("start Tx\n");
+		return iwl4965_mac_ht_tx_agg_start(hw, addr, tid, ssn);
+	case IEEE80211_AMPDU_TX_STOP:
+		IWL_DEBUG_HT("stop Tx\n");
+		return iwl4965_mac_ht_tx_agg_stop(hw, addr, tid);
 	default:
 		IWL_DEBUG_HT("unknown\n");
 		return -EINVAL;
@@ -4837,8 +4843,7 @@ int iwl4965_mac_ht_tx_agg_start(struct ieee80211_hw *hw, u8 *da, u16 tid,
 }
 
 
-int iwl4965_mac_ht_tx_agg_stop(struct ieee80211_hw *hw, u8 *da, u16 tid,
-			   int generator)
+int iwl4965_mac_ht_tx_agg_stop(struct ieee80211_hw *hw, u8 *da, u16 tid)
 {
 
 	struct iwl4965_priv *priv = hw->priv;
diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.h b/drivers/net/wireless/iwlwifi/iwl-4965.h
index 78bc148..8e9f756 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.h
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.h
@@ -778,12 +778,8 @@ extern void iwl4965_set_ht_add_station(struct iwl4965_priv *priv, u8 index,
 				struct ieee80211_ht_info *sta_ht_inf);
 extern int iwl4965_mac_ampdu_action(struct ieee80211_hw *hw,
 				    enum ieee80211_ampdu_mlme_action action,
-				    const u8 *addr, u16 tid, u16 ssn);
+				    u8 *addr, u16 tid, u16 *ssn);
 #ifdef CONFIG_IWL4965_HT_AGG
-extern int iwl4965_mac_ht_tx_agg_start(struct ieee80211_hw *hw, u8 *da,
-				   u16 tid, u16 *start_seq_num);
-extern int iwl4965_mac_ht_tx_agg_stop(struct ieee80211_hw *hw, u8 *da,
-				  u16 tid, int generator);
 extern void iwl4965_turn_off_agg(struct iwl4965_priv *priv, u8 tid);
 extern void iwl4965_tl_get_stats(struct iwl4965_priv *priv,
 				struct ieee80211_hdr *hdr);
diff --git a/drivers/net/wireless/iwlwifi/iwl4965-base.c b/drivers/net/wireless/iwlwifi/iwl4965-base.c
index 25001df..728271e 100644
--- a/drivers/net/wireless/iwlwifi/iwl4965-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl4965-base.c
@@ -8973,10 +8973,6 @@ static struct ieee80211_ops iwl4965_hw_ops = {
 #ifdef CONFIG_IWL4965_HT
 	.conf_ht = iwl4965_mac_conf_ht,
 	.ampdu_action = iwl4965_mac_ampdu_action,
-#ifdef CONFIG_IWL4965_HT_AGG
-	.ht_tx_agg_start = iwl4965_mac_ht_tx_agg_start,
-	.ht_tx_agg_stop = iwl4965_mac_ht_tx_agg_stop,
-#endif  /* CONFIG_IWL4965_HT_AGG */
 #endif  /* CONFIG_IWL4965_HT */
 	.hw_scan = iwl4965_mac_hw_scan
 };
-- 
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[RFC PATCH 09/10] iwlwifi: A-MPDU Tx conform flows to mac80211

[Ron Rindjunsky]

This patch alters the current iwlwifi behavior to fit the flows introduced
by the mac80211, mainly queues handling and start/ stop call backs flows

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-4965-commands.h |   24 ++-
 drivers/net/wireless/iwlwifi/iwl-4965.c          |  403 +++++++++++++---------
 drivers/net/wireless/iwlwifi/iwl-4965.h          |   20 +-
 drivers/net/wireless/iwlwifi/iwl4965-base.c      |  121 ++++---
 4 files changed, 331 insertions(+), 237 deletions(-)

Index: wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965-commands.h
===================================================================
--- wl2_6_24_rc5_upstream.orig/drivers/net/wireless/iwlwifi/iwl-4965-commands.h
+++ wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965-commands.h
@@ -1300,6 +1300,25 @@ struct iwl4965_tx_resp {
 	__le32 status;	/* TX status (for aggregation status of 1st frame) */
 } __attribute__ ((packed));
 
+struct agg_tx_status {
+	__le16 status;
+	__le16 sequence;
+} __attribute__ ((packed));
+
+struct iwl4965_tx_resp_agg {
+	u8 frame_count;         /* 1 no aggregation, >1 aggregation */
+	u8 reserved1;
+	u8 failure_rts;
+	u8 failure_frame;
+	__le32 rate_n_flags;
+	__le16 wireless_media_time;
+	__le16 reserved3;
+	__le32 pa_power1;
+	__le32 pa_power2;
+	struct agg_tx_status status;    /* TX status (for aggregation status */
+					/* of 1st frame) */
+} __attribute__ ((packed));
+
 /*
  * REPLY_COMPRESSED_BA = 0xc5 (response only, not a command)
  *
@@ -1313,9 +1332,8 @@ struct iwl4965_compressed_ba_resp {
 	/* Index of recipient (BA-sending) station in uCode's station table */
 	u8 sta_id;
 	u8 tid;
-	__le16 ba_seq_ctl;
-	__le32 ba_bitmap0;
-	__le32 ba_bitmap1;
+	__le16 seq_ctl;
+	__le64 bitmap;
 	__le16 scd_flow;
 	__le16 scd_ssn;
 } __attribute__ ((packed));
Index: wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965.c
===================================================================
--- wl2_6_24_rc5_upstream.orig/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -79,6 +79,30 @@ const struct iwl4965_rate_info iwl4965_r
 	IWL_DECLARE_RATE_INFO(60, 60, 48, INV, 48, INV, 48, INV),/* 60mbps */
 };
 
+#ifdef CONFIG_IWL4965_HT
+
+static const u16 default_tid_to_tx_fifo[] = {
+	IWL_TX_FIFO_AC1,
+	IWL_TX_FIFO_AC0,
+	IWL_TX_FIFO_AC0,
+	IWL_TX_FIFO_AC1,
+	IWL_TX_FIFO_AC2,
+	IWL_TX_FIFO_AC2,
+	IWL_TX_FIFO_AC3,
+	IWL_TX_FIFO_AC3,
+	IWL_TX_FIFO_NONE,
+	IWL_TX_FIFO_NONE,
+	IWL_TX_FIFO_NONE,
+	IWL_TX_FIFO_NONE,
+	IWL_TX_FIFO_NONE,
+	IWL_TX_FIFO_NONE,
+	IWL_TX_FIFO_NONE,
+	IWL_TX_FIFO_NONE,
+	IWL_TX_FIFO_AC3
+};
+
+#endif	/*CONFIG_IWL4965_HT */
+
 static int is_fat_channel(__le32 rxon_flags)
 {
 	return (rxon_flags & RXON_FLG_CHANNEL_MODE_PURE_40_MSK) ||
@@ -4187,6 +4211,7 @@ static void iwl4965_set_tx_status(struct
 	tx_status->control.tx_rate = rate;
 }
 
+#endif/* CONFIG_IWL4965_HT_AGG */
 
 /**
  * iwl4965_sta_modify_enable_tid_tx - Enable Tx for this TID in station table
@@ -4206,7 +4231,6 @@ static void iwl4965_sta_modify_enable_ti
 	iwl4965_send_add_station(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
 }
 
-
 /**
  * iwl4965_tx_status_reply_compressed_ba - Update tx status from block-ack
  *
@@ -4220,10 +4244,11 @@ static int iwl4965_tx_status_reply_compr
 
 {
 	int i, sh, ack;
-	u16 ba_seq_ctl = le16_to_cpu(ba_resp->ba_seq_ctl);
-	u32 bitmap0, bitmap1;
-	u32 resp_bitmap0 = le32_to_cpu(ba_resp->ba_bitmap0);
-	u32 resp_bitmap1 = le32_to_cpu(ba_resp->ba_bitmap1);
+	u16 seq_ctl = le16_to_cpu(ba_resp->seq_ctl);
+	u16 scd_flow = le16_to_cpu(ba_resp->scd_flow);
+	u64 bitmap;
+	int successes = 0;
+	struct ieee80211_tx_status *tx_status;
 
 	if (unlikely(!agg->wait_for_ba))  {
 		IWL_ERROR("Received BA when not expected\n");
@@ -4232,17 +4257,15 @@ static int iwl4965_tx_status_reply_compr
 
 	/* Mark that the expected block-ack response arrived */
 	agg->wait_for_ba = 0;
-	IWL_DEBUG_TX_REPLY("BA %d %d\n", agg->start_idx, ba_resp->ba_seq_ctl);
+	IWL_DEBUG_TX_REPLY("BA %d %d\n", agg->start_idx, ba_resp->seq_ctl);
 
 	/* Calculate shift to align block-ack bits with our Tx window bits */
-	sh = agg->start_idx - SEQ_TO_INDEX(ba_seq_ctl>>4);
+	sh = agg->start_idx - SEQ_TO_INDEX(seq_ctl>>4);
 	if (sh < 0) /* tbw something is wrong with indices */
 		sh += 0x100;
 
 	/* don't use 64-bit values for now */
-	bitmap0 = resp_bitmap0 >> sh;
-	bitmap1 = resp_bitmap1 >> sh;
-	bitmap0 |= (resp_bitmap1 & ((1<<sh)|((1<<sh)-1))) << (32 - sh);
+	bitmap = le64_to_cpu(ba_resp->bitmap) >> sh;
 
 	if (agg->frame_count > (64 - sh)) {
 		IWL_DEBUG_TX_REPLY("more frames than bitmap size");
@@ -4251,26 +4274,109 @@ static int iwl4965_tx_status_reply_compr
 
 	/* check for success or failure according to the
 	 * transmitted bitmap and block-ack bitmap */
-	bitmap0 &= agg->bitmap0;
-	bitmap1 &= agg->bitmap1;
+	bitmap &= agg->bitmap;
 
 	/* For each frame attempted in aggregation,
 	 * update driver's record of tx frame's status. */
 	for (i = 0; i < agg->frame_count ; i++) {
-		int idx = (agg->start_idx + i) & 0xff;
-		ack = bitmap0 & (1 << i);
+		ack = bitmap & (1 << i);
+		successes += !!ack;
 		IWL_DEBUG_TX_REPLY("%s ON i=%d idx=%d raw=%d\n",
-			ack? "ACK":"NACK", i, idx, agg->start_idx + i);
-		iwl4965_set_tx_status(priv, agg->txq_id, idx, ack, 0,
-			agg->rate_n_flags);
+			ack? "ACK":"NACK", i, (agg->start_idx + i) & 0xff,
+			agg->start_idx + i);
+	}
+
+	tx_status = &priv->txq[scd_flow].txb[agg->start_idx].status;
+	tx_status->flags = IEEE80211_TX_STATUS_ACK;
+	tx_status->retry_count++;
+#ifdef CONFIG_IWL4965_HT_AGG
+	tx_status->flags |= IEEE80211_TX_STATUS_AGG_STATS;
+	tx_status->successes = successes;
+	tx_status->frame_count = agg->frame_count;
+#endif	/* CONFIG_IWL4965_HT_AGG */
+	tx_status->control.tx_rate = agg->rate_n_flags;
+
+	IWL_DEBUG_TX_REPLY("Bitmap %llx\n", bitmap);
 
+	return 0;
+}
+
+/**
+ * iwl4965_tx_queue_stop_scheduler - Stop queue, but keep configuration
+ */
+static void iwl4965_tx_queue_stop_scheduler(struct iwl4965_priv *priv,
+					    u16 txq_id)
+{
+	/* Simply stop the queue, but don't change any configuration;
+	 * the SCD_ACT_EN bit is the write-enable mask for the ACTIVE bit. */
+	iwl4965_write_prph(priv,
+		KDR_SCD_QUEUE_STATUS_BITS(txq_id),
+		(0 << SCD_QUEUE_STTS_REG_POS_ACTIVE)|
+		(1 << SCD_QUEUE_STTS_REG_POS_SCD_ACT_EN));
+}
+
+/**
+ * txq_id must be greater than IWL_BACK_QUEUE_FIRST_ID
+ */
+static int iwl4965_tx_queue_agg_disable(struct iwl4965_priv *priv, u16 txq_id,
+					u16 ssn_idx, u8 tx_fifo)
+{
+	if (IWL_BACK_QUEUE_FIRST_ID > txq_id) {
+		IWL_WARNING("queue number too small: %d, must be > %d\n",
+				txq_id, IWL_BACK_QUEUE_FIRST_ID);
+		return -EINVAL;
 	}
 
-	IWL_DEBUG_TX_REPLY("Bitmap %x%x\n", bitmap0, bitmap1);
+	iwl4965_tx_queue_stop_scheduler(priv, txq_id);
+
+	iwl4965_clear_bits_prph(priv, KDR_SCD_QUEUECHAIN_SEL, (1 << txq_id));
+
+	priv->txq[txq_id].q.read_ptr = (ssn_idx & 0xff);
+	priv->txq[txq_id].q.write_ptr = (ssn_idx & 0xff);
+	/* supposes that ssn_idx is valid (!= 0xFFF) */
+	iwl4965_set_wr_ptrs(priv, txq_id, ssn_idx);
+
+	iwl4965_clear_bits_prph(priv, KDR_SCD_INTERRUPT_MASK, (1 << txq_id));
+	iwl4965_txq_ctx_deactivate(priv, txq_id);
+	iwl4965_tx_queue_set_status(priv, &priv->txq[txq_id], tx_fifo, 0);
 
 	return 0;
 }
 
+int iwl4965_check_empty_hw_queue(struct iwl4965_priv *priv, int sta_id,
+					 u8 tid, int txq_id)
+{
+	struct iwl4965_queue *q = &priv->txq[txq_id].q;
+	u8 *addr = priv->stations[sta_id].sta.sta.addr;
+	struct iwl4965_tid_data *tid_data = &priv->stations[sta_id].tid[tid];
+
+	switch (priv->stations[sta_id].tid[tid].agg.state) {
+	case IWL_EMPTYING_HW_QUEUE_DELBA:
+		/* We are reclaiming the last packet of the */
+		/* aggregated HW queue */
+		if (txq_id  == tid_data->agg.txq_id &&
+		    q->read_ptr == q->write_ptr) {
+			u16 ssn = SEQ_TO_SN(tid_data->seq_number);
+			int tx_fifo = default_tid_to_tx_fifo[tid];
+			IWL_DEBUG_HT("HW queue empty: continue DELBA flow\n");
+			iwl4965_tx_queue_agg_disable(priv, txq_id,
+						     ssn, tx_fifo);
+			tid_data->agg.state = IWL_AGG_OFF;
+			ieee80211_stop_tx_ba_cb_irqsafe(priv->hw, addr, tid);
+		}
+		break;
+	case IWL_EMPTYING_HW_QUEUE_ADDBA:
+		/* We are reclaiming the last packet of the queue */
+		if (tid_data->tfds_in_queue == 0) {
+			IWL_DEBUG_HT("HW queue empty: continue ADDBA flow\n");
+			tid_data->agg.state = IWL_AGG_ON;
+			ieee80211_start_tx_ba_cb_irqsafe(priv->hw, addr, tid);
+		}
+		break;
+	}
+	return 0;
+}
+
 /**
  * iwl4965_queue_dec_wrap - Decrement queue index, wrap back to end if needed
  * @index -- current index
@@ -4295,48 +4401,43 @@ static void iwl4965_rx_reply_compressed_
 	int index;
 	struct iwl4965_tx_queue *txq = NULL;
 	struct iwl4965_ht_agg *agg;
+	DECLARE_MAC_BUF(mac);
 
 	/* "flow" corresponds to Tx queue */
-	u16 ba_resp_scd_flow = le16_to_cpu(ba_resp->scd_flow);
+	u16 scd_flow = le16_to_cpu(ba_resp->scd_flow);
 
 	/* "ssn" is start of block-ack Tx window, corresponds to index
 	 * (in Tx queue's circular buffer) of first TFD/frame in window */
 	u16 ba_resp_scd_ssn = le16_to_cpu(ba_resp->scd_ssn);
 
-	if (ba_resp_scd_flow >= ARRAY_SIZE(priv->txq)) {
+	if (scd_flow >= ARRAY_SIZE(priv->txq)) {
 		IWL_ERROR("BUG_ON scd_flow is bigger than number of queues");
 		return;
 	}
 
-	txq = &priv->txq[ba_resp_scd_flow];
+	txq = &priv->txq[scd_flow];
 	agg = &priv->stations[ba_resp->sta_id].tid[ba_resp->tid].agg;
 
 	/* Find index just before block-ack window */
 	index = iwl4965_queue_dec_wrap(ba_resp_scd_ssn & 0xff, txq->q.n_bd);
 
 	/* TODO: Need to get this copy more safely - now good for debug */
-/*
-	{
-	DECLARE_MAC_BUF(mac);
+
 	IWL_DEBUG_TX_REPLY("REPLY_COMPRESSED_BA [%d]Received from %s, "
 			   "sta_id = %d\n",
 			   agg->wait_for_ba,
 			   print_mac(mac, (u8*) &ba_resp->sta_addr_lo32),
 			   ba_resp->sta_id);
-	IWL_DEBUG_TX_REPLY("TID = %d, SeqCtl = %d, bitmap = 0x%X%X, scd_flow = "
+	IWL_DEBUG_TX_REPLY("TID = %d, SeqCtl = %d, bitmap = 0x%llx, scd_flow = "
 			   "%d, scd_ssn = %d\n",
 			   ba_resp->tid,
-			   ba_resp->ba_seq_ctl,
-			   ba_resp->ba_bitmap1,
-			   ba_resp->ba_bitmap0,
+			   ba_resp->seq_ctl,
+			   ba_resp->bitmap,
 			   ba_resp->scd_flow,
 			   ba_resp->scd_ssn);
-	IWL_DEBUG_TX_REPLY("DAT start_idx = %d, bitmap = 0x%X%X \n",
+	IWL_DEBUG_TX_REPLY("DAT start_idx = %d, bitmap = 0x%llx \n",
 			   agg->start_idx,
-			   agg->bitmap1,
-			   agg->bitmap0);
-	}
-*/
+			   agg->bitmap);
 
 	/* Update driver's record of ACK vs. not for each frame in window */
 	iwl4965_tx_status_reply_compressed_ba(priv, agg, ba_resp);
@@ -4344,23 +4445,17 @@ static void iwl4965_rx_reply_compressed_
 	/* Release all TFDs before the SSN, i.e. all TFDs in front of
 	 * block-ack window (we assume that they've been successfully
 	 * transmitted ... if not, it's too late anyway). */
-	if (txq->q.read_ptr != (ba_resp_scd_ssn & 0xff))
-		iwl4965_tx_queue_reclaim(priv, ba_resp_scd_flow, index);
-
-}
-
-
-/**
- * iwl4965_tx_queue_stop_scheduler - Stop queue, but keep configuration
- */
-static void iwl4965_tx_queue_stop_scheduler(struct iwl4965_priv *priv, u16 txq_id)
-{
-	/* Simply stop the queue, but don't change any configuration;
-	 * the SCD_ACT_EN bit is the write-enable mask for the ACTIVE bit. */
-	iwl4965_write_prph(priv,
-		KDR_SCD_QUEUE_STATUS_BITS(txq_id),
-		(0 << SCD_QUEUE_STTS_REG_POS_ACTIVE)|
-		(1 << SCD_QUEUE_STTS_REG_POS_SCD_ACT_EN));
+	if (txq->q.read_ptr != (ba_resp_scd_ssn & 0xff)) {
+		int freed = iwl4965_tx_queue_reclaim(priv, scd_flow, index);
+		priv->stations[ba_resp->sta_id].
+			tid[ba_resp->tid].tfds_in_queue -= freed;
+		if (iwl4965_queue_space(&txq->q) > txq->q.low_mark &&
+			priv->mac80211_registered &&
+			agg->state != IWL_EMPTYING_HW_QUEUE_DELBA)
+			ieee80211_wake_queue(priv->hw, scd_flow);
+		iwl4965_check_empty_hw_queue(priv, ba_resp->sta_id,
+			ba_resp->tid, scd_flow);
+	}
 }
 
 /**
@@ -4390,6 +4485,7 @@ static int iwl4965_tx_queue_set_q2ratid(
 	return 0;
 }
 
+
 /**
  * iwl4965_tx_queue_agg_enable - Set up & enable aggregation for selected queue
  *
@@ -4457,48 +4553,6 @@ static int iwl4965_tx_queue_agg_enable(s
 	return 0;
 }
 
-/**
- * txq_id must be greater than IWL_BACK_QUEUE_FIRST_ID
- */
-static int iwl4965_tx_queue_agg_disable(struct iwl4965_priv *priv, u16 txq_id,
-					u16 ssn_idx, u8 tx_fifo)
-{
-	unsigned long flags;
-	int rc;
-
-	if (IWL_BACK_QUEUE_FIRST_ID > txq_id) {
-		IWL_WARNING("queue number too small: %d, must be > %d\n",
-				txq_id, IWL_BACK_QUEUE_FIRST_ID);
-		return -EINVAL;
-	}
-
-	spin_lock_irqsave(&priv->lock, flags);
-	rc = iwl4965_grab_nic_access(priv);
-	if (rc) {
-		spin_unlock_irqrestore(&priv->lock, flags);
-		return rc;
-	}
-
-	iwl4965_tx_queue_stop_scheduler(priv, txq_id);
-
-	iwl4965_clear_bits_prph(priv, KDR_SCD_QUEUECHAIN_SEL, (1 << txq_id));
-
-	priv->txq[txq_id].q.read_ptr = (ssn_idx & 0xff);
-	priv->txq[txq_id].q.write_ptr = (ssn_idx & 0xff);
-	/* supposes that ssn_idx is valid (!= 0xFFF) */
-	iwl4965_set_wr_ptrs(priv, txq_id, ssn_idx);
-
-	iwl4965_clear_bits_prph(priv, KDR_SCD_INTERRUPT_MASK, (1 << txq_id));
-	iwl4965_txq_ctx_deactivate(priv, txq_id);
-	iwl4965_tx_queue_set_status(priv, &priv->txq[txq_id], tx_fifo, 0);
-
-	iwl4965_release_nic_access(priv);
-	spin_unlock_irqrestore(&priv->lock, flags);
-
-	return 0;
-}
-
-#endif/* CONFIG_IWL4965_HT_AGG */
 #endif /* CONFIG_IWL4965_HT */
 
 /**
@@ -4719,62 +4773,6 @@ static void iwl4965_sta_modify_del_ba_ti
 	iwl4965_send_add_station(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
 }
 
-int iwl4965_mac_ampdu_action(struct ieee80211_hw *hw,
-			     enum ieee80211_ampdu_mlme_action action,
-			     u8 *addr, u16 tid, u16 *ssn)
-{
-	struct iwl4965_priv *priv = hw->priv;
-	int sta_id;
-	DECLARE_MAC_BUF(mac);
-
-	IWL_DEBUG_HT("A-MPDU action on da=%s tid=%d ",
-			print_mac(mac, addr), tid);
-	sta_id = iwl4965_hw_find_station(priv, addr);
-	switch (action) {
-	case IEEE80211_AMPDU_RX_START:
-		IWL_DEBUG_HT("start Rx\n");
-		iwl4965_sta_modify_add_ba_tid(priv, sta_id, tid, *ssn);
-		break;
-	case IEEE80211_AMPDU_RX_STOP:
-		IWL_DEBUG_HT("stop Rx\n");
-		iwl4965_sta_modify_del_ba_tid(priv, sta_id, tid);
-		break;
-	case IEEE80211_AMPDU_TX_START:
-		IWL_DEBUG_HT("start Tx\n");
-		return iwl4965_mac_ht_tx_agg_start(hw, addr, tid, ssn);
-	case IEEE80211_AMPDU_TX_STOP:
-		IWL_DEBUG_HT("stop Tx\n");
-		return iwl4965_mac_ht_tx_agg_stop(hw, addr, tid);
-	default:
-		IWL_DEBUG_HT("unknown\n");
-		return -EINVAL;
-		break;
-	}
-	return 0;
-}
-
-#ifdef CONFIG_IWL4965_HT_AGG
-
-static const u16 default_tid_to_tx_fifo[] = {
-	IWL_TX_FIFO_AC1,
-	IWL_TX_FIFO_AC0,
-	IWL_TX_FIFO_AC0,
-	IWL_TX_FIFO_AC1,
-	IWL_TX_FIFO_AC2,
-	IWL_TX_FIFO_AC2,
-	IWL_TX_FIFO_AC3,
-	IWL_TX_FIFO_AC3,
-	IWL_TX_FIFO_NONE,
-	IWL_TX_FIFO_NONE,
-	IWL_TX_FIFO_NONE,
-	IWL_TX_FIFO_NONE,
-	IWL_TX_FIFO_NONE,
-	IWL_TX_FIFO_NONE,
-	IWL_TX_FIFO_NONE,
-	IWL_TX_FIFO_NONE,
-	IWL_TX_FIFO_AC3
-};
-
 /*
  * Find first available (lowest unused) Tx Queue, mark it "active".
  * Called only when finding queue for aggregation.
@@ -4794,54 +4792,61 @@ static int iwl4965_txq_ctx_activate_free
 int iwl4965_mac_ht_tx_agg_start(struct ieee80211_hw *hw, u8 *da, u16 tid,
 			    u16 *start_seq_num)
 {
-
 	struct iwl4965_priv *priv = hw->priv;
 	int sta_id;
 	int tx_fifo;
 	int txq_id;
 	int ssn = -1;
+	int rc = 0;
 	unsigned long flags;
 	struct iwl4965_tid_data *tid_data;
 	DECLARE_MAC_BUF(mac);
 
-	/* Determine Tx DMA/FIFO channel for this Traffic ID */
 	if (likely(tid < ARRAY_SIZE(default_tid_to_tx_fifo)))
 		tx_fifo = default_tid_to_tx_fifo[tid];
 	else
 		return -EINVAL;
 
-	IWL_WARNING("iwl-AGG iwl4965_mac_ht_tx_agg_start on da=%s"
-		    " tid=%d\n", print_mac(mac, da), tid);
+	IWL_WARNING("%s on da = %s tid = %d\n",
+			__func__, print_mac(mac, da), tid);
 
-	/* Get index into station table */
 	sta_id = iwl4965_hw_find_station(priv, da);
 	if (sta_id == IWL_INVALID_STATION)
 		return -ENXIO;
 
-	/* Find available Tx queue for aggregation */
+	if (priv->stations[sta_id].tid[tid].agg.state != IWL_AGG_OFF) {
+		IWL_ERROR("Start AGG when state is not IWL_AGG_OFF !\n");
+		return -ENXIO;
+	}
+
 	txq_id = iwl4965_txq_ctx_activate_free(priv);
 	if (txq_id == -1)
 		return -ENXIO;
 
 	spin_lock_irqsave(&priv->sta_lock, flags);
 	tid_data = &priv->stations[sta_id].tid[tid];
-
-	/* Get starting sequence number for 1st frame in block ack window.
-	 * We'll use least signif byte as 1st frame's index into Tx queue. */
 	ssn = SEQ_TO_SN(tid_data->seq_number);
 	tid_data->agg.txq_id = txq_id;
 	spin_unlock_irqrestore(&priv->sta_lock, flags);
 
 	*start_seq_num = ssn;
-
-	/* Update driver's link quality manager */
-	iwl4965_ba_status(priv, tid, BA_STATUS_ACTIVE);
-
-	/* Set up and enable aggregation for selected Tx queue and FIFO */
-	return iwl4965_tx_queue_agg_enable(priv, txq_id, tx_fifo,
+	rc = iwl4965_tx_queue_agg_enable(priv, txq_id, tx_fifo,
 					   sta_id, tid, ssn);
-}
+	if (rc)
+		return rc;
 
+	rc = 0;
+	if (tid_data->tfds_in_queue == 0) {
+		printk(KERN_ERR "HW queue is empty\n");
+		tid_data->agg.state = IWL_AGG_ON;
+		ieee80211_start_tx_ba_cb_irqsafe(hw, da, tid);
+	} else {
+		IWL_DEBUG_HT("HW queue is NOT empty: %d packets in HW queue\n",
+				tid_data->tfds_in_queue);
+		tid_data->agg.state = IWL_EMPTYING_HW_QUEUE_ADDBA;
+	}
+	return rc;
+}
 
 int iwl4965_mac_ht_tx_agg_stop(struct ieee80211_hw *hw, u8 *da, u16 tid)
 {
@@ -4849,7 +4854,8 @@ int iwl4965_mac_ht_tx_agg_stop(struct ie
 	struct iwl4965_priv *priv = hw->priv;
 	int tx_fifo_id, txq_id, sta_id, ssn = -1;
 	struct iwl4965_tid_data *tid_data;
-	int rc;
+	int rc, write_ptr, read_ptr;
+	unsigned long flags;
 	DECLARE_MAC_BUF(mac);
 
 	if (!da) {
@@ -4867,24 +4873,82 @@ int iwl4965_mac_ht_tx_agg_stop(struct ie
 	if (sta_id == IWL_INVALID_STATION)
 		return -ENXIO;
 
+	if (priv->stations[sta_id].tid[tid].agg.state != IWL_AGG_ON)
+		IWL_WARNING("Stopping AGG while state not IWL_AGG_ON\n");
+
 	tid_data = &priv->stations[sta_id].tid[tid];
 	ssn = (tid_data->seq_number & IEEE80211_SCTL_SEQ) >> 4;
 	txq_id = tid_data->agg.txq_id;
 
+	write_ptr = priv->txq[txq_id].q.write_ptr;
+	read_ptr = priv->txq[txq_id].q.read_ptr;
+
+	/* The queue is not empty */
+	if (write_ptr != read_ptr) {
+		IWL_DEBUG_HT("Stopping a non empty AGG HW QUEUE\n");
+		priv->stations[sta_id].tid[tid].agg.state =
+			IWL_EMPTYING_HW_QUEUE_DELBA;
+		return 0;
+	}
+
+	IWL_DEBUG_HT("HW queue empty\n");;
+	priv->stations[sta_id].tid[tid].agg.state = IWL_AGG_OFF;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	rc = iwl4965_grab_nic_access(priv);
+	if (rc) {
+		spin_unlock_irqrestore(&priv->lock, flags);
+		return rc;
+	}
 	rc = iwl4965_tx_queue_agg_disable(priv, txq_id, ssn, tx_fifo_id);
+	iwl4965_release_nic_access(priv);
+	spin_unlock_irqrestore(&priv->lock, flags);
 	/* FIXME: need more safe way to handle error condition */
 	if (rc)
 		return rc;
 
-	iwl4965_ba_status(priv, tid, BA_STATUS_INITIATOR_DELBA);
+	ieee80211_stop_tx_ba_cb_irqsafe(priv->hw, da, tid);
+
 	IWL_DEBUG_INFO("iwl4965_mac_ht_tx_agg_stop on da=%s tid=%d\n",
 		       print_mac(mac, da), tid);
 
 	return 0;
 }
 
+int iwl4965_mac_ampdu_action(struct ieee80211_hw *hw,
+			     enum ieee80211_ampdu_mlme_action action,
+			     u8 *addr, u16 tid, u16 *ssn)
+{
+	struct iwl4965_priv *priv = hw->priv;
+	int sta_id;
+	DECLARE_MAC_BUF(mac);
+
+	IWL_DEBUG_HT("A-MPDU action on da=%s tid=%d ",
+			print_mac(mac, addr), tid);
+	sta_id = iwl4965_hw_find_station(priv, addr);
+	switch (action) {
+	case IEEE80211_AMPDU_RX_START:
+		IWL_DEBUG_HT("start Rx\n");
+		iwl4965_sta_modify_add_ba_tid(priv, sta_id, tid, *ssn);
+		break;
+	case IEEE80211_AMPDU_RX_STOP:
+		IWL_DEBUG_HT("stop Rx\n");
+		iwl4965_sta_modify_del_ba_tid(priv, sta_id, tid);
+		break;
+	case IEEE80211_AMPDU_TX_START:
+		IWL_DEBUG_HT("start Tx\n");
+		return iwl4965_mac_ht_tx_agg_start(hw, addr, tid, ssn);
+	case IEEE80211_AMPDU_TX_STOP:
+		IWL_DEBUG_HT("stop Tx\n");
+		return iwl4965_mac_ht_tx_agg_stop(hw, addr, tid);
+	default:
+		IWL_DEBUG_HT("unknown\n");
+		return -EINVAL;
+		break;
+	}
+	return 0;
+}
 
-#endif /* CONFIG_IWL4965_HT_AGG */
 #endif /* CONFIG_IWL4965_HT */
 
 /* Set up 4965-specific Rx frame reply handlers */
@@ -4901,9 +4965,7 @@ void iwl4965_hw_rx_handler_setup(struct 
 	    iwl4965_rx_missed_beacon_notif;
 
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
 	priv->rx_handlers[REPLY_COMPRESSED_BA] = iwl4965_rx_reply_compressed_ba;
-#endif /* CONFIG_IWL4965_HT_AGG */
 #endif /* CONFIG_IWL4965_HT */
 }
 
Index: wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965.h
===================================================================
--- wl2_6_24_rc5_upstream.orig/drivers/net/wireless/iwlwifi/iwl-4965.h
+++ wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965.h
@@ -433,7 +433,6 @@ struct iwl4965_rx_queue {
 #define IWL_INVALID_VALUE    -1
 
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
 /**
  * struct iwl4965_ht_agg -- aggregation status while waiting for block-ack
  * @txq_id: Tx queue used for Tx attempt
@@ -453,19 +452,22 @@ struct iwl4965_ht_agg {
 	u16 frame_count;
 	u16 wait_for_ba;
 	u16 start_idx;
-	u32 bitmap0;
-	u32 bitmap1;
+	u64 bitmap;
 	u32 rate_n_flags;
+#define IWL_AGG_OFF 0
+#define IWL_AGG_ON 1
+#define IWL_EMPTYING_HW_QUEUE_ADDBA 2
+#define IWL_EMPTYING_HW_QUEUE_DELBA 3
+	u8 state;
 };
-#endif /* CONFIG_IWL4965_HT_AGG */
+
 #endif /* CONFIG_IWL4965_HT */
 
 struct iwl4965_tid_data {
 	u16 seq_number;
+	u16 tfds_in_queue;
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
 	struct iwl4965_ht_agg agg;
-#endif	/* CONFIG_IWL4965_HT_AGG */
 #endif /* CONFIG_IWL4965_HT */
 };
 
@@ -743,7 +745,7 @@ extern u8 iwl4965_hw_find_station(struct
 
 extern int iwl4965_hw_channel_switch(struct iwl4965_priv *priv, u16 channel);
 extern int iwl4965_tx_queue_reclaim(struct iwl4965_priv *priv, int txq_id, int index);
-
+extern int iwl4965_queue_space(const struct iwl4965_queue *q);
 struct iwl4965_priv;
 
 /*
@@ -779,6 +781,8 @@ extern void iwl4965_set_ht_add_station(s
 extern int iwl4965_mac_ampdu_action(struct ieee80211_hw *hw,
 				    enum ieee80211_ampdu_mlme_action action,
 				    u8 *addr, u16 tid, u16 *ssn);
+extern int iwl4965_check_empty_hw_queue(struct iwl4965_priv *priv, int sta_id,
+					u8 tid, int txq_id);
 #ifdef CONFIG_IWL4965_HT_AGG
 extern void iwl4965_turn_off_agg(struct iwl4965_priv *priv, u8 tid);
 extern void iwl4965_tl_get_stats(struct iwl4965_priv *priv,
@@ -856,7 +860,7 @@ struct iwl4965_agg_control {
 	u32 ba_timeout;
 	struct iwl4965_traffic_load traffic_load[TID_MAX_LOAD_COUNT];
 };
-#endif				/*CONFIG_IWL4965_HT_AGG */
+#endif /*CONFIG_IWL4965_HT_AGG */
 
 struct iwl4965_lq_mngr {
 #ifdef CONFIG_IWL4965_HT_AGG
Index: wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl4965-base.c
===================================================================
--- wl2_6_24_rc5_upstream.orig/drivers/net/wireless/iwlwifi/iwl4965-base.c
+++ wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl4965-base.c
@@ -205,7 +205,7 @@ static void iwl4965_print_hex_dump(int l
  * See more detailed info in iwl-4965-hw.h.
  ***************************************************/
 
-static int iwl4965_queue_space(const struct iwl4965_queue *q)
+int iwl4965_queue_space(const struct iwl4965_queue *q)
 {
 	int s = q->read_ptr - q->write_ptr;
 
@@ -2959,11 +2959,10 @@ static int iwl4965_tx_skb(struct iwl4965
 				__constant_cpu_to_le16(IEEE80211_SCTL_FRAG));
 		seq_number += 0x10;
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
 		/* aggregation is on for this <sta,tid> */
-		if (ctl->flags & IEEE80211_TXCTL_HT_MPDU_AGG)
+		if (ctl->flags & IEEE80211_TXCTL_AMPDU)
 			txq_id = priv->stations[sta_id].tid[tid].agg.txq_id;
-#endif /* CONFIG_IWL4965_HT_AGG */
+		priv->stations[sta_id].tid[tid].tfds_in_queue++;
 #endif /* CONFIG_IWL4965_HT */
 	}
 
@@ -3515,10 +3514,10 @@ int iwl4965_tx_queue_reclaim(struct iwl4
 		nfreed++;
 	}
 
-	if (iwl4965_queue_space(q) > q->low_mark && (txq_id >= 0) &&
+/*	if (iwl4965_queue_space(q) > q->low_mark && (txq_id >= 0) &&
 			(txq_id != IWL_CMD_QUEUE_NUM) &&
 			priv->mac80211_registered)
-		ieee80211_wake_queue(priv->hw, txq_id);
+		ieee80211_wake_queue(priv->hw, txq_id); */
 
 
 	return nfreed;
@@ -3537,7 +3536,6 @@ static int iwl4965_is_tx_success(u32 sta
  *
  ******************************************************************************/
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
 
 static inline int iwl4965_get_ra_sta_id(struct iwl4965_priv *priv,
 				    struct ieee80211_hdr *hdr)
@@ -3572,11 +3570,11 @@ static inline u32 iwl4965_get_scd_ssn(st
  */
 static int iwl4965_tx_status_reply_tx(struct iwl4965_priv *priv,
 				      struct iwl4965_ht_agg *agg,
-				      struct iwl4965_tx_resp *tx_resp,
+				      struct iwl4965_tx_resp_agg *tx_resp,
 				      u16 start_idx)
 {
-	u32 status;
-	__le32 *frame_status = &tx_resp->status;
+	u16 status;
+	struct agg_tx_status *frame_status = &tx_resp->status;
 	struct ieee80211_tx_status *tx_status = NULL;
 	struct ieee80211_hdr *hdr = NULL;
 	int i, sh;
@@ -3589,26 +3587,25 @@ static int iwl4965_tx_status_reply_tx(st
 	agg->frame_count = tx_resp->frame_count;
 	agg->start_idx = start_idx;
 	agg->rate_n_flags = le32_to_cpu(tx_resp->rate_n_flags);
-	agg->bitmap0 = agg->bitmap1 = 0;
+	agg->bitmap = 0;
 
 	/* # frames attempted by Tx command */
 	if (agg->frame_count == 1) {
 		/* Only one frame was attempted; no block-ack will arrive */
-		struct iwl4965_tx_queue *txq ;
-		status = le32_to_cpu(frame_status[0]);
+		status = le16_to_cpu(frame_status[0].status);
+		seq  = le16_to_cpu(frame_status[0].sequence);
+		idx = SEQ_TO_INDEX(seq);
+		txq_id = SEQ_TO_QUEUE(seq);
 
-		txq_id = agg->txq_id;
-		txq = &priv->txq[txq_id];
 		/* FIXME: code repetition */
-		IWL_DEBUG_TX_REPLY("FrameCnt = %d, StartIdx=%d \n",
-				   agg->frame_count, agg->start_idx);
+		IWL_DEBUG_TX_REPLY("FrameCnt = %d, StartIdx=%d idx=%d\n",
+				   agg->frame_count, agg->start_idx, idx);
 
-		tx_status = &(priv->txq[txq_id].txb[txq->q.read_ptr].status);
+		tx_status = &(priv->txq[txq_id].txb[idx].status);
 		tx_status->retry_count = tx_resp->failure_frame;
 		tx_status->queue_number = status & 0xff;
-		tx_status->queue_length = tx_resp->bt_kill_count;
-		tx_status->queue_length |= tx_resp->failure_rts;
-
+		tx_status->queue_length = tx_resp->failure_rts;
+		tx_status->control.flags &= ~IEEE80211_TXCTL_AMPDU;
 		tx_status->flags = iwl4965_is_tx_success(status)?
 			IEEE80211_TX_STATUS_ACK : 0;
 		tx_status->control.tx_rate =
@@ -3629,8 +3626,8 @@ static int iwl4965_tx_status_reply_tx(st
 		/* Construct bit-map of pending frames within Tx window */
 		for (i = 0; i < agg->frame_count; i++) {
 			u16 sc;
-			status = le32_to_cpu(frame_status[i]);
-			seq  = status >> 16;
+			status = le16_to_cpu(frame_status[i].status);
+			seq  = le16_to_cpu(frame_status[i].sequence);
 			idx = SEQ_TO_INDEX(seq);
 			txq_id = SEQ_TO_QUEUE(seq);
 
@@ -3674,13 +3671,12 @@ static int iwl4965_tx_status_reply_tx(st
 					   start, (u32)(bitmap & 0xFFFFFFFF));
 		}
 
-		agg->bitmap0 = bitmap & 0xFFFFFFFF;
-		agg->bitmap1 = bitmap >> 32;
+		agg->bitmap = bitmap;
 		agg->start_idx = start;
 		agg->rate_n_flags = le32_to_cpu(tx_resp->rate_n_flags);
-		IWL_DEBUG_TX_REPLY("Frames %d start_idx=%d bitmap=0x%x\n",
+		IWL_DEBUG_TX_REPLY("Frames %d start_idx=%d bitmap=0x%llx\n",
 				   agg->frame_count, agg->start_idx,
-				   agg->bitmap0);
+				   agg->bitmap);
 
 		if (bitmap)
 			agg->wait_for_ba = 1;
@@ -3688,7 +3684,6 @@ static int iwl4965_tx_status_reply_tx(st
 	return 0;
 }
 #endif
-#endif
 
 /**
  * iwl4965_rx_reply_tx - Handle standard (non-aggregation) Tx response
@@ -3705,9 +3700,9 @@ static void iwl4965_rx_reply_tx(struct i
 	struct iwl4965_tx_resp *tx_resp = (void *)&pkt->u.raw[0];
 	u32  status = le32_to_cpu(tx_resp->status);
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
-	int tid, sta_id;
-#endif
+	int tid = MAX_TID_COUNT, sta_id = IWL_INVALID_STATION;
+	struct ieee80211_hdr *hdr;
+	__le16 *qc;
 #endif
 
 	if ((index >= txq->q.n_bd) || (x2_queue_used(&txq->q, index) == 0)) {
@@ -3719,44 +3714,51 @@ static void iwl4965_rx_reply_tx(struct i
 	}
 
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
+	hdr = iwl4965_tx_queue_get_hdr(priv, txq_id, index);
+	qc = ieee80211_get_qos_ctrl(hdr);
+
+	if (qc)
+		tid = le16_to_cpu(*qc) & 0xf;
+
+	sta_id = iwl4965_get_ra_sta_id(priv, hdr);
+	if (txq->sched_retry && unlikely(sta_id == IWL_INVALID_STATION)) {
+		IWL_ERROR("Station not known\n");
+		return;
+	}
+
 	if (txq->sched_retry) {
 		const u32 scd_ssn = iwl4965_get_scd_ssn(tx_resp);
-		struct ieee80211_hdr *hdr =
-			iwl4965_tx_queue_get_hdr(priv, txq_id, index);
 		struct iwl4965_ht_agg *agg = NULL;
-		__le16 *qc = ieee80211_get_qos_ctrl(hdr);
 
-		if (qc == NULL) {
-			IWL_ERROR("BUG_ON qc is null!!!!\n");
+		if (!qc)
 			return;
-		}
-
-		tid = le16_to_cpu(*qc) & 0xf;
-
-		sta_id = iwl4965_get_ra_sta_id(priv, hdr);
-		if (unlikely(sta_id == IWL_INVALID_STATION)) {
-			IWL_ERROR("Station not known for\n");
-			return;
-		}
 
 		agg = &priv->stations[sta_id].tid[tid].agg;
 
-		iwl4965_tx_status_reply_tx(priv, agg, tx_resp, index);
+		iwl4965_tx_status_reply_tx(priv, agg,
+				(struct iwl4965_tx_resp_agg *)tx_resp, index);
 
 		if ((tx_resp->frame_count == 1) &&
 		    !iwl4965_is_tx_success(status)) {
 			/* TODO: send BAR */
 		}
 
-		if ((txq->q.read_ptr != (scd_ssn & 0xff))) {
+		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
+			int freed;
 			index = iwl4965_queue_dec_wrap(scd_ssn & 0xff, txq->q.n_bd);
 			IWL_DEBUG_TX_REPLY("Retry scheduler reclaim scd_ssn "
 					   "%d index %d\n", scd_ssn , index);
-			iwl4965_tx_queue_reclaim(priv, txq_id, index);
+			freed = iwl4965_tx_queue_reclaim(priv, txq_id, index);
+			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+
+			if (iwl4965_queue_space(&txq->q) > txq->q.low_mark &&
+			    txq_id >= 0 && priv->mac80211_registered &&
+			    agg->state != IWL_EMPTYING_HW_QUEUE_DELBA)
+				ieee80211_wake_queue(priv->hw, txq_id);
+
+			iwl4965_check_empty_hw_queue(priv, sta_id, tid, txq_id);
 		}
 	} else {
-#endif /* CONFIG_IWL4965_HT_AGG */
 #endif /* CONFIG_IWL4965_HT */
 	tx_status = &(txq->txb[txq->q.read_ptr].status);
 
@@ -3777,12 +3779,21 @@ static void iwl4965_rx_reply_tx(struct i
 		     tx_resp->failure_frame);
 
 	IWL_DEBUG_TX_REPLY("Tx queue reclaim %d\n", index);
-	if (index != -1)
-		iwl4965_tx_queue_reclaim(priv, txq_id, index);
+	if (index != -1) {
+		int freed = iwl4965_tx_queue_reclaim(priv, txq_id, index);
+#ifdef CONFIG_IWL4965_HT
+		if (tid != MAX_TID_COUNT)
+			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+		if (iwl4965_queue_space(&txq->q) > txq->q.low_mark &&
+			(txq_id >= 0) &&
+			priv->mac80211_registered)
+			ieee80211_wake_queue(priv->hw, txq_id);
+		if (tid != MAX_TID_COUNT)
+			iwl4965_check_empty_hw_queue(priv, sta_id, tid, txq_id);
+#endif
+	}
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
 	}
-#endif /* CONFIG_IWL4965_HT_AGG */
 #endif /* CONFIG_IWL4965_HT */
 
 	if (iwl_check_bits(status, TX_ABORT_REQUIRED_MSK))
@@ -9041,10 +9052,8 @@ static int iwl4965_pci_probe(struct pci_
 	/* Default value; 4 EDCA QOS priorities */
 	hw->queues = 4;
 #ifdef CONFIG_IWL4965_HT
-#ifdef CONFIG_IWL4965_HT_AGG
 	/* Enhanced value; more queues, to support 11n aggregation */
 	hw->queues = 16;
-#endif /* CONFIG_IWL4965_HT_AGG */
 #endif /* CONFIG_IWL4965_HT */
 
 	spin_lock_init(&priv->lock);
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[RFC PATCH 10/10] iwlwifi: A-MPDU Tx conform block Ack rate scaling to mac80211

[Ron Rindjunsky]

This patch uses the changes in ieee80211_tx_status to pass Block Ack data
to rate scaling module, and uses this data in rate scaling calculations

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-4965-rs.c |   85 +++++++++++++++++++---------
 drivers/net/wireless/iwlwifi/iwl-4965.c    |    9 +--
 2 files changed, 60 insertions(+), 34 deletions(-)

Index: wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965-rs.c
===================================================================
--- wl2_6_24_rc5_upstream.orig/drivers/net/wireless/iwlwifi/iwl-4965-rs.c
+++ wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965-rs.c
@@ -277,7 +277,8 @@ static void rs_rate_scale_clear_window(s
  * packets.
  */
 static int rs_collect_tx_data(struct iwl4965_rate_scale_data *windows,
-			      int scale_index, s32 tpt, u32 status)
+			      int scale_index, s32 tpt, int retries,
+			      int successes)
 {
 	struct iwl4965_rate_scale_data *window = NULL;
 	u64 mask;
@@ -298,26 +299,33 @@ static int rs_collect_tx_data(struct iwl
 	 * subtract "1" from the success counter (this is the main reason
 	 * we keep these bitmaps!).
 	 */
-	if (window->counter >= win_size) {
-		window->counter = win_size - 1;
-		mask = 1;
-		mask = (mask << (win_size - 1));
-		if ((window->data & mask)) {
-			window->data &= ~mask;
-			window->success_counter = window->success_counter - 1;
+	while (retries > 0) {
+		if (window->counter >= win_size) {
+			window->counter = win_size - 1;
+			mask = 1;
+			mask = (mask << (win_size - 1));
+			if (window->data & mask) {
+				window->data &= ~mask;
+				window->success_counter =
+					window->success_counter - 1;
+			}
+		}
+
+		/* Increment frames-attempted counter */
+		window->counter++;
+
+		/* Shift bitmap by one frame (throw away oldest history),
+		 * OR in "1", and increment "success" if this
+		 * frame was successful. */
+		mask = window->data;
+		window->data = (mask << 1);
+		if (successes > 0) {
+			window->success_counter = window->success_counter + 1;
+			window->data |= 0x1;
+			successes--;
 		}
-	}
 
-	/* Increment frames-attempted counter */
-	window->counter = window->counter + 1;
-
-	/* Shift bitmap by one frame (throw away oldest history),
-	 * OR in "1", and increment "success" if this frame was successful. */
-	mask = window->data;
-	window->data = (mask << 1);
-	if (status != 0) {
-		window->success_counter = window->success_counter + 1;
-		window->data |= 0x1;
+		retries--;
 	}
 
 	/* Calculate current success ratio, avoid divide-by-0! */
@@ -677,6 +685,11 @@ static void rs_tx_status(void *priv_rate
 	if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1))
 		return;
 
+	/* This packet was aggregated but doesn't carry rate scale info */
+	if ((tx_resp->control.flags & IEEE80211_TXCTL_AMPDU) &&
+	    !(tx_resp->flags & IEEE80211_TX_STATUS_AMPDU))
+		return;
+
 	retries = tx_resp->retry_count;
 
 	if (retries > 15)
@@ -766,7 +779,7 @@ static void rs_tx_status(void *priv_rate
 				tpt = search_tbl->expected_tpt[rs_index];
 			else
 				tpt = 0;
-			rs_collect_tx_data(search_win, rs_index, tpt, 0);
+			rs_collect_tx_data(search_win, rs_index, tpt, 1, 0);
 
 		/* Else if type matches "current/active" table,
 		 * add failure to "current/active" history */
@@ -777,7 +790,7 @@ static void rs_tx_status(void *priv_rate
 				tpt = curr_tbl->expected_tpt[rs_index];
 			else
 				tpt = 0;
-			rs_collect_tx_data(window, rs_index, tpt, 0);
+			rs_collect_tx_data(window, rs_index, tpt, 1, 0);
 		}
 
 		/* If not searching for a new mode, increment failed counter
@@ -818,9 +831,13 @@ static void rs_tx_status(void *priv_rate
 			tpt = search_tbl->expected_tpt[rs_index];
 		else
 			tpt = 0;
-		rs_collect_tx_data(search_win,
-				    rs_index, tpt, status);
-
+		if (tx_resp->control.flags & IEEE80211_TXCTL_AMPDU)
+			rs_collect_tx_data(search_win, rs_index, tpt,
+					   tx_resp->ampdu_ack_len,
+					   tx_resp->ampdu_ack_map);
+		else
+			rs_collect_tx_data(search_win, rs_index, tpt,
+					   1, status);
 	/* Else if type matches "current/active" table,
 	 * add final tx status to "current/active" history */
 	} else if ((tbl_type.lq_type == curr_tbl->lq_type) &&
@@ -830,16 +847,28 @@ static void rs_tx_status(void *priv_rate
 			tpt = curr_tbl->expected_tpt[rs_index];
 		else
 			tpt = 0;
-		rs_collect_tx_data(window, rs_index, tpt, status);
+		if (tx_resp->control.flags & IEEE80211_TXCTL_AMPDU)
+			rs_collect_tx_data(window, rs_index, tpt,
+					   tx_resp->ampdu_ack_len,
+					   tx_resp->ampdu_ack_map);
+		else
+			rs_collect_tx_data(window, rs_index, tpt,
+					   1, status);
 	}
 
 	/* If not searching for new mode, increment success/failed counter
 	 * ... these help determine when to start searching again */
 	if (lq_sta->stay_in_tbl) {
-		if (status)
-			lq_sta->total_success++;
-		else
-			lq_sta->total_failed++;
+		if (tx_resp->control.flags & IEEE80211_TXCTL_AMPDU) {
+			lq_sta->total_success += tx_resp->ampdu_ack_map;
+			lq_sta->total_failed +=
+			     (tx_resp->ampdu_ack_len - tx_resp->ampdu_ack_map);
+		} else {
+			if (status)
+				lq_sta->total_success++;
+			else
+				lq_sta->total_failed++;
+		}
 	}
 
 	/* See if there's a better rate or modulation mode to try. */
Index: wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965.c
===================================================================
--- wl2_6_24_rc5_upstream.orig/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ wl2_6_24_rc5_upstream/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -4288,12 +4288,9 @@ static int iwl4965_tx_status_reply_compr
 
 	tx_status = &priv->txq[scd_flow].txb[agg->start_idx].status;
 	tx_status->flags = IEEE80211_TX_STATUS_ACK;
-	tx_status->retry_count++;
-#ifdef CONFIG_IWL4965_HT_AGG
-	tx_status->flags |= IEEE80211_TX_STATUS_AGG_STATS;
-	tx_status->successes = successes;
-	tx_status->frame_count = agg->frame_count;
-#endif	/* CONFIG_IWL4965_HT_AGG */
+	tx_status->flags |= IEEE80211_TX_STATUS_AMPDU;
+	tx_status->ampdu_ack_map = successes;
+	tx_status->ampdu_ack_len = agg->frame_count;
 	tx_status->control.tx_rate = agg->rate_n_flags;
 
 	IWL_DEBUG_TX_REPLY("Bitmap %llx\n", bitmap);
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.