On Sun, 2012-10-21 at 10:53 +0200, Eliad Peller wrote:
> > This would make the code in iwlwifi a lot simpler
> > as having bss_info_change do everything depending
> > on whether the beacon was enabled/disabled etc.
> > is tricky to get right, the explicit operations
> > make it easier.
> >
> is it really different from acting upon BSS_CHANGED_BEACON_ENABLED
> (when vif == ap)?
We did implement it this way but the error handling is a bit of a deal
breaker, it's just not good to have to ignore errors...
> > ieee80211_bss_info_change_notify(sdata, changed);
> >
> > + err = drv_start_ap(sdata->local, sdata);
> > + if (err) {
> > + old = rtnl_dereference(sdata->u.ap.beacon);
> > + if (old)
> > + kfree_rcu(old, rcu_head);
> > + RCU_INIT_POINTER(sdata->u.ap.beacon, NULL);
> > + return err;
>
> they'll probably be mutually exclusive, but i think you should still
> notify BSS_CHANGED_BEACON_ENABLED after removing the beacon (or more
> easily, call ieee80211_bss_info_change_notify only after this block).
Good point.
> > +DEFINE_EVENT(local_sdata_evt, drv_start_ap,
> > + TP_PROTO(struct ieee80211_local *local,
> > + struct ieee80211_sub_if_data *sdata),
> > + TP_ARGS(local, sdata)
> > +);
>
> adding traces for ssid and beacon_int might be nice :)
Yeah, quick & dirty for now.
> what about other direct notification of BSS_CHANGED_BEACON_ENABLED
> (like in offchannel and ieee80211_do_stop)? should we call
> start_ap/stop_ap there as well?
The one in do_stop() should probably just be cleaned up, cfg80211 will
now stop the AP operation while the interface goes down ... so that
won't actually do anything, but could be a concern for the old API
because it might cause an off/off transition.
For offchannel, I'm not sure. Does it really make sense to call it
there? I can do it, I suppose, but ... hmm. The software-offchannel
hopefully won't actually be needed much longer given channel
contexts ;-)
johannes
--
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
