mac80211: station state transition error handling

In the future, when we start notifying drivers,
state transitions could potentially fail. To make
it easier to distinguish between programming bugs
and driver failures:
 * rename sta_info_move_state() to
   sta_info_pre_move_state() which can only be
   called before the station is inserted (and
   check this with a new station flag).
 * rename sta_info_move_state_checked() to just
   plain sta_info_move_state(), as it will be
   the regular function that can fail for more
   than just one reason (bad transition or an
   error from the driver)

This makes the programming model easier -- one of
the functions can only be called before insertion
and can't fail, the other can fail.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 98460783c2d37abad1caad6ab40cd04b6fb0ea6c..dc7420441574318367838c5da331cf4f653f3332 100644 (file)
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -776,12 +776,10 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 
                if (set & BIT(NL80211_STA_FLAG_AUTHENTICATED) &&
                    !test_sta_flag(sta, WLAN_STA_AUTH)) {
-                       ret = sta_info_move_state_checked(sta,
-                                       IEEE80211_STA_AUTH);
+                       ret = sta_info_move_state(sta, IEEE80211_STA_AUTH);
                        if (ret)
                                return ret;
-                       ret = sta_info_move_state_checked(sta,
-                                       IEEE80211_STA_ASSOC);
+                       ret = sta_info_move_state(sta, IEEE80211_STA_ASSOC);
                        if (ret)
                                return ret;
                }
@@ -789,11 +787,9 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 
        if (mask & BIT(NL80211_STA_FLAG_AUTHORIZED)) {
                if (set & BIT(NL80211_STA_FLAG_AUTHORIZED))
-                       ret = sta_info_move_state_checked(sta,
-                                       IEEE80211_STA_AUTHORIZED);
+                       ret = sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
                else if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
-                       ret = sta_info_move_state_checked(sta,
-                                       IEEE80211_STA_ASSOC);
+                       ret = sta_info_move_state(sta, IEEE80211_STA_ASSOC);
                if (ret)
                        return ret;
        }
@@ -805,12 +801,10 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 
                if (!(set & BIT(NL80211_STA_FLAG_AUTHENTICATED)) &&
                    test_sta_flag(sta, WLAN_STA_AUTH)) {
-                       ret = sta_info_move_state_checked(sta,
-                                       IEEE80211_STA_AUTH);
+                       ret = sta_info_move_state(sta, IEEE80211_STA_AUTH);
                        if (ret)
                                return ret;
-                       ret = sta_info_move_state_checked(sta,
-                                       IEEE80211_STA_NONE);
+                       ret = sta_info_move_state(sta, IEEE80211_STA_NONE);
                        if (ret)
                                return ret;
                }
@@ -944,8 +938,8 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev,
        if (!sta)
                return -ENOMEM;
 
-       sta_info_move_state(sta, IEEE80211_STA_AUTH);
-       sta_info_move_state(sta, IEEE80211_STA_ASSOC);
+       sta_info_pre_move_state(sta, IEEE80211_STA_AUTH);
+       sta_info_pre_move_state(sta, IEEE80211_STA_ASSOC);
 
        err = sta_apply_parameters(local, sta, params);
        if (err) {

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 2406b3e7393fb042d8ec82b417c7e985a0801fa2..c8383712fdeca6f679944184cea5b42c58c44acb 100644 (file)
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -63,14 +63,15 @@ static ssize_t sta_flags_read(struct file *file, char __user *userbuf,
        test_sta_flag(sta, WLAN_STA_##flg) ? #flg "\n" : ""
 
        int res = scnprintf(buf, sizeof(buf),
-                           "%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
+                           "%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
                            TEST(AUTH), TEST(ASSOC), TEST(PS_STA),
                            TEST(PS_DRIVER), TEST(AUTHORIZED),
                            TEST(SHORT_PREAMBLE),
                            TEST(WME), TEST(WDS), TEST(CLEAR_PS_FILT),
                            TEST(MFP), TEST(BLOCK_BA), TEST(PSPOLL),
                            TEST(UAPSD), TEST(SP), TEST(TDLS_PEER),
-                           TEST(TDLS_PEER_AUTH));
+                           TEST(TDLS_PEER_AUTH), TEST(4ADDR_EVENT),
+                           TEST(INSERTED));
 #undef TEST
        return simple_read_from_buffer(userbuf, count, ppos, buf, res);
 }

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index d38baa41cf6c6a073bf9ed3de8cd81fbb7602d22..a98d370b56f6063516959629b53a35080d3f7e58 100644 (file)
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -265,9 +265,9 @@ static struct sta_info *ieee80211_ibss_finish_sta(struct sta_info *sta,
                    addr, sdata->name);
 #endif
 
-       sta_info_move_state(sta, IEEE80211_STA_AUTH);
-       sta_info_move_state(sta, IEEE80211_STA_ASSOC);
-       sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
+       sta_info_pre_move_state(sta, IEEE80211_STA_AUTH);
+       sta_info_pre_move_state(sta, IEEE80211_STA_ASSOC);
+       sta_info_pre_move_state(sta, IEEE80211_STA_AUTHORIZED);
 
        rate_control_rate_init(sta);
 

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index e47768cb8cb3b18eb98e3e16abc542a3735ad9e3..6b0d70e960d4fc182ba26633a6158bac2f2ea924 100644 (file)
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -318,9 +318,9 @@ static int ieee80211_do_open(struct net_device *dev, bool coming_up)
                        goto err_del_interface;
                }
 
-               sta_info_move_state(sta, IEEE80211_STA_AUTH);
-               sta_info_move_state(sta, IEEE80211_STA_ASSOC);
-               sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
+               sta_info_pre_move_state(sta, IEEE80211_STA_AUTH);
+               sta_info_pre_move_state(sta, IEEE80211_STA_ASSOC);
+               sta_info_pre_move_state(sta, IEEE80211_STA_AUTHORIZED);
 
                res = sta_info_insert(sta);
                if (res) {

diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index 41ef1b4764422a85d56334f16c320b8d1d4db292..bf92e2878fe6ae85438902c965e91239609473e3 100644 (file)
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -96,9 +96,9 @@ static struct sta_info *mesh_plink_alloc(struct ieee80211_sub_if_data *sdata,
        if (!sta)
                return NULL;
 
-       sta_info_move_state(sta, IEEE80211_STA_AUTH);
-       sta_info_move_state(sta, IEEE80211_STA_ASSOC);
-       sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
+       sta_info_pre_move_state(sta, IEEE80211_STA_AUTH);
+       sta_info_pre_move_state(sta, IEEE80211_STA_ASSOC);
+       sta_info_pre_move_state(sta, IEEE80211_STA_AUTHORIZED);
 
        set_sta_flag(sta, WLAN_STA_WME);
 

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index de3268c7be1e34e7c6c1e3ffe60e66cffd1620e9..acc11b5c16fa199466d27577bc76e3c9d3d95a8d 100644 (file)
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1587,10 +1587,19 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
                return false;
        }
 
-       sta_info_move_state(sta, IEEE80211_STA_AUTH);
-       sta_info_move_state(sta, IEEE80211_STA_ASSOC);
-       if (!(ifmgd->flags & IEEE80211_STA_CONTROL_PORT))
-               sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
+       err = sta_info_move_state(sta, IEEE80211_STA_AUTH);
+       if (!err)
+               err = sta_info_move_state(sta, IEEE80211_STA_ASSOC);
+       if (!err && !(ifmgd->flags & IEEE80211_STA_CONTROL_PORT))
+               err = sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
+       if (err) {
+               printk(KERN_DEBUG
+                      "%s: failed to move station %pM to desired state\n",
+                      sdata->name, sta->sta.addr);
+               WARN_ON(__sta_info_destroy(sta));
+               mutex_unlock(&sdata->local->sta_mtx);
+               return false;
+       }
 
        rates = 0;
        basic_rates = 0;

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index f28fa02b6e54cc8168dc3542cc745596ca570904..df8f0a2f0deed16eba643e13421fac3c44c1c84d 100644 (file)
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -403,6 +403,8 @@ static int sta_info_insert_finish(struct sta_info *sta) __acquires(RCU)
                sta_info_hash_add(local, sta);
 
                list_add(&sta->list, &local->sta_list);
+
+               set_sta_flag(sta, WLAN_STA_INSERTED);
        } else {
                sta->dummy = false;
        }
@@ -707,7 +709,7 @@ static bool sta_info_cleanup_expire_buffered(struct ieee80211_local *local,
        return have_buffered;
 }
 
-static int __must_check __sta_info_destroy(struct sta_info *sta)
+int __must_check __sta_info_destroy(struct sta_info *sta)
 {
        struct ieee80211_local *local;
        struct ieee80211_sub_if_data *sdata;
@@ -722,6 +724,8 @@ static int __must_check __sta_info_destroy(struct sta_info *sta)
        local = sta->local;
        sdata = sta->sdata;
 
+       lockdep_assert_held(&local->sta_mtx);
+
        /*
         * Before removing the station from the driver and
         * rate control, it might still start new aggregation
@@ -763,8 +767,13 @@ static int __must_check __sta_info_destroy(struct sta_info *sta)
        if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
                RCU_INIT_POINTER(sdata->u.vlan.sta, NULL);
 
-       while (sta->sta_state > IEEE80211_STA_NONE)
-               sta_info_move_state(sta, sta->sta_state - 1);
+       while (sta->sta_state > IEEE80211_STA_NONE) {
+               int err = sta_info_move_state(sta, sta->sta_state - 1);
+               if (err) {
+                       WARN_ON_ONCE(1);
+                       break;
+               }
+       }
 
        if (sta->uploaded) {
                if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
@@ -1391,8 +1400,8 @@ void ieee80211_sta_set_buffered(struct ieee80211_sta *pubsta,
 }
 EXPORT_SYMBOL(ieee80211_sta_set_buffered);
 
-int sta_info_move_state_checked(struct sta_info *sta,
-                               enum ieee80211_sta_state new_state)
+int sta_info_move_state(struct sta_info *sta,
+                       enum ieee80211_sta_state new_state)
 {
        might_sleep();
 

diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 6f77f12dc3fc313410964a91f318e48f1319d86c..381de37d2478bbc55518f4b2aec008e59279c92b 100644 (file)
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -52,6 +52,7 @@
  * @WLAN_STA_SP: Station is in a service period, so don't try to
  *     reply to other uAPSD trigger frames or PS-Poll.
  * @WLAN_STA_4ADDR_EVENT: 4-addr event was already sent for this frame.
+ * @WLAN_STA_INSERTED: This station is inserted into the hash table.
  */
 enum ieee80211_sta_info_flags {
        WLAN_STA_AUTH,
@@ -71,6 +72,7 @@ enum ieee80211_sta_info_flags {
        WLAN_STA_UAPSD,
        WLAN_STA_SP,
        WLAN_STA_4ADDR_EVENT,
+       WLAN_STA_INSERTED,
 };
 
 enum ieee80211_sta_state {
@@ -427,13 +429,17 @@ static inline int test_and_set_sta_flag(struct sta_info *sta,
        return test_and_set_bit(flag, &sta->_flags);
 }
 
-int sta_info_move_state_checked(struct sta_info *sta,
-                               enum ieee80211_sta_state new_state);
+int sta_info_move_state(struct sta_info *sta,
+                       enum ieee80211_sta_state new_state);
 
-static inline void sta_info_move_state(struct sta_info *sta,
-                                      enum ieee80211_sta_state new_state)
+static inline void sta_info_pre_move_state(struct sta_info *sta,
+                                          enum ieee80211_sta_state new_state)
 {
-       int ret = sta_info_move_state_checked(sta, new_state);
+       int ret;
+
+       WARN_ON_ONCE(test_sta_flag(sta, WLAN_STA_INSERTED));
+
+       ret = sta_info_move_state(sta, new_state);
        WARN_ON_ONCE(ret);
 }
 
@@ -544,6 +550,7 @@ int sta_info_insert(struct sta_info *sta);
 int sta_info_insert_rcu(struct sta_info *sta) __acquires(RCU);
 int sta_info_reinsert(struct sta_info *sta);
 
+int __must_check __sta_info_destroy(struct sta_info *sta);
 int sta_info_destroy_addr(struct ieee80211_sub_if_data *sdata,
                          const u8 *addr);
 int sta_info_destroy_addr_bss(struct ieee80211_sub_if_data *sdata,