Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1442367 > unrolled thread

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

Started byYaniv Machani <yanivma@ti.com>
First post2016-07-13 13:50 +0200
Last post2016-07-19 15:20 +0200
Articles 5 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template Yaniv Machani <yanivma@ti.com> - 2016-07-13 13:50 +0200
    Re: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-07-13 14:40 +0200
    Re: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template Bob Copeland <me@bobcopeland.com> - 2016-07-18 15:40 +0200
      Re: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template Johannes Berg <johannes@sipsolutions.net> - 2016-07-18 21:00 +0200
        RE: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template "Machani, Yaniv" <yanivma@ti.com> - 2016-07-19 15:20 +0200

#1442367 — [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template

FromYaniv Machani <yanivma@ti.com>
Date2016-07-13 13:50 +0200
Subject[PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template
Message-ID<rUqPU-2ww-21@gated-at.bofh.it>
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 <meiravk@ti.com>
Signed-off-by: Yaniv Machani <yanivma@ti.com>
---
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

[toc] | [next] | [standalone]

#1442398

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2016-07-13 14:40 +0200
Message-ID<rUrCi-34n-1@gated-at.bofh.it>
In reply to#1442367
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 <meiravk@ti.com>
> Signed-off-by: Yaniv Machani <yanivma@ti.com>
> ---
> 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

[toc] | [prev] | [next] | [standalone]

#1445486

FromBob Copeland <me@bobcopeland.com>
Date2016-07-18 15:40 +0200
Message-ID<rWgW5-6NT-11@gated-at.bofh.it>
In reply to#1442367
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/

[toc] | [prev] | [next] | [standalone]

#1445730

FromJohannes Berg <johannes@sipsolutions.net>
Date2016-07-18 21:00 +0200
Message-ID<rWlVL-1wg-5@gated-at.bofh.it>
In reply to#1445486
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

[toc] | [prev] | [next] | [standalone]

#1446455

From"Machani, Yaniv" <yanivma@ti.com>
Date2016-07-19 15:20 +0200
Message-ID<rWD6i-4u7-11@gated-at.bofh.it>
In reply to#1445730
On Mon, Jul 18, 2016 at 21:52:22, Johannes Berg wrote:
> linux- wireless@vger.kernel.org; netdev@vger.kernel.org
> 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?
> 

It was done to overcome another mismatch with the defaults of the hostap configuration,
We'll have another look on it.

There is an IOP question here, how to handle a case where you have mixed capabilities of peers.
is it possible to dynamically change the channel bandwidth to allow new peers to join ?

Yaniv

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web