This patchset adds support for the collection and propagating of
per-rate, per-station rx statistics when subscribed by userspace applications.
These statistics can be useful in understanding the quality of
communication with our peers and in evaluating how different peers
are communicating in different MCS/BW/NSS during different time periods and environment.
This patchset is based on the below implementation by Johannes, to publish the stats when
user space clients have subscribed and dump and clear when a certain threshold
of stats is reached.
http://thread.gmane.org/gmane.linux.kernel.wireless.general/133172
Few changes are made to the above implementation,
1. Dynamic memory allocation of stats data structures
2. Rate is encoded and used as key of hash table for scalablility.
Johannes Berg(1):
nl80211: support per-rate/per-station statistics
Sriram R (1):
mac80211: Add support for per-rate rx statistics
include/net/cfg80211.h | 51 +++++++++++
include/uapi/linux/nl80211.h | 76 ++++++++++++++++
net/mac80211/cfg.c | 36 ++++++++
net/mac80211/ieee80211_i.h | 2 +
net/mac80211/main.c | 2 +
net/mac80211/rx.c | 10 +-
net/mac80211/sta_info.c | 212 +++++++++++++++++++++++++++++++++++++++++++
net/mac80211/sta_info.h | 20 ++++
net/wireless/core.c | 18 ++++
net/wireless/nl80211.c | 175 ++++++++++++++++++++++++++++++++---
net/wireless/nl80211.h | 14 +++
net/wireless/rdev-ops.h | 10 ++
net/wireless/trace.h | 15 +++
13 files changed, 626 insertions(+), 15 deletions(-)
--
2.7.4
Per-rate/per-station statistics can be desirable to have but they're
quite expensive (keeping the four counters for each rate would take
close to 4k of memory per station only for VHT MCSes for a moderately
capable VHT chip (with 2 spatial streams and 80MHz support) so it's
not a good idea to keep all of this in the kernel.
Instead, this API provides a way for interested clients in userspace
to subscribe to such statistics. When supported by a driver, it can
then start collecting the data only when subscribers exist. To avoid
the kernel's data collection becoming too big, it can send out the
data at any point in time, for example to limit the counters to u16
internally and send it out when they're close to reaching the limit,
or to keep a hash table and sending it out when too many collisions
occur. Userspace can then keep track of the full state.
Based on below implementation by Johannes Berg <[email protected]>
http://thread.gmane.org/gmane.linux.kernel.wireless.general/133172
with following changes,
1. Allow rx bytes stats to be collected
2. Rate info sent to userspace is encoded into 32bit value
Signed-off-by: Sriram R <[email protected]>
---
include/net/cfg80211.h | 51 +++++++++++++
include/uapi/linux/nl80211.h | 76 +++++++++++++++++++
net/wireless/core.c | 18 +++++
net/wireless/nl80211.c | 175 +++++++++++++++++++++++++++++++++++++++----
net/wireless/nl80211.h | 14 ++++
net/wireless/rdev-ops.h | 10 +++
net/wireless/trace.h | 15 ++++
7 files changed, 346 insertions(+), 13 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5fbfe61..6796462 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1236,6 +1236,37 @@ struct station_info {
s8 avg_ack_signal;
};
+/**
+ * struct cfg80211_rate_stats - per rate statistics of station
+ *
+ * @rate: encoded rate value
+ * @bytes: Total number of bytes received at this @rate
+ * @packets: Packet count received at this @rate
+ */
+struct cfg80211_rate_stats {
+ u32 rate;
+ u32 bytes;
+ u16 packets;
+};
+
+/**
+ * cfg80211_report_rate_stats - report rate statistics for a station
+ * @wiphy: the wiphy that's reporting the data
+ * @wdev: the virtual interface the data is reported for
+ * @mac_addr: the station MAC address
+ * @rate_table_len: length of rate table
+ * @rate_stats_buf: per-rate,per-station statistics buffer
+ * @gfp: allocation flags
+ *
+ * Note that it is valid to call this function multiple times even for the
+ * same station, if the data isn't actually stored in an array.
+ */
+void cfg80211_report_rate_stats(struct wiphy *wiphy, struct wireless_dev *wdev,
+ const u8 *mac_addr, unsigned int rate_table_len,
+ struct cfg80211_rate_stats *rate_stats_buf,
+ gfp_t gfp);
+
+
#if IS_ENABLED(CONFIG_CFG80211)
/**
* cfg80211_get_station - retrieve information about a given station
@@ -2693,6 +2724,20 @@ struct cfg80211_external_auth_params {
};
/**
+ * enum cfg80211_rate_stats_ops - rate statistics operations
+ *
+ * @CFG80211_RATE_STATS_START: start data collection
+ * @CFG80211_RATE_STATS_DUMP: dump and clear the data
+ * (using cfg80211_report_rate_stats() to report it)
+ * @CFG80211_RATE_STATS_STOP: stop data collection
+ */
+enum cfg80211_rate_stats_ops {
+ CFG80211_RATE_STATS_START,
+ CFG80211_RATE_STATS_DUMP,
+ CFG80211_RATE_STATS_STOP,
+};
+
+/**
* struct cfg80211_ops - backend description for wireless configuration
*
* This struct is registered by fullmac card drivers and/or wireless stacks
@@ -3024,6 +3069,9 @@ struct cfg80211_external_auth_params {
*
* @tx_control_port: TX a control port frame (EAPoL). The noencrypt parameter
* tells the driver that the frame should not be encrypted.
+ *
+ * @rate_stats: rate statistics operation - if supported all operations must be
+ * supported, see &enum cfg80211_rate_stats_ops.
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3329,6 +3377,9 @@ struct cfg80211_ops {
const u8 *buf, size_t len,
const u8 *dest, const __be16 proto,
const bool noencrypt);
+
+ void (*rate_stats)(struct wiphy *wiphy,
+ enum cfg80211_rate_stats_ops op);
};
/*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 06f9af2..fca25b8 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -51,6 +51,7 @@
#define NL80211_MULTICAST_GROUP_VENDOR "vendor"
#define NL80211_MULTICAST_GROUP_NAN "nan"
#define NL80211_MULTICAST_GROUP_TESTMODE "testmode"
+#define NL80211_MULTICAST_GROUP_RATE_STATS "rate-stats"
/**
* DOC: Station handling
@@ -1033,6 +1034,10 @@
* &NL80211_ATTR_CHANNEL_WIDTH,&NL80211_ATTR_NSS attributes with its
* address(specified in &NL80211_ATTR_MAC).
*
+ * @NL80211_CMD_GET_RATE_STATS: This command can be used to trigger the
+ * rate statistics dump to userspace, which also clears the data in the
+ * kernel (driver). See the "per-rate statistics" documentation section.
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -1245,6 +1250,8 @@ enum nl80211_commands {
NL80211_CMD_CONTROL_PORT_FRAME,
+ NL80211_CMD_GET_RATE_STATS,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -2238,6 +2245,9 @@ enum nl80211_commands {
* @NL80211_ATTR_TXQ_QUANTUM: TXQ scheduler quantum (bytes). Number of bytes
* a flow is assigned on each round of the DRR scheduler.
*
+ * @NL80211_ATTR_RATE_STATS: per-rate statistics container attribute, this is
+ * contains the attributes from &enum nl80211_rate_stats.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2677,6 +2687,8 @@ enum nl80211_attrs {
NL80211_ATTR_TXQ_MEMORY_LIMIT,
NL80211_ATTR_TXQ_QUANTUM,
+ NL80211_ATTR_RATE_STATS,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -3116,6 +3128,67 @@ enum nl80211_txq_stats {
};
/**
+ * DOC: per-rate statistics
+ *
+ * The nl80211 API provides a way to subscribe to per-bitrate statistics. Since
+ * there are many bitrates it isn't always desirable to keep statistics for all
+ * of the rates in the kernel. As a consequence, the API allows the drivers to
+ * dump the information to userspace and reset their internal values. As it can
+ * also be expensive to keep the counters, this is only done when subscribers
+ * exist.
+ *
+ * To use this facility, a userspace client must subscribe to the data using the
+ * rate statistics multicast group (%NL80211_MULTICAST_GROUP_RATE_STATS).
+ * This is used by the kernel to start data collection. If, at this point,
+ * other clients exist, those are also sent the current statistics in order to
+ * allow the new client to collect only the data obtained while subscribed.
+ *
+ * While subscribed, the client must listen for %NL80211_CMD_GET_RATE_STATS
+ * events sent to the subscribed socket, and accumulate data retrieved in them.
+ * Every time such an event is sent by the kernel, the in-kernel data is also
+ * cleared. Therefore, to achieve data collection over longer periods of time,
+ * the subscribers must accumulate data. No guarantees are made about how long
+ * the kernel will collect data, but (as an implementation guideline) the data
+ * shouldn't be sent out frequently, and only while traffic is keeping the CPU
+ * busy anyway (i.e. it is recommended to not use timers in drivers supporting
+ * this facility.)
+ *
+ * In order to obtain a sample or clear the statistics at a given point in time,
+ * the %NL80211_CMD_GET_RATE_STATS command can be used. This command can
+ * be called by any nl80211 client (even non-subscribers) and causes the kernel
+ * to send out and clear (atomically) the currently accumulated data to all of
+ * the subscribers.
+ *
+ * Note that the data sent out in each notification contains only some data for
+ * a single station (identified by the interface index and the station's MAC
+ * address.) It is therefore expected that multiple messages will be received
+ * by an application, possibly even multiple messages for the same station and
+ * the same rate (e.g. for separate RX and TX counters), and subscribers need
+ * to ensure that their socket buffers are big enough to retrieve all the data.
+ */
+
+/**
+ * enum nl80211_rate_stats - per-rate stats attributes
+ * @__NL80211_RATE_STATS_INVALID :attribute number 0 is reserved
+ * @NL80211_ATTR_RATE_STATS_RATE :identifier for the encoded rate field.
+ * @NL80211_ATTR_RATE_STATS_RX_PACKETS: Number of packets which are received
+ * per @rate for this specific station identified by @NL80211_ATTR_MAC.
+ * @NL80211_ATTR_RATE_STATS_RX_BYTES: Number of bytes which are received
+ * per @rate for this specific station identified by @NL80211_ATTR_MAC.
+ * @NUM_NL80211_ATTR_RATE_STATS: Total number of rate-stats attributes
+ * @NL80211_ATTR_RATE_STATS_MAX: MAX rate-stats attribute number.
+ */
+enum nl80211_rate_stats {
+ __NL80211_RATE_STATS_INVALID,
+ NL80211_ATTR_RATE_STATS_RATE,
+ NL80211_ATTR_RATE_STATS_RX_PACKETS,
+ NL80211_ATTR_RATE_STATS_RX_BYTES,
+
+ /* keep last */
+ NUM_NL80211_ATTR_RATE_STATS,
+ NL80211_ATTR_RATE_STATS_MAX = NUM_NL80211_ATTR_RATE_STATS - 1
+};
+/**
* enum nl80211_mpath_flags - nl80211 mesh path flags
*
* @NL80211_MPATH_FLAG_ACTIVE: the mesh path is active
@@ -5133,6 +5206,8 @@ enum nl80211_feature_flags {
* support to nl80211.
* @NL80211_EXT_FEATURE_TXQS: Driver supports FQ-CoDel-enabled intermediate
* TXQs.
+ * @NL80211_EXT_FEATURE_RATE_STATS: This device supports the per-rate
+ * statistics subscription API.
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -5167,6 +5242,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211,
NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT,
NL80211_EXT_FEATURE_TXQS,
+ NL80211_EXT_FEATURE_RATE_STATS,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 5fe35aa..e345166 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -664,6 +664,12 @@ int wiphy_register(struct wiphy *wiphy)
return -EINVAL;
#endif
+ if (WARN_ON(wiphy_ext_feature_isset(wiphy,
+ NL80211_EXT_FEATURE_RATE_STATS) &&
+ !rdev->ops->rate_stats))
+ return -EINVAL;
+
+
/*
* if a wiphy has unsupported modes for regulatory channel enforcement,
* opt-out of enforcement checking
@@ -872,6 +878,12 @@ int wiphy_register(struct wiphy *wiphy)
break;
}
}
+
+ if (wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_RATE_STATS) &&
+ genl_has_listeners(&nl80211_fam, wiphy_net(wiphy),
+ NL80211_MCGRP_RATE_STATS))
+ rdev_rate_stats(rdev, CFG80211_RATE_STATS_START);
rdev->wiphy.registered = true;
rtnl_unlock();
@@ -922,6 +934,12 @@ void wiphy_unregister(struct wiphy *wiphy)
rfkill_unregister(rdev->rfkill);
rtnl_lock();
+ if (wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_RATE_STATS) &&
+ genl_has_listeners(&nl80211_fam, wiphy_net(wiphy),
+ NL80211_MCGRP_RATE_STATS))
+ rdev_rate_stats(rdev, CFG80211_RATE_STATS_STOP);
+
nl80211_notify_wiphy(rdev, NL80211_CMD_DEL_WIPHY);
rdev->wiphy.registered = false;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index bc40a78..27248d8 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -35,18 +35,8 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev,
int cipher_limit);
/* the netlink family */
-static struct genl_family nl80211_fam;
-
-/* multicast groups */
-enum nl80211_multicast_groups {
- NL80211_MCGRP_CONFIG,
- NL80211_MCGRP_SCAN,
- NL80211_MCGRP_REGULATORY,
- NL80211_MCGRP_MLME,
- NL80211_MCGRP_VENDOR,
- NL80211_MCGRP_NAN,
- NL80211_MCGRP_TESTMODE /* keep last - ifdef! */
-};
+struct genl_family nl80211_fam;
+
static const struct genl_multicast_group nl80211_mcgrps[] = {
[NL80211_MCGRP_CONFIG] = { .name = NL80211_MULTICAST_GROUP_CONFIG },
@@ -55,6 +45,9 @@ static const struct genl_multicast_group nl80211_mcgrps[] = {
[NL80211_MCGRP_MLME] = { .name = NL80211_MULTICAST_GROUP_MLME },
[NL80211_MCGRP_VENDOR] = { .name = NL80211_MULTICAST_GROUP_VENDOR },
[NL80211_MCGRP_NAN] = { .name = NL80211_MULTICAST_GROUP_NAN },
+ [NL80211_MCGRP_RATE_STATS] = {
+ .name = NL80211_MULTICAST_GROUP_RATE_STATS
+ },
#ifdef CONFIG_NL80211_TESTMODE
[NL80211_MCGRP_TESTMODE] = { .name = NL80211_MULTICAST_GROUP_TESTMODE }
#endif
@@ -4813,6 +4806,28 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
return genlmsg_reply(msg, info);
}
+static int nl80211_get_rate_stats(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev;
+
+ if (!genl_has_listeners(&nl80211_fam, genl_info_net(info),
+ NL80211_MCGRP_RATE_STATS))
+ return -ENODATA;
+
+ list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
+ if (wiphy_net(&rdev->wiphy) != genl_info_net(info))
+ continue;
+
+ if (!wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_RATE_STATS))
+ continue;
+
+ rdev_rate_stats(rdev, CFG80211_RATE_STATS_DUMP);
+ }
+
+ return 0;
+}
+
int cfg80211_check_station_change(struct wiphy *wiphy,
struct station_parameters *params,
enum cfg80211_station_type statype)
@@ -12991,6 +13006,56 @@ static void nl80211_post_doit(const struct genl_ops *ops, struct sk_buff *skb,
}
}
+static void nl80211_do_rate_stats(struct net *net,
+ enum cfg80211_rate_stats_ops op)
+{
+ struct cfg80211_registered_device *rdev;
+
+ rtnl_lock();
+
+ list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
+ if (wiphy_net(&rdev->wiphy) != net)
+ continue;
+
+ if (!wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_RATE_STATS))
+ continue;
+
+ rdev_rate_stats(rdev, op);
+ }
+
+ rtnl_unlock();
+}
+
+static int nl80211_mcast_bind(struct net *net, int group)
+{
+ enum cfg80211_rate_stats_ops op;
+
+ switch (group) {
+ case NL80211_MCGRP_RATE_STATS:
+ if (!genl_has_listeners(&nl80211_fam, net,
+ NL80211_MCGRP_RATE_STATS))
+ op = CFG80211_RATE_STATS_START;
+ else
+ op = CFG80211_RATE_STATS_DUMP;
+ nl80211_do_rate_stats(net, op);
+ break;
+ }
+
+ return 0;
+}
+
+static void nl80211_mcast_unbind(struct net *net, int group)
+{
+ switch (group) {
+ case NL80211_MCGRP_RATE_STATS:
+ if (!genl_has_listeners(&nl80211_fam, net,
+ NL80211_MCGRP_RATE_STATS))
+ nl80211_do_rate_stats(net, CFG80211_RATE_STATS_STOP);
+ break;
+ }
+}
+
static const struct genl_ops nl80211_ops[] = {
{
.cmd = NL80211_CMD_GET_WIPHY,
@@ -13799,9 +13864,16 @@ static const struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
+ {
+ .cmd = NL80211_CMD_GET_RATE_STATS,
+ .doit = nl80211_get_rate_stats,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_RTNL,
+ },
};
-static struct genl_family nl80211_fam __ro_after_init = {
+struct genl_family nl80211_fam __ro_after_init = {
.name = NL80211_GENL_NAME, /* have users key off the name instead */
.hdrsize = 0, /* no private header */
.version = 1, /* no particular meaning now */
@@ -13814,6 +13886,8 @@ static struct genl_family nl80211_fam __ro_after_init = {
.n_ops = ARRAY_SIZE(nl80211_ops),
.mcgrps = nl80211_mcgrps,
.n_mcgrps = ARRAY_SIZE(nl80211_mcgrps),
+ .mcast_bind = nl80211_mcast_bind,
+ .mcast_unbind = nl80211_mcast_unbind,
};
/* notification functions */
@@ -15885,6 +15959,81 @@ void cfg80211_crit_proto_stopped(struct wireless_dev *wdev, gfp_t gfp)
}
EXPORT_SYMBOL(cfg80211_crit_proto_stopped);
+void cfg80211_report_rate_stats(struct wiphy *wiphy, struct wireless_dev *wdev,
+ const u8 *mac_addr, unsigned int rate_table_len,
+ struct cfg80211_rate_stats *rate_stats_buf,
+ gfp_t gfp)
+{
+ struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
+ struct sk_buff *msg;
+ void *hdr;
+ struct nlattr *rstatsattr, *rentryattr;
+ struct cfg80211_rate_stats *report;
+ int rid;
+
+ if (!genl_has_listeners(&nl80211_fam, wiphy_net(wiphy),
+ NL80211_MCGRP_RATE_STATS))
+ return;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return;
+
+ hdr = nl80211hdr_put(msg, 0, 0, 0,
+ NL80211_CMD_GET_RATE_STATS);
+ if (!hdr)
+ goto out;
+
+ if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
+ nla_put_u32(msg, NL80211_ATTR_IFINDEX, wdev->netdev->ifindex) ||
+ nla_put_u64_64bit(msg, NL80211_ATTR_WDEV, wdev_id(wdev),
+ NL80211_ATTR_PAD))
+ goto out;
+
+ if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, mac_addr))
+ goto out;
+
+ if (1 && rate_table_len && rate_stats_buf) {
+ rstatsattr = nla_nest_start(msg, NL80211_ATTR_RATE_STATS);
+
+ if (!rstatsattr)
+ goto out;
+
+ report = rate_stats_buf;
+
+ for (rid = 0; rid < rate_table_len ; rid++) {
+ rentryattr = nla_nest_start(msg, rid + 1);
+ if (!rentryattr)
+ goto out;
+
+ nla_put_u32(msg, NL80211_ATTR_RATE_STATS_RATE,
+ report->rate);
+ nla_put_u16(msg, NL80211_ATTR_RATE_STATS_RX_PACKETS,
+ report->packets);
+ nla_put_u32(msg, NL80211_ATTR_RATE_STATS_RX_BYTES,
+ report->bytes);
+ nla_nest_end(msg, rentryattr);
+
+ if (rid + 1 < rate_table_len)
+ report++;
+ }
+
+ nla_nest_end(msg, rstatsattr);
+ }
+
+
+ genlmsg_end(msg, hdr);
+
+ genlmsg_multicast_netns(&nl80211_fam, wiphy_net(wiphy), msg, 0,
+ NL80211_MCGRP_RATE_STATS, GFP_KERNEL);
+ return;
+
+ out:
+ nlmsg_free(msg);
+}
+
+EXPORT_SYMBOL_GPL(cfg80211_report_rate_stats);
+
void nl80211_send_ap_stopped(struct wireless_dev *wdev)
{
struct wiphy *wiphy = wdev->wiphy;
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 79e47fe..df9763f 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -4,6 +4,20 @@
#include "core.h"
+/* netlink family */
+extern struct genl_family nl80211_fam;
+
+/* multicast groups */
+enum nl80211_multicast_groups {
+ NL80211_MCGRP_CONFIG,
+ NL80211_MCGRP_SCAN,
+ NL80211_MCGRP_REGULATORY,
+ NL80211_MCGRP_MLME,
+ NL80211_MCGRP_VENDOR,
+ NL80211_MCGRP_NAN,
+ NL80211_MCGRP_RATE_STATS,
+ NL80211_MCGRP_TESTMODE /* keep last - ifdef! */
+};
int nl80211_init(void);
void nl80211_exit(void);
void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 364f5d6..bffefc2 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -222,6 +222,16 @@ static inline int rdev_dump_station(struct cfg80211_registered_device *rdev,
return ret;
}
+static inline void
+rdev_rate_stats(struct cfg80211_registered_device *rdev,
+ enum cfg80211_rate_stats_ops op)
+{
+ trace_rdev_rate_stats(&rdev->wiphy, op);
+ if (rdev->ops->rate_stats)
+ rdev->ops->rate_stats(&rdev->wiphy, op);
+ trace_rdev_return_void(&rdev->wiphy);
+}
+
static inline int rdev_add_mpath(struct cfg80211_registered_device *rdev,
struct net_device *dev, u8 *dst, u8 *next_hop)
{
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 2b417a2..c82b21b 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2368,6 +2368,21 @@ TRACE_EVENT(rdev_external_auth,
__entry->bssid, __entry->ssid, __entry->status)
);
+TRACE_EVENT(rdev_rate_stats,
+ TP_PROTO(struct wiphy *wiphy, enum cfg80211_rate_stats_ops op),
+ TP_ARGS(wiphy, op),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ __field(u32, op)
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ __entry->op = op;
+ ),
+ TP_printk(WIPHY_PR_FMT ", op=%d", WIPHY_PR_ARG, __entry->op)
+);
+
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
--
2.7.4
This patch adds support for the mac80211 to collect rx statistics
(Packet count and total bytes received) per-rate, per-station when
subscribed by user space clients.
Note that the rate field passed on to the userspace from mac80211
is an encoded value where different rate attributes such as
type(VHT/HT/Legacy), MCS, BW, NSS, GI are collectively used to encode to
this rate and the userspace has to take care of decoding it.This is
done so as to allow scalability in future.
Once subscribers to the rate stats exist, each packet received is recorded
per rate and stored as an entry(per rate) in a hashtable with the encoded rate
being the key for the hashtable. As the rate changes, new entries gets added
into this table and this information will be populated and sent back
to userspace whenever a certain limit is hit , Say when Packet count is greater
than 65000 or the number of rate entries exceed say a count of 10,after
which these entries are cleared and new stats gets collected.
Signed-off-by: Sriram R <[email protected]>
---
net/mac80211/cfg.c | 36 ++++++++
net/mac80211/ieee80211_i.h | 2 +
net/mac80211/main.c | 2 +
net/mac80211/rx.c | 10 ++-
net/mac80211/sta_info.c | 212 +++++++++++++++++++++++++++++++++++++++++++++
net/mac80211/sta_info.h | 20 +++++
6 files changed, 280 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index bdf6fa7..03a2dab 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3803,6 +3803,41 @@ static int ieee80211_get_txq_stats(struct wiphy *wiphy,
return ret;
}
+static void
+ieee80211_rate_stats(struct wiphy *wiphy, enum cfg80211_rate_stats_ops ops)
+{
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+ struct sta_info *sta;
+
+ mutex_lock(&local->sta_mtx);
+
+ switch (ops) {
+ case CFG80211_RATE_STATS_START:
+ local->rate_stats_active = true;
+ list_for_each_entry(sta, &local->sta_list, list) {
+ ieee80211_sta_rate_table_init(sta);
+ }
+ break;
+
+ case CFG80211_RATE_STATS_STOP:
+ local->rate_stats_active = false;
+ list_for_each_entry(sta, &local->sta_list, list) {
+ ieee80211_sta_rate_table_free(sta->rate_table);
+ RCU_INIT_POINTER(sta->rate_table, NULL);
+ }
+ break;
+
+ case CFG80211_RATE_STATS_DUMP:
+ list_for_each_entry(sta, &local->sta_list, list) {
+ ieee80211_queue_work(&local->hw,
+ &sta->rate_stats_dump_wk);
+ }
+ break;
+ }
+
+ mutex_unlock(&local->sta_mtx);
+}
+
const struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -3897,4 +3932,5 @@ const struct cfg80211_ops mac80211_config_ops = {
.set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
.tx_control_port = ieee80211_tx_control_port,
.get_txq_stats = ieee80211_get_txq_stats,
+ .rate_stats = ieee80211_rate_stats,
};
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index d1978aa..60810e1 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1384,6 +1384,8 @@ struct ieee80211_local {
/* TDLS channel switch */
struct work_struct tdls_chsw_work;
struct sk_buff_head skb_queue_tdls_chsw;
+
+ bool rate_stats_active;
};
static inline struct ieee80211_sub_if_data *
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 4d2e797..030d19f 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -570,6 +570,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_RRM);
+ wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_RATE_STATS);
+
wiphy->bss_priv_size = sizeof(struct ieee80211_bss);
local = wiphy_priv(wiphy);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 0a38cc1..0f63b18 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1561,9 +1561,11 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
test_sta_flag(sta, WLAN_STA_AUTHORIZED)) {
sta->rx_stats.last_rx = jiffies;
if (ieee80211_is_data(hdr->frame_control) &&
- !is_multicast_ether_addr(hdr->addr1))
+ !is_multicast_ether_addr(hdr->addr1)) {
sta->rx_stats.last_rate =
sta_stats_encode_rate(status);
+ ieee80211_sta_update_rate_stats(&sta);
+ }
}
} else if (rx->sdata->vif.type == NL80211_IFTYPE_OCB) {
sta->rx_stats.last_rx = jiffies;
@@ -1573,8 +1575,10 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
* match the current local configuration when processed.
*/
sta->rx_stats.last_rx = jiffies;
- if (ieee80211_is_data(hdr->frame_control))
+ if (ieee80211_is_data(hdr->frame_control)) {
sta->rx_stats.last_rate = sta_stats_encode_rate(status);
+ ieee80211_sta_update_rate_stats(&sta);
+ }
}
if (rx->sdata->vif.type == NL80211_IFTYPE_STATION)
@@ -4067,6 +4071,8 @@ static bool ieee80211_invoke_fast_rx(struct ieee80211_rx_data *rx,
stats->last_rx = jiffies;
stats->last_rate = sta_stats_encode_rate(status);
+ ieee80211_sta_update_rate_stats(&sta);
+
stats->fragments++;
stats->packets++;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 6428f1a..b460f0d6 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -31,6 +31,8 @@
#include "mesh.h"
#include "wme.h"
+void ieee80211_rate_stats_dump(struct work_struct *wk);
+
/**
* DOC: STA information lifetime rules
*
@@ -324,6 +326,7 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
spin_lock_init(&sta->ps_lock);
INIT_WORK(&sta->drv_deliver_wk, sta_deliver_ps_frames);
INIT_WORK(&sta->ampdu_mlme.work, ieee80211_ba_session_work);
+ INIT_WORK(&sta->rate_stats_dump_wk, ieee80211_rate_stats_dump);
mutex_init(&sta->ampdu_mlme.mtx);
#ifdef CONFIG_MAC80211_MESH
if (ieee80211_vif_is_mesh(&sdata->vif)) {
@@ -609,6 +612,11 @@ static int sta_info_insert_finish(struct sta_info *sta) __acquires(RCU)
if (ieee80211_vif_is_mesh(&sdata->vif))
mesh_accept_plinks_update(sdata);
+ RCU_INIT_POINTER(sta->rate_table, NULL);
+
+ if (local->rate_stats_active)
+ ieee80211_sta_rate_table_init(sta);
+
return 0;
out_remove:
sta_info_hash_del(local, sta);
@@ -1006,6 +1014,10 @@ static void __sta_info_destroy_part2(struct sta_info *sta)
sta_dbg(sdata, "Removed STA %pM\n", sta->sta.addr);
+ ieee80211_sta_rate_table_free(sta->rate_table);
+
+ RCU_INIT_POINTER(sta->rate_table, NULL);
+
sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
if (sinfo)
sta_set_sinfo(sta, sinfo, true);
@@ -2371,3 +2383,203 @@ void ieee80211_sta_set_expected_throughput(struct ieee80211_sta *pubsta,
sta_update_codel_params(sta, thr);
}
+
+static u32 rate_table_hash(const void *rate, u32 len, u32 seed)
+{
+ return jhash_1word(*(u32 *)rate, seed);
+}
+
+static const struct rhashtable_params rate_rht_params = {
+ .nelem_hint = 5,
+ .automatic_shrinking = true,
+ .key_len = IEEE80211_ENCODED_RATE_LEN,
+ .key_offset = offsetof(struct ieee80211_sta_rate_entry, rate),
+ .head_offset = offsetof(struct ieee80211_sta_rate_entry, rhash),
+ .hashfn = rate_table_hash,
+};
+
+int ieee80211_sta_rate_table_init(struct sta_info *sta)
+{
+ struct rhashtable *new_rt;
+
+ new_rt = kmalloc(sizeof(*sta->rate_table), GFP_KERNEL);
+ rcu_assign_pointer(sta->rate_table, new_rt);
+
+ if (!sta->rate_table)
+ return -ENOMEM;
+
+ return rhashtable_init(sta->rate_table, &rate_rht_params);
+}
+
+static struct ieee80211_sta_rate_entry*
+ieee80211_sta_rate_table_lookup(struct rhashtable *rate_table, u32 rate)
+{
+ if (!rate_table)
+ return NULL;
+
+ return rhashtable_lookup_fast(rate_table, &rate, rate_rht_params);
+}
+
+static int
+ieee80211_sta_rate_entry_insert(struct rhashtable *rate_table,
+ struct ieee80211_sta_rate_entry *entry)
+{
+ if (!rate_table)
+ return -EINVAL;
+
+ return rhashtable_lookup_insert_fast(rate_table, &entry->rhash,
+ rate_rht_params);
+}
+
+static void ieee80211_sta_rate_entry_free(void *ptr, void *arg)
+{
+ struct ieee80211_sta_rate_entry *entry = ptr;
+
+ kfree_rcu(entry, rcu);
+}
+
+void ieee80211_sta_rate_table_free(struct rhashtable *rate_table)
+{
+ if (!rate_table)
+ return;
+ rhashtable_free_and_destroy(rate_table,
+ ieee80211_sta_rate_entry_free, NULL);
+ kfree(rate_table);
+}
+
+static int
+ieee80211_sta_get_rate_stats_report(struct rhashtable *rate_table,
+ struct cfg80211_rate_stats **report_buf,
+ int *len)
+{
+ int i = 0, ret, rt_len;
+ struct ieee80211_sta_rate_entry *entry = NULL;
+ struct rhashtable_iter iter;
+ struct cfg80211_rate_stats *report;
+
+ if (!rate_table)
+ return -EINVAL;
+
+ ret = rhashtable_walk_init(rate_table, &iter, GFP_KERNEL);
+
+ if (ret)
+ return -EINVAL;
+
+ rhashtable_walk_start(&iter);
+
+ rt_len = atomic_read(&(iter.ht->nelems));
+
+ /* Caller should take care of freeing this memory */
+ *report_buf = kzalloc(sizeof(struct cfg80211_rate_stats) * rt_len,
+ GFP_KERNEL);
+
+ if (*report_buf == NULL) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ while ((entry = rhashtable_walk_next(&iter))) {
+ if (IS_ERR(entry) && PTR_ERR(entry) == -EAGAIN)
+ continue;
+ if (IS_ERR(entry))
+ break;
+ if (i >= rt_len)
+ break;
+
+ report = *report_buf + i++;
+ report->rate = entry->rate;
+ report->packets = entry->packets;
+ report->bytes = entry->bytes;
+ }
+
+ *len = i;
+err:
+ rhashtable_walk_stop(&iter);
+ rhashtable_walk_exit(&iter);
+ return ret;
+}
+
+void ieee80211_rate_stats_dump(struct work_struct *wk)
+{
+ struct sta_info *sta;
+ struct cfg80211_rate_stats *report_buf = NULL;
+ struct rhashtable *old_rate_table;
+ unsigned int len = 0;
+
+ sta = container_of(wk, struct sta_info, rate_stats_dump_wk);
+
+ if (sta->dead)
+ return;
+
+ synchronize_net();
+ mutex_lock(&sta->local->sta_mtx);
+ old_rate_table = rcu_dereference(sta->rate_table);
+ if (!old_rate_table) {
+ mutex_unlock(&sta->local->sta_mtx);
+ return;
+ }
+
+ ieee80211_sta_rate_table_init(sta);
+ mutex_unlock(&sta->local->sta_mtx);
+
+ ieee80211_sta_get_rate_stats_report(old_rate_table, &report_buf, &len);
+
+ cfg80211_report_rate_stats(sta->local->hw.wiphy, &sta->sdata->wdev,
+ sta->sta.addr, len, report_buf,
+ GFP_KERNEL);
+
+ ieee80211_sta_rate_table_free(old_rate_table);
+ kfree(report_buf);
+}
+
+void ieee80211_sta_update_rate_stats(struct sta_info **sta_ptr)
+{
+ struct ieee80211_sta_rate_entry *entry;
+ struct sta_info *sta = *sta_ptr;
+ struct rhashtable *current_rate_table;
+ struct ieee80211_rx_data *rx;
+ u32 rx_pkt_len;
+ u32 rate;
+ int ret;
+
+ if (!sta->local->rate_stats_active)
+ return;
+
+ rx = container_of(sta_ptr, struct ieee80211_rx_data, sta);
+
+ rx_pkt_len = rx->skb->len;
+ rate = sta->rx_stats.last_rate;
+
+ if (!sta->rate_table)
+ return;
+
+ rcu_read_lock();
+
+ current_rate_table = rcu_dereference(sta->rate_table);
+
+ entry = ieee80211_sta_rate_table_lookup(current_rate_table, rate);
+ if (!entry) {
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry) {
+ rcu_read_unlock();
+ return;
+ }
+ entry->rate = rate;
+ ret = ieee80211_sta_rate_entry_insert(current_rate_table, entry);
+ if (ret) {
+ kfree(entry);
+ rcu_read_unlock();
+ return;
+ }
+ }
+
+ entry->packets++;
+ entry->bytes += rx_pkt_len;
+
+ if ((atomic_read(&(current_rate_table->nelems)) >= MAX_RATE_TABLE_ELEMS)
+ || (entry->packets >= MAX_RATE_TABLE_PACKETS)) {
+ ieee80211_queue_work(&sta->sdata->local->hw,
+ &sta->rate_stats_dump_wk);
+ }
+ rcu_read_unlock();
+}
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 81b35f6..8754e39 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -583,10 +583,25 @@ struct sta_info {
struct cfg80211_chan_def tdls_chandef;
+ struct rhashtable *rate_table;
+ struct work_struct rate_stats_dump_wk;
+
/* keep last! */
struct ieee80211_sta sta;
};
+#define IEEE80211_ENCODED_RATE_LEN 4
+#define MAX_RATE_TABLE_ELEMS 10
+#define MAX_RATE_TABLE_PACKETS 65000
+
+struct ieee80211_sta_rate_entry {
+ u32 rate;
+ u32 bytes;
+ struct rcu_head rcu;
+ struct rhash_head rhash;
+ u16 packets;
+};
+
static inline enum nl80211_plink_state sta_plink_state(struct sta_info *sta)
{
#ifdef CONFIG_MAC80211_MESH
@@ -759,6 +774,11 @@ void ieee80211_sta_ps_deliver_uapsd(struct sta_info *sta);
unsigned long ieee80211_sta_last_active(struct sta_info *sta);
+void ieee80211_sta_rate_table_free(struct rhashtable *rate_table);
+int ieee80211_sta_rate_table_init(struct sta_info *sta);
+void ieee80211_rate_stats_dump(struct work_struct *wk);
+void ieee80211_sta_update_rate_stats(struct sta_info **sta_ptr);
+
enum sta_stats_type {
STA_STATS_RATE_TYPE_INVALID = 0,
STA_STATS_RATE_TYPE_LEGACY,
--
2.7.4
Why did you change to rhashtable?
That seems very strange, since we explicitly want to limit the number of
entries, but rhashtable will grow/shrink as required.
I think I liked my approach better since it allowed us to clearly limit
the memory consumption for this.
johannes
On Mon, 2018-05-28 at 19:04 +0530, Sriram R wrote:
> Per-rate/per-station statistics can be desirable to have but they're
> quite expensive (keeping the four counters for each rate would take
> close to 4k of memory per station only for VHT MCSes for a moderately
> capable VHT chip (with 2 spatial streams and 80MHz support) so it's
> not a good idea to keep all of this in the kernel.
>
> Instead, this API provides a way for interested clients in userspace
> to subscribe to such statistics. When supported by a driver, it can
> then start collecting the data only when subscribers exist. To avoid
> the kernel's data collection becoming too big, it can send out the
> data at any point in time, for example to limit the counters to u16
> internally and send it out when they're close to reaching the limit,
> or to keep a hash table and sending it out when too many collisions
> occur. Userspace can then keep track of the full state.
>
> Based on below implementation by Johannes Berg <[email protected]>
> http://thread.gmane.org/gmane.linux.kernel.wireless.general/133172
> with following changes,
Err, that's pretty much an exact copy of my patch ...
You should keep the author attribution and signed-off-by, and if you
want your work to be separately attributed then just put the changes you
made into (a) separate patch(es).
> 1. Allow rx bytes stats to be collected
This seems OK.
> 2. Rate info sent to userspace is encoded into 32bit value
This I don't like at all. It's fine for the internal representation, and
I know I did something like that in mac80211 (though I guess at the time
it was only 16 bit), but it's not OK for the userspace representation.
I just merged patches to support HE in the userspace representation, and
that's properly taken into account in struct rate_info which I had here
in my version of the patch.
johannes
On 2018-08-28 14:30, Johannes Berg wrote:
> Hi,
>
> Sorry for the late reply, my work hours are limited right now.
>
>> I wanted to give a try with rhashtable and get your thoughts, since it
>> gave below flexibility to original patch,
>> 1. avoids creating static memory when userspace starts accumulating
>> stats. 36 rate entries were used in original patch based on 10 (MCS
>> count) * 3 (entries per mcs)+ 6 escape entries, which would also
>> increase with HE supported now.
>
> True, but rhashtable also has very significant overhead. I suspect it's
> around the same order of magnitude as the allocation you need to keep
> all the 36 entries?
>
> struct rhashtable is probably already ~120 bytes on 64-bit systems
> (very
> rough counting), and a single bucket table is >64, so you already have
> close to 200 bytes for just the basic structures, without *any*
> entries.
> And a significant amount of code too.
>
> 36 rate entries could probably be kept - I don't think you really need
> to go more than that since you're not going to switch HT/VHT/HE all the
> time, so that's only about 360 bytes total. You haven't gained much,
> but
> made it much more complex, doing much more allocations (create
> new/destroy old entries) etc.
>
> I don't really see much value in that.
>
Okay Johannes. I'll revive this patch based on your approach
and submit for review.
Thanks,
Sriram.R
>> 2. avoids restricting to only 3 entries per MCS in the table. Using
>> hashtable gave flexibility to add/read the table dynamically based on
>> encoded rate key.
>
> Yes but if you don't limit, you end up with run-away memory consumption
> again.
>
>> Yes you're right ,it might grow but, as per this patch when Packet
>> count
>> is greater
>> than 65000 in an exntry or when the number of rate table/hashtable
>> entries exceed a count of MAX_RATE_TABLE_ELEMS (10 was used in this
>> patch), the complete table is dumped to userspace and new stats starts
>> getting collected in a new table after we flush the old one.
>> Hence with this approach also the memory consumption is limited
>> similar
>> to the original patch.
>
> Right, so you limit to 10 entries, which is a total of
>
> ~120 + ~72 + 10 x (10 /* data */ + 3*8) = 524
>
> in 12 different allocations. I don't think that's going to be much
> better, and you seemed to think that the 10 would be commonly hit
> (otherwise having a relatively small limit of 36 shouldn't be an issue)
>
> So - I don't really see any advantage here, do you?
>
> johannes
Hi,
Sorry for the late reply, my work hours are limited right now.
> I wanted to give a try with rhashtable and get your thoughts, since it
> gave below flexibility to original patch,
> 1. avoids creating static memory when userspace starts accumulating
> stats. 36 rate entries were used in original patch based on 10 (MCS
> count) * 3 (entries per mcs)+ 6 escape entries, which would also
> increase with HE supported now.
True, but rhashtable also has very significant overhead. I suspect it's
around the same order of magnitude as the allocation you need to keep
all the 36 entries?
struct rhashtable is probably already ~120 bytes on 64-bit systems (very
rough counting), and a single bucket table is >64, so you already have
close to 200 bytes for just the basic structures, without *any* entries.
And a significant amount of code too.
36 rate entries could probably be kept - I don't think you really need
to go more than that since you're not going to switch HT/VHT/HE all the
time, so that's only about 360 bytes total. You haven't gained much, but
made it much more complex, doing much more allocations (create
new/destroy old entries) etc.
I don't really see much value in that.
> 2. avoids restricting to only 3 entries per MCS in the table. Using
> hashtable gave flexibility to add/read the table dynamically based on
> encoded rate key.
Yes but if you don't limit, you end up with run-away memory consumption
again.
> Yes you're right ,it might grow but, as per this patch when Packet count
> is greater
> than 65000 in an exntry or when the number of rate table/hashtable
> entries exceed a count of MAX_RATE_TABLE_ELEMS (10 was used in this
> patch), the complete table is dumped to userspace and new stats starts
> getting collected in a new table after we flush the old one.
> Hence with this approach also the memory consumption is limited similar
> to the original patch.
Right, so you limit to 10 entries, which is a total of
~120 + ~72 + 10 x (10 /* data */ + 3*8) = 524
in 12 different allocations. I don't think that's going to be much
better, and you seemed to think that the 10 would be commonly hit
(otherwise having a relatively small limit of 36 shouldn't be an issue)
So - I don't really see any advantage here, do you?
johannes
On 2018-06-15 17:49, Johannes Berg wrote:
> On Mon, 2018-05-28 at 19:04 +0530, Sriram R wrote:
>> Per-rate/per-station statistics can be desirable to have but they're
>> quite expensive (keeping the four counters for each rate would take
>> close to 4k of memory per station only for VHT MCSes for a moderately
>> capable VHT chip (with 2 spatial streams and 80MHz support) so it's
>> not a good idea to keep all of this in the kernel.
>>
>> Instead, this API provides a way for interested clients in userspace
>> to subscribe to such statistics. When supported by a driver, it can
>> then start collecting the data only when subscribers exist. To avoid
>> the kernel's data collection becoming too big, it can send out the
>> data at any point in time, for example to limit the counters to u16
>> internally and send it out when they're close to reaching the limit,
>> or to keep a hash table and sending it out when too many collisions
>> occur. Userspace can then keep track of the full state.
>>
>> Based on below implementation by Johannes Berg
>> <[email protected]>
>> http://thread.gmane.org/gmane.linux.kernel.wireless.general/133172
>> with following changes,
>
> Err, that's pretty much an exact copy of my patch ...
>
> You should keep the author attribution and signed-off-by, and if you
> want your work to be separately attributed then just put the changes
> you
> made into (a) separate patch(es).
Sure! I'll correct this in my next revision.
>
>> 1. Allow rx bytes stats to be collected
>
> This seems OK.
>
>> 2. Rate info sent to userspace is encoded into 32bit value
>
> This I don't like at all. It's fine for the internal representation,
> and
> I know I did something like that in mac80211 (though I guess at the
> time
> it was only 16 bit), but it's not OK for the userspace representation.
>
> I just merged patches to support HE in the userspace representation,
> and
> that's properly taken into account in struct rate_info which I had here
> in my version of the patch.
Okay Johannes. I'll revert this in my next revision.
>
> johannes
On 2018-06-15 17:55, Johannes Berg wrote:
> Why did you change to rhashtable?
Hi Johannes,
I wanted to give a try with rhashtable and get your thoughts, since it
gave below flexibility to original patch,
1. avoids creating static memory when userspace starts accumulating
stats. 36 rate entries were used in original patch based on 10 (MCS
count) * 3 (entries per mcs)+ 6 escape entries, which would also
increase with HE supported now.
2. avoids restricting to only 3 entries per MCS in the table. Using
hashtable gave flexibility to add/read the table dynamically based on
encoded rate key.
>
> That seems very strange, since we explicitly want to limit the number
> of
> entries, but rhashtable will grow/shrink as required.
Yes you're right ,it might grow but, as per this patch when Packet count
is greater
than 65000 in an exntry or when the number of rate table/hashtable
entries exceed a count of MAX_RATE_TABLE_ELEMS (10 was used in this
patch), the complete table is dumped to userspace and new stats starts
getting collected in a new table after we flush the old one.
Hence with this approach also the memory consumption is limited similar
to the original patch.
>
> I think I liked my approach better since it allowed us to clearly limit
> the memory consumption for this.
Sure Johannes, Could you please let me know if i can rebase your patch
and send it(with corresponding additional entries for HE MCS). Also as
mentioned above this patch also limits the memory consumption when the
rate table size exceeds MAX_RATE_TABLE_ELEMS.
>
> johannes