On Wed, 2008-11-05 at 08:57 -0800, Luis R. Rodriguez wrote: > >> +/* > >> + * IEEE 802.11-2007 7.3.2.9 Country information element > >> + * > >> + * Minimum length is 8 octects, ie len must be evenly > >> + * divisible by 2 > >> + */ > > > > That seems odd, I thought there are triplets in there? That mean it > > should be 2+N*3, no? > > Forgot the environment 'I', 'O' or ' '. > > alpha+environment (3) + n * 3 But then it should be evenly divisible by three, not two, right? And 8 doesn't make sense as a minimum length either then. > >> + */ > >> +#define IEEE80211_COUNTRY_EXTENSION_ID 201 > > > > Should that be something like EXTENSION_ID_OFFSET? Clearly they must > > mean to subtract this? > > Sure but just to determine if it is a new one as they seem to come in > increasing. Oh ok, I thought it was ok to have multiple in the same IE or something. > > Ok this I don't understand. Why the checksum thing? Why pass the > > regdomain struct in? Can't cfg80211 do the work of parsing the IE and > > you just parse the IE in, possibly "debouncing" when it hasn't changed > > (or do the debouncing in cfg80211 too, for the overhead of an extra > > function call) > > It seems wrong to offload onto cfg80211 some IE parsing work even if > its related to regulatory. Are you sure you want that? Why not? My scanning patch for cfg80211 also offloaded all of the parsing to it. > So we want two things: > > 1. A very quick and efficient way to determine if we can ignore the IE > processing in case the IE has already been processed > 2. A quick way to determine if a new device can ignore IE processing > > The alpha2 on the registered interface solves the first > The checksum solves the second Ok, but why put anything of that outside cfg80211? > If we leave the checksumming outside we need the helpers to check the > checksum exported. Otherwise then definitely we can stuff it into > cfg80211 but that means for a second device you'll pass the IE pointer > and len as part of the regulatory_hint_ie() and it *can* fail for > several reasons (-EALREADY, -EINVAL, -ENOMEM). Except nobody will care about the return value, so you might as well leave it off, no? > Another thing we should consider here is the case of an Indoor AP and > an Outdoor AP. According to Felix this is very possible and he's dealt > with it. I doubt 11d is being used in those cases but regardless it > seems we should account for it and deal with it. In that case it seems > two different registered devices can be using one country but > different environments. I have kept this in mind but admit not > thoroughly. I think its good to see now what changes we'll need. Maybe > we can deal with this later on though. We may well need different indoor/outdoor settings at some point and a way to tell the kernel which device is out/indoors, but that seems to only affect TX power so I wouldn't worry about it too much for now. > >> case REGDOM_SET_BY_USER: > >> if (last_request->initiator == REGDOM_SET_BY_COUNTRY_IE) > >> return REG_INTERSECT; > >> + /* If the user knows better the user should set the regdom > >> + * to their country before the IE is picked up */ > >> + if (last_request->initiator == REGDOM_SET_BY_USER && > >> + last_request->intersect) > >> + return -EOPNOTSUPP; > > > > Can you use a more customary indentation of the second if line to just > > after the parenthesis? > > Like what > > if (foo || > <TAB><SPACE><SPACE>bar) > <TAB><TAB>bar; > > ? at least that's what I do, yeah, sorry if this seems too trivial, if you wouldn't have to respin the patch anyway I wouldn't even mention it. > > When I suggested to use a checksum I was thinking of an easy way to > > debounce. But a checksum is never perfect. Hence, the WARN_ON you added > > might trigger despite the debounce checksum. Or we might fail to follow > > changes. > > Let me know what you think about the alpha2 + BSSID thing above. I > guess it should also consider the environment (I, O, ' ') in case the > AP is reconfigured to be outdoor (without being moved). I don't think > we should care about supporting these cases on the fly but we should > ensure the change won't break things. Well I haven't checked in detail which things you care about in the IE now, but shouldn't you check if those changed? johannes
Attachment:
signature.asc
Description: This is a digitally signed message part