Export Minstrel best API information via get_station()

RFC In review Antonio Quartulli 4 participants 19 messages Jan 21, 2014 – Jan 22, 2014

Subsystems (2)

mac80211 802.11 (including cfg80211/NL80211)

lore

Share & tools

lore

Antonio Quartulli

author

2014-01-21 11:11 UTC #0 lore raw

cover Export Minstrel best API information via get_station()

Hello list,

we (as batman-adv developers) are currently working on a new version of the our routing protocol which is going to use some Minstrel internal information to compute the metric.
In particular I am interested in the currently selected bitrate (which Minstrel selected because it "maximises the throughput") and it's probability of success.

To achieve so I am proposing here a change to the get_station API. This change is supposed to export the needed information only if the current driver is using the Minstrel (or Minstrel HT) RC algorithm.

Patch 1 introduced the change in get_station()
Patch 2 add a new rate_control API used to query the RC algorithm and retrieve
the information. Then it fills the sinfo object.
Patch 3 and 4 are implementing this rate_control API in minstrel and minstrel_ht Patch 5 exports the get_station API in order to allow other modules to use it.

I already had a discussion with Johannes about this patch being not generic enough and really focussed on Minstrel only.

However this change will just
introduce a new exported capability in the station_info object: if the driver does not support it (e.g. it does not use Minstrel) then we simply won't have this information (like we already do with other capabilities).

Cheers,

p.s. I may need to add some more kerneldoc

Antonio Quartulli (5):
cfg80211: export minstrel best rate information through get_station()
mac80211: export minstrel best rate information in set_sta_info()
mac80211: minstrel - implement get_minstrel_best_rate() API
mac80211: minstrel_ht - implement get_minstrel_best_rate() API
cfg80211: implement cfg80211_get_station

 include/net/cfg80211.h             | 28 ++++++++++++++++++++++++++++
 include/net/mac80211.h             | 15 +++++++++++++++
 net/mac80211/cfg.c                 | 15 +++++++++++++++
 net/mac80211/rc80211_minstrel.c    | 12 ++++++++++++
 net/mac80211/rc80211_minstrel_ht.c | 30 ++++++++++++++++++++++++++++++
 net/wireless/nl80211.c             | 22 ++++++++++++++++++++++
 6 files changed, 122 insertions(+)

Signature · 1 line

1.8.5.3

Antonio Quartulli

author

2014-01-21 11:11 UTC #1 lore raw

RFC 1/5 cfg80211: export minstrel best rate information through get_station()

From: Antonio Quartulli <[email protected]>

Users may need information about the best rate used by minstrel for a particular wireless device. Export such rate and its success probability through the get_station() API.

The exported information is Minstrel specific only, therefore drivers not using such RC algorithm should not report this data.

This information is useful to the batman-adv module which will use it for its new metric computation.

Signed-off-by Antonio Quartulli <[email protected]>

---
 include/net/cfg80211.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

include/net/cfg80211.h +17 -0

index d5e57bf..51d5d94 100644
@@ -854,6 +854,7 @@ int cfg80211_check_station_change(struct wiphy *wiphy,
  * @STATION_INFO_NONPEER_PM: @nonpeer_pm filled
  * @STATION_INFO_CHAIN_SIGNAL: @chain_signal filled
  * @STATION_INFO_CHAIN_SIGNAL_AVG: @chain_signal_avg filled
+ * @STATION_INFO_MINSTREL_BEST_RATE: @minstrel_best_rate filled
  */
 enum station_info_flags {
 	STATION_INFO_INACTIVE_TIME	= 1<<0,

@@ -884,6 +885,7 @@ enum station_info_flags {
 	STATION_INFO_TX_BYTES64		= 1<<25,
 	STATION_INFO_CHAIN_SIGNAL	= 1<<26,
 	STATION_INFO_CHAIN_SIGNAL_AVG	= 1<<27,
+	STATION_INFO_MINSTREL_BEST_RATE	= 1<<28,
 };
 
 /**

@@ -963,6 +965,16 @@ struct sta_bss_parameters {
 #define IEEE80211_MAX_CHAINS	4
 
 /**
+ * struct cfg80211_minstral_rate_info - exported minstrel rate information
+ * @bitrate: current chosen TX bitrate
+ * @prob: probability of success of this birate
+ */
+struct cfg80211_minstrel_rate_info {
+	u32 bitrate;
+	u32 prob;
+};
+
+/**
  * struct station_info - station information
  *
  * Station information filled by driver for get_station() and dump_station.

@@ -1005,6 +1017,9 @@ struct sta_bss_parameters {
  * @local_pm: local mesh STA power save mode
  * @peer_pm: peer mesh STA power save mode
  * @nonpeer_pm: non-peer mesh STA power save mode
+ * @minstrel_best_rate: information extracted from the Minstrel RC algorithm
+ *  about the bitrate having the maximum throughput. This field can be filled
+ *  only by drivers using Minstrel
  */
 struct station_info {
 	u32 filled;

@@ -1043,6 +1058,8 @@ struct station_info {
 	enum nl80211_mesh_power_mode peer_pm;
 	enum nl80211_mesh_power_mode nonpeer_pm;
 
+	struct cfg80211_minstrel_rate_info minstrel_best_rate;
+
 	/*
 	 * Note: Add a new enum station_info_flags value for each new field and
 	 * use it to check which fields are initialized.

Signature · 1 line

1.8.5.3

Johannes Berg

maintainer

2014-01-21 16:00 UTC #2 lore raw

Re: [RFC 1/5] cfg80211: export minstrel best rate information through get_station()

Quoted 1 line · Antonio Quartulli

+ * @prob: probability of success of this birate

typo: bitrate

include/net/cfg80211.h struct sta_bss_parameters { · line 975

Quoted 4 lines

include/net/cfg80211.h +4 -0

+struct cfg80211_minstrel_rate_info {
+	u32 bitrate;
+	u32 prob;
+};

Do you actually care about this, rather than bitrate*prob, ie. something like "expected throughput"? Actually this isn't expected throughput, but wouldn't something like expected throughput make even more sense?

johannes

Antonio Quartulli

2014-01-21 16:10 UTC #3 lore raw

Re: [RFC 1/5] cfg80211: export minstrel best rate information through get_station()

Quoted 6 lines · Johannes Berg

Quoted 1 line · Antonio Quartulli
+ * @prob: probability of success of this birate
typo: bitrate

Quoted 7 lines

include/net/cfg80211.h struct sta_bss_parameters { · line 975
Quoted 4 lines
include/net/cfg80211.h +4 -0
+struct cfg80211_minstrel_rate_info {
+	u32 bitrate;
+	u32 prob;
+};
Do you actually care about this, rather than bitrate*prob, ie. something like "expected throughput"?

Right now this is the way how we use it in the batman-adv code..but we may decide to change it for some reason. This is why I preferred to keep the two values separated.

Quoted 2 lines

Actually this isn't expected throughput, but
wouldn't something like expected throughput make even more sense?

How should I obtain that if not with the previous operation (bitrate*prob)?

Signature · 1 line

Antonio Quartulli

Johannes Berg

maintainer

2014-01-21 16:18 UTC #4 lore raw

Re: [RFC 1/5] cfg80211: export minstrel best rate information through get_station()

Quoted 4 lines · Antonio Quartulli

Quoted 2 lines
Actually this isn't expected throughput, but
wouldn't something like expected throughput make even more sense?
How should I obtain that if not with the previous operation (bitrate*prob)?

Minstrel actually has some tables for that, no? This would still just only be the raw bitrate, not the actual throughput.

johannes

Antonio Quartulli

author

2014-01-21 16:33 UTC #5 lore raw

Re: [RFC 1/5] cfg80211: export minstrel best rate information through get_station()

Quoted 9 lines · Johannes Berg

Quoted 4 lines · Antonio Quartulli
Quoted 2 lines
Actually this isn't expected throughput, but
wouldn't something like expected throughput make even more sense?
How should I obtain that if not with the previous operation (bitrate*prob)?
Minstrel actually has some tables for that, no? This would still just only be the raw bitrate, not the actual throughput.

I will check within Minstrel, but in this way we are limiting batman-adv to use only what Minstrel thinks to be the "expected throughput"....And if this turns to be something not adequate we will have to change API.

Signature · 1 line

Antonio Quartulli

Antonio Quartulli

author

2014-01-22 14:42 UTC #6 lore raw

Re: [RFC 1/5] cfg80211: export minstrel best rate information through get_station()

Quoted 8 lines · Antonio Quartulli

Quoted 7 lines · Johannes Berg
Quoted 4 lines · Antonio Quartulli
Quoted 2 lines
Actually this isn't expected throughput, but
wouldn't something like expected throughput make even more sense?
How should I obtain that if not with the previous operation (bitrate*prob)?

Quoted 9 lines

Quoted 2 lines
Minstrel actually has some tables for that, no? This would still just only be the raw bitrate, not the actual throughput.
I will check within Minstrel, but in this way we are limiting batman-adv to use only what Minstrel thinks to be the "expected throughput"....And if this turns to be something not adequate we will have to change API.

Johannes,

do you think that an API exporting the "expected throughput" would be a acceptable? At that point any RC algorithm can implement it the way it prefers.

Moreover, if users realise that the API is not returning a proper value we can still fix the implementation in the future (as soon as it still returns something called "expected throughput").

Can this be the way to go? At this point we forget about the concept of rate and we move to throughput, which is what we are really interested in.

Cheers,

Signature · 1 line

Antonio Quartulli

Johannes Berg

maintainer

2014-01-22 14:43 UTC #7 lore raw

Re: [RFC 1/5] cfg80211: export minstrel best rate information through get_station()

Quoted 3 lines · Antonio Quartulli

do you think that an API exporting the "expected throughput" would be a acceptable? At that point any RC algorithm can implement it the way it prefers.

I think that's a better choice, yes.

johannes

Thomas Hühn

2014-01-22 18:32 UTC #8 lore raw

Re: [RFC 1/5] cfg80211: export minstrel best rate information through get_station()

Hi all,

I do also agree that the expected throughput [=max_throughput(mac_throughput_rate) * success_probability(max_throughput_rate)] is the value of interest.
For my current power control development I just use those minstrel statistics per client, that are provided via debugfs and parse those values that I am interested.
Maybe it is an alternative option for your development of batman is such a way to use those debugfs statistics, where you have all information, expected throughput included. And once your experimentation shows which subset of those stats is sufficient for a better routing performance, you go for a proper api. Or are you already confident about the expected throughput value is the one and only ?

Greetings Thomas

Quoted 14 lines · Johannes Berg

Quoted 3 lines · Antonio Quartulli
do you think that an API exporting the "expected throughput" would be a acceptable? At that point any RC algorithm can implement it the way it prefers.
I think that's a better choice, yes.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Antonio Quartulli

2014-01-22 18:45 UTC #9 lore raw

Re: [RFC 1/5] cfg80211: export minstrel best rate information through get_station()

Quoted 6 lines · Thomas Hühn

Hi all,
I do also agree that the expected throughput [=max_throughput(mac_throughput_rate) * success_probability(max_throughput_rate)] is the value of interest.
For my current power control development I just use those minstrel statistics per client, that are provided via debugfs and parse those values that I am interested.
Maybe it is an alternative option for your development of batman is such a way to use those debugfs statistics, where you have all information, expected throughput included. And once your experimentation shows which subset of those stats is sufficient for a better routing performance, you go for a proper api. Or are you already confident about the expected throughput value is the one and only ?

The only stable point is that the our metric will be "throughput based"...the way how this "throughput" is computed could also change in the future..

Actually for experiment purposes I have already implemented my hacky cfg80211 API and I am using it (therefore I have no real need to use these debugfs stats).

Now I would like to bring this API to the next level and merge it.

The experiments performed shown that bitrate*probability is a reasonable value to use. But even if in the future we may decide to change how this "expected throughput" is computed I think there is no problem as soon as the semantic of the API is remains consistent (=return expected throughput).

Cheers,

Quoted 19 lines

Greetings Thomas
Quoted 14 lines · Johannes Berg
Quoted 3 lines · Antonio Quartulli
do you think that an API exporting the "expected throughput" would be a acceptable? At that point any RC algorithm can implement it the way it prefers.
I think that's a better choice, yes.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line

Antonio Quartulli

Antonio Quartulli

author

2014-01-21 11:11 UTC #10 lore raw

RFC 2/5 mac80211: export minstrel best rate information in set_sta_info()

In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>

Add the get_minstrel_best_rate() RC API in order to retrieve the rate having the highest throughput. If the RC algorithm implements such API, then fill the related field in station_info when dumping a station.

This API is supposed to be implemented by Minstrel only.

Signed-off-by Antonio Quartulli <[email protected]>

---
 include/net/mac80211.h | 15 +++++++++++++++
 net/mac80211/cfg.c     | 15 +++++++++++++++
 2 files changed, 30 insertions(+)

include/net/mac80211.h +15 -0

index df1004b..9ed56f8 100644
@@ -4452,6 +4452,17 @@ struct ieee80211_tx_rate_control {
 	bool bss;
 };
 
+/**
+ * struct ieee80211_minstrel_rate_info - rate information extracted from
+ *  the Minstrel RC algorithm
+ * @rate: the reported bitrate
+ * @prob: probability of success of rate computed by Minstrel
+ */
+struct ieee80211_minstrel_rate_info {
+	struct ieee80211_tx_rate rate;
+	u32 prob;
+};
+
 struct rate_control_ops {
 	struct module *module;
 	const char *name;

@@ -4478,6 +4489,10 @@ struct rate_control_ops {
 	void (*add_sta_debugfs)(void *priv, void *priv_sta,
 				struct dentry *dir);
 	void (*remove_sta_debugfs)(void *priv, void *priv_sta);
+
+	void (*get_minstrel_best_rate)
+		(void *priv, void *priv_sta,
+		 struct ieee80211_minstrel_rate_info *info);
 };
 
 static inline int rate_supported(struct ieee80211_sta *sta,

net/mac80211/cfg.c +15 -0

index 032081c..c0d394c 100644
@@ -463,6 +463,9 @@ static void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
 {
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
 	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_minstrel_rate_info info;
+	struct rate_control_ref *ref = local->rate_ctrl;
+	struct rate_info rinfo;
 	struct timespec uptime;
 	u64 packets = 0;
 	int i, ac;

@@ -578,6 +581,18 @@ static void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
 		sinfo->sta_flags.set |= BIT(NL80211_STA_FLAG_ASSOCIATED);
 	if (test_sta_flag(sta, WLAN_STA_TDLS_PEER))
 		sinfo->sta_flags.set |= BIT(NL80211_STA_FLAG_TDLS_PEER);
+
+	if (!ref->ops->get_minstrel_best_rate)
+		return;
+
+	sinfo->filled |= STATION_INFO_MINSTREL_BEST_RATE;
+
+	ref->ops->get_minstrel_best_rate(ref->priv, sta->rate_ctrl_priv, &info);
+
+	sta_set_rate_info_tx(sta, &info.rate, &rinfo);
+
+	sinfo->minstrel_best_rate.bitrate = cfg80211_calculate_bitrate(&rinfo);
+	sinfo->minstrel_best_rate.prob = info.prob;
 }
 
 static const char ieee80211_gstrings_sta_stats[][ETH_GSTRING_LEN] = {

Signature · 1 line

1.8.5.3

Johannes Berg

maintainer

2014-01-21 16:01 UTC #11 lore raw

Re: [RFC 2/5] mac80211: export minstrel best rate information in set_sta_info()

include/net/mac80211.h struct rate_control_ops { · line 4496

Quoted 12 lines · Antonio Quartulli

struct rate_control_ops {

include/net/mac80211.h +4 -0

 	struct module *module;
 	const char *name;

@@ -4478,6 +4489,10 @@ struct rate_control_ops {
 	void (*add_sta_debugfs)(void *priv, void *priv_sta,
 				struct dentry *dir);
 	void (*remove_sta_debugfs)(void *priv, void *priv_sta);
+
+	void (*get_minstrel_best_rate)
+		(void *priv, void *priv_sta,
+		 struct ieee80211_minstrel_rate_info *info);
 };

This, I think, is a terrible idea. Never mind the coding style (which too looks terrible imho), but this also bloats the struct for a single rate control algorithm's use only.

I'd be much happier if you actually declared this to be something like "expected throughput" and did some calculations and made it generic.

johannes

Antonio Quartulli

author

2014-01-21 11:11 UTC #12 lore raw

RFC 3/5 mac80211: minstrel - implement get_minstrel_best_rate() API

In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>

Signed-off-by Antonio Quartulli <[email protected]>

---
 net/mac80211/rc80211_minstrel.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

net/mac80211/rc80211_minstrel.c +12 -0

index f3d88b0..748b5db 100644
@@ -657,6 +657,17 @@ minstrel_free(void *priv)
 	kfree(priv);
 }
 
+static void
+minstrel_get_minstrel_best_rate(void *priv, void *priv_sta,
+				struct ieee80211_minstrel_rate_info *info)
+{
+	struct minstrel_sta_info *mi = priv_sta;
+	int idx = mi->max_tp_rate[0];
+
+	info->rate.idx = mi->r[idx].rix;
+	info->prob = MINSTREL_TRUNC(mi->r[idx].probability * 1000);
+}
+
 struct rate_control_ops mac80211_minstrel = {
 	.name = "minstrel",
 	.tx_status = minstrel_tx_status,

@@ -670,6 +681,7 @@ struct rate_control_ops mac80211_minstrel = {
 	.add_sta_debugfs = minstrel_add_sta_debugfs,
 	.remove_sta_debugfs = minstrel_remove_sta_debugfs,
 #endif
+	.get_minstrel_best_rate = minstrel_get_minstrel_best_rate,
 };
 
 int __init

Signature · 1 line

1.8.5.3

Antonio Quartulli

author

2014-01-21 11:11 UTC #13 lore raw

RFC 4/5 mac80211: minstrel_ht - implement get_minstrel_best_rate() API

In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>

Signed-off-by Antonio Quartulli <[email protected]>

---
 net/mac80211/rc80211_minstrel_ht.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

net/mac80211/rc80211_minstrel_ht.c +30 -0

index c1b5b73..f7b1de2 100644
@@ -1031,6 +1031,35 @@ minstrel_ht_free(void *priv)
 	mac80211_minstrel.free(priv);
 }
 
+static void
+minstrel_ht_get_minstrel_best_rate(void *priv, void *priv_sta,
+				   struct ieee80211_minstrel_rate_info *info)
+{
+	struct minstrel_ht_sta_priv *msp = priv_sta;
+	struct minstrel_ht_sta *mi = &msp->ht;
+	struct ieee80211_sta_rates ratetbl;
+	struct minstrel_priv *mp = priv;
+	struct minstrel_rate_stats *mr;
+	int i, j;
+
+	if (!msp->is_ht) {
+		mac80211_minstrel.get_minstrel_best_rate(priv, priv_sta, info);
+		return;
+	}
+
+	i = mi->max_tp_rate / MCS_GROUP_RATES;
+	j = mi->max_tp_rate % MCS_GROUP_RATES;
+
+	mr = &mi->groups[i].rates[j];
+
+	minstrel_ht_set_rate(mp, mi, &ratetbl, 0, mi->max_tp_rate);
+
+	info->rate.idx = ratetbl.rate[0].idx;
+	info->rate.count = ratetbl.rate[0].count;
+	info->rate.flags = ratetbl.rate[0].flags;
+	info->prob = MINSTREL_TRUNC(mr->probability * 1000);
+}
+
 static struct rate_control_ops mac80211_minstrel_ht = {
 	.name = "minstrel_ht",
 	.tx_status = minstrel_ht_tx_status,

@@ -1045,6 +1074,7 @@ static struct rate_control_ops mac80211_minstrel_ht = {
 	.add_sta_debugfs = minstrel_ht_add_sta_debugfs,
 	.remove_sta_debugfs = minstrel_ht_remove_sta_debugfs,
 #endif
+	.get_minstrel_best_rate = minstrel_ht_get_minstrel_best_rate,
 };

Signature · 1 line

1.8.5.3

Antonio Quartulli

author

2014-01-21 11:11 UTC #14 lore raw

RFC 5/5 cfg80211: implement cfg80211_get_station

In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>

Implement and export the new cfg80211_get_station() API. This utility can be used by other kernel modules to obtain detailed information about a given wireless station.

It will be in particular useful to batman-adv which will implement a wireless rate based metric.

Signed-off-by Antonio Quartulli <[email protected]>

---
 include/net/cfg80211.h | 11 +++++++++++
 net/wireless/nl80211.c | 22 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)

include/net/cfg80211.h +11 -0

index 51d5d94..8c4b6ee 100644
@@ -1067,6 +1067,17 @@ struct station_info {
 };
 
 /**
+ * cfg80211_get_station - retrieve information about a given station
+ * @dev: the device where the station is supposed to be connected to
+ * @mac_addr: the mac address of the station of interest
+ * @sinfo: pointer to the structure to fill with the information
+ *
+ * Returns 0 on success or a negative error code otherwise.
+ */
+int cfg80211_get_station(struct net_device *dev, const u8 *mac_addr,
+			 struct station_info *sinfo);
+
+/**
  * enum monitor_flags - monitor flags
  *
  * Monitor interface configuration flags. Note that these must be the bits

net/wireless/nl80211.c +22 -0

index 6e78c62..acba667 100644
@@ -3780,6 +3780,28 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
 	return genlmsg_reply(msg, info);
 }
 
+int cfg80211_get_station(struct net_device *dev, const u8 *mac_addr,
+			 struct station_info *sinfo)
+{
+	struct cfg80211_registered_device *rdev;
+	struct wireless_dev *wdev;
+	int err;
+
+	wdev = dev->ieee80211_ptr;
+	if (!wdev)
+		return -EOPNOTSUPP;
+
+	rdev = wiphy_to_dev(wdev->wiphy);
+	if (!rdev->ops->get_station)
+		return -EOPNOTSUPP;
+
+	err = rdev_get_station(rdev, dev, mac_addr, sinfo);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 int cfg80211_check_station_change(struct wiphy *wiphy,
 				  struct station_parameters *params,
 				  enum cfg80211_station_type statype)

Signature · 1 line

1.8.5.3

Johannes Berg

maintainer

2014-01-21 16:03 UTC #15 lore raw

Re: [RFC 5/5] cfg80211: implement cfg80211_get_station

net/wireless/nl80211.c static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info) · line 3784

Quoted 7 lines · Antonio Quartulli

+++ b/net/wireless/nl80211.c

net/wireless/nl80211.c +2 -0

@@ -3780,6 +3780,28 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
 	return genlmsg_reply(msg, info);
 }
 
+int cfg80211_get_station(struct net_device *dev, const u8 *mac_addr,
+			 struct station_info *sinfo)

Not sure nl80211.c is the best place for this?

Quoted 8 lines

net/wireless/nl80211.c +8 -0

+{
+	struct cfg80211_registered_device *rdev;
+	struct wireless_dev *wdev;
+	int err;
+
+	wdev = dev->ieee80211_ptr;
+	if (!wdev)
+		return -EOPNOTSUPP;

net/wireless/nl80211.c static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info) · line 3802

Quoted 10 lines

net/wireless/nl80211.c +10 -0

+
+	rdev = wiphy_to_dev(wdev->wiphy);
+	if (!rdev->ops->get_station)
+		return -EOPNOTSUPP;
+
+	err = rdev_get_station(rdev, dev, mac_addr, sinfo);
+	if (err)
+		return err;
+
+	return 0;

Those "err" variable checks and whatever are just pointless -- this is really just
return rdev_get_station(...)

net/wireless/nl80211.c static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info) · line 3803

Quoted 1 line

net/wireless/nl80211.c +1 -0
+}

Consider exporting it if you actually want to use your new function. :)

johannes

Antonio Quartulli

2014-01-21 16:08 UTC #16 lore raw

Re: [RFC 5/5] cfg80211: implement cfg80211_get_station

Quoted 11 lines · Johannes Berg

net/wireless/nl80211.c static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info) · line 3784
Quoted 7 lines · Antonio Quartulli
+++ b/net/wireless/nl80211.c
net/wireless/nl80211.c +2 -0
@@ -3780,6 +3780,28 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
 	return genlmsg_reply(msg, info);
 }
 
+int cfg80211_get_station(struct net_device *dev, const u8 *mac_addr,
+			 struct station_info *sinfo)
Not sure nl80211.c is the best place for this?

mh true, I saw other cfg80211_* function in there..but they are all NL80211 related. :)

I think util.c is a better place.

Quoted 15 lines

net/wireless/nl80211.c +14 -0

+{
+	struct cfg80211_registered_device *rdev;
+	struct wireless_dev *wdev;
+	int err;
+
+	wdev = dev->ieee80211_ptr;
+	if (!wdev)
+		return -EOPNOTSUPP;
+
+	rdev = wiphy_to_dev(wdev->wiphy);
+	if (!rdev->ops->get_station)
+		return -EOPNOTSUPP;
+
+	err = rdev_get_station(rdev, dev, mac_addr, sinfo);

Quoted 8 lines

net/wireless/nl80211.c static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info) · line 3802
Quoted 4 lines
net/wireless/nl80211.c +4 -0
+	if (err)
+		return err;
+
+	return 0;
Those "err" variable checks and whatever are just pointless -- this is really just
return rdev_get_station(...)

agreed!

Quoted 4 lines

net/wireless/nl80211.c static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info) · line 3803
Quoted 2 lines
net/wireless/nl80211.c +1 -0
+}
Consider exporting it if you actually want to use your new function. :)

agreed²!!

Thanks!

Signature · 1 line

Antonio Quartulli

Thomas Hühn

2014-01-21 13:59 UTC #17 lore raw

In reply to Antonio Quartulli

Hi Antonio,

I like you idea of making use of information from different layers to try to enhance wireless communication.
It seems reasonable for me to start exporting and further using/experimenting with that rate, where minstrel estimates the maximum throughput.
But on closer inspection, it might also be of certain interest (maybe not batman in particular) to use the most robust rate, which both Minstrels version do also have an estimate. So what do you think about exporting the whole sorted rate table structure (struct ieee80211_sta_rates) instead of extracting the single max thr. rate ?

Greetings Thomas

Quoted 55 lines · Antonio Quartulli

Hello list,
we (as batman-adv developers) are currently working on a new version of the our routing protocol which is going to use some Minstrel internal information to compute the metric.
In particular I am interested in the currently selected bitrate (which Minstrel selected because it "maximises the throughput") and it's probability of success.
To achieve so I am proposing here a change to the get_station API. This change is supposed to export the needed information only if the current driver is using the Minstrel (or Minstrel HT) RC algorithm.
Patch 1 introduced the change in get_station()
Patch 2 add a new rate_control API used to query the RC algorithm and retrieve
the information. Then it fills the sinfo object.
Patch 3 and 4 are implementing this rate_control API in minstrel and minstrel_ht Patch 5 exports the get_station API in order to allow other modules to use it.
I already had a discussion with Johannes about this patch being not generic enough and really focussed on Minstrel only.
However this change will just
introduce a new exported capability in the station_info object: if the driver does not support it (e.g. it does not use Minstrel) then we simply won't have this information (like we already do with other capabilities).
Cheers,
p.s. I may need to add some more kerneldoc
Antonio Quartulli (5):
cfg80211: export minstrel best rate information through get_station()
mac80211: export minstrel best rate information in set_sta_info()
mac80211: minstrel - implement get_minstrel_best_rate() API
mac80211: minstrel_ht - implement get_minstrel_best_rate() API
cfg80211: implement cfg80211_get_station
include/net/cfg80211.h             | 28 ++++++++++++++++++++++++++++
include/net/mac80211.h             | 15 +++++++++++++++
net/mac80211/cfg.c                 | 15 +++++++++++++++
net/mac80211/rc80211_minstrel.c    | 12 ++++++++++++
net/mac80211/rc80211_minstrel_ht.c | 30 ++++++++++++++++++++++++++++++
net/wireless/nl80211.c             | 22 ++++++++++++++++++++++
6 files changed, 122 insertions(+)
Signature · 6 lines
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Antonio Quartulli

2014-01-21 15:28 UTC #18 lore raw

Hi Thomas,

[added batman-adv mailing list to the CC]

Quoted 7 lines · Thomas Hühn

Hi Antonio,
I like you idea of making use of information from different layers to try to enhance wireless communication.
It seems reasonable for me to start exporting and further using/experimenting with that rate, where minstrel estimates the maximum throughput.
But on closer inspection, it might also be of certain interest (maybe not batman in particular) to use the most robust rate, which both Minstrels version do also have an estimate. So what do you think about exporting the whole sorted rate table structure (struct ieee80211_sta_rates) instead of extracting the single max thr. rate ?

Thank you very much for your feedback!

I like your idea about opening this change a bit more and allow other modules to use the same information.

However the ieee80211_sta_rates structure you mentioned is mac80211 private and it does not carry any data about the probability of success (which is a Minstrel specific value and for this reason it is not present in this ieee80211 generic structure).

Assuming that we want to keep the new exported data Minstrel specific (we really want this), we could extend the station_info to carry a *set* of "cfg80211_minstrel_rate_info" objects (new struct introduced within this patchset) which could represent the sorted rate table.

However I am not sure how this table can be used once exported, since the only thing having a meaning is the order. Other than that I don't see what a module could do with it (other than choosing the first entry).

Instead of exporting the whole rate table, should we only report those rates that have a particular meaning for Minstrel?
I think Minstrel is the only component having enough information about which rate is meaningful and which not. What do you think?

This could be done by exporting several cfg80211_minstrel_rate_info, e.g.
- max_throughput_rate
- most_robust_rate
- ...

(I used random names here)

Cheers,

Quoted 61 lines

Greetings Thomas
Quoted 55 lines · Antonio Quartulli
Hello list,
we (as batman-adv developers) are currently working on a new version of the our routing protocol which is going to use some Minstrel internal information to compute the metric.
In particular I am interested in the currently selected bitrate (which Minstrel selected because it "maximises the throughput") and it's probability of success.
To achieve so I am proposing here a change to the get_station API. This change is supposed to export the needed information only if the current driver is using the Minstrel (or Minstrel HT) RC algorithm.
Patch 1 introduced the change in get_station()
Patch 2 add a new rate_control API used to query the RC algorithm and retrieve
the information. Then it fills the sinfo object.
Patch 3 and 4 are implementing this rate_control API in minstrel and minstrel_ht Patch 5 exports the get_station API in order to allow other modules to use it.
I already had a discussion with Johannes about this patch being not generic enough and really focussed on Minstrel only.
However this change will just
introduce a new exported capability in the station_info object: if the driver does not support it (e.g. it does not use Minstrel) then we simply won't have this information (like we already do with other capabilities).
Cheers,
p.s. I may need to add some more kerneldoc
Antonio Quartulli (5):
cfg80211: export minstrel best rate information through get_station()
mac80211: export minstrel best rate information in set_sta_info()
mac80211: minstrel - implement get_minstrel_best_rate() API
mac80211: minstrel_ht - implement get_minstrel_best_rate() API
cfg80211: implement cfg80211_get_station
include/net/cfg80211.h             | 28 ++++++++++++++++++++++++++++
include/net/mac80211.h             | 15 +++++++++++++++
net/mac80211/cfg.c                 | 15 +++++++++++++++
net/mac80211/rc80211_minstrel.c    | 12 ++++++++++++
net/mac80211/rc80211_minstrel_ht.c | 30 ++++++++++++++++++++++++++++++
net/wireless/nl80211.c             | 22 ++++++++++++++++++++++
6 files changed, 122 insertions(+)
Signature · 6 lines
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Signature · 1 line

Antonio Quartulli