* [PATCH 1/5] ath9k: clean up block ack window handling @ 2010-09-20 11:45 Felix Fietkau 2010-09-20 11:45 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Felix Fietkau 0 siblings, 1 reply; 9+ messages in thread From: Felix Fietkau @ 2010-09-20 11:45 UTC (permalink / raw) To: linux-wireless; +Cc: linville, lrodriguez There's no reason to keep pointers to pending tx buffers around, if they're only used to keep track of which frames are still pending. Use a bitfield instead. Signed-off-by: Felix Fietkau <nbd@openwrt.org> --- drivers/net/wireless/ath/ath9k/ath9k.h | 2 +- drivers/net/wireless/ath/ath9k/xmit.c | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index 5fe570b..9fddd80 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -254,7 +254,7 @@ struct ath_atx_tid { struct list_head buf_q; struct ath_node *an; struct ath_atx_ac *ac; - struct ath_buf *tx_buf[ATH_TID_MAX_BUFS]; + unsigned long tx_buf[BITS_TO_LONGS(ATH_TID_MAX_BUFS)]; u16 seq_start; u16 seq_next; u16 baw_size; diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index 457f076..5323a4d 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -168,9 +168,9 @@ static void ath_tx_update_baw(struct ath_softc *sc, struct ath_atx_tid *tid, index = ATH_BA_INDEX(tid->seq_start, seqno); cindex = (tid->baw_head + index) & (ATH_TID_MAX_BUFS - 1); - tid->tx_buf[cindex] = NULL; + __clear_bit(cindex, tid->tx_buf); - while (tid->baw_head != tid->baw_tail && !tid->tx_buf[tid->baw_head]) { + while (tid->baw_head != tid->baw_tail && !test_bit(tid->baw_head, tid->tx_buf)) { INCR(tid->seq_start, IEEE80211_SEQ_MAX); INCR(tid->baw_head, ATH_TID_MAX_BUFS); } @@ -186,9 +186,7 @@ static void ath_tx_addto_baw(struct ath_softc *sc, struct ath_atx_tid *tid, index = ATH_BA_INDEX(tid->seq_start, bf->bf_seqno); cindex = (tid->baw_head + index) & (ATH_TID_MAX_BUFS - 1); - - BUG_ON(tid->tx_buf[cindex] != NULL); - tid->tx_buf[cindex] = bf; + __set_bit(cindex, tid->tx_buf); if (index >= ((tid->baw_tail - tid->baw_head) & (ATH_TID_MAX_BUFS - 1))) { -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/5] ath9k: fix an aggregation start related race condition 2010-09-20 11:45 [PATCH 1/5] ath9k: clean up block ack window handling Felix Fietkau @ 2010-09-20 11:45 ` Felix Fietkau 2010-09-20 11:45 ` [PATCH 3/5] ath9k: clean up / fix aggregation session flush Felix Fietkau ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Felix Fietkau @ 2010-09-20 11:45 UTC (permalink / raw) To: linux-wireless; +Cc: linville, lrodriguez A new aggregation session start can be issued by mac80211, even when the cleanup of the previous session has not completed yet. Since the data structure for the session is not recreated, this could corrupt the block ack window and lock up the aggregation session. Fix this by delaying the new session until the old one has been cleaned up. Signed-off-by: Felix Fietkau <nbd@openwrt.org> Cc: stable@kernel.org --- drivers/net/wireless/ath/ath9k/ath9k.h | 4 ++-- drivers/net/wireless/ath/ath9k/main.c | 2 +- drivers/net/wireless/ath/ath9k/xmit.c | 10 ++++++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index 9fddd80..dfd368a 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -346,8 +346,8 @@ void ath_tx_tasklet(struct ath_softc *sc); void ath_tx_edma_tasklet(struct ath_softc *sc); void ath_tx_cabq(struct ieee80211_hw *hw, struct sk_buff *skb); bool ath_tx_aggr_check(struct ath_softc *sc, struct ath_node *an, u8 tidno); -void ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, - u16 tid, u16 *ssn); +int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, + u16 tid, u16 *ssn); 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 ath9k_enable_ps(struct ath_softc *sc); diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 1165f90..f5af9f9 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -1975,7 +1975,7 @@ static int ath9k_ampdu_action(struct ieee80211_hw *hw, break; case IEEE80211_AMPDU_TX_START: ath9k_ps_wakeup(sc); - ath_tx_aggr_start(sc, sta, tid, ssn); + ret = ath_tx_aggr_start(sc, sta, tid, ssn); ieee80211_start_tx_ba_cb_irqsafe(vif, sta->addr, tid); ath9k_ps_restore(sc); break; diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index 5323a4d..d629bfb 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -783,17 +783,23 @@ static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq, status != ATH_AGGR_BAW_CLOSED); } -void ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, - u16 tid, u16 *ssn) +int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, + u16 tid, u16 *ssn) { struct ath_atx_tid *txtid; struct ath_node *an; an = (struct ath_node *)sta->drv_priv; txtid = ATH_AN_2_TID(an, tid); + + if (txtid->state & (AGGR_CLEANUP | AGGR_ADDBA_COMPLETE)) + return -EAGAIN; + txtid->state |= AGGR_ADDBA_PROGRESS; txtid->paused = true; *ssn = txtid->seq_start; + + return 0; } void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid) -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/5] ath9k: clean up / fix aggregation session flush 2010-09-20 11:45 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Felix Fietkau @ 2010-09-20 11:45 ` Felix Fietkau 2010-09-20 11:45 ` [PATCH 4/5] ath9k: move ath_tx_aggr_check() to the rate control module Felix Fietkau 2010-09-20 14:19 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Luis R. Rodriguez 2010-09-20 17:35 ` [PATCH v2 " Felix Fietkau 2 siblings, 1 reply; 9+ messages in thread From: Felix Fietkau @ 2010-09-20 11:45 UTC (permalink / raw) To: linux-wireless; +Cc: linville, lrodriguez The tid aggregation cleanup is a bit fragile, as it discards failed subframes in some places, and retransmits them in others. This could block the cleanup of an existing aggregation session, if a retransmission for a tid is issued, yet the tid is never scheduled again because of the cleanup state. Fix this by getting rid of as many subframes as possible, as early as possible, and immediately transmitting pending subframes as regular HT frames instead of waiting for the cleanup to complete. Drop all pending subframes while keeping track of the Block ACK window during aggregate tx completion to prevent sending out stale subframes, which could confuse the receiver side. Signed-off-by: Felix Fietkau <nbd@openwrt.org> Cc: stable@kernel.org --- drivers/net/wireless/ath/ath9k/xmit.c | 63 +++++++++++++------------------- 1 files changed, 26 insertions(+), 37 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index d629bfb..e53433e 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -61,6 +61,8 @@ static int ath_tx_num_badfrms(struct ath_softc *sc, struct ath_buf *bf, struct ath_tx_status *ts, int txok); static void ath_tx_rc_status(struct ath_buf *bf, struct ath_tx_status *ts, int nbad, int txok, bool update_rc); +static void ath_tx_update_baw(struct ath_softc *sc, struct ath_atx_tid *tid, + int seqno); enum { MCS_HT20, @@ -143,18 +145,23 @@ static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid) struct ath_txq *txq = &sc->tx.txq[tid->ac->qnum]; struct ath_buf *bf; struct list_head bf_head; - INIT_LIST_HEAD(&bf_head); + struct ath_tx_status ts; - WARN_ON(!tid->paused); + INIT_LIST_HEAD(&bf_head); + memset(&ts, 0, sizeof(ts)); spin_lock_bh(&txq->axq_lock); - tid->paused = false; while (!list_empty(&tid->buf_q)) { bf = list_first_entry(&tid->buf_q, struct ath_buf, list); - BUG_ON(bf_isretried(bf)); list_move_tail(&bf->list, &bf_head); - ath_tx_send_ht_normal(sc, txq, tid, &bf_head); + + if (bf_isretried(bf)) { + ath_tx_update_baw(sc, tid, bf->bf_seqno); + ath_tx_complete_buf(sc, bf, txq, &bf_head, &ts, 0, 0); + } else { + ath_tx_send_ht_normal(sc, txq, tid, &bf_head); + } } spin_unlock_bh(&txq->axq_lock); @@ -429,7 +436,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq, list_move_tail(&bf->list, &bf_head); } - if (!txpending) { + if (!txpending || (tid->state & AGGR_CLEANUP)) { /* * complete the acked-ones/xretried ones; update * block-ack window @@ -508,15 +515,12 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq, } if (tid->state & AGGR_CLEANUP) { + ath_tx_flush_tid(sc, tid); + if (tid->baw_head == tid->baw_tail) { tid->state &= ~AGGR_ADDBA_COMPLETE; tid->state &= ~AGGR_CLEANUP; - - /* send buffered frames as singles */ - ath_tx_flush_tid(sc, tid); } - rcu_read_unlock(); - return; } rcu_read_unlock(); @@ -807,12 +811,6 @@ void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid) struct ath_node *an = (struct ath_node *)sta->drv_priv; struct ath_atx_tid *txtid = ATH_AN_2_TID(an, tid); struct ath_txq *txq = &sc->tx.txq[txtid->ac->qnum]; - struct ath_tx_status ts; - struct ath_buf *bf; - struct list_head bf_head; - - memset(&ts, 0, sizeof(ts)); - INIT_LIST_HEAD(&bf_head); if (txtid->state & AGGR_CLEANUP) return; @@ -822,31 +820,22 @@ void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid) return; } - /* drop all software retried frames and mark this TID */ spin_lock_bh(&txq->axq_lock); txtid->paused = true; - while (!list_empty(&txtid->buf_q)) { - bf = list_first_entry(&txtid->buf_q, struct ath_buf, list); - if (!bf_isretried(bf)) { - /* - * NB: it's based on the assumption that - * software retried frame will always stay - * at the head of software queue. - */ - break; - } - list_move_tail(&bf->list, &bf_head); - ath_tx_update_baw(sc, txtid, bf->bf_seqno); - ath_tx_complete_buf(sc, bf, txq, &bf_head, &ts, 0, 0); - } - spin_unlock_bh(&txq->axq_lock); - if (txtid->baw_head != txtid->baw_tail) { + /* + * If frames are still being transmitted for this TID, they will be + * cleaned up during tx completion. To prevent race conditions, this + * TID can only be reused after all in-progress subframes have been + * completed. + */ + if (txtid->baw_head != txtid->baw_tail) txtid->state |= AGGR_CLEANUP; - } else { + else txtid->state &= ~AGGR_ADDBA_COMPLETE; - ath_tx_flush_tid(sc, txtid); - } + spin_unlock_bh(&txq->axq_lock); + + ath_tx_flush_tid(sc, txtid); } void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid) -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/5] ath9k: move ath_tx_aggr_check() to the rate control module 2010-09-20 11:45 ` [PATCH 3/5] ath9k: clean up / fix aggregation session flush Felix Fietkau @ 2010-09-20 11:45 ` Felix Fietkau 2010-09-20 11:45 ` [PATCH 5/5] ath9k: make the driver specific rate control module optional Felix Fietkau 0 siblings, 1 reply; 9+ messages in thread From: Felix Fietkau @ 2010-09-20 11:45 UTC (permalink / raw) To: linux-wireless; +Cc: linville, lrodriguez It is not used anywhere else and can be made static Signed-off-by: Felix Fietkau <nbd@openwrt.org> --- drivers/net/wireless/ath/ath9k/ath9k.h | 1 - drivers/net/wireless/ath/ath9k/rc.c | 16 ++++++++++++++++ drivers/net/wireless/ath/ath9k/xmit.c | 14 -------------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index dfd368a..e02013c 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -345,7 +345,6 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb, void ath_tx_tasklet(struct ath_softc *sc); void ath_tx_edma_tasklet(struct ath_softc *sc); void ath_tx_cabq(struct ieee80211_hw *hw, struct sk_buff *skb); -bool ath_tx_aggr_check(struct ath_softc *sc, struct ath_node *an, u8 tidno); int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid, u16 *ssn); void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid); diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c index e49be73..ce1cd6d 100644 --- a/drivers/net/wireless/ath/ath9k/rc.c +++ b/drivers/net/wireless/ath/ath9k/rc.c @@ -1320,6 +1320,22 @@ static u8 ath_rc_build_ht_caps(struct ath_softc *sc, struct ieee80211_sta *sta, return caps; } +static bool ath_tx_aggr_check(struct ath_softc *sc, struct ath_node *an, + u8 tidno) +{ + struct ath_atx_tid *txtid; + + if (!(sc->sc_flags & SC_OP_TXAGGR)) + return false; + + txtid = ATH_AN_2_TID(an, tidno); + + if (!(txtid->state & (AGGR_ADDBA_COMPLETE | AGGR_ADDBA_PROGRESS))) + return true; + return false; +} + + /***********************************/ /* mac80211 Rate Control callbacks */ /***********************************/ diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index e53433e..85a7323 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -855,20 +855,6 @@ void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid } } -bool ath_tx_aggr_check(struct ath_softc *sc, struct ath_node *an, u8 tidno) -{ - struct ath_atx_tid *txtid; - - if (!(sc->sc_flags & SC_OP_TXAGGR)) - return false; - - txtid = ATH_AN_2_TID(an, tidno); - - if (!(txtid->state & (AGGR_ADDBA_COMPLETE | AGGR_ADDBA_PROGRESS))) - return true; - return false; -} - /********************/ /* Queue Management */ /********************/ -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/5] ath9k: make the driver specific rate control module optional 2010-09-20 11:45 ` [PATCH 4/5] ath9k: move ath_tx_aggr_check() to the rate control module Felix Fietkau @ 2010-09-20 11:45 ` Felix Fietkau 0 siblings, 0 replies; 9+ messages in thread From: Felix Fietkau @ 2010-09-20 11:45 UTC (permalink / raw) To: linux-wireless; +Cc: linville, lrodriguez ath9k can use minstrel_ht instead, so it makes sense to save some space here. Signed-off-by: Felix Fietkau <nbd@openwrt.org> --- drivers/net/wireless/ath/ath9k/Kconfig | 8 ++++++++ drivers/net/wireless/ath/ath9k/Makefile | 2 +- drivers/net/wireless/ath/ath9k/init.c | 2 ++ drivers/net/wireless/ath/ath9k/rc.h | 11 +++++++++++ 4 files changed, 22 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig index 35f23bd..ad57a6d 100644 --- a/drivers/net/wireless/ath/ath9k/Kconfig +++ b/drivers/net/wireless/ath/ath9k/Kconfig @@ -32,6 +32,14 @@ config ATH9K_DEBUGFS Also required for changing debug message flags at run time. +config ATH9K_RATE_CONTROL + bool "Atheros ath9k rate control" + depends on ATH9K + default y + ---help--- + Say Y, if you want to use the ath9k specific rate control + module instead of minstrel_ht. + config ATH9K_HTC tristate "Atheros HTC based wireless cards support" depends on USB && MAC80211 diff --git a/drivers/net/wireless/ath/ath9k/Makefile b/drivers/net/wireless/ath/ath9k/Makefile index 4555e99..aca0162 100644 --- a/drivers/net/wireless/ath/ath9k/Makefile +++ b/drivers/net/wireless/ath/ath9k/Makefile @@ -5,8 +5,8 @@ ath9k-y += beacon.o \ recv.o \ xmit.o \ virtual.o \ - rc.o +ath9k-$(CONFIG_ATH9K_RATE_CONTROL) += rc.o ath9k-$(CONFIG_PCI) += pci.o ath9k-$(CONFIG_ATHEROS_AR71XX) += ahb.o ath9k-$(CONFIG_ATH9K_DEBUGFS) += debug.o diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c index e7c07b3..f9d84d4 100644 --- a/drivers/net/wireless/ath/ath9k/init.c +++ b/drivers/net/wireless/ath/ath9k/init.c @@ -654,7 +654,9 @@ void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw) hw->sta_data_size = sizeof(struct ath_node); hw->vif_data_size = sizeof(struct ath_vif); +#ifdef CONFIG_ATH9K_RATE_CONTROL hw->rate_control_algorithm = "ath9k_rate_control"; +#endif if (test_bit(ATH9K_MODE_11G, sc->sc_ah->caps.wireless_modes)) hw->wiphy->bands[IEEE80211_BAND_2GHZ] = diff --git a/drivers/net/wireless/ath/ath9k/rc.h b/drivers/net/wireless/ath/ath9k/rc.h index dc10826..268072f 100644 --- a/drivers/net/wireless/ath/ath9k/rc.h +++ b/drivers/net/wireless/ath/ath9k/rc.h @@ -224,7 +224,18 @@ enum ath9k_internal_frame_type { ATH9K_IFT_UNPAUSE }; +#ifdef CONFIG_ATH9K_RATE_CONTROL int ath_rate_control_register(void); void ath_rate_control_unregister(void); +#else +static inline int ath_rate_control_register(void) +{ + return 0; +} + +static inline void ath_rate_control_unregister(void) +{ +} +#endif #endif /* RC_H */ -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/5] ath9k: fix an aggregation start related race condition 2010-09-20 11:45 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Felix Fietkau 2010-09-20 11:45 ` [PATCH 3/5] ath9k: clean up / fix aggregation session flush Felix Fietkau @ 2010-09-20 14:19 ` Luis R. Rodriguez 2010-09-20 14:29 ` Felix Fietkau 2010-09-20 17:35 ` [PATCH v2 " Felix Fietkau 2 siblings, 1 reply; 9+ messages in thread From: Luis R. Rodriguez @ 2010-09-20 14:19 UTC (permalink / raw) To: Felix Fietkau; +Cc: linux-wireless, linville On Mon, Sep 20, 2010 at 4:45 AM, Felix Fietkau <nbd@openwrt.org> wrote: > A new aggregation session start can be issued by mac80211, even when the > cleanup of the previous session has not completed yet. Since the data structure > for the session is not recreated, this could corrupt the block ack window > and lock up the aggregation session. Fix this by delaying the new session > until the old one has been cleaned up. > > Signed-off-by: Felix Fietkau <nbd@openwrt.org> > Cc: stable@kernel.org Any reason to not fix this in mac80211 instead? Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/5] ath9k: fix an aggregation start related race condition 2010-09-20 14:19 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Luis R. Rodriguez @ 2010-09-20 14:29 ` Felix Fietkau 2010-09-20 14:35 ` Luis R. Rodriguez 0 siblings, 1 reply; 9+ messages in thread From: Felix Fietkau @ 2010-09-20 14:29 UTC (permalink / raw) To: Luis R. Rodriguez; +Cc: linux-wireless, linville On 2010-09-20 4:19 PM, Luis R. Rodriguez wrote: > On Mon, Sep 20, 2010 at 4:45 AM, Felix Fietkau <nbd@openwrt.org> wrote: >> A new aggregation session start can be issued by mac80211, even when the >> cleanup of the previous session has not completed yet. Since the data structure >> for the session is not recreated, this could corrupt the block ack window >> and lock up the aggregation session. Fix this by delaying the new session >> until the old one has been cleaned up. >> >> Signed-off-by: Felix Fietkau <nbd@openwrt.org> >> Cc: stable@kernel.org > > Any reason to not fix this in mac80211 instead? mac80211 is already doing the right thing here. The cleanup is ath9k specific. - Felix ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/5] ath9k: fix an aggregation start related race condition 2010-09-20 14:29 ` Felix Fietkau @ 2010-09-20 14:35 ` Luis R. Rodriguez 0 siblings, 0 replies; 9+ messages in thread From: Luis R. Rodriguez @ 2010-09-20 14:35 UTC (permalink / raw) To: Felix Fietkau; +Cc: linux-wireless, linville On Mon, Sep 20, 2010 at 7:29 AM, Felix Fietkau <nbd@openwrt.org> wrote: > On 2010-09-20 4:19 PM, Luis R. Rodriguez wrote: >> On Mon, Sep 20, 2010 at 4:45 AM, Felix Fietkau <nbd@openwrt.org> wrote: >>> A new aggregation session start can be issued by mac80211, even when the >>> cleanup of the previous session has not completed yet. Since the data structure >>> for the session is not recreated, this could corrupt the block ack window >>> and lock up the aggregation session. Fix this by delaying the new session >>> until the old one has been cleaned up. >>> >>> Signed-off-by: Felix Fietkau <nbd@openwrt.org> >>> Cc: stable@kernel.org >> >> Any reason to not fix this in mac80211 instead? > mac80211 is already doing the right thing here. The cleanup is ath9k > specific. Thanks for the clarification. Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/5] ath9k: fix an aggregation start related race condition 2010-09-20 11:45 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Felix Fietkau 2010-09-20 11:45 ` [PATCH 3/5] ath9k: clean up / fix aggregation session flush Felix Fietkau 2010-09-20 14:19 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Luis R. Rodriguez @ 2010-09-20 17:35 ` Felix Fietkau 2 siblings, 0 replies; 9+ messages in thread From: Felix Fietkau @ 2010-09-20 17:35 UTC (permalink / raw) To: linux-wireless; +Cc: linville, lrodriguez A new aggregation session start can be issued by mac80211, even when the cleanup of the previous session has not completed yet. Since the data structure for the session is not recreated, this could corrupt the block ack window and lock up the aggregation session. Fix this by delaying the new session until the old one has been cleaned up. Signed-off-by: Felix Fietkau <nbd@openwrt.org> Cc: stable@kernel.org --- v2: add a return code check before calling ieee80211_start_tx_ba_cb_irqsafe drivers/net/wireless/ath/ath9k/ath9k.h | 4 ++-- drivers/net/wireless/ath/ath9k/main.c | 5 +++-- drivers/net/wireless/ath/ath9k/xmit.c | 10 ++++++++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index 9fddd80..dfd368a 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -346,8 +346,8 @@ void ath_tx_tasklet(struct ath_softc *sc); void ath_tx_edma_tasklet(struct ath_softc *sc); void ath_tx_cabq(struct ieee80211_hw *hw, struct sk_buff *skb); bool ath_tx_aggr_check(struct ath_softc *sc, struct ath_node *an, u8 tidno); -void ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, - u16 tid, u16 *ssn); +int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, + u16 tid, u16 *ssn); 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 ath9k_enable_ps(struct ath_softc *sc); diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 1165f90..8ffeff4 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -1975,8 +1975,9 @@ static int ath9k_ampdu_action(struct ieee80211_hw *hw, break; case IEEE80211_AMPDU_TX_START: ath9k_ps_wakeup(sc); - ath_tx_aggr_start(sc, sta, tid, ssn); - ieee80211_start_tx_ba_cb_irqsafe(vif, sta->addr, tid); + ret = ath_tx_aggr_start(sc, sta, tid, ssn); + if (!ret) + ieee80211_start_tx_ba_cb_irqsafe(vif, sta->addr, tid); ath9k_ps_restore(sc); break; case IEEE80211_AMPDU_TX_STOP: diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index 5323a4d..d629bfb 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -783,17 +783,23 @@ static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq, status != ATH_AGGR_BAW_CLOSED); } -void ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, - u16 tid, u16 *ssn) +int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta, + u16 tid, u16 *ssn) { struct ath_atx_tid *txtid; struct ath_node *an; an = (struct ath_node *)sta->drv_priv; txtid = ATH_AN_2_TID(an, tid); + + if (txtid->state & (AGGR_CLEANUP | AGGR_ADDBA_COMPLETE)) + return -EAGAIN; + txtid->state |= AGGR_ADDBA_PROGRESS; txtid->paused = true; *ssn = txtid->seq_start; + + return 0; } void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid) ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-09-20 17:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-20 11:45 [PATCH 1/5] ath9k: clean up block ack window handling Felix Fietkau 2010-09-20 11:45 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Felix Fietkau 2010-09-20 11:45 ` [PATCH 3/5] ath9k: clean up / fix aggregation session flush Felix Fietkau 2010-09-20 11:45 ` [PATCH 4/5] ath9k: move ath_tx_aggr_check() to the rate control module Felix Fietkau 2010-09-20 11:45 ` [PATCH 5/5] ath9k: make the driver specific rate control module optional Felix Fietkau 2010-09-20 14:19 ` [PATCH 2/5] ath9k: fix an aggregation start related race condition Luis R. Rodriguez 2010-09-20 14:29 ` Felix Fietkau 2010-09-20 14:35 ` Luis R. Rodriguez 2010-09-20 17:35 ` [PATCH v2 " Felix Fietkau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).