Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

[Johannes Berg]

On Wed, 2016-07-13 at 10:11 +, Machani, Yaniv wrote:
> On Wed, Jun 29, 2016 at 10:14:19, Johannes Berg wrote:
> > Cc: Hahn, Maital
> > Subject: Re: [PATCH 1/4] mac80211: mesh: flush stations before
> > beacons 
> > are stopped
> > 
> > On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
> > > From: Maital Hahn <mait...@ti.com>
> > > 
> > > Some drivers (e.g. wl18xx) expect that the last stage in the 
> > > de-initialization process will be stopping the beacons, similar
> > > to ap.
> > > Update ieee80211_stop_mesh() flow accordingly.
> > > 
> > How well have you tested that with other drivers?
> > 
> 
> Sorry for the delayed response (I've been out) and thanks for your
> comments,
> I have tested it with RT3572 as well, and didn't see any issue.
> I'll update the comment to reflect that.
> 

I'm actually reasonably sure that it *must* work, similiar to the AP
mode change, since it's always valid to remove stations while the the
mesh beacon is still active.

I hoped you'd actually figure out that line of reasoning and put it
into the commit message :)

johannes

RE: [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

[Machani, Yaniv]

On Wed, Jul 13, 2016 at 16:33:38, Bob Copeland wrote:
> linux- wirel...@vger.kernel.org; netdev@vger.kernel.org; Hahn, Maital
> Subject: Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons 
> are stopped
> 
> On Wed, Jul 13, 2016 at 10:11:25AM +, Machani, Yaniv wrote:
> > > > Some drivers (e.g. wl18xx) expect that the last stage in the 
> > > > de-initialization process will be stopping the beacons, similar to ap.
> > > > Update ieee80211_stop_mesh() flow accordingly.
> > > >
> > > How well have you tested that with other drivers?
> > >
> >
> > Sorry for the delayed response (I've been out) and thanks for your 
> > comments, I have tested it with RT3572 as well, and didn't see any issue.
> > I'll update the comment to reflect that.
> 
> I'll give this a test on ath10k and wcn36xx as they are the ones most 
> likely to care about ordering.
> 

Thank you,
Yaniv
> --
> Bob Copeland %% http://bobcopeland.com/

Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

[Bob Copeland]

On Wed, Jul 13, 2016 at 10:11:25AM +, Machani, Yaniv wrote:
> > > Some drivers (e.g. wl18xx) expect that the last stage in the 
> > > de-initialization process will be stopping the beacons, similar to ap.
> > > Update ieee80211_stop_mesh() flow accordingly.
> > >
> > How well have you tested that with other drivers?
> > 
> 
> Sorry for the delayed response (I've been out) and thanks for your comments,
> I have tested it with RT3572 as well, and didn't see any issue.
> I'll update the comment to reflect that.

I'll give this a test on ath10k and wcn36xx as they are the ones most
likely to care about ordering.

-- 
Bob Copeland %% http://bobcopeland.com/

RE: [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

[Machani, Yaniv]

On Wed, Jun 29, 2016 at 10:14:19, Johannes Berg wrote:
> Cc: Hahn, Maital
> Subject: Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons 
> are stopped
> 
> On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
> > From: Maital Hahn <mait...@ti.com>
> >
> > Some drivers (e.g. wl18xx) expect that the last stage in the 
> > de-initialization process will be stopping the beacons, similar to ap.
> > Update ieee80211_stop_mesh() flow accordingly.
> >
> How well have you tested that with other drivers?
> 

Sorry for the delayed response (I've been out) and thanks for your comments,
I have tested it with RT3572 as well, and didn't see any issue.
I'll update the comment to reflect that.

Thanks,
Yaniv

> Changing behaviour to something a single driver desires isn't 
> necessarily the best thing to do, since there always are multiple drivers.
> 
> If you're able to demonstrate that it works with the other drivers I'm 
> willing to take that - the change makes sense after all, and it seems 
> drivers must support this ordering since peers are also removed 
> dynamically... But still. Don't just make a change like that without 
> even giving any indication why you think it's fine for other drivers!
> 
> johannes

Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

[Johannes Berg]

On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
> From: Maital Hahn 
> 
> Some drivers (e.g. wl18xx) expect that the last stage in the
> de-initialization process will be stopping the beacons, similar to
> ap. Update ieee80211_stop_mesh() flow accordingly.
> 
How well have you tested that with other drivers?

Changing behaviour to something a single driver desires isn't
necessarily the best thing to do, since there always are multiple
drivers.

If you're able to demonstrate that it works with the other drivers I'm
willing to take that - the change makes sense after all, and it seems
drivers must support this ordering since peers are also removed
dynamically... But still. Don't just make a change like that without
even giving any indication why you think it's fine for other drivers!

johannes