Re: rt2x00/rt2500 support

[Dan Williams <dcbw redhat com>]

On Fri, 2006-07-14 at 13:25 +1200, Eion Robb wrote:
> I created a testing function to fill in the wireless stats with 0 values,  
> and great news, the rt2x00 chipset is detected as a wireless card.  It  
> does everything except WPA, so not much better than the legacy  
> rt2400/rt2500 at the moment.

(Adding Jiri+Jouni back to this, since there appears to be a deficiency
in d80211 bits)

Ok, so in addition to not doing anything with the get_wireless_stats
handler, d80211 appears to make a mess out of the range stuff too.

The d80211 stack clearly supports the ENCODEEXT and AUTH handlers.
However, the ieee80211_ioctl_giwrange() handler in ieee80211_ioctl.c is
_pathetically_ small and shouldn't even be allowed out of the asylum:

	data->length = sizeof(struct iw_range);
	memset(range, 0, sizeof(struct iw_range));

	range->we_version_compiled = WIRELESS_EXT;
	range->we_version_source = 14;
	range->retry_capa = IW_RETRY_LIMIT;
	range->retry_flags = IW_RETRY_LIMIT;
	range->min_retry = 0;
	range->max_retry = 255;
	range->min_rts = 0;
	range->max_rts = 2347;
	range->min_frag = 256;
	range->max_frag = 2346;

I can't even really begin to describe how broken that is; look at eg the
airo driver or ipw2x00 for the real complete way of setting up the range
structure so that user apps can actually figure out all the driver's
capabilities.

The d80211 'range' structure needs to:

a) set range->we_version_source >= 18
b) set range->num_channels and range->num_freq correctly, and set up
supported frequencies in range->freq[]
c) set quality correctly in range->max_qual & range->avg_qual
d) set up supported rates in range->bitrate[]
e) maybe set range->throughput, though this isn't really used
f) set number of key sizes and fill in range->encoding_size[] array
g) set allowable transmit power values in range->txpower[] array
h) set range->event_capa with the correct event capabilities, which are
at _least_ SIOCGIWAP and SIOCGIWSCAN
i) set range->enc_capa to say that it supports WPA as such:

        range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
                IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;

(item "i" here will give you WPA capabilities in NetworkManager if you
add that snippet of code to ieee80211_ioctl_giwrange() before the
function returns)

> I seem to be getting a lot of "Device or resource busy" warnings from  
> NetworkManager like

Ok; that means the driver is returning a non-zero return value from the
ioctl()s that NM is calling.  NM is simply printing out the error that
the driver is returning.

> NetworkManager: <WARNING>        nm_device_802_11_wireless_set_mode ():  
> error setting card wlan0 to Infrastructure mode: Device or resource busy

This appears to happen due to this check in ieee80211_ioctl_siwmode() in
ieee80211_ioctl.c:

	if (netif_running(dev))
		return -EBUSY;

If I understand that correctly, that means the driver is marked
IFF_RUNNING, and I think returning -EBUSY here is also likely wrong.
What should a user space application have to take the device down (which
also clears IFF_RUNNING AFAIK) just to be able to switch modes?  That
hasn't been the case until now with d80211, and it seems somewhat wrong
to me.  I'm somewhat ambiguous here though, somebody could probably make
a case to convince me that we have to change the behavior of userspace
apps to require a down and then mode change.

At a minimum, it should silently return success if the mode that's about
to be set is the same mode the driver is already in.

> NetworkManager: <WARNING>        nm_device_802_11_wireless_scan (): could  
> not trigger wireless scan on device wlan0: Device or resource busy

It appears that is because the station is already scanning; I would
argue that if d80211 is already scanning, the driver should silently
return success from the SIWSCAN handler because scan result
notifications will come along to every application eventually anyway via
netlink, and each application that cares about scans can get the results
then.

I think d80211 is wrong here; which is to say in ieee80211_sta.c in
ieee80211_sta_req_scan() if I follow the call chain correctly.

> (even though it did actually scan here... I'm a bit confused)
> 
> The only other warning I can see that might have something to do with WPA  
> not working is
> ** (nm-applet:3052): WARNING **: <WARNING>       wsm_set_capabilities ():  
> capabilities='3000' and did not match any protocals, not even none!
> 
> But that's probably because all the values of wlan0 in /proc/net/wireless  
> are 0, except for status, which is 1, maybe?

NM actually doesn't care about /proc/net/wireless at all.  But NM _does_
care that HAL can detect the device, and that the SIOCGIWRANGE handler
does actually provide correct and mostly complete information.

Dan

> --Eion
> 
> On Fri, 14 Jul 2006 08:36:29 +1200, Dan Williams <dcbw redhat com> wrote:
> 
> > On Fri, 2006-07-14 at 07:12 +1200, Eion Robb wrote:
> >> I've attached both the output of /proc/net/wireless (pretty much blank)
> >
> > Ok; this is because d80211 (at least as included in rt2x00 driver) isn't
> > setting the wireless stats handler, which I believe makes the device
> > show up in /proc/net/wireless.
> >
> > See airo, for example:
> >
> > static const struct iw_handler_def	airo_handler_def =
> > {
> > 	.num_standard	= sizeof(airo_handler)/sizeof(iw_handler),
> > 	.num_private	= sizeof(airo_private_handler)/sizeof(iw_handler),
> > 	.num_private_args = sizeof(airo_private_args)/sizeof(struct  
> > iw_priv_args),
> > 	.standard	= airo_handler,
> > 	.private	= airo_private_handler,
> > 	.private_args	= airo_private_args,
> > 	.get_wireless_stats = airo_get_wireless_stats,
> > };
> >
> > The ieee80211/ieee80211_ioctl.c file in the rt2x00 driver doesn't set
> > the ".get_wireless_stats" handler:
> >
> > const struct iw_handler_def ieee80211_iw_handler_def =
> > {
> > 	.num_standard	= sizeof(ieee80211_handler) / sizeof(iw_handler),
> > 	.num_private	= sizeof(ieee80211_private_handler) /
> > 			  sizeof(iw_handler),
> > 	.num_private_args = sizeof(ieee80211_ioctl_priv) /
> > 			    sizeof(struct iw_priv_args),
> > 	.standard	= (iw_handler *) ieee80211_handler,
> > 	.private	= (iw_handler *) ieee80211_private_handler,
> > 	.private_args	= (struct iw_priv_args *) ieee80211_ioctl_priv,
> > };
> >
> > Jiri, what's involved here?  The actual handler for get_wireless_stats
> > is quite simple; see below.  It returns a 'struct iw_statistics' for
> > which it looks like all the information is available in d80211
> > elsewhere.
> >
> > static struct iw_statistics *airo_get_wireless_stats(struct net_device  
> > *dev)
> > {
> > 	struct airo_info *local =  dev->priv;
> >
> > 	...
> > 	airo_read_wireless_stats(local);
> > 	...
> >
> > 	return &local->wstats;
> > }
> >
> > Dan
> >
> >> and another attempt at  the "tree /sys" output.
> >>
> >> --Eion
> >>
> >> >
> >> > AFAIK, HAL looks at /proc/net/wireless, parses the interface names,  
> >> and
> >> > uses that to determine whether or not the device is a wireless device.
> >> > Since it appears that /proc/net/wireless is eventually going away, HAL
> >> > may need to get changed to recognize the /sys/class/ieee80211  
> >> directory
> >> > that dscape presents, or something like that.
> >> >
> >> > What does your /proc/net/wireless say when the card is plugged in and
> >> > the driver is loaded?
> >> >
> >> > Dan
> >
> 
>