Since rt2800 hardware isn't capable of reporting the TX status of
BlockAckReq frames implement the TX status handling of BARs in
rt2x00lib. We keep track of all BARs that are send out and try to
match incoming BAs to the appropriate BARs. This allows us to report a
more or less accurate TX status for BAR frames which in turn improves
BA session stability.
This is loosley based on Christian Lamparter's patch for carl9170
"carl9170: fix HT peer BA session corruption".
We have to walk the list of pending BARs for every rx'red BA even
though most BAs don't belong to any of these BARs as they are just
acknowledging an AMPDU. To keep that overhead low use RCU which allows
us to walk the list of pending BARs without the need to acquire a lock.
This however requires us to _copy_ relevant information from the BAR
(RA, TA, control field, start sequence number) into our BAR list entry.
Signed-off-by: Helmut Schaa <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800lib.c | 6 +-
drivers/net/wireless/rt2x00/rt2x00.h | 20 ++++++
drivers/net/wireless/rt2x00/rt2x00dev.c | 101 ++++++++++++++++++++++++++++-
drivers/net/wireless/rt2x00/rt2x00queue.c | 47 +++++++++++++
4 files changed, 169 insertions(+), 5 deletions(-)
Andreas, if you like you can add your tested-by ...
Thanks,
Helmut
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index f139a91..a5c694f 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -1296,8 +1296,7 @@ void rt2800_config_filter(struct rt2x00_dev *rt2x00dev,
!(filter_flags & FIF_CONTROL));
rt2x00_set_field32(®, RX_FILTER_CFG_DROP_PSPOLL,
!(filter_flags & FIF_PSPOLL));
- rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BA,
- !(filter_flags & FIF_CONTROL));
+ rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BA, 0);
rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BAR,
!(filter_flags & FIF_CONTROL));
rt2x00_set_field32(®, RX_FILTER_CFG_DROP_CNTL,
@@ -5146,8 +5145,7 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_PS_NULLFUNC_STACK |
IEEE80211_HW_AMPDU_AGGREGATION |
- IEEE80211_HW_REPORTS_TX_ACK_STATUS |
- IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL;
+ IEEE80211_HW_REPORTS_TX_ACK_STATUS;
/*
* Don't set IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING for USB devices
diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 0751b35..b52512b 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -1016,6 +1016,26 @@ struct rt2x00_dev {
* Protect the interrupt mask register.
*/
spinlock_t irqmask_lock;
+
+ /*
+ * List of BlockAckReq TX entries that need driver BlockAck processing.
+ */
+ struct list_head bar_list;
+ spinlock_t bar_list_lock;
+};
+
+struct rt2x00_bar_list_entry {
+ struct list_head list;
+ struct rcu_head head;
+
+ struct queue_entry *entry;
+ int block_acked;
+
+ /* Relevant parts of the IEEE80211 BAR header */
+ __u8 ra[6];
+ __u8 ta[6];
+ __le16 control;
+ __le16 start_seq_num;
};
/*
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 44f8b3f..b40a538 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -271,6 +271,50 @@ void rt2x00lib_dmadone(struct queue_entry *entry)
}
EXPORT_SYMBOL_GPL(rt2x00lib_dmadone);
+static inline int rt2x00lib_txdone_bar_status(struct queue_entry *entry)
+{
+ struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
+ struct ieee80211_bar *bar = (void *) entry->skb->data;
+ struct rt2x00_bar_list_entry *bar_entry;
+ int ret;
+
+ if (likely(!ieee80211_is_back_req(bar->frame_control)))
+ return 0;
+
+ /*
+ * Unlike all other frames, the status report for BARs does
+ * not directly come from the hardware as it is incapable of
+ * matching a BA to a previously send BAR. The hardware will
+ * report all BARs as if they weren't acked at all.
+ *
+ * Instead the RX-path will scan for incoming BAs and set the
+ * block_acked flag if it sees one that was likely caused by
+ * a BAR from us.
+ *
+ * Remove remaining BARs here and return their status for
+ * TX done processing.
+ */
+ ret = 0;
+ rcu_read_lock();
+ list_for_each_entry_rcu(bar_entry, &rt2x00dev->bar_list, list) {
+ if (bar_entry->entry != entry)
+ continue;
+
+ spin_lock_bh(&rt2x00dev->bar_list_lock);
+ /* Return whether this BAR was blockacked or not */
+ ret = bar_entry->block_acked;
+ /* Remove the BAR from our checklist */
+ list_del_rcu(&bar_entry->list);
+ spin_unlock_bh(&rt2x00dev->bar_list_lock);
+ kfree_rcu(bar_entry, head);
+
+ break;
+ }
+ rcu_read_unlock();
+
+ return ret;
+}
+
void rt2x00lib_txdone(struct queue_entry *entry,
struct txdone_entry_desc *txdesc)
{
@@ -324,9 +368,12 @@ void rt2x00lib_txdone(struct queue_entry *entry,
rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_TXDONE, entry->skb);
/*
- * Determine if the frame has been successfully transmitted.
+ * Determine if the frame has been successfully transmitted and
+ * remove BARs from our check list while checking for their
+ * TX status.
*/
success =
+ rt2x00lib_txdone_bar_status(entry) ||
test_bit(TXDONE_SUCCESS, &txdesc->flags) ||
test_bit(TXDONE_UNKNOWN, &txdesc->flags);
@@ -491,6 +538,50 @@ static void rt2x00lib_sleep(struct work_struct *work)
IEEE80211_CONF_CHANGE_PS);
}
+static void rt2x00lib_rxdone_check_ba(struct rt2x00_dev *rt2x00dev,
+ struct sk_buff *skb,
+ struct rxdone_entry_desc *rxdesc)
+{
+ struct rt2x00_bar_list_entry *entry;
+ struct ieee80211_bar *ba = (void *)skb->data;
+
+ if (likely(!ieee80211_is_back(ba->frame_control)))
+ return;
+
+ if (rxdesc->size < sizeof(*ba) + FCS_LEN)
+ return;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, &rt2x00dev->bar_list, list) {
+
+ if (ba->start_seq_num != entry->start_seq_num)
+ continue;
+
+#define TID_CHECK(a, b) ( \
+ ((a) & cpu_to_le16(IEEE80211_BAR_CTRL_TID_INFO_MASK)) == \
+ ((b) & cpu_to_le16(IEEE80211_BAR_CTRL_TID_INFO_MASK))) \
+
+ if (!TID_CHECK(ba->control, entry->control))
+ continue;
+
+#undef TID_CHECK
+
+ if (compare_ether_addr(ba->ra, entry->ta))
+ continue;
+
+ if (compare_ether_addr(ba->ta, entry->ra))
+ continue;
+
+ /* Mark BAR since we received the according BA */
+ spin_lock_bh(&rt2x00dev->bar_list_lock);
+ entry->block_acked = 1;
+ spin_unlock_bh(&rt2x00dev->bar_list_lock);
+ break;
+ }
+ rcu_read_unlock();
+
+}
+
static void rt2x00lib_rxdone_check_ps(struct rt2x00_dev *rt2x00dev,
struct sk_buff *skb,
struct rxdone_entry_desc *rxdesc)
@@ -674,6 +765,12 @@ void rt2x00lib_rxdone(struct queue_entry *entry, gfp_t gfp)
rt2x00lib_rxdone_check_ps(rt2x00dev, entry->skb, &rxdesc);
/*
+ * Check for incoming BlockAcks to match to the BlockAckReqs
+ * we've send out.
+ */
+ rt2x00lib_rxdone_check_ba(rt2x00dev, entry->skb, &rxdesc);
+
+ /*
* Update extra components
*/
rt2x00link_update_stats(rt2x00dev, entry->skb, &rxdesc);
@@ -1183,6 +1280,8 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
spin_lock_init(&rt2x00dev->irqmask_lock);
mutex_init(&rt2x00dev->csr_mutex);
+ INIT_LIST_HEAD(&rt2x00dev->bar_list);
+ spin_lock_init(&rt2x00dev->bar_list_lock);
set_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index e488b94..f35d85a 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -582,6 +582,48 @@ static void rt2x00queue_kick_tx_queue(struct data_queue *queue,
queue->rt2x00dev->ops->lib->kick_queue(queue);
}
+static void rt2x00queue_bar_check(struct queue_entry *entry)
+{
+ struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
+ struct ieee80211_bar *bar = (void *) (entry->skb->data +
+ rt2x00dev->ops->extra_tx_headroom);
+ struct rt2x00_bar_list_entry *bar_entry;
+
+ if (likely(!ieee80211_is_back_req(bar->frame_control)))
+ return;
+
+ bar_entry = kmalloc(sizeof(*bar_entry), GFP_ATOMIC);
+
+ /*
+ * If the alloc fails we still send the BAR out but just don't track
+ * it in our bar list. And as a result we will report it to mac80211
+ * back as failed.
+ */
+ if (!bar_entry)
+ return;
+
+ bar_entry->entry = entry;
+ bar_entry->block_acked = 0;
+
+ /*
+ * Copy the relevant parts of the 802.11 BAR into out check list
+ * such that we can use RCU for less-overhead in the RX path since
+ * sending BARs and processing the according BlockAck should be
+ * the exception.
+ */
+ memcpy(bar_entry->ra, bar->ra, sizeof(bar->ra));
+ memcpy(bar_entry->ta, bar->ta, sizeof(bar->ta));
+ bar_entry->control = bar->control;
+ bar_entry->start_seq_num = bar->start_seq_num;
+
+ /*
+ * Insert BAR into our BAR check list.
+ */
+ spin_lock_bh(&rt2x00dev->bar_list_lock);
+ list_add_tail_rcu(&bar_entry->list, &rt2x00dev->bar_list);
+ spin_unlock_bh(&rt2x00dev->bar_list_lock);
+}
+
int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
bool local)
{
@@ -680,6 +722,11 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
goto out;
}
+ /*
+ * Put BlockAckReqs into our check list for driver BA processing.
+ */
+ rt2x00queue_bar_check(entry);
+
set_bit(ENTRY_DATA_PENDING, &entry->flags);
rt2x00queue_index_inc(entry, Q_INDEX);
--
1.7.7
On Fri, Jan 25, 2013 at 06:53:23PM +0100, Christian Lamparter wrote:
> On Thursday 17 January 2013 19:42:52 Gertjan van Wingerde wrote:
> > On 17 jan. 2013, at 18:16, Christian Lamparter <[email protected]> wrote:
> > > On Thursday, January 17, 2013 05:34:32 PM Helmut Schaa wrote:
> > >> Since rt2800 hardware isn't capable of reporting the TX status of
> > >> BlockAckReq frames implement the TX status handling of BARs in
> > >> rt2x00lib. We keep track of all BARs that are send out and try to
> > >> match incoming BAs to the appropriate BARs. This allows us to report a
> > >> more or less accurate TX status for BAR frames which in turn improves
> > >> BA session stability.
> > > ---
> > > What about:
> > >> commit 5b632fe85ec82e5c43740b52e74c66df50a37db3
> > >> Author: Stanislaw Gruszka <[email protected]>
> > >> Date: Mon Dec 3 12:56:33 2012 +0100
> > >> mac80211: introduce IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL
> > >
> > > Should it be kept or dropped (as no driver uses it anymore?)
> >
> > I guess it should be dropped. I guess we could create a revert patch for that once
> > this patch has been merged.
> Looks like the rt2x00 patch has been accepted :)
> John, do you still want to remove the workaround, or
> can we keep it?
I'll post patch with revert.
Stanislaw
On Thursday, January 17, 2013 05:34:32 PM Helmut Schaa wrote:
> Since rt2800 hardware isn't capable of reporting the TX status of
> BlockAckReq frames implement the TX status handling of BARs in
> rt2x00lib. We keep track of all BARs that are send out and try to
> match incoming BAs to the appropriate BARs. This allows us to report a
> more or less accurate TX status for BAR frames which in turn improves
> BA session stability.
---
What about:
> commit 5b632fe85ec82e5c43740b52e74c66df50a37db3
> Author: Stanislaw Gruszka <[email protected]>
> Date: Mon Dec 3 12:56:33 2012 +0100
> mac80211: introduce IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL
Should it be kept or dropped (as no driver uses it anymore?)
Regards,
Chr
On Thu, Jan 17, 2013 at 7:42 PM, Gertjan van Wingerde
<[email protected]> wrote:
> On 17 jan. 2013, at 18:16, Christian Lamparter <[email protected]> wrote:
>
>> On Thursday, January 17, 2013 05:34:32 PM Helmut Schaa wrote:
>>> Since rt2800 hardware isn't capable of reporting the TX status of
>>> BlockAckReq frames implement the TX status handling of BARs in
>>> rt2x00lib. We keep track of all BARs that are send out and try to
>>> match incoming BAs to the appropriate BARs. This allows us to report a
>>> more or less accurate TX status for BAR frames which in turn improves
>>> BA session stability.
>> ---
>> What about:
>>> commit 5b632fe85ec82e5c43740b52e74c66df50a37db3
>>> Author: Stanislaw Gruszka <[email protected]>
>>> Date: Mon Dec 3 12:56:33 2012 +0100
>>> mac80211: introduce IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL
>>
>> Should it be kept or dropped (as no driver uses it anymore?)
>
> I guess it should be dropped. I guess we could create a revert patch for that once this patch has been merged.
Agreed, if it's unused now I'm in favor of reverting it.
Helmut
Sent from my iPad
On 17 jan. 2013, at 17:34, Helmut Schaa <[email protected]> wrote:
> Since rt2800 hardware isn't capable of reporting the TX status of
> BlockAckReq frames implement the TX status handling of BARs in
> rt2x00lib. We keep track of all BARs that are send out and try to
> match incoming BAs to the appropriate BARs. This allows us to report a
> more or less accurate TX status for BAR frames which in turn improves
> BA session stability.
>
> This is loosley based on Christian Lamparter's patch for carl9170
> "carl9170: fix HT peer BA session corruption".
>
> We have to walk the list of pending BARs for every rx'red BA even
> though most BAs don't belong to any of these BARs as they are just
> acknowledging an AMPDU. To keep that overhead low use RCU which allows
> us to walk the list of pending BARs without the need to acquire a lock.
> This however requires us to _copy_ relevant information from the BAR
> (RA, TA, control field, start sequence number) into our BAR list entry.
>
> Signed-off-by: Helmut Schaa <[email protected]>
Acked-by: Gertjan van Wingerde <[email protected]>
> ---
> drivers/net/wireless/rt2x00/rt2800lib.c | 6 +-
> drivers/net/wireless/rt2x00/rt2x00.h | 20 ++++++
> drivers/net/wireless/rt2x00/rt2x00dev.c | 101 ++++++++++++++++++++++++++++-
> drivers/net/wireless/rt2x00/rt2x00queue.c | 47 +++++++++++++
> 4 files changed, 169 insertions(+), 5 deletions(-)
>
> Andreas, if you like you can add your tested-by ...
>
> Thanks,
> Helmut
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index f139a91..a5c694f 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -1296,8 +1296,7 @@ void rt2800_config_filter(struct rt2x00_dev *rt2x00dev,
> !(filter_flags & FIF_CONTROL));
> rt2x00_set_field32(®, RX_FILTER_CFG_DROP_PSPOLL,
> !(filter_flags & FIF_PSPOLL));
> - rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BA,
> - !(filter_flags & FIF_CONTROL));
> + rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BA, 0);
> rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BAR,
> !(filter_flags & FIF_CONTROL));
> rt2x00_set_field32(®, RX_FILTER_CFG_DROP_CNTL,
> @@ -5146,8 +5145,7 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> IEEE80211_HW_SUPPORTS_PS |
> IEEE80211_HW_PS_NULLFUNC_STACK |
> IEEE80211_HW_AMPDU_AGGREGATION |
> - IEEE80211_HW_REPORTS_TX_ACK_STATUS |
> - IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL;
> + IEEE80211_HW_REPORTS_TX_ACK_STATUS;
>
> /*
> * Don't set IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING for USB devices
> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> index 0751b35..b52512b 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00.h
> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> @@ -1016,6 +1016,26 @@ struct rt2x00_dev {
> * Protect the interrupt mask register.
> */
> spinlock_t irqmask_lock;
> +
> + /*
> + * List of BlockAckReq TX entries that need driver BlockAck processing.
> + */
> + struct list_head bar_list;
> + spinlock_t bar_list_lock;
> +};
> +
> +struct rt2x00_bar_list_entry {
> + struct list_head list;
> + struct rcu_head head;
> +
> + struct queue_entry *entry;
> + int block_acked;
> +
> + /* Relevant parts of the IEEE80211 BAR header */
> + __u8 ra[6];
> + __u8 ta[6];
> + __le16 control;
> + __le16 start_seq_num;
> };
>
> /*
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 44f8b3f..b40a538 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -271,6 +271,50 @@ void rt2x00lib_dmadone(struct queue_entry *entry)
> }
> EXPORT_SYMBOL_GPL(rt2x00lib_dmadone);
>
> +static inline int rt2x00lib_txdone_bar_status(struct queue_entry *entry)
> +{
> + struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
> + struct ieee80211_bar *bar = (void *) entry->skb->data;
> + struct rt2x00_bar_list_entry *bar_entry;
> + int ret;
> +
> + if (likely(!ieee80211_is_back_req(bar->frame_control)))
> + return 0;
> +
> + /*
> + * Unlike all other frames, the status report for BARs does
> + * not directly come from the hardware as it is incapable of
> + * matching a BA to a previously send BAR. The hardware will
> + * report all BARs as if they weren't acked at all.
> + *
> + * Instead the RX-path will scan for incoming BAs and set the
> + * block_acked flag if it sees one that was likely caused by
> + * a BAR from us.
> + *
> + * Remove remaining BARs here and return their status for
> + * TX done processing.
> + */
> + ret = 0;
> + rcu_read_lock();
> + list_for_each_entry_rcu(bar_entry, &rt2x00dev->bar_list, list) {
> + if (bar_entry->entry != entry)
> + continue;
> +
> + spin_lock_bh(&rt2x00dev->bar_list_lock);
> + /* Return whether this BAR was blockacked or not */
> + ret = bar_entry->block_acked;
> + /* Remove the BAR from our checklist */
> + list_del_rcu(&bar_entry->list);
> + spin_unlock_bh(&rt2x00dev->bar_list_lock);
> + kfree_rcu(bar_entry, head);
> +
> + break;
> + }
> + rcu_read_unlock();
> +
> + return ret;
> +}
> +
> void rt2x00lib_txdone(struct queue_entry *entry,
> struct txdone_entry_desc *txdesc)
> {
> @@ -324,9 +368,12 @@ void rt2x00lib_txdone(struct queue_entry *entry,
> rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_TXDONE, entry->skb);
>
> /*
> - * Determine if the frame has been successfully transmitted.
> + * Determine if the frame has been successfully transmitted and
> + * remove BARs from our check list while checking for their
> + * TX status.
> */
> success =
> + rt2x00lib_txdone_bar_status(entry) ||
> test_bit(TXDONE_SUCCESS, &txdesc->flags) ||
> test_bit(TXDONE_UNKNOWN, &txdesc->flags);
>
> @@ -491,6 +538,50 @@ static void rt2x00lib_sleep(struct work_struct *work)
> IEEE80211_CONF_CHANGE_PS);
> }
>
> +static void rt2x00lib_rxdone_check_ba(struct rt2x00_dev *rt2x00dev,
> + struct sk_buff *skb,
> + struct rxdone_entry_desc *rxdesc)
> +{
> + struct rt2x00_bar_list_entry *entry;
> + struct ieee80211_bar *ba = (void *)skb->data;
> +
> + if (likely(!ieee80211_is_back(ba->frame_control)))
> + return;
> +
> + if (rxdesc->size < sizeof(*ba) + FCS_LEN)
> + return;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(entry, &rt2x00dev->bar_list, list) {
> +
> + if (ba->start_seq_num != entry->start_seq_num)
> + continue;
> +
> +#define TID_CHECK(a, b) ( \
> + ((a) & cpu_to_le16(IEEE80211_BAR_CTRL_TID_INFO_MASK)) == \
> + ((b) & cpu_to_le16(IEEE80211_BAR_CTRL_TID_INFO_MASK))) \
> +
> + if (!TID_CHECK(ba->control, entry->control))
> + continue;
> +
> +#undef TID_CHECK
> +
> + if (compare_ether_addr(ba->ra, entry->ta))
> + continue;
> +
> + if (compare_ether_addr(ba->ta, entry->ra))
> + continue;
> +
> + /* Mark BAR since we received the according BA */
> + spin_lock_bh(&rt2x00dev->bar_list_lock);
> + entry->block_acked = 1;
> + spin_unlock_bh(&rt2x00dev->bar_list_lock);
> + break;
> + }
> + rcu_read_unlock();
> +
> +}
> +
> static void rt2x00lib_rxdone_check_ps(struct rt2x00_dev *rt2x00dev,
> struct sk_buff *skb,
> struct rxdone_entry_desc *rxdesc)
> @@ -674,6 +765,12 @@ void rt2x00lib_rxdone(struct queue_entry *entry, gfp_t gfp)
> rt2x00lib_rxdone_check_ps(rt2x00dev, entry->skb, &rxdesc);
>
> /*
> + * Check for incoming BlockAcks to match to the BlockAckReqs
> + * we've send out.
> + */
> + rt2x00lib_rxdone_check_ba(rt2x00dev, entry->skb, &rxdesc);
> +
> + /*
> * Update extra components
> */
> rt2x00link_update_stats(rt2x00dev, entry->skb, &rxdesc);
> @@ -1183,6 +1280,8 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
>
> spin_lock_init(&rt2x00dev->irqmask_lock);
> mutex_init(&rt2x00dev->csr_mutex);
> + INIT_LIST_HEAD(&rt2x00dev->bar_list);
> + spin_lock_init(&rt2x00dev->bar_list_lock);
>
> set_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
> index e488b94..f35d85a 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
> @@ -582,6 +582,48 @@ static void rt2x00queue_kick_tx_queue(struct data_queue *queue,
> queue->rt2x00dev->ops->lib->kick_queue(queue);
> }
>
> +static void rt2x00queue_bar_check(struct queue_entry *entry)
> +{
> + struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
> + struct ieee80211_bar *bar = (void *) (entry->skb->data +
> + rt2x00dev->ops->extra_tx_headroom);
> + struct rt2x00_bar_list_entry *bar_entry;
> +
> + if (likely(!ieee80211_is_back_req(bar->frame_control)))
> + return;
> +
> + bar_entry = kmalloc(sizeof(*bar_entry), GFP_ATOMIC);
> +
> + /*
> + * If the alloc fails we still send the BAR out but just don't track
> + * it in our bar list. And as a result we will report it to mac80211
> + * back as failed.
> + */
> + if (!bar_entry)
> + return;
> +
> + bar_entry->entry = entry;
> + bar_entry->block_acked = 0;
> +
> + /*
> + * Copy the relevant parts of the 802.11 BAR into out check list
> + * such that we can use RCU for less-overhead in the RX path since
> + * sending BARs and processing the according BlockAck should be
> + * the exception.
> + */
> + memcpy(bar_entry->ra, bar->ra, sizeof(bar->ra));
> + memcpy(bar_entry->ta, bar->ta, sizeof(bar->ta));
> + bar_entry->control = bar->control;
> + bar_entry->start_seq_num = bar->start_seq_num;
> +
> + /*
> + * Insert BAR into our BAR check list.
> + */
> + spin_lock_bh(&rt2x00dev->bar_list_lock);
> + list_add_tail_rcu(&bar_entry->list, &rt2x00dev->bar_list);
> + spin_unlock_bh(&rt2x00dev->bar_list_lock);
> +}
> +
> int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
> bool local)
> {
> @@ -680,6 +722,11 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
> goto out;
> }
>
> + /*
> + * Put BlockAckReqs into our check list for driver BA processing.
> + */
> + rt2x00queue_bar_check(entry);
> +
> set_bit(ENTRY_DATA_PENDING, &entry->flags);
>
> rt2x00queue_index_inc(entry, Q_INDEX);
> --
> 1.7.7
>
Sent from my iPad
On 17 jan. 2013, at 18:16, Christian Lamparter <[email protected]> wrote:
> On Thursday, January 17, 2013 05:34:32 PM Helmut Schaa wrote:
>> Since rt2800 hardware isn't capable of reporting the TX status of
>> BlockAckReq frames implement the TX status handling of BARs in
>> rt2x00lib. We keep track of all BARs that are send out and try to
>> match incoming BAs to the appropriate BARs. This allows us to report a
>> more or less accurate TX status for BAR frames which in turn improves
>> BA session stability.
> ---
> What about:
>> commit 5b632fe85ec82e5c43740b52e74c66df50a37db3
>> Author: Stanislaw Gruszka <[email protected]>
>> Date: Mon Dec 3 12:56:33 2012 +0100
>> mac80211: introduce IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL
>
> Should it be kept or dropped (as no driver uses it anymore?)
I guess it should be dropped. I guess we could create a revert patch for that once this patch has been merged.
---
Gertjan
On Thursday 17 January 2013 19:42:52 Gertjan van Wingerde wrote:
> On 17 jan. 2013, at 18:16, Christian Lamparter <[email protected]> wrote:
> > On Thursday, January 17, 2013 05:34:32 PM Helmut Schaa wrote:
> >> Since rt2800 hardware isn't capable of reporting the TX status of
> >> BlockAckReq frames implement the TX status handling of BARs in
> >> rt2x00lib. We keep track of all BARs that are send out and try to
> >> match incoming BAs to the appropriate BARs. This allows us to report a
> >> more or less accurate TX status for BAR frames which in turn improves
> >> BA session stability.
> > ---
> > What about:
> >> commit 5b632fe85ec82e5c43740b52e74c66df50a37db3
> >> Author: Stanislaw Gruszka <[email protected]>
> >> Date: Mon Dec 3 12:56:33 2012 +0100
> >> mac80211: introduce IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL
> >
> > Should it be kept or dropped (as no driver uses it anymore?)
>
> I guess it should be dropped. I guess we could create a revert patch for that once
> this patch has been merged.
Looks like the rt2x00 patch has been accepted :)
John, do you still want to remove the workaround, or
can we keep it?
Regards,
Chr
On Thu, 17 Jan 2013 17:34:32 +0100
Helmut Schaa <[email protected]> wrote:
> Since rt2800 hardware isn't capable of reporting the TX status of
> BlockAckReq frames implement the TX status handling of BARs in
> rt2x00lib. We keep track of all BARs that are send out and try to
> match incoming BAs to the appropriate BARs. This allows us to report a
> more or less accurate TX status for BAR frames which in turn improves
> BA session stability.
>
> This is loosley based on Christian Lamparter's patch for carl9170
> "carl9170: fix HT peer BA session corruption".
>
> We have to walk the list of pending BARs for every rx'red BA even
> though most BAs don't belong to any of these BARs as they are just
> acknowledging an AMPDU. To keep that overhead low use RCU which allows
> us to walk the list of pending BARs without the need to acquire a lock.
> This however requires us to _copy_ relevant information from the BAR
> (RA, TA, control field, start sequence number) into our BAR list entry.
>
> Signed-off-by: Helmut Schaa <[email protected]>
Tested-by: Andreas Hartmann <[email protected]>
> ---
> drivers/net/wireless/rt2x00/rt2800lib.c | 6 +-
> drivers/net/wireless/rt2x00/rt2x00.h | 20 ++++++
> drivers/net/wireless/rt2x00/rt2x00dev.c | 101 ++++++++++++++++++++++++++++-
> drivers/net/wireless/rt2x00/rt2x00queue.c | 47 +++++++++++++
> 4 files changed, 169 insertions(+), 5 deletions(-)
>
> Andreas, if you like you can add your tested-by ...
>
> Thanks,
> Helmut
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index f139a91..a5c694f 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -1296,8 +1296,7 @@ void rt2800_config_filter(struct rt2x00_dev *rt2x00dev,
> !(filter_flags & FIF_CONTROL));
> rt2x00_set_field32(®, RX_FILTER_CFG_DROP_PSPOLL,
> !(filter_flags & FIF_PSPOLL));
> - rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BA,
> - !(filter_flags & FIF_CONTROL));
> + rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BA, 0);
> rt2x00_set_field32(®, RX_FILTER_CFG_DROP_BAR,
> !(filter_flags & FIF_CONTROL));
> rt2x00_set_field32(®, RX_FILTER_CFG_DROP_CNTL,
> @@ -5146,8 +5145,7 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> IEEE80211_HW_SUPPORTS_PS |
> IEEE80211_HW_PS_NULLFUNC_STACK |
> IEEE80211_HW_AMPDU_AGGREGATION |
> - IEEE80211_HW_REPORTS_TX_ACK_STATUS |
> - IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL;
> + IEEE80211_HW_REPORTS_TX_ACK_STATUS;
>
> /*
> * Don't set IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING for USB devices
> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> index 0751b35..b52512b 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00.h
> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> @@ -1016,6 +1016,26 @@ struct rt2x00_dev {
> * Protect the interrupt mask register.
> */
> spinlock_t irqmask_lock;
> +
> + /*
> + * List of BlockAckReq TX entries that need driver BlockAck processing.
> + */
> + struct list_head bar_list;
> + spinlock_t bar_list_lock;
> +};
> +
> +struct rt2x00_bar_list_entry {
> + struct list_head list;
> + struct rcu_head head;
> +
> + struct queue_entry *entry;
> + int block_acked;
> +
> + /* Relevant parts of the IEEE80211 BAR header */
> + __u8 ra[6];
> + __u8 ta[6];
> + __le16 control;
> + __le16 start_seq_num;
> };
>
> /*
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 44f8b3f..b40a538 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -271,6 +271,50 @@ void rt2x00lib_dmadone(struct queue_entry *entry)
> }
> EXPORT_SYMBOL_GPL(rt2x00lib_dmadone);
>
> +static inline int rt2x00lib_txdone_bar_status(struct queue_entry *entry)
> +{
> + struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
> + struct ieee80211_bar *bar = (void *) entry->skb->data;
> + struct rt2x00_bar_list_entry *bar_entry;
> + int ret;
> +
> + if (likely(!ieee80211_is_back_req(bar->frame_control)))
> + return 0;
> +
> + /*
> + * Unlike all other frames, the status report for BARs does
> + * not directly come from the hardware as it is incapable of
> + * matching a BA to a previously send BAR. The hardware will
> + * report all BARs as if they weren't acked at all.
> + *
> + * Instead the RX-path will scan for incoming BAs and set the
> + * block_acked flag if it sees one that was likely caused by
> + * a BAR from us.
> + *
> + * Remove remaining BARs here and return their status for
> + * TX done processing.
> + */
> + ret = 0;
> + rcu_read_lock();
> + list_for_each_entry_rcu(bar_entry, &rt2x00dev->bar_list, list) {
> + if (bar_entry->entry != entry)
> + continue;
> +
> + spin_lock_bh(&rt2x00dev->bar_list_lock);
> + /* Return whether this BAR was blockacked or not */
> + ret = bar_entry->block_acked;
> + /* Remove the BAR from our checklist */
> + list_del_rcu(&bar_entry->list);
> + spin_unlock_bh(&rt2x00dev->bar_list_lock);
> + kfree_rcu(bar_entry, head);
> +
> + break;
> + }
> + rcu_read_unlock();
> +
> + return ret;
> +}
> +
> void rt2x00lib_txdone(struct queue_entry *entry,
> struct txdone_entry_desc *txdesc)
> {
> @@ -324,9 +368,12 @@ void rt2x00lib_txdone(struct queue_entry *entry,
> rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_TXDONE, entry->skb);
>
> /*
> - * Determine if the frame has been successfully transmitted.
> + * Determine if the frame has been successfully transmitted and
> + * remove BARs from our check list while checking for their
> + * TX status.
> */
> success =
> + rt2x00lib_txdone_bar_status(entry) ||
> test_bit(TXDONE_SUCCESS, &txdesc->flags) ||
> test_bit(TXDONE_UNKNOWN, &txdesc->flags);
>
> @@ -491,6 +538,50 @@ static void rt2x00lib_sleep(struct work_struct *work)
> IEEE80211_CONF_CHANGE_PS);
> }
>
> +static void rt2x00lib_rxdone_check_ba(struct rt2x00_dev *rt2x00dev,
> + struct sk_buff *skb,
> + struct rxdone_entry_desc *rxdesc)
> +{
> + struct rt2x00_bar_list_entry *entry;
> + struct ieee80211_bar *ba = (void *)skb->data;
> +
> + if (likely(!ieee80211_is_back(ba->frame_control)))
> + return;
> +
> + if (rxdesc->size < sizeof(*ba) + FCS_LEN)
> + return;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(entry, &rt2x00dev->bar_list, list) {
> +
> + if (ba->start_seq_num != entry->start_seq_num)
> + continue;
> +
> +#define TID_CHECK(a, b) ( \
> + ((a) & cpu_to_le16(IEEE80211_BAR_CTRL_TID_INFO_MASK)) == \
> + ((b) & cpu_to_le16(IEEE80211_BAR_CTRL_TID_INFO_MASK))) \
> +
> + if (!TID_CHECK(ba->control, entry->control))
> + continue;
> +
> +#undef TID_CHECK
> +
> + if (compare_ether_addr(ba->ra, entry->ta))
> + continue;
> +
> + if (compare_ether_addr(ba->ta, entry->ra))
> + continue;
> +
> + /* Mark BAR since we received the according BA */
> + spin_lock_bh(&rt2x00dev->bar_list_lock);
> + entry->block_acked = 1;
> + spin_unlock_bh(&rt2x00dev->bar_list_lock);
> + break;
> + }
> + rcu_read_unlock();
> +
> +}
> +
> static void rt2x00lib_rxdone_check_ps(struct rt2x00_dev *rt2x00dev,
> struct sk_buff *skb,
> struct rxdone_entry_desc *rxdesc)
> @@ -674,6 +765,12 @@ void rt2x00lib_rxdone(struct queue_entry *entry, gfp_t gfp)
> rt2x00lib_rxdone_check_ps(rt2x00dev, entry->skb, &rxdesc);
>
> /*
> + * Check for incoming BlockAcks to match to the BlockAckReqs
> + * we've send out.
> + */
> + rt2x00lib_rxdone_check_ba(rt2x00dev, entry->skb, &rxdesc);
> +
> + /*
> * Update extra components
> */
> rt2x00link_update_stats(rt2x00dev, entry->skb, &rxdesc);
> @@ -1183,6 +1280,8 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
>
> spin_lock_init(&rt2x00dev->irqmask_lock);
> mutex_init(&rt2x00dev->csr_mutex);
> + INIT_LIST_HEAD(&rt2x00dev->bar_list);
> + spin_lock_init(&rt2x00dev->bar_list_lock);
>
> set_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
> index e488b94..f35d85a 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
> @@ -582,6 +582,48 @@ static void rt2x00queue_kick_tx_queue(struct data_queue *queue,
> queue->rt2x00dev->ops->lib->kick_queue(queue);
> }
>
> +static void rt2x00queue_bar_check(struct queue_entry *entry)
> +{
> + struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
> + struct ieee80211_bar *bar = (void *) (entry->skb->data +
> + rt2x00dev->ops->extra_tx_headroom);
> + struct rt2x00_bar_list_entry *bar_entry;
> +
> + if (likely(!ieee80211_is_back_req(bar->frame_control)))
> + return;
> +
> + bar_entry = kmalloc(sizeof(*bar_entry), GFP_ATOMIC);
> +
> + /*
> + * If the alloc fails we still send the BAR out but just don't track
> + * it in our bar list. And as a result we will report it to mac80211
> + * back as failed.
> + */
> + if (!bar_entry)
> + return;
> +
> + bar_entry->entry = entry;
> + bar_entry->block_acked = 0;
> +
> + /*
> + * Copy the relevant parts of the 802.11 BAR into out check list
> + * such that we can use RCU for less-overhead in the RX path since
> + * sending BARs and processing the according BlockAck should be
> + * the exception.
> + */
> + memcpy(bar_entry->ra, bar->ra, sizeof(bar->ra));
> + memcpy(bar_entry->ta, bar->ta, sizeof(bar->ta));
> + bar_entry->control = bar->control;
> + bar_entry->start_seq_num = bar->start_seq_num;
> +
> + /*
> + * Insert BAR into our BAR check list.
> + */
> + spin_lock_bh(&rt2x00dev->bar_list_lock);
> + list_add_tail_rcu(&bar_entry->list, &rt2x00dev->bar_list);
> + spin_unlock_bh(&rt2x00dev->bar_list_lock);
> +}
> +
> int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
> bool local)
> {
> @@ -680,6 +722,11 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
> goto out;
> }
>
> + /*
> + * Put BlockAckReqs into our check list for driver BA processing.
> + */
> + rt2x00queue_bar_check(entry);
> +
> set_bit(ENTRY_DATA_PENDING, &entry->flags);
>
> rt2x00queue_index_inc(entry, Q_INDEX);