[PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Larry Finger]

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 <Larry.Finger@lwfinger.net>
---

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

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Johannes Berg]

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Jiri Benc]

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

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Johannes Berg]

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Michael Wu]

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

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

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

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Larry Finger]

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

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Michael Wu]

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

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

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

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Larry Finger]

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

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Michael Wu]

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

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

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

Re: [PATCH V3] mac80211: Add support for SIOCGIWRATE ioctl to provide rate information

[Larry Finger]

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