2014-11-27 12:35:20

by Grumbach, Emmanuel

Subject: [RFC] mac80211: take reserved vif into account when calculating the min_def

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 <[email protected]>
---
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);
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;
}

--
1.9.1

2014-11-30 11:55:25

by Grumbach, Emmanuel

Subject: RE: [RFC] mac80211: take reserved vif into account when calculating the min_def

PiANCj4gT24gMjcgTm92ZW1iZXIgMjAxNCBhdCAxMzozMCwgRW1tYW51ZWwgR3J1bWJhY2gNCj4g
PGVtbWFudWVsLmdydW1iYWNoQGludGVsLmNvbT4gd3JvdGU6DQo+ID4gV2hlbiB3ZSB3YW50IHRv
IGNhbGN1bGF0ZSB0aGUgbWluaW1hbCBiYW5kd2lkdGggbmVlZGVkIGZvcg0KPiA+IGEgY2hhbm5l
bCBjb250ZXh0LCB3ZSBuZWVkIHRvIHRha2UgaW50byBhY2NvdW50IHZpZnMgdGhhdA0KPiA+IGhh
dmUgcmVzZXJ2ZWQgdGhlIGNoYW5uZWwgY29udGV4dC4NCj4gPiBJIGhpdCBhbiBpc3N1ZSB3aXRo
IGl3bHdpZmkgYW5kIGNoYW5uZWwgc3dpdGNoIGFzIGEgY2xpZW50Lg0KPiA+DQo+ID4gV2Ugd291
bGQgYWxsb2NhdGUgYSB2aXJnaW4gY2hhbm5lbCBjb250ZXh0IGFuZCByZXNlcnZlIGl0Lg0KPiA+
IEF0IHRoYXQgc3RhZ2UsIHRoZSBtaW5fZGVmIHdhcyAyME1Iei4NCj4gPiBUaGVuIHdlIHdvdWxk
IHVzZSBpdCBhZnRlciBDU0EsIGFuZCBzdGFydCB0cmFuc21pdHRpbmcsIGJ1dA0KPiA+IHRoZSBj
aGFubmVsIGNvbnRleHQgd2FzIHN0aWxsIDIwTUh6IGV2ZW4gaWYgdGhlIEdPIHdhcyBpbg0KPiA+
IDQwTUh6LiBUaGlzIG1hZGUgdGhlIGZpcm13YXJlIHVuaGFwcHkuDQo+ID4NCj4gPiBTaWduZWQt
b2ZmLWJ5OiBFbW1hbnVlbCBHcnVtYmFjaCA8ZW1tYW51ZWwuZ3J1bWJhY2hAaW50ZWwuY29tPg0K
PiA+IC0tLQ0KPiA+ICBuZXQvbWFjODAyMTEvY2hhbi5jIHwgNiArKysrKy0NCj4gPiAgMSBmaWxl
IGNoYW5nZWQsIDUgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPiA+DQo+ID4gZGlmZiAt
LWdpdCBhL25ldC9tYWM4MDIxMS9jaGFuLmMgYi9uZXQvbWFjODAyMTEvY2hhbi5jDQo+ID4gaW5k
ZXggNGM3NGU4ZC4uNzY5ZTBjNSAxMDA2NDQNCj4gPiAtLS0gYS9uZXQvbWFjODAyMTEvY2hhbi5j
DQo+ID4gKysrIGIvbmV0L21hYzgwMjExL2NoYW4uYw0KPiA+IEBAIC0yNTYsNyArMjU2LDggQEAg
aWVlZTgwMjExX2dldF9jaGFuY3R4X21heF9yZXF1aXJlZF9idyhzdHJ1Y3QNCj4gaWVlZTgwMjEx
X2xvY2FsICpsb2NhbCwNCj4gPiAgICAgICAgICAgICAgICAgaWYgKCFpZWVlODAyMTFfc2RhdGFf
cnVubmluZyhzZGF0YSkpDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgY29udGludWU7DQo+
ID4NCj4gPiAtICAgICAgICAgICAgICAgaWYgKHJjdV9hY2Nlc3NfcG9pbnRlcihzZGF0YS0+dmlm
LmNoYW5jdHhfY29uZikgIT0gY29uZikNCj4gPiArICAgICAgICAgICAgICAgaWYgKHJjdV9hY2Nl
c3NfcG9pbnRlcihzZGF0YS0+dmlmLmNoYW5jdHhfY29uZikgIT0gY29uZiAmJg0KPiA+ICsgICAg
ICAgICAgICAgICAgICAgJnNkYXRhLT5yZXNlcnZlZF9jaGFuY3R4LT5jb25mICE9IGNvbmYpDQo+
ID4gICAgICAgICAgICAgICAgICAgICAgICAgY29udGludWU7DQo+ID4NCj4gPiAgICAgICAgICAg
ICAgICAgc3dpdGNoICh2aWYtPnR5cGUpIHsNCj4gPiBAQCAtMjcxLDYgKzI3Miw3IEBAIGllZWU4
MDIxMV9nZXRfY2hhbmN0eF9tYXhfcmVxdWlyZWRfYncoc3RydWN0DQo+IGllZWU4MDIxMV9sb2Nh
bCAqbG9jYWwsDQo+ID4gICAgICAgICAgICAgICAgIGNhc2UgTkw4MDIxMV9JRlRZUEVfV0RTOg0K
PiA+ICAgICAgICAgICAgICAgICBjYXNlIE5MODAyMTFfSUZUWVBFX01FU0hfUE9JTlQ6DQo+ID4g
ICAgICAgICAgICAgICAgICAgICAgICAgd2lkdGggPSB2aWYtPmJzc19jb25mLmNoYW5kZWYud2lk
dGg7DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgd2lkdGggPSBtYXgod2lkdGgsIHNkYXRh
LT5yZXNlcnZlZF9jaGFuZGVmLndpZHRoKTsNCj4gDQo+IE5vdCByZWFsbHkgc3VyZSB3aHkgdGhp
cyBpcyBuZWVkZWQgaW4gdGhpcyBwYXRjaD8NCj4gDQoNCkhtbS4uLiBZb3UgYXJlIHJpZ2h0IC0g
SSB0aGluayBJIGdvdCBjb25mdXNlZCBoZXJlIDopDQpJIGd1ZXNzIEkgbmVlZCB0byB2ZXJpZnkg
dGhhdCByZW1vdmluZyB0aGlzIGh1bmsgc3RpbGwgc29sdmVzIG15IGJ1Zy4NCkluIGdlbmVyYWwg
dGhvdWdoLCB3aGF0IG9wdGlvbiB3b3VsZCB5b3UgcHJlZmVyPw0KDQpJbnRlcm5hbGx5LCB3ZSBo
YWQgZGlmZmVyZW50IG9waW5pb25zIGhlcmUuDQoNCj4gDQo+ID4gICAgICAgICAgICAgICAgICAg
ICAgICAgYnJlYWs7DQo+ID4gICAgICAgICAgICAgICAgIGNhc2UgTkw4MDIxMV9JRlRZUEVfVU5T
UEVDSUZJRUQ6DQo+ID4gICAgICAgICAgICAgICAgIGNhc2UgTlVNX05MODAyMTFfSUZUWVBFUzoN
Cj4gPiBAQCAtODk5LDYgKzkwMSw4IEBAIGludCBpZWVlODAyMTFfdmlmX3Jlc2VydmVfY2hhbmN0
eChzdHJ1Y3QNCj4gaWVlZTgwMjExX3N1Yl9pZl9kYXRhICpzZGF0YSwNCj4gPiAgICAgICAgIHNk
YXRhLT5yZXNlcnZlZF9yYWRhcl9yZXF1aXJlZCA9IHJhZGFyX3JlcXVpcmVkOw0KPiA+ICAgICAg
ICAgc2RhdGEtPnJlc2VydmVkX3JlYWR5ID0gZmFsc2U7DQo+ID4NCj4gPiArICAgICAgIGllZWU4
MDIxMV9yZWNhbGNfY2hhbmN0eF9taW5fZGVmKGxvY2FsLCBuZXdfY3R4KTsNCj4gPiArDQo+IA0K
PiBIbW0uLiBXb3VsZG4ndCBpdCBtYWtlIHNlbnNlIHRvIHJlY2FsYyB0aGlzIGluDQo+IGllZWU4
MDIxMV92aWZfdW5yZXNlcnZlXyBjaGFuY3R4KCkgYXMgd2VsbD8NCg0KUHJvYmFibHkgLSBJIG5l
ZWQgdG8gYWRkIHRoaXMuDQoNCj4gDQo+IA0KPiBNaWNoYcWCDQo=

2014-11-28 06:54:57

by Michal Kazior

Subject: Re: [RFC] mac80211: take reserved vif into account when calculating the min_def

On 27 November 2014 at 13:30, Emmanuel Grumbach
<[email protected]> 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 <[email protected]>
> ---
> 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ł