2008-10-21 08:30:30

by Sujith

[permalink] [raw]
Subject: [PATCH v3] mac80211: Re-enable aggregation

Wireless HW without any dedicated queues for aggregation
do not need the ampdu_queues mechanism present right now
in mac80211. Since mac80211 is still incomplete wrt TX MQ
changes, do not allow aggregation sessions for drivers that
set ampdu_queues.

This is only an interim hack until Intel fixes the requeue issue.

Signed-off-by: Sujith <[email protected]>
Signed-off-by: Luis Rodriguez <[email protected]>
---
V2:

Remove is_part_ampdu from struct sk_buff, and set IEEE80211_TX_CTL_AMPDU
in __ieee80211_tx_prepare() - which I think is a better place than ieee80211_tx_h_sequence().

Also, add requeue to __skb_clone() - pointed out by Jouni.

V3:

Add a HW flag IEEE80211_HW_AMPDU_AGGREGATION and check in
ieee80211_start_tx_ba_session().
Remove ampdu_queues initialization in ath9k.

Tomas - can you verify if hw->flags in iwl-core.c is the only place that needs
to be set ?

drivers/net/wireless/ath9k/main.c | 6 +--
drivers/net/wireless/iwlwifi/iwl-core.c | 3 +-
include/linux/skbuff.h | 4 ++
include/net/mac80211.h | 6 ++-
net/core/skbuff.c | 1 +
net/mac80211/ht.c | 60 ++++++++++++++++++-------------
net/mac80211/main.c | 7 +---
net/mac80211/rx.c | 7 +---
net/mac80211/tx.c | 19 +++++++---
net/mac80211/wme.c | 24 +++++-------
10 files changed, 76 insertions(+), 61 deletions(-)

diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
index 56552a9..e614fa5 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -962,10 +962,7 @@ static int ath_attach(u16 devid,
&sc->sbands[IEEE80211_BAND_5GHZ];
}

- /* FIXME: Have to figure out proper hw init values later */
-
hw->queues = 4;
- hw->ampdu_queues = 1;

/* Register rate control */
hw->rate_control_algorithm = "ath9k_rate_control";
@@ -1746,7 +1743,8 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
hw->flags = IEEE80211_HW_RX_INCLUDES_FCS |
IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
IEEE80211_HW_SIGNAL_DBM |
- IEEE80211_HW_NOISE_DBM;
+ IEEE80211_HW_NOISE_DBM |
+ IEEE80211_HW_AMPDU_AGGREGATION;

hw->wiphy->interface_modes =
BIT(NL80211_IFTYPE_AP) |
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
index 4678da4..bc53d3a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.c
+++ b/drivers/net/wireless/iwlwifi/iwl-core.c
@@ -869,7 +869,8 @@ int iwl_setup_mac(struct iwl_priv *priv)

/* Tell mac80211 our characteristics */
hw->flags = IEEE80211_HW_SIGNAL_DBM |
- IEEE80211_HW_NOISE_DBM;
+ IEEE80211_HW_NOISE_DBM |
+ IEEE80211_HW_AMPDU_AGGREGATION;
hw->wiphy->interface_modes =
BIT(NL80211_IFTYPE_AP) |
BIT(NL80211_IFTYPE_STATION) |
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9099237..4724211 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -244,6 +244,9 @@ typedef unsigned char *sk_buff_data_t;
* @tc_verd: traffic control verdict
* @ndisc_nodetype: router type (from link layer)
* @do_not_encrypt: set to prevent encryption of this frame
+ * @requeue: set to indicate that the wireless core should attempt
+ * a software retry on this frame if we failed to
+ * receive an ACK for it
* @dma_cookie: a cookie to one of several possible DMA operations
* done by skb DMA functions
* @secmark: security marking
@@ -319,6 +322,7 @@ struct sk_buff {
#endif
#if defined(CONFIG_MAC80211) || defined(CONFIG_MAC80211_MODULE)
__u8 do_not_encrypt:1;
+ __u8 requeue:1;
#endif
/* 0/13/14 bit hole */

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index afcdfaa..a52a486 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -229,7 +229,6 @@ struct ieee80211_bss_conf {
* @IEEE80211_TX_CTL_RATE_CTRL_PROBE: TBD
* @IEEE80211_TX_CTL_CLEAR_PS_FILT: clear powersave filter for destination
* station
- * @IEEE80211_TX_CTL_REQUEUE: TBD
* @IEEE80211_TX_CTL_FIRST_FRAGMENT: this is a first fragment of the frame
* @IEEE80211_TX_CTL_SHORT_PREAMBLE: TBD
* @IEEE80211_TX_CTL_LONG_RETRY_LIMIT: this frame should be send using the
@@ -271,7 +270,6 @@ enum mac80211_tx_control_flags {
IEEE80211_TX_CTL_NO_ACK = BIT(4),
IEEE80211_TX_CTL_RATE_CTRL_PROBE = BIT(5),
IEEE80211_TX_CTL_CLEAR_PS_FILT = BIT(6),
- IEEE80211_TX_CTL_REQUEUE = BIT(7),
IEEE80211_TX_CTL_FIRST_FRAGMENT = BIT(8),
IEEE80211_TX_CTL_SHORT_PREAMBLE = BIT(9),
IEEE80211_TX_CTL_LONG_RETRY_LIMIT = BIT(10),
@@ -794,6 +792,9 @@ enum ieee80211_tkip_key_type {
* @IEEE80211_HW_SPECTRUM_MGMT:
* Hardware supports spectrum management defined in 802.11h
* Measurement, Channel Switch, Quieting, TPC
+ *
+ * @IEEE80211_HW_AMPDU_AGGREGATION:
+ * Hardware supports 11n A-MPDU aggregation.
*/
enum ieee80211_hw_flags {
IEEE80211_HW_RX_INCLUDES_FCS = 1<<1,
@@ -805,6 +806,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SIGNAL_DBM = 1<<7,
IEEE80211_HW_NOISE_DBM = 1<<8,
IEEE80211_HW_SPECTRUM_MGMT = 1<<9,
+ IEEE80211_HW_AMPDU_AGGREGATION = 1<<10,
};

/**
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ca1ccdf..b0acc86 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -487,6 +487,7 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
C(truesize);
#if defined(CONFIG_MAC80211) || defined(CONFIG_MAC80211_MODULE)
C(do_not_encrypt);
+ C(requeue);
#endif
atomic_set(&n->users, 1);

diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index d09e8bf..876f4c6 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -492,7 +492,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
int ret;
DECLARE_MAC_BUF(mac);

- if (tid >= STA_TID_NUM)
+ if ((tid >= STA_TID_NUM) || !(hw->flags & IEEE80211_HW_AMPDU_AGGREGATION))
return -EINVAL;

#ifdef CONFIG_MAC80211_HT_DEBUG
@@ -549,17 +549,19 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
(unsigned long)&sta->timer_to_tid[tid];
init_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);

- /* create a new queue for this aggregation */
- ret = ieee80211_ht_agg_queue_add(local, sta, tid);
+ if (hw->ampdu_queues) {
+ /* 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) {
+ /* 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 - queue unavailable for"
- " tid %d\n", tid);
+ printk(KERN_DEBUG "BA request denied - "
+ "queue unavailable for tid %d\n", tid);
#endif /* CONFIG_MAC80211_HT_DEBUG */
- goto err_unlock_queue;
+ goto err_unlock_queue;
+ }
}
sdata = sta->sdata;

@@ -578,7 +580,8 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
/* 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);
+ if (hw->ampdu_queues)
+ ieee80211_ht_agg_queue_remove(local, sta, tid, 0);
#ifdef CONFIG_MAC80211_HT_DEBUG
printk(KERN_DEBUG "BA request denied - HW unavailable for"
" tid %d\n", tid);
@@ -588,7 +591,8 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
}

/* Will put all the packets in the new SW queue */
- ieee80211_requeue(local, ieee802_1d_to_ac[tid]);
+ if (hw->ampdu_queues)
+ ieee80211_requeue(local, ieee802_1d_to_ac[tid]);
spin_unlock_bh(&sta->lock);

/* send an addBA request */
@@ -657,7 +661,8 @@ int ieee80211_stop_tx_ba_session(struct ieee80211_hw *hw,
print_mac(mac, ra), tid);
#endif /* CONFIG_MAC80211_HT_DEBUG */

- ieee80211_stop_queue(hw, sta->tid_to_tx_q[tid]);
+ if (hw->ampdu_queues)
+ ieee80211_stop_queue(hw, sta->tid_to_tx_q[tid]);

*state = HT_AGG_STATE_REQ_STOP_BA_MSK |
(initiator << HT_AGG_STATE_INITIATOR_SHIFT);
@@ -670,7 +675,8 @@ int ieee80211_stop_tx_ba_session(struct ieee80211_hw *hw,
if (ret) {
WARN_ON(ret != -EBUSY);
*state = HT_AGG_STATE_OPERATIONAL;
- ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
+ if (hw->ampdu_queues)
+ ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
goto stop_BA_exit;
}

@@ -728,7 +734,8 @@ void ieee80211_start_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u16 tid)
#ifdef CONFIG_MAC80211_HT_DEBUG
printk(KERN_DEBUG "Aggregation is on for tid %d \n", tid);
#endif
- ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
+ if (hw->ampdu_queues)
+ ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);
}
spin_unlock_bh(&sta->lock);
rcu_read_unlock();
@@ -784,16 +791,18 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u8 tid)
ieee80211_send_delba(sta->sdata, ra, tid,
WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE);

- agg_queue = sta->tid_to_tx_q[tid];
-
- ieee80211_ht_agg_queue_remove(local, sta, tid, 1);
-
- /* We just requeued the all the frames that were in the
- * removed queue, and since we might miss a softirq we do
- * netif_schedule_queue. ieee80211_wake_queue is not used
- * here as this queue is not necessarily stopped
- */
- netif_schedule_queue(netdev_get_tx_queue(local->mdev, agg_queue));
+ if (hw->ampdu_queues) {
+ agg_queue = sta->tid_to_tx_q[tid];
+ ieee80211_ht_agg_queue_remove(local, sta, tid, 1);
+
+ /* We just requeued the all the frames that were in the
+ * removed queue, and since we might miss a softirq we do
+ * netif_schedule_queue. ieee80211_wake_queue is not used
+ * here as this queue is not necessarily stopped
+ */
+ netif_schedule_queue(netdev_get_tx_queue(local->mdev,
+ agg_queue));
+ }
spin_lock_bh(&sta->lock);
*state = HT_AGG_STATE_IDLE;
sta->ampdu_mlme.addba_req_num[tid] = 0;
@@ -1050,7 +1059,8 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local,
*state |= HT_ADDBA_RECEIVED_MSK;
sta->ampdu_mlme.addba_req_num[tid] = 0;

- if (*state == HT_AGG_STATE_OPERATIONAL)
+ if (*state == HT_AGG_STATE_OPERATIONAL &&
+ local->hw.ampdu_queues)
ieee80211_wake_queue(hw, sta->tid_to_tx_q[tid]);

spin_unlock_bh(&sta->lock);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index c936017..4883aa9 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -382,8 +382,6 @@ static void ieee80211_handle_filtered_frame(struct ieee80211_local *local,
struct sta_info *sta,
struct sk_buff *skb)
{
- struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-
sta->tx_filtered_count++;

/*
@@ -430,10 +428,9 @@ static void ieee80211_handle_filtered_frame(struct ieee80211_local *local,
return;
}

- if (!test_sta_flags(sta, WLAN_STA_PS) &&
- !(info->flags & IEEE80211_TX_CTL_REQUEUE)) {
+ if (!test_sta_flags(sta, WLAN_STA_PS) && !skb->requeue) {
/* Software retry the packet once */
- info->flags |= IEEE80211_TX_CTL_REQUEUE;
+ skb->requeue = 1;
ieee80211_remove_tx_extra(local, sta->key, skb);
dev_queue_xmit(skb);
return;
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index a0db162..7c3702c 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -670,7 +670,6 @@ static int ap_sta_ps_end(struct sta_info *sta)
struct ieee80211_local *local = sdata->local;
struct sk_buff *skb;
int sent = 0;
- struct ieee80211_tx_info *info;
DECLARE_MAC_BUF(mac);

atomic_dec(&sdata->bss->num_sta_ps);
@@ -687,13 +686,11 @@ static int ap_sta_ps_end(struct sta_info *sta)

/* Send all buffered frames to the station */
while ((skb = skb_dequeue(&sta->tx_filtered)) != NULL) {
- info = IEEE80211_SKB_CB(skb);
sent++;
- info->flags |= IEEE80211_TX_CTL_REQUEUE;
+ skb->requeue = 1;
dev_queue_xmit(skb);
}
while ((skb = skb_dequeue(&sta->ps_tx_buf)) != NULL) {
- info = IEEE80211_SKB_CB(skb);
local->total_ps_buffered--;
sent++;
#ifdef CONFIG_MAC80211_VERBOSE_PS_DEBUG
@@ -701,7 +698,7 @@ static int ap_sta_ps_end(struct sta_info *sta)
"since STA not sleeping anymore\n", sdata->dev->name,
print_mac(mac, sta->sta.addr), sta->sta.aid);
#endif /* CONFIG_MAC80211_VERBOSE_PS_DEBUG */
- info->flags |= IEEE80211_TX_CTL_REQUEUE;
+ skb->requeue = 1;
dev_queue_xmit(skb);
}

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index cdedd37..7a1c8a6 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -642,6 +642,7 @@ ieee80211_tx_h_sequence(struct ieee80211_tx_data *tx)
static ieee80211_tx_result debug_noinline
ieee80211_tx_h_fragment(struct ieee80211_tx_data *tx)
{
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
size_t hdrlen, per_fragm, num_fragm, payload_len, left;
struct sk_buff **frags, *first, *frag;
@@ -658,9 +659,7 @@ ieee80211_tx_h_fragment(struct ieee80211_tx_data *tx)
* This scenario is handled in __ieee80211_tx_prepare but extra
* caution taken here as fragmented ampdu may cause Tx stop.
*/
- if (WARN_ON(tx->flags & IEEE80211_TX_CTL_AMPDU ||
- skb_get_queue_mapping(tx->skb) >=
- ieee80211_num_regular_queues(&tx->local->hw)))
+ if (WARN_ON(info->flags & IEEE80211_TX_CTL_AMPDU))
return TX_DROP;

first = tx->skb;
@@ -943,7 +942,8 @@ __ieee80211_tx_prepare(struct ieee80211_tx_data *tx,
struct ieee80211_sub_if_data *sdata;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);

- int hdrlen;
+ int hdrlen, tid;
+ u8 *qc, *state;

memset(tx, 0, sizeof(*tx));
tx->skb = skb;
@@ -976,6 +976,15 @@ __ieee80211_tx_prepare(struct ieee80211_tx_data *tx,

tx->sta = sta_info_get(local, hdr->addr1);

+ if (tx->sta && ieee80211_is_data_qos(hdr->frame_control)) {
+ qc = ieee80211_get_qos_ctl(hdr);
+ tid = *qc & IEEE80211_QOS_CTL_TID_MASK;
+
+ state = &tx->sta->ampdu_mlme.tid_state_tx[tid];
+ if (*state == HT_AGG_STATE_OPERATIONAL)
+ info->flags |= IEEE80211_TX_CTL_AMPDU;
+ }
+
if (is_multicast_ether_addr(hdr->addr1)) {
tx->flags &= ~IEEE80211_TX_UNICAST;
info->flags |= IEEE80211_TX_CTL_NO_ACK;
@@ -1178,7 +1187,7 @@ retry:
* queues, there's no reason for a driver to reject
* a frame there, warn and drop it.
*/
- if (WARN_ON(queue >= ieee80211_num_regular_queues(&local->hw)))
+ if (WARN_ON(info->flags & IEEE80211_TX_CTL_AMPDU))
goto drop;

store = &local->pending_packet[queue];
diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index f10f770..3247312 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -114,8 +114,8 @@ u16 ieee80211_select_queue(struct net_device *dev, struct sk_buff *skb)
{
struct ieee80211_master_priv *mpriv = netdev_priv(dev);
struct ieee80211_local *local = mpriv->local;
+ struct ieee80211_hw *hw = &local->hw;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
- struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct sta_info *sta;
u16 queue;
u8 tid;
@@ -124,21 +124,19 @@ u16 ieee80211_select_queue(struct net_device *dev, struct sk_buff *skb)
if (unlikely(queue >= local->hw.queues))
queue = local->hw.queues - 1;

- if (info->flags & IEEE80211_TX_CTL_REQUEUE) {
+ if (skb->requeue) {
+ if (!hw->ampdu_queues)
+ return queue;
+
rcu_read_lock();
sta = sta_info_get(local, hdr->addr1);
tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
if (sta) {
- struct ieee80211_hw *hw = &local->hw;
int ampdu_queue = sta->tid_to_tx_q[tid];

if ((ampdu_queue < ieee80211_num_queues(hw)) &&
- test_bit(ampdu_queue, local->queue_pool)) {
+ test_bit(ampdu_queue, local->queue_pool))
queue = ampdu_queue;
- info->flags |= IEEE80211_TX_CTL_AMPDU;
- } else {
- info->flags &= ~IEEE80211_TX_CTL_AMPDU;
- }
}
rcu_read_unlock();

@@ -159,20 +157,18 @@ u16 ieee80211_select_queue(struct net_device *dev, struct sk_buff *skb)
*p++ = ack_policy | tid;
*p = 0;

+ if (!hw->ampdu_queues)
+ return queue;
+
rcu_read_lock();

sta = sta_info_get(local, hdr->addr1);
if (sta) {
int ampdu_queue = sta->tid_to_tx_q[tid];
- struct ieee80211_hw *hw = &local->hw;

if ((ampdu_queue < ieee80211_num_queues(hw)) &&
- test_bit(ampdu_queue, local->queue_pool)) {
+ test_bit(ampdu_queue, local->queue_pool))
queue = ampdu_queue;
- info->flags |= IEEE80211_TX_CTL_AMPDU;
- } else {
- info->flags &= ~IEEE80211_TX_CTL_AMPDU;
- }
}

rcu_read_unlock();
--
1.6.0.2



2008-10-23 14:58:39

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v3] mac80211: Re-enable aggregation

On Wed, Oct 22, 2008 at 08:10:42PM -0700, Luis R. Rodriguez wrote:
> On Tue, Oct 21, 2008 at 1:27 AM, Sujith <[email protected]> wrote:
> > Wireless HW without any dedicated queues for aggregation
> > do not need the ampdu_queues mechanism present right now
> > in mac80211. Since mac80211 is still incomplete wrt TX MQ
> > changes, do not allow aggregation sessions for drivers that
> > set ampdu_queues.
> >
> > This is only an interim hack until Intel fixes the requeue issue.
> >
> > Signed-off-by: Sujith <[email protected]>
> > Signed-off-by: Luis Rodriguez <[email protected]>
>
> Poke.

Just waiting to see comments from those who understand aggregation
better than I do...

John
--
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.

2008-10-23 15:00:55

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3] mac80211: Re-enable aggregation

On Thu, 2008-10-23 at 10:55 -0400, John W. Linville wrote:
> On Wed, Oct 22, 2008 at 08:10:42PM -0700, Luis R. Rodriguez wrote:
> > On Tue, Oct 21, 2008 at 1:27 AM, Sujith <[email protected]> wrote:
> > > Wireless HW without any dedicated queues for aggregation
> > > do not need the ampdu_queues mechanism present right now
> > > in mac80211. Since mac80211 is still incomplete wrt TX MQ
> > > changes, do not allow aggregation sessions for drivers that
> > > set ampdu_queues.
> > >
> > > This is only an interim hack until Intel fixes the requeue issue.
> > >
> > > Signed-off-by: Sujith <[email protected]>
> > > Signed-off-by: Luis Rodriguez <[email protected]>
> >
> > Poke.
>
> Just waiting to see comments from those who understand aggregation
> better than I do...

Who does? Code looks fine to me, but obviously it doesn't really solve
the problem, just adds workarounds for the current ath9k driver. Whether
you want that or not I'll leave up to you.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-23 03:10:44

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v3] mac80211: Re-enable aggregation

On Tue, Oct 21, 2008 at 1:27 AM, Sujith <[email protected]> wrote:
> Wireless HW without any dedicated queues for aggregation
> do not need the ampdu_queues mechanism present right now
> in mac80211. Since mac80211 is still incomplete wrt TX MQ
> changes, do not allow aggregation sessions for drivers that
> set ampdu_queues.
>
> This is only an interim hack until Intel fixes the requeue issue.
>
> Signed-off-by: Sujith <[email protected]>
> Signed-off-by: Luis Rodriguez <[email protected]>

Poke.

Luis

2008-10-23 16:54:58

by Sujith

[permalink] [raw]
Subject: Re: [PATCH v3] mac80211: Re-enable aggregation

John W. Linville wrote:
> > Poke.
>
> Just waiting to see comments from those who understand aggregation
> better than I do...

It doesn't change the existing behaviour in any way, ath9k doesn't need
all the extra queue stuff, so this is only a temporary fix until
we have a decent model in mac80211 to support HW with no ampdu_queues.

But that patch probably won't apply now, I'll rebase it and send again.

Sujith

2008-10-23 17:33:04

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v3] mac80211: Re-enable aggregation

On Thu, Oct 23, 2008 at 9:52 AM, Sujith <[email protected]> wrote:
> John W. Linville wrote:
>> > Poke.
>>
>> Just waiting to see comments from those who understand aggregation
>> better than I do...
>
> It doesn't change the existing behaviour in any way, ath9k doesn't need
> all the extra queue stuff, so this is only a temporary fix until
> we have a decent model in mac80211 to support HW with
> no ampdu_queues.

Just to give some perspective, the reason this has been painful is we
are dealing with the two most distinct type of wireless devices you
could get and we deal with aggregation in different ways. Aggregation
is also very new to mac80211 but its implementation was designed
specifically for cases where you have "amdpu queues". Our design
requires everything done in software and as such the real clean
solution is to eventually try to move *as much as is possible* to
mac80211 to allow other devices which behave the same to be able to
use the same aggregation code. For now though for the non amdpu queue
case we only have Atheros 11n though and we are trying to resolve that
case first.

We will commit to move as much stuff up as we can, and this will
become more important as we get another device supported using the
same design. I've been trying hard to review and ask about the design
details of the other devices out in the market. In any case I think
this is the right step forward in the meantime, I also made a
suggestion as to how the ampdu queue case might be resolved. We just
need to be sure we keep in mind what needs to be done and move towards
it.

That's my two colones.

Luis