[PATCH 1/3] cfg80211: add get_max_tp() API

[PATCH 1/3] cfg80211: add get_max_tp() API

[Antonio Quartulli]

This new API is aimed to let other modules in the kernel
fetch the maximum throughput value towards a peer over
a given VIF.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 include/net/cfg80211.h | 20 ++++++++++++++++++++
 net/wireless/core.c    | 11 +++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 57870b6..5019f67 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2002,6 +2002,9 @@ struct cfg80211_update_ft_ies_params {
  * @update_ft_ies: Provide updated Fast BSS Transition information to the
  *	driver. If the SME is in the driver/firmware, this information can be
  *	used in building Authentication and Reassociation Request frames.
+ *
+ * @get_max_tp: Get the maximum throughput estimated by the rate control
+ *	algorithm towards a given peer
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -2231,6 +2234,9 @@ struct cfg80211_ops {
 					 struct cfg80211_chan_def *chandef);
 	int	(*update_ft_ies)(struct wiphy *wiphy, struct net_device *dev,
 				 struct cfg80211_update_ft_ies_params *ftie);
+
+	int	(*get_max_tp)(struct wireless_dev *wdev, const u8 *peer,
+			      u32 *tp);
 };
 
 /*
@@ -4126,6 +4132,20 @@ void cfg80211_report_wowlan_wakeup(struct wireless_dev *wdev,
 				   struct cfg80211_wowlan_wakeup *wakeup,
 				   gfp_t gfp);
 
+/**
+ * cfg80211_get_max_tp - get the maximum estimated throughput towards a peer
+ * @wdev: the wireless device which the peer is connected to
+ * @peer: MAC address of the peer
+ * @tp: output buffer. Will contain the throughput value
+ *
+ * This functions queries the underlaying driver and gets the maximum
+ * estimated throughput towards the given peer. The result is then stored in the
+ * variable pointed by tp
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+int cfg80211_get_max_tp(struct wireless_dev *wdev, u8 *peer, u32 *tp);
+
 /* Logging, debugging and troubleshooting/diagnostic helpers. */
 
 /* wiphy_printk helpers, similar to dev_printk */
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 92e3fd4..ae86515 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -854,6 +854,17 @@ void cfg80211_leave(struct cfg80211_registered_device *rdev,
 	wdev->beacon_interval = 0;
 }
 
+int cfg80211_get_max_tp(struct wireless_dev *wdev, u8 *peer, u32 *tp)
+{
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
+
+	if (!rdev->ops->get_max_tp)
+		return -EOPNOTSUPP;
+
+	return rdev->ops->get_max_tp(wdev, peer, tp);
+}
+EXPORT_SYMBOL(cfg80211_get_max_tp);
+
 static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 					 unsigned long state,
 					 void *ndev)
-- 
1.8.1.5

[PATCH 2/3] mac80211: add rate_control_ops::get_max_tp() and implement it

[Antonio Quartulli]

This new API is aimed to return the maximum throughput
estimated by a rate control algorithm towards a given peer.

The API is implemented for minstrel and minstrel_ht.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 include/net/mac80211.h             |  2 ++
 net/mac80211/rc80211_minstrel.c    |  9 +++++++++
 net/mac80211/rc80211_minstrel_ht.c | 21 +++++++++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 64faf01..5a1b7c2 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -4147,6 +4147,8 @@ 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_max_tp)(void *priv_sta, u32 *tp);
 };
 
 static inline int rate_supported(struct ieee80211_sta *sta,
diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index 1c36c9b..7af6884 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -604,6 +604,14 @@ minstrel_free(void *priv)
 	kfree(priv);
 }
 
+static void minstrel_get_max_tp(void *priv_sta, u32 *tp)
+{
+	struct minstrel_sta_info *mi = priv_sta;
+	int idx = mi->max_tp_rate[0];
+
+	*tp = MINSTREL_TRUNC(mi->r[idx].cur_tp / 10);
+}
+
 struct rate_control_ops mac80211_minstrel = {
 	.name = "minstrel",
 	.tx_status = minstrel_tx_status,
@@ -617,6 +625,7 @@ struct rate_control_ops mac80211_minstrel = {
 	.add_sta_debugfs = minstrel_add_sta_debugfs,
 	.remove_sta_debugfs = minstrel_remove_sta_debugfs,
 #endif
+	.get_max_tp = minstrel_get_max_tp,
 };
 
 int __init
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index d2b264d..421c770 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -960,6 +960,26 @@ error:
 	return NULL;
 }
 
+static void minstrel_ht_get_max_tp(void *priv_sta, u32 *tp)
+{
+	struct minstrel_ht_sta_priv *msp = priv_sta;
+	struct minstrel_ht_sta *mi = &msp->ht;
+	struct minstrel_rate_stats *mr;
+	int i, j;
+
+	if (!msp->is_ht) {
+		mac80211_minstrel.get_max_tp(priv_sta, tp);
+		return;
+	}
+
+	i = mi->max_tp_rate / MCS_GROUP_RATES;
+	j = mi->max_tp_rate % MCS_GROUP_RATES;
+
+	mr = &mi->groups[i].rates[j];
+
+	*tp = mr->cur_tp / 10;
+}
+
 static void
 minstrel_ht_free_sta(void *priv, struct ieee80211_sta *sta, void *priv_sta)
 {
@@ -996,6 +1016,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_max_tp = minstrel_ht_get_max_tp,
 };
 
 
-- 
1.8.1.5

[PATCH 3/3] mac80211: implement cfg80211_ops::get_max_tp() API

[Antonio Quartulli]

Implement the get_max_tp() API function by asking the
rate control algorithm for the maximum estimated throughput

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

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index edca2a2..2a78957 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3374,6 +3374,27 @@ static int ieee80211_cfg_get_channel(struct wiphy *wiphy,
 	return ret;
 }
 
+int ieee80211_get_max_tp(struct wireless_dev *wdev, const u8 *peer, u32 *tp)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
+	struct rate_control_ref *ref = sdata->local->rate_ctrl;
+	struct sta_info *sta;
+
+	rcu_read_lock();
+	sta = sta_info_get(sdata, peer);
+	rcu_read_unlock();
+
+	if (!sta)
+		return -ENOENT;
+
+	if (!ref->ops->get_max_tp)
+		return -EOPNOTSUPP;
+
+	ref->ops->get_max_tp(sta->rate_ctrl_priv, tp);
+
+	return 0;
+}
+
 #ifdef CONFIG_PM
 static void ieee80211_set_wakeup(struct wiphy *wiphy, bool enabled)
 {
@@ -3459,4 +3480,5 @@ struct cfg80211_ops mac80211_config_ops = {
 	.get_et_strings = ieee80211_get_et_strings,
 	.get_channel = ieee80211_cfg_get_channel,
 	.start_radar_detection = ieee80211_start_radar_detection,
+	.get_max_tp = ieee80211_get_max_tp,
 };
-- 
1.8.1.5

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Helmut Schaa]

Hi Antonio,

On Thu, Apr 4, 2013 at 9:57 PM, Antonio Quartulli <antonio@open-mesh.com> wrote:
> This new API is aimed to let other modules in the kernel
> fetch the maximum throughput value towards a peer over
> a given VIF.

Just curios, what's the intended use for this?

Thanks,
Helmut

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Antonio Quartulli]

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

Hi Helmut,

On Fri, Apr 05, 2013 at 01:21:06AM -0700, Helmut Schaa wrote:
> Hi Antonio,
> 
> On Thu, Apr 4, 2013 at 9:57 PM, Antonio Quartulli <antonio@open-mesh.com> wrote:
> > This new API is aimed to let other modules in the kernel
> > fetch the maximum throughput value towards a peer over
> > a given VIF.
> 
> Just curios, what's the intended use for this?
> 

In the batman-adv module (which implements a routing protocol for mesh networks
on layer 2) we are trying to switch metric from packet loss to throughput and
the idea is to read the estimation from the rate control component (thanks to
the API mechanism in cfg/mac80211 this can be eventually changed later).

I am not entirely sure that the value I proposed to read (the maximum in the
rate table) is the one which better plays the needed role. But the preliminar
tests demonstrated that it  :)


Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Johannes Berg]

On Fri, 2013-04-05 at 10:39 +0200, Antonio Quartulli wrote:

> In the batman-adv module (which implements a routing protocol for mesh networks
> on layer 2) we are trying to switch metric from packet loss to throughput and
> the idea is to read the estimation from the rate control component (thanks to
> the API mechanism in cfg/mac80211 this can be eventually changed later).

While this makes some sense, going into the details of your patchset I
find that it's overly complex.

I think you should fix minstrel to report the best rate in
txrc.reported_rate. This would also have the effect of not showing
sampling attempts to userspace in the "current TX rate", which generally
makes a lot of sense.

After doing that, reading the rate becomes a get_station_info() call or
so.

One more detail:

int cfg80211_get_max_tp(struct wireless_dev *wdev, u8 *peer, u32 *tp)

I really don't think that the wireless_dev should be necessary for this,
it ought to be just a netdev IMHO. Also, the peer should be const :)

johannes

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Antonio Quartulli]

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

Hi all,

On Fri, Apr 05, 2013 at 06:20:01AM -0700, Johannes Berg wrote:
> On Fri, 2013-04-05 at 10:39 +0200, Antonio Quartulli wrote:
> 
> > In the batman-adv module (which implements a routing protocol for mesh networks
> > on layer 2) we are trying to switch metric from packet loss to throughput and
> > the idea is to read the estimation from the rate control component (thanks to
> > the API mechanism in cfg/mac80211 this can be eventually changed later).
> 
> While this makes some sense, going into the details of your patchset I
> find that it's overly complex.
> 
> I think you should fix minstrel to report the best rate in
> txrc.reported_rate. This would also have the effect of not showing
> sampling attempts to userspace in the "current TX rate", which generally
> makes a lot of sense.

But the reported_rate field would just contain the index of the selected rate,
not the throughput. As far as I can tell the latter is an RC private information
(it not exported anywhere outside of the RC algorithm) and that is why I made
this API which would directly talk to minstrel and get this value.

> 
> After doing that, reading the rate becomes a get_station_info() call or
> so.
> 

true, but still we have the problem above..unless I alter the ieee8011_tx_rate
and rate_info struct..but I don't know if this would make much sense.

> One more detail:
> 
> int cfg80211_get_max_tp(struct wireless_dev *wdev, u8 *peer, u32 *tp)
> 
> I really don't think that the wireless_dev should be necessary for this,
> it ought to be just a netdev IMHO.

well, yes, I can pass the struct net_dev only, even if I need the wdev to obtain
the cf80211_register_dev object for the ops.

> Also, the peer should be const :)

Right


Thanks.
Cheer,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Johannes Berg]

> But the reported_rate field would just contain the index of the selected rate,
> not the throughput. As far as I can tell the latter is an RC private information
> (it not exported anywhere outside of the RC algorithm) and that is why I made
> this API which would directly talk to minstrel and get this value.

Actually it would contain the entire rate configuration.

Anyway my concern is that you're adding something that's rather minstrel
specific. It's not really usable by any other algorithm, you're
reporting minstrel's estimation of the throughput. If you report the
current "best" rate, that'll probably get you pretty much the same
behaviour overall, but be more portable to other algorithms I think.

johannes

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Antonio Quartulli]

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

On Tue, Apr 09, 2013 at 03:25:08 -0700, Johannes Berg wrote:
> 
> > But the reported_rate field would just contain the index of the selected rate,
> > not the throughput. As far as I can tell the latter is an RC private information
> > (it not exported anywhere outside of the RC algorithm) and that is why I made
> > this API which would directly talk to minstrel and get this value.
> 
> Actually it would contain the entire rate configuration.
> 
> Anyway my concern is that you're adding something that's rather minstrel
> specific. It's not really usable by any other algorithm, you're
> reporting minstrel's estimation of the throughput. If you report the
> current "best" rate, that'll probably get you pretty much the same
> behaviour overall, but be more portable to other algorithms I think.

I understand your concern. My guess was that every algorithm was "somehow" able
to provide such measurement. The point is that the throughput value is computed
so that it also take probability of success into consideration.
This means that two nodes using the same rate may have different throughputs
(and this is important when building our distributed metric).

However, nothing prevents any algorithm to implement the API the way it can do.
I've not looked into other RC implementations yet, but I guess they would have a
similar value to return too?

Cheers,

> 
> johannes
> 

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Johannes Berg]

On Tue, 2013-04-09 at 13:34 +0200, Antonio Quartulli wrote:

> > Anyway my concern is that you're adding something that's rather minstrel
> > specific. It's not really usable by any other algorithm, you're
> > reporting minstrel's estimation of the throughput. If you report the
> > current "best" rate, that'll probably get you pretty much the same
> > behaviour overall, but be more portable to other algorithms I think.
> 
> I understand your concern. My guess was that every algorithm was "somehow" able
> to provide such measurement. The point is that the throughput value is computed
> so that it also take probability of success into consideration.
> This means that two nodes using the same rate may have different throughputs
> (and this is important when building our distributed metric).
> 
> However, nothing prevents any algorithm to implement the API the way it can do.
> I've not looked into other RC implementations yet, but I guess they would have a
> similar value to return too?

Maybe, yeah.

Anyway, I think having a separate externally visible API here is
overkill. It would seem a lot simpler to return it (to userspace) in the
station information, and (separately) allow other kernel modules to
request station information as well.

Also I'm not sure it should be called "max_tp"? It's more like "expected
throughput" or something like that?

johannes

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Johannes Berg]

On Thu, 2013-04-11 at 11:56 +0200, Johannes Berg wrote:
> On Tue, 2013-04-09 at 13:34 +0200, Antonio Quartulli wrote:
> 
> > > Anyway my concern is that you're adding something that's rather minstrel
> > > specific. It's not really usable by any other algorithm, you're
> > > reporting minstrel's estimation of the throughput. If you report the
> > > current "best" rate, that'll probably get you pretty much the same
> > > behaviour overall, but be more portable to other algorithms I think.
> > 
> > I understand your concern. My guess was that every algorithm was "somehow" able
> > to provide such measurement. The point is that the throughput value is computed
> > so that it also take probability of success into consideration.
> > This means that two nodes using the same rate may have different throughputs
> > (and this is important when building our distributed metric).
> > 
> > However, nothing prevents any algorithm to implement the API the way it can do.
> > I've not looked into other RC implementations yet, but I guess they would have a
> > similar value to return too?
> 
> Maybe, yeah.
> 
> Anyway, I think having a separate externally visible API here is
> overkill. It would seem a lot simpler to return it (to userspace) in the
> station information, and (separately) allow other kernel modules to
> request station information as well.
> 
> Also I'm not sure it should be called "max_tp"? It's more like "expected
> throughput" or something like that?

Hm, no, that's not really it either ... It's maybe more like "current
usable data rate" (as opposed to PHY rate?)

johannes

Re: [PATCH 1/3] cfg80211: add get_max_tp() API

[Antonio Quartulli]

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

On Thu, Apr 11, 2013 at 02:57:41AM -0700, Johannes Berg wrote:
> On Thu, 2013-04-11 at 11:56 +0200, Johannes Berg wrote:
> > On Tue, 2013-04-09 at 13:34 +0200, Antonio Quartulli wrote:
> > 
> > > > Anyway my concern is that you're adding something that's rather minstrel
> > > > specific. It's not really usable by any other algorithm, you're
> > > > reporting minstrel's estimation of the throughput. If you report the
> > > > current "best" rate, that'll probably get you pretty much the same
> > > > behaviour overall, but be more portable to other algorithms I think.
> > > 
> > > I understand your concern. My guess was that every algorithm was "somehow" able
> > > to provide such measurement. The point is that the throughput value is computed
> > > so that it also take probability of success into consideration.
> > > This means that two nodes using the same rate may have different throughputs
> > > (and this is important when building our distributed metric).
> > > 
> > > However, nothing prevents any algorithm to implement the API the way it can do.
> > > I've not looked into other RC implementations yet, but I guess they would have a
> > > similar value to return too?
> > 
> > Maybe, yeah.
> > 
> > Anyway, I think having a separate externally visible API here is
> > overkill. It would seem a lot simpler to return it (to userspace) in the
> > station information, and (separately) allow other kernel modules to
> > request station information as well.

Ok, sounds good!

> > 
> > Also I'm not sure it should be called "max_tp"? It's more like "expected
> > throughput" or something like that?
> 
> Hm, no, that's not really it either ... It's maybe more like "current
> usable data rate" (as opposed to PHY rate?)
> 

Mh..well, it is not a "real" value, so I would not use neither "expected" nor
"usable". It is supposed to be the result of a computation giving us an
estimation of the current/last throughput. "estimated throughput" sounds bad?

Well the name is not really important I guess :) I start sending some code using
the latter. We may change it at the end before merging (if you decide to do so).

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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