2021-11-12 10:56:19

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH] mac80211: fix throughput LED trigger

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 <[email protected]>
---
net/mac80211/led.h | 8 ++++----
net/mac80211/rx.c | 7 ++++---
net/mac80211/tx.c | 32 +++++++++++++++-----------------
3 files changed, 23 insertions(+), 24 deletions(-)

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;
--
2.30.1



2021-11-12 13:07:48

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix throughput LED trigger

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 <[email protected]>

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/[email protected]


Attachments:
(No filename) (5.89 kB)
.config.gz (28.66 kB)
Download all attachments

2021-11-12 23:18:40

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix throughput LED trigger

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 <[email protected]>

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/[email protected]


Attachments:
(No filename) (6.01 kB)
.config.gz (65.07 kB)
Download all attachments