Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: KASAN+netlink, was: [PATCH] [net-next?] hns: avoid stack overflow with CONFIG_KASAN
From: Johannes Berg @ 2017-02-08 12:24 UTC (permalink / raw)
  To: Arnd Bergmann, David Miller, netdev
  Cc: stable, linux-kernel, Andrey Ryabinin, nikolay, nicolas.dichtel,
	adobriyan, linux-wireless
In-Reply-To: <4910112.l20yySyWnA@wuerfel>

On Wed, 2017-02-08 at 13:03 +0100, Arnd Bergmann wrote:
> 
> - Moving nla_put_{u8,u16,u32} out of line is probably uncontroversial
> and
>   it helps enough with br_netlink.c, but nl820211 is worse and needs
> some
>   additional fiddling.

Why would that not be sufficient by itself for nl80211?

Btw, what's causing this to start with? Can't the compiler reuse the
stack places?

johannes

^ permalink raw reply

* [PATCH v3] iw: Add support for controlling tx power for per station
From: Ashok Raj Nagarajan @ 2017-02-08 12:01 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, ath10k, arnagara, Ashok Raj Nagarajan

This patch allows userspace to set transmit power, in mBm units, to a
station associated to the AP.

To set a limit tx power of 2000 mBm:
iw wlan0 station set <mac-addr> txpwr limit 2000

To revert the user defined tx power for a station:
iw wlan0 station set <mac-addr> txpwr auto

Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com>
---
v3:
	- rebased

 station.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/station.c b/station.c
index f3e3da8..8364593 100644
--- a/station.c
+++ b/station.c
@@ -569,6 +569,7 @@ COMMAND(station, del, "<MAC address> [subtype <subtype>] [reason-code <code>]",
 static const struct cmd *station_set_plink;
 static const struct cmd *station_set_vlan;
 static const struct cmd *station_set_mesh_power_mode;
+static const struct cmd *station_set_txpwr;
 
 static const struct cmd *select_station_cmd(int argc, char **argv)
 {
@@ -580,6 +581,8 @@ static const struct cmd *select_station_cmd(int argc, char **argv)
 		return station_set_vlan;
 	if (strcmp(argv[1], "mesh_power_mode") == 0)
 		return station_set_mesh_power_mode;
+	if (strcmp(argv[1], "txpwr") == 0)
+		return station_set_txpwr;
 	return NULL;
 }
 
@@ -731,6 +734,72 @@ COMMAND_ALIAS(station, set, "<MAC address> mesh_power_mode "
 	"Set link-specific mesh power mode for this station",
 	select_station_cmd, station_set_mesh_power_mode);
 
+static int handle_station_set_txpwr(struct nl80211_state *state,
+				    struct nl_msg *msg,
+				    int argc, char **argv,
+				    enum id_input id)
+{
+	enum nl80211_tx_power_setting type;
+	unsigned char mac_addr[ETH_ALEN];
+	unsigned int sta_txpwr = 0;
+	char *err = NULL;
+
+	if (argc != 3 && argc != 4)
+		return 1;
+
+	if (mac_addr_a2n(mac_addr, argv[0])) {
+		fprintf(stderr, "invalid mac address\n");
+		return 2;
+	}
+
+	NLA_PUT(msg, NL80211_ATTR_MAC, ETH_ALEN, mac_addr);
+	argc--;
+	argv++;
+
+	if (strcmp("txpwr", argv[0]) != 0)
+		return 1;
+	argc--;
+	argv++;
+
+	if (!strcmp(argv[0], "auto"))
+		type = NL80211_TX_POWER_AUTOMATIC;
+	else if (!strcmp(argv[0], "limit"))
+		type = NL80211_TX_POWER_LIMITED;
+	else {
+		printf("Invalid parameter: %s\n", argv[0]);
+		return 2;
+	}
+
+	NLA_PUT_U32(msg, NL80211_ATTR_STA_TX_POWER_SETTING, type);
+
+	if (type != NL80211_TX_POWER_AUTOMATIC) {
+		if (argc != 2) {
+			printf("Missing TX power level argument.\n");
+			return 2;
+		}
+
+		argc--;
+		argv++;
+
+		sta_txpwr = strtoul(argv[0], &err, 0);
+		NLA_PUT_U32(msg, NL80211_ATTR_STA_TX_POWER, sta_txpwr);
+	}
+
+	argc--;
+	argv++;
+
+	if (argc)
+		return 1;
+
+	return 0;
+ nla_put_failure:
+	return -ENOBUFS;
+}
+COMMAND_ALIAS(station, set, "<MAC address> txpwr <auto|limit> [<tx power mBm>]",
+	NL80211_CMD_SET_STATION, 0, CIB_NETDEV, handle_station_set_txpwr,
+	"Set Tx power for this station.",
+	select_station_cmd, station_set_txpwr);
+
 static int handle_station_dump(struct nl80211_state *state,
 			       struct nl_msg *msg,
 			       int argc, char **argv,
-- 
1.9.1

^ permalink raw reply related

* [PATCH v3] ath10k: add support for controlling tx power to a station
From: Ashok Raj Nagarajan @ 2017-02-08 11:58 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, arnagara, Ashok Raj Nagarajan

This patch will add the support to control the transmit power for traffic
to a station associated with the AP. Userspace provide the transmit power
value in mBm units. ath10k firmware expects the value to be in dBm and
hence will convert the value from to dBm before passing down to firmware.

Underlying FW will enforce that the maximum tx power will be based on the
regulatory requirements. If the user given transmit power is greater than
the allowed tx power in the given channel, then the FW will use the maximum
tx power in the same channel.

When 0 is sent to the FW as tx power, it will revert to the automatic tx
power for the station.

Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com>
---
v3:
	- reword commit log
	- remove range check for the input from user. (Ben Greear)

 drivers/net/wireless/ath/ath10k/mac.c | 31 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h |  1 +
 2 files changed, 32 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 9977829..3b91468 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5985,6 +5985,32 @@ static int ath10k_mac_tdls_vifs_count(struct ieee80211_hw *hw)
 	return num_tdls_vifs;
 }
 
+static int ath10k_sta_set_txpwr(struct ieee80211_hw *hw,
+				struct ieee80211_vif *vif,
+				struct ieee80211_sta *sta)
+{
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+	int ret = 0;
+	u16 txpwr;
+
+	txpwr = MBM_TO_DBM(sta->txpwr);
+
+	mutex_lock(&ar->conf_mutex);
+
+	ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+					WMI_PEER_USE_FIXED_PWR, txpwr);
+	if (ret) {
+		ath10k_warn(ar, "failed to set tx power for station ret: %d\n",
+			    ret);
+		goto out;
+	}
+
+out:
+	mutex_unlock(&ar->conf_mutex);
+	return ret;
+}
+
 static int ath10k_sta_state(struct ieee80211_hw *hw,
 			    struct ieee80211_vif *vif,
 			    struct ieee80211_sta *sta,
@@ -7550,6 +7576,7 @@ static void ath10k_mac_op_sta_pre_rcu_remove(struct ieee80211_hw *hw,
 	.set_key			= ath10k_set_key,
 	.set_default_unicast_key        = ath10k_set_default_unicast_key,
 	.sta_state			= ath10k_sta_state,
+	.sta_set_txpwr			= ath10k_sta_set_txpwr,
 	.conf_tx			= ath10k_conf_tx,
 	.remain_on_channel		= ath10k_remain_on_channel,
 	.cancel_remain_on_channel	= ath10k_cancel_remain_on_channel,
@@ -8193,11 +8220,15 @@ int ath10k_mac_register(struct ath10k *ar)
 		ar->hw->wiphy->iface_combinations = ath10k_10x_if_comb;
 		ar->hw->wiphy->n_iface_combinations =
 			ARRAY_SIZE(ath10k_10x_if_comb);
+		wiphy_ext_feature_set(ar->hw->wiphy,
+				      NL80211_EXT_FEATURE_STA_TX_PWR);
 		break;
 	case ATH10K_FW_WMI_OP_VERSION_10_4:
 		ar->hw->wiphy->iface_combinations = ath10k_10_4_if_comb;
 		ar->hw->wiphy->n_iface_combinations =
 			ARRAY_SIZE(ath10k_10_4_if_comb);
+		wiphy_ext_feature_set(ar->hw->wiphy,
+				      NL80211_EXT_FEATURE_STA_TX_PWR);
 		break;
 	case ATH10K_FW_WMI_OP_VERSION_UNSET:
 	case ATH10K_FW_WMI_OP_VERSION_MAX:
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 861c2d8..1ccb6bf 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -5811,6 +5811,7 @@ enum wmi_peer_param {
 	WMI_PEER_CHAN_WIDTH = 0x4,
 	WMI_PEER_NSS        = 0x5,
 	WMI_PEER_USE_4ADDR  = 0x6,
+	WMI_PEER_USE_FIXED_PWR = 0x8,
 	WMI_PEER_DUMMY_VAR  = 0xff, /* dummy parameter for STA PS workaround */
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v3 2/2] mac80211: store tx power value from user to station
From: Ashok Raj Nagarajan @ 2017-02-08 11:56 UTC (permalink / raw)
  To: linux-wireless; +Cc: ath10k, johannes, arnagara, Ashok Raj Nagarajan

This patch introduce a new driver callback drv_sta_set_txpwr. This API will
copy the transmit power value passed from user space and call the driver
callback to set the tx power for the station.

Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com>
---
v3:
	- Store txpwr in mBm units. Push down the converstions at the driver
	  level (Ben Greear)
 include/net/mac80211.h    |  6 ++++++
 net/mac80211/cfg.c        |  8 ++++++++
 net/mac80211/driver-ops.c | 21 +++++++++++++++++++++
 net/mac80211/driver-ops.h |  5 +++++
 net/mac80211/trace.h      | 27 +++++++++++++++++++++++++++
 5 files changed, 67 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 5345d35..49d48f3 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1777,6 +1777,8 @@ struct ieee80211_sta_rates {
  *	This is defined by the spec (IEEE 802.11-2012 section 8.3.2.2 NOTE 2).
  * @support_p2p_ps: indicates whether the STA supports P2P PS mechanism or not.
  * @max_rc_amsdu_len: Maximum A-MSDU size in bytes recommended by rate control.
+ * @txpwr: indicates the tx power, in mBm, to be used when sending data frames
+ *	to the STA. Value of 0 means, automatic (default) tx power.
  * @txq: per-TID data TX queues (if driver uses the TXQ abstraction)
  */
 struct ieee80211_sta {
@@ -1800,6 +1802,7 @@ struct ieee80211_sta {
 	u16 max_amsdu_len;
 	bool support_p2p_ps;
 	u16 max_rc_amsdu_len;
+	u16 txpwr;
 
 	struct ieee80211_txq *txq[IEEE80211_NUM_TIDS];
 
@@ -3545,6 +3548,9 @@ struct ieee80211_ops {
 #endif
 	void (*sta_notify)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			enum sta_notify_cmd, struct ieee80211_sta *sta);
+	int (*sta_set_txpwr)(struct ieee80211_hw *hw,
+			     struct ieee80211_vif *vif,
+			     struct ieee80211_sta *sta);
 	int (*sta_state)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			 struct ieee80211_sta *sta,
 			 enum ieee80211_sta_state old_state,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index e91e503..0084258 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1346,6 +1346,14 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 	if (params->listen_interval >= 0)
 		sta->listen_interval = params->listen_interval;
 
+	if (params->sta_modify_mask & STATION_PARAM_APPLY_STA_TXPOWER &&
+	    params->txpwr >= 0) {
+		sta->sta.txpwr = params->txpwr;
+		ret = drv_sta_set_txpwr(local, sdata, sta);
+		if (ret)
+			return ret;
+	}
+
 	if (params->supported_rates) {
 		ieee80211_parse_bitrates(&sdata->vif.bss_conf.chandef,
 					 sband, params->supported_rates,
diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index bb886e7..839c002 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -138,6 +138,27 @@ int drv_sta_state(struct ieee80211_local *local,
 	return ret;
 }
 
+__must_check
+int drv_sta_set_txpwr(struct ieee80211_local *local,
+		      struct ieee80211_sub_if_data *sdata,
+		      struct sta_info *sta)
+{
+	int ret = -EOPNOTSUPP;
+
+	might_sleep();
+
+	sdata = get_bss_sdata(sdata);
+	if (!check_sdata_in_driver(sdata))
+		return -EIO;
+
+	trace_drv_sta_set_txpwr(local, sdata, &sta->sta);
+	if (local->ops->sta_set_txpwr)
+		ret = local->ops->sta_set_txpwr(&local->hw, &sdata->vif,
+						&sta->sta);
+	trace_drv_return_int(local, ret);
+	return ret;
+}
+
 void drv_sta_rc_update(struct ieee80211_local *local,
 		       struct ieee80211_sub_if_data *sdata,
 		       struct ieee80211_sta *sta, u32 changed)
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 09f77e4..2b13c39 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -526,6 +526,11 @@ int drv_sta_state(struct ieee80211_local *local,
 		  enum ieee80211_sta_state old_state,
 		  enum ieee80211_sta_state new_state);
 
+__must_check
+int drv_sta_set_txpwr(struct ieee80211_local *local,
+		      struct ieee80211_sub_if_data *sdata,
+		      struct sta_info *sta);
+
 void drv_sta_rc_update(struct ieee80211_local *local,
 		       struct ieee80211_sub_if_data *sdata,
 		       struct ieee80211_sta *sta, u32 changed);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 92a47af..3c505c1 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -823,6 +823,33 @@
 	)
 );
 
+TRACE_EVENT(drv_sta_set_txpwr,
+	TP_PROTO(struct ieee80211_local *local,
+		 struct ieee80211_sub_if_data *sdata,
+		 struct ieee80211_sta *sta),
+
+	TP_ARGS(local, sdata, sta),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		VIF_ENTRY
+		STA_ENTRY
+		__field(u16, txpwr)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		VIF_ASSIGN;
+		STA_ASSIGN;
+		__entry->txpwr = sta->txpwr;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT  VIF_PR_FMT  STA_PR_FMT " txpwr: %d",
+		LOCAL_PR_ARG, VIF_PR_ARG, STA_PR_ARG, __entry->txpwr
+	)
+);
+
 TRACE_EVENT(drv_sta_rc_update,
 	TP_PROTO(struct ieee80211_local *local,
 		 struct ieee80211_sub_if_data *sdata,
-- 
1.9.1

^ permalink raw reply related

* [PATCH v3 1/2] cfg80211: Add support to set tx power for a station associated
From: Ashok Raj Nagarajan @ 2017-02-08 11:56 UTC (permalink / raw)
  To: linux-wireless; +Cc: ath10k, johannes, arnagara, Ashok Raj Nagarajan

This patch adds support to set transmit power setting type and transmit
power level attributes to NL80211_CMD_SET_STATION in order to facilitate
adjusting the transmit power level of a station associated to the AP.

The added attributes allow selection of automatic and limited transmit
power level, with the level defined in mBm format.

Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com>
---
v2:
	- refactor the shared code
	- "keep what you had" using STATION_PARAM_APPLY_* BIT
	- feature flag to let the user know if this feature is supported or not (Johannes)
v3:
	- fix the limitation of changing the txpwr after association (Ben Greear)

 include/net/cfg80211.h       |  6 ++++++
 include/uapi/linux/nl80211.h | 15 ++++++++++++++
 net/wireless/nl80211.c       | 48 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 814be4b..661e0ea 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -808,6 +808,7 @@ enum station_parameters_apply_mask {
 	STATION_PARAM_APPLY_UAPSD = BIT(0),
 	STATION_PARAM_APPLY_CAPABILITY = BIT(1),
 	STATION_PARAM_APPLY_PLINK_STATE = BIT(2),
+	STATION_PARAM_APPLY_STA_TXPOWER = BIT(3),
 };
 
 /**
@@ -849,6 +850,10 @@ enum station_parameters_apply_mask {
  * @opmode_notif: operating mode field from Operating Mode Notification
  * @opmode_notif_used: information if operating mode field is used
  * @support_p2p_ps: information if station supports P2P PS mechanism
+ * @txpwr: tx power (in mBm) to be used for sending data traffic. If tx power
+ *	is not provided, the default per-interface tx power setting will be
+ *	overriding. Driver should be picking up the lowest tx power, either tx
+ *	power per-interface or per-station.
  */
 struct station_parameters {
 	const u8 *supported_rates;
@@ -876,6 +881,7 @@ struct station_parameters {
 	u8 opmode_notif;
 	bool opmode_notif_used;
 	int support_p2p_ps;
+	u16 txpwr;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6b76e3b..de2f72c 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1980,6 +1980,15 @@ enum nl80211_commands {
  * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
  *	used in various commands/events for specifying the BSSID.
  *
+ * @NL80211_ATTR_STA_TX_POWER_SETTING: Transmit power setting type (u32) for
+ *	station associated with the AP. See &enum nl80211_tx_power_setting for
+ *	possible values.
+ * @NL80211_ATTR_STA_TX_POWER: Transmit power level (u32) in mBm units. This
+ *	allows to set Tx power for a station. If this attribute is not included,
+ *	the default per-interface tx power setting will be overriding. Driver
+ *	should be picking up the lowest tx power, either tx power per-interface
+ *	or per-station.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2386,6 +2395,9 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_BSSID,
 
+	NL80211_ATTR_STA_TX_POWER_SETTING,
+	NL80211_ATTR_STA_TX_POWER,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -4697,6 +4709,8 @@ enum nl80211_feature_flags {
  *	configuration (AP/mesh) with VHT rates.
  * @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial Link Setup
  *	with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
+ * @NL80211_EXT_FEATURE_STA_TX_PWR: This driver supports controlling tx power
+ *	to a station.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4712,6 +4726,7 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_BEACON_RATE_HT,
 	NL80211_EXT_FEATURE_BEACON_RATE_VHT,
 	NL80211_EXT_FEATURE_FILS_STA,
+	NL80211_EXT_FEATURE_STA_TX_PWR,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ef5eff93..701c08a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -246,6 +246,8 @@ enum nl80211_multicast_groups {
 	[NL80211_ATTR_STA_SUPPORTED_RATES] = { .type = NLA_BINARY,
 					       .len = NL80211_MAX_SUPP_RATES },
 	[NL80211_ATTR_STA_PLINK_ACTION] = { .type = NLA_U8 },
+	[NL80211_ATTR_STA_TX_POWER_SETTING] = { .type = NLA_U32 },
+	[NL80211_ATTR_STA_TX_POWER] = { .type = NLA_U32 },
 	[NL80211_ATTR_STA_VLAN] = { .type = NLA_U32 },
 	[NL80211_ATTR_MNTR_FLAGS] = { /* NLA_NESTED can't be empty */ },
 	[NL80211_ATTR_MESH_ID] = { .type = NLA_BINARY,
@@ -4755,6 +4757,40 @@ static int nl80211_set_station_tdls(struct genl_info *info,
 	return nl80211_parse_sta_wme(info, params);
 }
 
+static int nl80211_parse_sta_txpower_setting(struct genl_info *info,
+					     struct station_parameters *params)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	enum nl80211_tx_power_setting type;
+	int idx;
+
+	if (!rdev->ops->set_tx_power ||
+	    !wiphy_ext_feature_isset(&rdev->wiphy,
+				     NL80211_EXT_FEATURE_STA_TX_PWR))
+		return -EOPNOTSUPP;
+
+	idx = NL80211_ATTR_STA_TX_POWER_SETTING;
+	type = nla_get_u32(info->attrs[idx]);
+
+	if (!info->attrs[NL80211_ATTR_STA_TX_POWER] &&
+	    (type != NL80211_TX_POWER_AUTOMATIC))
+		return -EINVAL;
+
+	if (type != NL80211_TX_POWER_AUTOMATIC) {
+		if (type == NL80211_TX_POWER_LIMITED) {
+			idx = NL80211_ATTR_STA_TX_POWER;
+			params->txpwr = nla_get_u32(info->attrs[idx]);
+		} else {
+			return -EINVAL;
+		}
+	} else {
+		params->txpwr = 0;
+	}
+	params->sta_modify_mask |= STATION_PARAM_APPLY_STA_TXPOWER;
+
+	return 0;
+}
+
 static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -4854,6 +4890,12 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
 		params.local_pm = pm;
 	}
 
+	if (info->attrs[NL80211_ATTR_STA_TX_POWER_SETTING]) {
+		err = nl80211_parse_sta_txpower_setting(info, &params);
+		if (err)
+			return err;
+	}
+
 	/* Include parameters for TDLS peer (will check later) */
 	err = nl80211_set_station_tdls(info, &params);
 	if (err)
@@ -4981,6 +5023,12 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 			return -EINVAL;
 	}
 
+	if (info->attrs[NL80211_ATTR_STA_TX_POWER_SETTING]) {
+		err = nl80211_parse_sta_txpower_setting(info, &params);
+		if (err)
+			return err;
+	}
+
 	err = nl80211_parse_sta_channel_info(info, &params);
 	if (err)
 		return err;
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v4 3/5] cfg80211: Accept multiple RSSI thresholds for CQM
From: Andrew Zaborowski @ 2017-02-08 11:26 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1486547933.24745.2.camel@sipsolutions.net>

Hi,

On 8 February 2017 at 10:58, Johannes Berg <johannes@sipsolutions.net> wrote:
>
>> This method doesn't have a hysteresis parameter because there's no
>> benefit to the cfg80211 code from having the hysteresis be handled by
>> hardware/driver in terms of the number of wakeups.  At the same time
>> it would likely be less consistent between drivers if offloaded or
>> done in the drivers.
>
> I'm not really sure I buy this.
>
> What if I configure a few ranges, and let's say one of the boundaries
> is -50dBm. Now if I sit just on that value of -50dBm and thus my signal
> fluctuates say -48..-52, I'd have to continuously wake up the host.
>
> You try to avoid that here, I think:
>
> +       if (low > (s32) (last - hyst))
> +               low = last - hyst;
> +       if (high < (s32) (last + hyst))
> +               high = last + hyst;
>
> but it's not clear to me that this is effective?
>
> Let's see. last is -52, so low will be -60 and high will be -50.
>
> Let's say hyst is 3 since I chose the ranges so closely. I'm guessing a
> higher hysteresis and larger ranges would actually be better, but let's
> stick to this for the sake of the example.
>
> last-hyst = -55, so low isn't > that, low = -60
> last+hyst = -49, so high = -49
>
> but now it still fluctuates around -50, so if I next hit -48 you do the
> same dance again with -50/-40, getting -51/-40 and never really
> applying a full hysteresis, no?
>
> I'll probably see an intermediate value of -50 at some point, but I'll
> never actually *report* it, so "last" can switch between -48 and -52
> constantly in this scenario, no?

Yes, sounds like this could happen.

>
>
> I think it would make sense to unconditionally apply the hysteresis to
> low/high, i.e. always set
>  low = low - hyst
>  high = high + hyst
>
> so that you get "sticky" ranges once you're in them?

Yes, maybe that's better, I guess I want to avoid just adding a lag /
delay in reporting changes that are not due to measurement error or
temporary.  Could also do something in between, e.g. use "low - hyst"
if signal is close to low, otherwise just "low".

The question is how the current hysteresis parameter is defined, what
is expected of the firmware and how do driver authors decide whether
their firmware/hardware implements the same mechanism as expected by
the kernel.  Would the same thing happen with firmware implementations
if the hysteresis value is 3 and the rssi fluctuates by +/- 2?  Or
would it have to be +/- 3 or +/- 4 before this starts to happen.  (If
this isn't well specified then it makes more sense to try to do it in
one place for consistency.)

>
>
>
>> +     if (!wiphy_ext_feature_isset(&rdev->wiphy,
>> +
>>            NL80211_EXT_FEATURE_CQM_RSSI_LIST))
>> +             return
>> -EOPNOTSUPP;
>
>
> That check should be earlier in the function
> cfg80211_cqm_rssi_update(), no?

Oh looks like it can actually be removed in the current code because
nl80211_set_cqm_rssi already has this check and wdev->cqm_config can't
be set outside that function.

Best regards

^ permalink raw reply

* [PATCH 2/2] rt2x00usb: fix anchor initialization
From: Stanislaw Gruszka @ 2017-02-08 11:18 UTC (permalink / raw)
  To: linux-wireless; +Cc: Helmut Schaa, Vishal Thanki
In-Reply-To: <1486552690-27597-1-git-send-email-sgruszka@redhat.com>

If device fail to initialize we can OOPS in rt2x00lib_remove_dev(), due
to using uninitialized usb_anchor structure:

[  855.435820] ieee80211 phy3: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x1000 with error -19
[  855.435826] ieee80211 phy3: rt2800_probe_rt: Error - Invalid RT chipset 0x0000, rev 0000 detected
[  855.435829] ieee80211 phy3: rt2x00lib_probe_dev: Error - Failed to allocate device
[  855.435845] BUG: unable to handle kernel NULL pointer dereference at 0000000000000028
[  855.435900] IP: _raw_spin_lock_irq+0xd/0x30
[  855.435926] PGD 0
[  855.435953] Oops: 0002 [#1] SMP
<snip>
[  855.437011] Call Trace:
[  855.437029]  ? usb_kill_anchored_urbs+0x27/0xc0
[  855.437061]  rt2x00lib_remove_dev+0x190/0x1c0 [rt2x00lib]
[  855.437097]  rt2x00lib_probe_dev+0x246/0x7a0 [rt2x00lib]
[  855.437149]  ? ieee80211_roc_setup+0x9e/0xd0 [mac80211]
[  855.437183]  ? __kmalloc+0x1af/0x1f0
[  855.437207]  ? rt2x00usb_probe+0x13d/0xc50 [rt2x00usb]
[  855.437240]  rt2x00usb_probe+0x155/0xc50 [rt2x00usb]
[  855.437273]  rt2800usb_probe+0x15/0x20 [rt2800usb]
[  855.437304]  usb_probe_interface+0x159/0x2d0
[  855.437333]  driver_probe_device+0x2bb/0x460

Patch changes initialization sequence to fix the problem.

Cc: Vishal Thanki <vishalthanki@gmail.com>
Fixes: 8b4c0009313f ("rt2x00usb: Use usb anchor to manage URB")
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2x00usb.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
index fe13dd0..c696f0a 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
@@ -825,10 +825,6 @@ int rt2x00usb_probe(struct usb_interface *usb_intf,
 	if (retval)
 		goto exit_free_device;
 
-	retval = rt2x00lib_probe_dev(rt2x00dev);
-	if (retval)
-		goto exit_free_reg;
-
 	rt2x00dev->anchor = devm_kmalloc(&usb_dev->dev,
 					sizeof(struct usb_anchor),
 					GFP_KERNEL);
@@ -836,10 +832,17 @@ int rt2x00usb_probe(struct usb_interface *usb_intf,
 		retval = -ENOMEM;
 		goto exit_free_reg;
 	}
-
 	init_usb_anchor(rt2x00dev->anchor);
+
+	retval = rt2x00lib_probe_dev(rt2x00dev);
+	if (retval)
+		goto exit_free_anchor;
+
 	return 0;
 
+exit_free_anchor:
+	usb_kill_anchored_urbs(rt2x00dev->anchor);
+
 exit_free_reg:
 	rt2x00usb_free_reg(rt2x00dev);
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 1/2] rt2x00usb: do not anchor rx and tx urb's
From: Stanislaw Gruszka @ 2017-02-08 11:18 UTC (permalink / raw)
  To: linux-wireless; +Cc: Helmut Schaa, Vishal Thanki

We might kill TX or RX urb during rt2x00usb_flush_entry(), what can
cause anchor list corruption like shown below:

[ 2074.035633] WARNING: CPU: 2 PID: 14480 at lib/list_debug.c:33 __list_add+0xac/0xc0
[ 2074.035634] list_add corruption. prev->next should be next (ffff88020f362c28), but was dead000000000100. (prev=ffff8801d161bb70).
<snip>
[ 2074.035670] Call Trace:
[ 2074.035672]  [<ffffffff813bde47>] dump_stack+0x63/0x8c
[ 2074.035674]  [<ffffffff810a2231>] __warn+0xd1/0xf0
[ 2074.035676]  [<ffffffff810a22af>] warn_slowpath_fmt+0x5f/0x80
[ 2074.035678]  [<ffffffffa073855d>] ? rt2x00usb_register_write_lock+0x3d/0x60 [rt2800usb]
[ 2074.035679]  [<ffffffff813dbe4c>] __list_add+0xac/0xc0
[ 2074.035681]  [<ffffffff81591c6c>] usb_anchor_urb+0x4c/0xa0
[ 2074.035683]  [<ffffffffa07322af>] rt2x00usb_kick_rx_entry+0xaf/0x100 [rt2x00usb]
[ 2074.035684]  [<ffffffffa0732322>] rt2x00usb_clear_entry+0x22/0x30 [rt2x00usb]

To fix do not anchor TX and RX urb's, it is not needed as during
shutdown we kill those urbs in rt2x00usb_free_entries().

Cc: Vishal Thanki <vishalthanki@gmail.com>
Fixes: 8b4c0009313f ("rt2x00usb: Use usb anchor to manage URB")
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2x00usb.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
index 5a2bf9f..fe13dd0 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
@@ -319,10 +319,8 @@ static bool rt2x00usb_kick_tx_entry(struct queue_entry *entry, void *data)
 			  entry->skb->data, length,
 			  rt2x00usb_interrupt_txdone, entry);
 
-	usb_anchor_urb(entry_priv->urb, rt2x00dev->anchor);
 	status = usb_submit_urb(entry_priv->urb, GFP_ATOMIC);
 	if (status) {
-		usb_unanchor_urb(entry_priv->urb);
 		if (status == -ENODEV)
 			clear_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
 		set_bit(ENTRY_DATA_IO_FAILED, &entry->flags);
@@ -410,10 +408,8 @@ static bool rt2x00usb_kick_rx_entry(struct queue_entry *entry, void *data)
 			  entry->skb->data, entry->skb->len,
 			  rt2x00usb_interrupt_rxdone, entry);
 
-	usb_anchor_urb(entry_priv->urb, rt2x00dev->anchor);
 	status = usb_submit_urb(entry_priv->urb, GFP_ATOMIC);
 	if (status) {
-		usb_unanchor_urb(entry_priv->urb);
 		if (status == -ENODEV)
 			clear_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
 		set_bit(ENTRY_DATA_IO_FAILED, &entry->flags);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 05/17] iwlwifi: mvm: fix a print of NSS for HT rate
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Gregory Greenman, Luca Coelho
In-Reply-To: <20170208112322.29413-1-luca@coelho.fi>

From: Gregory Greenman <gregory.greenman@intel.com>

Handling of the number of space time streams was missing for HT rate in
rate printing function. Fix it.

Signed-off-by: Gregory Greenman <gregory.greenman@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index 80f99c365b6a..13be9a5b83ee 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -3616,6 +3616,8 @@ int rs_pretty_print_rate(char *buf, const u32 rate)
 	} else if (rate & RATE_MCS_HT_MSK) {
 		type = "HT";
 		mcs = rate & RATE_HT_MCS_INDEX_MSK;
+		nss = ((rate & RATE_HT_MCS_NSS_MSK)
+		       >> RATE_HT_MCS_NSS_POS) + 1;
 	} else {
 		type = "Unknown"; /* shouldn't happen */
 	}
-- 
2.11.0

^ permalink raw reply related

* [PATCH 07/17] iwlwifi: mvm: fix reorder timer re-arming
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Sara Sharon, Luca Coelho
In-Reply-To: <20170208112322.29413-1-luca@coelho.fi>

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

When NSSN is behind the reorder buffer due to timeout
the reorder timer isn't getting re-armed until NSSN
catches up. Fix it.

Fixes: 0690405fef29 ("iwlwifi: mvm: add reorder timeout per frame")

Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index c154ab42c80d..d79e9c2a2654 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -418,7 +418,7 @@ static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
 
 	/* ignore nssn smaller than head sn - this can happen due to timeout */
 	if (iwl_mvm_is_sn_less(nssn, ssn, reorder_buf->buf_size))
-		return;
+		goto set_timer;
 
 	while (iwl_mvm_is_sn_less(ssn, nssn, reorder_buf->buf_size)) {
 		int index = ssn % reorder_buf->buf_size;
@@ -441,6 +441,7 @@ static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
 	}
 	reorder_buf->head_sn = nssn;
 
+set_timer:
 	if (reorder_buf->num_stored && !reorder_buf->removed) {
 		u16 index = reorder_buf->head_sn % reorder_buf->buf_size;
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 03/17] iwlwifi: pcie: re-configure IVAR table after suspend-resume
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Haim Dreyfuss, Luca Coelho
In-Reply-To: <20170208112322.29413-1-luca@coelho.fi>

From: Haim Dreyfuss <haim.dreyfuss@intel.com>

During the suspend/resume flow some HW blocks are reset.  This causes
the IVAR table to be completely erased.  This table is where interrupt
causes are bound to specific IRQs.  When the table is empty the
interrupt handlers are not called correctly.  Fix this by reconfiguring
the IVAR table after resume.

Fixes: 2e5d4a8f61dc ("iwlwifi: pcie: Add new configuration to enable MSIX")
Signed-off-by: Haim Dreyfuss <haim.dreyfuss@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index e7a26f594386..bb2a9a151957 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1152,13 +1152,19 @@ static void iwl_pcie_conf_msix_hw(struct iwl_trans_pcie *trans_pcie)
 	struct iwl_trans *trans = trans_pcie->trans;
 
 	if (!trans_pcie->msix_enabled) {
-		if (trans->cfg->mq_rx_supported)
+		if (trans->cfg->mq_rx_supported &&
+		    test_bit(STATUS_DEVICE_ENABLED, &trans->status))
 			iwl_write_prph(trans, UREG_CHICK,
 				       UREG_CHICK_MSI_ENABLE);
 		return;
 	}
-
-	iwl_write_prph(trans, UREG_CHICK, UREG_CHICK_MSIX_ENABLE);
+	/*
+	 * The IVAR table needs to be configured again after reset,
+	 * but if the device is disabled, we can't write to
+	 * prph.
+	 */
+	if (test_bit(STATUS_DEVICE_ENABLED, &trans->status))
+		iwl_write_prph(trans, UREG_CHICK, UREG_CHICK_MSIX_ENABLE);
 
 	/*
 	 * Each cause from the causes list above and the RX causes is
@@ -1457,6 +1463,7 @@ static int iwl_trans_pcie_d3_resume(struct iwl_trans *trans,
 				    enum iwl_d3_status *status,
 				    bool test,  bool reset)
 {
+	struct iwl_trans_pcie *trans_pcie =  IWL_TRANS_GET_PCIE_TRANS(trans);
 	u32 val;
 	int ret;
 
@@ -1469,11 +1476,15 @@ static int iwl_trans_pcie_d3_resume(struct iwl_trans *trans,
 	iwl_pcie_enable_rx_wake(trans, true);
 
 	/*
-	 * Also enables interrupts - none will happen as the device doesn't
-	 * know we're waking it up, only when the opmode actually tells it
-	 * after this call.
+	 * Reconfigure IVAR table in case of MSIX or reset ict table in
+	 * MSI mode since HW reset erased it.
+	 * Also enables interrupts - none will happen as
+	 * the device doesn't know we're waking it up, only when
+	 * the opmode actually tells it after this call.
 	 */
-	iwl_pcie_reset_ict(trans);
+	iwl_pcie_conf_msix_hw(trans_pcie);
+	if (!trans_pcie->msix_enabled)
+		iwl_pcie_reset_ict(trans);
 	iwl_enable_interrupts(trans);
 
 	iwl_set_bit(trans, CSR_GP_CNTRL, CSR_GP_CNTRL_REG_FLAG_MAC_ACCESS_REQ);
-- 
2.11.0

^ permalink raw reply related

* [PATCH 08/17] iwlwifi: mvm: use the PROBE_RESP_QUEUE to send deauth to unknown station
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20170208112322.29413-1-luca@coelho.fi>

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

When we send a deauth to a station we don't know about, we
need to use the PROBE_RESP queue. This can happen when we
send a deauth to a station that is not associated to us.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/tx.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index 1d147599cca9..dd2b4a300819 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -506,15 +506,17 @@ static int iwl_mvm_get_ctrl_vif_queue(struct iwl_mvm *mvm,
 	switch (info->control.vif->type) {
 	case NL80211_IFTYPE_AP:
 		/*
-		 * handle legacy hostapd as well, where station may be added
-		 * only after assoc.
+		 * Handle legacy hostapd as well, where station may be added
+		 * only after assoc. Take care of the case where we send a
+		 * deauth to a station that we don't have.
 		 */
-		if (ieee80211_is_probe_resp(fc) || ieee80211_is_auth(fc))
+		if (ieee80211_is_probe_resp(fc) || ieee80211_is_auth(fc) ||
+		    ieee80211_is_deauth(fc))
 			return IWL_MVM_DQA_AP_PROBE_RESP_QUEUE;
 		if (info->hw_queue == info->control.vif->cab_queue)
 			return info->hw_queue;
 
-		WARN_ON_ONCE(1);
+		WARN_ONCE(1, "fc=0x%02x", le16_to_cpu(fc));
 		return IWL_MVM_DQA_AP_PROBE_RESP_QUEUE;
 	case NL80211_IFTYPE_P2P_DEVICE:
 		if (ieee80211_is_mgmt(fc))
-- 
2.11.0

^ permalink raw reply related

* [PATCH 06/17] iwlwifi: mvm: fix references to first_agg_queue in DQA mode
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Sara Sharon, Luca Coelho
In-Reply-To: <20170208112322.29413-1-luca@coelho.fi>

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

In DQA mode, first_agg_queue is initialized to
IWL_MVM_DQA_MIN_DATA_QUEUE. This causes two bugs in the tx response
flow:

1. When TX fails, we set IEEE80211_TX_STAT_AMPDU_NO_BACK regardless
   if we actually have aggregation open on the queue. This causes
   mac80211 to send a BAR frame even though there is no aggregation
   open.
   Fix that by simply checking the AMPDU flag that is set on by
   mac80211 for AMPDU packets.

2. When reclaiming frames in aggregation mode, we reclaim based on
   scheduler ssn and not the SN.
   The reason is that scheduler ssn may be ahead of SN due to a hole
   in the BA window that was filled.
   However, if we have aggregations open on IWL_MVM_DQA_BSS_CLIENT_QUEUE
   the reclaim flow will still go to the code of non-aggregation
   instead of the aggregation code since IWL_MVM_DQA_BSS_CLIENT_QUEUE
   is smaller than IWL_MVM_DQA_MIN_DATA_QUEUE, although it is a valid
   aggregation queue.
   Fix that by always using the aggregation reclaim code by default in
   DQA mode (currently it is implicitly used by default for all queues
   except the reserved BSS queue).

Fixes: cf961e16620f ("iwlwifi: mvm: support dqa-mode agg on non-shared queue")
Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/tx.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index 4ba639eda7a3..1d147599cca9 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -1274,8 +1274,6 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
 
 		memset(&info->status, 0, sizeof(info->status));
 
-		info->flags &= ~IEEE80211_TX_CTL_AMPDU;
-
 		/* inform mac80211 about what happened with the frame */
 		switch (status & TX_STATUS_MSK) {
 		case TX_STATUS_SUCCESS:
@@ -1298,10 +1296,11 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
 			(void *)(uintptr_t)le32_to_cpu(tx_resp->initial_rate);
 
 		/* Single frame failure in an AMPDU queue => send BAR */
-		if (txq_id >= mvm->first_agg_queue &&
+		if (info->flags & IEEE80211_TX_CTL_AMPDU &&
 		    !(info->flags & IEEE80211_TX_STAT_ACK) &&
 		    !(info->flags & IEEE80211_TX_STAT_TX_FILTERED))
 			info->flags |= IEEE80211_TX_STAT_AMPDU_NO_BACK;
+		info->flags &= ~IEEE80211_TX_CTL_AMPDU;
 
 		/* W/A FW bug: seq_ctl is wrong when the status isn't success */
 		if (status != TX_STATUS_SUCCESS) {
@@ -1336,7 +1335,7 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
 		ieee80211_tx_status(mvm->hw, skb);
 	}
 
-	if (txq_id >= mvm->first_agg_queue) {
+	if (iwl_mvm_is_dqa_supported(mvm) || txq_id >= mvm->first_agg_queue) {
 		/* If this is an aggregation queue, we use the ssn since:
 		 * ssn = wifi seq_num % 256.
 		 * The seq_ctl is the sequence control of the packet to which
-- 
2.11.0

^ permalink raw reply related

* [PATCH 01/17] iwlwifi: pcie: move msix conf functions above other functions
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Haim Dreyfuss, Luca Coelho
In-Reply-To: <20170208112322.29413-1-luca@coelho.fi>

From: Haim Dreyfuss <haim.dreyfuss@intel.com>

msix configuration functions should be called by other functions.
For example by pcie_d3_resume, move it above to enable it.

Signed-off-by: Haim Dreyfuss <haim.dreyfuss@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 206 ++++++++++++------------
 1 file changed, 103 insertions(+), 103 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index b28d99f61a35..f795ebea4c4a 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1076,6 +1076,109 @@ static bool iwl_trans_check_hw_rf_kill(struct iwl_trans *trans)
 	return hw_rfkill;
 }
 
+struct iwl_causes_list {
+	u32 cause_num;
+	u32 mask_reg;
+	u8 addr;
+};
+
+static struct iwl_causes_list causes_list[] = {
+	{MSIX_FH_INT_CAUSES_D2S_CH0_NUM,	CSR_MSIX_FH_INT_MASK_AD, 0},
+	{MSIX_FH_INT_CAUSES_D2S_CH1_NUM,	CSR_MSIX_FH_INT_MASK_AD, 0x1},
+	{MSIX_FH_INT_CAUSES_S2D,		CSR_MSIX_FH_INT_MASK_AD, 0x3},
+	{MSIX_FH_INT_CAUSES_FH_ERR,		CSR_MSIX_FH_INT_MASK_AD, 0x5},
+	{MSIX_HW_INT_CAUSES_REG_ALIVE,		CSR_MSIX_HW_INT_MASK_AD, 0x10},
+	{MSIX_HW_INT_CAUSES_REG_WAKEUP,		CSR_MSIX_HW_INT_MASK_AD, 0x11},
+	{MSIX_HW_INT_CAUSES_REG_CT_KILL,	CSR_MSIX_HW_INT_MASK_AD, 0x16},
+	{MSIX_HW_INT_CAUSES_REG_RF_KILL,	CSR_MSIX_HW_INT_MASK_AD, 0x17},
+	{MSIX_HW_INT_CAUSES_REG_PERIODIC,	CSR_MSIX_HW_INT_MASK_AD, 0x18},
+	{MSIX_HW_INT_CAUSES_REG_SW_ERR,		CSR_MSIX_HW_INT_MASK_AD, 0x29},
+	{MSIX_HW_INT_CAUSES_REG_SCD,		CSR_MSIX_HW_INT_MASK_AD, 0x2A},
+	{MSIX_HW_INT_CAUSES_REG_FH_TX,		CSR_MSIX_HW_INT_MASK_AD, 0x2B},
+	{MSIX_HW_INT_CAUSES_REG_HW_ERR,		CSR_MSIX_HW_INT_MASK_AD, 0x2D},
+	{MSIX_HW_INT_CAUSES_REG_HAP,		CSR_MSIX_HW_INT_MASK_AD, 0x2E},
+};
+
+static void iwl_pcie_map_non_rx_causes(struct iwl_trans *trans)
+{
+	struct iwl_trans_pcie *trans_pcie =  IWL_TRANS_GET_PCIE_TRANS(trans);
+	int val = trans_pcie->def_irq | MSIX_NON_AUTO_CLEAR_CAUSE;
+	int i;
+
+	/*
+	 * Access all non RX causes and map them to the default irq.
+	 * In case we are missing at least one interrupt vector,
+	 * the first interrupt vector will serve non-RX and FBQ causes.
+	 */
+	for (i = 0; i < ARRAY_SIZE(causes_list); i++) {
+		iwl_write8(trans, CSR_MSIX_IVAR(causes_list[i].addr), val);
+		iwl_clear_bit(trans, causes_list[i].mask_reg,
+			      causes_list[i].cause_num);
+	}
+}
+
+static void iwl_pcie_map_rx_causes(struct iwl_trans *trans)
+{
+	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
+	u32 offset =
+		trans_pcie->shared_vec_mask & IWL_SHARED_IRQ_FIRST_RSS ? 1 : 0;
+	u32 val, idx;
+
+	/*
+	 * The first RX queue - fallback queue, which is designated for
+	 * management frame, command responses etc, is always mapped to the
+	 * first interrupt vector. The other RX queues are mapped to
+	 * the other (N - 2) interrupt vectors.
+	 */
+	val = BIT(MSIX_FH_INT_CAUSES_Q(0));
+	for (idx = 1; idx < trans->num_rx_queues; idx++) {
+		iwl_write8(trans, CSR_MSIX_RX_IVAR(idx),
+			   MSIX_FH_INT_CAUSES_Q(idx - offset));
+		val |= BIT(MSIX_FH_INT_CAUSES_Q(idx));
+	}
+	iwl_write32(trans, CSR_MSIX_FH_INT_MASK_AD, ~val);
+
+	val = MSIX_FH_INT_CAUSES_Q(0);
+	if (trans_pcie->shared_vec_mask & IWL_SHARED_IRQ_NON_RX)
+		val |= MSIX_NON_AUTO_CLEAR_CAUSE;
+	iwl_write8(trans, CSR_MSIX_RX_IVAR(0), val);
+
+	if (trans_pcie->shared_vec_mask & IWL_SHARED_IRQ_FIRST_RSS)
+		iwl_write8(trans, CSR_MSIX_RX_IVAR(1), val);
+}
+
+static void iwl_pcie_init_msix(struct iwl_trans_pcie *trans_pcie)
+{
+	struct iwl_trans *trans = trans_pcie->trans;
+
+	if (!trans_pcie->msix_enabled) {
+		if (trans->cfg->mq_rx_supported)
+			iwl_write_prph(trans, UREG_CHICK,
+				       UREG_CHICK_MSI_ENABLE);
+		return;
+	}
+
+	iwl_write_prph(trans, UREG_CHICK, UREG_CHICK_MSIX_ENABLE);
+
+	/*
+	 * Each cause from the causes list above and the RX causes is
+	 * represented as a byte in the IVAR table. The first nibble
+	 * represents the bound interrupt vector of the cause, the second
+	 * represents no auto clear for this cause. This will be set if its
+	 * interrupt vector is bound to serve other causes.
+	 */
+	iwl_pcie_map_rx_causes(trans);
+
+	iwl_pcie_map_non_rx_causes(trans);
+
+	trans_pcie->fh_init_mask =
+		~iwl_read32(trans, CSR_MSIX_FH_INT_MASK_AD);
+	trans_pcie->fh_mask = trans_pcie->fh_init_mask;
+	trans_pcie->hw_init_mask =
+		~iwl_read32(trans, CSR_MSIX_HW_INT_MASK_AD);
+	trans_pcie->hw_mask = trans_pcie->hw_init_mask;
+}
+
 static void _iwl_trans_pcie_stop_device(struct iwl_trans *trans, bool low_power)
 {
 	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
@@ -1405,109 +1508,6 @@ static int iwl_trans_pcie_d3_resume(struct iwl_trans *trans,
 	return 0;
 }
 
-struct iwl_causes_list {
-	u32 cause_num;
-	u32 mask_reg;
-	u8 addr;
-};
-
-static struct iwl_causes_list causes_list[] = {
-	{MSIX_FH_INT_CAUSES_D2S_CH0_NUM,	CSR_MSIX_FH_INT_MASK_AD, 0},
-	{MSIX_FH_INT_CAUSES_D2S_CH1_NUM,	CSR_MSIX_FH_INT_MASK_AD, 0x1},
-	{MSIX_FH_INT_CAUSES_S2D,		CSR_MSIX_FH_INT_MASK_AD, 0x3},
-	{MSIX_FH_INT_CAUSES_FH_ERR,		CSR_MSIX_FH_INT_MASK_AD, 0x5},
-	{MSIX_HW_INT_CAUSES_REG_ALIVE,		CSR_MSIX_HW_INT_MASK_AD, 0x10},
-	{MSIX_HW_INT_CAUSES_REG_WAKEUP,		CSR_MSIX_HW_INT_MASK_AD, 0x11},
-	{MSIX_HW_INT_CAUSES_REG_CT_KILL,	CSR_MSIX_HW_INT_MASK_AD, 0x16},
-	{MSIX_HW_INT_CAUSES_REG_RF_KILL,	CSR_MSIX_HW_INT_MASK_AD, 0x17},
-	{MSIX_HW_INT_CAUSES_REG_PERIODIC,	CSR_MSIX_HW_INT_MASK_AD, 0x18},
-	{MSIX_HW_INT_CAUSES_REG_SW_ERR,		CSR_MSIX_HW_INT_MASK_AD, 0x29},
-	{MSIX_HW_INT_CAUSES_REG_SCD,		CSR_MSIX_HW_INT_MASK_AD, 0x2A},
-	{MSIX_HW_INT_CAUSES_REG_FH_TX,		CSR_MSIX_HW_INT_MASK_AD, 0x2B},
-	{MSIX_HW_INT_CAUSES_REG_HW_ERR,		CSR_MSIX_HW_INT_MASK_AD, 0x2D},
-	{MSIX_HW_INT_CAUSES_REG_HAP,		CSR_MSIX_HW_INT_MASK_AD, 0x2E},
-};
-
-static void iwl_pcie_map_non_rx_causes(struct iwl_trans *trans)
-{
-	struct iwl_trans_pcie *trans_pcie =  IWL_TRANS_GET_PCIE_TRANS(trans);
-	int val = trans_pcie->def_irq | MSIX_NON_AUTO_CLEAR_CAUSE;
-	int i;
-
-	/*
-	 * Access all non RX causes and map them to the default irq.
-	 * In case we are missing at least one interrupt vector,
-	 * the first interrupt vector will serve non-RX and FBQ causes.
-	 */
-	for (i = 0; i < ARRAY_SIZE(causes_list); i++) {
-		iwl_write8(trans, CSR_MSIX_IVAR(causes_list[i].addr), val);
-		iwl_clear_bit(trans, causes_list[i].mask_reg,
-			      causes_list[i].cause_num);
-	}
-}
-
-static void iwl_pcie_map_rx_causes(struct iwl_trans *trans)
-{
-	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
-	u32 offset =
-		trans_pcie->shared_vec_mask & IWL_SHARED_IRQ_FIRST_RSS ? 1 : 0;
-	u32 val, idx;
-
-	/*
-	 * The first RX queue - fallback queue, which is designated for
-	 * management frame, command responses etc, is always mapped to the
-	 * first interrupt vector. The other RX queues are mapped to
-	 * the other (N - 2) interrupt vectors.
-	 */
-	val = BIT(MSIX_FH_INT_CAUSES_Q(0));
-	for (idx = 1; idx < trans->num_rx_queues; idx++) {
-		iwl_write8(trans, CSR_MSIX_RX_IVAR(idx),
-			   MSIX_FH_INT_CAUSES_Q(idx - offset));
-		val |= BIT(MSIX_FH_INT_CAUSES_Q(idx));
-	}
-	iwl_write32(trans, CSR_MSIX_FH_INT_MASK_AD, ~val);
-
-	val = MSIX_FH_INT_CAUSES_Q(0);
-	if (trans_pcie->shared_vec_mask & IWL_SHARED_IRQ_NON_RX)
-		val |= MSIX_NON_AUTO_CLEAR_CAUSE;
-	iwl_write8(trans, CSR_MSIX_RX_IVAR(0), val);
-
-	if (trans_pcie->shared_vec_mask & IWL_SHARED_IRQ_FIRST_RSS)
-		iwl_write8(trans, CSR_MSIX_RX_IVAR(1), val);
-}
-
-static void iwl_pcie_init_msix(struct iwl_trans_pcie *trans_pcie)
-{
-	struct iwl_trans *trans = trans_pcie->trans;
-
-	if (!trans_pcie->msix_enabled) {
-		if (trans->cfg->mq_rx_supported)
-			iwl_write_prph(trans, UREG_CHICK,
-				       UREG_CHICK_MSI_ENABLE);
-		return;
-	}
-
-	iwl_write_prph(trans, UREG_CHICK, UREG_CHICK_MSIX_ENABLE);
-
-	/*
-	 * Each cause from the causes list above and the RX causes is
-	 * represented as a byte in the IVAR table. The first nibble
-	 * represents the bound interrupt vector of the cause, the second
-	 * represents no auto clear for this cause. This will be set if its
-	 * interrupt vector is bound to serve other causes.
-	 */
-	iwl_pcie_map_rx_causes(trans);
-
-	iwl_pcie_map_non_rx_causes(trans);
-
-	trans_pcie->fh_init_mask =
-		~iwl_read32(trans, CSR_MSIX_FH_INT_MASK_AD);
-	trans_pcie->fh_mask = trans_pcie->fh_init_mask;
-	trans_pcie->hw_init_mask =
-		~iwl_read32(trans, CSR_MSIX_HW_INT_MASK_AD);
-	trans_pcie->hw_mask = trans_pcie->hw_init_mask;
-}
-
 static void iwl_pcie_set_interrupt_capa(struct pci_dev *pdev,
 					struct iwl_trans *trans)
 {
-- 
2.11.0

^ permalink raw reply related

* [PATCH 02/17] iwlwifi: pcie: separate between SW and HW MSIX configuration
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Haim Dreyfuss, Luca Coelho
In-Reply-To: <20170208112322.29413-1-luca@coelho.fi>

From: Haim Dreyfuss <haim.dreyfuss@intel.com>

The MSIX configuration flow includes two different stages:
configuring the HW by writing to the IVAR table and configuring the SW
to reflect the HW configuration.
The HW configuration is needed on each HW reset,
whereas the SW configuration is only needed during the init flow.

Signed-off-by: Haim Dreyfuss <haim.dreyfuss@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index f795ebea4c4a..e7a26f594386 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1147,7 +1147,7 @@ static void iwl_pcie_map_rx_causes(struct iwl_trans *trans)
 		iwl_write8(trans, CSR_MSIX_RX_IVAR(1), val);
 }
 
-static void iwl_pcie_init_msix(struct iwl_trans_pcie *trans_pcie)
+static void iwl_pcie_conf_msix_hw(struct iwl_trans_pcie *trans_pcie)
 {
 	struct iwl_trans *trans = trans_pcie->trans;
 
@@ -1170,12 +1170,20 @@ static void iwl_pcie_init_msix(struct iwl_trans_pcie *trans_pcie)
 	iwl_pcie_map_rx_causes(trans);
 
 	iwl_pcie_map_non_rx_causes(trans);
+}
+
+static void iwl_pcie_init_msix(struct iwl_trans_pcie *trans_pcie)
+{
+	struct iwl_trans *trans = trans_pcie->trans;
+
+	iwl_pcie_conf_msix_hw(trans_pcie);
 
-	trans_pcie->fh_init_mask =
-		~iwl_read32(trans, CSR_MSIX_FH_INT_MASK_AD);
+	if (!trans_pcie->msix_enabled)
+		return;
+
+	trans_pcie->fh_init_mask = ~iwl_read32(trans, CSR_MSIX_FH_INT_MASK_AD);
 	trans_pcie->fh_mask = trans_pcie->fh_init_mask;
-	trans_pcie->hw_init_mask =
-		~iwl_read32(trans, CSR_MSIX_HW_INT_MASK_AD);
+	trans_pcie->hw_init_mask = ~iwl_read32(trans, CSR_MSIX_HW_INT_MASK_AD);
 	trans_pcie->hw_mask = trans_pcie->hw_init_mask;
 }
 
@@ -1675,6 +1683,7 @@ static int _iwl_trans_pcie_start_hw(struct iwl_trans *trans, bool low_power)
 	iwl_pcie_apm_init(trans);
 
 	iwl_pcie_init_msix(trans_pcie);
+
 	/* From now on, the op_mode will be kept updated about RF kill state */
 	iwl_enable_rfkill_int(trans);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 00/17] iwlwifi: updates intended for v4.11 2017-02-08
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Luca Coelho

From: Luca Coelho <luciano.coelho@intel.com>

Hi,

This is the third and final pull-request before 4.11's merge window.
This time I concentrated in bugfixes:

* Fix 802.11w, which was failing to due an IGTK bug;
* A few more bugzilla bug fixes;
* A channel-switch race condition fix;
* Some fixes related to suspend/resume with new HW;
* The RF-kill saga continues;
* And some other fixes here and there...

As usual, I'm pushing this to a pending branch, for kbuild bot, and
will send a pull-request later.

Please review.

Cheers,
Luca.


Avraham Stern (1):
  iwlwifi: mvm: Fix CSA received immediately after association

Emmanuel Grumbach (5):
  iwlwifi: mvm: use the PROBE_RESP_QUEUE to send deauth to unknown
    station
  iwlwifi: pcie: don't increment / decrement a bool
  iwlwifi: make RTPM depend on EXPERT
  iwlwifi: dvm: don't call << operator with a negative value
  iwlwifi: mvm: don't call << operator with a negative value

Golan Ben Ami (2):
  iwlwifi: pcie: Re-configure IVAR table after stop device
  iwlwifi: pcie: set STATUS_RFKILL immediately after interrupt

Golan Ben-Ami (1):
  iwlwifi: mvm: avoid exceeding the allowed print length

Goodstein, Mordechay (1):
  iwlwifi: mvm: avoid race condition in ADD_STA.

Gregory Greenman (1):
  iwlwifi: mvm: fix a print of NSS for HT rate

Haim Dreyfuss (3):
  iwlwifi: pcie: move msix conf functions above other functions
  iwlwifi: pcie: separate between SW and HW MSIX configuration
  iwlwifi: pcie: re-configure IVAR table after suspend-resume

Ilan Peer (1):
  iwlwifi: mvm: Fix removal of IGTK

Sara Sharon (2):
  iwlwifi: mvm: fix references to first_agg_queue in DQA mode
  iwlwifi: mvm: fix reorder timer re-arming

 drivers/net/wireless/intel/iwlwifi/Kconfig         |   7 +-
 drivers/net/wireless/intel/iwlwifi/dvm/rs.c        |   5 +-
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c        |  20 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c  |  15 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c        |   6 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c      |   3 +-
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c       |   7 +-
 drivers/net/wireless/intel/iwlwifi/mvm/tx.c        |  17 +-
 drivers/net/wireless/intel/iwlwifi/pcie/internal.h |   2 +-
 drivers/net/wireless/intel/iwlwifi/pcie/rx.c       |   8 +-
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c    | 243 ++++++++++++---------
 11 files changed, 188 insertions(+), 145 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH 04/17] iwlwifi: pcie: Re-configure IVAR table after stop device
From: Luca Coelho @ 2017-02-08 11:23 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Golan Ben Ami, Luca Coelho
In-Reply-To: <20170208112322.29413-1-luca@coelho.fi>

From: Golan Ben Ami <golan.ben.ami@intel.com>

When getting RF_KILL and disabling radio, the device gets stopped
and reset. This erases the IVAR table that matches the interrupt
to its cause, and is essential for MSIX proper functionality.
Till now, the table wasn't re-configured after the reset, and
therefore the interrupt that enabled radio didn't fire on the
right irq, and the driver didn't handle it correctly.

To fix this, configure the IVAR table again after resetting the
device.

Signed-off-by: Golan Ben-Ami <golan.ben.ami@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index bb2a9a151957..7f05fc56587a 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1246,6 +1246,15 @@ static void _iwl_trans_pcie_stop_device(struct iwl_trans *trans, bool low_power)
 	usleep_range(1000, 2000);
 
 	/*
+	 * Upon stop, the IVAR table gets erased, so msi-x won't
+	 * work. This causes a bug in RF-KILL flows, since the interrupt
+	 * that enables radio won't fire on the correct irq, and the
+	 * driver won't be able to handle the interrupt.
+	 * Configure the IVAR table again after reset.
+	 */
+	iwl_pcie_conf_msix_hw(trans_pcie);
+
+	/*
 	 * Upon stop, the APM issues an interrupt if HW RF kill is set.
 	 * This is a bug in certain verions of the hardware.
 	 * Certain devices also keep sending HW RF kill interrupt all
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH net] brcmfmac: clear skb head state on xmit
From: Arend Van Spriel @ 2017-02-08 10:43 UTC (permalink / raw)
  To: Paolo Abeni, Rafał Miłecki
  Cc: Kalle Valo, linux-wireless@vger.kernel.org,
	open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
	Franky Lin, hante Meuleman
In-Reply-To: <1486546181.2533.5.camel@redhat.com>

On 8-2-2017 10:29, Paolo Abeni wrote:
> On Wed, 2017-02-08 at 09:52 +0100, Rafał Miłecki wrote:
>> On 8 February 2017 at 09:38, Paolo Abeni <pabeni@redhat.com> wrote:
>>> On Tue, 2017-02-07 at 20:23 +0100, Arend Van Spriel wrote:
>>>> On 7-2-2017 17:50, Paolo Abeni wrote:
>>>>> the skbs can be held by the driver for a long time, so we need
>>>>> to clear any state on xmit to avoid hanging other subsystems.
>>>>> The skbs are already orphaned later in cmsg code, so we just
>>>>> need to clear the nf/dst/secpath.
>>>>> Do it early, while the relevant entries are hopefully still
>>>>> hot in the cache.
>>>>
>>>> What is this about really? A bit more background about the issue
>>>> might
>>>> help understanding the need for this patch. Is this really
>>>> specific
>>>> to
>>>> brcmfmac. For instance is something similar already done in
>>>> mac80211?
>>>
>>> The issue is apparently driver specific, as reported in:
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1294415
>>>
>>> This is caused by xmit skbs carrying a notrack ct entry not being
>>> freed
>>> by the device driver in a timely manner. Removing the ct module
>>> waits
>>> for such entries refcount going to zero and hangs the kernel in
>>> busy
>>> loop (for several minutes).
>>>
>>> The relevant skbs are icmp6 packets (ND if I recall correctly, they
>>> bcast packets at the mac level).
>>>
>>> The only other known device driver suffering for the issue is the
>>> infiniband ipoib driver, I send a separate patch for it.
>>>
>>> I lack the broadcom h/w, but with infiniband the bug can be
>>> reproduced
>>> with the following steps:
>>>
>>> - ensure ipv6 is enabled on the target device, and firewalld is
>>> running
>>> (e.g. the module nf_conntrack_ipv6 is loaded)
>>> - assign a static ip to the device
>>> - shut down the firewall (e.g. try to remove the module
>>> nf_conntrack)
>>>
>>> For the brcmfmac driver most probably it is necessary being
>>> disassociated from the AP before shutting down the firewall (but I
>>> can't double check). This is probably why mac80211 does not suffer
>>> this
>>> issue.
>>>
>>> The root cause for the issue could be actually a firmware issue,
>>> any
>>> better clues are more than welcome!
>>
>> Do I get this correctly brcmfmac gets some skb for transmitting it
>> and
>> doesn't free it for few minutes? It sounds like some bug that should
>> be fixed instead of hiding it.
> 
> I mostly agreed, but please also note that early clearing the skb head
> state makes sense from a performance pov: we can do the costly,
> required, atomic operations while the data is still hot in the cpu L1
> cache.
> 
> I'm unable to find anything obvious at the driver level, and I think
> the root cause could be in the firmware. I hope some more knowledgeable
> than me on this topics can have a better look.

Hi Paolo,

I agree with Rafał that several minutes for an skb to be freed is a red
flag. Now I know our driver and firmware but not played to much with
IPv6 and firewalls. Hopefully I have all the netfilter stuff in my
kernel when I try to reproduce it over here.

Regards,
Arend

^ permalink raw reply

* Re: [PATCH V3 4/9] brcmfmac: add struct brcmf_pub parameter to the __brcmf_err
From: Arend Van Spriel @ 2017-02-08  9:54 UTC (permalink / raw)
  To: Rafał Miłecki, Kalle Valo
  Cc: Franky Lin, Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
	linux-wireless, brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <20170202213321.11591-4-zajec5@gmail.com>

On 2-2-2017 22:33, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This will allow getting struct device reference from the passed
> brcmf_pub for the needs of dev_err. More detailed messages are really
> important for home routers which frequently have 2 (or even 3) wireless
> cards supported by brcmfmac.
> 
> Note that all calls are yet to be updated as for now brcmf_err macro
> always passes NULL. This will be handled in following patch to make this
> change easier to review.

I prefer brcmf_err() to have struct device reference directly instead of
using brcmf_pub. That would remove the need for patches 5 till 7 as bus
specific code already has struct device. So I have acked the first three
patches, but would like to revise 8 and 9.

Kalle,

I acked the first three patches. Can those three still go in for 4.11?

Regards,
Arend
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Add missing include
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h        | 2 ++
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c     | 2 +-
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h      | 9 +++++----
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c | 4 +++-
>  4 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
> index 76693df34742..35d4801a62e6 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
> @@ -17,6 +17,8 @@
>  #ifndef BRCMFMAC_BUS_H
>  #define BRCMFMAC_BUS_H
>  
> +#include <linux/device.h>
> +
>  #include "debug.h"
>  
>  /* IDs of the 6 default common rings of msgbuf protocol */
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> index 33b133f7e63a..f8ce597c4781 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> @@ -219,7 +219,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
>  }
>  
>  #ifndef CONFIG_BRCM_TRACING
> -void __brcmf_err(const char *func, const char *fmt, ...)
> +void __brcmf_err(struct brcmf_pub *pub, const char *func, const char *fmt, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> index 066126123e96..99acc095c834 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> @@ -19,6 +19,8 @@
>  
>  #include <linux/net.h>	/* net_ratelimit() */
>  
> +struct brcmf_pub;
> +
>  /* message levels */
>  #define BRCMF_TRACE_VAL		0x00000002
>  #define BRCMF_INFO_VAL		0x00000004
> @@ -45,8 +47,8 @@
>  #undef pr_fmt
>  #define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
>  
> -__printf(2, 3)
> -void __brcmf_err(const char *func, const char *fmt, ...);
> +__printf(3, 4)
> +void __brcmf_err(struct brcmf_pub *pub, const char *func, const char *fmt, ...);
>  /* Macro for error messages. When debugging / tracing the driver all error
>   * messages are important to us.
>   */
> @@ -55,7 +57,7 @@ void __brcmf_err(const char *func, const char *fmt, ...);
>  		if (IS_ENABLED(CONFIG_BRCMDBG) ||			\
>  		    IS_ENABLED(CONFIG_BRCM_TRACING) ||			\
>  		    net_ratelimit())					\
> -			__brcmf_err(__func__, fmt, ##__VA_ARGS__);	\
> +			__brcmf_err(NULL, __func__, fmt, ##__VA_ARGS__);\
>  	} while (0)
>  
>  #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
> @@ -99,7 +101,6 @@ do {									\
>  
>  extern int brcmf_msg_level;
>  
> -struct brcmf_pub;
>  #ifdef DEBUG
>  void brcmf_debugfs_init(void);
>  void brcmf_debugfs_exit(void);
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
> index fe6755944b7b..329cb65eb78b 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
> @@ -18,10 +18,12 @@
>  
>  #ifndef __CHECKER__
>  #define CREATE_TRACE_POINTS
> +#include "bus.h"
> +#include "core.h"
>  #include "tracepoint.h"
>  #include "debug.h"
>  
> -void __brcmf_err(const char *func, const char *fmt, ...)
> +void __brcmf_err(struct brcmf_pub *pub, const char *func, const char *fmt, ...)
>  {
>  	struct va_format vaf = {
>  		.fmt = fmt,
> 

^ permalink raw reply

* Re: [PATCH V3 3/9] brcmfmac: merge two remaining brcmf_err macros
From: Arend Van Spriel @ 2017-02-08  9:52 UTC (permalink / raw)
  To: Rafał Miłecki, Kalle Valo
  Cc: Franky Lin, Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
	linux-wireless, brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <20170202213321.11591-3-zajec5@gmail.com>

On 2-2-2017 22:33, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Now we always have __brcmf_err function we can do perfectly fine with
> just one macro.

Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V3: Add this (new) patch
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> index 441a6661eac0..066126123e96 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> @@ -47,20 +47,16 @@
>  
>  __printf(2, 3)
>  void __brcmf_err(const char *func, const char *fmt, ...);
> -#ifndef CONFIG_BRCM_TRACING
> -/* Macro for error messages. net_ratelimit() is used when driver
> - * debugging is not selected. When debugging the driver error
> - * messages are as important as other tracing or even more so.
> +/* Macro for error messages. When debugging / tracing the driver all error
> + * messages are important to us.
>   */
>  #define brcmf_err(fmt, ...)						\
>  	do {								\
> -		if (IS_ENABLED(CONFIG_BRCMDBG) || net_ratelimit())	\
> +		if (IS_ENABLED(CONFIG_BRCMDBG) ||			\
> +		    IS_ENABLED(CONFIG_BRCM_TRACING) ||			\
> +		    net_ratelimit())					\
>  			__brcmf_err(__func__, fmt, ##__VA_ARGS__);	\
>  	} while (0)
> -#else
> -#define brcmf_err(fmt, ...) \
> -	__brcmf_err(__func__, fmt, ##__VA_ARGS__)
> -#endif
>  
>  #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
>  __printf(3, 4)
> 

^ permalink raw reply

* Re: [PATCH net] brcmfmac: clear skb head state on xmit
From: Kalle Valo @ 2017-02-08 10:27 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Arend Van Spriel, linux-wireless, brcm80211-dev-list.pdl,
	Franky Lin, hante Meuleman
In-Reply-To: <1486543119.2533.3.camel@redhat.com>

Paolo Abeni <pabeni@redhat.com> writes:

> On Tue, 2017-02-07 at 20:23 +0100, Arend Van Spriel wrote:
>> On 7-2-2017 17:50, Paolo Abeni wrote:
>> > the skbs can be held by the driver for a long time, so we need
>> > to clear any state on xmit to avoid hanging other subsystems.
>> > The skbs are already orphaned later in cmsg code, so we just
>> > need to clear the nf/dst/secpath.
>> > Do it early, while the relevant entries are hopefully still
>> > hot in the cache.
>> 
>> What is this about really? A bit more background about the issue
>> might
>> help understanding the need for this patch. Is this really specific
>> to
>> brcmfmac. For instance is something similar already done in mac80211?
>
> The issue is apparently driver specific, as reported in:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1294415
>
> This is caused by xmit skbs carrying a notrack ct entry not being freed
> by the device driver in a timely manner. Removing the ct module waits
> for such entries refcount going to zero and hangs the kernel in busy
> loop (for several minutes).
>
> The relevant skbs are icmp6 packets (ND if I recall correctly, they
> bcast packets at the mac level).
>
> The only other known device driver suffering for the issue is the
> infiniband ipoib driver, I send a separate patch for it.
>
> I lack the broadcom h/w, but with infiniband the bug can be reproduced
> with the following steps:
>
> - ensure ipv6 is enabled on the target device, and firewalld is running
> (e.g. the module nf_conntrack_ipv6 is loaded)
> - assign a static ip to the device
> - shut down the firewall (e.g. try to remove the module nf_conntrack)
>
> For the brcmfmac driver most probably it is necessary being
> disassociated from the AP before shutting down the firewall (but I
> can't double check). This is probably why mac80211 does not suffer this
> issue.
>
> The root cause for the issue could be actually a firmware issue, any
> better clues are more than welcome!

BTW, you should have added all this to the commit log. After reading the
original description it left more questions open than answered them.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH net] brcmfmac: clear skb head state on xmit
From: Paolo Abeni @ 2017-02-08 10:34 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend Van Spriel, linux-wireless, brcm80211-dev-list.pdl,
	Franky Lin, hante Meuleman
In-Reply-To: <87zihxm0yp.fsf@kamboji.qca.qualcomm.com>

On Wed, 2017-02-08 at 12:27 +0200, Kalle Valo wrote:
> Paolo Abeni <pabeni@redhat.com> writes:
> 
> > On Tue, 2017-02-07 at 20:23 +0100, Arend Van Spriel wrote:
> > > On 7-2-2017 17:50, Paolo Abeni wrote:
> > > > the skbs can be held by the driver for a long time, so we need
> > > > to clear any state on xmit to avoid hanging other subsystems.
> > > > The skbs are already orphaned later in cmsg code, so we just
> > > > need to clear the nf/dst/secpath.
> > > > Do it early, while the relevant entries are hopefully still
> > > > hot in the cache.
> > > 
> > > What is this about really? A bit more background about the issue
> > > might
> > > help understanding the need for this patch. Is this really specific
> > > to
> > > brcmfmac. For instance is something similar already done in mac80211?
> > 
> > The issue is apparently driver specific, as reported in:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1294415
> > 
> > This is caused by xmit skbs carrying a notrack ct entry not being freed
> > by the device driver in a timely manner. Removing the ct module waits
> > for such entries refcount going to zero and hangs the kernel in busy
> > loop (for several minutes).
> > 
> > The relevant skbs are icmp6 packets (ND if I recall correctly, they
> > bcast packets at the mac level).
> > 
> > The only other known device driver suffering for the issue is the
> > infiniband ipoib driver, I send a separate patch for it.
> > 
> > I lack the broadcom h/w, but with infiniband the bug can be reproduced
> > with the following steps:
> > 
> > - ensure ipv6 is enabled on the target device, and firewalld is running
> > (e.g. the module nf_conntrack_ipv6 is loaded)
> > - assign a static ip to the device
> > - shut down the firewall (e.g. try to remove the module nf_conntrack)
> > 
> > For the brcmfmac driver most probably it is necessary being
> > disassociated from the AP before shutting down the firewall (but I
> > can't double check). This is probably why mac80211 does not suffer this
> > issue.
> > 
> > The root cause for the issue could be actually a firmware issue, any
> > better clues are more than welcome!
> 
> BTW, you should have added all this to the commit log. After reading the
> original description it left more questions open than answered them.

I'm sorry for the lack of clarity, thank you for the advice, will do
next time. 

I tried to keep the commit message short since others reviewer in other
area of the kernel really prefer shorter ones.

Cheers,

Paolo

^ permalink raw reply

* Re: [PATCH v4 5/5] wireless: Set NL80211_EXT_FEATURE_CQM_RSSI_LIST in multiple drivers
From: Johannes Berg @ 2017-02-08 10:00 UTC (permalink / raw)
  To: Andrew Zaborowski, linux-wireless
In-Reply-To: <20170125114344.8179-5-andrew.zaborowski@intel.com>

On Wed, 2017-01-25 at 12:43 +0100, Andrew Zaborowski wrote:
> Set the NL80211_EXT_FEATURE_CQM_RSSI_LIST wiphy extended feature
> wholesale in all mac80211-based drivers that do not set the
> IEEE80211_VIF_BEACON_FILTER flags on their interfaces.  mac80211 will
> be processing supplied RSSI values in ieee80211_rx_mgmt_beacon and
> will detect when the thresholds set by
> ieee80211_set_cqm_rssi_range_config are crossed.  Remaining (few)
> drivers need code to enable the firmware to monitor the thresholds.
> This is mostly only compile-tested.

4 and 5 look fine, but obviously depend on 3.

johannes

^ permalink raw reply

* Re: [PATCH v5 3/3] mt76: add driver code for MT76x2e
From: Stanislaw Gruszka @ 2017-02-08  9:54 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, kvalo
In-Reply-To: <20170207202035.64224-4-nbd@nbd.name>

On Tue, Feb 07, 2017 at 09:20:35PM +0100, Felix Fietkau wrote:
> This is a 2x2 PCIe 802.11ac chipset by MediaTek
> 
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>

Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>

^ permalink raw reply

* Re: [PATCH v4 3/5] cfg80211: Accept multiple RSSI thresholds for CQM
From: Johannes Berg @ 2017-02-08  9:58 UTC (permalink / raw)
  To: Andrew Zaborowski, linux-wireless
In-Reply-To: <20170125114344.8179-3-andrew.zaborowski@intel.com>


> This method doesn't have a hysteresis parameter because there's no
> benefit to the cfg80211 code from having the hysteresis be handled by
> hardware/driver in terms of the number of wakeups.  At the same time
> it would likely be less consistent between drivers if offloaded or
> done in the drivers.

I'm not really sure I buy this.

What if I configure a few ranges, and let's say one of the boundaries
is -50dBm. Now if I sit just on that value of -50dBm and thus my signal
fluctuates say -48..-52, I'd have to continuously wake up the host.

You try to avoid that here, I think:

+	if (low > (s32) (last - hyst))
+		low = last - hyst;
+	if (high < (s32) (last + hyst))
+		high = last + hyst;

but it's not clear to me that this is effective?

Let's see. last is -52, so low will be -60 and high will be -50.

Let's say hyst is 3 since I chose the ranges so closely. I'm guessing a
higher hysteresis and larger ranges would actually be better, but let's
stick to this for the sake of the example.

last-hyst = -55, so low isn't > that, low = -60
last+hyst = -49, so high = -49

but now it still fluctuates around -50, so if I next hit -48 you do the
same dance again with -50/-40, getting -51/-40 and never really
applying a full hysteresis, no?

I'll probably see an intermediate value of -50 at some point, but I'll
never actually *report* it, so "last" can switch between -48 and -52
constantly in this scenario, no?


I think it would make sense to unconditionally apply the hysteresis to
low/high, i.e. always set
 low = low - hyst
 high = high + hyst

so that you get "sticky" ranges once you're in them?



> +	if (!wiphy_ext_feature_isset(&rdev->wiphy,
> +			
> 	     NL80211_EXT_FEATURE_CQM_RSSI_LIST))
> +		return
> -EOPNOTSUPP;


That check should be earlier in the function
cfg80211_cqm_rssi_update(), no?



johannes

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox