This patch fixes a long standing issue of pending packets in the queue being
sent (and retransmitted many times) to sleeping stations.
This was made worse by aggregation through driver-internal retransmitting
of A-MDPU subframes.
Previously the hardware tx filter was cleared unconditionally for every
single packet - with this patch it uses the IEEE80211_TX_CTL_CLEAR_PS_FILT
for unaggregated frames.
A sta_notify driver op is added to stop aggregation for stations when they
enter powersave mode. Subframes stay buffered inside the driver, to ensure
that the BlockAck window keeps a sane state.
Since the driver uses software aggregation, the clearing of the tx filter
needs to be handled by the driver instead of mac80211 for aggregated frames.
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ar9002_mac.c | 12 +++-
drivers/net/wireless/ath/ath9k/ar9003_mac.c | 12 +++-
drivers/net/wireless/ath/ath9k/ath9k.h | 6 ++
drivers/net/wireless/ath/ath9k/hw-ops.h | 5 ++
drivers/net/wireless/ath/ath9k/hw.h | 1 +
drivers/net/wireless/ath/ath9k/mac.h | 1 -
drivers/net/wireless/ath/ath9k/main.c | 22 ++++++
drivers/net/wireless/ath/ath9k/xmit.c | 101 +++++++++++++++++++++++---
8 files changed, 145 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ar9002_mac.c b/drivers/net/wireless/ath/ath9k/ar9002_mac.c
index 8dd8f63..c338efb 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_mac.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_mac.c
@@ -290,7 +290,6 @@ static void ar9002_hw_set11n_txdesc(struct ath_hw *ah, void *ds,
| (flags & ATH9K_TXDESC_VMF ? AR_VirtMoreFrag : 0)
| SM(txPower, AR_XmitPower)
| (flags & ATH9K_TXDESC_VEOL ? AR_VEOL : 0)
- | (flags & ATH9K_TXDESC_CLRDMASK ? AR_ClrDestMask : 0)
| (flags & ATH9K_TXDESC_INTREQ ? AR_TxIntrReq : 0)
| (keyIx != ATH9K_TXKEYIX_INVALID ? AR_DestIdxValid : 0);
@@ -311,6 +310,16 @@ static void ar9002_hw_set11n_txdesc(struct ath_hw *ah, void *ds,
}
}
+static void ar9002_hw_set_clrdmask(struct ath_hw *ah, void *ds, bool val)
+{
+ struct ar5416_desc *ads = AR5416DESC(ds);
+
+ if (val)
+ ads->ds_ctl0 |= AR_ClrDestMask;
+ else
+ ads->ds_ctl0 &= ~AR_ClrDestMask;
+}
+
static void ar9002_hw_set11n_ratescenario(struct ath_hw *ah, void *ds,
void *lastds,
u32 durUpdateEn, u32 rtsctsRate,
@@ -448,4 +457,5 @@ void ar9002_hw_attach_mac_ops(struct ath_hw *ah)
ops->set11n_aggr_last = ar9002_hw_set11n_aggr_last;
ops->clr11n_aggr = ar9002_hw_clr11n_aggr;
ops->set11n_burstduration = ar9002_hw_set11n_burstduration;
+ ops->set_clrdmask = ar9002_hw_set_clrdmask;
}
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_mac.c b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
index 724ac24..c1264d6 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_mac.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
@@ -329,7 +329,6 @@ static void ar9003_hw_set11n_txdesc(struct ath_hw *ah, void *ds,
| (flags & ATH9K_TXDESC_VMF ? AR_VirtMoreFrag : 0)
| SM(txpower, AR_XmitPower)
| (flags & ATH9K_TXDESC_VEOL ? AR_VEOL : 0)
- | (flags & ATH9K_TXDESC_CLRDMASK ? AR_ClrDestMask : 0)
| (keyIx != ATH9K_TXKEYIX_INVALID ? AR_DestIdxValid : 0)
| (flags & ATH9K_TXDESC_LOWRXCHAIN ? AR_LowRxChain : 0);
@@ -350,6 +349,16 @@ static void ar9003_hw_set11n_txdesc(struct ath_hw *ah, void *ds,
ads->ctl22 = 0;
}
+static void ar9003_hw_set_clrdmask(struct ath_hw *ah, void *ds, bool val)
+{
+ struct ar9003_txc *ads = (struct ar9003_txc *) ds;
+
+ if (val)
+ ads->ctl11 |= AR_ClrDestMask;
+ else
+ ads->ctl11 &= ~AR_ClrDestMask;
+}
+
static void ar9003_hw_set11n_ratescenario(struct ath_hw *ah, void *ds,
void *lastds,
u32 durUpdateEn, u32 rtsctsRate,
@@ -510,6 +519,7 @@ void ar9003_hw_attach_mac_ops(struct ath_hw *hw)
ops->set11n_aggr_last = ar9003_hw_set11n_aggr_last;
ops->clr11n_aggr = ar9003_hw_clr11n_aggr;
ops->set11n_burstduration = ar9003_hw_set11n_burstduration;
+ ops->set_clrdmask = ar9003_hw_set_clrdmask;
}
void ath9k_hw_set_rx_bufsize(struct ath_hw *ah, u16 buf_size)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 77ad407..a2ddabf 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -200,6 +200,7 @@ struct ath_atx_ac {
int sched;
struct list_head list;
struct list_head tid_q;
+ bool clear_ps_filter;
};
struct ath_frame_info {
@@ -257,6 +258,8 @@ struct ath_node {
struct ath_atx_ac ac[WME_NUM_AC];
u16 maxampdu;
u8 mpdudensity;
+
+ bool sleeping;
};
#define AGGR_CLEANUP BIT(1)
@@ -338,6 +341,9 @@ int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid);
void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid);
+void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an);
+bool ath_tx_aggr_sleep(struct ath_softc *sc, struct ath_node *an);
+
/********/
/* VIFs */
/********/
diff --git a/drivers/net/wireless/ath/ath9k/hw-ops.h b/drivers/net/wireless/ath/ath9k/hw-ops.h
index 22ee888..9dd90a8 100644
--- a/drivers/net/wireless/ath/ath9k/hw-ops.h
+++ b/drivers/net/wireless/ath/ath9k/hw-ops.h
@@ -122,6 +122,11 @@ static inline void ath9k_hw_set11n_burstduration(struct ath_hw *ah, void *ds,
ath9k_hw_ops(ah)->set11n_burstduration(ah, ds, burstDuration);
}
+static inline void ath9k_hw_set_clrdmask(struct ath_hw *ah, void *ds, bool val)
+{
+ ath9k_hw_ops(ah)->set_clrdmask(ah, ds, val);
+}
+
/* Private hardware call ops */
/* PHY ops */
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 073bc9e..1018d6c 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -626,6 +626,7 @@ struct ath_hw_ops {
void (*clr11n_aggr)(struct ath_hw *ah, void *ds);
void (*set11n_burstduration)(struct ath_hw *ah, void *ds,
u32 burstDuration);
+ void (*set_clrdmask)(struct ath_hw *ah, void *ds, bool val);
};
struct ath_nf_limits {
diff --git a/drivers/net/wireless/ath/ath9k/mac.h b/drivers/net/wireless/ath/ath9k/mac.h
index c2a5938..b60c130 100644
--- a/drivers/net/wireless/ath/ath9k/mac.h
+++ b/drivers/net/wireless/ath/ath9k/mac.h
@@ -239,7 +239,6 @@ struct ath_desc {
void *ds_vdata;
} __packed __aligned(4);
-#define ATH9K_TXDESC_CLRDMASK 0x0001
#define ATH9K_TXDESC_NOACK 0x0002
#define ATH9K_TXDESC_RTSENA 0x0004
#define ATH9K_TXDESC_CTSENA 0x0008
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 3580bc1..44f4d59 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1753,6 +1753,27 @@ static int ath9k_sta_remove(struct ieee80211_hw *hw,
return 0;
}
+static void ath9k_sta_notify(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ enum sta_notify_cmd cmd,
+ struct ieee80211_sta *sta)
+{
+ struct ath_softc *sc = hw->priv;
+ struct ath_node *an = (struct ath_node *) sta->drv_priv;
+
+ switch (cmd) {
+ case STA_NOTIFY_SLEEP:
+ an->sleeping = true;
+ if (ath_tx_aggr_sleep(sc, an))
+ ieee80211_sta_set_tim(sta);
+ break;
+ case STA_NOTIFY_AWAKE:
+ an->sleeping = false;
+ ath_tx_aggr_wakeup(sc, an);
+ break;
+ }
+}
+
static int ath9k_conf_tx(struct ieee80211_hw *hw, u16 queue,
const struct ieee80211_tx_queue_params *params)
{
@@ -2238,6 +2259,7 @@ struct ieee80211_ops ath9k_ops = {
.configure_filter = ath9k_configure_filter,
.sta_add = ath9k_sta_add,
.sta_remove = ath9k_sta_remove,
+ .sta_notify = ath9k_sta_notify,
.conf_tx = ath9k_conf_tx,
.bss_info_changed = ath9k_bss_info_changed,
.set_key = ath9k_set_key,
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 3cea3f7..a1230c0 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -357,6 +357,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
struct ath_frame_info *fi;
int nframes;
u8 tidno;
+ bool clear_filter;
skb = bf->bf_mpdu;
hdr = (struct ieee80211_hdr *)skb->data;
@@ -441,22 +442,24 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
/* transmit completion */
acked_cnt++;
} else {
- if (!(tid->state & AGGR_CLEANUP) && retry) {
- if (fi->retries < ATH_MAX_SW_RETRIES) {
- ath_tx_set_retry(sc, txq, bf->bf_mpdu);
- txpending = 1;
- } else {
- bf->bf_state.bf_type |= BUF_XRETRY;
- txfail = 1;
- sendbar = 1;
- txfail_cnt++;
- }
- } else {
+ if ((tid->state & AGGR_CLEANUP) || !retry) {
/*
* cleanup in progress, just fail
* the un-acked sub-frames
*/
txfail = 1;
+ } else if (fi->retries < ATH_MAX_SW_RETRIES) {
+ if (!(ts->ts_status & ATH9K_TXERR_FILT) ||
+ !an->sleeping)
+ ath_tx_set_retry(sc, txq, bf->bf_mpdu);
+
+ clear_filter = true;
+ txpending = 1;
+ } else {
+ bf->bf_state.bf_type |= BUF_XRETRY;
+ txfail = 1;
+ sendbar = 1;
+ txfail_cnt++;
}
}
@@ -496,6 +499,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
!txfail, sendbar);
} else {
/* retry the un-acked ones */
+ ath9k_hw_set_clrdmask(sc->sc_ah, bf->bf_desc, false);
if (!(sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_EDMA)) {
if (bf->bf_next == NULL && bf_last->bf_stale) {
struct ath_buf *tbf;
@@ -546,7 +550,12 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
/* prepend un-acked frames to the beginning of the pending frame queue */
if (!list_empty(&bf_pending)) {
+ if (an->sleeping)
+ ieee80211_sta_set_tim(sta);
+
spin_lock_bh(&txq->axq_lock);
+ if (clear_filter)
+ tid->ac->clear_ps_filter = true;
list_splice(&bf_pending, &tid->buf_q);
ath_tx_queue_tid(txq, tid);
spin_unlock_bh(&txq->axq_lock);
@@ -816,6 +825,11 @@ static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
bf = list_first_entry(&bf_q, struct ath_buf, list);
bf->bf_lastbf = list_entry(bf_q.prev, struct ath_buf, list);
+ if (tid->ac->clear_ps_filter) {
+ tid->ac->clear_ps_filter = false;
+ ath9k_hw_set_clrdmask(sc->sc_ah, bf->bf_desc, true);
+ }
+
/* if only one frame, send as non-aggregate */
if (bf == bf->bf_lastbf) {
fi = get_frame_info(bf->bf_mpdu);
@@ -896,6 +910,67 @@ void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid)
ath_tx_flush_tid(sc, txtid);
}
+bool ath_tx_aggr_sleep(struct ath_softc *sc, struct ath_node *an)
+{
+ struct ath_atx_tid *tid;
+ struct ath_atx_ac *ac;
+ struct ath_txq *txq;
+ bool buffered = false;
+ int tidno;
+
+ for (tidno = 0, tid = &an->tid[tidno];
+ tidno < WME_NUM_TID; tidno++, tid++) {
+
+ if (!tid->sched)
+ continue;
+
+ ac = tid->ac;
+ txq = ac->txq;
+
+ spin_lock_bh(&txq->axq_lock);
+
+ if (!list_empty(&tid->buf_q))
+ buffered = true;
+
+ tid->sched = false;
+ list_del(&tid->list);
+
+ if (ac->sched) {
+ ac->sched = false;
+ list_del(&ac->list);
+ }
+
+ spin_unlock_bh(&txq->axq_lock);
+ }
+
+ return buffered;
+}
+
+void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an)
+{
+ struct ath_atx_tid *tid;
+ struct ath_atx_ac *ac;
+ struct ath_txq *txq;
+ int tidno;
+
+ for (tidno = 0, tid = &an->tid[tidno];
+ tidno < WME_NUM_TID; tidno++, tid++) {
+
+ ac = tid->ac;
+ txq = ac->txq;
+
+ spin_lock_bh(&txq->axq_lock);
+ ac->clear_ps_filter = true;
+
+ if (!list_empty(&tid->buf_q) && !tid->paused) {
+ ath_tx_queue_tid(txq, tid);
+ ath_txq_schedule(sc, txq);
+ }
+
+ spin_unlock_bh(&txq->axq_lock);
+ }
+}
+
void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid)
{
struct ath_atx_tid *txtid;
@@ -1491,7 +1566,6 @@ static int setup_tx_flags(struct sk_buff *skb)
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
int flags = 0;
- flags |= ATH9K_TXDESC_CLRDMASK; /* needed for crypto errors */
flags |= ATH9K_TXDESC_INTREQ;
if (tx_info->flags & IEEE80211_TX_CTL_NO_ACK)
@@ -1754,6 +1828,9 @@ static void ath_tx_start_dma(struct ath_softc *sc, struct ath_buf *bf,
if (txctl->paprd)
bf->bf_state.bfs_paprd_timestamp = jiffies;
+ if (tx_info->flags & IEEE80211_TX_CTL_CLEAR_PS_FILT)
+ ath9k_hw_set_clrdmask(sc->sc_ah, bf->bf_desc, true);
+
ath_tx_send_normal(sc, txctl->txq, tid, &bf_head);
}
--
1.7.3.2
Felix Fietkau wrote:
> I should probably add an extra check that tests for tid->buf_q being
> empty - and some comments.
> This handles a race condition that occurs if an A-MPDU completes (with
> some pending frames) after the station entered PS and
> ieee80211_sta_set_tim was not called because the buffer queue was empty.
Alright, thanks for the explanation.
Sujith
Felix Fietkau wrote:
> The driver calls ieee80211_sta_set_tim on STA_NOTIFY_SLEEP whenever it
> still has some buffered frames that it intends to keep until the client
> wakes up again (i.e. frames that will not be returned to mac80211 as
> filtered).
Why should ieee80211_sta_set_tim() be called from complete_aggr() ?
Sujith
On 2011-04-18 1:10 PM, Sujith wrote:
> Felix Fietkau wrote:
>> The driver calls ieee80211_sta_set_tim on STA_NOTIFY_SLEEP whenever it
>> still has some buffered frames that it intends to keep until the client
>> wakes up again (i.e. frames that will not be returned to mac80211 as
>> filtered).
>
> Why should ieee80211_sta_set_tim() be called from complete_aggr() ?
I should probably add an extra check that tests for tid->buf_q being
empty - and some comments.
This handles a race condition that occurs if an A-MPDU completes (with
some pending frames) after the station entered PS and
ieee80211_sta_set_tim was not called because the buffer queue was empty.
- Felix
On 2011-04-18 6:26 AM, Sujith wrote:
> Felix Fietkau wrote:
>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> index 77ad407..a2ddabf 100644
>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> @@ -200,6 +200,7 @@ struct ath_atx_ac {
>> int sched;
>> struct list_head list;
>> struct list_head tid_q;
>> + bool clear_ps_filter;
>
> The destination mask is per-station, so would ath_node be a better place for this
> variable ?
That's what I thought at first as well, but the problem with that is
that when using multiple queues there is no guarantee that the A-MPDU
enqueued first is also transmitted first, so I decided to do it once per
AC. Most of the time, only BE is active anyway...
>> +static void ath9k_sta_notify(struct ieee80211_hw *hw,
>> + struct ieee80211_vif *vif,
>> + enum sta_notify_cmd cmd,
>> + struct ieee80211_sta *sta)
>> +{
>> + struct ath_softc *sc = hw->priv;
>> + struct ath_node *an = (struct ath_node *) sta->drv_priv;
>> +
>> + switch (cmd) {
>> + case STA_NOTIFY_SLEEP:
>> + an->sleeping = true;
>> + if (ath_tx_aggr_sleep(sc, an))
>> + ieee80211_sta_set_tim(sta);
>> + break;
>> + case STA_NOTIFY_AWAKE:
>> + an->sleeping = false;
>> + ath_tx_aggr_wakeup(sc, an);
>> + break;
>> + }
>> +}
>> +
>
> an->sleeping needs to be locked, no ?
> It seems to be accessed from the TX tasklet also.
It is only updated in this function and these updates should be atomic
anyway, so I don't think locking would be useful here.
>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>> index 3cea3f7..a1230c0 100644
>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>> @@ -357,6 +357,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>> struct ath_frame_info *fi;
>> int nframes;
>> u8 tidno;
>> + bool clear_filter;
>>
>> skb = bf->bf_mpdu;
>> hdr = (struct ieee80211_hdr *)skb->data;
>> @@ -816,6 +825,11 @@ static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
>> bf = list_first_entry(&bf_q, struct ath_buf, list);
>> bf->bf_lastbf = list_entry(bf_q.prev, struct ath_buf, list);
>>
>> + if (tid->ac->clear_ps_filter) {
>> + tid->ac->clear_ps_filter = false;
>> + ath9k_hw_set_clrdmask(sc->sc_ah, bf->bf_desc, true);
>> + }
>> +
>
> Can you explain a bit more on the flow ? How exactly is ieee80211_sta_set_tim() to be used
> by a driver ? (I would like to port this fix to ath9k_htc).
The driver calls ieee80211_sta_set_tim on STA_NOTIFY_SLEEP whenever it
still has some buffered frames that it intends to keep until the client
wakes up again (i.e. frames that will not be returned to mac80211 as
filtered).
>> /* if only one frame, send as non-aggregate */
>> if (bf == bf->bf_lastbf) {
>> fi = get_frame_info(bf->bf_mpdu);
>> @@ -896,6 +910,67 @@ void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid)
>> ath_tx_flush_tid(sc, txtid);
>> }
>>
>> +bool ath_tx_aggr_sleep(struct ath_softc *sc, struct ath_node *an)
>> +{
>> + struct ath_atx_tid *tid;
>> + struct ath_atx_ac *ac;
>> + struct ath_txq *txq;
>> + bool buffered = false;
>> + int tidno;
>> +
>> + for (tidno = 0, tid =&an->tid[tidno];
>> + tidno< WME_NUM_TID; tidno++, tid++) {
>> +
>> + if (!tid->sched)
>> + continue;
>
> tid->sched has to be locked ?
Yeah, I think that one might need locking.
- Felix
Frame filtering relies on having a valid destination index (keycache slot),
to keep track of the destination. Assigning a keycache slot (configured
to unencrypted, with no key data attached) improves powersave handling in
AP mode with no encryption.
The dummy keycache entry for a station is cleared, when a real key gets
added.
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 2 ++
drivers/net/wireless/ath/ath9k/main.c | 22 ++++++++++++++++++++++
drivers/net/wireless/ath/ath9k/xmit.c | 10 +++++++---
drivers/net/wireless/ath/key.c | 6 +++++-
4 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index a2ddabf..a6b5388 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -256,6 +256,8 @@ struct ath_node {
#endif
struct ath_atx_tid tid[WME_NUM_TID];
struct ath_atx_ac ac[WME_NUM_AC];
+ int ps_key;
+
u16 maxampdu;
u8 mpdudensity;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 44f4d59..a8d9009 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1736,18 +1736,37 @@ static int ath9k_sta_add(struct ieee80211_hw *hw,
struct ieee80211_sta *sta)
{
struct ath_softc *sc = hw->priv;
+ struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_node *an = (struct ath_node *) sta->drv_priv;
+ struct ieee80211_key_conf ps_key = { };
ath_node_attach(sc, sta);
+ an->ps_key = ath_key_config(common, vif, sta, &ps_key);
return 0;
}
+static void ath9k_del_ps_key(struct ath_softc *sc,
+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta)
+{
+ struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_node *an = (struct ath_node *) sta->drv_priv;
+ struct ieee80211_key_conf ps_key = { .hw_key_idx = an->ps_key };
+
+ if (!an->ps_key)
+ return;
+
+ ath_key_delete(common, &ps_key);
+}
+
static int ath9k_sta_remove(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *sta)
{
struct ath_softc *sc = hw->priv;
+ ath9k_del_ps_key(sc, vif, sta);
ath_node_detach(sc, sta);
return 0;
@@ -1850,6 +1869,9 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
switch (cmd) {
case SET_KEY:
+ if (sta)
+ ath9k_del_ps_key(sc, vif, sta);
+
ret = ath_key_config(common, vif, sta, key);
if (ret >= 0) {
key->hw_key_idx = ret;
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index a1230c0..e9e99f7 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1526,7 +1526,7 @@ static void setup_frame_info(struct ieee80211_hw *hw, struct sk_buff *skb,
struct ieee80211_key_conf *hw_key = tx_info->control.hw_key;
struct ieee80211_hdr *hdr;
struct ath_frame_info *fi = get_frame_info(skb);
- struct ath_node *an;
+ struct ath_node *an = NULL;
struct ath_atx_tid *tid;
enum ath9k_key_type keytype;
u16 seqno = 0;
@@ -1534,11 +1534,13 @@ static void setup_frame_info(struct ieee80211_hw *hw, struct sk_buff *skb,
keytype = ath9k_cmn_get_hw_crypto_keytype(skb);
+ if (sta)
+ an = (struct ath_node *) sta->drv_priv;
+
hdr = (struct ieee80211_hdr *)skb->data;
- if (sta && ieee80211_is_data_qos(hdr->frame_control) &&
+ if (an && ieee80211_is_data_qos(hdr->frame_control) &&
conf_is_ht(&hw->conf) && (sc->sc_flags & SC_OP_TXAGGR)) {
- an = (struct ath_node *) sta->drv_priv;
tidno = ieee80211_get_qos_ctl(hdr)[0] & IEEE80211_QOS_CTL_TID_MASK;
/*
@@ -1554,6 +1556,8 @@ static void setup_frame_info(struct ieee80211_hw *hw, struct sk_buff *skb,
memset(fi, 0, sizeof(*fi));
if (hw_key)
fi->keyix = hw_key->hw_key_idx;
+ else if (an && ieee80211_is_data(hdr->frame_control) && an->ps_key > 0)
+ fi->keyix = an->ps_key;
else
fi->keyix = ATH9K_TXKEYIX_INVALID;
fi->keytype = keytype;
diff --git a/drivers/net/wireless/ath/key.c b/drivers/net/wireless/ath/key.c
index 0d4f39c..a61ef3d6 100644
--- a/drivers/net/wireless/ath/key.c
+++ b/drivers/net/wireless/ath/key.c
@@ -483,6 +483,9 @@ int ath_key_config(struct ath_common *common,
memset(&hk, 0, sizeof(hk));
switch (key->cipher) {
+ case 0:
+ hk.kv_type = ATH_CIPHER_CLR;
+ break;
case WLAN_CIPHER_SUITE_WEP40:
case WLAN_CIPHER_SUITE_WEP104:
hk.kv_type = ATH_CIPHER_WEP;
@@ -498,7 +501,8 @@ int ath_key_config(struct ath_common *common,
}
hk.kv_len = key->keylen;
- memcpy(hk.kv_val, key->key, key->keylen);
+ if (key->keylen)
+ memcpy(hk.kv_val, key->key, key->keylen);
if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
switch (vif->type) {
--
1.7.3.2
Felix Fietkau wrote:
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 77ad407..a2ddabf 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -200,6 +200,7 @@ struct ath_atx_ac {
> int sched;
> struct list_head list;
> struct list_head tid_q;
> + bool clear_ps_filter;
The destination mask is per-station, so would ath_node be a better place for this
variable ?
> +static void ath9k_sta_notify(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + enum sta_notify_cmd cmd,
> + struct ieee80211_sta *sta)
> +{
> + struct ath_softc *sc = hw->priv;
> + struct ath_node *an = (struct ath_node *) sta->drv_priv;
> +
> + switch (cmd) {
> + case STA_NOTIFY_SLEEP:
> + an->sleeping = true;
> + if (ath_tx_aggr_sleep(sc, an))
> + ieee80211_sta_set_tim(sta);
> + break;
> + case STA_NOTIFY_AWAKE:
> + an->sleeping = false;
> + ath_tx_aggr_wakeup(sc, an);
> + break;
> + }
> +}
> +
an->sleeping needs to be locked, no ?
It seems to be accessed from the TX tasklet also.
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 3cea3f7..a1230c0 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -357,6 +357,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> struct ath_frame_info *fi;
> int nframes;
> u8 tidno;
> + bool clear_filter;
>
> skb = bf->bf_mpdu;
> hdr = (struct ieee80211_hdr *)skb->data;
> @@ -441,22 +442,24 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> /* transmit completion */
> acked_cnt++;
> } else {
> - if (!(tid->state & AGGR_CLEANUP) && retry) {
> - if (fi->retries < ATH_MAX_SW_RETRIES) {
> - ath_tx_set_retry(sc, txq, bf->bf_mpdu);
> - txpending = 1;
> - } else {
> - bf->bf_state.bf_type |= BUF_XRETRY;
> - txfail = 1;
> - sendbar = 1;
> - txfail_cnt++;
> - }
> - } else {
> + if ((tid->state & AGGR_CLEANUP) || !retry) {
> /*
> * cleanup in progress, just fail
> * the un-acked sub-frames
> */
> txfail = 1;
> + } else if (fi->retries < ATH_MAX_SW_RETRIES) {
> + if (!(ts->ts_status & ATH9K_TXERR_FILT) ||
> + !an->sleeping)
> + ath_tx_set_retry(sc, txq, bf->bf_mpdu);
> +
> + clear_filter = true;
> + txpending = 1;
> + } else {
> + bf->bf_state.bf_type |= BUF_XRETRY;
> + txfail = 1;
> + sendbar = 1;
> + txfail_cnt++;
> }
> }
>
> @@ -496,6 +499,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> !txfail, sendbar);
> } else {
> /* retry the un-acked ones */
> + ath9k_hw_set_clrdmask(sc->sc_ah, bf->bf_desc, false);
> if (!(sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_EDMA)) {
> if (bf->bf_next == NULL && bf_last->bf_stale) {
> struct ath_buf *tbf;
> @@ -546,7 +550,12 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>
> /* prepend un-acked frames to the beginning of the pending frame queue */
> if (!list_empty(&bf_pending)) {
> + if (an->sleeping)
> + ieee80211_sta_set_tim(sta);
> +
> spin_lock_bh(&txq->axq_lock);
> + if (clear_filter)
> + tid->ac->clear_ps_filter = true;
> list_splice(&bf_pending, &tid->buf_q);
> ath_tx_queue_tid(txq, tid);
> spin_unlock_bh(&txq->axq_lock);
> @@ -816,6 +825,11 @@ static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
> bf = list_first_entry(&bf_q, struct ath_buf, list);
> bf->bf_lastbf = list_entry(bf_q.prev, struct ath_buf, list);
>
> + if (tid->ac->clear_ps_filter) {
> + tid->ac->clear_ps_filter = false;
> + ath9k_hw_set_clrdmask(sc->sc_ah, bf->bf_desc, true);
> + }
> +
Can you explain a bit more on the flow ? How exactly is ieee80211_sta_set_tim() to be used
by a driver ? (I would like to port this fix to ath9k_htc).
> /* if only one frame, send as non-aggregate */
> if (bf == bf->bf_lastbf) {
> fi = get_frame_info(bf->bf_mpdu);
> @@ -896,6 +910,67 @@ void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid)
> ath_tx_flush_tid(sc, txtid);
> }
>
> +bool ath_tx_aggr_sleep(struct ath_softc *sc, struct ath_node *an)
> +{
> + struct ath_atx_tid *tid;
> + struct ath_atx_ac *ac;
> + struct ath_txq *txq;
> + bool buffered = false;
> + int tidno;
> +
> + for (tidno = 0, tid = &an->tid[tidno];
> + tidno < WME_NUM_TID; tidno++, tid++) {
> +
> + if (!tid->sched)
> + continue;
tid->sched has to be locked ?
Sujith