At present, transmission rate information for mac80211 is available only
if verbose debugging is turned on, and then only in the logs. This patch
implements the SIOCGIWRATE ioctl, which adds the current transmission rate to
the output of iwconfig.
Signed-off-by: Larry Finger <[email protected]>
---
Index: wireless-dev/net/mac80211/ieee80211_ioctl.c
===================================================================
--- wireless-dev.orig/net/mac80211/ieee80211_ioctl.c
+++ wireless-dev/net/mac80211/ieee80211_ioctl.c
@@ -2013,6 +2013,27 @@ static int ieee80211_ioctl_giwscan(struc
}
+static int ieee80211_ioctl_giwrate(struct net_device *dev,
+ struct iw_request_info *info,
+ struct iw_param *rate, char *extra)
+{
+ struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct sta_info * sta;
+ struct ieee80211_sub_if_data * sdata;
+
+ sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ if (sdata->type == IEEE80211_IF_TYPE_STA ||
+ sdata->type == IEEE80211_IF_TYPE_IBSS)
+ sta = sta_info_get(local, sdata->u.sta.bssid);
+ else
+ return -EOPNOTSUPP;
+ if (!sta)
+ return -ENODEV;
+ rate->value = local->oper_hw_mode->rates[sta->txrate].rate * 100000;
+ sta_info_put(sta);
+ return 0;
+}
+
static int ieee80211_ioctl_siwrts(struct net_device *dev,
struct iw_request_info *info,
struct iw_param *rts, char *extra)
@@ -3137,7 +3158,7 @@ static const iw_handler ieee80211_handle
(iw_handler) NULL, /* -- hole -- */
(iw_handler) NULL, /* -- hole -- */
(iw_handler) NULL, /* SIOCSIWRATE */
- (iw_handler) NULL, /* SIOCGIWRATE */
+ (iw_handler) ieee80211_ioctl_giwrate, /* SIOCGIWRATE */
(iw_handler) ieee80211_ioctl_siwrts, /* SIOCSIWRTS */
(iw_handler) ieee80211_ioctl_giwrts, /* SIOCGIWRTS */
(iw_handler) ieee80211_ioctl_siwfrag, /* SIOCSIWFRAG */
On Fri, 04 May 2007 12:58:32 +0200, Johannes Berg wrote:
> Is this racy if somebody changes the type of the interface at the same
> time? Would a fix be to sta_info_get() beforehand? I really don't know
> without digging the code, Jiri, Michael?
It's under rtnl, so that's fine.
Jiri
--
Jiri Benc
SUSE Labs
Michael Wu wrote:
> On Thursday 10 May 2007 10:56, Larry Finger wrote:
>> Why has the review of this patch been so strung out? Everything you comment
>> on here was in versions 1 & 2, but not mentioned in any review of those
>> versions.
>>
> Sorry, I've been busy. I'm pretty sure these issues are the last ones.
>
>> As to the usefulness of this function, I am testing in infrastructure, not
>> ad-hoc, mode, and the output matches the average rate in the log files if
>> debugging is enabled. If the address in u.sta.bssid is not correct, which
>> address should I be using? I still maintain that the user is entitled to
>> know the transmission rate from user space even if debugging is turned off,
>> and that this function is needed.
>>
> I didn't say infrastructure wasn't gonna work. That's the one that should
> work. However, adhoc probably won't.
Sorry, I misread your previous comment.
Why is txrate in the sta_info structure an int? I think it should be a small, positive integer.
Would it not be better as a u8, or a u16 if you think a larger range is needed?
Larry
>
> -Michael Wu
On Fri, 2007-05-04 at 13:00 +0200, Jiri Benc wrote:
> On Fri, 04 May 2007 12:58:32 +0200, Johannes Berg wrote:
> > Is this racy if somebody changes the type of the interface at the same
> > time? Would a fix be to sta_info_get() beforehand? I really don't know
> > without digging the code, Jiri, Michael?
>
> It's under rtnl, so that's fine.
Hah, good point, sorry.
johannes
On Thursday 03 May 2007 21:21, Larry Finger wrote:
> +static int ieee80211_ioctl_giwrate(struct net_device *dev,
> + struct iw_request_info *info,
> + struct iw_param *rate, char *extra)
> +{
> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> + struct sta_info * sta;
> + struct ieee80211_sub_if_data * sdata;
Eliminate the spaces between * and the variable name.
> +
> + sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> + if (sdata->type == IEEE80211_IF_TYPE_STA ||
> + sdata->type == IEEE80211_IF_TYPE_IBSS)
> + sta = sta_info_get(local, sdata->u.sta.bssid);
> + else
> + return -EOPNOTSUPP;
> + if (!sta)
> + return -ENODEV;
> + rate->value = local->oper_hw_mode->rates[sta->txrate].rate * 100000;
The index from sta->txrate needs to be checked to make sure it is less than
mode->num_rates.
Also, this function is safe in adhoc mode.. but it most likely won't do
anything useful since all the sta entries have different addresses and it is
likely that none of them are the same as the one in sdata->u.sta.bssid.
-Michael Wu
On Thursday 10 May 2007 10:56, Larry Finger wrote:
> Why has the review of this patch been so strung out? Everything you comment
> on here was in versions 1 & 2, but not mentioned in any review of those
> versions.
>
Sorry, I've been busy. I'm pretty sure these issues are the last ones.
> As to the usefulness of this function, I am testing in infrastructure, not
> ad-hoc, mode, and the output matches the average rate in the log files if
> debugging is enabled. If the address in u.sta.bssid is not correct, which
> address should I be using? I still maintain that the user is entitled to
> know the transmission rate from user space even if debugging is turned off,
> and that this function is needed.
>
I didn't say infrastructure wasn't gonna work. That's the one that should
work. However, adhoc probably won't.
-Michael Wu
Michael Wu wrote:
> On Thursday 03 May 2007 21:21, Larry Finger wrote:
>> +static int ieee80211_ioctl_giwrate(struct net_device *dev,
>> + struct iw_request_info *info,
>> + struct iw_param *rate, char *extra)
>> +{
>> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
>> + struct sta_info * sta;
>> + struct ieee80211_sub_if_data * sdata;
> Eliminate the spaces between * and the variable name.
>
>> +
>> + sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>> + if (sdata->type == IEEE80211_IF_TYPE_STA ||
>> + sdata->type == IEEE80211_IF_TYPE_IBSS)
>> + sta = sta_info_get(local, sdata->u.sta.bssid);
>> + else
>> + return -EOPNOTSUPP;
>> + if (!sta)
>> + return -ENODEV;
>> + rate->value = local->oper_hw_mode->rates[sta->txrate].rate * 100000;
> The index from sta->txrate needs to be checked to make sure it is less than
> mode->num_rates.
>
> Also, this function is safe in adhoc mode.. but it most likely won't do
> anything useful since all the sta entries have different addresses and it is
> likely that none of them are the same as the one in sdata->u.sta.bssid.
Why has the review of this patch been so strung out? Everything you comment on here was in versions
1 & 2, but not mentioned in any review of those versions.
As to the usefulness of this function, I am testing in infrastructure, not ad-hoc, mode, and the
output matches the average rate in the log files if debugging is enabled. If the address in
u.sta.bssid is not correct, which address should I be using? I still maintain that the user is
entitled to know the transmission rate from user space even if debugging is turned off, and that
this function is needed.
Larry
On Thursday 10 May 2007 12:39, Larry Finger wrote:
> Why is txrate in the sta_info structure an int? I think it should be a
> small, positive integer. Would it not be better as a u8, or a u16 if you
> think a larger range is needed?
>
u8 is probably fine, but then last_txrate and last_nonerp_idx should be
changed to u8 too plus all the stuff referring to the rate indexes.
But there's more interesting things to do, like implementing siwrate so we can
get rid of the corresponding prism2 ioctl.
-Michael Wu
Michael Wu wrote:
>
> But there's more interesting things to do, like implementing siwrate so we can
> get rid of the corresponding prism2 ioctl.
I'll take a stab at siwrate. It will be more interesting than what I'm doing now.
Larry
On Thu, 2007-05-03 at 20:21 -0500, Larry Finger wrote:
> + sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> + if (sdata->type == IEEE80211_IF_TYPE_STA ||
> + sdata->type == IEEE80211_IF_TYPE_IBSS)
> + sta = sta_info_get(local, sdata->u.sta.bssid);
> + else
> + return -EOPNOTSUPP;
Is this racy if somebody changes the type of the interface at the same
time? Would a fix be to sta_info_get() beforehand? I really don't know
without digging the code, Jiri, Michael?
johannes