2014-08-28 11:08:33

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 0/2] ath10k: monitor fixes

Hi,

This cleans up the monitor code a bit and
introduces a workaround for a fw bug (leading to a
fw crash) when station interface associates while
a monitor vdev is running.

People have been complaining about the fw crash
for some time now. I guess we have no choice but
to make a workaround in ath10k. The most common
usecase that will benefit from this fix is
probably 4addr sta briding.


Michal Kazior (2):
ath10k: fix monitor start/stop sequences
ath10k: stop monitor vdev for sta assoc

drivers/net/wireless/ath/ath10k/core.h | 1 -
drivers/net/wireless/ath/ath10k/mac.c | 131 +++++++++++++++------------------
2 files changed, 58 insertions(+), 74 deletions(-)

--
1.8.5.3



2014-08-28 11:08:35

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 1/2] ath10k: fix monitor start/stop sequences

Fix some cases where monitor start failure left
the driver in a confused state.

This also makes the monitor code simpler.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 1 -
drivers/net/wireless/ath/ath10k/mac.c | 121 +++++++++++++--------------------
2 files changed, 49 insertions(+), 73 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 4ef4760..797741d 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -472,7 +472,6 @@ struct ath10k {
struct cfg80211_chan_def chandef;

int free_vdev_map;
- bool promisc;
bool monitor;
int monitor_vdev_id;
bool monitor_started;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index b858c82..19562a7 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -493,19 +493,6 @@ static inline int ath10k_vdev_setup_sync(struct ath10k *ar)
return 0;
}

-static bool ath10k_monitor_is_enabled(struct ath10k *ar)
-{
- lockdep_assert_held(&ar->conf_mutex);
-
- ath10k_dbg(ar, ATH10K_DBG_MAC,
- "mac monitor refs: promisc %d monitor %d cac %d\n",
- ar->promisc, ar->monitor,
- test_bit(ATH10K_CAC_RUNNING, &ar->dev_flags));
-
- return ar->promisc || ar->monitor ||
- test_bit(ATH10K_CAC_RUNNING, &ar->dev_flags);
-}
-
static int ath10k_monitor_vdev_start(struct ath10k *ar, int vdev_id)
{
struct cfg80211_chan_def *chandef = &ar->chandef;
@@ -649,16 +636,6 @@ static int ath10k_monitor_start(struct ath10k *ar)

lockdep_assert_held(&ar->conf_mutex);

- if (!ath10k_monitor_is_enabled(ar)) {
- ath10k_warn(ar, "trying to start monitor with no references\n");
- return 0;
- }
-
- if (ar->monitor_started) {
- ath10k_dbg(ar, ATH10K_DBG_MAC, "mac monitor already started\n");
- return 0;
- }
-
ret = ath10k_monitor_vdev_create(ar);
if (ret) {
ath10k_warn(ar, "failed to create monitor vdev: %d\n", ret);
@@ -678,34 +655,51 @@ static int ath10k_monitor_start(struct ath10k *ar)
return 0;
}

-static void ath10k_monitor_stop(struct ath10k *ar)
+static int ath10k_monitor_stop(struct ath10k *ar)
{
int ret;

lockdep_assert_held(&ar->conf_mutex);

- if (ath10k_monitor_is_enabled(ar)) {
- ath10k_dbg(ar, ATH10K_DBG_MAC,
- "mac monitor will be stopped later\n");
- return;
- }
-
- if (!ar->monitor_started) {
- ath10k_dbg(ar, ATH10K_DBG_MAC,
- "mac monitor probably failed to start earlier\n");
- return;
- }
-
ret = ath10k_monitor_vdev_stop(ar);
- if (ret)
+ if (ret) {
ath10k_warn(ar, "failed to stop monitor vdev: %d\n", ret);
+ return ret;
+ }

ret = ath10k_monitor_vdev_delete(ar);
- if (ret)
+ if (ret) {
ath10k_warn(ar, "failed to delete monitor vdev: %d\n", ret);
+ return ret;
+ }

ar->monitor_started = false;
ath10k_dbg(ar, ATH10K_DBG_MAC, "mac monitor stopped\n");
+
+ return 0;
+}
+
+static int ath10k_monitor_recalc(struct ath10k *ar)
+{
+ bool should_start;
+
+ lockdep_assert_held(&ar->conf_mutex);
+
+ should_start = ar->monitor ||
+ ar->filter_flags & FIF_PROMISC_IN_BSS ||
+ test_bit(ATH10K_CAC_RUNNING, &ar->dev_flags);
+
+ ath10k_dbg(ar, ATH10K_DBG_MAC,
+ "mac monitor recalc started? %d should? %d\n",
+ ar->monitor_started, should_start);
+
+ if (should_start == ar->monitor_started)
+ return 0;
+
+ if (should_start)
+ return ath10k_monitor_start(ar);
+ else
+ return ath10k_monitor_stop(ar);
}

static int ath10k_recalc_rtscts_prot(struct ath10k_vif *arvif)
@@ -736,7 +730,7 @@ static int ath10k_start_cac(struct ath10k *ar)

set_bit(ATH10K_CAC_RUNNING, &ar->dev_flags);

- ret = ath10k_monitor_start(ar);
+ ret = ath10k_monitor_recalc(ar);
if (ret) {
ath10k_warn(ar, "failed to start monitor (cac): %d\n", ret);
clear_bit(ATH10K_CAC_RUNNING, &ar->dev_flags);
@@ -2369,12 +2363,14 @@ void ath10k_halt(struct ath10k *ar)

lockdep_assert_held(&ar->conf_mutex);

- if (ath10k_monitor_is_enabled(ar)) {
- clear_bit(ATH10K_CAC_RUNNING, &ar->dev_flags);
- ar->promisc = false;
- ar->monitor = false;
+ clear_bit(ATH10K_CAC_RUNNING, &ar->dev_flags);
+ ar->filter_flags = 0;
+ ar->monitor = false;
+
+ if (ar->monitor_started)
ath10k_monitor_stop(ar);
- }
+
+ ar->monitor_started = false;

ath10k_scan_finish(ar);
ath10k_peer_cleanup_all(ar);
@@ -2629,7 +2625,7 @@ static void ath10k_config_chan(struct ath10k *ar)
/* First stop monitor interface. Some FW versions crash if there's a
* lone monitor interface. */
if (ar->monitor_started)
- ath10k_monitor_vdev_stop(ar);
+ ath10k_monitor_stop(ar);

list_for_each_entry(arvif, &ar->arvifs, list) {
if (!arvif->is_started)
@@ -2677,8 +2673,7 @@ static void ath10k_config_chan(struct ath10k *ar)
}
}

- if (ath10k_monitor_is_enabled(ar))
- ath10k_monitor_vdev_start(ar, ar->monitor_vdev_id);
+ ath10k_monitor_recalc(ar);
}

static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
@@ -2733,19 +2728,10 @@ static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
ath10k_config_ps(ar);

if (changed & IEEE80211_CONF_CHANGE_MONITOR) {
- if (conf->flags & IEEE80211_CONF_MONITOR && !ar->monitor) {
- ar->monitor = true;
- ret = ath10k_monitor_start(ar);
- if (ret) {
- ath10k_warn(ar, "failed to start monitor (config): %d\n",
- ret);
- ar->monitor = false;
- }
- } else if (!(conf->flags & IEEE80211_CONF_MONITOR) &&
- ar->monitor) {
- ar->monitor = false;
- ath10k_monitor_stop(ar);
- }
+ ar->monitor = conf->flags & IEEE80211_CONF_MONITOR;
+ ret = ath10k_monitor_recalc(ar);
+ if (ret)
+ ath10k_warn(ar, "failed to recalc monitor: %d\n", ret);
}

mutex_unlock(&ar->conf_mutex);
@@ -3009,18 +2995,9 @@ static void ath10k_configure_filter(struct ieee80211_hw *hw,
*total_flags &= SUPPORTED_FILTERS;
ar->filter_flags = *total_flags;

- if (ar->filter_flags & FIF_PROMISC_IN_BSS && !ar->promisc) {
- ar->promisc = true;
- ret = ath10k_monitor_start(ar);
- if (ret) {
- ath10k_warn(ar, "failed to start monitor (promisc): %d\n",
- ret);
- ar->promisc = false;
- }
- } else if (!(ar->filter_flags & FIF_PROMISC_IN_BSS) && ar->promisc) {
- ar->promisc = false;
- ath10k_monitor_stop(ar);
- }
+ ret = ath10k_monitor_recalc(ar);
+ if (ret)
+ ath10k_warn(ar, "failed to recalc montior: %d\n", ret);

mutex_unlock(&ar->conf_mutex);
}
--
1.8.5.3


2014-08-28 11:08:36

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 2/2] ath10k: stop monitor vdev for sta assoc

This prevents some fw revisions from crashing in
many cases when user is trying to run a
promiscuous station interface (e.g. sniffing,
4addr bridge).

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 19562a7..bf8a402 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3169,8 +3169,16 @@ static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
}

if (changed & BSS_CHANGED_ASSOC) {
- if (info->assoc)
+ if (info->assoc) {
+ /* Workaround: Make sure monitor vdev is not running
+ * when associating to prevent some firmware revisions
+ * (e.g. 10.1 and 10.2) from crashing.
+ */
+ if (ar->monitor_started)
+ ath10k_monitor_stop(ar);
ath10k_bss_assoc(hw, vif, info);
+ ath10k_monitor_recalc(ar);
+ }
}

exit:
--
1.8.5.3


2014-09-02 07:29:11

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/2] ath10k: monitor fixes

Michal Kazior <[email protected]> writes:

> Hi,
>
> This cleans up the monitor code a bit and
> introduces a workaround for a fw bug (leading to a
> fw crash) when station interface associates while
> a monitor vdev is running.
>
> People have been complaining about the fw crash
> for some time now. I guess we have no choice but
> to make a workaround in ath10k. The most common
> usecase that will benefit from this fix is
> probably 4addr sta briding.
>
>
> Michal Kazior (2):
> ath10k: fix monitor start/stop sequences
> ath10k: stop monitor vdev for sta assoc

Thanks, both patches applied.

--
Kalle Valo