Re: Should we check netif_running in cfg80211_calculate_bi_data?

[Johannes Berg <johannes@sipsolutions.net>]

On Tue, 2018-10-23 at 13:22 -0700, Ben Greear wrote:
> On 10/23/2018 12:53 PM, Johannes Berg wrote:
> > On Tue, 2018-10-23 at 12:19 -0700, Ben Greear wrote:
> > > 
> > > Oct 23 12:11:05 ben-ota-2.candelatech.com kernel: Assigning beacon-in-gcd to: 240 from wdev: vap39
> > > Oct 23 12:11:05 ben-ota-2.candelatech.com kernel: beacon-int-diff, beacon-int-gcd: 240  new-beacon-int: 100
> > 
> > This new-beacon-int 100 seems strange and suspicious. Why is it even
> > trying to look at this? Hmm.
> > 
> > > Maybe we need to clear beacon-interval back to 0 on admin down of the wifi dev?
> > 
> > We should be doing this, we should end up in __cfg80211_stop_ap() and
> > that does clear it? Hmm... perhaps this _fails_ somehow, and we don't
> > clear it in the error path? I suppose we really should make that
> > unconditional because there's nothing we can do to recover from that
> > error ...
> 
> I am suspicious about this...  that is not the same memory location as wdev->beacon_interval,
> so maybe this is the thing that is not properly cleared?
> 
> if (sdata->vif.type == NL80211_IFTYPE_AP ||
> 	    sdata->vif.type == NL80211_IFTYPE_MESH_POINT) {
> 		/*
> 		 * always passing this is harmless, since it'll be the
> 		 * same value that cfg80211 finds if it finds the same
> 		 * interface ... and that's always allowed
> 		 */
> 		params.new_beacon_int = sdata->vif.bss_conf.beacon_int;
> 	}

Oh, this is mac80211. Yes, that has a different place, and that does
indeed look like it doesn't get cleared? Odd. Perhaps you can try what
happens if you just clear that in ieee80211_stop_ap()?

johannes