| Message ID | 1365105442-31876-1-git-send-email-antonio@open-mesh.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 -- 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
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,
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 -- 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
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,
> 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 -- 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
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 >
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 -- 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
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 -- 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
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,
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)
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(+)