Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCHv2 2/2] ath10k: add support for per sta tx bitrate
From: akolli @ 2016-10-18  9:31 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, akolli, Anilkumar Kolli
In-Reply-To: <1476783085-10724-1-git-send-email-akolli@qti.qualcomm.com>

From: Anilkumar Kolli <akolli@qti.qualcomm.com>

Per STA tx bitrate info is filled from peer stats.
Export per sta txrate info to cfg80211/nl80211

Signed-off-by: Anilkumar Kolli <akolli@qti.qualcomm.com>
---
v2:
 * addressed kalle's comments

 drivers/net/wireless/ath/ath10k/debugfs_sta.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/debugfs_sta.c b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
index 9955fea0802a..fce6f8137d33 100644
--- a/drivers/net/wireless/ath/ath10k/debugfs_sta.c
+++ b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
@@ -77,6 +77,19 @@ void ath10k_sta_statistics(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	sinfo->rx_duration = arsta->rx_duration;
 	sinfo->filled |= 1ULL << NL80211_STA_INFO_RX_DURATION;
+
+	if (!arsta->txrate.legacy && !arsta->txrate.nss)
+		return;
+
+	if (arsta->txrate.legacy) {
+		sinfo->txrate.legacy = arsta->txrate.legacy;
+	} else {
+		sinfo->txrate.mcs = arsta->txrate.mcs;
+		sinfo->txrate.nss = arsta->txrate.nss;
+		sinfo->txrate.bw = arsta->txrate.bw;
+	}
+	sinfo->txrate.flags = arsta->txrate.flags;
+	sinfo->filled |= 1ULL << NL80211_STA_INFO_TX_BITRATE;
 }
 
 static ssize_t ath10k_dbg_sta_read_aggr_mode(struct file *file,
-- 
1.7.9.5

^ permalink raw reply related

* [PATCHv2 1/2] ath10k: add per peer htt tx stats support for 10.4
From: akolli @ 2016-10-18  9:31 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, akolli, Anilkumar Kolli
In-Reply-To: <1476783085-10724-1-git-send-email-akolli@qti.qualcomm.com>

From: Anilkumar Kolli <akolli@qti.qualcomm.com>

Per peer tx stats are part of 'HTT_10_4_T2H_MSG_TYPE_PEER_STATS'
event, Firmware sends one HTT event for every four PPDUs.
HTT payload has success pkts/bytes, failed pkts/bytes, retry
pkts/bytes and rate info per ppdu.
Peer stats are enabled through 'WMI_SERVICE_PEER_STATS',
which are nowadays enabled by default.

Parse peer stats and update the tx rate information per STA.

tx rate, Peer stats are tested on QCA4019 with Firmware version
10.4-3.2.1-00028.

Signed-off-by: Anilkumar Kolli <akolli@qti.qualcomm.com>
---
v2:
 * address Kalle's comments 
 * fix compilation warnings

 drivers/net/wireless/ath/ath10k/core.h   |   17 ++++
 drivers/net/wireless/ath/ath10k/htt.c    |    2 +
 drivers/net/wireless/ath/ath10k/htt.h    |   25 ++++++
 drivers/net/wireless/ath/ath10k/htt_rx.c |  125 ++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h    |   10 ++-
 5 files changed, 178 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index dda49af1eb74..fc3d3bded265 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -337,6 +337,7 @@ struct ath10k_sta {
 	u32 nss;
 	u32 smps;
 	u16 peer_id;
+	struct rate_info txrate;
 
 	struct work_struct update_wk;
 
@@ -694,6 +695,21 @@ struct ath10k_fw_components {
 	struct ath10k_fw_file fw_file;
 };
 
+struct ath10k_per_peer_tx_stats {
+	u32	succ_bytes;
+	u32	retry_bytes;
+	u32	failed_bytes;
+	u8	ratecode;
+	u8	flags;
+	u16	peer_id;
+	u16	succ_pkts;
+	u16	retry_pkts;
+	u16	failed_pkts;
+	u16	duration;
+	u32	reserved1;
+	u32	reserved2;
+};
+
 struct ath10k {
 	struct ath_common ath_common;
 	struct ieee80211_hw *hw;
@@ -906,6 +922,7 @@ struct ath10k {
 
 	struct ath10k_thermal thermal;
 	struct ath10k_wow wow;
+	struct ath10k_per_peer_tx_stats peer_tx_stats;
 
 	/* NAPI */
 	struct net_device napi_dev;
diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 130cd9502021..cd160b16db1e 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -137,6 +137,8 @@ static const enum htt_t2h_msg_type htt_10_4_t2h_msg_types[] = {
 				HTT_T2H_MSG_TYPE_STATS_NOUPLOAD,
 	[HTT_10_4_T2H_MSG_TYPE_TX_MODE_SWITCH_IND] =
 				HTT_T2H_MSG_TYPE_TX_MODE_SWITCH_IND,
+	[HTT_10_4_T2H_MSG_TYPE_PEER_STATS] =
+				HTT_T2H_MSG_TYPE_PEER_STATS,
 };
 
 int ath10k_htt_connect(struct ath10k_htt *htt)
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 0d2ed09f202b..164eb3a10566 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -419,6 +419,7 @@ enum htt_10_4_t2h_msg_type {
 	HTT_10_4_T2H_MSG_TYPE_STATS_NOUPLOAD         = 0x18,
 	/* 0x19 to 0x2f are reserved */
 	HTT_10_4_T2H_MSG_TYPE_TX_MODE_SWITCH_IND     = 0x30,
+	HTT_10_4_T2H_MSG_TYPE_PEER_STATS	     = 0x31,
 	/* keep this last */
 	HTT_10_4_T2H_NUM_MSGS
 };
@@ -453,6 +454,7 @@ enum htt_t2h_msg_type {
 	HTT_T2H_MSG_TYPE_TX_FETCH_IND,
 	HTT_T2H_MSG_TYPE_TX_FETCH_CONFIRM,
 	HTT_T2H_MSG_TYPE_TX_MODE_SWITCH_IND,
+	HTT_T2H_MSG_TYPE_PEER_STATS,
 	/* keep this last */
 	HTT_T2H_NUM_MSGS
 };
@@ -1470,6 +1472,28 @@ struct htt_channel_change {
 	__le32 phymode;
 } __packed;
 
+struct htt_per_peer_tx_stats_ind {
+	__le32	succ_bytes;
+	__le32  retry_bytes;
+	__le32  failed_bytes;
+	u8	ratecode;
+	u8	flags;
+	__le16	peer_id;
+	__le16  succ_pkts;
+	__le16	retry_pkts;
+	__le16	failed_pkts;
+	__le16	tx_duration;
+	__le32	reserved1;
+	__le32	reserved2;
+} __packed;
+
+struct htt_peer_tx_stats {
+	u8 num_ppdu;
+	u8 ppdu_len;
+	u8 version;
+	u8 payload[0];
+} __packed;
+
 union htt_rx_pn_t {
 	/* WEP: 24-bit PN */
 	u32 pn24;
@@ -1521,6 +1545,7 @@ struct htt_resp {
 		struct htt_tx_fetch_confirm tx_fetch_confirm;
 		struct htt_tx_mode_switch_ind tx_mode_switch_ind;
 		struct htt_channel_change chan_change;
+		struct htt_peer_tx_stats peer_tx_stats;
 	};
 } __packed;
 
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 0b4c1562420f..ef28b358cf5e 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2194,6 +2194,128 @@ void ath10k_htt_htc_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 		dev_kfree_skb_any(skb);
 }
 
+static inline bool is_valid_legacy_rate(u8 rate)
+{
+	static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
+					  18, 24, 36, 48, 54};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
+		if (rate == legacy_rates[i])
+			return true;
+	}
+
+	return false;
+}
+
+static void
+ath10k_update_per_peer_tx_stats(struct ath10k *ar,
+				struct ieee80211_sta *sta,
+				struct ath10k_per_peer_tx_stats *peer_stats)
+{
+	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
+	u8 rate = 0, sgi;
+	struct rate_info txrate;
+
+	lockdep_assert_held(&ar->data_lock);
+
+	txrate.flags = ATH10K_HW_PREAMBLE(peer_stats->ratecode);
+	txrate.bw = ATH10K_HW_BW(peer_stats->flags);
+	txrate.nss = ATH10K_HW_NSS(peer_stats->ratecode);
+	txrate.mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
+	sgi = ATH10K_HW_GI(peer_stats->flags);
+
+	if (((txrate.flags == WMI_RATE_PREAMBLE_HT) ||
+	     (txrate.flags == WMI_RATE_PREAMBLE_VHT)) && txrate.mcs > 9) {
+		ath10k_warn(ar, "Invalid mcs %hhd peer stats", txrate.mcs);
+		return;
+	}
+
+	if (txrate.flags == WMI_RATE_PREAMBLE_CCK ||
+	    txrate.flags == WMI_RATE_PREAMBLE_OFDM) {
+		rate = ATH10K_HW_LEGACY_RATE(peer_stats->ratecode);
+
+		if (!is_valid_legacy_rate(rate)) {
+			ath10k_warn(ar, "Invalid legacy rate %hhd peer stats",
+				    rate);
+			return;
+		}
+
+		/* This is hacky, FW sends CCK rate 5.5Mbps as 6 */
+		rate *= 10;
+		if (rate == 60 && txrate.flags == WMI_RATE_PREAMBLE_CCK)
+			rate = rate - 5;
+		arsta->txrate.legacy = rate * 10;
+	} else if (txrate.flags == WMI_RATE_PREAMBLE_HT) {
+		arsta->txrate.flags = RATE_INFO_FLAGS_MCS;
+		arsta->txrate.mcs = txrate.mcs;
+	} else {
+		arsta->txrate.flags = RATE_INFO_FLAGS_VHT_MCS;
+		arsta->txrate.mcs = txrate.mcs;
+	}
+
+	if (sgi)
+		arsta->txrate.flags |= RATE_INFO_FLAGS_SHORT_GI;
+
+	arsta->txrate.nss = txrate.nss;
+	arsta->txrate.bw = txrate.bw + RATE_INFO_BW_20;
+}
+
+static void ath10k_htt_fetch_peer_stats(struct ath10k *ar,
+					struct sk_buff *skb)
+{
+	struct htt_resp *resp = (struct htt_resp *)skb->data;
+	struct ath10k_per_peer_tx_stats *p_tx_stats = &ar->peer_tx_stats;
+	struct htt_per_peer_tx_stats_ind *tx_stats;
+	struct ieee80211_sta *sta;
+	struct ath10k_peer *peer;
+	int peer_id, i;
+	u8 ppdu_len, num_ppdu;
+
+	num_ppdu = resp->peer_tx_stats.num_ppdu;
+	ppdu_len = resp->peer_tx_stats.ppdu_len * sizeof(__le32);
+
+	if (skb->len < sizeof(struct htt_resp_hdr) + num_ppdu * ppdu_len) {
+		ath10k_warn(ar, "Invalid peer stats buf length %d\n", skb->len);
+		return;
+	}
+
+	tx_stats = (struct htt_per_peer_tx_stats_ind *)
+			(resp->peer_tx_stats.payload);
+	peer_id = tx_stats->peer_id;
+
+	rcu_read_lock();
+	spin_lock_bh(&ar->data_lock);
+	peer = ath10k_peer_find_by_id(ar, peer_id);
+	if (!peer) {
+		ath10k_warn(ar, "Invalid peer id %d peer stats buffer\n",
+			    peer_id);
+		goto out;
+	}
+
+	sta = peer->sta;
+	for (i = 0; i < num_ppdu; i++) {
+		tx_stats = (struct htt_per_peer_tx_stats_ind *)
+			   (resp->peer_tx_stats.payload + i * ppdu_len);
+
+		p_tx_stats->succ_bytes = __le32_to_cpu(tx_stats->succ_bytes);
+		p_tx_stats->retry_bytes = __le32_to_cpu(tx_stats->retry_bytes);
+		p_tx_stats->failed_bytes =
+				__le32_to_cpu(tx_stats->failed_bytes);
+		p_tx_stats->ratecode = tx_stats->ratecode;
+		p_tx_stats->flags = tx_stats->flags;
+		p_tx_stats->succ_pkts = __le16_to_cpu(tx_stats->succ_pkts);
+		p_tx_stats->retry_pkts = __le16_to_cpu(tx_stats->retry_pkts);
+		p_tx_stats->failed_pkts = __le16_to_cpu(tx_stats->failed_pkts);
+
+		ath10k_update_per_peer_tx_stats(ar, sta, p_tx_stats);
+	}
+
+out:
+	spin_unlock_bh(&ar->data_lock);
+	rcu_read_unlock();
+}
+
 bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct ath10k_htt *htt = &ar->htt;
@@ -2354,6 +2476,9 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 	case HTT_T2H_MSG_TYPE_TX_MODE_SWITCH_IND:
 		ath10k_htt_rx_tx_mode_switch_ind(ar, skb);
 		break;
+	case HTT_T2H_MSG_TYPE_PEER_STATS:
+		ath10k_htt_fetch_peer_stats(ar, skb);
+		break;
 	case HTT_T2H_MSG_TYPE_EN_STATS:
 	default:
 		ath10k_warn(ar, "htt event (%d) not handled\n",
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 1b243c899bef..e108d49998c3 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -4603,9 +4603,17 @@ enum wmi_rate_preamble {
 
 #define ATH10K_HW_NSS(rate)		(1 + (((rate) >> 4) & 0x3))
 #define ATH10K_HW_PREAMBLE(rate)	(((rate) >> 6) & 0x3)
-#define ATH10K_HW_RATECODE(rate, nss, preamble)	\
+#define ATH10K_HW_MCS_RATE(rate)	((rate) & 0xf)
+#define ATH10K_HW_LEGACY_RATE(rate)	((rate) & 0x3f)
+#define ATH10K_HW_BW(flags)		(((flags) >> 3) & 0x3)
+#define ATH10K_HW_GI(flags)		(((flags) >> 5) & 0x1)
+#define ATH10K_HW_RATECODE(rate, nss, preamble) \
 	(((preamble) << 6) | ((nss) << 4) | (rate))
 
+#define VHT_MCS_NUM     10
+#define VHT_BW_NUM      4
+#define VHT_NSS_NUM     4
+
 /* Value to disable fixed rate setting */
 #define WMI_FIXED_RATE_NONE    (0xff)
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCHv2 0/2] ath10k: add support for tx bitrate
From: akolli @ 2016-10-18  9:31 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, akolli

From: Anilkumar Kolli <akolli@codeaurora.org>

This patch series adds support for tx bitrate using 
.sta_statistics callback.

tx bitrate tested on QCA4019 using iw.

Anilkumar Kolli (2):
  ath10k: add per peer htt tx stats support for 10.4
  ath10k: add support for per sta tx bitrate

 drivers/net/wireless/ath/ath10k/core.h        |   17 ++++
 drivers/net/wireless/ath/ath10k/debugfs_sta.c |   13 +++
 drivers/net/wireless/ath/ath10k/htt.c         |    2 +
 drivers/net/wireless/ath/ath10k/htt.h         |   25 +++++
 drivers/net/wireless/ath/ath10k/htt_rx.c      |  125 +++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h         |   10 +-
 6 files changed, 191 insertions(+), 1 deletion(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH 1/2] ath10k: add per peer htt tx stats support for 10.4
From: akolli @ 2016-10-18  9:29 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, akolli, Anilkumar Kolli
In-Reply-To: <1476782982-10683-1-git-send-email-akolli@qti.qualcomm.com>

From: Anilkumar Kolli <akolli@qti.qualcomm.com>

Per peer tx stats are part of 'HTT_10_4_T2H_MSG_TYPE_PEER_STATS'
event, Firmware sends one HTT event for every four PPDUs.
HTT payload has success pkts/bytes, failed pkts/bytes, retry
pkts/bytes and rate info per ppdu.
Peer stats are enabled through 'WMI_SERVICE_PEER_STATS',
which are nowadays enabled by default.

Parse peer stats and update the tx rate information per STA.

tx rate, Peer stats are tested on QCA4019 with Firmware version
10.4-3.2.1-00028.

Signed-off-by: Anilkumar Kolli <akolli@qti.qualcomm.com>
---
v2:
 * address Kalle's comments 
 * fix compilation warnings

 drivers/net/wireless/ath/ath10k/core.h   |   17 ++++
 drivers/net/wireless/ath/ath10k/htt.c    |    2 +
 drivers/net/wireless/ath/ath10k/htt.h    |   25 ++++++
 drivers/net/wireless/ath/ath10k/htt_rx.c |  125 ++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h    |   10 ++-
 5 files changed, 178 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index dda49af1eb74..fc3d3bded265 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -337,6 +337,7 @@ struct ath10k_sta {
 	u32 nss;
 	u32 smps;
 	u16 peer_id;
+	struct rate_info txrate;
 
 	struct work_struct update_wk;
 
@@ -694,6 +695,21 @@ struct ath10k_fw_components {
 	struct ath10k_fw_file fw_file;
 };
 
+struct ath10k_per_peer_tx_stats {
+	u32	succ_bytes;
+	u32	retry_bytes;
+	u32	failed_bytes;
+	u8	ratecode;
+	u8	flags;
+	u16	peer_id;
+	u16	succ_pkts;
+	u16	retry_pkts;
+	u16	failed_pkts;
+	u16	duration;
+	u32	reserved1;
+	u32	reserved2;
+};
+
 struct ath10k {
 	struct ath_common ath_common;
 	struct ieee80211_hw *hw;
@@ -906,6 +922,7 @@ struct ath10k {
 
 	struct ath10k_thermal thermal;
 	struct ath10k_wow wow;
+	struct ath10k_per_peer_tx_stats peer_tx_stats;
 
 	/* NAPI */
 	struct net_device napi_dev;
diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 130cd9502021..cd160b16db1e 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -137,6 +137,8 @@ static const enum htt_t2h_msg_type htt_10_4_t2h_msg_types[] = {
 				HTT_T2H_MSG_TYPE_STATS_NOUPLOAD,
 	[HTT_10_4_T2H_MSG_TYPE_TX_MODE_SWITCH_IND] =
 				HTT_T2H_MSG_TYPE_TX_MODE_SWITCH_IND,
+	[HTT_10_4_T2H_MSG_TYPE_PEER_STATS] =
+				HTT_T2H_MSG_TYPE_PEER_STATS,
 };
 
 int ath10k_htt_connect(struct ath10k_htt *htt)
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 0d2ed09f202b..164eb3a10566 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -419,6 +419,7 @@ enum htt_10_4_t2h_msg_type {
 	HTT_10_4_T2H_MSG_TYPE_STATS_NOUPLOAD         = 0x18,
 	/* 0x19 to 0x2f are reserved */
 	HTT_10_4_T2H_MSG_TYPE_TX_MODE_SWITCH_IND     = 0x30,
+	HTT_10_4_T2H_MSG_TYPE_PEER_STATS	     = 0x31,
 	/* keep this last */
 	HTT_10_4_T2H_NUM_MSGS
 };
@@ -453,6 +454,7 @@ enum htt_t2h_msg_type {
 	HTT_T2H_MSG_TYPE_TX_FETCH_IND,
 	HTT_T2H_MSG_TYPE_TX_FETCH_CONFIRM,
 	HTT_T2H_MSG_TYPE_TX_MODE_SWITCH_IND,
+	HTT_T2H_MSG_TYPE_PEER_STATS,
 	/* keep this last */
 	HTT_T2H_NUM_MSGS
 };
@@ -1470,6 +1472,28 @@ struct htt_channel_change {
 	__le32 phymode;
 } __packed;
 
+struct htt_per_peer_tx_stats_ind {
+	__le32	succ_bytes;
+	__le32  retry_bytes;
+	__le32  failed_bytes;
+	u8	ratecode;
+	u8	flags;
+	__le16	peer_id;
+	__le16  succ_pkts;
+	__le16	retry_pkts;
+	__le16	failed_pkts;
+	__le16	tx_duration;
+	__le32	reserved1;
+	__le32	reserved2;
+} __packed;
+
+struct htt_peer_tx_stats {
+	u8 num_ppdu;
+	u8 ppdu_len;
+	u8 version;
+	u8 payload[0];
+} __packed;
+
 union htt_rx_pn_t {
 	/* WEP: 24-bit PN */
 	u32 pn24;
@@ -1521,6 +1545,7 @@ struct htt_resp {
 		struct htt_tx_fetch_confirm tx_fetch_confirm;
 		struct htt_tx_mode_switch_ind tx_mode_switch_ind;
 		struct htt_channel_change chan_change;
+		struct htt_peer_tx_stats peer_tx_stats;
 	};
 } __packed;
 
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 0b4c1562420f..ef28b358cf5e 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2194,6 +2194,128 @@ void ath10k_htt_htc_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 		dev_kfree_skb_any(skb);
 }
 
+static inline bool is_valid_legacy_rate(u8 rate)
+{
+	static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
+					  18, 24, 36, 48, 54};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
+		if (rate == legacy_rates[i])
+			return true;
+	}
+
+	return false;
+}
+
+static void
+ath10k_update_per_peer_tx_stats(struct ath10k *ar,
+				struct ieee80211_sta *sta,
+				struct ath10k_per_peer_tx_stats *peer_stats)
+{
+	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
+	u8 rate = 0, sgi;
+	struct rate_info txrate;
+
+	lockdep_assert_held(&ar->data_lock);
+
+	txrate.flags = ATH10K_HW_PREAMBLE(peer_stats->ratecode);
+	txrate.bw = ATH10K_HW_BW(peer_stats->flags);
+	txrate.nss = ATH10K_HW_NSS(peer_stats->ratecode);
+	txrate.mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
+	sgi = ATH10K_HW_GI(peer_stats->flags);
+
+	if (((txrate.flags == WMI_RATE_PREAMBLE_HT) ||
+	     (txrate.flags == WMI_RATE_PREAMBLE_VHT)) && txrate.mcs > 9) {
+		ath10k_warn(ar, "Invalid mcs %hhd peer stats", txrate.mcs);
+		return;
+	}
+
+	if (txrate.flags == WMI_RATE_PREAMBLE_CCK ||
+	    txrate.flags == WMI_RATE_PREAMBLE_OFDM) {
+		rate = ATH10K_HW_LEGACY_RATE(peer_stats->ratecode);
+
+		if (!is_valid_legacy_rate(rate)) {
+			ath10k_warn(ar, "Invalid legacy rate %hhd peer stats",
+				    rate);
+			return;
+		}
+
+		/* This is hacky, FW sends CCK rate 5.5Mbps as 6 */
+		rate *= 10;
+		if (rate == 60 && txrate.flags == WMI_RATE_PREAMBLE_CCK)
+			rate = rate - 5;
+		arsta->txrate.legacy = rate * 10;
+	} else if (txrate.flags == WMI_RATE_PREAMBLE_HT) {
+		arsta->txrate.flags = RATE_INFO_FLAGS_MCS;
+		arsta->txrate.mcs = txrate.mcs;
+	} else {
+		arsta->txrate.flags = RATE_INFO_FLAGS_VHT_MCS;
+		arsta->txrate.mcs = txrate.mcs;
+	}
+
+	if (sgi)
+		arsta->txrate.flags |= RATE_INFO_FLAGS_SHORT_GI;
+
+	arsta->txrate.nss = txrate.nss;
+	arsta->txrate.bw = txrate.bw + RATE_INFO_BW_20;
+}
+
+static void ath10k_htt_fetch_peer_stats(struct ath10k *ar,
+					struct sk_buff *skb)
+{
+	struct htt_resp *resp = (struct htt_resp *)skb->data;
+	struct ath10k_per_peer_tx_stats *p_tx_stats = &ar->peer_tx_stats;
+	struct htt_per_peer_tx_stats_ind *tx_stats;
+	struct ieee80211_sta *sta;
+	struct ath10k_peer *peer;
+	int peer_id, i;
+	u8 ppdu_len, num_ppdu;
+
+	num_ppdu = resp->peer_tx_stats.num_ppdu;
+	ppdu_len = resp->peer_tx_stats.ppdu_len * sizeof(__le32);
+
+	if (skb->len < sizeof(struct htt_resp_hdr) + num_ppdu * ppdu_len) {
+		ath10k_warn(ar, "Invalid peer stats buf length %d\n", skb->len);
+		return;
+	}
+
+	tx_stats = (struct htt_per_peer_tx_stats_ind *)
+			(resp->peer_tx_stats.payload);
+	peer_id = tx_stats->peer_id;
+
+	rcu_read_lock();
+	spin_lock_bh(&ar->data_lock);
+	peer = ath10k_peer_find_by_id(ar, peer_id);
+	if (!peer) {
+		ath10k_warn(ar, "Invalid peer id %d peer stats buffer\n",
+			    peer_id);
+		goto out;
+	}
+
+	sta = peer->sta;
+	for (i = 0; i < num_ppdu; i++) {
+		tx_stats = (struct htt_per_peer_tx_stats_ind *)
+			   (resp->peer_tx_stats.payload + i * ppdu_len);
+
+		p_tx_stats->succ_bytes = __le32_to_cpu(tx_stats->succ_bytes);
+		p_tx_stats->retry_bytes = __le32_to_cpu(tx_stats->retry_bytes);
+		p_tx_stats->failed_bytes =
+				__le32_to_cpu(tx_stats->failed_bytes);
+		p_tx_stats->ratecode = tx_stats->ratecode;
+		p_tx_stats->flags = tx_stats->flags;
+		p_tx_stats->succ_pkts = __le16_to_cpu(tx_stats->succ_pkts);
+		p_tx_stats->retry_pkts = __le16_to_cpu(tx_stats->retry_pkts);
+		p_tx_stats->failed_pkts = __le16_to_cpu(tx_stats->failed_pkts);
+
+		ath10k_update_per_peer_tx_stats(ar, sta, p_tx_stats);
+	}
+
+out:
+	spin_unlock_bh(&ar->data_lock);
+	rcu_read_unlock();
+}
+
 bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct ath10k_htt *htt = &ar->htt;
@@ -2354,6 +2476,9 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 	case HTT_T2H_MSG_TYPE_TX_MODE_SWITCH_IND:
 		ath10k_htt_rx_tx_mode_switch_ind(ar, skb);
 		break;
+	case HTT_T2H_MSG_TYPE_PEER_STATS:
+		ath10k_htt_fetch_peer_stats(ar, skb);
+		break;
 	case HTT_T2H_MSG_TYPE_EN_STATS:
 	default:
 		ath10k_warn(ar, "htt event (%d) not handled\n",
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 1b243c899bef..e108d49998c3 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -4603,9 +4603,17 @@ enum wmi_rate_preamble {
 
 #define ATH10K_HW_NSS(rate)		(1 + (((rate) >> 4) & 0x3))
 #define ATH10K_HW_PREAMBLE(rate)	(((rate) >> 6) & 0x3)
-#define ATH10K_HW_RATECODE(rate, nss, preamble)	\
+#define ATH10K_HW_MCS_RATE(rate)	((rate) & 0xf)
+#define ATH10K_HW_LEGACY_RATE(rate)	((rate) & 0x3f)
+#define ATH10K_HW_BW(flags)		(((flags) >> 3) & 0x3)
+#define ATH10K_HW_GI(flags)		(((flags) >> 5) & 0x1)
+#define ATH10K_HW_RATECODE(rate, nss, preamble) \
 	(((preamble) << 6) | ((nss) << 4) | (rate))
 
+#define VHT_MCS_NUM     10
+#define VHT_BW_NUM      4
+#define VHT_NSS_NUM     4
+
 /* Value to disable fixed rate setting */
 #define WMI_FIXED_RATE_NONE    (0xff)
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCHv2 0/2] ath10k: add support for tx bitrate
From: akolli @ 2016-10-18  9:29 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, akolli

From: Anilkumar Kolli <akolli@codeaurora.org>

This patch series adds support for tx bitrate using 
.sta_statistics callback.

tx bitrate tested on QCA4019 using iw.

Anilkumar Kolli (2):
  ath10k: add per peer htt tx stats support for 10.4
  ath10k: add support for per sta tx bitrate

 drivers/net/wireless/ath/ath10k/core.h        |   17 ++++
 drivers/net/wireless/ath/ath10k/debugfs_sta.c |   13 +++
 drivers/net/wireless/ath/ath10k/htt.c         |    2 +
 drivers/net/wireless/ath/ath10k/htt.h         |   25 +++++
 drivers/net/wireless/ath/ath10k/htt_rx.c      |  125 +++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h         |   10 +-
 6 files changed, 191 insertions(+), 1 deletion(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCHv2 0/2] ath10k: add support for tx bitrate
From: akolli @ 2016-10-18  9:26 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, akolli

From: Anilkumar Kolli <akolli@codeaurora.org>

This patch series adds support for tx bitrate using 
.sta_statistics callback.

tx bitrate tested on QCA4019 using iw.

Anilkumar Kolli (2):
  ath10k: add per peer htt tx stats support for 10.4
  ath10k: add support for per sta tx bitrate

 drivers/net/wireless/ath/ath10k/core.h        |   17 ++++
 drivers/net/wireless/ath/ath10k/debugfs_sta.c |   13 +++
 drivers/net/wireless/ath/ath10k/htt.c         |    2 +
 drivers/net/wireless/ath/ath10k/htt.h         |   25 +++++
 drivers/net/wireless/ath/ath10k/htt_rx.c      |  125 +++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h         |   10 +-
 6 files changed, 191 insertions(+), 1 deletion(-)

-- 
1.7.9.5

^ permalink raw reply

* [NOT FOR MERGE] ath9k: work around key cache corruption
From: Antonio Quartulli @ 2016-10-18  8:35 UTC (permalink / raw)
  To: ath9k-devel; +Cc: linux-wireless, sw, Antonio Quartulli

From: Antonio Quartulli <antonio@open-mesh.com>

This patch was crafted long time ago to work around a key cache
corruption problem on ath9k chipsets.

The workaround consists in periodically triggering a worker that
uploads all the keys to the HW cache. The worker is triggered also
when the vif detects 21 undecryptable packets.


This patch is based on top compat-wireless-2015-10-26.


I was asked to release this code to the public and, since it is
GPL, I am now doing it.


Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>

---
 drivers/net/wireless/ath/ath.h         |  11 +++
 drivers/net/wireless/ath/ath9k/ath9k.h |   7 ++
 drivers/net/wireless/ath/ath9k/init.c  |   1 +
 drivers/net/wireless/ath/ath9k/main.c  |  21 +++++
 drivers/net/wireless/ath/ath9k/recv.c  |  25 ++++++
 drivers/net/wireless/ath/key.c         | 149 +++++++++++++++++++++++++++++++++
 include/net/mac80211.h                 |   4 +
 net/mac80211/key.c                     |  22 ++---
 net/mac80211/key.h                     |   1 +
 net/mac80211/sta_info.c                |  13 +++
 10 files changed, 243 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 91eeca5..85377bc 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -187,6 +187,13 @@ struct ath_common {
 
 	int last_rssi;
 	struct ieee80211_supported_band sbands[IEEE80211_NUM_BANDS];
+
+	struct {
+		struct mutex mtx;
+		atomic_t running;
+		void (*refresh_cb)(struct ieee80211_sta *sta);
+		struct work_struct refresh_work;
+	} key_cache;
 };
 
 static inline const struct ath_ps_ops *ath_ps_ops(struct ath_common *common)
@@ -201,10 +208,14 @@ bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr);
 
 void ath_hw_setbssidmask(struct ath_common *common);
 void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key);
+void ath_keys_config(struct ath_common *common);
 int ath_key_config(struct ath_common *common,
 			  struct ieee80211_vif *vif,
 			  struct ieee80211_sta *sta,
 			  struct ieee80211_key_conf *key);
+void ath_key_refresher_init(struct ath_common *common,
+			    void (*cb)(struct ieee80211_sta *sta));
+void ath_key_refresher_start(struct ath_common *common);
 bool ath_hw_keyreset(struct ath_common *common, u16 entry);
 void ath_hw_cycle_counters_update(struct ath_common *common);
 int32_t ath_hw_get_listen_time(struct ath_common *common);
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 0aab323..3930962 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -23,6 +23,8 @@
 #include <linux/leds.h>
 #include <linux/completion.h>
 #include <linux/time.h>
+#include <linux/hw_random.h>
+#include <net/mac80211.h>
 
 #include "common.h"
 #include "debug.h"
@@ -38,6 +40,8 @@ extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
 extern int ath9k_use_chanctx;
 
+#define ATH_RX_DEC_MAX_ERR	20
+
 /*************************/
 /* Descriptor Management */
 /*************************/
@@ -266,6 +270,8 @@ struct ath_node {
 #endif
 	u8 key_idx[4];
 
+	atomic_t decrypt_errors;
+
 	u32 ackto;
 	struct list_head list;
 };
@@ -584,6 +590,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
 				   u16 tids, int nframes,
 				   enum ieee80211_frame_release_type reason,
 				   bool more_data);
+void ath9k_refresh_iter(struct ieee80211_sta *sta);
 
 /********/
 /* VIFs */
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 1b57d12..adec135 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -612,6 +612,7 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 		common->bt_ant_diversity = 1;
 
 	spin_lock_init(&common->cc_lock);
+	ath_key_refresher_init(common, ath9k_refresh_iter);
 	spin_lock_init(&sc->sc_serial_rw);
 	spin_lock_init(&sc->sc_pm_lock);
 	spin_lock_init(&sc->chan_lock);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 56520ac..8ddb0e0 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -16,6 +16,7 @@
 
 #include <linux/nl80211.h>
 #include <linux/delay.h>
+#include <net/mac80211.h>
 #include "ath9k.h"
 #include "btcoex.h"
 
@@ -1690,6 +1691,9 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 	if (sta)
 		an = (struct ath_node *)sta->drv_priv;
 
+	if (sta)
+		an = (struct ath_node *)sta->drv_priv;
+
 	switch (cmd) {
 	case SET_KEY:
 		if (sta)
@@ -1717,6 +1721,14 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 			}
 			WARN_ON(i == ARRAY_SIZE(an->key_idx));
 		}
+		/* QCA chips seems to have a known (and old) bug which corrupts
+		 * the key cache 'every now and then'. We observed that the
+		 * corruption happens after having uploaded a new (GTK) key. For
+		 * this reason we try to refresh the cache each time a key is
+		 * uploaded (we could do this after uploading the GTK only, but
+		 * in this way we try to catch more corruptions at a low cost).
+		 */
+		ath_key_refresher_start(common);
 		break;
 	case DISABLE_KEY:
 		ath_key_delete(common, key);
@@ -1740,6 +1752,15 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 	return ret;
 }
 
+void ath9k_refresh_iter(struct ieee80211_sta *sta)
+{
+	struct ath_node *an;
+
+	an = (struct ath_node *)sta->drv_priv;
+
+	atomic_set(&an->decrypt_errors, 0);
+}
+
 static void ath9k_bss_info_changed(struct ieee80211_hw *hw,
 				   struct ieee80211_vif *vif,
 				   struct ieee80211_bss_conf *bss_conf,
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index c9f5baf..e28185e 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -995,12 +995,16 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 	struct ieee80211_rx_status *rxs;
 	struct ath_hw *ah = sc->sc_ah;
 	struct ath_common *common = ath9k_hw_common(ah);
+	struct ath_node *an;
 	struct ieee80211_hw *hw = sc->hw;
+	/* TODO remove */
+	struct ieee80211_sta *sta;
 	int retval;
 	struct ath_rx_status rs;
 	enum ath9k_rx_qtype qtype;
 	bool edma = !!(ah->caps.hw_caps & ATH9K_HW_CAP_EDMA);
 	int dma_type;
+	u8 rx_status_len = ah->caps.rx_status_len;
 	u64 tsf = 0;
 	unsigned long flags;
 	dma_addr_t new_buf_addr;
@@ -1041,6 +1045,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		else
 			hdr_skb = skb;
 
+		hdr = (struct ieee80211_hdr *) (hdr_skb->data + rx_status_len);
 		rxs = IEEE80211_SKB_RXCB(hdr_skb);
 		memset(rxs, 0, sizeof(struct ieee80211_rx_status));
 
@@ -1049,6 +1054,26 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		if (retval)
 			goto requeue_drop_frag;
 
+		/* check if it is the case to refresh the entire key cache.. */
+		if (decrypt_error) {
+			int errors;
+
+			rcu_read_lock();
+			sta = ieee80211_find_sta_by_ifaddr(hw, hdr->addr2, NULL);
+			if (sta) {
+				an = (struct ath_node *)sta->drv_priv;
+				errors = atomic_inc_return(&an->decrypt_errors);
+				/* refresh the cache after some errors */
+				if (errors > ATH_RX_DEC_MAX_ERR) {
+					if (net_ratelimit())
+						printk("ath: %d decryption error for %pM - refreshing cache\n", errors, hdr->addr2);
+					atomic_set(&an->decrypt_errors, 0);
+					ath_key_refresher_start(common);
+				}
+			}
+			rcu_read_unlock();
+		}
+
 		/* Ensure we always have an skb to requeue once we are done
 		 * processing the current buffer's skb */
 		requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);
diff --git a/drivers/net/wireless/ath/key.c b/drivers/net/wireless/ath/key.c
index 1816b4e..7896ea6 100644
--- a/drivers/net/wireless/ath/key.c
+++ b/drivers/net/wireless/ath/key.c
@@ -18,6 +18,7 @@
 #include <linux/export.h>
 #include <asm/unaligned.h>
 #include <net/mac80211.h>
+#include "../../../../net/mac80211/key.h"
 
 #include "ath.h"
 #include "reg.h"
@@ -607,3 +608,151 @@ void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key)
 	}
 }
 EXPORT_SYMBOL(ath_key_delete);
+
+static int ath_key_refresh(struct ath_common *common, struct ieee80211_vif *vif,
+			   struct ieee80211_sta *sta,
+			   struct ieee80211_key_conf *key)
+{
+	struct ath_keyval hk;
+	const u8 *mac = NULL;
+	u8 gmac[ETH_ALEN];
+	int ret = 0;
+	int idx;
+
+	memset(&hk, 0, sizeof(hk));
+
+	switch (key->cipher) {
+	case 0:
+		hk.kv_type = ATH_CIPHER_CLR;
+		break;
+	case WLAN_CIPHER_SUITE_WEP40:
+	case WLAN_CIPHER_SUITE_WEP104:
+		hk.kv_type = ATH_CIPHER_WEP;
+		break;
+	case WLAN_CIPHER_SUITE_TKIP:
+		hk.kv_type = ATH_CIPHER_TKIP;
+		break;
+	case WLAN_CIPHER_SUITE_CCMP:
+		hk.kv_type = ATH_CIPHER_AES_CCM;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	hk.kv_len = key->keylen;
+	if (key->keylen)
+		memcpy(hk.kv_val, key->key, key->keylen);
+
+	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
+		switch (vif->type) {
+		case NL80211_IFTYPE_AP:
+			memcpy(gmac, vif->addr, ETH_ALEN);
+			gmac[0] |= 0x01;
+			mac = gmac;
+			break;
+		case NL80211_IFTYPE_ADHOC:
+			memcpy(gmac, sta->addr, ETH_ALEN);
+			gmac[0] |= 0x01;
+			mac = gmac;
+			break;
+		default:
+			break;
+		}
+	} else if (key->keyidx) {
+		if (WARN_ON(!sta))
+			return -EOPNOTSUPP;
+		mac = sta->addr;
+
+		if (vif->type == NL80211_IFTYPE_AP)
+			return -EIO;
+	} else {
+		if (WARN_ON(!sta))
+			return -EOPNOTSUPP;
+		mac = sta->addr;
+	}
+
+	/* get the index that is already used by this key */
+	idx = key->hw_key_idx;
+
+	if (key->cipher == WLAN_CIPHER_SUITE_TKIP)
+		ret = ath_setkey_tkip(common, idx, key->key, &hk, mac,
+				      vif->type == NL80211_IFTYPE_AP);
+	else
+		ret = ath_hw_set_keycache_entry(common, idx, &hk, mac);
+
+	if (!ret)
+		return -EIO;
+
+	return 0;
+}
+
+/**
+ * ath_key_config_iter - refresh one key in the cache
+ */
+static void ath_key_config_iter(struct ieee80211_hw *hw,
+				struct ieee80211_vif *vif,
+				struct ieee80211_sta *sta,
+				struct ieee80211_key_conf *keyconf, void *data)
+{
+	struct ath_common *common = data;
+	struct ieee80211_key *key;
+
+	key = container_of(keyconf, struct ieee80211_key, conf);
+
+	/* skip keys which were not programmed into the hardware */
+	if (!(key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE))
+		return;
+
+	/* delete and re-install the key */
+	ath_key_refresh(common, vif, sta, keyconf);
+}
+
+/**
+ * ath_key_refresher - refresh the entire key cache until it becomes sane
+ */
+static void ath_key_refresher(struct work_struct *work)
+{
+	struct ath_common *common;
+
+	common = container_of(work, struct ath_common, key_cache.refresh_work);
+
+	mutex_lock(&common->key_cache.mtx);
+	ieee80211_iter_keys(common->hw, NULL, ath_key_config_iter, common);
+	mutex_unlock(&common->key_cache.mtx);
+
+	/* invoke the driver callback to reset the private sta state */
+	ieee80211_for_each_sta(common->hw, common->key_cache.refresh_cb);
+
+	atomic_dec(&common->key_cache.running);
+}
+
+void ath_key_refresher_init(struct ath_common *common,
+			    void (*cb)(struct ieee80211_sta *sta))
+{
+	INIT_WORK(&common->key_cache.refresh_work, ath_key_refresher);
+	common->key_cache.refresh_cb = cb;
+	mutex_init(&common->key_cache.mtx);
+	atomic_set(&common->key_cache.running, 0);
+}
+EXPORT_SYMBOL(ath_key_refresher_init);
+
+/**
+ * starts a refresher worker for asynchronous cache refresh
+ */
+void ath_key_refresher_start(struct ath_common *common)
+{
+	if (!atomic_add_unless(&common->key_cache.running, 1, 1))
+		return;
+
+	ieee80211_queue_work(common->hw, &common->key_cache.refresh_work);
+}
+EXPORT_SYMBOL(ath_key_refresher_start);
+
+/**
+ * decrease the refcounter and possibly stop the key refresh worker
+ */
+/*void ath_key_refresher_stop(struct ath_common *common)
+{
+	cancel_work_sync(&common->key_cache.refresh_work);
+}
+EXPORT_SYMBOL(ath_key_refresher_stop);*/
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 19cde95..22d7164 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -4775,6 +4775,10 @@ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, const u8 *ra,
 struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
 					 const u8 *addr);
 
+/* iterate over stations */
+void ieee80211_for_each_sta(struct ieee80211_hw *hw,
+			    void (*iter)(struct ieee80211_sta *sta));
+
 /**
  * ieee80211_find_sta_by_ifaddr - find a station on hardware
  *
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 44388d6..6373c48 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -320,7 +320,7 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
 		return;
 
 	if (new)
-		list_add_tail(&new->list, &sdata->key_list);
+		list_add_tail_rcu(&new->list, &sdata->key_list);
 
 	WARN_ON(new && old && new->conf.keyidx != old->conf.keyidx);
 
@@ -368,7 +368,7 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (old)
-		list_del(&old->list);
+		list_del_rcu(&old->list);
 }
 
 struct ieee80211_key *
@@ -720,27 +720,27 @@ void ieee80211_iter_keys(struct ieee80211_hw *hw,
 			 void *iter_data)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
-	struct ieee80211_key *key, *tmp;
+	struct ieee80211_key *key;
 	struct ieee80211_sub_if_data *sdata;
 
-	ASSERT_RTNL();
-
-	mutex_lock(&local->key_mtx);
+	/* WARNING removed proper locking + _safe because only intel driver
+	 * depends on it and we need to avoid locking problems
+	 */
+	rcu_read_lock();
 	if (vif) {
 		sdata = vif_to_sdata(vif);
-		list_for_each_entry_safe(key, tmp, &sdata->key_list, list)
+		list_for_each_entry_rcu(key, &sdata->key_list, list)
 			iter(hw, &sdata->vif,
 			     key->sta ? &key->sta->sta : NULL,
 			     &key->conf, iter_data);
 	} else {
-		list_for_each_entry(sdata, &local->interfaces, list)
-			list_for_each_entry_safe(key, tmp,
-						 &sdata->key_list, list)
+		list_for_each_entry_rcu(sdata, &local->interfaces, list)
+			list_for_each_entry_rcu(key, &sdata->key_list, list)
 				iter(hw, &sdata->vif,
 				     key->sta ? &key->sta->sta : NULL,
 				     &key->conf, iter_data);
 	}
-	mutex_unlock(&local->key_mtx);
+	rcu_read_unlock();
 }
 EXPORT_SYMBOL(ieee80211_iter_keys);
 
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index 5d9e028..d18b132 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -55,6 +55,7 @@ struct ieee80211_key {
 	struct ieee80211_local *local;
 	struct ieee80211_sub_if_data *sdata;
 	struct sta_info *sta;
+	struct work_struct work;
 
 	/* for sdata list */
 	struct list_head list;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 6eed9db..ccf0634 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1109,6 +1109,19 @@ struct ieee80211_sta *ieee80211_find_sta_by_ifaddr(struct ieee80211_hw *hw,
 }
 EXPORT_SYMBOL_GPL(ieee80211_find_sta_by_ifaddr);
 
+void ieee80211_for_each_sta(struct ieee80211_hw *hw,
+			    void (*iter)(struct ieee80211_sta *sta))
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct sta_info *sta;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(sta, &local->sta_list, list)
+		iter(&sta->sta);
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL(ieee80211_for_each_sta);
+
 struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
 					 const u8 *addr)
 {

^ permalink raw reply related

* [PATCH] wireless: deprecate WDS and disable by default
From: Johannes Berg @ 2016-10-18  8:28 UTC (permalink / raw)
  To: linux-wireless
  Cc: Stanislaw Gruszka, Helmut Schaa, b43-dev, Larry Finger,
	ath9k-devel, ath9k-devel, Johannes Berg

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

The old WDS 4-addr frame support is very limited, e.g.
 * no encryption is possible on such links
 * it cannot support rate/HT/VHT negotiation
 * management APIs are very restricted

These make the WDS legacy mode useless in practice.

All of these are resolved by the 4-addr AP/client support,
so there's also no reason to improve WDS in the future.

Therefore, add a Kconfig option to disable legacy WDS.
This gives people an "emergency valve" while they migrate
to the better-supported 4-addr AP/client option; we plan
to remove it (and the associated cfg80211/mac80211 code,
which is the ultimate goal) in the future.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/Kconfig                   | 13 +++++++++++++
 drivers/net/wireless/ath/ath9k/init.c          |  6 ++++++
 drivers/net/wireless/broadcom/b43/main.c       |  2 ++
 drivers/net/wireless/broadcom/b43legacy/main.c |  2 ++
 drivers/net/wireless/ralink/rt2x00/rt2x00dev.c |  6 ++++--
 net/wireless/core.c                            | 10 ++++++++++
 6 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
index 8c8edaf1bba6..fa9c4fa9477e 100644
--- a/drivers/net/wireless/Kconfig
+++ b/drivers/net/wireless/Kconfig
@@ -17,6 +17,19 @@ menuconfig WLAN
 
 if WLAN
 
+config WIRELESS_WDS
+	bool "mac80211-based legacy WDS support"
+	help
+	  This option enables the deprecated WDS support, the newer
+	  mac80211-based 4-addr AP/client support supersedes it with
+	  a much better feature set (HT, VHT, ...)
+
+	  We plan to remove this option and code, so if you find
+	  that you have to enable it, please let us know on the
+	  linux-wireless@vger.kernel.org mailing list, so we can
+	  help you migrate to 4-addr AP/client (or, if it's really
+	  necessary, give up on our plan of removing it).
+
 source "drivers/net/wireless/admtek/Kconfig"
 source "drivers/net/wireless/ath/Kconfig"
 source "drivers/net/wireless/atmel/Kconfig"
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index cfa3fe82ade3..368d9b313823 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -734,9 +734,11 @@ static const struct ieee80211_iface_limit if_limits[] = {
 				 BIT(NL80211_IFTYPE_P2P_GO) },
 };
 
+#ifdef CONFIG_WIRELESS_WDS
 static const struct ieee80211_iface_limit wds_limits[] = {
 	{ .max = 2048,	.types = BIT(NL80211_IFTYPE_WDS) },
 };
+#endif
 
 #ifdef CONFIG_ATH9K_CHANNEL_CONTEXT
 
@@ -774,6 +776,7 @@ static const struct ieee80211_iface_combination if_comb[] = {
 					BIT(NL80211_CHAN_WIDTH_40),
 #endif
 	},
+#ifdef CONFIG_WIRELESS_WDS
 	{
 		.limits = wds_limits,
 		.n_limits = ARRAY_SIZE(wds_limits),
@@ -781,6 +784,7 @@ static const struct ieee80211_iface_combination if_comb[] = {
 		.num_different_channels = 1,
 		.beacon_int_infra_match = true,
 	},
+#endif
 };
 
 #ifdef CONFIG_ATH9K_CHANNEL_CONTEXT
@@ -851,7 +855,9 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 			BIT(NL80211_IFTYPE_STATION) |
 			BIT(NL80211_IFTYPE_ADHOC) |
 			BIT(NL80211_IFTYPE_MESH_POINT) |
+#ifdef CONFIG_WIRELESS_WDS
 			BIT(NL80211_IFTYPE_WDS) |
+#endif
 			BIT(NL80211_IFTYPE_OCB);
 
 		if (ath9k_is_chanctx_enabled())
diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c
index 6e5d9095b195..52f3541ecbcf 100644
--- a/drivers/net/wireless/broadcom/b43/main.c
+++ b/drivers/net/wireless/broadcom/b43/main.c
@@ -5591,7 +5591,9 @@ static struct b43_wl *b43_wireless_init(struct b43_bus_dev *dev)
 		BIT(NL80211_IFTYPE_AP) |
 		BIT(NL80211_IFTYPE_MESH_POINT) |
 		BIT(NL80211_IFTYPE_STATION) |
+#ifdef CONFIG_WIRELESS_WDS
 		BIT(NL80211_IFTYPE_WDS) |
+#endif
 		BIT(NL80211_IFTYPE_ADHOC);
 
 	hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c b/drivers/net/wireless/broadcom/b43legacy/main.c
index 83770d2ea057..e97ab2b91663 100644
--- a/drivers/net/wireless/broadcom/b43legacy/main.c
+++ b/drivers/net/wireless/broadcom/b43legacy/main.c
@@ -3838,7 +3838,9 @@ static int b43legacy_wireless_init(struct ssb_device *dev)
 	hw->wiphy->interface_modes =
 		BIT(NL80211_IFTYPE_AP) |
 		BIT(NL80211_IFTYPE_STATION) |
+#ifdef CONFIG_WIRELESS_WDS
 		BIT(NL80211_IFTYPE_WDS) |
+#endif
 		BIT(NL80211_IFTYPE_ADHOC);
 	hw->queues = 1; /* FIXME: hardware has more queues */
 	hw->max_rates = 2;
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 4e0c5653054b..236f790e097a 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -1362,11 +1362,13 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
 	if (rt2x00dev->bcn->limit > 0)
 		rt2x00dev->hw->wiphy->interface_modes |=
 		    BIT(NL80211_IFTYPE_ADHOC) |
-		    BIT(NL80211_IFTYPE_AP) |
 #ifdef CONFIG_MAC80211_MESH
 		    BIT(NL80211_IFTYPE_MESH_POINT) |
 #endif
-		    BIT(NL80211_IFTYPE_WDS);
+#ifdef CONFIG_WIRELESS_WDS
+		    BIT(NL80211_IFTYPE_WDS) |
+#endif
+		    BIT(NL80211_IFTYPE_AP);
 
 	rt2x00dev->hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
 
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 8201e6d7449e..f433f15f9222 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -571,6 +571,11 @@ static int wiphy_verify_combinations(struct wiphy *wiphy)
 				return -EINVAL;
 		}
 
+#ifndef CONFIG_WIRELESS_WDS
+		if (WARN_ON(all_iftypes & BIT(NL80211_IFTYPE_WDS)))
+			return -EINVAL;
+#endif
+
 		/* You can't even choose that many! */
 		if (WARN_ON(cnt < c->max_interfaces))
 			return -EINVAL;
@@ -609,6 +614,11 @@ int wiphy_register(struct wiphy *wiphy)
 		     !rdev->ops->add_nan_func || !rdev->ops->del_nan_func)))
 		return -EINVAL;
 
+#ifndef CONFIG_WIRELESS_WDS
+	if (WARN_ON(wiphy->interface_modes & BIT(NL80211_IFTYPE_WDS)))
+		return -EINVAL;
+#endif
+
 	/*
 	 * if a wiphy has unsupported modes for regulatory channel enforcement,
 	 * opt-out of enforcement checking
-- 
2.8.1

^ permalink raw reply related

* pull-request: mac80211 2016-10-18
From: Johannes Berg @ 2016-10-18  7:00 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-wireless

Hi Dave,

As I mention in the tag message, the most urgent fix here is for
the VMAP_STACK vs. software crypto usage. I ended up applying Ard's
fix that dynamically allocates everything in one go, perhaps we'll
find a better solution in the future, but in the meantime this will
get things working again (rather than crashing or getting BUG_ON),
and normally it's a rarely used code path since hardware crypto is
used for almost all devices.

Please pull, or let me know if there's any problem.

Thanks,
johannes



The following changes since commit 6b25e21fa6f26d0f0d45f161d169029411c84286:

  Merge tag 'drm-for-v4.9' of git://people.freedesktop.org/~airlied/linux (2016-10-11 18:12:22 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-davem-2016-10-18

for you to fetch changes up to f4a067f9ffca603b45f7e82ddd2ba50e5904cea3:

  mac80211: move struct aead_req off the stack (2016-10-17 16:14:04 +0200)

----------------------------------------------------------------
This is relatively small, mostly to get the SG/crypto
from stack removal fix that crashes things when VMAP
stack is used in conjunction with software crypto.

Aside from that, we have:
 * a fix for AP_VLAN usage with the nl80211 frame command
 * two fixes (and two preparation patches) for A-MSDU, one
   to discard group-addressed (multicast) and unexpected
   4-address A-MSDUs, the other to validate A-MSDU inner
   MAC addresses properly to prevent controlled port bypass

----------------------------------------------------------------
Ard Biesheuvel (1):
      mac80211: move struct aead_req off the stack

Johannes Berg (4):
      mac80211: discard multicast and 4-addr A-MSDUs
      cfg80211: let ieee80211_amsdu_to_8023s() take only header-less SKB
      cfg80211: add ability to check DA/SA in A-MSDU decapsulation
      mac80211: validate DA/SA during A-MSDU decapsulation

Michael Braun (1):
      mac80211: fix CMD_FRAME for AP_VLAN

 .../net/wireless/marvell/mwifiex/11n_rxreorder.c   |  2 +-
 include/net/cfg80211.h                             | 32 ++++++++++----
 net/mac80211/aes_ccm.c                             | 46 ++++++++++++-------
 net/mac80211/aes_ccm.h                             |  8 ++--
 net/mac80211/aes_gcm.c                             | 43 +++++++++++-------
 net/mac80211/aes_gcm.h                             |  6 ++-
 net/mac80211/aes_gmac.c                            | 26 +++++------
 net/mac80211/aes_gmac.h                            |  4 ++
 net/mac80211/offchannel.c                          |  2 +-
 net/mac80211/rx.c                                  | 51 +++++++++++++++++-----
 net/mac80211/wpa.c                                 | 22 ++++------
 net/wireless/util.c                                | 34 +++++++--------
 12 files changed, 175 insertions(+), 101 deletions(-)

^ permalink raw reply

* Re: [PATCH -next] cfg80211: fix possible memory leak in cfg80211_iter_combinations()
From: Johannes Berg @ 2016-10-18  6:51 UTC (permalink / raw)
  To: Wei Yongjun, Purushottam Kushwaha; +Cc: Wei Yongjun, linux-wireless
In-Reply-To: <1476717916-23302-1-git-send-email-weiyj.lk@gmail.com>

On Mon, 2016-10-17 at 15:25 +0000, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
> 
> 'limits' is malloced in cfg80211_iter_combinations() and should be
> freed
> before leaving from the error handling cases, otherwise it will cause
> memory leak.

Yep, thanks; applied.

johannes

^ permalink raw reply

* Re: [PATCH 1/2] ath10k: add per peer htt tx stats support for 10.4
From: akolli @ 2016-10-18  5:38 UTC (permalink / raw)
  To: Valo, Kalle; +Cc: Kolli, Anilkumar, ath10k, linux-wireless
In-Reply-To: <87vawygo2w.fsf@kamboji.qca.qualcomm.com>


On 2016-10-12 10:29, Valo, Kalle wrote:
> <akolli@qti.qualcomm.com> writes:
> 
>> From: Anilkumar Kolli <akolli@qti.qualcomm.com>
>> 
>> Per peer tx stats are part of 'HTT_10_4_T2H_MSG_TYPE_PEER_STATS'
>> event, Firmware sends one HTT event for every four PPDUs.
>> HTT payload has success pkts/bytes, failed pkts/bytes, retry
>> pkts/bytes and rate info per ppdu.
>> Peer stats are enabled through 'WMI_SERVICE_PEER_STATS',
>> which are nowadays enabled by default.
>> 
>> Parse peer stats and update the tx rate information per STA.
>> 
>> tx rate, Peer stats are tested on QCA4019 with Firmware version
>> 10.4-3.2.1-00028.
>> 
>> Signed-off-by: Anilkumar Kolli <akolli@qti.qualcomm.com>
> 
> This and patch 2 add few new warnings:
> 
> drivers/net/wireless/ath/ath10k/htt_rx.c: In function
> 'ath10k_htt_fetch_peer_stats':
> drivers/net/wireless/ath/ath10k/htt_rx.c:2279:3: warning: too many
> arguments for format [-Wformat-extra-args]
> drivers/net/wireless/ath/ath10k/htt_rx.c:2285:17: warning: incorrect
> type in assignment (different base types)
> drivers/net/wireless/ath/ath10k/htt_rx.c:2285:17:    expected unsigned
> int [unsigned] [usertype] peer_id
> drivers/net/wireless/ath/ath10k/htt_rx.c:2285:17:    got restricted
> __le16 [usertype] peer_id
> drivers/net/wireless/ath/ath10k/debugfs_sta.c:84: space required
> before the open parenthesis '('

I will fix and send V2.

Thanks,
Anil.

^ permalink raw reply

* [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Hante Meuleman,
	Kalle Valo, Franky Lin, Pieter-Paul Giesberts,
	Franky (Zhenhui) Lin, Rafał Miłecki, linux-wireless,
	brcm80211-dev-list.pdl, netdev
In-Reply-To: <20161017220342.1627073-1-arnd@arndb.de>

A bugfix added a sanity check around the assignment and use of the
'is_11d' variable, which looks correct to me, but as the function is
rather complex already, this confuses the compiler to the point where
it can no longer figure out if the variable is always initialized
correctly:

brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds an initialization for the newly introduced case in which
the variable should not really be used, in order to make the warning
go away.

Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
Cc: Hante Meuleman <hante.meuleman@broadcom.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b777e1b..78d9966 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4516,7 +4516,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 	/* store current 11d setting */
 	if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
 				  &ifp->vif->is_11d)) {
-		supports_11d = false;
+		is_11d = supports_11d = false;
 	} else {
 		country_ie = brcmf_parse_tlvs((u8 *)settings->beacon.tail,
 					      settings->beacon.tail_len,
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH] mac80211_hwsim: suggest nl80211 instead of wext driver in documentation
From: Linus Lüssing @ 2016-10-17 18:14 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, Jouni Malinen, David S . Miller, Jonathan Corbet,
	netdev, linux-kernel, linux-doc
In-Reply-To: <1476697144.19992.16.camel@sipsolutions.net>

On Mon, Oct 17, 2016 at 11:39:04AM +0200, Johannes Berg wrote:
> On Mon, 2016-10-17 at 00:39 +0200, Linus Lüssing wrote:
> > For mac80211_hwsim interfaces, suggest to use wpa_supplicant with the
> > more modern, netlink based driver instead of wext.
> 
> Makes sense, applied.
> 
> > Actually, I wasn't even able to make a connection with the
> > configuration
> > files and information provided in
> > Documentation/networking/mac80211_hwsim/{README,hostapd.conf/wpa_supp
> > licant.conf}
> > 
> This less so, we even have a few test cases we run regularly, but I
> don't know. Maybe there's something special in those configuration
> files that we don't test for otherwise.

I investigated a little further and the problem is probably that
I'm running a minimal Linux kernel which was compiled without
CONFIG_CFG80211_WEXT :-).

Anyways, thanks for applying the patch!

Regards, Linus

^ permalink raw reply

* Re: [PATCH] crypto: ccm - avoid scatterlist for MAC encryption
From: Ard Biesheuvel @ 2016-10-17 17:21 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Johannes Berg, Sergey Senozhatsky, <netdev@vger.kernel.org>,
	Herbert Xu, David S. Miller,
	<linux-wireless@vger.kernel.org>,
	linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CALCETrUWW2A1XK7BzcajTcBzg58SDUCTE35Hj1vmbywTLCe2vQ@mail.gmail.com>

On 17 October 2016 at 18:08, Andy Lutomirski <luto@amacapital.net> wrote:
> On Mon, Oct 17, 2016 at 12:37 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> On 17 October 2016 at 08:28, Johannes Berg <johannes@sipsolutions.net> wrote:
>>> On Sat, 2016-10-15 at 18:16 +0100, Ard Biesheuvel wrote:
>>>> The CCM code goes out of its way to perform the CTR encryption of the
>>>> MAC using the subordinate CTR driver. To this end, it tweaks the
>>>> input and output scatterlists so the aead_req 'odata' and/or
>>>> 'auth_tag' fields [which may live on the stack] are prepended to the
>>>> CTR payload. This involves calling sg_set_buf() on addresses which
>>>> are not direct mapped, which is not supported.
>>>
>>>> Since the calculation of the MAC keystream involves a single call
>>>> into the cipher, to which we have a handle already given that the
>>>> CBC-MAC calculation uses it as well, just calculate the MAC keystream
>>>> directly, and record it in the aead_req private context so we can
>>>> apply it to the MAC in cypto_ccm_auth_mac(). This greatly simplifies
>>>> the scatterlist manipulation, and no longer requires scatterlists to
>>>> refer to buffers that may live on the stack.
>>>
>>> No objection from me, Herbert?
>>>
>>> I'm getting a bit nervous though - I'd rather have any fix first so
>>> people get things working again - so maybe I'll apply your other patch
>>> and mine first, and then we can replace yours by this later.
>>>
>>
>> Could we get a statement first whether it is supported to allocate
>> aead_req (and other crypto req structures) on the stack? If not, then
>> we have our work cut out for us. But if it is, I'd rather we didn't
>> apply the kzalloc/kfree patch, since it is just a workaround for the
>> broken generic CCM driver, for which a fix is already available.
>
> I'm not a crypto person, but I don't see why not.  There's even a
> helper called SKCIPHER_REQUEST_ON_STACK. :)  The only problem I know
> of is pointing a scatterlist at the stack, which is bad for much the
> same reason as doing real DMA from the stack.

Excellent point!

So the CCM code was an easy fix, although the RFC4309 part is still broken:

It does

"""
scatterwalk_map_and_copy(iv + 16, req->src, 0, req->assoclen - 8, 0);
...
sg_set_buf(rctx->src, iv + 16, req->assoclen - 8);
sg = scatterwalk_ffwd(rctx->src + 1, req->src, req->assoclen);
"""

which essentially just hides the last 8 bytes of associated data from
the inner CCM transform. So we'd need to rewrite this part to create a
new scatterlist that omits those 8 bytes instead of just replacing the
first sg entry and point it to another buffer entirely (and copy the
data into it)

The GCM code is much more complicated, and does not easily allow the
offending sg_set_buf() calls to be turned into something that only
involves direct mapped memory references. I haven't looked at anything
else.

Annoyingly, all this complication with scatterlists etc is for doing
asynchronous crypto via DMA capable crypto accelerators, and the
networking code (ipsec as well as mac80211, afaik) only allow
synchronous in the first place, given that they execute in softirq
context.

^ permalink raw reply

* Re: [PATCH] crypto: ccm - avoid scatterlist for MAC encryption
From: Andy Lutomirski @ 2016-10-17 17:08 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Johannes Berg, Sergey Senozhatsky, <netdev@vger.kernel.org>,
	Herbert Xu, David S. Miller,
	<linux-wireless@vger.kernel.org>,
	linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu_P-6sHW3t7tp1N+3v9ZGqWcQrzMZkASY5g=QR76KN4wg@mail.gmail.com>

On Mon, Oct 17, 2016 at 12:37 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 17 October 2016 at 08:28, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Sat, 2016-10-15 at 18:16 +0100, Ard Biesheuvel wrote:
>>> The CCM code goes out of its way to perform the CTR encryption of the
>>> MAC using the subordinate CTR driver. To this end, it tweaks the
>>> input and output scatterlists so the aead_req 'odata' and/or
>>> 'auth_tag' fields [which may live on the stack] are prepended to the
>>> CTR payload. This involves calling sg_set_buf() on addresses which
>>> are not direct mapped, which is not supported.
>>
>>> Since the calculation of the MAC keystream involves a single call
>>> into the cipher, to which we have a handle already given that the
>>> CBC-MAC calculation uses it as well, just calculate the MAC keystream
>>> directly, and record it in the aead_req private context so we can
>>> apply it to the MAC in cypto_ccm_auth_mac(). This greatly simplifies
>>> the scatterlist manipulation, and no longer requires scatterlists to
>>> refer to buffers that may live on the stack.
>>
>> No objection from me, Herbert?
>>
>> I'm getting a bit nervous though - I'd rather have any fix first so
>> people get things working again - so maybe I'll apply your other patch
>> and mine first, and then we can replace yours by this later.
>>
>
> Could we get a statement first whether it is supported to allocate
> aead_req (and other crypto req structures) on the stack? If not, then
> we have our work cut out for us. But if it is, I'd rather we didn't
> apply the kzalloc/kfree patch, since it is just a workaround for the
> broken generic CCM driver, for which a fix is already available.

I'm not a crypto person, but I don't see why not.  There's even a
helper called SKCIPHER_REQUEST_ON_STACK. :)  The only problem I know
of is pointing a scatterlist at the stack, which is bad for much the
same reason as doing real DMA from the stack.

^ permalink raw reply

* [PATCH -next] cfg80211: fix possible memory leak in cfg80211_iter_combinations()
From: Wei Yongjun @ 2016-10-17 15:25 UTC (permalink / raw)
  To: Johannes Berg, Purushottam Kushwaha; +Cc: Wei Yongjun, linux-wireless

From: Wei Yongjun <weiyongjun1@huawei.com>

'limits' is malloced in cfg80211_iter_combinations() and should be freed
before leaving from the error handling cases, otherwise it will cause
memory leak.

Fixes: 0c317a02ca98 ("cfg80211: support virtual interfaces with different
beacon intervals")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 net/wireless/util.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/wireless/util.c b/net/wireless/util.c
index dd545ff..e36dbae 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1672,8 +1672,10 @@ int cfg80211_iter_combinations(struct wiphy *wiphy,
 
 		if (params->beacon_int_gcd) {
 			if (c->beacon_int_min_gcd &&
-			    params->beacon_int_gcd < c->beacon_int_min_gcd)
+			    params->beacon_int_gcd < c->beacon_int_min_gcd) {
+				kfree(limits);
 				return -EINVAL;
+			}
 			if (!c->beacon_int_min_gcd &&
 			    params->beacon_int_different)
 				goto cont;

^ permalink raw reply related

* Re: [PATCH v4] mac80211: move struct aead_req off the stack
From: Johannes Berg @ 2016-10-17 14:17 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-wireless, netdev; +Cc: herbert, sergey.senozhatsky.work
In-Reply-To: <1476713133-18355-1-git-send-email-ard.biesheuvel@linaro.org>

On Mon, 2016-10-17 at 15:05 +0100, Ard Biesheuvel wrote:
> From: Johannes Berg <johannes.berg@intel.com>

That really ought to have been you - I think I may have lost that by
accident. I'll change it.

> In addition, take care not to put any of our own stack allocations
> into
> scatterlists. This involves reserving some extra room when allocating
> the
> aead_request structures, and referring to those allocations in the
> scatter-
> lists (while copying the data from/to the stack before/after the
> crypto
> operation, as appropriate)

I removed the "to the stack after" language, since that doesn't
actually occur :)


> --- a/net/mac80211/aes_cmac.h
> +++ b/net/mac80211/aes_cmac.h
> @@ -11,6 +11,8 @@
>  
>  #include <linux/crypto.h>
>  
> +#define CMAC_AAD_LEN	20

This part (and aes_cmac.c changes) shouldn't have been there, but that
was my mistake - removed.

With that, I've applied this. Will send it to davem tomorrow.

johannes

^ permalink raw reply

* [PATCH v4] mac80211: move struct aead_req off the stack
From: Ard Biesheuvel @ 2016-10-17 14:05 UTC (permalink / raw)
  To: linux-wireless, johannes, netdev
  Cc: herbert, sergey.senozhatsky.work, Johannes Berg, Ard Biesheuvel

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

Some crypto implementations (such as the generic CCM wrapper in crypto/)
use scatterlists to map fields of private data in their struct aead_req.
This means these data structures cannot live in the vmalloc area, which
means that they cannot live on the stack (with CONFIG_VMAP_STACK.)

This currently occurs only with the generic software implementation, but
the private data and usage is implementation specific, so move the whole
data structures off the stack into heap by allocating every time we need
to use them.

In addition, take care not to put any of our own stack allocations into
scatterlists. This involves reserving some extra room when allocating the
aead_request structures, and referring to those allocations in the scatter-
lists (while copying the data from/to the stack before/after the crypto
operation, as appropriate)

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 net/mac80211/aes_ccm.c  | 46 +++++++++++++-------
 net/mac80211/aes_ccm.h  |  8 ++--
 net/mac80211/aes_cmac.c |  5 +--
 net/mac80211/aes_cmac.h |  2 +
 net/mac80211/aes_gcm.c  | 43 +++++++++++-------
 net/mac80211/aes_gcm.h  |  6 ++-
 net/mac80211/aes_gmac.c | 26 +++++------
 net/mac80211/aes_gmac.h |  4 ++
 net/mac80211/wpa.c      | 22 ++++------
 9 files changed, 97 insertions(+), 65 deletions(-)

diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
index 7663c28ba353..a4e0d59a40dd 100644
--- a/net/mac80211/aes_ccm.c
+++ b/net/mac80211/aes_ccm.c
@@ -18,21 +18,24 @@
 #include "key.h"
 #include "aes_ccm.h"
 
-void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
-			       u8 *data, size_t data_len, u8 *mic,
-			       size_t mic_len)
+int ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
+			      u8 *data, size_t data_len, u8 *mic,
+			      size_t mic_len)
 {
 	struct scatterlist sg[3];
+	struct aead_request *aead_req;
+	int reqsize = sizeof(*aead_req) + crypto_aead_reqsize(tfm);
+	u8 *__aad;
 
-	char aead_req_data[sizeof(struct aead_request) +
-			   crypto_aead_reqsize(tfm)]
-		__aligned(__alignof__(struct aead_request));
-	struct aead_request *aead_req = (void *) aead_req_data;
+	aead_req = kzalloc(reqsize + CCM_AAD_LEN, GFP_ATOMIC);
+	if (!aead_req)
+		return -ENOMEM;
 
-	memset(aead_req, 0, sizeof(aead_req_data));
+	__aad = (u8 *)aead_req + reqsize;
+	memcpy(__aad, aad, CCM_AAD_LEN);
 
 	sg_init_table(sg, 3);
-	sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
+	sg_set_buf(&sg[0], &__aad[2], be16_to_cpup((__be16 *)__aad));
 	sg_set_buf(&sg[1], data, data_len);
 	sg_set_buf(&sg[2], mic, mic_len);
 
@@ -41,6 +44,9 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
 	aead_request_set_ad(aead_req, sg[0].length);
 
 	crypto_aead_encrypt(aead_req);
+	kzfree(aead_req);
+
+	return 0;
 }
 
 int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
@@ -48,18 +54,23 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
 			      size_t mic_len)
 {
 	struct scatterlist sg[3];
-	char aead_req_data[sizeof(struct aead_request) +
-			   crypto_aead_reqsize(tfm)]
-		__aligned(__alignof__(struct aead_request));
-	struct aead_request *aead_req = (void *) aead_req_data;
+	struct aead_request *aead_req;
+	int reqsize = sizeof(*aead_req) + crypto_aead_reqsize(tfm);
+	u8 *__aad;
+	int err;
 
 	if (data_len == 0)
 		return -EINVAL;
 
-	memset(aead_req, 0, sizeof(aead_req_data));
+	aead_req = kzalloc(reqsize + CCM_AAD_LEN, GFP_ATOMIC);
+	if (!aead_req)
+		return -ENOMEM;
+
+	__aad = (u8 *)aead_req + reqsize;
+	memcpy(__aad, aad, CCM_AAD_LEN);
 
 	sg_init_table(sg, 3);
-	sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
+	sg_set_buf(&sg[0], &__aad[2], be16_to_cpup((__be16 *)__aad));
 	sg_set_buf(&sg[1], data, data_len);
 	sg_set_buf(&sg[2], mic, mic_len);
 
@@ -67,7 +78,10 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
 	aead_request_set_crypt(aead_req, sg, sg, data_len + mic_len, b_0);
 	aead_request_set_ad(aead_req, sg[0].length);
 
-	return crypto_aead_decrypt(aead_req);
+	err = crypto_aead_decrypt(aead_req);
+	kzfree(aead_req);
+
+	return err;
 }
 
 struct crypto_aead *ieee80211_aes_key_setup_encrypt(const u8 key[],
diff --git a/net/mac80211/aes_ccm.h b/net/mac80211/aes_ccm.h
index 6a73d1e4d186..fcd3254c5cf0 100644
--- a/net/mac80211/aes_ccm.h
+++ b/net/mac80211/aes_ccm.h
@@ -12,12 +12,14 @@
 
 #include <linux/crypto.h>
 
+#define CCM_AAD_LEN	32
+
 struct crypto_aead *ieee80211_aes_key_setup_encrypt(const u8 key[],
 						    size_t key_len,
 						    size_t mic_len);
-void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
-			       u8 *data, size_t data_len, u8 *mic,
-			       size_t mic_len);
+int ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
+			      u8 *data, size_t data_len, u8 *mic,
+			      size_t mic_len);
 int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
 			      u8 *data, size_t data_len, u8 *mic,
 			      size_t mic_len);
diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index bdf0790d89cc..ebb8c2dc9928 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -20,7 +20,6 @@
 
 #define CMAC_TLEN 8 /* CMAC TLen = 64 bits (8 octets) */
 #define CMAC_TLEN_256 16 /* CMAC TLen = 128 bits (16 octets) */
-#define AAD_LEN 20
 
 
 static void gf_mulx(u8 *pad)
@@ -101,7 +100,7 @@ void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
 
 	memset(zero, 0, CMAC_TLEN);
 	addr[0] = aad;
-	len[0] = AAD_LEN;
+	len[0] = CMAC_AAD_LEN;
 	addr[1] = data;
 	len[1] = data_len - CMAC_TLEN;
 	addr[2] = zero;
@@ -119,7 +118,7 @@ void ieee80211_aes_cmac_256(struct crypto_cipher *tfm, const u8 *aad,
 
 	memset(zero, 0, CMAC_TLEN_256);
 	addr[0] = aad;
-	len[0] = AAD_LEN;
+	len[0] = CMAC_AAD_LEN;
 	addr[1] = data;
 	len[1] = data_len - CMAC_TLEN_256;
 	addr[2] = zero;
diff --git a/net/mac80211/aes_cmac.h b/net/mac80211/aes_cmac.h
index 3702041f44fd..0afb17aa2914 100644
--- a/net/mac80211/aes_cmac.h
+++ b/net/mac80211/aes_cmac.h
@@ -11,6 +11,8 @@
 
 #include <linux/crypto.h>
 
+#define CMAC_AAD_LEN	20
+
 struct crypto_cipher *ieee80211_aes_cmac_key_setup(const u8 key[],
 						   size_t key_len);
 void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
diff --git a/net/mac80211/aes_gcm.c b/net/mac80211/aes_gcm.c
index 3afe361fd27c..8a4397cc1b08 100644
--- a/net/mac80211/aes_gcm.c
+++ b/net/mac80211/aes_gcm.c
@@ -15,20 +15,23 @@
 #include "key.h"
 #include "aes_gcm.h"
 
-void ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
-			       u8 *data, size_t data_len, u8 *mic)
+int ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
+			      u8 *data, size_t data_len, u8 *mic)
 {
 	struct scatterlist sg[3];
+	struct aead_request *aead_req;
+	int reqsize = sizeof(*aead_req) + crypto_aead_reqsize(tfm);
+	u8 *__aad;
 
-	char aead_req_data[sizeof(struct aead_request) +
-			   crypto_aead_reqsize(tfm)]
-		__aligned(__alignof__(struct aead_request));
-	struct aead_request *aead_req = (void *)aead_req_data;
+	aead_req = kzalloc(reqsize + GCM_AAD_LEN, GFP_ATOMIC);
+	if (!aead_req)
+		return -ENOMEM;
 
-	memset(aead_req, 0, sizeof(aead_req_data));
+	__aad = (u8 *)aead_req + reqsize;
+	memcpy(__aad, aad, GCM_AAD_LEN);
 
 	sg_init_table(sg, 3);
-	sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
+	sg_set_buf(&sg[0], &__aad[2], be16_to_cpup((__be16 *)__aad));
 	sg_set_buf(&sg[1], data, data_len);
 	sg_set_buf(&sg[2], mic, IEEE80211_GCMP_MIC_LEN);
 
@@ -37,24 +40,31 @@ void ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
 	aead_request_set_ad(aead_req, sg[0].length);
 
 	crypto_aead_encrypt(aead_req);
+	kzfree(aead_req);
+	return 0;
 }
 
 int ieee80211_aes_gcm_decrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
 			      u8 *data, size_t data_len, u8 *mic)
 {
 	struct scatterlist sg[3];
-	char aead_req_data[sizeof(struct aead_request) +
-			   crypto_aead_reqsize(tfm)]
-		__aligned(__alignof__(struct aead_request));
-	struct aead_request *aead_req = (void *)aead_req_data;
+	struct aead_request *aead_req;
+	int reqsize = sizeof(*aead_req) + crypto_aead_reqsize(tfm);
+	u8 *__aad;
+	int err;
 
 	if (data_len == 0)
 		return -EINVAL;
 
-	memset(aead_req, 0, sizeof(aead_req_data));
+	aead_req = kzalloc(reqsize + GCM_AAD_LEN, GFP_ATOMIC);
+	if (!aead_req)
+		return -ENOMEM;
+
+	__aad = (u8 *)aead_req + reqsize;
+	memcpy(__aad, aad, GCM_AAD_LEN);
 
 	sg_init_table(sg, 3);
-	sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
+	sg_set_buf(&sg[0], &__aad[2], be16_to_cpup((__be16 *)__aad));
 	sg_set_buf(&sg[1], data, data_len);
 	sg_set_buf(&sg[2], mic, IEEE80211_GCMP_MIC_LEN);
 
@@ -63,7 +73,10 @@ int ieee80211_aes_gcm_decrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
 			       data_len + IEEE80211_GCMP_MIC_LEN, j_0);
 	aead_request_set_ad(aead_req, sg[0].length);
 
-	return crypto_aead_decrypt(aead_req);
+	err = crypto_aead_decrypt(aead_req);
+	kzfree(aead_req);
+
+	return err;
 }
 
 struct crypto_aead *ieee80211_aes_gcm_key_setup_encrypt(const u8 key[],
diff --git a/net/mac80211/aes_gcm.h b/net/mac80211/aes_gcm.h
index 1347fda6b76a..55aed5352494 100644
--- a/net/mac80211/aes_gcm.h
+++ b/net/mac80211/aes_gcm.h
@@ -11,8 +11,10 @@
 
 #include <linux/crypto.h>
 
-void ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
-			       u8 *data, size_t data_len, u8 *mic);
+#define GCM_AAD_LEN	32
+
+int ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
+			      u8 *data, size_t data_len, u8 *mic);
 int ieee80211_aes_gcm_decrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
 			      u8 *data, size_t data_len, u8 *mic);
 struct crypto_aead *ieee80211_aes_gcm_key_setup_encrypt(const u8 key[],
diff --git a/net/mac80211/aes_gmac.c b/net/mac80211/aes_gmac.c
index 3ddd927aaf30..bd72a862ddb7 100644
--- a/net/mac80211/aes_gmac.c
+++ b/net/mac80211/aes_gmac.c
@@ -17,28 +17,27 @@
 #include "key.h"
 #include "aes_gmac.h"
 
-#define GMAC_MIC_LEN 16
-#define GMAC_NONCE_LEN 12
-#define AAD_LEN 20
-
 int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
 		       const u8 *data, size_t data_len, u8 *mic)
 {
 	struct scatterlist sg[4];
-	char aead_req_data[sizeof(struct aead_request) +
-			   crypto_aead_reqsize(tfm)]
-		__aligned(__alignof__(struct aead_request));
-	struct aead_request *aead_req = (void *)aead_req_data;
-	u8 zero[GMAC_MIC_LEN], iv[AES_BLOCK_SIZE];
+	u8 *zero, *__aad, iv[AES_BLOCK_SIZE];
+	struct aead_request *aead_req;
+	int reqsize = sizeof(*aead_req) + crypto_aead_reqsize(tfm);
 
 	if (data_len < GMAC_MIC_LEN)
 		return -EINVAL;
 
-	memset(aead_req, 0, sizeof(aead_req_data));
+	aead_req = kzalloc(reqsize + GMAC_MIC_LEN + GMAC_AAD_LEN, GFP_ATOMIC);
+	if (!aead_req)
+		return -ENOMEM;
+
+	zero = (u8 *)aead_req + reqsize;
+	__aad = zero + GMAC_MIC_LEN;
+	memcpy(__aad, aad, GMAC_AAD_LEN);
 
-	memset(zero, 0, GMAC_MIC_LEN);
 	sg_init_table(sg, 4);
-	sg_set_buf(&sg[0], aad, AAD_LEN);
+	sg_set_buf(&sg[0], __aad, GMAC_AAD_LEN);
 	sg_set_buf(&sg[1], data, data_len - GMAC_MIC_LEN);
 	sg_set_buf(&sg[2], zero, GMAC_MIC_LEN);
 	sg_set_buf(&sg[3], mic, GMAC_MIC_LEN);
@@ -49,9 +48,10 @@ int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
 
 	aead_request_set_tfm(aead_req, tfm);
 	aead_request_set_crypt(aead_req, sg, sg, 0, iv);
-	aead_request_set_ad(aead_req, AAD_LEN + data_len);
+	aead_request_set_ad(aead_req, GMAC_AAD_LEN + data_len);
 
 	crypto_aead_encrypt(aead_req);
+	kzfree(aead_req);
 
 	return 0;
 }
diff --git a/net/mac80211/aes_gmac.h b/net/mac80211/aes_gmac.h
index d328204d73a8..32e6442c95be 100644
--- a/net/mac80211/aes_gmac.h
+++ b/net/mac80211/aes_gmac.h
@@ -11,6 +11,10 @@
 
 #include <linux/crypto.h>
 
+#define GMAC_AAD_LEN	20
+#define GMAC_MIC_LEN	16
+#define GMAC_NONCE_LEN	12
+
 struct crypto_aead *ieee80211_aes_gmac_key_setup(const u8 key[],
 						 size_t key_len);
 int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index b48c1e13e281..14b28998c571 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -405,7 +405,7 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
 	u8 *pos;
 	u8 pn[6];
 	u64 pn64;
-	u8 aad[2 * AES_BLOCK_SIZE];
+	u8 aad[CCM_AAD_LEN];
 	u8 b_0[AES_BLOCK_SIZE];
 
 	if (info->control.hw_key &&
@@ -461,10 +461,8 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
 
 	pos += IEEE80211_CCMP_HDR_LEN;
 	ccmp_special_blocks(skb, pn, b_0, aad);
-	ieee80211_aes_ccm_encrypt(key->u.ccmp.tfm, b_0, aad, pos, len,
-				  skb_put(skb, mic_len), mic_len);
-
-	return 0;
+	return ieee80211_aes_ccm_encrypt(key->u.ccmp.tfm, b_0, aad, pos, len,
+					 skb_put(skb, mic_len), mic_len);
 }
 
 
@@ -639,7 +637,7 @@ static int gcmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 	u8 *pos;
 	u8 pn[6];
 	u64 pn64;
-	u8 aad[2 * AES_BLOCK_SIZE];
+	u8 aad[GCM_AAD_LEN];
 	u8 j_0[AES_BLOCK_SIZE];
 
 	if (info->control.hw_key &&
@@ -696,10 +694,8 @@ static int gcmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 
 	pos += IEEE80211_GCMP_HDR_LEN;
 	gcmp_special_blocks(skb, pn, j_0, aad);
-	ieee80211_aes_gcm_encrypt(key->u.gcmp.tfm, j_0, aad, pos, len,
-				  skb_put(skb, IEEE80211_GCMP_MIC_LEN));
-
-	return 0;
+	return ieee80211_aes_gcm_encrypt(key->u.gcmp.tfm, j_0, aad, pos, len,
+					 skb_put(skb, IEEE80211_GCMP_MIC_LEN));
 }
 
 ieee80211_tx_result
@@ -1123,9 +1119,9 @@ ieee80211_crypto_aes_gmac_encrypt(struct ieee80211_tx_data *tx)
 	struct ieee80211_key *key = tx->key;
 	struct ieee80211_mmie_16 *mmie;
 	struct ieee80211_hdr *hdr;
-	u8 aad[20];
+	u8 aad[GMAC_AAD_LEN];
 	u64 pn64;
-	u8 nonce[12];
+	u8 nonce[GMAC_NONCE_LEN];
 
 	if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
 		return TX_DROP;
@@ -1171,7 +1167,7 @@ ieee80211_crypto_aes_gmac_decrypt(struct ieee80211_rx_data *rx)
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_key *key = rx->key;
 	struct ieee80211_mmie_16 *mmie;
-	u8 aad[20], mic[16], ipn[6], nonce[12];
+	u8 aad[GMAC_AAD_LEN], mic[GMAC_MIC_LEN], ipn[6], nonce[GMAC_NONCE_LEN];
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 
 	if (!ieee80211_is_mgmt(hdr->frame_control))
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v4] mac80211: move extra crypto data off the stack
From: Johannes Berg @ 2016-10-17 13:25 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: <linux-wireless@vger.kernel.org>, Sergey Senozhatsky,
	<netdev@vger.kernel.org>, Herbert Xu
In-Reply-To: <CAKv+Gu_jdiQn+o5PAZmPfMsQYt8PsOkDE6YNU11VH9WM_QPcvg@mail.gmail.com>


> Indeed. And it keeps the clutter inside the aes_xxx.c files, which
> could easily be updated in the future to use some auxdata feature if
> it ever materializes.
> 
> I think it would help this code, but also the ESP code you pointed
> out, to have some kind of 'ordered synchronous' CRYPTO_xxx flag,
> where the crypto API could manage the kmem cache and percpu pointers
> to allocations.

Yeah, could be useful to have that more generally.

>  This goes well beyond what we can do as a fix, though, so we need an
> intermediate solution in any case.
> 
> Shall I propose the patch?

I assume you mean a mac80211 patch - sure, I'll take that instead of
the two I have now.

johannes

^ permalink raw reply

* Re: [PATCH v4] mac80211: move extra crypto data off the stack
From: Ard Biesheuvel @ 2016-10-17 13:20 UTC (permalink / raw)
  To: Johannes Berg
  Cc: <linux-wireless@vger.kernel.org>, Sergey Senozhatsky,
	<netdev@vger.kernel.org>, Herbert Xu
In-Reply-To: <1476710169.315.1.camel@sipsolutions.net>

On 17 October 2016 at 14:16, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2016-10-17 at 14:06 +0100, Ard Biesheuvel wrote:
>>
>> Actually, while I think it will be worthwhile going forward to
>> implement such an 'auxiliary data' feature in a generic way, I still
>> think we should address the issue at hand without too much
>> complication.
>>
>> If we pedal back to the version of 'mac80211: move struct aead_req
>> off the stack' that uses kzalloc() instead of aead_request_alloc(),
>> we can simply add some space for aad[] and/or zero[], and get rid of
>> the kmem cache entirely.
>>
>> If you're past this point already, i won't bother but otherwise I can
>> rework 'mac80211: move struct aead_req off the stack' so that the
>> other patch is no longer required (and IIRC, this is actually
>> something you proposed yourself a couple of iterations ago?)
>
> Yes, I did consider that.
>
> It makes some sense, and I guess the extra memcpy() would be cheaper
> than the extra alloc?
>
> I'd happily use that instead of the combination of my two patches. The
> aead_request_alloc() is just a simple inline anyway, so no real problem
> not using it.
>

Indeed. And it keeps the clutter inside the aes_xxx.c files, which
could easily be updated in the future to use some auxdata feature if
it ever materializes.

I think it would help this code, but also the ESP code you pointed
out, to have some kind of 'ordered synchronous' CRYPTO_xxx flag, where
the crypto API could manage the kmem cache and percpu pointers to
allocations. This goes well beyond what we can do as a fix, though, so
we need an intermediate solution in any case.

Shall I propose the patch?

^ permalink raw reply

* Re: [PATCH v4] mac80211: move extra crypto data off the stack
From: Johannes Berg @ 2016-10-17 13:16 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: <linux-wireless@vger.kernel.org>, Sergey Senozhatsky,
	<netdev@vger.kernel.org>, Herbert Xu
In-Reply-To: <CAKv+Gu9OT6r3gbrLi_wzBcPXoA6K9z=-4ybrgQy4X=FvjOOuhA@mail.gmail.com>

On Mon, 2016-10-17 at 14:06 +0100, Ard Biesheuvel wrote:
> 
> Actually, while I think it will be worthwhile going forward to
> implement such an 'auxiliary data' feature in a generic way, I still
> think we should address the issue at hand without too much
> complication.
> 
> If we pedal back to the version of 'mac80211: move struct aead_req
> off the stack' that uses kzalloc() instead of aead_request_alloc(),
> we can simply add some space for aad[] and/or zero[], and get rid of
> the kmem cache entirely.
> 
> If you're past this point already, i won't bother but otherwise I can
> rework 'mac80211: move struct aead_req off the stack' so that the
> other patch is no longer required (and IIRC, this is actually
> something you proposed yourself a couple of iterations ago?)

Yes, I did consider that.

It makes some sense, and I guess the extra memcpy() would be cheaper
than the extra alloc?

I'd happily use that instead of the combination of my two patches. The
aead_request_alloc() is just a simple inline anyway, so no real problem
not using it.

johannes

^ permalink raw reply

* Re: [PATCH v4] mac80211: move extra crypto data off the stack
From: Ard Biesheuvel @ 2016-10-17 13:06 UTC (permalink / raw)
  To: Johannes Berg
  Cc: <linux-wireless@vger.kernel.org>, Sergey Senozhatsky,
	<netdev@vger.kernel.org>, Herbert Xu
In-Reply-To: <41D0AC2F-E86E-4A98-AD8B-1239D7E6123A@linaro.org>

On 17 October 2016 at 11:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>
>> On 17 Oct 2016, at 10:54, Johannes Berg <johannes@sipsolutions.net> wrote:
>>
>>
>>>> Well, if your other patch to make it OK to be on-stack would be
>>>> applied instead, this wouldn't make much sense either :)
>>>>
>>>
>>> Yes but that one only fixes ccm not gcm
>>
>> Yes, but we can do the same for GCM, no?
>>
>
> No, not really. ccm happens to use aes with the same key for the mac and the encryption. gcm uses an another algo entirely for the mac
>
>>>> In this particular patch, we could reduce the size of the struct,
>>>> but I
>>>> don't actually think it'll make a difference to go from 48 to 36
>>>> bytes,
>>>> given alignment etc., so I think I'll just leave it as is.
>>>>
>>>
>>> I understand you are in a hurry, but this is unlikely to be the only
>>> such issue. I will propose an 'auxdata' feature for the crypto api
>>> that can be used here, but also for any other occurrence where client
>>> data assoiciated with the request can no longer be allocated on the
>>> stack
>>
>> No objections. I'll merge this anyway today I think, reverting is easy
>> later.
>>
>
> ok fair enough

Actually, while I think it will be worthwhile going forward to
implement such an 'auxiliary data' feature in a generic way, I still
think we should address the issue at hand without too much
complication.

If we pedal back to the version of 'mac80211: move struct aead_req off
the stack' that uses kzalloc() instead of aead_request_alloc(), we can
simply add some space for aad[] and/or zero[], and get rid of the kmem
cache entirely.

If you're past this point already, i won't bother but otherwise I can
rework 'mac80211: move struct aead_req off the stack' so that the
other patch is no longer required (and IIRC, this is actually
something you proposed yourself a couple of iterations ago?)

^ permalink raw reply

* Re: [Projekt-wlan] [PATCHv5 2/2] mac80211: fix A-MSDU outer SA/DA
From: Johannes Berg @ 2016-10-17 10:39 UTC (permalink / raw)
  To: michael-dev; +Cc: projekt-wlan, Fietkau, Felix, linux-wireless
In-Reply-To: <d793195b149aebb974dd596632d2ba83@fami-braun.de>

On Mon, 2016-10-17 at 12:29 +0200, michael-dev wrote:
> 
> ether_addr_copy requires both arguments to be __aligned(2) according
> to 
> include/linux/etherdevice.h.
> bssid might be sdata->u.mgd.bssid, which is bssid in struct 
> ieee80211_if_managed, but neither sdata->u, sdata->u.mgd nor 
> sdata->u.mgd.bssid seem to be declared as __aligned(2).So
> ether_addr_copy should not be used.

They are in fact aligned today, given that they occur after a pointer.
However, I did add the __aligned(2) annotation to make that plenty
clear for the future.

johannes

^ permalink raw reply

* Re: [Projekt-wlan] [PATCHv5 2/2] mac80211: fix A-MSDU outer SA/DA
From: michael-dev @ 2016-10-17 10:29 UTC (permalink / raw)
  To: Johannes Berg; +Cc: projekt-wlan, Fietkau, Felix, linux-wireless
In-Reply-To: <1476697463.19992.18.camel@sipsolutions.net>

Am 17.10.2016 11:44, schrieb Johannes Berg:
>> +	if (bssid && ieee80211_has_fromds(hdr->frame_control))
>> +		memcpy(h_80211_src, bssid, ETH_ALEN);
>> +
>> +	if (bssid && ieee80211_has_tods(hdr->frame_control))
>> +		memcpy(h_80211_dst, bssid, ETH_ALEN);
> 
> but I also changed these to ether_addr_copy()

ether_addr_copy requires both arguments to be __aligned(2) according to 
include/linux/etherdevice.h.
bssid might be sdata->u.mgd.bssid, which is bssid in struct 
ieee80211_if_managed, but neither sdata->u, sdata->u.mgd nor 
sdata->u.mgd.bssid seem to be declared as __aligned(2).
So ether_addr_copy should not be used.

What am I missing?

Regards,
M. Braun

^ permalink raw reply

* Re: [PATCH v4] mac80211: move extra crypto data off the stack
From: Ard Biesheuvel @ 2016-10-17 10:02 UTC (permalink / raw)
  To: Johannes Berg
  Cc: <linux-wireless@vger.kernel.org>, Sergey Senozhatsky,
	<netdev@vger.kernel.org>, Herbert Xu
In-Reply-To: <1476698041.19992.19.camel@sipsolutions.net>



> On 17 Oct 2016, at 10:54, Johannes Berg <johannes@sipsolutions.net> wrote:=

>=20
>=20
>>> Well, if your other patch to make it OK to be on-stack would be
>>> applied instead, this wouldn't make much sense either :)
>>>=20
>>=20
>> Yes but that one only fixes ccm not gcm
>=20
> Yes, but we can do the same for GCM, no?
>=20

No, not really. ccm happens to use aes with the same key for the mac and the=
 encryption. gcm uses an another algo entirely for the mac

>>> In this particular patch, we could reduce the size of the struct,
>>> but I
>>> don't actually think it'll make a difference to go from 48 to 36
>>> bytes,
>>> given alignment etc., so I think I'll just leave it as is.
>>>=20
>>=20
>> I understand you are in a hurry, but this is unlikely to be the only
>> such issue. I will propose an 'auxdata' feature for the crypto api
>> that can be used here, but also for any other occurrence where client
>> data assoiciated with the request can no longer be allocated on the
>> stack
>=20
> No objections. I'll merge this anyway today I think, reverting is easy
> later.
>=20

ok fair enough=

^ 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