From patchwork Wed Jan 15 12:04:50 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Kazior X-Patchwork-Id: 3491481 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 83517C02DC for ; Wed, 15 Jan 2014 12:09:41 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4DA2C20108 for ; Wed, 15 Jan 2014 12:09:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B66DD200F2 for ; Wed, 15 Jan 2014 12:09:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751688AbaAOMJe (ORCPT ); Wed, 15 Jan 2014 07:09:34 -0500 Received: from mail-ea0-f179.google.com ([209.85.215.179]:63936 "EHLO mail-ea0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751473AbaAOMJa (ORCPT ); Wed, 15 Jan 2014 07:09:30 -0500 Received: by mail-ea0-f179.google.com with SMTP id r15so417849ead.24 for ; Wed, 15 Jan 2014 04:09:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tieto.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=BDvB696OjcelpydEm1ZngArYpjuTZKxGt/LbbHlTzww=; b=IodRq89XMAAksW1H3aguERM8VL8Z+TDKP25ICRc7vxvKht8Ct27Qi4JPI6zblCQell ZVfHCUxAeSTkARg4eEpT9qvOZIWDaxCf44IT3/pRg/pd8iuW+Uh+u8HWB5gjPOPwsTx6 f+CyjaRSzZfKTQOAc5Pw/vGc73AyNMQaY4lIU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=BDvB696OjcelpydEm1ZngArYpjuTZKxGt/LbbHlTzww=; b=XIxSxViMDJzGsEm5Dsi9/06LuEUodpTk+Y/fjiuUNz36N9ljfof9dJYcYU120H2FKQ 2/I0R30dYXVPSpddVm/Yg+GpCoxWd+8vmmp11KIdjnVIRgUa+7EZdfIe50h3ABPKvwsA 6CRhDer7M6IWi6/MkPUWt9hAmG35fIlxu/tIQ+N94AjY3hX3ORH8s0ofAHfmoMYCjk43 bScbZ9ZkiUsXgygJWAZBoL6n3DjRvhXE31Q1x0tql/4CAxCqwSKQGFeZtpkz/XSVGuQt 7O9Txj/Zd4mTyCusObg9nriwBw6sbJKF+goRH0rkFXYe6442zXpjJZLZkowQeFpcR1HH 3nxw== X-Gm-Message-State: ALoCoQnq8dqHuB2e1d6U6Q6XkjHnL1xS6qUcZ2V5VcequIjeT3mjy/BNnh40RrBUnxigaMWe+bdBF+wICYACHh5jOxtv5GzxItRseEqRbWtl3MAoNym6xN8= X-Received: by 10.14.175.131 with SMTP id z3mr3101445eel.65.1389787768777; Wed, 15 Jan 2014 04:09:28 -0800 (PST) Received: from localhost.localdomain ([91.198.246.8]) by mx.google.com with ESMTPSA id w4sm9780548eef.20.2014.01.15.04.09.27 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Jan 2014 04:09:28 -0800 (PST) From: Michal Kazior To: linux-wireless@vger.kernel.org Cc: johannes@sipsolutions.net, Michal Kazior Subject: [RFC 5/9] mac80211: improve CSA locking Date: Wed, 15 Jan 2014 13:04:50 +0100 Message-Id: <1389787494-7361-6-git-send-email-michal.kazior@tieto.com> X-Mailer: git-send-email 1.8.4.rc3 In-Reply-To: <1389787494-7361-1-git-send-email-michal.kazior@tieto.com> References: <1389787494-7361-1-git-send-email-michal.kazior@tieto.com> X-DomainID: tieto.com Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The patch improves channel switch related locking (STA, IBSS, AP, mesh). Now read access to sdata->vif.csa_active is protected by wdev.mtx and local->mtx so holding either is enough for read access but both are required for write access. The only exception is ieee80211_beacon_get_tim() but it's safe to leave it as is and it doesn't influence mac80211 state in any way. The patch adds a few lockdep assertions along for easier code/locking maintenance. This also prepares for multi-interface CSA. Signed-off-by: Michal Kazior --- net/mac80211/cfg.c | 18 +++++++++++++++--- net/mac80211/ibss.c | 18 ++++++++++++++---- net/mac80211/iface.c | 27 +++++++++++++++++++++++++-- net/mac80211/mesh.c | 13 ++++++++++++- net/mac80211/mlme.c | 20 ++++++++++++++------ 5 files changed, 80 insertions(+), 16 deletions(-) diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index ef679de..9a2421d 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -1052,6 +1052,7 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev, int err; sdata = IEEE80211_DEV_TO_SUB_IF(dev); + sdata_assert_lock(sdata); /* don't allow changing the beacon while CSA is in place - offset * of channel switch counter may change @@ -1079,15 +1080,19 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev) struct probe_resp *old_probe_resp; struct cfg80211_chan_def chandef; + sdata_assert_lock(sdata); + old_beacon = sdata_dereference(sdata->u.ap.beacon, sdata); if (!old_beacon) return -ENOENT; old_probe_resp = sdata_dereference(sdata->u.ap.probe_resp, sdata); /* abort any running channel switch */ + mutex_lock(&local->mtx); sdata->vif.csa_active = false; kfree(sdata->u.ap.next_beacon); sdata->u.ap.next_beacon = NULL; + mutex_unlock(&local->mtx); cancel_work_sync(&sdata->u.ap.request_smps_work); @@ -2991,6 +2996,8 @@ static int ieee80211_ap_finish_csa(struct ieee80211_sub_if_data *sdata) { int err; + lockdep_assert_held(&sdata->local->mtx); + err = ieee80211_assign_beacon(sdata, sdata->u.ap.next_beacon); kfree(sdata->u.ap.next_beacon); sdata->u.ap.next_beacon = NULL; @@ -3011,6 +3018,7 @@ void ieee80211_csa_finalize_work(struct work_struct *work) int err, changed = 0; sdata_lock(sdata); + mutex_lock(&local->mtx); /* AP might have been stopped while waiting for the lock. */ if (!sdata->vif.csa_active) goto unlock; @@ -3018,10 +3026,9 @@ void ieee80211_csa_finalize_work(struct work_struct *work) if (!ieee80211_sdata_running(sdata)) goto unlock; - mutex_lock(&local->mtx); sdata->radar_required = sdata->csa_radar_required; + err = ieee80211_vif_change_channel(sdata, &changed); - mutex_unlock(&local->mtx); if (WARN_ON(err < 0)) goto unlock; @@ -3063,6 +3070,7 @@ void ieee80211_csa_finalize_work(struct work_struct *work) cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef); unlock: + mutex_unlock(&local->mtx); sdata_unlock(sdata); } @@ -3076,7 +3084,8 @@ int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, struct ieee80211_if_mesh __maybe_unused *ifmsh; int err, num_chanctx; - lockdep_assert_held(&sdata->wdev.mtx); + sdata_assert_lock(sdata); + lockdep_assert_held(&local->mtx); if (!list_empty(&local->roc_list) || local->scanning) return -EBUSY; @@ -3219,8 +3228,11 @@ int ieee80211_channel_switch(struct wiphy *wiphy, return -EOPNOTSUPP; sdata = IEEE80211_DEV_TO_SUB_IF(params[0].dev); + sdata_lock(sdata); + mutex_lock(&sdata->local->mtx); err = __ieee80211_channel_switch(wiphy, params[0].dev, ¶ms[0]); + mutex_unlock(&sdata->local->mtx); sdata_unlock(sdata); return err; diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c index d1dc641..706b666 100644 --- a/net/mac80211/ibss.c +++ b/net/mac80211/ibss.c @@ -802,6 +802,12 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata, int err; u32 sta_flags; + sdata_assert_lock(sdata); + lockdep_assert_held(&sdata->local->mtx); + + if (sdata->vif.csa_active) + return true; + sta_flags = IEEE80211_STA_DISABLE_VHT; switch (ifibss->chandef.width) { case NL80211_CHAN_WIDTH_5: @@ -943,8 +949,9 @@ ieee80211_rx_mgmt_spectrum_mgmt(struct ieee80211_sub_if_data *sdata, if (len < required_len) return; - if (!sdata->vif.csa_active) - ieee80211_ibss_process_chanswitch(sdata, elems, false); + mutex_lock(&sdata->local->mtx); + ieee80211_ibss_process_chanswitch(sdata, elems, false); + mutex_unlock(&sdata->local->mtx); } static void ieee80211_rx_mgmt_deauth_ibss(struct ieee80211_sub_if_data *sdata, @@ -1125,9 +1132,12 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata, goto put_bss; /* process channel switch */ - if (sdata->vif.csa_active || - ieee80211_ibss_process_chanswitch(sdata, elems, true)) + mutex_lock(&local->mtx); + if (ieee80211_ibss_process_chanswitch(sdata, elems, true)) { + mutex_unlock(&local->mtx); goto put_bss; + } + mutex_unlock(&local->mtx); /* same BSSID */ if (ether_addr_equal(cbss->bssid, sdata->u.ibss.bssid)) diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index 0aa9675..a339334 100644 --- a/net/mac80211/iface.c +++ b/net/mac80211/iface.c @@ -250,14 +250,18 @@ static inline int identical_mac_addr_allowed(int type1, int type2) type2 == NL80211_IFTYPE_AP_VLAN)); } -static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata, - enum nl80211_iftype iftype) +static int +__ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata, + enum nl80211_iftype iftype) { struct ieee80211_local *local = sdata->local; struct ieee80211_sub_if_data *nsdata; ASSERT_RTNL(); + /* access to vif.csa_active should be protected with `local->mtx` */ + lockdep_assert_held(&local->mtx); + /* we hold the RTNL here so can safely walk the list */ list_for_each_entry(nsdata, &local->interfaces, list) { if (nsdata != sdata && ieee80211_sdata_running(nsdata)) { @@ -308,6 +312,21 @@ static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata, return 0; } +static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata, + enum nl80211_iftype iftype) +{ + struct ieee80211_local *local = sdata->local; + int err; + + ASSERT_RTNL(); + + mutex_lock(&local->mtx); + err = __ieee80211_check_concurrent_iface(sdata, iftype); + mutex_unlock(&local->mtx); + + return err; +} + static int ieee80211_check_queues(struct ieee80211_sub_if_data *sdata, enum nl80211_iftype iftype) { @@ -822,7 +841,11 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, cancel_work_sync(&local->dynamic_ps_enable_work); cancel_work_sync(&sdata->recalc_smps); + sdata_lock(sdata); + mutex_lock(&local->mtx); sdata->vif.csa_active = false; + mutex_unlock(&local->mtx); + sdata_unlock(sdata); cancel_work_sync(&sdata->csa_finalize_work); cancel_delayed_work_sync(&sdata->dfs_cac_timer_work); diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c index 5a74b24..fa758dc 100644 --- a/net/mac80211/mesh.c +++ b/net/mac80211/mesh.c @@ -865,6 +865,9 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata, int err, num_chanctx; u32 sta_flags; + sdata_assert_lock(sdata); + lockdep_assert_held(&sdata->local->mtx); + if (sdata->vif.csa_active) return true; @@ -967,6 +970,7 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata, IEEE80211_QUEUE_STOP_REASON_CSA); sdata->csa_chandef = params.chandef; + sdata->vif.csa_active = true; ieee80211_bss_info_change_notify(sdata, err); @@ -1085,8 +1089,10 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata, ifmsh->sync_ops->rx_bcn_presp(sdata, stype, mgmt, &elems, rx_status); + mutex_lock(&sdata->local->mtx); if (!ifmsh->chsw_init) ieee80211_mesh_process_chnswitch(sdata, &elems, true); + mutex_unlock(&sdata->local->mtx); } int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata) @@ -1189,6 +1195,7 @@ static void mesh_rx_csa_frame(struct ieee80211_sub_if_data *sdata, bool fwd_csa = true; size_t baselen; u8 *pos; + int err; if (mgmt->u.action.u.measurement.action_code != WLAN_ACTION_SPCT_CHL_SWITCH) @@ -1209,7 +1216,11 @@ static void mesh_rx_csa_frame(struct ieee80211_sub_if_data *sdata, ifmsh->pre_value = pre_value; - if (!ieee80211_mesh_process_chnswitch(sdata, &elems, false)) { + mutex_lock(&sdata->local->mtx); + err = ieee80211_mesh_process_chnswitch(sdata, &elems, false); + mutex_unlock(&sdata->local->mtx); + + if (!err) { mcsa_dbg(sdata, "Failed to process CSA action frame"); return; } diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index bfb81cb..d898dc9 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -885,12 +885,11 @@ static void ieee80211_chswitch_work(struct work_struct *work) return; sdata_lock(sdata); + mutex_lock(&local->mtx); if (!ifmgd->associated) goto out; - mutex_lock(&local->mtx); ret = ieee80211_vif_change_channel(sdata, &changed); - mutex_unlock(&local->mtx); if (ret) { sdata_info(sdata, "vif channel switch failed, disconnecting\n"); @@ -923,6 +922,7 @@ static void ieee80211_chswitch_work(struct work_struct *work) out: sdata->vif.csa_active = false; ifmgd->flags &= ~IEEE80211_STA_CSA_RECEIVED; + mutex_unlock(&local->mtx); sdata_unlock(sdata); } @@ -965,6 +965,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata, int res; sdata_assert_lock(sdata); + lockdep_assert_held(&sdata->local->mtx); if (!cbss) return; @@ -1987,10 +1988,9 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata) u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN]; sdata_lock(sdata); - if (!ifmgd->associated) { - sdata_unlock(sdata); - return; - } + mutex_lock(&sdata->local->mtx); + if (!ifmgd->associated) + goto out; ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DEAUTH, WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY, @@ -2003,6 +2003,8 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata) cfg80211_tx_mlme_mgmt(sdata->dev, frame_buf, IEEE80211_DEAUTH_FRAME_LEN); +out: + mutex_unlock(&sdata->local->mtx); sdata_unlock(sdata); } @@ -2969,8 +2971,10 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata, ieee80211_rx_bss_info(sdata, mgmt, len, rx_status, &elems); + mutex_lock(&local->mtx); ieee80211_sta_process_chanswitch(sdata, rx_status->mactime, &elems, true); + mutex_unlock(&local->mtx); if (!(ifmgd->flags & IEEE80211_STA_DISABLE_WMM) && ieee80211_sta_wmm_params(local, sdata, elems.wmm_param, @@ -3101,9 +3105,11 @@ void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata, if (elems.parse_error) break; + mutex_lock(&sdata->local->mtx); ieee80211_sta_process_chanswitch(sdata, rx_status->mactime, &elems, false); + mutex_unlock(&sdata->local->mtx); } else if (mgmt->u.action.category == WLAN_CATEGORY_PUBLIC) { ies_len = skb->len - offsetof(struct ieee80211_mgmt, @@ -3123,9 +3129,11 @@ void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata, elems.ext_chansw_ie = &mgmt->u.action.u.ext_chan_switch.data; + mutex_lock(&sdata->local->mtx); ieee80211_sta_process_chanswitch(sdata, rx_status->mactime, &elems, false); + mutex_unlock(&sdata->local->mtx); } break; }