Hi,
These are some patches from my multi-interface
patchset. It's probably a good idea to apply them
regardless of current multi-interface
implementation acceptance.
I've updated the patches according to feedback on
the mailing list.
The odd one here seems to be sdata->radar_required
fix although it is related to CSA.
The last one is a split from the original CSA
locking patch. I've skipped the part that adds
local->mtx locking to CSA. It only makes sense
with my current multi-interface CSA. Since the
plan is to make CSA more chanctx oriented it
actually might be better to use chanctx instead of
mtx for CSA locking purposes.
Michal Kazior (5):
mac80211: batch CSA bss info notification
mac80211: fix possible memory leak on AP CSA failure
mac80211: move csa_active setting in STA CSA
mac80211: fix sdata->radar_required locking
mac80211: add missing CSA locking
net/mac80211/cfg.c | 30 ++++++++++++++++++++----------
net/mac80211/chan.c | 2 ++
net/mac80211/ibss.c | 11 ++++++-----
net/mac80211/iface.c | 2 ++
net/mac80211/mesh.c | 7 +++++--
net/mac80211/mlme.c | 2 +-
6 files changed, 36 insertions(+), 18 deletions(-)
--
1.8.5.3
On Wed, 2014-01-29 at 07:56 +0100, Michal Kazior wrote:
> Hi,
>
> These are some patches from my multi-interface
> patchset. It's probably a good idea to apply them
> regardless of current multi-interface
> implementation acceptance.
All applied, with a fix in your first patch:
- ieee80211_bss_info_change_notify(sdata, err);
+ ieee80211_bss_info_change_notify(sdata, changed);
johannes
On Wed, 2014-01-29 at 07:56 +0100, Michal Kazior wrote:
> Instead of having
> ieee80211_bss_info_change_notify() scattered all
> over the place just call it once when finalizing
> CSA.
>
> As a side effect this patch adds missing error
> checking for IBSS CSA beacon update.
>
> Signed-off-by: Michal Kazior <[email protected]>
> ---
Looks good to me:
Reviewed-by: Luciano Coelho <[email protected]>
If CSA for AP interface failed and the interface
was not stopped afterwards another CSA request
would leak sdata->u.ap.next_beacon.
Signed-off-by: Michal Kazior <[email protected]>
---
net/mac80211/cfg.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b503f92..20d4ee6 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3018,12 +3018,13 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
switch (sdata->vif.type) {
case NL80211_IFTYPE_AP:
err = ieee80211_assign_beacon(sdata, sdata->u.ap.next_beacon);
+ kfree(sdata->u.ap.next_beacon);
+ sdata->u.ap.next_beacon = NULL;
+
if (err < 0)
return;
changed |= err;
- kfree(sdata->u.ap.next_beacon);
- sdata->u.ap.next_beacon = NULL;
break;
case NL80211_IFTYPE_ADHOC:
err = ieee80211_ibss_finish_csa(sdata);
--
1.8.5.3
radar_required setting wasn't protected by
local->mtx in some places. This should prevent
from scanning/radar detection/roc colliding.
Signed-off-by: Michal Kazior <[email protected]>
---
net/mac80211/cfg.c | 4 ++--
net/mac80211/chan.c | 2 ++
net/mac80211/ibss.c | 2 +-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 20d4ee6..18e8687 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -970,9 +970,9 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
/* TODO: make hostapd tell us what it wants */
sdata->smps_mode = IEEE80211_SMPS_OFF;
sdata->needed_rx_chains = sdata->local->rx_chains;
- sdata->radar_required = params->radar_required;
mutex_lock(&local->mtx);
+ sdata->radar_required = params->radar_required;
err = ieee80211_vif_use_channel(sdata, ¶ms->chandef,
IEEE80211_CHANCTX_SHARED);
mutex_unlock(&local->mtx);
@@ -3002,8 +3002,8 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
struct ieee80211_local *local = sdata->local;
int err, changed = 0;
- sdata->radar_required = sdata->csa_radar_required;
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))
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index f43613a..42c6592 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -196,6 +196,8 @@ static bool ieee80211_is_radar_required(struct ieee80211_local *local)
{
struct ieee80211_sub_if_data *sdata;
+ lockdep_assert_held(&local->mtx);
+
rcu_read_lock();
list_for_each_entry_rcu(sdata, &local->interfaces, list) {
if (sdata->radar_required) {
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 82d3d14..f01d468 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -303,6 +303,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
mutex_unlock(&local->mtx);
return;
}
+ sdata->radar_required = radar_required;
mutex_unlock(&local->mtx);
memcpy(ifibss->bssid, bssid, ETH_ALEN);
@@ -318,7 +319,6 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
rcu_assign_pointer(ifibss->presp, presp);
mgmt = (void *)presp->head;
- sdata->radar_required = radar_required;
sdata->vif.bss_conf.enable_beacon = true;
sdata->vif.bss_conf.beacon_int = beacon_int;
sdata->vif.bss_conf.basic_rates = basic_rates;
--
1.8.5.3
On Wed, 2014-01-29 at 07:56 +0100, Michal Kazior wrote:
> The sdata->vif.csa_active could be left set after,
> e.g. channel context constraints check fail in STA
> mode leaving the interface in a strange state for
> a brief period of time until it is disconnected.
> This was harmless but ugly.
>
> Signed-off-by: Michal Kazior <[email protected]>
> ---
Reviewed-by: Luciano Coelho <[email protected]>
On Wed, 2014-01-29 at 07:56 +0100, Michal Kazior wrote:
> If CSA for AP interface failed and the interface
> was not stopped afterwards another CSA request
> would leak sdata->u.ap.next_beacon.
>
> Signed-off-by: Michal Kazior <[email protected]>
> ---
Reviewed-by: Luciano Coelho <[email protected]>
The sdata->vif.csa_active could be left set after,
e.g. channel context constraints check fail in STA
mode leaving the interface in a strange state for
a brief period of time until it is disconnected.
This was harmless but ugly.
Signed-off-by: Michal Kazior <[email protected]>
---
net/mac80211/mlme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index cadf059..6c9ebca 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1012,7 +1012,6 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
}
ifmgd->flags |= IEEE80211_STA_CSA_RECEIVED;
- sdata->vif.csa_active = true;
mutex_lock(&local->chanctx_mtx);
if (local->use_chanctx) {
@@ -1050,6 +1049,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
mutex_unlock(&local->chanctx_mtx);
sdata->csa_chandef = csa_ie.chandef;
+ sdata->vif.csa_active = true;
if (csa_ie.mode)
ieee80211_stop_queues_by_reason(&local->hw,
--
1.8.5.3
On Wed, 2014-01-29 at 07:56 +0100, Michal Kazior wrote:
> The patch adds a missing sdata lock and adds a few
> lockdeps for easier maintenance.
>
> Signed-off-by: Michal Kazior <[email protected]>
> ---
Looks okay at first glance, but Johannes is the lock master and should
review this. ;)
Otherwise, the series looks fine to me.
--
Luca.
The patch adds a missing sdata lock and adds a few
lockdeps for easier maintenance.
Signed-off-by: Michal Kazior <[email protected]>
---
net/mac80211/cfg.c | 7 ++++++-
net/mac80211/ibss.c | 2 ++
net/mac80211/iface.c | 2 ++
net/mac80211/mesh.c | 2 ++
4 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 18e8687..d7f805e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1053,6 +1053,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
@@ -1080,6 +1081,8 @@ 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;
@@ -3002,6 +3005,8 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
struct ieee80211_local *local = sdata->local;
int err, changed = 0;
+ sdata_assert_lock(sdata);
+
mutex_lock(&local->mtx);
sdata->radar_required = sdata->csa_radar_required;
err = ieee80211_vif_change_channel(sdata, &changed);
@@ -3086,7 +3091,7 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_if_mesh __maybe_unused *ifmsh;
int err, num_chanctx, changed = 0;
- lockdep_assert_held(&sdata->wdev.mtx);
+ sdata_assert_lock(sdata);
if (!list_empty(&local->roc_list) || local->scanning)
return -EBUSY;
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index f01d468..b2da79f 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -795,6 +795,8 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
int err;
u32 sta_flags;
+ sdata_assert_lock(sdata);
+
sta_flags = IEEE80211_STA_DISABLE_VHT;
switch (ifibss->chandef.width) {
case NL80211_CHAN_WIDTH_5:
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 0aa9675..5f25d30 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -822,7 +822,9 @@ 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);
sdata->vif.csa_active = false;
+ 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 bd55115..f70e9cd 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -864,6 +864,8 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
int err;
u32 sta_flags;
+ sdata_assert_lock(sdata);
+
sta_flags = IEEE80211_STA_DISABLE_VHT;
switch (sdata->vif.bss_conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
--
1.8.5.3
Instead of having
ieee80211_bss_info_change_notify() scattered all
over the place just call it once when finalizing
CSA.
As a side effect this patch adds missing error
checking for IBSS CSA beacon update.
Signed-off-by: Michal Kazior <[email protected]>
---
net/mac80211/cfg.c | 14 +++++++++-----
net/mac80211/ibss.c | 7 +++----
net/mac80211/mesh.c | 5 +++--
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index cf27c62..b503f92 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3014,8 +3014,6 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
ieee80211_hw_config(local, 0);
}
- ieee80211_bss_info_change_notify(sdata, changed);
-
sdata->vif.csa_active = false;
switch (sdata->vif.type) {
case NL80211_IFTYPE_AP:
@@ -3026,17 +3024,21 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
changed |= err;
kfree(sdata->u.ap.next_beacon);
sdata->u.ap.next_beacon = NULL;
-
- ieee80211_bss_info_change_notify(sdata, err);
break;
case NL80211_IFTYPE_ADHOC:
- ieee80211_ibss_finish_csa(sdata);
+ err = ieee80211_ibss_finish_csa(sdata);
+ if (err < 0)
+ return;
+
+ changed |= err;
break;
#ifdef CONFIG_MAC80211_MESH
case NL80211_IFTYPE_MESH_POINT:
err = ieee80211_mesh_finish_csa(sdata);
if (err < 0)
return;
+
+ changed |= err;
break;
#endif
default:
@@ -3044,6 +3046,8 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
return;
}
+ ieee80211_bss_info_change_notify(sdata, err);
+
ieee80211_wake_queues_by_reason(&sdata->local->hw,
IEEE80211_MAX_QUEUE_MAP,
IEEE80211_QUEUE_STOP_REASON_CSA);
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index ed7eec3..82d3d14 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -530,7 +530,7 @@ int ieee80211_ibss_finish_csa(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
struct cfg80211_bss *cbss;
- int err;
+ int err, changed = 0;
u16 capability;
sdata_assert_lock(sdata);
@@ -562,10 +562,9 @@ int ieee80211_ibss_finish_csa(struct ieee80211_sub_if_data *sdata)
if (err < 0)
return err;
- if (err)
- ieee80211_bss_info_change_notify(sdata, err);
+ changed |= err;
- return 0;
+ return changed;
}
void ieee80211_ibss_stop(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 836ec01..bd55115 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1058,6 +1058,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
struct mesh_csa_settings *tmp_csa_settings;
int ret = 0;
+ int changed = 0;
/* Reset the TTL value and Initiator flag */
ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
@@ -1072,11 +1073,11 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
if (ret)
return -EINVAL;
- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
+ changed |= BSS_CHANGED_BEACON;
mcsa_dbg(sdata, "complete switching to center freq %d MHz",
sdata->vif.bss_conf.chandef.chan->center_freq);
- return 0;
+ return changed;
}
int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
--
1.8.5.3