[PATCH 0/9] various mac80211 / cfg80211 patches

[PATCH 0/9] various mac80211 / cfg80211 patches

[Emmanuel Grumbach]

Hi Johannes,

This is a mix of a few random things. A few of things seem
to deserved to be merge in mac80211.git, but all this is
based on mac80211-next.git.

Emmanuel Grumbach (1):
  mac80211: limit the A-MSDU Tx based on peer's capabilities

Eyal Shapira (1):
  mac80211: handle width changes from opmode notification IE in beacon

Johannes Berg (4):
  mac80211: document status.freq restrictions
  mac80211: suppress unchanged "limiting TX power" messages
  mac80211: run scan completed work on reconfig failure
  mac80211: reprogram in interface order

Sara Sharon (3):
  mac80211: process and save VHT MU-MIMO group frame
  mac80211: add flag for duplication check
  mac80211: pass aggregation window size to lower level

 include/linux/ieee80211.h  |  26 ++++++++++
 include/net/mac80211.h     |  43 +++++++++++++++--
 net/mac80211/agg-rx.c      |   2 +-
 net/mac80211/cfg.c         |  32 ++++++++++++-
 net/mac80211/ht.c          |   5 ++
 net/mac80211/ieee80211_i.h |   6 ++-
 net/mac80211/iface.c       |  10 ++++
 net/mac80211/mlme.c        |  24 +++++++---
 net/mac80211/rx.c          |  11 ++++-
 net/mac80211/util.c        | 116 ++++++++++++++++++++++++++-------------------
 net/mac80211/vht.c         |  53 ++++++++++++++++++---
 11 files changed, 254 insertions(+), 74 deletions(-)

-- 
2.5.0

[PATCH 1/9] mac80211: process and save VHT MU-MIMO group frame

[Emmanuel Grumbach]

From: Sara Sharon <sara.sharon@intel.com>

The Group ID Management frame is an Action frame of
category VHT. It is transmitted by the AP to assign
or change the user position of a STA for one or more
group IDs.
Process and save the group membership data. Notify
underlying driver of changes.

Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 include/linux/ieee80211.h  |  7 +++++++
 include/net/mac80211.h     | 17 +++++++++++++++++
 net/mac80211/ieee80211_i.h |  2 ++
 net/mac80211/iface.c       | 10 ++++++++++
 net/mac80211/mlme.c        |  7 +++++++
 net/mac80211/rx.c          |  5 +++++
 net/mac80211/util.c        |  3 +++
 net/mac80211/vht.c         | 25 +++++++++++++++++++++++++
 8 files changed, 76 insertions(+)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 452c0b0..d9ddb89 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -843,6 +843,8 @@ enum ieee80211_vht_opmode_bits {
 };
 
 #define WLAN_SA_QUERY_TR_ID_LEN 2
+#define WLAN_MEMBERSHIP_LEN 8
+#define WLAN_USER_POSITION_LEN 16
 
 /**
  * struct ieee80211_tpc_report_ie
@@ -991,6 +993,11 @@ struct ieee80211_mgmt {
 				} __packed vht_opmode_notif;
 				struct {
 					u8 action_code;
+					u8 membership[WLAN_MEMBERSHIP_LEN];
+					u8 position[WLAN_USER_POSITION_LEN];
+				} __packed vht_group_notif;
+				struct {
+					u8 action_code;
 					u8 dialog_token;
 					u8 tpc_elem_id;
 					u8 tpc_elem_length;
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 7c30faf..8da483b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -298,6 +298,7 @@ struct ieee80211_vif_chanctx_switch {
  *	note that this is only called when it changes after the channel
  *	context had been assigned.
  * @BSS_CHANGED_OCB: OCB join status changed
+ * @BSS_CHANGED_MU_GROUPS: VHT MU-MIMO group id or user position changed
  */
 enum ieee80211_bss_change {
 	BSS_CHANGED_ASSOC		= 1<<0,
@@ -323,6 +324,7 @@ enum ieee80211_bss_change {
 	BSS_CHANGED_BEACON_INFO		= 1<<20,
 	BSS_CHANGED_BANDWIDTH		= 1<<21,
 	BSS_CHANGED_OCB                 = 1<<22,
+	BSS_CHANGED_MU_GROUPS		= 1<<23,
 
 	/* when adding here, make sure to change ieee80211_reconfig */
 };
@@ -436,6 +438,19 @@ struct ieee80211_event {
 };
 
 /**
+ * struct ieee80211_mu_group_data - STA's VHT MU-MIMO group data
+ *
+ * This structure describes the group id data of VHT MU-MIMO
+ *
+ * @membership: 64 bits array - a bit is set if station is member of the group
+ * @position: 2 bits per group id indicating the position in the group
+ */
+struct ieee80211_mu_group_data {
+	u8 membership[WLAN_MEMBERSHIP_LEN];
+	u8 position[WLAN_USER_POSITION_LEN];
+};
+
+/**
  * struct ieee80211_bss_conf - holds the BSS's changing parameters
  *
  * This structure keeps information about a BSS (and an association
@@ -477,6 +492,7 @@ struct ieee80211_event {
  * @enable_beacon: whether beaconing should be enabled or not
  * @chandef: Channel definition for this BSS -- the hardware might be
  *	configured a higher bandwidth than this BSS uses, for example.
+ * @mu_group: VHT MU-MIMO group membership data
  * @ht_operation_mode: HT operation mode like in &struct ieee80211_ht_operation.
  *	This field is only valid when the channel is a wide HT/VHT channel.
  *	Note that with TDLS this can be the case (channel is HT, protection must
@@ -535,6 +551,7 @@ struct ieee80211_bss_conf {
 	s32 cqm_rssi_thold;
 	u32 cqm_rssi_hyst;
 	struct cfg80211_chan_def chandef;
+	struct ieee80211_mu_group_data mu_group;
 	__be32 arp_addr_list[IEEE80211_BSS_ARP_ADDR_LIST_LEN];
 	int arp_addr_cnt;
 	bool qos;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c30b684..582ea86 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1714,6 +1714,8 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct ieee80211_sub_if_data *sdata,
 enum ieee80211_sta_rx_bandwidth ieee80211_sta_cap_rx_bw(struct sta_info *sta);
 enum ieee80211_sta_rx_bandwidth ieee80211_sta_cur_vht_bw(struct sta_info *sta);
 void ieee80211_sta_set_rx_nss(struct sta_info *sta);
+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);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index c9e325d..33ae3c8 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1271,6 +1271,16 @@ static void ieee80211_iface_work(struct work_struct *work)
 				}
 			}
 			mutex_unlock(&local->sta_mtx);
+		} else if (ieee80211_is_action(mgmt->frame_control) &&
+			   mgmt->u.action.category == WLAN_CATEGORY_VHT) {
+			switch (mgmt->u.action.u.vht_group_notif.action_code) {
+			case WLAN_VHT_ACTION_GROUPID_MGMT:
+				ieee80211_process_mu_groups(sdata, mgmt);
+				break;
+			default:
+				WARN_ON(1);
+				break;
+			}
 		} else if (ieee80211_is_data_qos(mgmt->frame_control)) {
 			struct ieee80211_hdr *hdr = (void *)mgmt;
 			/*
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 123b26d..1e6b337 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2074,6 +2074,13 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 	memset(&ifmgd->ht_capa_mask, 0, sizeof(ifmgd->ht_capa_mask));
 	memset(&ifmgd->vht_capa, 0, sizeof(ifmgd->vht_capa));
 	memset(&ifmgd->vht_capa_mask, 0, sizeof(ifmgd->vht_capa_mask));
+
+	/* reset MU-MIMO ownership and group data */
+	memset(sdata->vif.bss_conf.mu_group.membership, 0,
+	       sizeof(sdata->vif.bss_conf.mu_group.membership));
+	memset(sdata->vif.bss_conf.mu_group.position, 0,
+	       sizeof(sdata->vif.bss_conf.mu_group.position));
+	changed |= BSS_CHANGED_MU_GROUPS;
 	sdata->flags &= ~IEEE80211_SDATA_MU_MIMO_OWNER;
 
 	sdata->ap_power_level = IEEE80211_UNSET_POWER_LEVEL;
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 1f82753..c4198d48 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2739,6 +2739,11 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
 						    false);
 			goto handled;
 		}
+		case WLAN_VHT_ACTION_GROUPID_MGMT: {
+			if (len < IEEE80211_MIN_ACTION_SIZE + 25)
+				goto invalid;
+			goto queue;
+		}
 		default:
 			break;
 		}
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 08af2b3..4d6130b 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1905,6 +1905,9 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 			  BSS_CHANGED_IDLE |
 			  BSS_CHANGED_TXPOWER;
 
+		if (sdata->flags & IEEE80211_SDATA_MU_MIMO_OWNER)
+			changed |= BSS_CHANGED_MU_GROUPS;
+
 		switch (sdata->vif.type) {
 		case NL80211_IFTYPE_STATION:
 			changed |= BSS_CHANGED_ASSOC |
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index ff1c798..92c9843 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -1,6 +1,9 @@
 /*
  * VHT handling
  *
+ * Portions of this file
+ * Copyright(c) 2015 Intel Deutschland GmbH
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
@@ -428,6 +431,28 @@ u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
 	return changed;
 }
 
+void ieee80211_process_mu_groups(struct ieee80211_sub_if_data *sdata,
+				 struct ieee80211_mgmt *mgmt)
+{
+	struct ieee80211_bss_conf *bss_conf = &sdata->vif.bss_conf;
+
+	if (!(sdata->flags & IEEE80211_SDATA_MU_MIMO_OWNER))
+		return;
+
+	if (!memcmp(mgmt->u.action.u.vht_group_notif.position,
+		    bss_conf->mu_group.position, WLAN_USER_POSITION_LEN) &&
+	    !memcmp(mgmt->u.action.u.vht_group_notif.membership,
+		    bss_conf->mu_group.membership, WLAN_MEMBERSHIP_LEN))
+		return;
+
+	memcpy(mgmt->u.action.u.vht_group_notif.membership,
+	       bss_conf->mu_group.membership, WLAN_MEMBERSHIP_LEN);
+	memcpy(mgmt->u.action.u.vht_group_notif.position,
+	       bss_conf->mu_group.position, WLAN_USER_POSITION_LEN);
+
+	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_MU_GROUPS);
+}
+
 void ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
 				 struct sta_info *sta, u8 opmode,
 				 enum ieee80211_band band, bool nss_only)
-- 
2.5.0

[PATCH 2/9] mac80211: limit the A-MSDU Tx based on peer's capabilities

[Emmanuel Grumbach]

In VHT, the specification allows to limit the number of
MSDUs in an A-MSDU in the Extended Capabilities IE. There
is also a limitation on the byte size in the VHT IE.
In HT, the only limitation is on the byte size.
Parse the capabilities from the peer and make them
available to the driver.

In HT, there is another limitation when a BA agreement
is active: the byte size can't be greater than 4095.
This is not enforced here.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 include/linux/ieee80211.h | 19 +++++++++++++++++++
 include/net/mac80211.h    | 14 ++++++++++++++
 net/mac80211/cfg.c        | 29 +++++++++++++++++++++++++++++
 net/mac80211/ht.c         |  5 +++++
 net/mac80211/vht.c        | 18 ++++++++++++++++++
 5 files changed, 85 insertions(+)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index d9ddb89..3b1f6ce 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -163,6 +163,14 @@ static inline u16 ieee80211_sn_sub(u16 sn1, u16 sn2)
 /* 30 byte 4 addr hdr, 2 byte QoS, 2304 byte MSDU, 12 byte crypt, 4 byte FCS */
 #define IEEE80211_MAX_FRAME_LEN		2352
 
+/* Maximal size of an A-MSDU */
+#define IEEE80211_MAX_MPDU_LEN_HT_3839		3839
+#define IEEE80211_MAX_MPDU_LEN_HT_7935		7935
+
+#define IEEE80211_MAX_MPDU_LEN_VHT_3895		3895
+#define IEEE80211_MAX_MPDU_LEN_VHT_7991		7991
+#define IEEE80211_MAX_MPDU_LEN_VHT_11454	11454
+
 #define IEEE80211_MAX_SSID_LEN		32
 
 #define IEEE80211_MAX_MESH_ID_LEN	32
@@ -1505,6 +1513,7 @@ struct ieee80211_vht_operation {
 #define IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895			0x00000000
 #define IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991			0x00000001
 #define IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454			0x00000002
+#define IEEE80211_VHT_CAP_MAX_MPDU_MASK				0x00000003
 #define IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ		0x00000004
 #define IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ	0x00000008
 #define IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK			0x0000000C
@@ -2086,6 +2095,16 @@ enum ieee80211_tdls_actioncode {
 #define WLAN_EXT_CAPA8_TDLS_WIDE_BW_ENABLED	BIT(5)
 #define WLAN_EXT_CAPA8_OPMODE_NOTIF	BIT(6)
 
+/* Defines the maximal number of MSDUs in an A-MSDU. */
+#define WLAN_EXT_CAPA8_MAX_MSDU_IN_AMSDU_LSB	BIT(7)
+#define WLAN_EXT_CAPA9_MAX_MSDU_IN_AMSDU_MSB	BIT(0)
+
+/*
+ * Fine Timing Measurement Initiator - bit 71 of @WLAN_EID_EXT_CAPABILITY
+ * information element
+ */
+#define WLAN_EXT_CAPA9_FTM_INITIATOR	BIT(7)
+
 /* TDLS specific payload type in the LLC/SNAP header */
 #define WLAN_TDLS_SNAP_RFTYPE	0x2
 
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 8da483b..71be30d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1701,6 +1701,18 @@ struct ieee80211_sta_rates {
  * @tdls_initiator: indicates the STA is an initiator of the TDLS link. Only
  *	valid if the STA is a TDLS peer in the first place.
  * @mfp: indicates whether the STA uses management frame protection or not.
+ * @max_amsdu_subframes: indicates the maximal number of MSDUs in a single
+ *	A-MSDU. Taken from the Extended Capabilities element. 0 means
+ *	unlimited.
+ * @max_amsdu_len: indicates the maximal length of an A-MSDU in bytes. This
+ *	field is always valid for packets with a VHT preamble. For packets
+ *	with a HT preamble, additional limits apply:
+ *		+ If the skb is transmitted as part of a BA agreement, the
+ *		  A-MSDU maximal size is min(max_amsdu_len, 4065) bytes.
+ *		+ If the skb is not part of a BA aggreement, the A-MSDU maximal
+ *		  size is min(max_amsdu_len, 7935) bytes.
+ *	Both additional HT limits must be enforced by the low level driver.
+ *	This is defined by the spec (IEEE 802.11-2012 section 8.3.2.2 NOTE 2).
  * @txq: per-TID data TX queues (if driver uses the TXQ abstraction)
  */
 struct ieee80211_sta {
@@ -1719,6 +1731,8 @@ struct ieee80211_sta {
 	bool tdls;
 	bool tdls_initiator;
 	bool mfp;
+	u8 max_amsdu_subframes;
+	u16 max_amsdu_len;
 
 	struct ieee80211_txq *txq[IEEE80211_NUM_TIDS];
 
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 2d1c4c3..b9e2c2f 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1131,6 +1131,34 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 		sta->sta.max_sp = params->max_sp;
 	}
 
+	/* The sender might not have sent the last bit, consider it to be 0 */
+	if (params->ext_capab_len >= 8) {
+		u8 val = (params->ext_capab[7] &
+			  WLAN_EXT_CAPA8_MAX_MSDU_IN_AMSDU_LSB) >> 7;
+
+		/* we did get all the bits, take the MSB as well */
+		if (params->ext_capab_len >= 9) {
+			u8 val_msb = params->ext_capab[8] &
+				WLAN_EXT_CAPA9_MAX_MSDU_IN_AMSDU_MSB;
+			val_msb <<= 1;
+			val |= val_msb;
+		}
+
+		switch (val) {
+		case 1:
+			sta->sta.max_amsdu_subframes = 32;
+			break;
+		case 2:
+			sta->sta.max_amsdu_subframes = 16;
+			break;
+		case 3:
+			sta->sta.max_amsdu_subframes = 8;
+			break;
+		default:
+			sta->sta.max_amsdu_subframes = 0;
+		};
+	}
+
 	/*
 	 * cfg80211 validates this (1-2007) and allows setting the AID
 	 * only when creating a new station entry
@@ -1160,6 +1188,7 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 		ieee80211_ht_cap_ie_to_sta_ht_cap(sdata, sband,
 						  params->ht_capa, sta);
 
+	/* VHT can override some HT caps such as the A-MSDU max length */
 	if (params->vht_capa)
 		ieee80211_vht_cap_ie_to_sta_vht_cap(sdata, sband,
 						    params->vht_capa, sta);
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index 7a76ce6..f4a5287 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -230,6 +230,11 @@ bool ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data *sdata,
 	/* set Rx highest rate */
 	ht_cap.mcs.rx_highest = ht_cap_ie->mcs.rx_highest;
 
+	if (ht_cap.cap & IEEE80211_HT_CAP_MAX_AMSDU)
+		sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_HT_7935;
+	else
+		sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_HT_3839;
+
  apply:
 	changed = memcmp(&sta->sta.ht_cap, &ht_cap, sizeof(ht_cap));
 
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index 92c9843..d2f2ff6 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -281,6 +281,24 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct ieee80211_sub_if_data *sdata,
 	}
 
 	sta->sta.bandwidth = ieee80211_sta_cur_vht_bw(sta);
+
+	/* If HT IE reported 3839 bytes only, stay with that size. */
+	if (sta->sta.max_amsdu_len == IEEE80211_MAX_MPDU_LEN_HT_3839)
+		return;
+
+	switch (vht_cap->cap & IEEE80211_VHT_CAP_MAX_MPDU_MASK) {
+	case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454:
+		sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_11454;
+		break;
+	case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991:
+		sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_7991;
+		break;
+	case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895:
+	default:
+		sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_3895;
+		break;
+	}
+
 }
 
 enum ieee80211_sta_rx_bandwidth ieee80211_sta_cap_rx_bw(struct sta_info *sta)
-- 
2.5.0

Re: [PATCH 2/9] mac80211: limit the A-MSDU Tx based on peer's capabilities

[Krishna Chaitanya]

On Tue, Dec 8, 2015 at 7:34 PM, Emmanuel Grumbach
<emmanuel.grumbach@intel.com> wrote:
> index 7a76ce6..f4a5287 100644
> --- a/net/mac80211/ht.c
> +++ b/net/mac80211/ht.c
> @@ -230,6 +230,11 @@ bool ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data *sdata,
>         /* set Rx highest rate */
>         ht_cap.mcs.rx_highest = ht_cap_ie->mcs.rx_highest;
>
> +       if (ht_cap.cap & IEEE80211_HT_CAP_MAX_AMSDU)
> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_HT_7935;
> +       else
> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_HT_3839;
> +
>   apply:
>         changed = memcmp(&sta->sta.ht_cap, &ht_cap, sizeof(ht_cap));
>
> diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
> index 92c9843..d2f2ff6 100644
> --- a/net/mac80211/vht.c
> +++ b/net/mac80211/vht.c
> @@ -281,6 +281,24 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct ieee80211_sub_if_data *sdata,
>         }
>
>         sta->sta.bandwidth = ieee80211_sta_cur_vht_bw(sta);
> +
> +       /* If HT IE reported 3839 bytes only, stay with that size. */
> +       if (sta->sta.max_amsdu_len == IEEE80211_MAX_MPDU_LEN_HT_3839)
> +               return;
> +
> +       switch (vht_cap->cap & IEEE80211_VHT_CAP_MAX_MPDU_MASK) {
> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454:
> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_11454;
> +               break;
> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991:
> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_7991;
> +               break;
> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895:
> +       default:
> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_3895;
> +               break;
> +       }
Ideally speaking shouldn't we use different variables for HT and VHT
and depending on
rate HT/VHT we should aggregate accordingly? of course that will be
tricky as we dont
have the rate control info at the time of aggregation. Standard is
clear about usage of
independent lengths depending on whether its an VHT/HT PPDU.

If we use the same variable to arrive at max_amsdu_len for both VHT
and HT, we might
end up sending 11454 sized MSDU in a HT rate.

Re: [PATCH 2/9] mac80211: limit the A-MSDU Tx based on peer's capabilities

[Grumbach, Emmanuel]

On 12/08/2015 06:03 PM, Krishna Chaitanya wrote:
> On Tue, Dec 8, 2015 at 7:34 PM, Emmanuel Grumbach
> <emmanuel.grumbach@intel.com> wrote:
>> index 7a76ce6..f4a5287 100644
>> --- a/net/mac80211/ht.c
>> +++ b/net/mac80211/ht.c
>> @@ -230,6 +230,11 @@ bool ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data *sdata,
>>         /* set Rx highest rate */
>>         ht_cap.mcs.rx_highest = ht_cap_ie->mcs.rx_highest;
>>
>> +       if (ht_cap.cap & IEEE80211_HT_CAP_MAX_AMSDU)
>> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_HT_7935;
>> +       else
>> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_HT_3839;
>> +
>>   apply:
>>         changed = memcmp(&sta->sta.ht_cap, &ht_cap, sizeof(ht_cap));
>>
>> diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
>> index 92c9843..d2f2ff6 100644
>> --- a/net/mac80211/vht.c
>> +++ b/net/mac80211/vht.c
>> @@ -281,6 +281,24 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct ieee80211_sub_if_data *sdata,
>>         }
>>
>>         sta->sta.bandwidth = ieee80211_sta_cur_vht_bw(sta);
>> +
>> +       /* If HT IE reported 3839 bytes only, stay with that size. */
>> +       if (sta->sta.max_amsdu_len == IEEE80211_MAX_MPDU_LEN_HT_3839)
>> +               return;
>> +
>> +       switch (vht_cap->cap & IEEE80211_VHT_CAP_MAX_MPDU_MASK) {
>> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454:
>> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_11454;
>> +               break;
>> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991:
>> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_7991;
>> +               break;
>> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895:
>> +       default:
>> +               sta->sta.max_amsdu_len = IEEE80211_MAX_MPDU_LEN_VHT_3895;
>> +               break;
>> +       }
> Ideally speaking shouldn't we use different variables for HT and VHT
> and depending on
> rate HT/VHT we should aggregate accordingly? of course that will be
> tricky as we dont
> have the rate control info at the time of aggregation. Standard is
> clear about usage of
> independent lengths depending on whether its an VHT/HT PPDU.

Yes - but since it is tricky, I preferred to be on the safe side and
limit VHT amsdus for the very peculiar AP that would decide to have
large A-MSDUs in VHT and small ones in HT (?!).

> If we use the same variable to arrive at max_amsdu_len for both VHT
> and HT, we might
> end up sending 11454 sized MSDU in a HT rate.
>
And since that can't be handled at the time of the A-MSDU building,
leave that to another entity :)

Re: [PATCH 2/9] mac80211: limit the A-MSDU Tx based on peer's capabilities

[Grumbach, Emmanuel]

On 12/08/2015 06:35 PM, Krishna Chaitanya wrote:
>
>
> On Dec 8, 2015 10:01 PM, "Grumbach, Emmanuel"
> <emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>> wrote:
> >
> >
> >
> > On 12/08/2015 06:03 PM, Krishna Chaitanya wrote:
> > > On Tue, Dec 8, 2015 at 7:34 PM, Emmanuel Grumbach
> > > <emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>>
> wrote:
> > >> index 7a76ce6..f4a5287 100644
> > >> --- a/net/mac80211/ht.c
> > >> +++ b/net/mac80211/ht.c
> > >> @@ -230,6 +230,11 @@ bool
> ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data *sdata,
> > >>         /* set Rx highest rate */
> > >>         ht_cap.mcs.rx_highest = ht_cap_ie->mcs.rx_highest;
> > >>
> > >> +       if (ht_cap.cap & IEEE80211_HT_CAP_MAX_AMSDU)
> > >> +               sta->sta.max_amsdu_len =
> IEEE80211_MAX_MPDU_LEN_HT_7935;
> > >> +       else
> > >> +               sta->sta.max_amsdu_len =
> IEEE80211_MAX_MPDU_LEN_HT_3839;
> > >> +
> > >>   apply:
> > >>         changed = memcmp(&sta->sta.ht_cap, &ht_cap, sizeof(ht_cap));
> > >>
> > >> diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
> > >> index 92c9843..d2f2ff6 100644
> > >> --- a/net/mac80211/vht.c
> > >> +++ b/net/mac80211/vht.c
> > >> @@ -281,6 +281,24 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct
> ieee80211_sub_if_data *sdata,
> > >>         }
> > >>
> > >>         sta->sta.bandwidth = ieee80211_sta_cur_vht_bw(sta);
> > >> +
> > >> +       /* If HT IE reported 3839 bytes only, stay with that size. */
> > >> +       if (sta->sta.max_amsdu_len == IEEE80211_MAX_MPDU_LEN_HT_3839)
> > >> +               return;
> > >> +
> > >> +       switch (vht_cap->cap & IEEE80211_VHT_CAP_MAX_MPDU_MASK) {
> > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454:
> > >> +               sta->sta.max_amsdu_len =
> IEEE80211_MAX_MPDU_LEN_VHT_11454;
> > >> +               break;
> > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991:
> > >> +               sta->sta.max_amsdu_len =
> IEEE80211_MAX_MPDU_LEN_VHT_7991;
> > >> +               break;
> > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895:
> > >> +       default:
> > >> +               sta->sta.max_amsdu_len =
> IEEE80211_MAX_MPDU_LEN_VHT_3895;
> > >> +               break;
> > >> +       }
> > > Ideally speaking shouldn't we use different variables for HT and VHT
> > > and depending on
> > > rate HT/VHT we should aggregate accordingly? of course that will be
> > > tricky as we dont
> > > have the rate control info at the time of aggregation. Standard is
> > > clear about usage of
> > > independent lengths depending on whether its an VHT/HT PPDU.
> >
> > Yes - but since it is tricky, I preferred to be on the safe side and
> > limit VHT amsdus for the very peculiar AP that would decide to have
> > large A-MSDUs in VHT and small ones in HT (?!).
> Yes but wouldn't it be safer to just use min(ht , vht)? For a good AP
> it shouldn't matter and bad AP it will still work.
>
This is the de-facto what this code does I think.
If the HT limit is 4K, then don't even take the VHT limit into account
and the VHT limit can't be smaller than the HT one in that case.
If the HT limit is 8K, then we can limit it further to 4K in case VHT
has a limit of 4K (which is another weird case), but we can also make it
larger for VHT frames?

So I don't really see the bug here, am I missing something?

> > > If we use the same variable to arrive at max_amsdu_len for both VHT
> > > and HT, we might
> > > end up sending 11454 sized MSDU in a HT rate.
> > >
> > And since that can't be handled at the time of the A-MSDU building,
> > leave that to another entity :)
>

Re: [PATCH 2/9] mac80211: limit the A-MSDU Tx based on peer's capabilities

[Grumbach, Emmanuel]

On 12/08/2015 07:49 PM, Krishna Chaitanya wrote:
>
>
> On Dec 8, 2015 10:13 PM, "Grumbach, Emmanuel"
> <emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>> wrote:
> >
> >
> >
> > On 12/08/2015 06:35 PM, Krishna Chaitanya wrote:
> > >
> > >
> > > On Dec 8, 2015 10:01 PM, "Grumbach, Emmanuel"
> > > <emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>
> <mailto:emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>>> wrote:
> > > >
> > > >
> > > >
> > > > On 12/08/2015 06:03 PM, Krishna Chaitanya wrote:
> > > > > On Tue, Dec 8, 2015 at 7:34 PM, Emmanuel Grumbach
> > > > > <emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>
> <mailto:emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>>>
> > > wrote:
> > > > >> index 7a76ce6..f4a5287 100644
> > > > >> --- a/net/mac80211/ht.c
> > > > >> +++ b/net/mac80211/ht.c
> > > > >> @@ -230,6 +230,11 @@ bool
> > > ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data *sdata,
> > > > >>         /* set Rx highest rate */
> > > > >>         ht_cap.mcs.rx_highest = ht_cap_ie->mcs.rx_highest;
> > > > >>
> > > > >> +       if (ht_cap.cap & IEEE80211_HT_CAP_MAX_AMSDU)
> > > > >> +               sta->sta.max_amsdu_len =
> > > IEEE80211_MAX_MPDU_LEN_HT_7935;
> > > > >> +       else
> > > > >> +               sta->sta.max_amsdu_len =
> > > IEEE80211_MAX_MPDU_LEN_HT_3839;
> > > > >> +
> > > > >>   apply:
> > > > >>         changed = memcmp(&sta->sta.ht_cap, &ht_cap,
> sizeof(ht_cap));
> > > > >>
> > > > >> diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
> > > > >> index 92c9843..d2f2ff6 100644
> > > > >> --- a/net/mac80211/vht.c
> > > > >> +++ b/net/mac80211/vht.c
> > > > >> @@ -281,6 +281,24 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct
> > > ieee80211_sub_if_data *sdata,
> > > > >>         }
> > > > >>
> > > > >>         sta->sta.bandwidth = ieee80211_sta_cur_vht_bw(sta);
> > > > >> +
> > > > >> +       /* If HT IE reported 3839 bytes only, stay with that
> size. */
> > > > >> +       if (sta->sta.max_amsdu_len ==
> IEEE80211_MAX_MPDU_LEN_HT_3839)
> > > > >> +               return;
> > > > >> +
> > > > >> +       switch (vht_cap->cap & IEEE80211_VHT_CAP_MAX_MPDU_MASK) {
> > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454:
> > > > >> +               sta->sta.max_amsdu_len =
> > > IEEE80211_MAX_MPDU_LEN_VHT_11454;
> > > > >> +               break;
> > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991:
> > > > >> +               sta->sta.max_amsdu_len =
> > > IEEE80211_MAX_MPDU_LEN_VHT_7991;
> > > > >> +               break;
> > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895:
> > > > >> +       default:
> > > > >> +               sta->sta.max_amsdu_len =
> > > IEEE80211_MAX_MPDU_LEN_VHT_3895;
> > > > >> +               break;
> > > > >> +       }
> > > > > Ideally speaking shouldn't we use different variables for HT
> and VHT
> > > > > and depending on
> > > > > rate HT/VHT we should aggregate accordingly? of course that
> will be
> > > > > tricky as we dont
> > > > > have the rate control info at the time of aggregation. Standard is
> > > > > clear about usage of
> > > > > independent lengths depending on whether its an VHT/HT PPDU.
> > > >
> > > > Yes - but since it is tricky, I preferred to be on the safe side and
> > > > limit VHT amsdus for the very peculiar AP that would decide to have
> > > > large A-MSDUs in VHT and small ones in HT (?!).
> > > Yes but wouldn't it be safer to just use min(ht , vht)? For a good AP
> > > it shouldn't matter and bad AP it will still work.
> > >
> > This is the de-facto what this code does I think.
> > If the HT limit is 4K, then don't even take the VHT limit into account
> > and the VHT limit can't be smaller than the HT one in that case.
> > If the HT limit is 8K, then we can limit it further to 4K in case VHT
> > has a limit of 4K (which is another weird case), but we can also make it
> > larger for VHT frames?
> >
> > So I don't really see the bug here, am I missing something?
> If we take a maximum case HT=8K and VHT=12K then we endup sending 12K
> frames using HT rates... Which is not expected...
>
Right, but it is explicitly mentioned in the API that if you use an HT
preamble you should limit the A-MSDU to 8K.
If the HT limit was to be 4K, then the VHT would be 4K as well (even if
a broken peer would advertise 8K in HT and 4K in VHT).

> > > > > If we use the same variable to arrive at max_amsdu_len for
> both VHT
> > > > > and HT, we might
> > > > > end up sending 11454 sized MSDU in a HT rate.
> > > > >
> > > > And since that can't be handled at the time of the A-MSDU building,
> > > > leave that to another entity :)
> > >
> >
>

Re: [PATCH 2/9] mac80211: limit the A-MSDU Tx based on peer's capabilities

[Grumbach, Emmanuel]

On 12/08/2015 09:11 PM, Krishna Chaitanya wrote:
>
>
> On Dec 9, 2015 12:37 AM, "Grumbach, Emmanuel"
> <emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>> wrote:
> >
> >
> >
> > On 12/08/2015 07:49 PM, Krishna Chaitanya wrote:
> > >
> > >
> > > On Dec 8, 2015 10:13 PM, "Grumbach, Emmanuel"
> > > <emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>
> <mailto:emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>>> wrote:
> > > >
> > > >
> > > >
> > > > On 12/08/2015 06:35 PM, Krishna Chaitanya wrote:
> > > > >
> > > > >
> > > > > On Dec 8, 2015 10:01 PM, "Grumbach, Emmanuel"
> > > > > <emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>
> <mailto:emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>>
> > > <mailto:emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>
> > > <mailto:emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>>>> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 12/08/2015 06:03 PM, Krishna Chaitanya wrote:
> > > > > > > On Tue, Dec 8, 2015 at 7:34 PM, Emmanuel Grumbach
> > > > > > > <emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>
> > > <mailto:emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>>
> > > <mailto:emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>
> <mailto:emmanuel.grumbach@intel.com
> <mailto:emmanuel.grumbach@intel.com>>>>
> > > > > wrote:
> > > > > > >> index 7a76ce6..f4a5287 100644
> > > > > > >> --- a/net/mac80211/ht.c
> > > > > > >> +++ b/net/mac80211/ht.c
> > > > > > >> @@ -230,6 +230,11 @@ bool
> > > > > ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data
> *sdata,
> > > > > > >>         /* set Rx highest rate */
> > > > > > >>         ht_cap.mcs.rx_highest = ht_cap_ie->mcs.rx_highest;
> > > > > > >>
> > > > > > >> +       if (ht_cap.cap & IEEE80211_HT_CAP_MAX_AMSDU)
> > > > > > >> +               sta->sta.max_amsdu_len =
> > > > > IEEE80211_MAX_MPDU_LEN_HT_7935;
> > > > > > >> +       else
> > > > > > >> +               sta->sta.max_amsdu_len =
> > > > > IEEE80211_MAX_MPDU_LEN_HT_3839;
> > > > > > >> +
> > > > > > >>   apply:
> > > > > > >>         changed = memcmp(&sta->sta.ht_cap, &ht_cap,
> > > sizeof(ht_cap));
> > > > > > >>
> > > > > > >> diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
> > > > > > >> index 92c9843..d2f2ff6 100644
> > > > > > >> --- a/net/mac80211/vht.c
> > > > > > >> +++ b/net/mac80211/vht.c
> > > > > > >> @@ -281,6 +281,24 @@
> ieee80211_vht_cap_ie_to_sta_vht_cap(struct
> > > > > ieee80211_sub_if_data *sdata,
> > > > > > >>         }
> > > > > > >>
> > > > > > >>         sta->sta.bandwidth = ieee80211_sta_cur_vht_bw(sta);
> > > > > > >> +
> > > > > > >> +       /* If HT IE reported 3839 bytes only, stay with that
> > > size. */
> > > > > > >> +       if (sta->sta.max_amsdu_len ==
> > > IEEE80211_MAX_MPDU_LEN_HT_3839)
> > > > > > >> +               return;
> > > > > > >> +
> > > > > > >> +       switch (vht_cap->cap &
> IEEE80211_VHT_CAP_MAX_MPDU_MASK) {
> > > > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454:
> > > > > > >> +               sta->sta.max_amsdu_len =
> > > > > IEEE80211_MAX_MPDU_LEN_VHT_11454;
> > > > > > >> +               break;
> > > > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991:
> > > > > > >> +               sta->sta.max_amsdu_len =
> > > > > IEEE80211_MAX_MPDU_LEN_VHT_7991;
> > > > > > >> +               break;
> > > > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895:
> > > > > > >> +       default:
> > > > > > >> +               sta->sta.max_amsdu_len =
> > > > > IEEE80211_MAX_MPDU_LEN_VHT_3895;
> > > > > > >> +               break;
> > > > > > >> +       }
> > > > > > > Ideally speaking shouldn't we use different variables for HT
> > > and VHT
> > > > > > > and depending on
> > > > > > > rate HT/VHT we should aggregate accordingly? of course that
> > > will be
> > > > > > > tricky as we dont
> > > > > > > have the rate control info at the time of aggregation.
> Standard is
> > > > > > > clear about usage of
> > > > > > > independent lengths depending on whether its an VHT/HT PPDU.
> > > > > >
> > > > > > Yes - but since it is tricky, I preferred to be on the safe
> side and
> > > > > > limit VHT amsdus for the very peculiar AP that would decide
> to have
> > > > > > large A-MSDUs in VHT and small ones in HT (?!).
> > > > > Yes but wouldn't it be safer to just use min(ht , vht)? For a
> good AP
> > > > > it shouldn't matter and bad AP it will still work.
> > > > >
> > > > This is the de-facto what this code does I think.
> > > > If the HT limit is 4K, then don't even take the VHT limit into
> account
> > > > and the VHT limit can't be smaller than the HT one in that case.
> > > > If the HT limit is 8K, then we can limit it further to 4K in
> case VHT
> > > > has a limit of 4K (which is another weird case), but we can also
> make it
> > > > larger for VHT frames?
> > > >
> > > > So I don't really see the bug here, am I missing something?
> > > If we take a maximum case HT=8K and VHT=12K then we endup sending 12K
> > > frames using HT rates... Which is not expected...
> > >
> > Right, but it is explicitly mentioned in the API that if you use an HT
> > preamble you should limit the A-MSDU to 8K.
> > If the HT limit was to be 4K, then the VHT would be 4K as well (even if
> > a broken peer would advertise 8K in HT and 4K in VHT).
> OK I did not read the documentation ;-) yes its clear now but I still
> think min() should make things easier rather than driver taking care
> of this.
>
Not really... You'd need another variable since min() would limit the
AMSDU length to the smallest (HT realistically) limit.
And as you pointed out, typically, the Tx MPDU and its rate are decided
/ built in two different layers. In iwlwifi we plan to be using VHT
preamble only when we have a VHT association (and forbid AMSDU when the
rates drop below a TBD limit).

> > > > > > > If we use the same variable to arrive at max_amsdu_len for
> > > both VHT
> > > > > > > and HT, we might
> > > > > > > end up sending 11454 sized MSDU in a HT rate.
> > > > > > >
> > > > > > And since that can't be handled at the time of the A-MSDU
> building,
> > > > > > leave that to another entity :)
> > > > >
> > > >
> > >
> >
>

Re: [PATCH 2/9] mac80211: limit the A-MSDU Tx based on peer's capabilities

[Krishna Chaitanya]

On Wed, Dec 9, 2015 at 12:52 AM, Grumbach, Emmanuel
<emmanuel.grumbach@intel.com> wrote:
>
>
> On 12/08/2015 09:11 PM, Krishna Chaitanya wrote:
>>
>>
>> On Dec 9, 2015 12:37 AM, "Grumbach, Emmanuel"
>> <emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>> wrote:
>> >
>> >
>> >
>> > On 12/08/2015 07:49 PM, Krishna Chaitanya wrote:
>> > >
>> > >
>> > > On Dec 8, 2015 10:13 PM, "Grumbach, Emmanuel"
>> > > <emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>
>> <mailto:emmanuel.grumbach@intel.com
>> <mailto:emmanuel.grumbach@intel.com>>> wrote:
>> > > >
>> > > >
>> > > >
>> > > > On 12/08/2015 06:35 PM, Krishna Chaitanya wrote:
>> > > > >
>> > > > >
>> > > > > On Dec 8, 2015 10:01 PM, "Grumbach, Emmanuel"
>> > > > > <emmanuel.grumbach@intel.com
>> <mailto:emmanuel.grumbach@intel.com>
>> <mailto:emmanuel.grumbach@intel.com <mailto:emmanuel.grumbach@intel.com>>
>> > > <mailto:emmanuel.grumbach@intel.com
>> <mailto:emmanuel.grumbach@intel.com>
>> > > <mailto:emmanuel.grumbach@intel.com
>> <mailto:emmanuel.grumbach@intel.com>>>> wrote:
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > On 12/08/2015 06:03 PM, Krishna Chaitanya wrote:
>> > > > > > > On Tue, Dec 8, 2015 at 7:34 PM, Emmanuel Grumbach
>> > > > > > > <emmanuel.grumbach@intel.com
>> <mailto:emmanuel.grumbach@intel.com>
>> > > <mailto:emmanuel.grumbach@intel.com
>> <mailto:emmanuel.grumbach@intel.com>>
>> > > <mailto:emmanuel.grumbach@intel.com
>> <mailto:emmanuel.grumbach@intel.com>
>> <mailto:emmanuel.grumbach@intel.com
>> <mailto:emmanuel.grumbach@intel.com>>>>
>> > > > > wrote:
>> > > > > > >> index 7a76ce6..f4a5287 100644
>> > > > > > >> --- a/net/mac80211/ht.c
>> > > > > > >> +++ b/net/mac80211/ht.c
>> > > > > > >> @@ -230,6 +230,11 @@ bool
>> > > > > ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data
>> *sdata,
>> > > > > > >>         /* set Rx highest rate */
>> > > > > > >>         ht_cap.mcs.rx_highest = ht_cap_ie->mcs.rx_highest;
>> > > > > > >>
>> > > > > > >> +       if (ht_cap.cap & IEEE80211_HT_CAP_MAX_AMSDU)
>> > > > > > >> +               sta->sta.max_amsdu_len =
>> > > > > IEEE80211_MAX_MPDU_LEN_HT_7935;
>> > > > > > >> +       else
>> > > > > > >> +               sta->sta.max_amsdu_len =
>> > > > > IEEE80211_MAX_MPDU_LEN_HT_3839;
>> > > > > > >> +
>> > > > > > >>   apply:
>> > > > > > >>         changed = memcmp(&sta->sta.ht_cap, &ht_cap,
>> > > sizeof(ht_cap));
>> > > > > > >>
>> > > > > > >> diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
>> > > > > > >> index 92c9843..d2f2ff6 100644
>> > > > > > >> --- a/net/mac80211/vht.c
>> > > > > > >> +++ b/net/mac80211/vht.c
>> > > > > > >> @@ -281,6 +281,24 @@
>> ieee80211_vht_cap_ie_to_sta_vht_cap(struct
>> > > > > ieee80211_sub_if_data *sdata,
>> > > > > > >>         }
>> > > > > > >>
>> > > > > > >>         sta->sta.bandwidth = ieee80211_sta_cur_vht_bw(sta);
>> > > > > > >> +
>> > > > > > >> +       /* If HT IE reported 3839 bytes only, stay with that
>> > > size. */
>> > > > > > >> +       if (sta->sta.max_amsdu_len ==
>> > > IEEE80211_MAX_MPDU_LEN_HT_3839)
>> > > > > > >> +               return;
>> > > > > > >> +
>> > > > > > >> +       switch (vht_cap->cap &
>> IEEE80211_VHT_CAP_MAX_MPDU_MASK) {
>> > > > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454:
>> > > > > > >> +               sta->sta.max_amsdu_len =
>> > > > > IEEE80211_MAX_MPDU_LEN_VHT_11454;
>> > > > > > >> +               break;
>> > > > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991:
>> > > > > > >> +               sta->sta.max_amsdu_len =
>> > > > > IEEE80211_MAX_MPDU_LEN_VHT_7991;
>> > > > > > >> +               break;
>> > > > > > >> +       case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895:
>> > > > > > >> +       default:
>> > > > > > >> +               sta->sta.max_amsdu_len =
>> > > > > IEEE80211_MAX_MPDU_LEN_VHT_3895;
>> > > > > > >> +               break;
>> > > > > > >> +       }
>> > > > > > > Ideally speaking shouldn't we use different variables for HT
>> > > and VHT
>> > > > > > > and depending on
>> > > > > > > rate HT/VHT we should aggregate accordingly? of course that
>> > > will be
>> > > > > > > tricky as we dont
>> > > > > > > have the rate control info at the time of aggregation.
>> Standard is
>> > > > > > > clear about usage of
>> > > > > > > independent lengths depending on whether its an VHT/HT PPDU.
>> > > > > >
>> > > > > > Yes - but since it is tricky, I preferred to be on the safe
>> side and
>> > > > > > limit VHT amsdus for the very peculiar AP that would decide
>> to have
>> > > > > > large A-MSDUs in VHT and small ones in HT (?!).
>> > > > > Yes but wouldn't it be safer to just use min(ht , vht)? For a
>> good AP
>> > > > > it shouldn't matter and bad AP it will still work.
>> > > > >
>> > > > This is the de-facto what this code does I think.
>> > > > If the HT limit is 4K, then don't even take the VHT limit into
>> account
>> > > > and the VHT limit can't be smaller than the HT one in that case.
>> > > > If the HT limit is 8K, then we can limit it further to 4K in
>> case VHT
>> > > > has a limit of 4K (which is another weird case), but we can also
>> make it
>> > > > larger for VHT frames?
>> > > >
>> > > > So I don't really see the bug here, am I missing something?
>> > > If we take a maximum case HT=8K and VHT=12K then we endup sending 12K
>> > > frames using HT rates... Which is not expected...
>> > >
>> > Right, but it is explicitly mentioned in the API that if you use an HT
>> > preamble you should limit the A-MSDU to 8K.
>> > If the HT limit was to be 4K, then the VHT would be 4K as well (even if
>> > a broken peer would advertise 8K in HT and 4K in VHT).
>> OK I did not read the documentation ;-) yes its clear now but I still
>> think min() should make things easier rather than driver taking care
>> of this.
>>
> Not really... You'd need another variable since min() would limit the
> AMSDU length to the smallest (HT realistically) limit.
Yes.
> And as you pointed out, typically, the Tx MPDU and its rate are decided
> / built in two different layers. In iwlwifi we plan to be using VHT
> preamble only when we have a VHT association (and forbid AMSDU when the
> rates drop below a TBD limit).
Ok, that makes sense, no point in using AMSDU if the rates are dropping.

[PATCH 3/9] mac80211: add flag for duplication check

[Emmanuel Grumbach]

From: Sara Sharon <sara.sharon@intel.com>

Add an option for driver to check for packet duplication
by itself.
This is needed for example by the iwlwifi driver which
parallelizes the RX path and does the duplcation check
per queue.

Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 include/net/mac80211.h | 2 +-
 net/mac80211/rx.c      | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 71be30d..f4cbad4 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1063,7 +1063,7 @@ enum mac80211_rx_flags {
 	RX_FLAG_HT_GF			= BIT(13),
 	RX_FLAG_AMPDU_DETAILS		= BIT(14),
 	RX_FLAG_PN_VALIDATED		= BIT(15),
-	/* bit 16 free */
+	RX_FLAG_DUP_VALIDATED		= BIT(16),
 	RX_FLAG_AMPDU_LAST_KNOWN	= BIT(17),
 	RX_FLAG_AMPDU_IS_LAST		= BIT(18),
 	RX_FLAG_AMPDU_DELIM_CRC_ERROR	= BIT(19),
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index c4198d48..901c72b 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1099,6 +1099,9 @@ ieee80211_rx_h_check_dup(struct ieee80211_rx_data *rx)
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)rx->skb->data;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(rx->skb);
 
+	if (status->flag & RX_FLAG_DUP_VALIDATED)
+		return RX_CONTINUE;
+
 	/*
 	 * Drop duplicate 802.11 retransmissions
 	 * (IEEE 802.11-2012: 9.3.2.10 "Duplicate detection and recovery")
-- 
2.5.0

[PATCH 4/9] mac80211: pass aggregation window size to lower level

[Emmanuel Grumbach]

From: Sara Sharon <sara.sharon@intel.com>

Currently mac80211 does not inform the driver of the window
size when starting a rx aggregation session.
Following patches will enable managing the reorder buffer in
the driver or hardware, and the window size is needed.

Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 include/net/mac80211.h | 8 +++++---
 net/mac80211/agg-rx.c  | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f4cbad4..dca010a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3061,9 +3061,11 @@ enum ieee80211_reconfig_type {
  * 	ieee80211_ampdu_mlme_action. Starting sequence number (@ssn)
  * 	is the first frame we expect to perform the action on. Notice
  * 	that TX/RX_STOP can pass NULL for this parameter.
- *	The @buf_size parameter is only valid when the action is set to
- *	%IEEE80211_AMPDU_TX_OPERATIONAL and indicates the peer's reorder
- *	buffer size (number of subframes) for this session -- the driver
+ *	The @buf_size parameter is valid only when the action is set to
+ *	%IEEE80211_AMPDU_RX_START or %IEEE80211_AMPDU_TX_OPERATIONAL and
+ *	indicates the reorder buffer size (number of subframes) for this
+ *	session.
+ *	When the action is set to %IEEE80211_AMPDU_TX_OPERATIONAL the driver
  *	may neither send aggregates containing more subframes than this
  *	nor send aggregates in a way that lost frames would exceed the
  *	buffer size. If just limiting the aggregate size, this would be
diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 10ad4ac..7867273 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -323,7 +323,7 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
 		__skb_queue_head_init(&tid_agg_rx->reorder_buf[i]);
 
 	ret = drv_ampdu_action(local, sta->sdata, IEEE80211_AMPDU_RX_START,
-			       &sta->sta, tid, &start_seq_num, 0, false);
+			       &sta->sta, tid, &start_seq_num, buf_size, false);
 	ht_dbg(sta->sdata, "Rx A-MPDU request on %pM tid %d result %d\n",
 	       sta->sta.addr, tid, ret);
 	if (ret) {
-- 
2.5.0

[PATCH 5/9] mac80211: document status.freq restrictions

[Emmanuel Grumbach]

From: Johannes Berg <johannes.berg@intel.com>

It's not always necessary to set the status.freq field, for example
when this would be an expensive calculation. It must be set for all
management frames (as they might be reported to userspace), but for
data frames it's not really required. Document this.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/mac80211.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index dca010a..54af948 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1108,6 +1108,8 @@ enum mac80211_rx_vht_flags {
  *	it but can store it and pass it back to the driver for synchronisation
  * @band: the active band when this frame was received
  * @freq: frequency the radio was tuned to when receiving this frame, in MHz
+ *	This field must be set for management frames, but isn't strictly needed
+ *	for data (other) frames - for those it only affects radiotap reporting.
  * @signal: signal strength when receiving this frame, either in dBm, in dB or
  *	unspecified depending on the hardware capabilities flags
  *	@IEEE80211_HW_SIGNAL_*
-- 
2.5.0

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

[Emmanuel Grumbach]

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);
-- 
2.5.0

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

[Krishna Chaitanya]

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)

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

[Johannes Berg]

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

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

[Krishna Chaitanya]

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.

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

[Johannes Berg]

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

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

[Krishna Chaitanya]

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.

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

[Johannes Berg]

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

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

[Krishna Chaitanya]

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 :-).

[PATCH 7/9] mac80211: suppress unchanged "limiting TX power" messages

[Emmanuel Grumbach]

From: Johannes Berg <johannes.berg@intel.com>

When the AP is advertising limited TX power, the message can be
printed over and over again. Suppress it when the power level
isn't changing.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=106011

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/mlme.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 62c8e4f..31d5881 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1379,21 +1379,26 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 	 */
 	if (has_80211h_pwr &&
 	    (!has_cisco_pwr || pwr_level_80211h <= pwr_level_cisco)) {
+		new_ap_level = pwr_level_80211h;
+
+		if (sdata->ap_power_level == new_ap_level)
+			return 0;
+
 		sdata_dbg(sdata,
 			  "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
 			  pwr_level_80211h, chan_pwr, pwr_reduction_80211h,
 			  sdata->u.mgd.bssid);
-		new_ap_level = pwr_level_80211h;
 	} else {  /* has_cisco_pwr is always true here. */
+		new_ap_level = pwr_level_cisco;
+
+		if (sdata->ap_power_level == new_ap_level)
+			return 0;
+
 		sdata_dbg(sdata,
 			  "Limiting TX power to %d dBm as advertised by %pM\n",
 			  pwr_level_cisco, sdata->u.mgd.bssid);
-		new_ap_level = pwr_level_cisco;
 	}
 
-	if (sdata->ap_power_level == new_ap_level)
-		return 0;
-
 	sdata->ap_power_level = new_ap_level;
 	if (__ieee80211_recalc_txpower(sdata))
 		return BSS_CHANGED_TXPOWER;
-- 
2.5.0

[PATCH 8/9] mac80211: run scan completed work on reconfig failure

[Emmanuel Grumbach]

From: Johannes Berg <johannes.berg@intel.com>

When reconfiguration during resume fails while a scan is pending
for completion work, that work will never run, and the scan will
be stuck forever. Factor out the code to recover this and call it
also in ieee80211_handle_reconfig_failure().

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/util.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 4d6130b..1a26ebd 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1644,6 +1644,29 @@ void ieee80211_stop_device(struct ieee80211_local *local)
 	drv_stop(local);
 }
 
+static void ieee80211_flush_completed_scan(struct ieee80211_local *local,
+					   bool aborted)
+{
+	/* It's possible that we don't handle the scan completion in
+	 * time during suspend, so if it's still marked as completed
+	 * here, queue the work and flush it to clean things up.
+	 * Instead of calling the worker function directly here, we
+	 * really queue it to avoid potential races with other flows
+	 * scheduling the same work.
+	 */
+	if (test_bit(SCAN_COMPLETED, &local->scanning)) {
+		/* If coming from reconfiguration failure, abort the scan so
+		 * we don't attempt to continue a partial HW scan - which is
+		 * possible otherwise if (e.g.) the 2.4 GHz portion was the
+		 * completed scan, and a 5 GHz portion is still pending.
+		 */
+		if (aborted)
+			set_bit(SCAN_ABORTED, &local->scanning);
+		ieee80211_queue_delayed_work(&local->hw, &local->scan_work, 0);
+		flush_delayed_work(&local->scan_work);
+	}
+}
+
 static void ieee80211_handle_reconfig_failure(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
@@ -1663,6 +1686,8 @@ static void ieee80211_handle_reconfig_failure(struct ieee80211_local *local)
 	local->suspended = false;
 	local->in_reconfig = false;
 
+	ieee80211_flush_completed_scan(local, true);
+
 	/* scheduled scan clearly can't be running any more, but tell
 	 * cfg80211 and clear local state
 	 */
@@ -2080,17 +2105,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 	mb();
 	local->resuming = false;
 
-	/* It's possible that we don't handle the scan completion in
-	 * time during suspend, so if it's still marked as completed
-	 * here, queue the work and flush it to clean things up.
-	 * Instead of calling the worker function directly here, we
-	 * really queue it to avoid potential races with other flows
-	 * scheduling the same work.
-	 */
-	if (test_bit(SCAN_COMPLETED, &local->scanning)) {
-		ieee80211_queue_delayed_work(&local->hw, &local->scan_work, 0);
-		flush_delayed_work(&local->scan_work);
-	}
+	ieee80211_flush_completed_scan(local, false);
 
 	if (local->open_count && !reconfig_due_to_wowlan)
 		drv_reconfig_complete(local, IEEE80211_RECONFIG_TYPE_SUSPEND);
-- 
2.5.0

[PATCH 9/9] mac80211: reprogram in interface order

[Emmanuel Grumbach]

From: Johannes Berg <johannes.berg@intel.com>

During reprogramming, mac80211 currently first adds all the channel
contexts, then binds them to the vifs and then goes to reconfigure
all the interfaces. Drivers might, perhaps implicitly, rely on the
operation order for certain things that typically happen within a
single function elsewhere in mac80211. To avoid problems with that,
reorder the code in mac80211's restart/reprogramming to work fully
within the interface loop so that the order of operations is like
in normal operation.

For iwlwifi, this fixes a firmware crash when reprogramming with an
AP/GO interface active.

Reported-by: David Spinadel <david.spinadel@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/mac80211/util.c | 76 ++++++++++++++++++++++++++---------------------------
 1 file changed, 37 insertions(+), 39 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 1a26ebd..f4b2c04 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1726,6 +1726,27 @@ static void ieee80211_assign_chanctx(struct ieee80211_local *local,
 	mutex_unlock(&local->chanctx_mtx);
 }
 
+static void ieee80211_reconfig_stations(struct ieee80211_sub_if_data *sdata)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct sta_info *sta;
+
+	/* add STAs back */
+	mutex_lock(&local->sta_mtx);
+	list_for_each_entry(sta, &local->sta_list, list) {
+		enum ieee80211_sta_state state;
+
+		if (!sta->uploaded || sta->sdata != sdata)
+			continue;
+
+		for (state = IEEE80211_STA_NOTEXIST;
+		     state < sta->sta_state; state++)
+			WARN_ON(drv_sta_state(local, sta->sdata, sta, state,
+					      state + 1));
+	}
+	mutex_unlock(&local->sta_mtx);
+}
+
 int ieee80211_reconfig(struct ieee80211_local *local)
 {
 	struct ieee80211_hw *hw = &local->hw;
@@ -1861,50 +1882,11 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 				WARN_ON(drv_add_chanctx(local, ctx));
 		mutex_unlock(&local->chanctx_mtx);
 
-		list_for_each_entry(sdata, &local->interfaces, list) {
-			if (!ieee80211_sdata_running(sdata))
-				continue;
-			ieee80211_assign_chanctx(local, sdata);
-		}
-
 		sdata = rtnl_dereference(local->monitor_sdata);
 		if (sdata && ieee80211_sdata_running(sdata))
 			ieee80211_assign_chanctx(local, sdata);
 	}
 
-	/* add STAs back */
-	mutex_lock(&local->sta_mtx);
-	list_for_each_entry(sta, &local->sta_list, list) {
-		enum ieee80211_sta_state state;
-
-		if (!sta->uploaded)
-			continue;
-
-		/* AP-mode stations will be added later */
-		if (sta->sdata->vif.type == NL80211_IFTYPE_AP)
-			continue;
-
-		for (state = IEEE80211_STA_NOTEXIST;
-		     state < sta->sta_state; state++)
-			WARN_ON(drv_sta_state(local, sta->sdata, sta, state,
-					      state + 1));
-	}
-	mutex_unlock(&local->sta_mtx);
-
-	/* reconfigure tx conf */
-	if (hw->queues >= IEEE80211_NUM_ACS) {
-		list_for_each_entry(sdata, &local->interfaces, list) {
-			if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
-			    sdata->vif.type == NL80211_IFTYPE_MONITOR ||
-			    !ieee80211_sdata_running(sdata))
-				continue;
-
-			for (i = 0; i < IEEE80211_NUM_ACS; i++)
-				drv_conf_tx(local, sdata, i,
-					    &sdata->tx_conf[i]);
-		}
-	}
-
 	/* reconfigure hardware */
 	ieee80211_hw_config(local, ~0);
 
@@ -1917,6 +1899,22 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 		if (!ieee80211_sdata_running(sdata))
 			continue;
 
+		ieee80211_assign_chanctx(local, sdata);
+
+		switch (sdata->vif.type) {
+		case NL80211_IFTYPE_AP_VLAN:
+		case NL80211_IFTYPE_MONITOR:
+			break;
+		default:
+			ieee80211_reconfig_stations(sdata);
+			/* fall through */
+		case NL80211_IFTYPE_AP: /* AP stations are handled later */
+			for (i = 0; i < IEEE80211_NUM_ACS; i++)
+				drv_conf_tx(local, sdata, i,
+					    &sdata->tx_conf[i]);
+			break;
+		}
+
 		/* common change flags for all interface types */
 		changed = BSS_CHANGED_ERP_CTS_PROT |
 			  BSS_CHANGED_ERP_PREAMBLE |
-- 
2.5.0