Message ID | 20211112105611.99991-1-nbd@nbd.name (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Johannes Berg |
Headers | show |
Series | mac80211: fix throughput LED trigger | expand |
Hi Felix,
I love your patch! Perhaps something to improve:
[auto build test WARNING on jberg-mac80211-next/master]
[also build test WARNING on v5.15 next-20211112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Felix-Fietkau/mac80211-fix-throughput-LED-trigger/20211112-185656
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: i386-defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/e7c97026e87b3302843d614f414335befbeab31e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Felix-Fietkau/mac80211-fix-throughput-LED-trigger/20211112-185656
git checkout e7c97026e87b3302843d614f414335befbeab31e
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
net/mac80211/tx.c: In function '__ieee80211_tx':
>> net/mac80211/tx.c:1732:9: warning: variable 'fc' set but not used [-Wunused-but-set-variable]
1732 | __le16 fc;
| ^~
vim +/fc +1732 net/mac80211/tx.c
11127e9121d4dd Johannes Berg 2011-11-16 1719
11127e9121d4dd Johannes Berg 2011-11-16 1720 /*
11127e9121d4dd Johannes Berg 2011-11-16 1721 * Returns false if the frame couldn't be transmitted but was queued instead.
11127e9121d4dd Johannes Berg 2011-11-16 1722 */
11127e9121d4dd Johannes Berg 2011-11-16 1723 static bool __ieee80211_tx(struct ieee80211_local *local,
e7c97026e87b33 Felix Fietkau 2021-11-12 1724 struct sk_buff_head *skbs, struct sta_info *sta,
e7c97026e87b33 Felix Fietkau 2021-11-12 1725 bool txpending)
11127e9121d4dd Johannes Berg 2011-11-16 1726 {
11127e9121d4dd Johannes Berg 2011-11-16 1727 struct ieee80211_tx_info *info;
11127e9121d4dd Johannes Berg 2011-11-16 1728 struct ieee80211_sub_if_data *sdata;
11127e9121d4dd Johannes Berg 2011-11-16 1729 struct ieee80211_vif *vif;
11127e9121d4dd Johannes Berg 2011-11-16 1730 struct sk_buff *skb;
958574cbcc3ae3 Colin Ian King 2021-03-28 1731 bool result;
11127e9121d4dd Johannes Berg 2011-11-16 @1732 __le16 fc;
11127e9121d4dd Johannes Berg 2011-11-16 1733
11127e9121d4dd Johannes Berg 2011-11-16 1734 if (WARN_ON(skb_queue_empty(skbs)))
11127e9121d4dd Johannes Berg 2011-11-16 1735 return true;
11127e9121d4dd Johannes Berg 2011-11-16 1736
11127e9121d4dd Johannes Berg 2011-11-16 1737 skb = skb_peek(skbs);
11127e9121d4dd Johannes Berg 2011-11-16 1738 fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
11127e9121d4dd Johannes Berg 2011-11-16 1739 info = IEEE80211_SKB_CB(skb);
5061b0c2b9066d Johannes Berg 2009-07-14 1740 sdata = vif_to_sdata(info->control.vif);
11127e9121d4dd Johannes Berg 2011-11-16 1741 if (sta && !sta->uploaded)
11127e9121d4dd Johannes Berg 2011-11-16 1742 sta = NULL;
11127e9121d4dd Johannes Berg 2011-11-16 1743
5061b0c2b9066d Johannes Berg 2009-07-14 1744 switch (sdata->vif.type) {
5061b0c2b9066d Johannes Berg 2009-07-14 1745 case NL80211_IFTYPE_MONITOR:
d82121845d4433 Aviya Erenfeld 2016-08-29 1746 if (sdata->u.mntr.flags & MONITOR_FLAG_ACTIVE) {
c82b5a74cc7393 Johannes Berg 2013-07-14 1747 vif = &sdata->vif;
c82b5a74cc7393 Johannes Berg 2013-07-14 1748 break;
c82b5a74cc7393 Johannes Berg 2013-07-14 1749 }
4b6f1dd6a6faf4 Johannes Berg 2012-04-03 1750 sdata = rcu_dereference(local->monitor_sdata);
3a25a8c8b75b43 Johannes Berg 2012-04-03 1751 if (sdata) {
4b6f1dd6a6faf4 Johannes Berg 2012-04-03 1752 vif = &sdata->vif;
3a25a8c8b75b43 Johannes Berg 2012-04-03 1753 info->hw_queue =
3a25a8c8b75b43 Johannes Berg 2012-04-03 1754 vif->hw_queue[skb_get_queue_mapping(skb)];
30686bf7f5b3c3 Johannes Berg 2015-06-02 1755 } else if (ieee80211_hw_check(&local->hw, QUEUE_CONTROL)) {
63b4d8b3736b83 Johannes Berg 2015-11-24 1756 ieee80211_purge_tx_queue(&local->hw, skbs);
3a25a8c8b75b43 Johannes Berg 2012-04-03 1757 return true;
3a25a8c8b75b43 Johannes Berg 2012-04-03 1758 } else
11127e9121d4dd Johannes Berg 2011-11-16 1759 vif = NULL;
5061b0c2b9066d Johannes Berg 2009-07-14 1760 break;
5061b0c2b9066d Johannes Berg 2009-07-14 1761 case NL80211_IFTYPE_AP_VLAN:
11127e9121d4dd Johannes Berg 2011-11-16 1762 sdata = container_of(sdata->bss,
11127e9121d4dd Johannes Berg 2011-11-16 1763 struct ieee80211_sub_if_data, u.ap);
fc0561dc6a9c61 Gustavo A. R. Silva 2020-07-07 1764 fallthrough;
5061b0c2b9066d Johannes Berg 2009-07-14 1765 default:
11127e9121d4dd Johannes Berg 2011-11-16 1766 vif = &sdata->vif;
5061b0c2b9066d Johannes Berg 2009-07-14 1767 break;
5061b0c2b9066d Johannes Berg 2009-07-14 1768 }
5061b0c2b9066d Johannes Berg 2009-07-14 1769
4fd0328d2f6314 Johannes Berg 2019-10-01 1770 result = ieee80211_tx_frags(local, vif, sta, skbs, txpending);
21f5fc75deca63 Luis R. Rodriguez 2009-07-24 1771
4db4e0a17fb0e7 Johannes Berg 2011-11-24 1772 WARN_ON_ONCE(!skb_queue_empty(skbs));
252b86c43225d0 Johannes Berg 2011-11-16 1773
11127e9121d4dd Johannes Berg 2011-11-16 1774 return result;
e2ebc74d7e3d71 Johannes Berg 2007-07-27 1775 }
e2ebc74d7e3d71 Johannes Berg 2007-07-27 1776
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Felix,
I love your patch! Yet something to improve:
[auto build test ERROR on jberg-mac80211-next/master]
[also build test ERROR on jberg-mac80211/master v5.15 next-20211112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Felix-Fietkau/mac80211-fix-throughput-LED-trigger/20211112-185656
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/e7c97026e87b3302843d614f414335befbeab31e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Felix-Fietkau/mac80211-fix-throughput-LED-trigger/20211112-185656
git checkout e7c97026e87b3302843d614f414335befbeab31e
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/mac80211/tx.c: In function '__ieee80211_tx':
>> net/mac80211/tx.c:1732:9: error: variable 'fc' set but not used [-Werror=unused-but-set-variable]
1732 | __le16 fc;
| ^~
cc1: all warnings being treated as errors
vim +/fc +1732 net/mac80211/tx.c
11127e9121d4dd9 Johannes Berg 2011-11-16 1719
11127e9121d4dd9 Johannes Berg 2011-11-16 1720 /*
11127e9121d4dd9 Johannes Berg 2011-11-16 1721 * Returns false if the frame couldn't be transmitted but was queued instead.
11127e9121d4dd9 Johannes Berg 2011-11-16 1722 */
11127e9121d4dd9 Johannes Berg 2011-11-16 1723 static bool __ieee80211_tx(struct ieee80211_local *local,
e7c97026e87b330 Felix Fietkau 2021-11-12 1724 struct sk_buff_head *skbs, struct sta_info *sta,
e7c97026e87b330 Felix Fietkau 2021-11-12 1725 bool txpending)
11127e9121d4dd9 Johannes Berg 2011-11-16 1726 {
11127e9121d4dd9 Johannes Berg 2011-11-16 1727 struct ieee80211_tx_info *info;
11127e9121d4dd9 Johannes Berg 2011-11-16 1728 struct ieee80211_sub_if_data *sdata;
11127e9121d4dd9 Johannes Berg 2011-11-16 1729 struct ieee80211_vif *vif;
11127e9121d4dd9 Johannes Berg 2011-11-16 1730 struct sk_buff *skb;
958574cbcc3ae31 Colin Ian King 2021-03-28 1731 bool result;
11127e9121d4dd9 Johannes Berg 2011-11-16 @1732 __le16 fc;
11127e9121d4dd9 Johannes Berg 2011-11-16 1733
11127e9121d4dd9 Johannes Berg 2011-11-16 1734 if (WARN_ON(skb_queue_empty(skbs)))
11127e9121d4dd9 Johannes Berg 2011-11-16 1735 return true;
11127e9121d4dd9 Johannes Berg 2011-11-16 1736
11127e9121d4dd9 Johannes Berg 2011-11-16 1737 skb = skb_peek(skbs);
11127e9121d4dd9 Johannes Berg 2011-11-16 1738 fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
11127e9121d4dd9 Johannes Berg 2011-11-16 1739 info = IEEE80211_SKB_CB(skb);
5061b0c2b9066de Johannes Berg 2009-07-14 1740 sdata = vif_to_sdata(info->control.vif);
11127e9121d4dd9 Johannes Berg 2011-11-16 1741 if (sta && !sta->uploaded)
11127e9121d4dd9 Johannes Berg 2011-11-16 1742 sta = NULL;
11127e9121d4dd9 Johannes Berg 2011-11-16 1743
5061b0c2b9066de Johannes Berg 2009-07-14 1744 switch (sdata->vif.type) {
5061b0c2b9066de Johannes Berg 2009-07-14 1745 case NL80211_IFTYPE_MONITOR:
d82121845d44334 Aviya Erenfeld 2016-08-29 1746 if (sdata->u.mntr.flags & MONITOR_FLAG_ACTIVE) {
c82b5a74cc73938 Johannes Berg 2013-07-14 1747 vif = &sdata->vif;
c82b5a74cc73938 Johannes Berg 2013-07-14 1748 break;
c82b5a74cc73938 Johannes Berg 2013-07-14 1749 }
4b6f1dd6a6faf4e Johannes Berg 2012-04-03 1750 sdata = rcu_dereference(local->monitor_sdata);
3a25a8c8b75b430 Johannes Berg 2012-04-03 1751 if (sdata) {
4b6f1dd6a6faf4e Johannes Berg 2012-04-03 1752 vif = &sdata->vif;
3a25a8c8b75b430 Johannes Berg 2012-04-03 1753 info->hw_queue =
3a25a8c8b75b430 Johannes Berg 2012-04-03 1754 vif->hw_queue[skb_get_queue_mapping(skb)];
30686bf7f5b3c30 Johannes Berg 2015-06-02 1755 } else if (ieee80211_hw_check(&local->hw, QUEUE_CONTROL)) {
63b4d8b3736b831 Johannes Berg 2015-11-24 1756 ieee80211_purge_tx_queue(&local->hw, skbs);
3a25a8c8b75b430 Johannes Berg 2012-04-03 1757 return true;
3a25a8c8b75b430 Johannes Berg 2012-04-03 1758 } else
11127e9121d4dd9 Johannes Berg 2011-11-16 1759 vif = NULL;
5061b0c2b9066de Johannes Berg 2009-07-14 1760 break;
5061b0c2b9066de Johannes Berg 2009-07-14 1761 case NL80211_IFTYPE_AP_VLAN:
11127e9121d4dd9 Johannes Berg 2011-11-16 1762 sdata = container_of(sdata->bss,
11127e9121d4dd9 Johannes Berg 2011-11-16 1763 struct ieee80211_sub_if_data, u.ap);
fc0561dc6a9c616 Gustavo A. R. Silva 2020-07-07 1764 fallthrough;
5061b0c2b9066de Johannes Berg 2009-07-14 1765 default:
11127e9121d4dd9 Johannes Berg 2011-11-16 1766 vif = &sdata->vif;
5061b0c2b9066de Johannes Berg 2009-07-14 1767 break;
5061b0c2b9066de Johannes Berg 2009-07-14 1768 }
5061b0c2b9066de Johannes Berg 2009-07-14 1769
4fd0328d2f6314a Johannes Berg 2019-10-01 1770 result = ieee80211_tx_frags(local, vif, sta, skbs, txpending);
21f5fc75deca63b Luis R. Rodriguez 2009-07-24 1771
4db4e0a17fb0e7b Johannes Berg 2011-11-24 1772 WARN_ON_ONCE(!skb_queue_empty(skbs));
252b86c43225d06 Johannes Berg 2011-11-16 1773
11127e9121d4dd9 Johannes Berg 2011-11-16 1774 return result;
e2ebc74d7e3d716 Johannes Berg 2007-07-27 1775 }
e2ebc74d7e3d716 Johannes Berg 2007-07-27 1776
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/net/mac80211/led.h b/net/mac80211/led.h index fb3aaa3c5606..b71a1428d883 100644 --- a/net/mac80211/led.h +++ b/net/mac80211/led.h @@ -72,19 +72,19 @@ static inline void ieee80211_mod_tpt_led_trig(struct ieee80211_local *local, #endif static inline void -ieee80211_tpt_led_trig_tx(struct ieee80211_local *local, __le16 fc, int bytes) +ieee80211_tpt_led_trig_tx(struct ieee80211_local *local, int bytes) { #ifdef CONFIG_MAC80211_LEDS - if (ieee80211_is_data(fc) && atomic_read(&local->tpt_led_active)) + if (atomic_read(&local->tpt_led_active)) local->tpt_led_trigger->tx_bytes += bytes; #endif } static inline void -ieee80211_tpt_led_trig_rx(struct ieee80211_local *local, __le16 fc, int bytes) +ieee80211_tpt_led_trig_rx(struct ieee80211_local *local, int bytes) { #ifdef CONFIG_MAC80211_LEDS - if (ieee80211_is_data(fc) && atomic_read(&local->tpt_led_active)) + if (atomic_read(&local->tpt_led_active)) local->tpt_led_trigger->rx_bytes += bytes; #endif } diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index fc5c608d02e2..3079328a30e8 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -4863,6 +4863,7 @@ void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta, struct ieee80211_rate *rate = NULL; struct ieee80211_supported_band *sband; struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb); + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; WARN_ON_ONCE(softirq_count() == 0); @@ -4959,9 +4960,9 @@ void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta, if (!(status->flag & RX_FLAG_8023)) skb = ieee80211_rx_monitor(local, skb, rate); if (skb) { - ieee80211_tpt_led_trig_rx(local, - ((struct ieee80211_hdr *)skb->data)->frame_control, - skb->len); + if ((status->flag & RX_FLAG_8023) || + ieee80211_is_data_present(hdr->frame_control)) + ieee80211_tpt_led_trig_rx(local, skb->len); if (status->flag & RX_FLAG_8023) __ieee80211_rx_handle_8023(hw, pubsta, skb, list); diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index a756a197c770..5b968d0793a7 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -1721,8 +1721,8 @@ static bool ieee80211_tx_frags(struct ieee80211_local *local, * Returns false if the frame couldn't be transmitted but was queued instead. */ static bool __ieee80211_tx(struct ieee80211_local *local, - struct sk_buff_head *skbs, int led_len, - struct sta_info *sta, bool txpending) + struct sk_buff_head *skbs, struct sta_info *sta, + bool txpending) { struct ieee80211_tx_info *info; struct ieee80211_sub_if_data *sdata; @@ -1769,8 +1769,6 @@ static bool __ieee80211_tx(struct ieee80211_local *local, result = ieee80211_tx_frags(local, vif, sta, skbs, txpending); - ieee80211_tpt_led_trig_tx(local, fc, led_len); - WARN_ON_ONCE(!skb_queue_empty(skbs)); return result; @@ -1920,7 +1918,6 @@ static bool ieee80211_tx(struct ieee80211_sub_if_data *sdata, ieee80211_tx_result res_prepare; struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); bool result = true; - int led_len; if (unlikely(skb->len < 10)) { dev_kfree_skb(skb); @@ -1928,7 +1925,6 @@ static bool ieee80211_tx(struct ieee80211_sub_if_data *sdata, } /* initialises tx */ - led_len = skb->len; res_prepare = ieee80211_tx_prepare(sdata, &tx, sta, skb); if (unlikely(res_prepare == TX_DROP)) { @@ -1951,8 +1947,7 @@ static bool ieee80211_tx(struct ieee80211_sub_if_data *sdata, return true; if (!invoke_tx_handlers_late(&tx)) - result = __ieee80211_tx(local, &tx.skbs, led_len, - tx.sta, txpending); + result = __ieee80211_tx(local, &tx.skbs, tx.sta, txpending); return result; } @@ -4175,6 +4170,7 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb, struct ieee80211_local *local = sdata->local; struct sta_info *sta; struct sk_buff *next; + int len = skb->len; if (unlikely(skb->len < ETH_HLEN)) { kfree_skb(skb); @@ -4221,10 +4217,8 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb, } } else { /* we cannot process non-linear frames on this path */ - if (skb_linearize(skb)) { - kfree_skb(skb); - goto out; - } + if (skb_linearize(skb)) + goto out_free; /* the frame could be fragmented, software-encrypted, and other * things so we cannot really handle checksum offload with it - @@ -4258,7 +4252,10 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb, goto out; out_free: kfree_skb(skb); + len = 0; out: + if (len) + ieee80211_tpt_led_trig_tx(local, len); rcu_read_unlock(); } @@ -4396,8 +4393,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, } static bool ieee80211_tx_8023(struct ieee80211_sub_if_data *sdata, - struct sk_buff *skb, int led_len, - struct sta_info *sta, + struct sk_buff *skb, struct sta_info *sta, bool txpending) { struct ieee80211_local *local = sdata->local; @@ -4410,6 +4406,8 @@ static bool ieee80211_tx_8023(struct ieee80211_sub_if_data *sdata, if (sta) sk_pacing_shift_update(skb->sk, local->hw.tx_sk_pacing_shift); + ieee80211_tpt_led_trig_tx(local, skb->len); + if (ieee80211_queue_skb(local, sdata, sta, skb)) return true; @@ -4498,7 +4496,7 @@ static void ieee80211_8023_xmit(struct ieee80211_sub_if_data *sdata, if (key) info->control.hw_key = &key->conf; - ieee80211_tx_8023(sdata, skb, skb->len, sta, false); + ieee80211_tx_8023(sdata, skb, sta, false); return; @@ -4637,7 +4635,7 @@ static bool ieee80211_tx_pending_skb(struct ieee80211_local *local, if (IS_ERR(sta) || (sta && !sta->uploaded)) sta = NULL; - result = ieee80211_tx_8023(sdata, skb, skb->len, sta, true); + result = ieee80211_tx_8023(sdata, skb, sta, true); } else { struct sk_buff_head skbs; @@ -4647,7 +4645,7 @@ static bool ieee80211_tx_pending_skb(struct ieee80211_local *local, hdr = (struct ieee80211_hdr *)skb->data; sta = sta_info_get(sdata, hdr->addr1); - result = __ieee80211_tx(local, &skbs, skb->len, sta, true); + result = __ieee80211_tx(local, &skbs, sta, true); } return result;
The codepaths for rx with decap offload and tx with itxq were not updating the counters for the throughput led trigger. Signed-off-by: Felix Fietkau <nbd@nbd.name> --- net/mac80211/led.h | 8 ++++---- net/mac80211/rx.c | 7 ++++--- net/mac80211/tx.c | 32 +++++++++++++++----------------- 3 files changed, 23 insertions(+), 24 deletions(-)