On Fri, Jul 31, 2009 at 9:43 AM, Johannes Berg<johannes@xxxxxxxxxxxxxxxx> wrote: > On Fri, 2009-07-31 at 09:32 -0700, Luis R. Rodriguez wrote: > >> Its not that simple. Consider the fact we build our own custom >> regulatory domain and stuff what is intended for each one of them. The >> _orig stuff is set upon channel registration so it is correct that >> we'd have to avoid setting the channel flags prior to wiphy >> registration. How you do that is left up to implementation. Since >> cfg80211 will set the channel *_orig params based on >> wiphy_registration() you're only option is to either prevent your >> wiphy being registered with the flags set or to reset the channel >> flags on the reg_notifier(). The later seems like the way to go but we >> currently do not call the reg_notifier() upon beacon hints -- we >> currently only call the reg_notifier() upon regulatory domain changes >> and upon wiphy registration. My point is both of these options require >> considerable changes for 2.6.31. > > Aha. So the problem really is that we don't have a reg notifier on > updates due to beacons. Yeah, seems that's the core issue from the original implementation. We should have added that. >> > and set them in ath_reg_apply_beaconing_flags and >> > ath_reg_apply_active_scan_flags, changing the polarity? >> > >> > I mean, right now you tell cfg80211 you don't support it, >> > and then try >> > to support it anyhow. >> >> Not sure I follow, support what? The reg_notifier() is there for >> regulatory domain changes, we didn't call it upon beacon hints. We >> can, and I agree its the right approach, but not for 2.6.31. > > I'm thinking more in terms of iwlwifi here, for all intents and purposes > it doesn't support beaconing on channel N, so it only supports NO_IBSS > on that channel. Which is what it puts into flags before registration. Ah I see, well but remember also that devices may have custom world regulatory domains for which they can set the no-ibss/passive scan flags and then allow for it due to a beacon hint. >> > Instead, you could tell cfg80211 you _do_ support >> > it, and then not support them depending on the notifier? It seems like >> > that should work and not break cfg80211's assumption that you can never >> > ever support _more_ than registration flags (orig_flags). >> >> I'm not following at all, orig_flags do not tell cfg80211 the channel >> flags the device supports. Most devices set flag to 0 upon wiphy >> registration and therefore cfg80211 sets orig_flags to 0 as well >> during wiphy registration. > > Well ok, "supports" is a bad word since we're talking about > restrictions. But for iwlwifi the orig_flags _do_ tell cfg80211 the > channel restrictions that the device _requires_. Oh ok, got it, yeah agreed, but its important to note we never documented nor was I aware we wanted to treat flags set prior to wiphy registration as capabilities. Truth is the device is actually capable of active scanning and beaconing on those channels but the design of the firmware doesn't allow for it. > Your devices, however, do not _strictly_ _require_ those channel flags, > they just require them for compliance. Unlike iwlwifi which crashes when > you don't do it right :) > > So I think we're in agreement -- the proper solution would be to call > the reg notifier on beacon hints too. Yeah, unless we want to just document an exception to the rules, but I do agree that seems like a slippery slope. > What we do for .31 I can't say I > really care, OK, I do, please consider this patch for 2.6.31 and also on wireless-testing for now. > how much work do you think this would be? It doesn't seem > all _that_ bad, at least for core changes? After some thinking the reg_notifier() does not seem to be the appropriate place for this. The reg_notifier() was designed to account for regulatory domain changes, not enhancements such as beacon hints. Consider the regulatory_request structure passed as the second argument to reg_notifier() and the currently supported types of regulatory domain changes, %REGDOM_SET_BY_*. What seems cleaner is to add a beacon_hint() notifier and use that at the end of handle_reg_beacon() so that we already give the driver the channel on which a beacon was received. If we really want to stick to using the reg_notifier() on ath.ko we'd need to make some changes to reg_notifier API, maybe consider a new struct for the beacon hint event, and then have ath.ko iterate over all channels and when a channel has chan->beacon_found lift the flags.. A beacon_hint() callback seems cleaner and to the point. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
