mac80211: proper STA info locking

As discussed earlier, we can unify locking in struct sta_info
and use just a single spinlock protecting all members of the
structure that need protection. Many don't, but one of the
especially bad ones is the 'flags' member that can currently
be clobbered when RX and TX is being processed on different
CPUs at the same time.

Because having four spinlocks for different, mostly exclusive
parts of a single structure is overkill, this patch also kills
the ampdu and mesh plink spinlocks and uses just a single one
for everything. Because none of the spinlocks are nested, this
is safe.

It remains to be seen whether or not we should make the sta
flags use atomic bit operations instead, for now though this
is a safe thing and using atomic operations instead will be
very simple using the new static inline functions this patch
introduces for accessing sta->flags.

Since spin_lock_bh() is used with this lock, there shouldn't
be any contention even if aggregation is enabled at around the
same time as both requires frame transmission/reception which
is in a bh context.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Tomas Winkler <tomasw@gmail.com>
Cc: Ron Rindjunsky <ron.rindjunsky@intel.com>
Cc: Luis Carlos Cobo <luisca@cozybit.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

1 files changed, 63 insertions, 9 deletions

diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index f8c95bc9659..97a61c39ad9 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -129,23 +129,19 @@ enum plink_state {
  *
  * @tid_state_rx: TID's state in Rx session state machine.
  * @tid_rx: aggregation info for Rx per TID
- * @ampdu_rx: for locking sections in aggregation Rx flow
  * @tid_state_tx: TID's state in Tx session state machine.
  * @tid_tx: aggregation info for Tx per TID
  * @addba_req_num: number of times addBA request has been sent.
- * @ampdu_tx: for locking sectionsi in aggregation Tx flow
  * @dialog_token_allocator: dialog token enumerator for each new session;
  */
 struct sta_ampdu_mlme {
 	/* rx */
 	u8 tid_state_rx[STA_TID_NUM];
 	struct tid_ampdu_rx *tid_rx[STA_TID_NUM];
-	spinlock_t ampdu_rx;
 	/* tx */
 	u8 tid_state_tx[STA_TID_NUM];
 	struct tid_ampdu_tx *tid_tx[STA_TID_NUM];
 	u8 addba_req_num[STA_TID_NUM];
-	spinlock_t ampdu_tx;
 	u8 dialog_token_allocator;
 };
 
@@ -177,6 +173,8 @@ struct sta_ampdu_mlme {
  * @rx_bytes: Number of bytes received from this STA
  * @supp_rates: Bitmap of supported rates (per band)
  * @ht_info: HT capabilities of this STA
+ * @lock: used for locking all fields that require locking, see comments
+ *	in the header file.
  */
 struct sta_info {
 	/* General information, mostly static */
@@ -187,6 +185,7 @@ struct sta_info {
 	struct ieee80211_key *key;
 	struct rate_control_ref *rate_ctrl;
 	void *rate_ctrl_priv;
+	spinlock_t lock;
 	struct ieee80211_ht_info ht_info;
 	u64 supp_rates[IEEE80211_NUM_BANDS];
 	u8 addr[ETH_ALEN];
@@ -199,7 +198,7 @@ struct sta_info {
 	 */
 	u8 pin_status;
 
-	/* frequently updated information, needs locking? */
+	/* frequently updated information, locked with lock spinlock */
 	u32 flags;
 
 	/*
@@ -251,7 +250,7 @@ struct sta_info {
 	int channel_use_raw;
 
 	/*
-	 * Aggregation information, comes with own locking.
+	 * Aggregation information, locked with lock.
 	 */
 	struct sta_ampdu_mlme ampdu_mlme;
 	u8 timer_to_tid[STA_TID_NUM];	/* identity mapping to ID timers */
@@ -270,9 +269,6 @@ struct sta_info {
 	enum plink_state plink_state;
 	u32 plink_timeout;
 	struct timer_list plink_timer;
-	spinlock_t plink_lock;	/* For peer_state reads / updates and other
-				   updates in the structure. Ensures robust
-				   transitions for the peerlink FSM */
 #endif
 
 #ifdef CONFIG_MAC80211_DEBUGFS
@@ -299,6 +295,64 @@ static inline enum plink_state sta_plink_state(struct sta_info *sta)
 	return PLINK_LISTEN;
 }
 
+static inline void set_sta_flags(struct sta_info *sta, const u32 flags)
+{
+	spin_lock_bh(&sta->lock);
+	sta->flags |= flags;
+	spin_unlock_bh(&sta->lock);
+}
+
+static inline void clear_sta_flags(struct sta_info *sta, const u32 flags)
+{
+	spin_lock_bh(&sta->lock);
+	sta->flags &= ~flags;
+	spin_unlock_bh(&sta->lock);
+}
+
+static inline void set_and_clear_sta_flags(struct sta_info *sta,
+					   const u32 set, const u32 clear)
+{
+	spin_lock_bh(&sta->lock);
+	sta->flags |= set;
+	sta->flags &= ~clear;
+	spin_unlock_bh(&sta->lock);
+}
+
+static inline u32 test_sta_flags(struct sta_info *sta, const u32 flags)
+{
+	u32 ret;
+
+	spin_lock_bh(&sta->lock);
+	ret = sta->flags & flags;
+	spin_unlock_bh(&sta->lock);
+
+	return ret;
+}
+
+static inline u32 test_and_clear_sta_flags(struct sta_info *sta,
+					   const u32 flags)
+{
+	u32 ret;
+
+	spin_lock_bh(&sta->lock);
+	ret = sta->flags & flags;
+	sta->flags &= ~flags;
+	spin_unlock_bh(&sta->lock);
+
+	return ret;
+}
+
+static inline u32 get_sta_flags(struct sta_info *sta)
+{
+	u32 ret;
+
+	spin_lock_bh(&sta->lock);
+	ret = sta->flags;
+	spin_unlock_bh(&sta->lock);
+
+	return ret;
+}
+
 
 /* Maximum number of concurrently registered stations */
 #define MAX_STA_COUNT 2007