2016-07-13 11:42:27

by Yaniv Machani

[permalink] [raw]
Subject: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template

The HT capab info field inside the HT capab IE of the mesh beacon
is incorrect (in the case of 20MHz channel width).
To fix this driver will check configuration from cfg and
will build it accordingly.

Signed-off-by: Meirav Kama <[email protected]>
Signed-off-by: Yaniv Machani <[email protected]>
---
V3 - Updated comment
- Removed CFG changes, as they are not correct.

net/mac80211/mesh.c | 33 ++++++++++++++++++++++++++++++++-
net/mac80211/util.c | 3 ---
2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 9214bc1..275131d 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -422,6 +422,8 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data *sdata,
enum nl80211_band band = ieee80211_get_sdata_band(sdata);
struct ieee80211_supported_band *sband;
u8 *pos;
+ u16 cap;
+

sband = local->hw.wiphy->bands[band];
if (!sband->ht_cap.ht_supported ||
@@ -430,11 +432,40 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data *sdata,
sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_10)
return 0;

+ /* determine capability flags */
+ cap = sband->ht_cap.cap;
+
+ /* if channel width is 20MHz - configure HT capab accordingly*/
+ if (sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_20) {
+ cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+ cap &= ~IEEE80211_HT_CAP_DSSSCCK40;
+ }
+
+ /* set SM PS mode properly */
+ cap &= ~IEEE80211_HT_CAP_SM_PS;
+ switch (sdata->smps_mode) {
+ case IEEE80211_SMPS_AUTOMATIC:
+ case IEEE80211_SMPS_NUM_MODES:
+ WARN_ON(1);
+ case IEEE80211_SMPS_OFF:
+ cap |= WLAN_HT_CAP_SM_PS_DISABLED <<
+ IEEE80211_HT_CAP_SM_PS_SHIFT;
+ break;
+ case IEEE80211_SMPS_STATIC:
+ cap |= WLAN_HT_CAP_SM_PS_STATIC <<
+ IEEE80211_HT_CAP_SM_PS_SHIFT;
+ break;
+ case IEEE80211_SMPS_DYNAMIC:
+ cap |= WLAN_HT_CAP_SM_PS_DYNAMIC <<
+ IEEE80211_HT_CAP_SM_PS_SHIFT;
+ break;
+ }
+
if (skb_tailroom(skb) < 2 + sizeof(struct ieee80211_ht_cap))
return -ENOMEM;

pos = skb_put(skb, 2 + sizeof(struct ieee80211_ht_cap));
- ieee80211_ie_build_ht_cap(pos, &sband->ht_cap, sband->ht_cap.cap);
+ ieee80211_ie_build_ht_cap(pos, &sband->ht_cap, cap);

return 0;
}
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 42bf0b6..5375a82 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2349,10 +2349,7 @@ u8 *ieee80211_ie_build_ht_oper(u8 *pos, struct ieee80211_sta_ht_cap *ht_cap,
ht_oper->operation_mode = cpu_to_le16(prot_mode);
ht_oper->stbc_param = 0x0000;

- /* It seems that Basic MCS set and Supported MCS set
- are identical for the first 10 bytes */
memset(&ht_oper->basic_set, 0, 16);
- memcpy(&ht_oper->basic_set, &ht_cap->mcs, 10);

return pos + sizeof(struct ieee80211_ht_operation);
}
--
2.9.0



2016-07-19 13:17:37

by Yaniv Machani

[permalink] [raw]
Subject: RE: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template

T24gTW9uLCBKdWwgMTgsIDIwMTYgYXQgMjE6NTI6MjIsIEpvaGFubmVzIEJlcmcgd3JvdGU6DQo+
IGxpbnV4LSB3aXJlbGVzc0B2Z2VyLmtlcm5lbC5vcmc7IG5ldGRldkB2Z2VyLmtlcm5lbC5vcmcN
Cj4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzNdIG1hYzgwMjExOiBtZXNoOiBmaXhlZCBIVCBpZXMg
aW4gYmVhY29uIA0KPiB0ZW1wbGF0ZQ0KPiANCj4gT24gTW9uLCAyMDE2LTA3LTE4IGF0IDA5OjM4
IC0wNDAwLCBCb2IgQ29wZWxhbmQgd3JvdGU6DQo+ID4gT24gV2VkLCBKdWwgMTMsIDIwMTYgYXQg
MDI6NDU6NDBQTSArMDMwMCwgWWFuaXYgTWFjaGFuaSB3cm90ZToNCj4gPiA+IFRoZSBIVCBjYXBh
YiBpbmZvIGZpZWxkIGluc2lkZSB0aGUgSFQgY2FwYWIgSUUgb2YgdGhlIG1lc2ggYmVhY29uIA0K
PiA+ID4gaXMgaW5jb3JyZWN0IChpbiB0aGUgY2FzZSBvZiAyME1IeiBjaGFubmVsIHdpZHRoKS4N
Cj4gPiA+IFRvIGZpeCB0aGlzIGRyaXZlciB3aWxsIGNoZWNrIGNvbmZpZ3VyYXRpb24gZnJvbSBj
ZmcgYW5kIHdpbGwgDQo+ID4gPiBidWlsZCBpdCBhY2NvcmRpbmdseS4NCj4gPg0KPiA+ID4gK8Kg
wqDCoMKgLyogZGV0ZXJtaW5lIGNhcGFiaWxpdHkgZmxhZ3MgKi8NCj4gPiA+ICsJY2FwID0gc2Jh
bmQtPmh0X2NhcC5jYXA7DQo+ID4gPiArDQo+ID4gPiArwqDCoMKgwqAvKiBpZiBjaGFubmVsIHdp
ZHRoIGlzIDIwTUh6IC0gY29uZmlndXJlIEhUIGNhcGFiDQo+ID4gPiBhY2NvcmRpbmdseSovDQo+
ID4gPiArCWlmIChzZGF0YS0+dmlmLmJzc19jb25mLmNoYW5kZWYud2lkdGggPT0NCj4gPiA+IE5M
ODAyMTFfQ0hBTl9XSURUSF8yMCkgew0KPiA+ID4gKwkJY2FwICY9IH5JRUVFODAyMTFfSFRfQ0FQ
X1NVUF9XSURUSF8yMF80MDsNCj4gPiA+ICsJCWNhcCAmPSB+SUVFRTgwMjExX0hUX0NBUF9EU1NT
Q0NLNDA7DQo+ID4gPiArCX0NCj4gPg0KPiA+IElzIGl0IHJlcXVpcmVkIHRoYXQgSFQgY2FwYWJp
bGl0eSBtYXRjaCB0aGUgSFQgb3BlcmF0aW9uIGluIHRoaXMgY2FzZT8NCj4gPg0KPiANCj4gSXMg
dGhlcmUgZXZlciBhIGNhc2UgdGhhdCBIVCAqY2FwYWJpbGl0eSogc2hvdWxkIGJlIHJlc3RyaWN0
ZWQgDQo+IGFydGlmaWNpYWxseSBsaWtlIHRoYXQ/IEkgY2FuJ3QgcmVtZW1iZXIgYW55IGNhc2Vz
IC0gd2UgZG8gc29tZXRoaW5nIA0KPiBsaWtlIHRoYXQgdG8gd29yayBhcm91bmQgYnJva2VuIEFQ
cyBpbiBzb21lIGNhc2VzLCBidXQgaGVyZT8NCj4gDQoNCkl0IHdhcyBkb25lIHRvIG92ZXJjb21l
IGFub3RoZXIgbWlzbWF0Y2ggd2l0aCB0aGUgZGVmYXVsdHMgb2YgdGhlIGhvc3RhcCBjb25maWd1
cmF0aW9uLA0KV2UnbGwgaGF2ZSBhbm90aGVyIGxvb2sgb24gaXQuDQoNClRoZXJlIGlzIGFuIElP
UCBxdWVzdGlvbiBoZXJlLCBob3cgdG8gaGFuZGxlIGEgY2FzZSB3aGVyZSB5b3UgaGF2ZSBtaXhl
ZCBjYXBhYmlsaXRpZXMgb2YgcGVlcnMuDQppcyBpdCBwb3NzaWJsZSB0byBkeW5hbWljYWxseSBj
aGFuZ2UgdGhlIGNoYW5uZWwgYmFuZHdpZHRoIHRvIGFsbG93IG5ldyBwZWVycyB0byBqb2luID8N
Cg0KWWFuaXYNCg0K

2016-07-18 13:38:33

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template

On Wed, Jul 13, 2016 at 02:45:40PM +0300, Yaniv Machani wrote:
> The HT capab info field inside the HT capab IE of the mesh beacon
> is incorrect (in the case of 20MHz channel width).
> To fix this driver will check configuration from cfg and
> will build it accordingly.

> + /* determine capability flags */
> + cap = sband->ht_cap.cap;
> +
> + /* if channel width is 20MHz - configure HT capab accordingly*/
> + if (sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_20) {
> + cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
> + cap &= ~IEEE80211_HT_CAP_DSSSCCK40;
> + }

Is it required that HT capability match the HT operation in this case?

> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 42bf0b6..5375a82 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -2349,10 +2349,7 @@ u8 *ieee80211_ie_build_ht_oper(u8 *pos, struct ieee80211_sta_ht_cap *ht_cap,
> ht_oper->operation_mode = cpu_to_le16(prot_mode);
> ht_oper->stbc_param = 0x0000;
>
> - /* It seems that Basic MCS set and Supported MCS set
> - are identical for the first 10 bytes */
> memset(&ht_oper->basic_set, 0, 16);
> - memcpy(&ht_oper->basic_set, &ht_cap->mcs, 10);

This change doesn't look right (basic mcs set will be all zeroes) but
then, neither does the original code. Basic MCS set for a mesh STA should
be the mandatory MCSes according to 9.7.4.

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

2016-07-13 12:33:37

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template

Hello.

On 7/13/2016 2:45 PM, Yaniv Machani wrote:

> The HT capab info field inside the HT capab IE of the mesh beacon
> is incorrect (in the case of 20MHz channel width).
> To fix this driver will check configuration from cfg and
> will build it accordingly.
>
> Signed-off-by: Meirav Kama <[email protected]>
> Signed-off-by: Yaniv Machani <[email protected]>
> ---
> V3 - Updated comment
> - Removed CFG changes, as they are not correct.
>
> net/mac80211/mesh.c | 33 ++++++++++++++++++++++++++++++++-
> net/mac80211/util.c | 3 ---
> 2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
> index 9214bc1..275131d 100644
> --- a/net/mac80211/mesh.c
> +++ b/net/mac80211/mesh.c
> @@ -422,6 +422,8 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data *sdata,
> enum nl80211_band band = ieee80211_get_sdata_band(sdata);
> struct ieee80211_supported_band *sband;
> u8 *pos;
> + u16 cap;
> +

Why add more empty lines where you already one?

>
> sband = local->hw.wiphy->bands[band];
> if (!sband->ht_cap.ht_supported ||
> @@ -430,11 +432,40 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data *sdata,
> sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_10)
> return 0;
>
> + /* determine capability flags */

Please use tab, not spaces.

> + cap = sband->ht_cap.cap;
> +
> + /* if channel width is 20MHz - configure HT capab accordingly*/

Likewise.

> + if (sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_20) {
> + cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
> + cap &= ~IEEE80211_HT_CAP_DSSSCCK40;
> + }
> +
> + /* set SM PS mode properly */
> + cap &= ~IEEE80211_HT_CAP_SM_PS;
> + switch (sdata->smps_mode) {
> + case IEEE80211_SMPS_AUTOMATIC:
> + case IEEE80211_SMPS_NUM_MODES:
> + WARN_ON(1);

No *break*? You need a comment like /* FALL THRU */ then...

> + case IEEE80211_SMPS_OFF:
> + cap |= WLAN_HT_CAP_SM_PS_DISABLED <<
> + IEEE80211_HT_CAP_SM_PS_SHIFT;
> + break;
> + case IEEE80211_SMPS_STATIC:
> + cap |= WLAN_HT_CAP_SM_PS_STATIC <<
> + IEEE80211_HT_CAP_SM_PS_SHIFT;
> + break;
> + case IEEE80211_SMPS_DYNAMIC:
> + cap |= WLAN_HT_CAP_SM_PS_DYNAMIC <<
> + IEEE80211_HT_CAP_SM_PS_SHIFT;
> + break;
> + }
> +
> if (skb_tailroom(skb) < 2 + sizeof(struct ieee80211_ht_cap))
> return -ENOMEM;
>
> pos = skb_put(skb, 2 + sizeof(struct ieee80211_ht_cap));
> - ieee80211_ie_build_ht_cap(pos, &sband->ht_cap, sband->ht_cap.cap);
> + ieee80211_ie_build_ht_cap(pos, &sband->ht_cap, cap);
>
> return 0;
> }
[...]

MBR, Sergei


2016-07-18 18:52:28

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template

On Mon, 2016-07-18 at 09:38 -0400, Bob Copeland wrote:
> On Wed, Jul 13, 2016 at 02:45:40PM +0300, Yaniv Machani wrote:
> > The HT capab info field inside the HT capab IE of the mesh beacon
> > is incorrect (in the case of 20MHz channel width).
> > To fix this driver will check configuration from cfg and
> > will build it accordingly.
>
> > +    /* determine capability flags */
> > + cap = sband->ht_cap.cap;
> > +
> > +    /* if channel width is 20MHz - configure HT capab
> > accordingly*/
> > + if (sdata->vif.bss_conf.chandef.width ==
> > NL80211_CHAN_WIDTH_20) {
> > + cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
> > + cap &= ~IEEE80211_HT_CAP_DSSSCCK40;
> > + }
>
> Is it required that HT capability match the HT operation in this
> case?
>

Is there ever a case that HT *capability* should be restricted
artificially like that? I can't remember any cases - we do something
like that to work around broken APs in some cases, but here?

johannes