backports: mac80211: revert to old stats behaviour for older kernels

PATCH In review Arend van Spriel 2 participants 5 messages May 1, 2015 – May 4, 2015

Share & tools

Arend van Spriel

author

2015-05-01 13:46 UTC #0 lore raw

PATCH 1/1 backports: mac80211: revert to old stats behaviour for older kernels

Commit 5a490510ba5f ("mac80211: use per-CPU TX/RX statistics") introduced the use of per-cpu statistics, ie. struct netdevice::tstats. This is not supported in kernel 3.14 and older.

Signed-off-by Arend van Spriel <[email protected]>

---
I am not entirely sure whether 3.14 is the correct version to check for. The functionality seems introduced there but there were some subsequent patches on the per-cpu netdev stats functionality.

Regards,
Arend

---
 ...ac80211-revert-to-old-stats-behaviour-for.patch | 127 +++++++++++++++++++++
 1 file changed, 127 insertions(+)
 create mode 100644 patches/0000-upstream-backport-changes/0001-backports-mac80211-revert-to-old-stats-behaviour-for.patch

patches/0000-upstream-backport-changes/0001-backports-mac80211-revert-to-old-stats-behaviour-for.patch new file +127 -0

new file mode 100644
index 0000000..2b91a11
@@ -0,0 +1,127 @@
+From 92115bd3d3c3988fafe29053e8bb28022e809ca5 Mon Sep 17 00:00:00 2001
+From: Arend van Spriel <[email protected]>
+Date: Thu, 30 Apr 2015 20:57:51 +0200
+Subject: [PATCH] backports: mac80211: revert to old stats behaviour for older
+ kernels
+
+The per_cpu stats were added since 3.15. Just revert to old behaviour
+for older kernels.
+
+Signed-off-by: Arend van Spriel <[email protected]>
+---
+ net/mac80211/iface.c | 15 +++++++++++----
+ net/mac80211/rx.c    |  5 +++++
+ net/mac80211/tx.c    |  5 +++++
+ 3 files changed, 21 insertions(+), 4 deletions(-)
+
+diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
+index 4ee8fea..60e446e 100644
+--- a/net/mac80211/iface.c
++++ b/net/mac80211/iface.c
+@@ -1096,6 +1096,7 @@ static u16 ieee80211_netdev_select_queue(struct net_device *dev,
+ 	return ieee80211_select_queue(IEEE80211_DEV_TO_SUB_IF(dev), skb);
+ }
+ 
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0)
+ static struct rtnl_link_stats64 *
+ ieee80211_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
+ {
+@@ -1124,6 +1125,10 @@ ieee80211_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
+ 
+ 	return stats;
+ }
++#define GET_STATS64(_fp)	_fp
++#else
++#define GET_STATS64(_fp)	NULL
++#endif
+ 
+ static const struct net_device_ops ieee80211_dataif_ops = {
+ 	.ndo_open		= ieee80211_open,
+@@ -1134,7 +1139,7 @@ static const struct net_device_ops ieee80211_dataif_ops = {
+ 	.ndo_change_mtu 	= ieee80211_change_mtu,
+ 	.ndo_set_mac_address 	= ieee80211_change_mac,
+ 	.ndo_select_queue	= ieee80211_netdev_select_queue,
+-	.ndo_get_stats64	= ieee80211_get_stats64,
++	.ndo_get_stats64	= GET_STATS64(ieee80211_get_stats64),
+ };
+ 
+ static u16 ieee80211_monitor_select_queue(struct net_device *dev,
+@@ -1168,12 +1173,14 @@ static const struct net_device_ops ieee80211_monitorif_ops = {
+ 	.ndo_change_mtu 	= ieee80211_change_mtu,
+ 	.ndo_set_mac_address 	= ieee80211_change_mac,
+ 	.ndo_select_queue	= ieee80211_monitor_select_queue,
+-	.ndo_get_stats64	= ieee80211_get_stats64,
++	.ndo_get_stats64	= GET_STATS64(ieee80211_get_stats64),
+ };
+ 
+ static void ieee80211_if_free(struct net_device *dev)
+ {
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0)
+ 	free_percpu(dev->tstats);
++#endif
+ 	free_netdev(dev);
+ }
+ 
+@@ -1722,13 +1729,13 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
+ 		if (!ndev)
+ 			return -ENOMEM;
+ 		dev_net_set(ndev, wiphy_net(local->hw.wiphy));
+-
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0)
+ 		ndev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
+ 		if (!ndev->tstats) {
+ 			free_netdev(ndev);
+ 			return -ENOMEM;
+ 		}
+-
++#endif
+ 		ndev->needed_headroom = local->tx_headroom +
+ 					4*6 /* four MAC addresses */
+ 					+ 2 + 2 + 2 + 2 /* ctl, dur, seq, qos */
+diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
+index e082535..0e15823 100644
+--- a/net/mac80211/rx.c
++++ b/net/mac80211/rx.c
+@@ -34,12 +34,17 @@
+ 
+ static inline void ieee80211_rx_stats(struct net_device *dev, u32 len)
+ {
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0)
+ 	struct pcpu_sw_netstats *tstats = this_cpu_ptr(dev->tstats);
+ 
+ 	u64_stats_update_begin(&tstats->syncp);
+ 	tstats->rx_packets++;
+ 	tstats->rx_bytes += len;
+ 	u64_stats_update_end(&tstats->syncp);
++#else
++	dev->stats.rx_packets++;
++	dev->stats.rx_bytes += len;
++#endif
+ }
+ 
+ /*
+diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
+index 745fdf5..7b77c93 100644
+--- a/net/mac80211/tx.c
++++ b/net/mac80211/tx.c
+@@ -39,12 +39,17 @@
+ 
+ static inline void ieee80211_tx_stats(struct net_device *dev, u32 len)
+ {
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0)
+ 	struct pcpu_sw_netstats *tstats = this_cpu_ptr(dev->tstats);
+ 
+ 	u64_stats_update_begin(&tstats->syncp);
+ 	tstats->tx_packets++;
+ 	tstats->tx_bytes += len;
+ 	u64_stats_update_end(&tstats->syncp);
++#else
++	dev->stats.tx_packets++;
++	dev->stats.tx_bytes += len;
++#endif
+ }
+ 
+ static __le16 ieee80211_duration(struct ieee80211_tx_data *tx,
+-- 
+1.9.1
+

Signature · 1 line

1.9.1

Johannes Berg

2015-05-03 19:07 UTC #1 lore raw

Quoted 9 lines · Arend van Spriel

Commit 5a490510ba5f ("mac80211: use per-CPU TX/RX statistics") introduced the use of per-cpu statistics, ie. struct netdevice::tstats. This is not supported in kernel 3.14 and older.
Signed-off-by Arend van Spriel <[email protected]>
---
I am not entirely sure whether 3.14 is the correct version to check for. The functionality seems introduced there but there were some subsequent patches on the per-cpu netdev stats functionality.

I don't really like just reverting to the old behaviour - that'll make this very hard to port.

What part of this isn't supported? Is there no pointer whatsoever that we can use? We still implement the ndo_getstats64 [or whatever it's called] ourselves anyway, so we don't really use any API here apart from
* netdev_alloc_pcpu_stats, which is trivially backported
* dev->tstats, which is just a single pointer and we could use anything
else,
even perhaps in sdata

johannes

Arend van Spriel

author

2015-05-03 19:49 UTC #2 lore raw

Quoted 5 lines · Johannes Berg

Quoted 4 lines · Arend van Spriel
Commit 5a490510ba5f ("mac80211: use per-CPU TX/RX statistics") introduced the use of per-cpu statistics, ie. struct netdevice::tstats. This is not supported in kernel 3.14 and older.

Quoted 8 lines

Quoted 5 lines
Signed-off-by Arend van Spriel<[email protected]>
---
I am not entirely sure whether 3.14 is the correct version to check for. The functionality seems introduced there but there were some subsequent patches on the per-cpu netdev stats functionality.
I don't really like just reverting to the old behaviour - that'll make this very hard to port.

I don't either, but I had a bunch of test systems that I wanted to get going.

Quoted 7 lines

What part of this isn't supported? Is there no pointer whatsoever that we can use? We still implement the ndo_getstats64 [or whatever it's called] ourselves anyway, so we don't really use any API here apart from
* netdev_alloc_pcpu_stats, which is trivially backported
* dev->tstats, which is just a single pointer and we could use anything
else,
even perhaps in sdata

I have to learn about the per-cpu operations someday so why not now while spending two weeks vacation ;-)

Thanks,
Arend

Johannes Berg

2015-05-04 07:43 UTC #3 lore raw

Quoted 2 lines · Arend van Spriel

I don't either, but I had a bunch of test systems that I wanted to get going.

While on vacation? :)

Quoted 10 lines

Quoted 7 lines
What part of this isn't supported? Is there no pointer whatsoever that we can use? We still implement the ndo_getstats64 [or whatever it's called] ourselves anyway, so we don't really use any API here apart from
* netdev_alloc_pcpu_stats, which is trivially backported
* dev->tstats, which is just a single pointer and we could use anything
else,
even perhaps in sdata
I have to learn about the per-cpu operations someday so why not now while spending two weeks vacation ;-)

Heh. Go enjoy your vacation, I can handle this :)

I think you arrived at kernel 3.14 due to 8f84985fec10d. However, I think we can just do the following

* backport "struct pcpu_sw_netstats" (needed on < 3.14)
* backport "netdev_alloc_pcpu_stats" (needed on < 3.15)
* make an spatch replacing dev->tstats with ((struct pcpu_sw_netstats *)dev->ml_priv) (since ml_priv has existed for a long time, and they're in a union)

johannes

Arend van Spriel

author

2015-05-04 08:05 UTC #4 lore raw

Quoted 6 lines · Johannes Berg

Quoted 2 lines · Arend van Spriel
I don't either, but I had a bunch of test systems that I wanted to get going.
While on vacation? :)

It happened just before as such things tend to do. There is probably some law for this ;-)

Quoted 8 lines

Quoted 8 lines
Quoted 7 lines
What part of this isn't supported? Is there no pointer whatsoever that we can use? We still implement the ndo_getstats64 [or whatever it's called] ourselves anyway, so we don't really use any API here apart from
* netdev_alloc_pcpu_stats, which is trivially backported
* dev->tstats, which is just a single pointer and we could use anything
else,
even perhaps in sdata

Quoted 4 lines

Quoted 2 lines
I have to learn about the per-cpu operations someday so why not now while spending two weeks vacation ;-)
Heh. Go enjoy your vacation, I can handle this :)

no doubt.

Quoted 7 lines

I think you arrived at kernel 3.14 due to 8f84985fec10d. However, I think we can just do the following
* backport "struct pcpu_sw_netstats" (needed on< 3.14)
* backport "netdev_alloc_pcpu_stats" (needed on< 3.15)
* make an spatch replacing dev->tstats with ((struct pcpu_sw_netstats *)dev->ml_priv) (since ml_priv has existed for a long time, and they're in a union)

That will work. Guess you will drop the mac80211 patch I sent using sdata instead. Fine by me.

Regards,
Arend