mac80211: fix an issue with the TXQ scheduling API and powersave clients
This commit is contained in:
parent
6c1c7e4a1e
commit
f0dd52f007
@ -0,0 +1,198 @@
|
||||
From: Felix Fietkau <nbd@nbd.name>
|
||||
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 <nbd@nbd.name>
|
||||
---
|
||||
|
||||
--- 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;
|
||||
}
|
||||
@ -0,0 +1,105 @@
|
||||
From: Felix Fietkau <nbd@nbd.name>
|
||||
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 <nbd@nbd.name>
|
||||
---
|
||||
|
||||
--- 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)
|
||||
@ -0,0 +1,31 @@
|
||||
From: Felix Fietkau <nbd@nbd.name>
|
||||
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 <nbd@nbd.name>
|
||||
---
|
||||
|
||||
--- 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);
|
||||
Loading…
Reference in New Issue
Block a user