Requires software tx queueing support. frag_list support (for zero-copy)
is optional.
Signed-off-by: Felix Fietkau <[email protected]>
---
include/net/mac80211.h | 13 ++++
net/mac80211/agg-tx.c | 5 ++
net/mac80211/debugfs.c | 2 +
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/tx.c | 160 +++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 181 insertions(+)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 5714774..8c28210 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -709,6 +709,7 @@ enum mac80211_tx_info_flags {
* @IEEE80211_TX_CTRL_PS_RESPONSE: This frame is a response to a poll
* frame (PS-Poll or uAPSD).
* @IEEE80211_TX_CTRL_RATE_INJECT: This frame is injected with rate information
+ * @IEEE80211_TX_CTRL_AMSDU: This frame is an A-MSDU frame
*
* These flags are used in tx_info->control.flags.
*/
@@ -716,6 +717,7 @@ enum mac80211_tx_control_flags {
IEEE80211_TX_CTRL_PORT_CTRL_PROTO = BIT(0),
IEEE80211_TX_CTRL_PS_RESPONSE = BIT(1),
IEEE80211_TX_CTRL_RATE_INJECT = BIT(2),
+ IEEE80211_TX_CTRL_AMSDU = BIT(3),
};
/*
@@ -1964,6 +1966,12 @@ struct ieee80211_txq {
* order and does not need to manage its own reorder buffer or BA session
* timeout.
*
+ * @IEEE80211_HW_TX_AMSDU: Hardware (or driver) supports software aggregated
+ * A-MSDU frames. Requires software tx queueing support.
+ *
+ * @IEEE80211_HW_TX_FRAG_LIST: Hardware (or driver) supports sending frag_list
+ * skbs, needed for zero-copy software A-MSDU.
+ *
* @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
*/
enum ieee80211_hw_flags {
@@ -2001,6 +2009,8 @@ enum ieee80211_hw_flags {
IEEE80211_HW_BEACON_TX_STATUS,
IEEE80211_HW_NEEDS_UNIQUE_STA_ADDR,
IEEE80211_HW_SUPPORTS_REORDERING_BUFFER,
+ IEEE80211_HW_TX_AMSDU,
+ IEEE80211_HW_TX_FRAG_LIST,
/* keep last, obviously */
NUM_IEEE80211_HW_FLAGS
@@ -2073,6 +2083,8 @@ enum ieee80211_hw_flags {
* size is smaller (an example is LinkSys WRT120N with FW v1.0.07
* build 002 Jun 18 2012).
*
+ * @max_tx_fragments: maximum fragments per (A-)MSDU.
+ *
* @offchannel_tx_hw_queue: HW queue ID to use for offchannel TX
* (if %IEEE80211_HW_QUEUE_CONTROL is set)
*
@@ -2127,6 +2139,7 @@ struct ieee80211_hw {
u8 max_rate_tries;
u8 max_rx_aggregation_subframes;
u8 max_tx_aggregation_subframes;
+ u8 max_tx_fragments;
u8 offchannel_tx_hw_queue;
u8 radiotap_mcs_details;
u16 radiotap_vht_details;
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 4932e9f..42fa810 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -935,6 +935,7 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local,
size_t len)
{
struct tid_ampdu_tx *tid_tx;
+ struct ieee80211_txq *txq;
u16 capab, tid;
u8 buf_size;
bool amsdu;
@@ -945,6 +946,10 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local,
buf_size = (capab & IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK) >> 6;
buf_size = min(buf_size, local->hw.max_tx_aggregation_subframes);
+ txq = sta->sta.txq[tid];
+ if (!amsdu && txq)
+ set_bit(IEEE80211_TXQ_NO_AMSDU, &to_txq_info(txq)->flags);
+
mutex_lock(&sta->ampdu_mlme.mtx);
tid_tx = rcu_dereference_protected_tid_tx(sta, tid);
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index e433d0c..847779d 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -127,6 +127,8 @@ static const char *hw_flag_names[NUM_IEEE80211_HW_FLAGS + 1] = {
FLAG(BEACON_TX_STATUS),
FLAG(NEEDS_UNIQUE_STA_ADDR),
FLAG(SUPPORTS_REORDERING_BUFFER),
+ FLAG(TX_AMSDU),
+ FLAG(TX_FRAG_LIST),
/* keep last for the build bug below */
(void *)0x1
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a49c103..e68d8db 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -799,6 +799,7 @@ struct mac80211_qos_map {
enum txq_info_flags {
IEEE80211_TXQ_STOP,
IEEE80211_TXQ_AMPDU,
+ IEEE80211_TXQ_NO_AMSDU,
};
struct txq_info {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7bb67fa9..0c9cc6b 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1324,6 +1324,10 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
out:
spin_unlock_bh(&txqi->queue.lock);
+ if (skb && skb_has_frag_list(skb) &&
+ !ieee80211_hw_check(&local->hw, TX_FRAG_LIST))
+ skb_linearize(skb);
+
return skb;
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);
@@ -2766,6 +2770,158 @@ void ieee80211_clear_fast_xmit(struct sta_info *sta)
kfree_rcu(fast_tx, rcu_head);
}
+static int ieee80211_amsdu_pad(struct sk_buff *skb, int subframe_len)
+{
+ int amsdu_len = subframe_len + sizeof(struct ethhdr);
+ int padding = (4 - amsdu_len) & 3;
+
+ if (padding)
+ memset(skb_put(skb, padding), 0, padding);
+
+ return padding;
+}
+
+static bool ieee80211_amsdu_prepare_head(struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_fast_tx *fast_tx,
+ struct sk_buff *skb)
+{
+ struct ieee80211_local *local = sdata->local;
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ struct ieee80211_hdr *hdr;
+ struct ethhdr amsdu_hdr;
+ int hdr_len = fast_tx->hdr_len - sizeof(rfc1042_header);
+ int subframe_len = skb->len - hdr_len;
+ void *data;
+ u8 *qc;
+
+ if (info->control.flags & IEEE80211_TX_CTRL_AMSDU)
+ return true;
+
+ if (skb_headroom(skb) < sizeof(amsdu_hdr) || skb_tailroom(skb) < 3) {
+ I802_DEBUG_INC(local->tx_expand_skb_head);
+
+ if (pskb_expand_head(skb, sizeof(amsdu_hdr), 3, GFP_ATOMIC)) {
+ wiphy_debug(local->hw.wiphy,
+ "failed to reallocate TX buffer\n");
+ return false;
+ }
+ }
+
+ subframe_len += ieee80211_amsdu_pad(skb, subframe_len);
+
+ amsdu_hdr.h_proto = cpu_to_be16(subframe_len);
+ memcpy(amsdu_hdr.h_source, skb->data + fast_tx->sa_offs, ETH_ALEN);
+ memcpy(amsdu_hdr.h_dest, skb->data + fast_tx->da_offs, ETH_ALEN);
+
+ data = skb_push(skb, sizeof(amsdu_hdr));
+ memmove(data, data + sizeof(amsdu_hdr), hdr_len);
+ memcpy(data + hdr_len, &amsdu_hdr, sizeof(amsdu_hdr));
+
+ hdr = data;
+ qc = ieee80211_get_qos_ctl(hdr);
+ *qc |= IEEE80211_QOS_CTL_A_MSDU_PRESENT;
+
+ info->control.flags |= IEEE80211_TX_CTRL_AMSDU;
+
+ return true;
+}
+
+static bool ieee80211_amsdu_aggregate(struct ieee80211_sub_if_data *sdata,
+ struct sta_info *sta,
+ struct ieee80211_fast_tx *fast_tx,
+ struct sk_buff *skb)
+{
+ struct ieee80211_local *local = sdata->local;
+ u8 tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
+ struct ieee80211_txq *txq = sta->sta.txq[tid];
+ struct txq_info *txqi;
+ struct sk_buff **frag_tail, *head;
+ int subframe_len = skb->len - ETH_ALEN;
+ u8 max_subframes = sta->sta.max_amsdu_subframes;
+ int max_frags = local->hw.max_tx_fragments;
+ int max_amsdu_len;
+ __be16 len;
+ void *data;
+ bool ret = false;
+ int n = 1, nfrags;
+
+ if (!ieee80211_hw_check(&local->hw, TX_AMSDU))
+ return false;
+
+ if (!txq)
+ return false;
+
+ txqi = to_txq_info(txq);
+ if (test_bit(IEEE80211_TXQ_NO_AMSDU, &txqi->flags))
+ return false;
+
+ spin_lock_bh(&txqi->queue.lock);
+
+ head = skb_peek_tail(&txqi->queue);
+ if (!head)
+ goto out;
+
+ if (skb->len + head->len > max_amsdu_len)
+ goto out;
+
+ /*
+ * HT A-MPDU limits maximum MPDU size to 4095 bytes. Since aggregation
+ * sessions are started/stopped without txq flush, use the limit here
+ * to avoid having to de-aggregate later.
+ */
+ if (skb->len + head->len > 4095 &&
+ !sta->sta.vht_cap.vht_supported)
+ goto out;
+
+ if (!ieee80211_amsdu_prepare_head(sdata, fast_tx, head))
+ goto out;
+
+ nfrags = 1 + skb_shinfo(skb)->nr_frags;
+ nfrags += 1 + skb_shinfo(head)->nr_frags;
+ frag_tail = &skb_shinfo(head)->frag_list;
+ while (*frag_tail) {
+ nfrags += 1 + skb_shinfo(*frag_tail)->nr_frags;
+ frag_tail = &(*frag_tail)->next;
+ n++;
+ }
+
+ if (max_subframes && n > max_subframes)
+ goto out;
+
+ if (max_frags && nfrags > max_frags)
+ goto out;
+
+ if (skb_headroom(skb) < 8 || skb_tailroom(skb) < 3) {
+ I802_DEBUG_INC(local->tx_expand_skb_head);
+
+ if (pskb_expand_head(skb, 8, 3, GFP_ATOMIC)) {
+ wiphy_debug(local->hw.wiphy,
+ "failed to reallocate TX buffer\n");
+ goto out;
+ }
+ }
+
+ subframe_len += ieee80211_amsdu_pad(skb, subframe_len);
+
+ ret = true;
+ data = skb_push(skb, ETH_ALEN + 2);
+ memmove(data, data + ETH_ALEN + 2, 2 * ETH_ALEN);
+
+ data += 2 * ETH_ALEN;
+ len = cpu_to_be16(subframe_len);
+ memcpy(data, &len, 2);
+ memcpy(data + 2, rfc1042_header, ETH_ALEN);
+
+ head->len += skb->len;
+ head->data_len += skb->len;
+ *frag_tail = skb;
+
+out:
+ spin_unlock_bh(&txqi->queue.lock);
+
+ return ret;
+}
+
static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
struct net_device *dev, struct sta_info *sta,
struct ieee80211_fast_tx *fast_tx,
@@ -2820,6 +2976,10 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
ieee80211_tx_stats(dev, skb->len + extra_head);
+ if ((hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) &&
+ ieee80211_amsdu_aggregate(sdata, sta, fast_tx, skb))
+ return true;
+
/* will not be crypto-handled beyond what we do here, so use false
* as the may-encrypt argument for the resize to not account for
* more room than we already have in 'extra_head'
--
2.2.2
On Mon, Feb 8, 2016 at 3:23 AM, Felix Fietkau <[email protected]> wrote:
> On 2016-02-08 12:06, Krishna Chaitanya wrote:
>> On Mon, Feb 8, 2016 at 4:34 PM, Felix Fietkau <[email protected]> wrote:
>>> On 2016-02-08 10:54, Krishna Chaitanya wrote:
>>>> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>>>>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>>>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>>>>> is optional.
>>>>>>
>>>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>>>> ---
>>>>>
>>>>>
>>>>> Ok - looks fine, but... and here comes the hard stuff.
>>>>> The frame size in the PLCP is limited in a way that you can't - from a
>>>>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>>>>> do that for low rates at all regardless of the spec.
>>>>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>>>>> of the link quality, how do we prevent A-MSDU if the rate is low /
>>>>> dropping.
>>>>> I'd even argue that when the rates get lower, you'll have more
>>>>> packets piling up in the software queue and ... even more chance to
>>>>> get A-MSDU in the exact case where you really want to avoid it?
>>>>
>>>> Similar to triggering AMPDU setup, we should put this control
>>>> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
>>>> drop below a pre-defined MCS (or) only for best-throughput rates.
>>> I think starting/stopping A-MSDU based on the rate is a bad idea.
>>> Even with low rates, using A-MSDU can be a good thing (especially for
>>> TCP ACKs), it just needs different size limits.
>>
>> By low rates, i was referring to bad channel conditions (more
>> retries/crc errors)
>> so using AMSDU might trigger more TCP level retries and for case
>> of TCP ACK's its still worse in that it triggers TCP data retires from the
>> peer.
> Based on the research and data from the Bufferbloat project, I'd say
> that in this case the latency due to queue buildup is a lot more harmful
> than lost packets.
+1.
> With unmanaged queues, the latency will cause unnecessary
> retransmissions anyway.
typically at greater than 250ms, yes.
It's my hope that everyone has agreed that > 250ms of queuing is bad,
except for actual transit to the moon and back, at this point. My hope
is that everyone can aim for < 20ms in the more general case.
I was very happy to see the iwl patch go by cutting things down from
seconds to 30-250ms under load, but am now puzzled.
One iwl user has reported that AMPDUs are disabled by default on iwl,
and had to do this:
options iwlwifi 11n_disable=8
For a 4fold reduction in latency under load in his tests, iwl to ath9k.
What enables/disable AMPDU and AMSDU in the general case?
> With managed queues, packet drops start increasing with latency until
> TCP starts behaving properly.
> In both cases you have extra TCP retransmissions...
One thrust of mine has been to get ecn more widely adopted, which
would eliminate the tcp retransmit problem on devices using it for
congestion control (except on actual loss).
Apple turned ecn on universally on all dev devices last august. They
ran into trouble with one major ISP and had to turn it off, I am not
sure if they shipped with it on or not.
https://plus.google.com/u/0/107942175615993706558/posts/1j8nXtLGZDm
Stuart Cheshire also points at a real case where excessive queuing
hurts and fq doesn't help - screen sharing - with a nice demo
combining fq_codel + the tcp_netsent_lowat option. (his talk starts at
about 16 minutes in, he goes into the relevant bits at about 24
minutes in, and I'm sorry for always pointing people at talks rather
than papers)
Toke ("the good, the bad, and the wifi") points to excessive queuing
*not* being a problem so long as the fq portion is sufficiently close
to the hardware, in other scenarios.
http://www.sciencedirect.com/science/article/pii/S1389128615002479
Now, as to whether a driver or device doing retransmits should (or
even could) drop or mark packets is a good question. I am biased
towards achieving low latency and per device fairness so I lean
towards giving up a lot sooner in bad conditions and moving on....
> With bad conditions you also get a strong increase in per-TXOP latency.
Having an agreed upon table for all the "bad" conditions and what
should be done in these cases would be good. I don't regard
"contention" as a bad condition, but as an indicator to be smarter
about bunching up and/or discarding packets. Interference is a "bad"
condition. Low rates for one station compared to the others is not a
bad condition, as striving for airtime fairness between all stations
is a reasonable goal... etc...
> With A-MSDU you need fewer TXOPs for the same amount of data in the queue.
I like it. :)
I note that for stations, I am perpetually seeing this sort of behavior:
ap -> transmits a bunch of packets to station
station -> takes 2 or more txops to respond
why? it takes some processing time for the station to send out it's
next batch of packets. Slightly better to wait until more stuff is
batched up (and I know, this is hard)
(it sounds like the iwl defers the aggregation decision, so might not
do this. ath9k will do this)
I would like to be testing for this behavior.
>
> - Felix
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 8, 2016 at 3:56 PM, Emmanuel Grumbach <[email protected]> wrote:
> On Mon, Feb 8, 2016 at 11:54 AM, Krishna Chaitanya
> <[email protected]> wrote:
>> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>>> is optional.
>>>>
>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>> ---
>>>
>>>
>>> Ok - looks fine, but... and here comes the hard stuff.
>>> The frame size in the PLCP is limited in a way that you can't - from a
>>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>>> do that for low rates at all regardless of the spec.
>>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>>> of the link quality, how do we prevent A-MSDU if the rate is low /
>>> dropping.
>>> I'd even argue that when the rates get lower, you'll have more
>>> packets piling up in the software queue and ... even more chance to
>>> get A-MSDU in the exact case where you really want to avoid it?
>>
>> Similar to triggering AMPDU setup, we should put this control
>> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
>> drop below a pre-defined MCS (or) only for best-throughput rates.
>
> Ok - but the size of A-MPDU is determined in the firmware / hardware
> typically (at least for Intel devices and I have to admit my ignorance
> about other designs...). So that if you get bad link conditions, the
> firmware can play for that size. A-MSDU built in the host is a
> different story. I remember that athXk devices attach the rate on the
> Tx packet itself so that the rate control is serialized with the data
> path.
> iwlwifi works differently: we have a map of rates in the firmware.
> This map is maintained in the driver updated based on the feedback we
> get from the device. When an update is needed, iwlwifi sends a command
> to the firmware and that command bypasses all the Tx packets currently
> queue so that a packet that is already waiting in the queue will be
> sent with the updated map. This allows iwlwifi to react downgrade
> faster when needed.
> So - for athXk (and maybe the MediaTek devices Felix is working on),
> this may not be a big problem: you'd need to add another input to the
> A-MSDU building code coming from the rate control.
> FWIW: in iwlwifi I simply decided to be on the safe side and allowed
> the driver to build A-MSDUs only if the rate is fairly high and we are
> not downgrading. But you saw (and commented on) that patch already I
> think :)
Yes, this approach is going to be tough for those devices with HW RC.
We might need something like amsdu_action to get some feedback from
device about using of amsdu?
On 8 February 2016 at 12:09, Krishna Chaitanya <[email protected]> wrote:
> On Mon, Feb 8, 2016 at 3:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>> On Mon, Feb 8, 2016 at 11:54 AM, Krishna Chaitanya
>> <[email protected]> wrote:
>>> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>>>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>>>> is optional.
>>>>>
>>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>>> ---
>>>>
>>>>
>>>> Ok - looks fine, but... and here comes the hard stuff.
>>>> The frame size in the PLCP is limited in a way that you can't - from a
>>>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>>>> do that for low rates at all regardless of the spec.
>>>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>>>> of the link quality, how do we prevent A-MSDU if the rate is low /
>>>> dropping.
>>>> I'd even argue that when the rates get lower, you'll have more
>>>> packets piling up in the software queue and ... even more chance to
>>>> get A-MSDU in the exact case where you really want to avoid it?
>>>
>>> Similar to triggering AMPDU setup, we should put this control
>>> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
>>> drop below a pre-defined MCS (or) only for best-throughput rates.
>>
>> Ok - but the size of A-MPDU is determined in the firmware / hardware
>> typically (at least for Intel devices and I have to admit my ignorance
>> about other designs...). So that if you get bad link conditions, the
>> firmware can play for that size. A-MSDU built in the host is a
>> different story. I remember that athXk devices attach the rate on the
>> Tx packet itself so that the rate control is serialized with the data
>> path.
>> iwlwifi works differently: we have a map of rates in the firmware.
>> This map is maintained in the driver updated based on the feedback we
>> get from the device. When an update is needed, iwlwifi sends a command
>> to the firmware and that command bypasses all the Tx packets currently
>> queue so that a packet that is already waiting in the queue will be
>> sent with the updated map. This allows iwlwifi to react downgrade
>> faster when needed.
>> So - for athXk (and maybe the MediaTek devices Felix is working on),
>> this may not be a big problem: you'd need to add another input to the
>> A-MSDU building code coming from the rate control.
>> FWIW: in iwlwifi I simply decided to be on the safe side and allowed
>> the driver to build A-MSDUs only if the rate is fairly high and we are
>> not downgrading. But you saw (and commented on) that patch already I
>> think :)
>
> Yes, this approach is going to be tough for those devices with HW RC.
> We might need something like amsdu_action to get some feedback from
> device about using of amsdu?
The HW/FW rate control still may not have sufficient knobs for driver
to implement an amsdu_action().
What about if mac80211 tracked per-station-tid packet sojourn time and
based on that derive max A-MSDU bytelimit? This could also be useful
for the bufferbloat problem (discussed in the other thread) as well.
Michał
On Mon, Feb 8, 2016 at 1:15 PM, Felix Fietkau <[email protected]> wrote:
> On 2016-02-08 11:26, Emmanuel Grumbach wrote:
>> On Mon, Feb 8, 2016 at 11:54 AM, Krishna Chaitanya
>> <[email protected]> wrote:
>>> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>>>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>>>> is optional.
>>>>>
>>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>>> ---
>>>>
>>>>
>>>> Ok - looks fine, but... and here comes the hard stuff.
>>>> The frame size in the PLCP is limited in a way that you can't - from a
>>>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>>>> do that for low rates at all regardless of the spec.
>>>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>>>> of the link quality, how do we prevent A-MSDU if the rate is low /
>>>> dropping.
>>>> I'd even argue that when the rates get lower, you'll have more
>>>> packets piling up in the software queue and ... even more chance to
>>>> get A-MSDU in the exact case where you really want to avoid it?
>>>
>>> Similar to triggering AMPDU setup, we should put this control
>>> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
>>> drop below a pre-defined MCS (or) only for best-throughput rates.
>>
>> Ok - but the size of A-MPDU is determined in the firmware / hardware
>> typically (at least for Intel devices and I have to admit my ignorance
>> about other designs...). So that if you get bad link conditions, the
>> firmware can play for that size. A-MSDU built in the host is a
>> different story. I remember that athXk devices attach the rate on the
>> Tx packet itself so that the rate control is serialized with the data
>> path.
>> iwlwifi works differently: we have a map of rates in the firmware.
>> This map is maintained in the driver updated based on the feedback we
>> get from the device. When an update is needed, iwlwifi sends a command
>> to the firmware and that command bypasses all the Tx packets currently
>> queue so that a packet that is already waiting in the queue will be
>> sent with the updated map. This allows iwlwifi to react downgrade
>> faster when needed.
> Even ath9k has been converted to use a per-sta rate table with optional
> per-packet rate overrides for probing.
>
>> So - for athXk (and maybe the MediaTek devices Felix is working on),
>> this may not be a big problem: you'd need to add another input to the
>> A-MSDU building code coming from the rate control.
>> FWIW: in iwlwifi I simply decided to be on the safe side and allowed
>> the driver to build A-MSDUs only if the rate is fairly high and we are
>> not downgrading. But you saw (and commented on) that patch already I
>> think :)
> I think it would be a good idea for iwlwifi to start using the rate
> control API properly. Not only would that simplify integration with the
> A-MSDU code, it would also make it possible to compare minstrel_ht with
> the iwlwifi rate control code.
> The iwlwifi rate control code is doing some crazy things, and I'd be
> really interested in seeing how it compares to the more simple and
> structured approach taken in minstrel_ht.
Yeah ok. So I agree that iwlwifi uses the rate control API in a crazy
way, the change is planned (well.. I've said that long ago
already...). The plan is to move to internal API only (not go through
the rate control API) though...
I guess we have enough material for a beer in Seville I think :)
>
> - Felix
On Mon, Feb 8, 2016 at 1:23 PM, Felix Fietkau <[email protected]> wrote:
> On 2016-02-08 12:06, Krishna Chaitanya wrote:
>> On Mon, Feb 8, 2016 at 4:34 PM, Felix Fietkau <[email protected]> wrote:
>>> On 2016-02-08 10:54, Krishna Chaitanya wrote:
>>>> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>>>>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>>>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>>>>> is optional.
>>>>>>
>>>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>>>> ---
>>>>>
>>>>>
>>>>> Ok - looks fine, but... and here comes the hard stuff.
>>>>> The frame size in the PLCP is limited in a way that you can't - from a
>>>>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>>>>> do that for low rates at all regardless of the spec.
>>>>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>>>>> of the link quality, how do we prevent A-MSDU if the rate is low /
>>>>> dropping.
>>>>> I'd even argue that when the rates get lower, you'll have more
>>>>> packets piling up in the software queue and ... even more chance to
>>>>> get A-MSDU in the exact case where you really want to avoid it?
>>>>
>>>> Similar to triggering AMPDU setup, we should put this control
>>>> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
>>>> drop below a pre-defined MCS (or) only for best-throughput rates.
>>> I think starting/stopping A-MSDU based on the rate is a bad idea.
>>> Even with low rates, using A-MSDU can be a good thing (especially for
>>> TCP ACKs), it just needs different size limits.
>>
>> By low rates, i was referring to bad channel conditions (more
>> retries/crc errors)
>> so using AMSDU might trigger more TCP level retries and for case
>> of TCP ACK's its still worse in that it triggers TCP data retires from the
>> peer.
> Based on the research and data from the Bufferbloat project, I'd say
> that in this case the latency due to queue buildup is a lot more harmful
> than lost packets.
> With unmanaged queues, the latency will cause unnecessary
> retransmissions anyway.
> With managed queues, packet drops start increasing with latency until
> TCP starts behaving properly.
> In both cases you have extra TCP retransmissions...
>
> With bad conditions you also get a strong increase in per-TXOP latency.
> With A-MSDU you need fewer TXOPs for the same amount of data in the queue.
Assuming you don't have collisions, then yes. But when things go bad,
you may very well have more collisions, and A-MSDU becomes a disaster.
The way I see it, collisions and "bad conditions" are tightly related.
On 2016-02-08 12:06, Krishna Chaitanya wrote:
> On Mon, Feb 8, 2016 at 4:34 PM, Felix Fietkau <[email protected]> wrote:
>> On 2016-02-08 10:54, Krishna Chaitanya wrote:
>>> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>>>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>>>> is optional.
>>>>>
>>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>>> ---
>>>>
>>>>
>>>> Ok - looks fine, but... and here comes the hard stuff.
>>>> The frame size in the PLCP is limited in a way that you can't - from a
>>>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>>>> do that for low rates at all regardless of the spec.
>>>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>>>> of the link quality, how do we prevent A-MSDU if the rate is low /
>>>> dropping.
>>>> I'd even argue that when the rates get lower, you'll have more
>>>> packets piling up in the software queue and ... even more chance to
>>>> get A-MSDU in the exact case where you really want to avoid it?
>>>
>>> Similar to triggering AMPDU setup, we should put this control
>>> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
>>> drop below a pre-defined MCS (or) only for best-throughput rates.
>> I think starting/stopping A-MSDU based on the rate is a bad idea.
>> Even with low rates, using A-MSDU can be a good thing (especially for
>> TCP ACKs), it just needs different size limits.
>
> By low rates, i was referring to bad channel conditions (more
> retries/crc errors)
> so using AMSDU might trigger more TCP level retries and for case
> of TCP ACK's its still worse in that it triggers TCP data retires from the
> peer.
Based on the research and data from the Bufferbloat project, I'd say
that in this case the latency due to queue buildup is a lot more harmful
than lost packets.
With unmanaged queues, the latency will cause unnecessary
retransmissions anyway.
With managed queues, packet drops start increasing with latency until
TCP starts behaving properly.
In both cases you have extra TCP retransmissions...
With bad conditions you also get a strong increase in per-TXOP latency.
With A-MSDU you need fewer TXOPs for the same amount of data in the queue.
- Felix
On 2016-02-08 11:26, Emmanuel Grumbach wrote:
> On Mon, Feb 8, 2016 at 11:54 AM, Krishna Chaitanya
> <[email protected]> wrote:
>> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>>> is optional.
>>>>
>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>> ---
>>>
>>>
>>> Ok - looks fine, but... and here comes the hard stuff.
>>> The frame size in the PLCP is limited in a way that you can't - from a
>>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>>> do that for low rates at all regardless of the spec.
>>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>>> of the link quality, how do we prevent A-MSDU if the rate is low /
>>> dropping.
>>> I'd even argue that when the rates get lower, you'll have more
>>> packets piling up in the software queue and ... even more chance to
>>> get A-MSDU in the exact case where you really want to avoid it?
>>
>> Similar to triggering AMPDU setup, we should put this control
>> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
>> drop below a pre-defined MCS (or) only for best-throughput rates.
>
> Ok - but the size of A-MPDU is determined in the firmware / hardware
> typically (at least for Intel devices and I have to admit my ignorance
> about other designs...). So that if you get bad link conditions, the
> firmware can play for that size. A-MSDU built in the host is a
> different story. I remember that athXk devices attach the rate on the
> Tx packet itself so that the rate control is serialized with the data
> path.
> iwlwifi works differently: we have a map of rates in the firmware.
> This map is maintained in the driver updated based on the feedback we
> get from the device. When an update is needed, iwlwifi sends a command
> to the firmware and that command bypasses all the Tx packets currently
> queue so that a packet that is already waiting in the queue will be
> sent with the updated map. This allows iwlwifi to react downgrade
> faster when needed.
Even ath9k has been converted to use a per-sta rate table with optional
per-packet rate overrides for probing.
> So - for athXk (and maybe the MediaTek devices Felix is working on),
> this may not be a big problem: you'd need to add another input to the
> A-MSDU building code coming from the rate control.
> FWIW: in iwlwifi I simply decided to be on the safe side and allowed
> the driver to build A-MSDUs only if the rate is fairly high and we are
> not downgrading. But you saw (and commented on) that patch already I
> think :)
I think it would be a good idea for iwlwifi to start using the rate
control API properly. Not only would that simplify integration with the
A-MSDU code, it would also make it possible to compare minstrel_ht with
the iwlwifi rate control code.
The iwlwifi rate control code is doing some crazy things, and I'd be
really interested in seeing how it compares to the more simple and
structured approach taken in minstrel_ht.
- Felix
On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>> Requires software tx queueing support. frag_list support (for zero-copy)
>> is optional.
>>
>> Signed-off-by: Felix Fietkau <[email protected]>
>> ---
>
>
> Ok - looks fine, but... and here comes the hard stuff.
> The frame size in the PLCP is limited in a way that you can't - from a
> spec POV - enable A-MSDU for low rates. Of course, you don't want to
> do that for low rates at all regardless of the spec.
> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
> of the link quality, how do we prevent A-MSDU if the rate is low /
> dropping.
> I'd even argue that when the rates get lower, you'll have more
> packets piling up in the software queue and ... even more chance to
> get A-MSDU in the exact case where you really want to avoid it?
Similar to triggering AMPDU setup, we should put this control
in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
drop below a pre-defined MCS (or) only for best-throughput rates.
On 2016-02-08 10:54, Krishna Chaitanya wrote:
> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>> is optional.
>>>
>>> Signed-off-by: Felix Fietkau <[email protected]>
>>> ---
>>
>>
>> Ok - looks fine, but... and here comes the hard stuff.
>> The frame size in the PLCP is limited in a way that you can't - from a
>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>> do that for low rates at all regardless of the spec.
>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>> of the link quality, how do we prevent A-MSDU if the rate is low /
>> dropping.
>> I'd even argue that when the rates get lower, you'll have more
>> packets piling up in the software queue and ... even more chance to
>> get A-MSDU in the exact case where you really want to avoid it?
>
> Similar to triggering AMPDU setup, we should put this control
> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
> drop below a pre-defined MCS (or) only for best-throughput rates.
I think starting/stopping A-MSDU based on the rate is a bad idea.
Even with low rates, using A-MSDU can be a good thing (especially for
TCP ACKs), it just needs different size limits.
- Felix
On Mon, Feb 8, 2016 at 4:34 PM, Felix Fietkau <[email protected]> wrote:
> On 2016-02-08 10:54, Krishna Chaitanya wrote:
>> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>>> is optional.
>>>>
>>>> Signed-off-by: Felix Fietkau <[email protected]>
>>>> ---
>>>
>>>
>>> Ok - looks fine, but... and here comes the hard stuff.
>>> The frame size in the PLCP is limited in a way that you can't - from a
>>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>>> do that for low rates at all regardless of the spec.
>>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>>> of the link quality, how do we prevent A-MSDU if the rate is low /
>>> dropping.
>>> I'd even argue that when the rates get lower, you'll have more
>>> packets piling up in the software queue and ... even more chance to
>>> get A-MSDU in the exact case where you really want to avoid it?
>>
>> Similar to triggering AMPDU setup, we should put this control
>> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
>> drop below a pre-defined MCS (or) only for best-throughput rates.
> I think starting/stopping A-MSDU based on the rate is a bad idea.
> Even with low rates, using A-MSDU can be a good thing (especially for
> TCP ACKs), it just needs different size limits.
By low rates, i was referring to bad channel conditions (more
retries/crc errors)
so using AMSDU might trigger more TCP level retries and for case
of TCP ACK's its still worse in that it triggers TCP data retires from the
peer.
On Mon, Feb 8, 2016 at 11:54 AM, Krishna Chaitanya
<[email protected]> wrote:
> On Mon, Feb 8, 2016 at 2:56 PM, Emmanuel Grumbach <[email protected]> wrote:
>> On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
>>> Requires software tx queueing support. frag_list support (for zero-copy)
>>> is optional.
>>>
>>> Signed-off-by: Felix Fietkau <[email protected]>
>>> ---
>>
>>
>> Ok - looks fine, but... and here comes the hard stuff.
>> The frame size in the PLCP is limited in a way that you can't - from a
>> spec POV - enable A-MSDU for low rates. Of course, you don't want to
>> do that for low rates at all regardless of the spec.
>> Since you build the A-MSDU in the mac80211 Tx queue which is not aware
>> of the link quality, how do we prevent A-MSDU if the rate is low /
>> dropping.
>> I'd even argue that when the rates get lower, you'll have more
>> packets piling up in the software queue and ... even more chance to
>> get A-MSDU in the exact case where you really want to avoid it?
>
> Similar to triggering AMPDU setup, we should put this control
> in RC (minstrel) to start/stop AMSDU based on link quality/if the rates
> drop below a pre-defined MCS (or) only for best-throughput rates.
Ok - but the size of A-MPDU is determined in the firmware / hardware
typically (at least for Intel devices and I have to admit my ignorance
about other designs...). So that if you get bad link conditions, the
firmware can play for that size. A-MSDU built in the host is a
different story. I remember that athXk devices attach the rate on the
Tx packet itself so that the rate control is serialized with the data
path.
iwlwifi works differently: we have a map of rates in the firmware.
This map is maintained in the driver updated based on the feedback we
get from the device. When an update is needed, iwlwifi sends a command
to the firmware and that command bypasses all the Tx packets currently
queue so that a packet that is already waiting in the queue will be
sent with the updated map. This allows iwlwifi to react downgrade
faster when needed.
So - for athXk (and maybe the MediaTek devices Felix is working on),
this may not be a big problem: you'd need to add another input to the
A-MSDU building code coming from the rate control.
FWIW: in iwlwifi I simply decided to be on the safe side and allowed
the driver to build A-MSDUs only if the rate is fairly high and we are
not downgrading. But you saw (and commented on) that patch already I
think :)
On Mon, Feb 8, 2016 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
> Requires software tx queueing support. frag_list support (for zero-copy)
> is optional.
>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
Ok - looks fine, but... and here comes the hard stuff.
The frame size in the PLCP is limited in a way that you can't - from a
spec POV - enable A-MSDU for low rates. Of course, you don't want to
do that for low rates at all regardless of the spec.
Since you build the A-MSDU in the mac80211 Tx queue which is not aware
of the link quality, how do we prevent A-MSDU if the rate is low /
dropping.
I'd even argue that when the rates get lower, you'll have more
packets piling up in the software queue and ... even more chance to
get A-MSDU in the exact case where you really want to avoid it?
> include/net/mac80211.h | 13 ++++
> net/mac80211/agg-tx.c | 5 ++
> net/mac80211/debugfs.c | 2 +
> net/mac80211/ieee80211_i.h | 1 +
> net/mac80211/tx.c | 160 +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 181 insertions(+)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 5714774..8c28210 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -709,6 +709,7 @@ enum mac80211_tx_info_flags {
> * @IEEE80211_TX_CTRL_PS_RESPONSE: This frame is a response to a poll
> * frame (PS-Poll or uAPSD).
> * @IEEE80211_TX_CTRL_RATE_INJECT: This frame is injected with rate information
> + * @IEEE80211_TX_CTRL_AMSDU: This frame is an A-MSDU frame
> *
> * These flags are used in tx_info->control.flags.
> */
> @@ -716,6 +717,7 @@ enum mac80211_tx_control_flags {
> IEEE80211_TX_CTRL_PORT_CTRL_PROTO = BIT(0),
> IEEE80211_TX_CTRL_PS_RESPONSE = BIT(1),
> IEEE80211_TX_CTRL_RATE_INJECT = BIT(2),
> + IEEE80211_TX_CTRL_AMSDU = BIT(3),
> };
>
> /*
> @@ -1964,6 +1966,12 @@ struct ieee80211_txq {
> * order and does not need to manage its own reorder buffer or BA session
> * timeout.
> *
> + * @IEEE80211_HW_TX_AMSDU: Hardware (or driver) supports software aggregated
> + * A-MSDU frames. Requires software tx queueing support.
> + *
> + * @IEEE80211_HW_TX_FRAG_LIST: Hardware (or driver) supports sending frag_list
> + * skbs, needed for zero-copy software A-MSDU.
> + *
> * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
> */
> enum ieee80211_hw_flags {
> @@ -2001,6 +2009,8 @@ enum ieee80211_hw_flags {
> IEEE80211_HW_BEACON_TX_STATUS,
> IEEE80211_HW_NEEDS_UNIQUE_STA_ADDR,
> IEEE80211_HW_SUPPORTS_REORDERING_BUFFER,
> + IEEE80211_HW_TX_AMSDU,
> + IEEE80211_HW_TX_FRAG_LIST,
>
> /* keep last, obviously */
> NUM_IEEE80211_HW_FLAGS
> @@ -2073,6 +2083,8 @@ enum ieee80211_hw_flags {
> * size is smaller (an example is LinkSys WRT120N with FW v1.0.07
> * build 002 Jun 18 2012).
> *
> + * @max_tx_fragments: maximum fragments per (A-)MSDU.
> + *
> * @offchannel_tx_hw_queue: HW queue ID to use for offchannel TX
> * (if %IEEE80211_HW_QUEUE_CONTROL is set)
> *
> @@ -2127,6 +2139,7 @@ struct ieee80211_hw {
> u8 max_rate_tries;
> u8 max_rx_aggregation_subframes;
> u8 max_tx_aggregation_subframes;
> + u8 max_tx_fragments;
> u8 offchannel_tx_hw_queue;
> u8 radiotap_mcs_details;
> u16 radiotap_vht_details;
> diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
> index 4932e9f..42fa810 100644
> --- a/net/mac80211/agg-tx.c
> +++ b/net/mac80211/agg-tx.c
> @@ -935,6 +935,7 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local,
> size_t len)
> {
> struct tid_ampdu_tx *tid_tx;
> + struct ieee80211_txq *txq;
> u16 capab, tid;
> u8 buf_size;
> bool amsdu;
> @@ -945,6 +946,10 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local,
> buf_size = (capab & IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK) >> 6;
> buf_size = min(buf_size, local->hw.max_tx_aggregation_subframes);
>
> + txq = sta->sta.txq[tid];
> + if (!amsdu && txq)
> + set_bit(IEEE80211_TXQ_NO_AMSDU, &to_txq_info(txq)->flags);
> +
> mutex_lock(&sta->ampdu_mlme.mtx);
>
> tid_tx = rcu_dereference_protected_tid_tx(sta, tid);
> diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
> index e433d0c..847779d 100644
> --- a/net/mac80211/debugfs.c
> +++ b/net/mac80211/debugfs.c
> @@ -127,6 +127,8 @@ static const char *hw_flag_names[NUM_IEEE80211_HW_FLAGS + 1] = {
> FLAG(BEACON_TX_STATUS),
> FLAG(NEEDS_UNIQUE_STA_ADDR),
> FLAG(SUPPORTS_REORDERING_BUFFER),
> + FLAG(TX_AMSDU),
> + FLAG(TX_FRAG_LIST),
>
> /* keep last for the build bug below */
> (void *)0x1
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index a49c103..e68d8db 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -799,6 +799,7 @@ struct mac80211_qos_map {
> enum txq_info_flags {
> IEEE80211_TXQ_STOP,
> IEEE80211_TXQ_AMPDU,
> + IEEE80211_TXQ_NO_AMSDU,
> };
>
> struct txq_info {
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 7bb67fa9..0c9cc6b 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1324,6 +1324,10 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> out:
> spin_unlock_bh(&txqi->queue.lock);
>
> + if (skb && skb_has_frag_list(skb) &&
> + !ieee80211_hw_check(&local->hw, TX_FRAG_LIST))
> + skb_linearize(skb);
> +
> return skb;
> }
> EXPORT_SYMBOL(ieee80211_tx_dequeue);
> @@ -2766,6 +2770,158 @@ void ieee80211_clear_fast_xmit(struct sta_info *sta)
> kfree_rcu(fast_tx, rcu_head);
> }
>
> +static int ieee80211_amsdu_pad(struct sk_buff *skb, int subframe_len)
> +{
> + int amsdu_len = subframe_len + sizeof(struct ethhdr);
> + int padding = (4 - amsdu_len) & 3;
> +
> + if (padding)
> + memset(skb_put(skb, padding), 0, padding);
> +
> + return padding;
> +}
> +
> +static bool ieee80211_amsdu_prepare_head(struct ieee80211_sub_if_data *sdata,
> + struct ieee80211_fast_tx *fast_tx,
> + struct sk_buff *skb)
> +{
> + struct ieee80211_local *local = sdata->local;
> + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> + struct ieee80211_hdr *hdr;
> + struct ethhdr amsdu_hdr;
> + int hdr_len = fast_tx->hdr_len - sizeof(rfc1042_header);
> + int subframe_len = skb->len - hdr_len;
> + void *data;
> + u8 *qc;
> +
> + if (info->control.flags & IEEE80211_TX_CTRL_AMSDU)
> + return true;
> +
> + if (skb_headroom(skb) < sizeof(amsdu_hdr) || skb_tailroom(skb) < 3) {
> + I802_DEBUG_INC(local->tx_expand_skb_head);
> +
> + if (pskb_expand_head(skb, sizeof(amsdu_hdr), 3, GFP_ATOMIC)) {
> + wiphy_debug(local->hw.wiphy,
> + "failed to reallocate TX buffer\n");
> + return false;
> + }
> + }
> +
> + subframe_len += ieee80211_amsdu_pad(skb, subframe_len);
> +
> + amsdu_hdr.h_proto = cpu_to_be16(subframe_len);
> + memcpy(amsdu_hdr.h_source, skb->data + fast_tx->sa_offs, ETH_ALEN);
> + memcpy(amsdu_hdr.h_dest, skb->data + fast_tx->da_offs, ETH_ALEN);
> +
> + data = skb_push(skb, sizeof(amsdu_hdr));
> + memmove(data, data + sizeof(amsdu_hdr), hdr_len);
> + memcpy(data + hdr_len, &amsdu_hdr, sizeof(amsdu_hdr));
> +
> + hdr = data;
> + qc = ieee80211_get_qos_ctl(hdr);
> + *qc |= IEEE80211_QOS_CTL_A_MSDU_PRESENT;
> +
> + info->control.flags |= IEEE80211_TX_CTRL_AMSDU;
> +
> + return true;
> +}
> +
> +static bool ieee80211_amsdu_aggregate(struct ieee80211_sub_if_data *sdata,
> + struct sta_info *sta,
> + struct ieee80211_fast_tx *fast_tx,
> + struct sk_buff *skb)
> +{
> + struct ieee80211_local *local = sdata->local;
> + u8 tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
> + struct ieee80211_txq *txq = sta->sta.txq[tid];
> + struct txq_info *txqi;
> + struct sk_buff **frag_tail, *head;
> + int subframe_len = skb->len - ETH_ALEN;
> + u8 max_subframes = sta->sta.max_amsdu_subframes;
> + int max_frags = local->hw.max_tx_fragments;
> + int max_amsdu_len;
> + __be16 len;
> + void *data;
> + bool ret = false;
> + int n = 1, nfrags;
> +
> + if (!ieee80211_hw_check(&local->hw, TX_AMSDU))
> + return false;
> +
> + if (!txq)
> + return false;
> +
> + txqi = to_txq_info(txq);
> + if (test_bit(IEEE80211_TXQ_NO_AMSDU, &txqi->flags))
> + return false;
> +
> + spin_lock_bh(&txqi->queue.lock);
> +
> + head = skb_peek_tail(&txqi->queue);
> + if (!head)
> + goto out;
> +
> + if (skb->len + head->len > max_amsdu_len)
> + goto out;
> +
> + /*
> + * HT A-MPDU limits maximum MPDU size to 4095 bytes. Since aggregation
> + * sessions are started/stopped without txq flush, use the limit here
> + * to avoid having to de-aggregate later.
> + */
> + if (skb->len + head->len > 4095 &&
> + !sta->sta.vht_cap.vht_supported)
> + goto out;
> +
> + if (!ieee80211_amsdu_prepare_head(sdata, fast_tx, head))
> + goto out;
> +
> + nfrags = 1 + skb_shinfo(skb)->nr_frags;
> + nfrags += 1 + skb_shinfo(head)->nr_frags;
> + frag_tail = &skb_shinfo(head)->frag_list;
> + while (*frag_tail) {
> + nfrags += 1 + skb_shinfo(*frag_tail)->nr_frags;
> + frag_tail = &(*frag_tail)->next;
> + n++;
> + }
> +
> + if (max_subframes && n > max_subframes)
> + goto out;
> +
> + if (max_frags && nfrags > max_frags)
> + goto out;
> +
> + if (skb_headroom(skb) < 8 || skb_tailroom(skb) < 3) {
> + I802_DEBUG_INC(local->tx_expand_skb_head);
> +
> + if (pskb_expand_head(skb, 8, 3, GFP_ATOMIC)) {
> + wiphy_debug(local->hw.wiphy,
> + "failed to reallocate TX buffer\n");
> + goto out;
> + }
> + }
> +
> + subframe_len += ieee80211_amsdu_pad(skb, subframe_len);
> +
> + ret = true;
> + data = skb_push(skb, ETH_ALEN + 2);
> + memmove(data, data + ETH_ALEN + 2, 2 * ETH_ALEN);
> +
> + data += 2 * ETH_ALEN;
> + len = cpu_to_be16(subframe_len);
> + memcpy(data, &len, 2);
> + memcpy(data + 2, rfc1042_header, ETH_ALEN);
> +
> + head->len += skb->len;
> + head->data_len += skb->len;
> + *frag_tail = skb;
> +
> +out:
> + spin_unlock_bh(&txqi->queue.lock);
> +
> + return ret;
> +}
> +
> static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
> struct net_device *dev, struct sta_info *sta,
> struct ieee80211_fast_tx *fast_tx,
> @@ -2820,6 +2976,10 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
>
> ieee80211_tx_stats(dev, skb->len + extra_head);
>
> + if ((hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) &&
> + ieee80211_amsdu_aggregate(sdata, sta, fast_tx, skb))
> + return true;
> +
> /* will not be crypto-handled beyond what we do here, so use false
> * as the may-encrypt argument for the resize to not account for
> * more room than we already have in 'extra_head'
> --
> 2.2.2
>
> One iwl user has reported that AMPDUs are disabled by default on iwl,
That was me.
> and had to do this:
>
> options iwlwifi 11n_disable=8
>
> For a 4fold reduction in latency under load in his tests, iwl to ath9k.
No, it was ath9k to iwl. I don't understand it either: enabling AMPDUs on
the STA dramatically reduces latency under load in the AP->STA direction.
-- Juliusz