On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@xxxxxxxxxx> wrote: > When the regulatory settings change, some channels might become invalid. > Disconnect interfaces acting on these channels, after giving userspace > code a grace period to leave them. > > Because of cfg80211 channel tracking limitations, only disconnect > STA/P2P_CL interfaces if their current center channel became disabled. > > Change-Id: I308cc02c06a11cfc30b206efaeabe64d67186228 > Signed-off-by: Arik Nemtsov <arikx.nemtsov@xxxxxxxxx> > Reviewed-on: https://gerrit.rds.intel.com/32422 > Tested-by: IWL Jenkins > Reviewed-by: Johannes Berg <johannes.berg@xxxxxxxxx> I welcome this change, and thanks for the work! But I think we need to think about the cases where this really would be required. This should not be an issue drivers using the public wireless-regd completely given that the default world regulatory domain is a mathematical intersection, and as such its restrictive by nature and only world-allowed channels with the the minimum max EIRP would be allowed. For drivers using an initial custom world regdomain with custom apply API a change in regulatory would be permissive and as such we can lift some restrictions when we associate to an AP with a country IE specified. For the dynamic firmware juju that Intel wants to introduce this race is intrinsically unavoidable although the likelihood of it remains unclear. It should then be possible for some drivers to not have to use this sanity check. For example drivers that use ath properly (ath5k, ath9k, ath10k) could likely skip this stuff, unless I'm missing something. For drivers without regulatory data, that should also be the case as the public regdb would be used. It'd be interesting to see if that' not the case though so an informative print for now would be useful for the case where bailing out does happen, in fact perhaps it makes sense to issue a multicast nl80211 event for this so that userspace knows something has happened and why. Since we already have a reg change multicast event this seems then to provide a "safe" proactive measure for the loose cases, as perhaps an API can be added to let userspace acknowledge a regulatory change event -- that way upon receipt we should be able to cancel this work. Thoughts?v > --- > net/wireless/reg.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 83 insertions(+), 1 deletion(-) > > diff --git a/net/wireless/reg.c b/net/wireless/reg.c > index d6b83f9..1d6afe6 100644 > --- a/net/wireless/reg.c > +++ b/net/wireless/reg.c > @@ -221,6 +221,9 @@ struct reg_beacon { > struct ieee80211_channel chan; > }; > > +static void reg_check_chans_work(struct work_struct *work); > +static DECLARE_DELAYED_WORK(reg_check_chans, reg_check_chans_work); > + > static void reg_todo(struct work_struct *work); > static DECLARE_WORK(reg_work, reg_todo); > > @@ -1520,6 +1523,80 @@ static void reg_call_notifier(struct wiphy *wiphy, > wiphy->reg_notifier(wiphy, request); > } > > +static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev) > +{ > + struct ieee80211_channel *ch; > + bool ret = true; > + > + wdev_lock(wdev); > + > + if (!wdev->netdev || !netif_running(wdev->netdev)) > + goto out; > + > + switch (wdev->iftype) { > + case NL80211_IFTYPE_AP: > + case NL80211_IFTYPE_P2P_GO: > + if (!wdev->beacon_interval) > + goto out; > + > + ret = cfg80211_reg_can_beacon(wiphy, > + &wdev->chandef, wdev->iftype); > + break; > + case NL80211_IFTYPE_STATION: > + case NL80211_IFTYPE_P2P_CLIENT: > + if (!wdev->current_bss || > + !wdev->current_bss->pub.channel) > + goto out; > + > + /* TODO: more elaborate tracking for wide channels */ > + ch = wdev->current_bss->pub.channel; > + ret = !(ch->flags & IEEE80211_CHAN_DISABLED); That does not seem enough, the max EIRP could have changed too for example -- but the reg_notifier() should be used for ensuring changes to power requirements are respected, at least the Atheros shared ath module does this, that after all is one of the reasons for the reg_notifier(). Documenting as part of this change would be appreciated. > + break; > + default: > + /* others not implemented for now */ > + break; > + } > + > +out: > + wdev_unlock(wdev); > + return ret; > +} > + > +static void reg_leave_invalid_chans(struct wiphy *wiphy) > +{ > + struct wireless_dev *wdev; > + struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy); > + > + ASSERT_RTNL(); > + > + list_for_each_entry(wdev, &rdev->wdev_list, list) > + if (!reg_wdev_chan_valid(wiphy, wdev)) > + cfg80211_leave(rdev, wdev); An informative print or multicast event and reason would be nice here. 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
