From f0dd52f0076d85fac7bdf21ef9362e8bbeda6534 Mon Sep 17 00:00:00 2001 From: coolsnowwolf Date: Thu, 21 Mar 2019 14:09:06 +0800 Subject: [PATCH] mac80211: fix an issue with the TXQ scheduling API and powersave clients --- .../357-mac80211-optimize-skb-resizing.patch | 198 ++++++++++++++++++ ...ee80211_schedule_txq-schedule-empty-.patch | 105 ++++++++++ ...-un-schedule-TXQs-on-powersave-start.patch | 31 +++ 3 files changed, 334 insertions(+) create mode 100644 package/kernel/mac80211/patches/subsys/357-mac80211-optimize-skb-resizing.patch create mode 100644 package/kernel/mac80211/patches/subsys/358-mac80211-make-ieee80211_schedule_txq-schedule-empty-.patch create mode 100644 package/kernel/mac80211/patches/subsys/359-mac80211-un-schedule-TXQs-on-powersave-start.patch diff --git a/package/kernel/mac80211/patches/subsys/357-mac80211-optimize-skb-resizing.patch b/package/kernel/mac80211/patches/subsys/357-mac80211-optimize-skb-resizing.patch new file mode 100644 index 0000000000..8fd85fedcc --- /dev/null +++ b/package/kernel/mac80211/patches/subsys/357-mac80211-optimize-skb-resizing.patch @@ -0,0 +1,198 @@ +From: Felix Fietkau +Date: Sun, 17 Mar 2019 18:11:30 +0100 +Subject: [PATCH] mac80211: optimize skb resizing + +When forwarding unicast packets from ethernet to batman-adv over 802.11s +(with forwarding disabled), the typical required headroom to transmit +encrypted packets on mt76 is 32 (802.11) + 6 (802.11s) + 8 (CCMP) + +2 (padding) + 6 (LLC) + 18 (batman-adv) - 14 (old ethernet header) = 58 bytes. + +On systems where NET_SKB_PAD is 64 this leads to a call to pskb_expand_head +for every packet, since mac80211 also tries to allocate 16 bytes status +headroom for radiotap headers. + +This patch fixes these unnecessary reallocations by only requiring the extra +status headroom in ieee80211_tx_monitor() +If however a reallocation happens before that call, the status headroom gets +added there as well, in order to avoid double reallocation. + +The patch also cleans up the code by moving the headroom calculation to +ieee80211_skb_resize. + +Signed-off-by: Felix Fietkau +--- + +--- a/net/mac80211/ieee80211_i.h ++++ b/net/mac80211/ieee80211_i.h +@@ -1761,6 +1761,8 @@ void ieee80211_clear_fast_xmit(struct st + int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev, + const u8 *buf, size_t len, + const u8 *dest, __be16 proto, bool unencrypted); ++int ieee80211_skb_resize(struct ieee80211_sub_if_data *sdata, ++ struct sk_buff *skb, int hdrlen, int hdr_add); + + /* HT */ + void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata, +--- a/net/mac80211/status.c ++++ b/net/mac80211/status.c +@@ -672,6 +672,11 @@ void ieee80211_tx_monitor(struct ieee802 + } + } + ++ if (ieee80211_skb_resize(NULL, skb, 0, 0)) { ++ dev_kfree_skb(skb); ++ return; ++ } ++ + /* send frame to monitor interfaces now */ + rtap_len = ieee80211_tx_radiotap_len(info); + if (WARN_ON_ONCE(skb_headroom(skb) < rtap_len)) { +--- a/net/mac80211/tx.c ++++ b/net/mac80211/tx.c +@@ -1914,37 +1914,53 @@ static bool ieee80211_tx(struct ieee8021 + } + + /* device xmit handlers */ +- +-static int ieee80211_skb_resize(struct ieee80211_sub_if_data *sdata, +- struct sk_buff *skb, +- int head_need, bool may_encrypt) ++int ieee80211_skb_resize(struct ieee80211_sub_if_data *sdata, ++ struct sk_buff *skb, int hdr_len, int hdr_extra) + { + struct ieee80211_local *local = sdata->local; ++ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); + struct ieee80211_hdr *hdr; +- bool enc_tailroom; +- int tail_need = 0; ++ int head_need, head_max; ++ int tail_need, tail_max; ++ bool enc_tailroom = false; + +- hdr = (struct ieee80211_hdr *) skb->data; +- enc_tailroom = may_encrypt && +- (sdata->crypto_tx_tailroom_needed_cnt || +- ieee80211_is_mgmt(hdr->frame_control)); ++ if (sdata && !hdr_len && ++ !(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) { ++ hdr = (struct ieee80211_hdr *) skb->data; ++ enc_tailroom = (sdata->crypto_tx_tailroom_needed_cnt || ++ ieee80211_is_mgmt(hdr->frame_control)); ++ hdr_len += sdata->encrypt_headroom; ++ } + +- if (enc_tailroom) { +- tail_need = IEEE80211_ENCRYPT_TAILROOM; +- tail_need -= skb_tailroom(skb); +- tail_need = max_t(int, tail_need, 0); ++ head_need = head_max = hdr_len; ++ tail_need = tail_max = 0; ++ if (!sdata) { ++ head_need = head_max = local->tx_headroom; ++ } else { ++ head_max += hdr_extra; ++ head_max += max_t(int, local->tx_headroom, ++ local->hw.extra_tx_headroom); ++ head_need += local->hw.extra_tx_headroom; ++ ++ tail_max = IEEE80211_ENCRYPT_TAILROOM; ++ if (enc_tailroom) ++ tail_need = tail_max; + } + + if (skb_cloned(skb) && + (!ieee80211_hw_check(&local->hw, SUPPORTS_CLONED_SKBS) || + !skb_clone_writable(skb, ETH_HLEN) || enc_tailroom)) + I802_DEBUG_INC(local->tx_expand_skb_head_cloned); +- else if (head_need || tail_need) ++ else if (head_need > skb_headroom(skb) || ++ tail_need > skb_tailroom(skb)) + I802_DEBUG_INC(local->tx_expand_skb_head); + else + return 0; + +- if (pskb_expand_head(skb, head_need, tail_need, GFP_ATOMIC)) { ++ head_max = max_t(int, 0, head_max - skb_headroom(skb)); ++ tail_max = max_t(int, 0, tail_max - skb_tailroom(skb)); ++ ++ if (pskb_expand_head(skb, head_max, tail_max, GFP_ATOMIC)) { + wiphy_debug(local->hw.wiphy, + "failed to reallocate TX buffer\n"); + return -ENOMEM; +@@ -1960,18 +1976,8 @@ void ieee80211_xmit(struct ieee80211_sub + struct ieee80211_local *local = sdata->local; + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); + struct ieee80211_hdr *hdr; +- int headroom; +- bool may_encrypt; +- +- may_encrypt = !(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT); +- +- headroom = local->tx_headroom; +- if (may_encrypt) +- headroom += sdata->encrypt_headroom; +- headroom -= skb_headroom(skb); +- headroom = max_t(int, 0, headroom); + +- if (ieee80211_skb_resize(sdata, skb, headroom, may_encrypt)) { ++ if (ieee80211_skb_resize(sdata, skb, 0, 0)) { + ieee80211_free_txskb(&local->hw, skb); + return; + } +@@ -2740,30 +2746,14 @@ static struct sk_buff *ieee80211_build_h + + skb_pull(skb, skip_header_bytes); + padsize = ieee80211_hdr_padsize(&local->hw, hdrlen); +- head_need = hdrlen + encaps_len + meshhdrlen - skb_headroom(skb); ++ head_need = hdrlen + encaps_len + meshhdrlen; + head_need += padsize; + +- /* +- * So we need to modify the skb header and hence need a copy of +- * that. The head_need variable above doesn't, so far, include +- * the needed header space that we don't need right away. If we +- * can, then we don't reallocate right now but only after the +- * frame arrives at the master device (if it does...) +- * +- * If we cannot, however, then we will reallocate to include all +- * the ever needed space. Also, if we need to reallocate it anyway, +- * make it big enough for everything we may ever need. +- */ +- +- if (head_need > 0 || skb_cloned(skb)) { +- head_need += sdata->encrypt_headroom; +- head_need += local->tx_headroom; +- head_need = max_t(int, 0, head_need); +- if (ieee80211_skb_resize(sdata, skb, head_need, true)) { +- ieee80211_free_txskb(&local->hw, skb); +- skb = NULL; +- return ERR_PTR(-ENOMEM); +- } ++ if (ieee80211_skb_resize(sdata, skb, head_need, ++ sdata->encrypt_headroom)) { ++ ieee80211_free_txskb(&local->hw, skb); ++ skb = NULL; ++ return ERR_PTR(-ENOMEM); + } + + if (encaps_data) +@@ -3377,7 +3367,6 @@ static bool ieee80211_xmit_fast(struct i + struct ieee80211_local *local = sdata->local; + u16 ethertype = (skb->data[12] << 8) | skb->data[13]; + int extra_head = fast_tx->hdr_len - (ETH_HLEN - 2); +- int hw_headroom = sdata->local->hw.extra_tx_headroom; + struct ethhdr eth; + struct ieee80211_tx_info *info; + struct ieee80211_hdr *hdr = (void *)fast_tx->hdr; +@@ -3429,10 +3418,7 @@ static bool ieee80211_xmit_fast(struct i + * as the may-encrypt argument for the resize to not account for + * more room than we already have in 'extra_head' + */ +- if (unlikely(ieee80211_skb_resize(sdata, skb, +- max_t(int, extra_head + hw_headroom - +- skb_headroom(skb), 0), +- false))) { ++ if (unlikely(ieee80211_skb_resize(sdata, skb, extra_head, 0))) { + kfree_skb(skb); + return true; + } diff --git a/package/kernel/mac80211/patches/subsys/358-mac80211-make-ieee80211_schedule_txq-schedule-empty-.patch b/package/kernel/mac80211/patches/subsys/358-mac80211-make-ieee80211_schedule_txq-schedule-empty-.patch new file mode 100644 index 0000000000..a0a65f35a8 --- /dev/null +++ b/package/kernel/mac80211/patches/subsys/358-mac80211-make-ieee80211_schedule_txq-schedule-empty-.patch @@ -0,0 +1,105 @@ +From: Felix Fietkau +Date: Sun, 17 Mar 2019 14:26:59 +0100 +Subject: [PATCH] mac80211: make ieee80211_schedule_txq schedule empty TXQs + +Currently there is no way for the driver to signal to mac80211 that it should +schedule a TXQ even if there are no packets on the mac80211 part of that queue. +This is problematic if the driver has an internal retry queue to deal with +software A-MPDU retry. + +This patch changes the behavior of ieee80211_schedule_txq to always schedule +the queue, as its only user (ath9k) seems to expect such behavior already: +it calls this function on tx status and on powersave wakeup whenever its +internal retry queue is not empty. + +Also add an extra argument to ieee80211_return_txq to get the same behavior. + +This fixes an issue on ath9k where tx queues with packets to retry (and no +new packets in mac80211) would not get serviced. + +Fixes: 89cea7493a346 ("ath9k: Switch to mac80211 TXQ scheduling and airtime APIs") +Signed-off-by: Felix Fietkau +--- + +--- a/include/net/mac80211.h ++++ b/include/net/mac80211.h +@@ -6090,26 +6090,42 @@ static inline void ieee80211_txq_schedul + { + } + ++void __ieee80211_schedule_txq(struct ieee80211_hw *hw, ++ struct ieee80211_txq *txq, bool force); ++ + /** + * ieee80211_schedule_txq - schedule a TXQ for transmission + * + * @hw: pointer as obtained from ieee80211_alloc_hw() + * @txq: pointer obtained from station or virtual interface + * +- * Schedules a TXQ for transmission if it is not already scheduled. ++ * Schedules a TXQ for transmission if it is not already scheduled, ++ * even if mac80211 does not have any packets buffered. ++ * ++ * The driver may call this function if it has buffered packets for ++ * this TXQ internally. + */ +-void ieee80211_schedule_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq); ++static inline void ++ieee80211_schedule_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq) ++{ ++ __ieee80211_schedule_txq(hw, txq, true); ++} + + /** + * ieee80211_return_txq - return a TXQ previously acquired by ieee80211_next_txq() + * + * @hw: pointer as obtained from ieee80211_alloc_hw() + * @txq: pointer obtained from station or virtual interface ++ * @force: schedule txq even if mac80211 does not have any buffered packets. ++ * ++ * The driver may set force=true if it has buffered packets for this TXQ ++ * internally. + */ + static inline void +-ieee80211_return_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq) ++ieee80211_return_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq, ++ bool force) + { +- ieee80211_schedule_txq(hw, txq); ++ __ieee80211_schedule_txq(hw, txq, force); + } + + /** +--- a/net/mac80211/tx.c ++++ b/net/mac80211/tx.c +@@ -3655,8 +3655,9 @@ out: + } + EXPORT_SYMBOL(ieee80211_next_txq); + +-void ieee80211_schedule_txq(struct ieee80211_hw *hw, +- struct ieee80211_txq *txq) ++void __ieee80211_schedule_txq(struct ieee80211_hw *hw, ++ struct ieee80211_txq *txq, ++ bool force) + { + struct ieee80211_local *local = hw_to_local(hw); + struct txq_info *txqi = to_txq_info(txq); +@@ -3664,7 +3665,8 @@ void ieee80211_schedule_txq(struct ieee8 + spin_lock_bh(&local->active_txq_lock[txq->ac]); + + if (list_empty(&txqi->schedule_order) && +- (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) { ++ (force || !skb_queue_empty(&txqi->frags) || ++ txqi->tin.backlog_packets)) { + /* If airtime accounting is active, always enqueue STAs at the + * head of the list to ensure that they only get moved to the + * back by the airtime DRR scheduler once they have a negative +@@ -3684,7 +3686,7 @@ void ieee80211_schedule_txq(struct ieee8 + + spin_unlock_bh(&local->active_txq_lock[txq->ac]); + } +-EXPORT_SYMBOL(ieee80211_schedule_txq); ++EXPORT_SYMBOL(__ieee80211_schedule_txq); + + bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw, + struct ieee80211_txq *txq) diff --git a/package/kernel/mac80211/patches/subsys/359-mac80211-un-schedule-TXQs-on-powersave-start.patch b/package/kernel/mac80211/patches/subsys/359-mac80211-un-schedule-TXQs-on-powersave-start.patch new file mode 100644 index 0000000000..1abb2db7c2 --- /dev/null +++ b/package/kernel/mac80211/patches/subsys/359-mac80211-un-schedule-TXQs-on-powersave-start.patch @@ -0,0 +1,31 @@ +From: Felix Fietkau +Date: Tue, 19 Mar 2019 11:36:12 +0100 +Subject: [PATCH] mac80211: un-schedule TXQs on powersave start + +Once a station enters powersave, its queues should not be returned by +ieee80211_next_txq() anymore. They will be re-scheduled again after the +station has woken up again + +Fixes: 1866760096bf4 ("mac80211: Add TXQ scheduling API") +Signed-off-by: Felix Fietkau +--- + +--- a/net/mac80211/rx.c ++++ b/net/mac80211/rx.c +@@ -1508,7 +1508,15 @@ static void sta_ps_start(struct sta_info + return; + + for (tid = 0; tid < ARRAY_SIZE(sta->sta.txq); tid++) { +- if (txq_has_queue(sta->sta.txq[tid])) ++ struct ieee80211_txq *txq = sta->sta.txq[tid]; ++ struct txq_info *txqi = to_txq_info(txq); ++ ++ spin_lock(&local->active_txq_lock[txq->ac]); ++ if (!list_empty(&txqi->schedule_order)) ++ list_del_init(&txqi->schedule_order); ++ spin_unlock(&local->active_txq_lock[txq->ac]); ++ ++ if (txq_has_queue(txq)) + set_bit(tid, &sta->txq_buffered_tids); + else + clear_bit(tid, &sta->txq_buffered_tids);