diff mbox series

mac80211: fix throughput LED trigger

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

Commit Message

Felix Fietkau Nov. 12, 2021, 10:56 a.m. UTC 
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(-)

Comments

kernel test robot Nov. 12, 2021, 1:07 p.m. UTC | #1 
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
kernel test robot Nov. 12, 2021, 11:18 p.m. UTC | #2 
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 mbox series

Patch

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;