mac80211: fix an issue with the TXQ scheduling API and powersave clients

hemuzhao · Mar 21, 2019 · f0dd52f · f0dd52f
1 parent 6c1c7e4
commit f0dd52f
Show file tree

Hide file tree

Showing 3 changed files with 334 additions and 0 deletions.
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
@@ -0,0 +1,198 @@
+From: Felix Fietkau <[email protected]>
+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 <[email protected]>
+---
+
+--- 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/...el/mac80211/patches/subsys/358-mac80211-make-ieee80211_schedule_txq-schedule-empty-.patch b/...el/mac80211/patches/subsys/358-mac80211-make-ieee80211_schedule_txq-schedule-empty-.patch
@@ -0,0 +1,105 @@
+From: Felix Fietkau <[email protected]>
+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 <[email protected]>
+---
+
+--- 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/...age/kernel/mac80211/patches/subsys/359-mac80211-un-schedule-TXQs-on-powersave-start.patch b/...age/kernel/mac80211/patches/subsys/359-mac80211-un-schedule-TXQs-on-powersave-start.patch
@@ -0,0 +1,31 @@
+From: Felix Fietkau <[email protected]>
+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 <[email protected]>
+---
+
+--- 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);