[RFC 0/5] Export Minstrel best API information via get_station()

[RFC 0/5] Export Minstrel best API information via get_station()

[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(+)

-- 
1.8.5.3

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

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

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 <antonio@open-mesh.com>
---
 include/net/cfg80211.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d5e57bf..51d5d94 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -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.
-- 
1.8.5.3

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

[Johannes Berg]

On Tue, 2014-01-21 at 12:09 +0100, Antonio Quartulli wrote:

> + * @prob: probability of success of this birate

typo: bitrate

> +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

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

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

On 21/01/14 17:00, Johannes Berg wrote:
> On Tue, 2014-01-21 at 12:09 +0100, Antonio Quartulli wrote:
> 
>> + * @prob: probability of success of this birate
> 
> typo: bitrate
> 
>> +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.

> 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)?


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Johannes Berg]

On Tue, 2014-01-21 at 17:09 +0100, Antonio Quartulli wrote:

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

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

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

On 21/01/14 17:18, Johannes Berg wrote:
> On Tue, 2014-01-21 at 17:09 +0100, Antonio Quartulli wrote:
> 
>>> 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.


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]

On 21/01/14 17:32, Antonio Quartulli wrote:
> On 21/01/14 17:18, Johannes Berg wrote:
>> On Tue, 2014-01-21 at 17:09 +0100, Antonio Quartulli wrote:
>>
>>>> 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.
> 
> 

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,

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Johannes Berg]

On Wed, 2014-01-22 at 15:41 +0100, Antonio Quartulli wrote:

> 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

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

[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 ?

Greetings Thomas

On 22.01.2014, at 15:43, Johannes Berg <johannes@sipsolutions.net> wrote:

> On Wed, 2014-01-22 at 15:41 +0100, Antonio Quartulli wrote:
> 
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]

On 22/01/14 19:32, Thomas Hühn wrote:
> 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,


> Greetings Thomas
> 
> On 22.01.2014, at 15:43, Johannes Berg <johannes@sipsolutions.net> wrote:
> 
>> On Wed, 2014-01-22 at 15:41 +0100, Antonio Quartulli wrote:
>>
>>> 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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

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 <antonio@open-mesh.com>
---
 include/net/mac80211.h | 15 +++++++++++++++
 net/mac80211/cfg.c     | 15 +++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index df1004b..9ed56f8 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -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,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 032081c..c0d394c 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -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] = {
-- 
1.8.5.3

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

[Johannes Berg]

On Tue, 2014-01-21 at 12:09 +0100, Antonio Quartulli wrote:

>  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);
>  };

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

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

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 net/mac80211/rc80211_minstrel.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index f3d88b0..748b5db 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -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
-- 
1.8.5.3

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

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 net/mac80211/rc80211_minstrel_ht.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index c1b5b73..f7b1de2 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -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,
 };
 
 
-- 
1.8.5.3

[RFC 5/5] cfg80211: implement cfg80211_get_station

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

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 <antonio@open-mesh.com>
---
 include/net/cfg80211.h | 11 +++++++++++
 net/wireless/nl80211.c | 22 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 51d5d94..8c4b6ee 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -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
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 6e78c62..acba667 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -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)
-- 
1.8.5.3

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

[Johannes Berg]

On Tue, 2014-01-21 at 12:09 +0100, Antonio Quartulli wrote:

> +++ b/net/wireless/nl80211.c
> @@ -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?

> +{
> +	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;

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

> +}

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

johannes

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

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]

On 21/01/14 17:03, Johannes Berg wrote:
> On Tue, 2014-01-21 at 12:09 +0100, Antonio Quartulli wrote:
> 
>> +++ b/net/wireless/nl80211.c
>> @@ -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.

> 
>> +{
>> +	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;
> 
> Those "err" variable checks and whatever are just pointless -- this is
> really just
> return rdev_get_station(...)

agreed!

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

agreed²!!


Thanks!


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 0/5] Export Minstrel best API information via get_station()

[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 ?

Greetings Thomas


On 21.01.2014, at 12:09, Antonio Quartulli <antonio@meshcoding.com> wrote:

> 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(+)
> 
> -- 
> 1.8.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 0/5] Export Minstrel best API information via get_station()

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 4687 bytes --]

Hi Thomas,

[added batman-adv mailing list to the CC]

On 21/01/14 14:52, Thomas Hühn wrote:
> 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,

> Greetings Thomas
> 
> 
> On 21.01.2014, at 12:09, Antonio Quartulli <antonio@meshcoding.com> wrote:
> 
>> 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(+)
>>
>> -- 
>> 1.8.5.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]