| Message ID | 1417091446-16308-1-git-send-email-emmanuel.grumbach@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 27 November 2014 at 13:30, Emmanuel Grumbach <emmanuel.grumbach@intel.com> wrote: > When we want to calculate the minimal bandwidth needed for > a channel context, we need to take into account vifs that > have reserved the channel context. > I hit an issue with iwlwifi and channel switch as a client. > > We would allocate a virgin channel context and reserve it. > At that stage, the min_def was 20MHz. > Then we would use it after CSA, and start transmitting, but > the channel context was still 20MHz even if the GO was in > 40MHz. This made the firmware unhappy. > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> > --- > net/mac80211/chan.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c > index 4c74e8d..769e0c5 100644 > --- a/net/mac80211/chan.c > +++ b/net/mac80211/chan.c > @@ -256,7 +256,8 @@ ieee80211_get_chanctx_max_required_bw(struct ieee80211_local *local, > if (!ieee80211_sdata_running(sdata)) > continue; > > - if (rcu_access_pointer(sdata->vif.chanctx_conf) != conf) > + if (rcu_access_pointer(sdata->vif.chanctx_conf) != conf && > + &sdata->reserved_chanctx->conf != conf) > continue; > > switch (vif->type) { > @@ -271,6 +272,7 @@ ieee80211_get_chanctx_max_required_bw(struct ieee80211_local *local, > case NL80211_IFTYPE_WDS: > case NL80211_IFTYPE_MESH_POINT: > width = vif->bss_conf.chandef.width; > + width = max(width, sdata->reserved_chandef.width); Not really sure why this is needed in this patch? > break; > case NL80211_IFTYPE_UNSPECIFIED: > case NUM_NL80211_IFTYPES: > @@ -899,6 +901,8 @@ int ieee80211_vif_reserve_chanctx(struct ieee80211_sub_if_data *sdata, > sdata->reserved_radar_required = radar_required; > sdata->reserved_ready = false; > > + ieee80211_recalc_chanctx_min_def(local, new_ctx); > + Hmm.. Wouldn't it make sense to recalc this in ieee80211_vif_unreserve_ chanctx() as well? Micha? -- 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 27 November 2014 at 13:30, Emmanuel Grumbach > <emmanuel.grumbach@intel.com> wrote: > > When we want to calculate the minimal bandwidth needed for > > a channel context, we need to take into account vifs that > > have reserved the channel context. > > I hit an issue with iwlwifi and channel switch as a client. > > > > We would allocate a virgin channel context and reserve it. > > At that stage, the min_def was 20MHz. > > Then we would use it after CSA, and start transmitting, but > > the channel context was still 20MHz even if the GO was in > > 40MHz. This made the firmware unhappy. > > > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> > > --- > > net/mac80211/chan.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c > > index 4c74e8d..769e0c5 100644 > > --- a/net/mac80211/chan.c > > +++ b/net/mac80211/chan.c > > @@ -256,7 +256,8 @@ ieee80211_get_chanctx_max_required_bw(struct > ieee80211_local *local, > > if (!ieee80211_sdata_running(sdata)) > > continue; > > > > - if (rcu_access_pointer(sdata->vif.chanctx_conf) != conf) > > + if (rcu_access_pointer(sdata->vif.chanctx_conf) != conf && > > + &sdata->reserved_chanctx->conf != conf) > > continue; > > > > switch (vif->type) { > > @@ -271,6 +272,7 @@ ieee80211_get_chanctx_max_required_bw(struct > ieee80211_local *local, > > case NL80211_IFTYPE_WDS: > > case NL80211_IFTYPE_MESH_POINT: > > width = vif->bss_conf.chandef.width; > > + width = max(width, sdata->reserved_chandef.width); > > Not really sure why this is needed in this patch? > Hmm... You are right - I think I got confused here :) I guess I need to verify that removing this hunk still solves my bug. In general though, what option would you prefer? Internally, we had different opinions here. > > > break; > > case NL80211_IFTYPE_UNSPECIFIED: > > case NUM_NL80211_IFTYPES: > > @@ -899,6 +901,8 @@ int ieee80211_vif_reserve_chanctx(struct > ieee80211_sub_if_data *sdata, > > sdata->reserved_radar_required = radar_required; > > sdata->reserved_ready = false; > > > > + ieee80211_recalc_chanctx_min_def(local, new_ctx); > > + > > Hmm.. Wouldn't it make sense to recalc this in > ieee80211_vif_unreserve_ chanctx() as well? Probably - I need to add this. > > > Micha?
PiA+DQo+ID4gT24gMjcgTm92ZW1iZXIgMjAxNCBhdCAxMzozMCwgRW1tYW51ZWwgR3J1bWJhY2gN Cj4gPiA8ZW1tYW51ZWwuZ3J1bWJhY2hAaW50ZWwuY29tPiB3cm90ZToNCj4gPiA+IFdoZW4gd2Ug d2FudCB0byBjYWxjdWxhdGUgdGhlIG1pbmltYWwgYmFuZHdpZHRoIG5lZWRlZCBmb3IgYSBjaGFu bmVsDQo+ID4gPiBjb250ZXh0LCB3ZSBuZWVkIHRvIHRha2UgaW50byBhY2NvdW50IHZpZnMgdGhh dCBoYXZlIHJlc2VydmVkIHRoZQ0KPiA+ID4gY2hhbm5lbCBjb250ZXh0Lg0KPiA+ID4gSSBoaXQg YW4gaXNzdWUgd2l0aCBpd2x3aWZpIGFuZCBjaGFubmVsIHN3aXRjaCBhcyBhIGNsaWVudC4NCj4g PiA+DQo+ID4gPiBXZSB3b3VsZCBhbGxvY2F0ZSBhIHZpcmdpbiBjaGFubmVsIGNvbnRleHQgYW5k IHJlc2VydmUgaXQuDQo+ID4gPiBBdCB0aGF0IHN0YWdlLCB0aGUgbWluX2RlZiB3YXMgMjBNSHou DQo+ID4gPiBUaGVuIHdlIHdvdWxkIHVzZSBpdCBhZnRlciBDU0EsIGFuZCBzdGFydCB0cmFuc21p dHRpbmcsIGJ1dCB0aGUNCj4gPiA+IGNoYW5uZWwgY29udGV4dCB3YXMgc3RpbGwgMjBNSHogZXZl biBpZiB0aGUgR08gd2FzIGluIDQwTUh6LiBUaGlzDQo+ID4gPiBtYWRlIHRoZSBmaXJtd2FyZSB1 bmhhcHB5Lg0KPiA+ID4NCj4gPiA+IFNpZ25lZC1vZmYtYnk6IEVtbWFudWVsIEdydW1iYWNoIDxl bW1hbnVlbC5ncnVtYmFjaEBpbnRlbC5jb20+DQoNClNvIHdlIGRpc2N1c3NlZCBpdCBpbnRlcm5h bGx5IC0gYW5kIGl0IHNlZW1zIHRoYXQgdGhlIG90aGVyIHBhdGNoIGlzIGJldHRlciAoaHR0cHM6 Ly9wYXRjaHdvcmsua2VybmVsLm9yZy9wYXRjaC81Mzk2MzUxLykuDQpMZXQncyBkcm9wIHRoaXMg b25lLg0KDQo+ID4gPiAtLS0NCj4gPiA+ICBuZXQvbWFjODAyMTEvY2hhbi5jIHwgNiArKysrKy0N Cj4gPiA+ICAxIGZpbGUgY2hhbmdlZCwgNSBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pDQo+ ID4gPg0KPiA+ID4gZGlmZiAtLWdpdCBhL25ldC9tYWM4MDIxMS9jaGFuLmMgYi9uZXQvbWFjODAy MTEvY2hhbi5jIGluZGV4DQo+ID4gPiA0Yzc0ZThkLi43NjllMGM1IDEwMDY0NA0KPiA+ID4gLS0t IGEvbmV0L21hYzgwMjExL2NoYW4uYw0KPiA+ID4gKysrIGIvbmV0L21hYzgwMjExL2NoYW4uYw0K PiA+ID4gQEAgLTI1Niw3ICsyNTYsOCBAQCBpZWVlODAyMTFfZ2V0X2NoYW5jdHhfbWF4X3JlcXVp cmVkX2J3KHN0cnVjdA0KPiA+IGllZWU4MDIxMV9sb2NhbCAqbG9jYWwsDQo+ID4gPiAgICAgICAg ICAgICAgICAgaWYgKCFpZWVlODAyMTFfc2RhdGFfcnVubmluZyhzZGF0YSkpDQo+ID4gPiAgICAg ICAgICAgICAgICAgICAgICAgICBjb250aW51ZTsNCj4gPiA+DQo+ID4gPiAtICAgICAgICAgICAg ICAgaWYgKHJjdV9hY2Nlc3NfcG9pbnRlcihzZGF0YS0+dmlmLmNoYW5jdHhfY29uZikgIT0gY29u ZikNCj4gPiA+ICsgICAgICAgICAgICAgICBpZiAocmN1X2FjY2Vzc19wb2ludGVyKHNkYXRhLT52 aWYuY2hhbmN0eF9jb25mKSAhPSBjb25mICYmDQo+ID4gPiArICAgICAgICAgICAgICAgICAgICZz ZGF0YS0+cmVzZXJ2ZWRfY2hhbmN0eC0+Y29uZiAhPSBjb25mKQ0KPiA+ID4gICAgICAgICAgICAg ICAgICAgICAgICAgY29udGludWU7DQo+ID4gPg0KPiA+ID4gICAgICAgICAgICAgICAgIHN3aXRj aCAodmlmLT50eXBlKSB7IEBAIC0yNzEsNiArMjcyLDcgQEANCj4gPiA+IGllZWU4MDIxMV9nZXRf Y2hhbmN0eF9tYXhfcmVxdWlyZWRfYncoc3RydWN0DQo+ID4gaWVlZTgwMjExX2xvY2FsICpsb2Nh bCwNCj4gPiA+ICAgICAgICAgICAgICAgICBjYXNlIE5MODAyMTFfSUZUWVBFX1dEUzoNCj4gPiA+ ICAgICAgICAgICAgICAgICBjYXNlIE5MODAyMTFfSUZUWVBFX01FU0hfUE9JTlQ6DQo+ID4gPiAg ICAgICAgICAgICAgICAgICAgICAgICB3aWR0aCA9IHZpZi0+YnNzX2NvbmYuY2hhbmRlZi53aWR0 aDsNCj4gPiA+ICsgICAgICAgICAgICAgICAgICAgICAgIHdpZHRoID0gbWF4KHdpZHRoLA0KPiA+ ID4gKyBzZGF0YS0+cmVzZXJ2ZWRfY2hhbmRlZi53aWR0aCk7DQo+ID4NCj4gPiBOb3QgcmVhbGx5 IHN1cmUgd2h5IHRoaXMgaXMgbmVlZGVkIGluIHRoaXMgcGF0Y2g/DQo+ID4NCj4gDQo+IEhtbS4u LiBZb3UgYXJlIHJpZ2h0IC0gSSB0aGluayBJIGdvdCBjb25mdXNlZCBoZXJlIDopIEkgZ3Vlc3Mg SSBuZWVkIHRvIHZlcmlmeQ0KPiB0aGF0IHJlbW92aW5nIHRoaXMgaHVuayBzdGlsbCBzb2x2ZXMg bXkgYnVnLg0KPiBJbiBnZW5lcmFsIHRob3VnaCwgd2hhdCBvcHRpb24gd291bGQgeW91IHByZWZl cj8NCj4gDQo+IEludGVybmFsbHksIHdlIGhhZCBkaWZmZXJlbnQgb3BpbmlvbnMgaGVyZS4NCj4g DQo+ID4NCj4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIGJyZWFrOw0KPiA+ID4gICAgICAg ICAgICAgICAgIGNhc2UgTkw4MDIxMV9JRlRZUEVfVU5TUEVDSUZJRUQ6DQo+ID4gPiAgICAgICAg ICAgICAgICAgY2FzZSBOVU1fTkw4MDIxMV9JRlRZUEVTOg0KPiA+ID4gQEAgLTg5OSw2ICs5MDEs OCBAQCBpbnQgaWVlZTgwMjExX3ZpZl9yZXNlcnZlX2NoYW5jdHgoc3RydWN0DQo+ID4gaWVlZTgw MjExX3N1Yl9pZl9kYXRhICpzZGF0YSwNCj4gPiA+ICAgICAgICAgc2RhdGEtPnJlc2VydmVkX3Jh ZGFyX3JlcXVpcmVkID0gcmFkYXJfcmVxdWlyZWQ7DQo+ID4gPiAgICAgICAgIHNkYXRhLT5yZXNl cnZlZF9yZWFkeSA9IGZhbHNlOw0KPiA+ID4NCj4gPiA+ICsgICAgICAgaWVlZTgwMjExX3JlY2Fs Y19jaGFuY3R4X21pbl9kZWYobG9jYWwsIG5ld19jdHgpOw0KPiA+ID4gKw0KPiA+DQo+ID4gSG1t Li4gV291bGRuJ3QgaXQgbWFrZSBzZW5zZSB0byByZWNhbGMgdGhpcyBpbg0KPiA+IGllZWU4MDIx MV92aWZfdW5yZXNlcnZlXyBjaGFuY3R4KCkgYXMgd2VsbD8NCj4gDQo+IFByb2JhYmx5IC0gSSBu ZWVkIHRvIGFkZCB0aGlzLg0KPiANCj4gPg0KPiA+DQo+ID4gTWljaGHFgg0K -- 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/chan.c b/net/mac80211/chan.c index 4c74e8d..769e0c5 100644 --- a/net/mac80211/chan.c +++ b/net/mac80211/chan.c @@ -256,7 +256,8 @@ ieee80211_get_chanctx_max_required_bw(struct ieee80211_local *local, if (!ieee80211_sdata_running(sdata)) continue; - if (rcu_access_pointer(sdata->vif.chanctx_conf) != conf) + if (rcu_access_pointer(sdata->vif.chanctx_conf) != conf && + &sdata->reserved_chanctx->conf != conf) continue; switch (vif->type) { @@ -271,6 +272,7 @@ ieee80211_get_chanctx_max_required_bw(struct ieee80211_local *local, case NL80211_IFTYPE_WDS: case NL80211_IFTYPE_MESH_POINT: width = vif->bss_conf.chandef.width; + width = max(width, sdata->reserved_chandef.width); break; case NL80211_IFTYPE_UNSPECIFIED: case NUM_NL80211_IFTYPES: @@ -899,6 +901,8 @@ int ieee80211_vif_reserve_chanctx(struct ieee80211_sub_if_data *sdata, sdata->reserved_radar_required = radar_required; sdata->reserved_ready = false; + ieee80211_recalc_chanctx_min_def(local, new_ctx); + return 0; }
When we want to calculate the minimal bandwidth needed for a channel context, we need to take into account vifs that have reserved the channel context. I hit an issue with iwlwifi and channel switch as a client. We would allocate a virgin channel context and reserve it. At that stage, the min_def was 20MHz. Then we would use it after CSA, and start transmitting, but the channel context was still 20MHz even if the GO was in 40MHz. This made the firmware unhappy. Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> --- net/mac80211/chan.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)