diff mbox

[6/9] mac80211: handle width changes from opmode notification IE in beacon

Message ID 1449583479-26658-7-git-send-email-emmanuel.grumbach@intel.com
State Accepted
Delegated to: Johannes Berg
Headers show

Commit Message

Emmanuel Grumbach Dec. 8, 2015, 2:04 p.m. UTC
From: Eyal Shapira <eyal@wizery.com>

An AP can send an operating channel width change in a beacon
opmode notification IE as long as there's a change in the nss as
well (See 802.11ac-2013 section 10.41).
So don't limit updating to nss only from an opmode notification IE.

Signed-off-by: Eyal Shapira <eyalx.shapira@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/mac80211/cfg.c         |  3 +--
 net/mac80211/ieee80211_i.h |  4 ++--
 net/mac80211/mlme.c        |  2 +-
 net/mac80211/rx.c          |  3 +--
 net/mac80211/vht.c         | 10 +++-------
 5 files changed, 8 insertions(+), 14 deletions(-)

Comments

Krishna Chaitanya Dec. 8, 2015, 3:38 p.m. UTC | #1
On Tue, Dec 8, 2015 at 7:34 PM, Emmanuel Grumbach
<emmanuel.grumbach@intel.com> wrote:
>
> From: Eyal Shapira <eyal@wizery.com>
>
> An AP can send an operating channel width change in a beacon
> opmode notification IE as long as there's a change in the nss as
> well (See 802.11ac-2013 section 10.41).
> So don't limit updating to nss only from an opmode notification IE.
>
> Signed-off-by: Eyal Shapira <eyalx.shapira@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> ---
>  net/mac80211/cfg.c         |  3 +--
>  net/mac80211/ieee80211_i.h |  4 ++--
>  net/mac80211/mlme.c        |  2 +-
>  net/mac80211/rx.c          |  3 +--
>  net/mac80211/vht.c         | 10 +++-------
>  5 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index b9e2c2f..a90d875 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1198,8 +1198,7 @@ static int sta_apply_parameters(struct ieee80211_local *local,
>                  * rc isn't initialized here yet, so ignore it
>                  */
>                 __ieee80211_vht_handle_opmode(sdata, sta,
> -                                             params->opmode_notif,
> -                                             band, false);
> +                                             params->opmode_notif, band);
>         }
>
>         if (ieee80211_vif_is_mesh(&sdata->vif))
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index 582ea86..747402d 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1718,10 +1718,10 @@ void ieee80211_process_mu_groups(struct ieee80211_sub_if_data *sdata,
>                                  struct ieee80211_mgmt *mgmt);
>  u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
>                                    struct sta_info *sta, u8 opmode,
> -                                  enum ieee80211_band band, bool nss_only);
> +                                 enum ieee80211_band band);
>  void ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
>                                  struct sta_info *sta, u8 opmode,
> -                                enum ieee80211_band band, bool nss_only);
> +                                enum ieee80211_band band);
>  void ieee80211_apply_vhtcap_overrides(struct ieee80211_sub_if_data *sdata,
>                                       struct ieee80211_sta_vht_cap *vht_cap);
>  void ieee80211_get_vht_mask_from_cap(__le16 vht_cap,
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 1e6b337..62c8e4f 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -3584,7 +3584,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
>
>         if (sta && elems.opmode_notif)
>                 ieee80211_vht_handle_opmode(sdata, sta, *elems.opmode_notif,
> -                                           rx_status->band, true);
> +                                           rx_status->band);
>         mutex_unlock(&local->sta_mtx);
>
>         changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 901c72b..fe675d7 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -2738,8 +2738,7 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
>                         opmode = mgmt->u.action.u.vht_opmode_notif.operating_mode;
>
>                         ieee80211_vht_handle_opmode(rx->sdata, rx->sta,
> -                                                   opmode, status->band,
> -                                                   false);
> +                                                   opmode, status->band);
>                         goto handled;
>                 }
>                 case WLAN_VHT_ACTION_GROUPID_MGMT: {
> diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
> index d2f2ff6..a02525a 100644
> --- a/net/mac80211/vht.c
> +++ b/net/mac80211/vht.c
> @@ -399,7 +399,7 @@ void ieee80211_sta_set_rx_nss(struct sta_info *sta)
>
>  u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
>                                   struct sta_info *sta, u8 opmode,
> -                                 enum ieee80211_band band, bool nss_only)
> +                                 enum ieee80211_band band)
>  {
>         struct ieee80211_local *local = sdata->local;
>         struct ieee80211_supported_band *sband;
> @@ -422,9 +422,6 @@ u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
>                 changed |= IEEE80211_RC_NSS_CHANGED;
>         }
>
> -       if (nss_only)
> -               return changed;
> -
>         switch (opmode & IEEE80211_OPMODE_NOTIF_CHANWIDTH_MASK) {
>         case IEEE80211_OPMODE_NOTIF_CHANWIDTH_20MHZ:
>                 sta->cur_max_bandwidth = IEEE80211_STA_RX_BW_20;
> @@ -473,13 +470,12 @@ void ieee80211_process_mu_groups(struct ieee80211_sub_if_data *sdata,
>
>  void ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
>                                  struct sta_info *sta, u8 opmode,
> -                                enum ieee80211_band band, bool nss_only)
> +                                enum ieee80211_band band)
>  {
>         struct ieee80211_local *local = sdata->local;
>         struct ieee80211_supported_band *sband = local->hw.wiphy->bands[band];
>
> -       u32 changed = __ieee80211_vht_handle_opmode(sdata, sta, opmode,
> -                                                   band, nss_only);
> +       u32 changed = __ieee80211_vht_handle_opmode(sdata, sta, opmode, band);
>
>         if (changed > 0)
>                 rate_control_rate_update(local, sband, sta, changed);

Not related to current change.

I was looking at this code a while ago and found that rate_control_rate_update
doesn't update the rates from rx_nss, rather it updates from HT/VHT
capabilities.

So how does the NSS update from OP MODE IE work?
(BW part is handled in minstrel from sta->bandwidth)
--
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
Johannes Berg Dec. 8, 2015, 3:42 p.m. UTC | #2
On Tue, 2015-12-08 at 21:08 +0530, Krishna Chaitanya wrote:

> >  void ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data
> > *sdata,
> >                                  struct sta_info *sta, u8 opmode,
> > -                                enum ieee80211_band band, bool
> > nss_only)
> > +                                enum ieee80211_band band)
> >  {
> >         struct ieee80211_local *local = sdata->local;
> >         struct ieee80211_supported_band *sband = local->hw.wiphy-
> > >bands[band];
> > 
> > -       u32 changed = __ieee80211_vht_handle_opmode(sdata, sta,
> > opmode,
> > -                                                   band,
> > nss_only);
> > +       u32 changed = __ieee80211_vht_handle_opmode(sdata, sta,
> > opmode, band);
> > 
> >         if (changed > 0)
> >                 rate_control_rate_update(local, sband, sta,
> > changed);
> 
> Not related to current change.
> 
> I was looking at this code a while ago and found that
> rate_control_rate_update
> doesn't update the rates from rx_nss, rather it updates from HT/VHT
> capabilities.
> 
> So how does the NSS update from OP MODE IE work?
> 

Huh? You just quoted the code that does this?

If the rate control algorithm doesn't look at sta->sta.rx_nss then
that's their bug.

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
Krishna Chaitanya Dec. 8, 2015, 3:47 p.m. UTC | #3
On Tue, Dec 8, 2015 at 9:12 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2015-12-08 at 21:08 +0530, Krishna Chaitanya wrote:
>>
>> >  void ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data
>> > *sdata,
>> >                                  struct sta_info *sta, u8 opmode,
>> > -                                enum ieee80211_band band, bool
>> > nss_only)
>> > +                                enum ieee80211_band band)
>> >  {
>> >         struct ieee80211_local *local = sdata->local;
>> >         struct ieee80211_supported_band *sband = local->hw.wiphy-
>> > >bands[band];
>> >
>> > -       u32 changed = __ieee80211_vht_handle_opmode(sdata, sta,
>> > opmode,
>> > -                                                   band,
>> > nss_only);
>> > +       u32 changed = __ieee80211_vht_handle_opmode(sdata, sta,
>> > opmode, band);
>> >
>> >         if (changed > 0)
>> >                 rate_control_rate_update(local, sband, sta,
>> > changed);
>>
>> Not related to current change.
>>
>> I was looking at this code a while ago and found that
>> rate_control_rate_update
>> doesn't update the rates from rx_nss, rather it updates from HT/VHT
>> capabilities.
>>
>> So how does the NSS update from OP MODE IE work?
>>
>
> Huh? You just quoted the code that does this?
MLME is updating the rx_nss.

> If the rate control algorithm doesn't look at sta->sta.rx_nss then
> that's their bug.

Yes, it looks like it. Only BW is handled, not the NSS change, and
without this patch OP MODE IE in beacon updates neither NSS nor BW.

For Action frame OP MODE IE, NSS will be updated.
--
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
Johannes Berg Dec. 8, 2015, 3:53 p.m. UTC | #4
On Tue, 2015-12-08 at 21:17 +0530, Krishna Chaitanya wrote:

> > If the rate control algorithm doesn't look at sta->sta.rx_nss then
> > that's their bug.
> 
> Yes, it looks like it. Only BW is handled, not the NSS change, and
> without this patch OP MODE IE in beacon updates neither NSS nor BW.
> 
> For Action frame OP MODE IE, NSS will be updated.
> 

I don't see how that would happen - they end up in exactly the same
code path.

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
Krishna Chaitanya Dec. 8, 2015, 3:55 p.m. UTC | #5
On Tue, Dec 8, 2015 at 9:23 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2015-12-08 at 21:17 +0530, Krishna Chaitanya wrote:
>>
>> > If the rate control algorithm doesn't look at sta->sta.rx_nss then
>> > that's their bug.
>>
>> Yes, it looks like it. Only BW is handled, not the NSS change, and
>> without this patch OP MODE IE in beacon updates neither NSS nor BW.
>>
>> For Action frame OP MODE IE, NSS will be updated.
Typo BW will be updated.
>
> I don't see how that would happen - they end up in exactly the same
> code path.

As nss_only is false, BW will be update in case of action frame.
--
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
Johannes Berg Dec. 8, 2015, 3:58 p.m. UTC | #6
On Tue, 2015-12-08 at 21:25 +0530, Krishna Chaitanya wrote:
> On Tue, Dec 8, 2015 at 9:23 PM, Johannes Berg <johannes@sipsolutions.
> net> wrote:
> > On Tue, 2015-12-08 at 21:17 +0530, Krishna Chaitanya wrote:
> > > 
> > > > If the rate control algorithm doesn't look at sta->sta.rx_nss
> > > > then
> > > > that's their bug.
> > > 
> > > Yes, it looks like it. Only BW is handled, not the NSS change,
> > > and
> > > without this patch OP MODE IE in beacon updates neither NSS nor
> > > BW.
> > > 
> > > For Action frame OP MODE IE, NSS will be updated.
> Typo BW will be updated.

Ok.

> > I don't see how that would happen - they end up in exactly the same
> > code path.
> 
> As nss_only is false, BW will be update in case of action frame.

Well, this patch removes that since the whole concept of nss_only was
wrong.

So basically you're just saying that minstrel should be fixed to look
at rx_nss (and the relevant change bit, perhaps)...

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
Krishna Chaitanya Dec. 8, 2015, 4:03 p.m. UTC | #7
On Tue, Dec 8, 2015 at 9:28 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2015-12-08 at 21:25 +0530, Krishna Chaitanya wrote:
>> On Tue, Dec 8, 2015 at 9:23 PM, Johannes Berg <johannes@sipsolutions.
>> net> wrote:
>> > On Tue, 2015-12-08 at 21:17 +0530, Krishna Chaitanya wrote:
>> > >
>> > > > If the rate control algorithm doesn't look at sta->sta.rx_nss
>> > > > then
>> > > > that's their bug.
>> > >
>> > > Yes, it looks like it. Only BW is handled, not the NSS change,
>> > > and
>> > > without this patch OP MODE IE in beacon updates neither NSS nor
>> > > BW.
>> > >
>> > > For Action frame OP MODE IE, NSS will be updated.
>> Typo BW will be updated.
>
> Ok.
>
>> > I don't see how that would happen - they end up in exactly the same
>> > code path.
>>
>> As nss_only is false, BW will be update in case of action frame.
>
> Well, this patch removes that since the whole concept of nss_only was
> wrong.
>
> So basically you're just saying that minstrel should be fixed to look
> at rx_nss (and the relevant change bit, perhaps)...
>
Exactly :-).
--
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 mbox

Patch

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b9e2c2f..a90d875 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1198,8 +1198,7 @@  static int sta_apply_parameters(struct ieee80211_local *local,
 		 * rc isn't initialized here yet, so ignore it
 		 */
 		__ieee80211_vht_handle_opmode(sdata, sta,
-					      params->opmode_notif,
-					      band, false);
+					      params->opmode_notif, band);
 	}
 
 	if (ieee80211_vif_is_mesh(&sdata->vif))
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 582ea86..747402d 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1718,10 +1718,10 @@  void ieee80211_process_mu_groups(struct ieee80211_sub_if_data *sdata,
 				 struct ieee80211_mgmt *mgmt);
 u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
                                   struct sta_info *sta, u8 opmode,
-                                  enum ieee80211_band band, bool nss_only);
+				  enum ieee80211_band band);
 void ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
 				 struct sta_info *sta, u8 opmode,
-				 enum ieee80211_band band, bool nss_only);
+				 enum ieee80211_band band);
 void ieee80211_apply_vhtcap_overrides(struct ieee80211_sub_if_data *sdata,
 				      struct ieee80211_sta_vht_cap *vht_cap);
 void ieee80211_get_vht_mask_from_cap(__le16 vht_cap,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 1e6b337..62c8e4f 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3584,7 +3584,7 @@  static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 
 	if (sta && elems.opmode_notif)
 		ieee80211_vht_handle_opmode(sdata, sta, *elems.opmode_notif,
-					    rx_status->band, true);
+					    rx_status->band);
 	mutex_unlock(&local->sta_mtx);
 
 	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 901c72b..fe675d7 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2738,8 +2738,7 @@  ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
 			opmode = mgmt->u.action.u.vht_opmode_notif.operating_mode;
 
 			ieee80211_vht_handle_opmode(rx->sdata, rx->sta,
-						    opmode, status->band,
-						    false);
+						    opmode, status->band);
 			goto handled;
 		}
 		case WLAN_VHT_ACTION_GROUPID_MGMT: {
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index d2f2ff6..a02525a 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -399,7 +399,7 @@  void ieee80211_sta_set_rx_nss(struct sta_info *sta)
 
 u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
 				  struct sta_info *sta, u8 opmode,
-				  enum ieee80211_band band, bool nss_only)
+				  enum ieee80211_band band)
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_supported_band *sband;
@@ -422,9 +422,6 @@  u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
 		changed |= IEEE80211_RC_NSS_CHANGED;
 	}
 
-	if (nss_only)
-		return changed;
-
 	switch (opmode & IEEE80211_OPMODE_NOTIF_CHANWIDTH_MASK) {
 	case IEEE80211_OPMODE_NOTIF_CHANWIDTH_20MHZ:
 		sta->cur_max_bandwidth = IEEE80211_STA_RX_BW_20;
@@ -473,13 +470,12 @@  void ieee80211_process_mu_groups(struct ieee80211_sub_if_data *sdata,
 
 void ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
 				 struct sta_info *sta, u8 opmode,
-				 enum ieee80211_band band, bool nss_only)
+				 enum ieee80211_band band)
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_supported_band *sband = local->hw.wiphy->bands[band];
 
-	u32 changed = __ieee80211_vht_handle_opmode(sdata, sta, opmode,
-						    band, nss_only);
+	u32 changed = __ieee80211_vht_handle_opmode(sdata, sta, opmode, band);
 
 	if (changed > 0)
 		rate_control_rate_update(local, sband, sta, changed);