| Message ID | 20160628111307.8784-2-yanivma@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote: > From: Maital Hahn <maitalm@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? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gV2VkLCBKdW4gMjksIDIwMTYgYXQgMTA6MTQ6MTksIEpvaGFubmVzIEJlcmcgd3JvdGU6DQo+ IENjOiBIYWhuLCBNYWl0YWwNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAxLzRdIG1hYzgwMjExOiBt ZXNoOiBmbHVzaCBzdGF0aW9ucyBiZWZvcmUgYmVhY29ucyANCj4gYXJlIHN0b3BwZWQNCj4gDQo+ IE9uIFR1ZSwgMjAxNi0wNi0yOCBhdCAxNDoxMyArMDMwMCwgWWFuaXYgTWFjaGFuaSB3cm90ZToN Cj4gPiBGcm9tOiBNYWl0YWwgSGFobiA8bWFpdGFsbUB0aS5jb20+DQo+ID4NCj4gPiBTb21lIGRy aXZlcnMgKGUuZy4gd2wxOHh4KSBleHBlY3QgdGhhdCB0aGUgbGFzdCBzdGFnZSBpbiB0aGUgDQo+ ID4gZGUtaW5pdGlhbGl6YXRpb24gcHJvY2VzcyB3aWxsIGJlIHN0b3BwaW5nIHRoZSBiZWFjb25z LCBzaW1pbGFyIHRvIGFwLg0KPiA+IFVwZGF0ZSBpZWVlODAyMTFfc3RvcF9tZXNoKCkgZmxvdyBh Y2NvcmRpbmdseS4NCj4gPg0KPiBIb3cgd2VsbCBoYXZlIHlvdSB0ZXN0ZWQgdGhhdCB3aXRoIG90 aGVyIGRyaXZlcnM/DQo+IA0KDQpTb3JyeSBmb3IgdGhlIGRlbGF5ZWQgcmVzcG9uc2UgKEkndmUg YmVlbiBvdXQpIGFuZCB0aGFua3MgZm9yIHlvdXIgY29tbWVudHMsDQpJIGhhdmUgdGVzdGVkIGl0 IHdpdGggUlQzNTcyIGFzIHdlbGwsIGFuZCBkaWRuJ3Qgc2VlIGFueSBpc3N1ZS4NCkknbGwgdXBk YXRlIHRoZSBjb21tZW50IHRvIHJlZmxlY3QgdGhhdC4NCg0KVGhhbmtzLA0KWWFuaXYNCg0KPiBD aGFuZ2luZyBiZWhhdmlvdXIgdG8gc29tZXRoaW5nIGEgc2luZ2xlIGRyaXZlciBkZXNpcmVzIGlz bid0IA0KPiBuZWNlc3NhcmlseSB0aGUgYmVzdCB0aGluZyB0byBkbywgc2luY2UgdGhlcmUgYWx3 YXlzIGFyZSBtdWx0aXBsZSBkcml2ZXJzLg0KPiANCj4gSWYgeW91J3JlIGFibGUgdG8gZGVtb25z dHJhdGUgdGhhdCBpdCB3b3JrcyB3aXRoIHRoZSBvdGhlciBkcml2ZXJzIEknbSANCj4gd2lsbGlu ZyB0byB0YWtlIHRoYXQgLSB0aGUgY2hhbmdlIG1ha2VzIHNlbnNlIGFmdGVyIGFsbCwgYW5kIGl0 IHNlZW1zIA0KPiBkcml2ZXJzIG11c3Qgc3VwcG9ydCB0aGlzIG9yZGVyaW5nIHNpbmNlIHBlZXJz IGFyZSBhbHNvIHJlbW92ZWQgDQo+IGR5bmFtaWNhbGx5Li4uIEJ1dCBzdGlsbC4gRG9uJ3QganVz dCBtYWtlIGEgY2hhbmdlIGxpa2UgdGhhdCB3aXRob3V0IA0KPiBldmVuIGdpdmluZyBhbnkgaW5k aWNhdGlvbiB3aHkgeW91IHRoaW5rIGl0J3MgZmluZSBmb3Igb3RoZXIgZHJpdmVycyENCj4gDQo+ IGpvaGFubmVzDQoNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 13, 2016 at 10:11:25AM +0000, 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.
On Wed, Jul 13, 2016 at 16:33:38, Bob Copeland wrote: > linux- wireless@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 +0000, 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/ -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2016-07-13 at 10:11 +0000, 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 <maitalm@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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c index 21b1fdf..9214bc1 100644 --- a/net/mac80211/mesh.c +++ b/net/mac80211/mesh.c @@ -896,20 +896,22 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata) netif_carrier_off(sdata->dev); + /* flush STAs and mpaths on this iface */ + sta_info_flush(sdata); + mesh_path_flush_by_iface(sdata); + /* stop the beacon */ ifmsh->mesh_id_len = 0; sdata->vif.bss_conf.enable_beacon = false; clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state); ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED); + + /* remove beacon */ bcn = rcu_dereference_protected(ifmsh->beacon, lockdep_is_held(&sdata->wdev.mtx)); RCU_INIT_POINTER(ifmsh->beacon, NULL); kfree_rcu(bcn, rcu_head); - /* flush STAs and mpaths on this iface */ - sta_info_flush(sdata); - mesh_path_flush_by_iface(sdata); - /* free all potentially still buffered group-addressed frames */ local->total_ps_buffered -= skb_queue_len(&ifmsh->ps.bc_buf); skb_queue_purge(&ifmsh->ps.bc_buf);