[PATCH] ath10k: Fix interrupt storm

[PATCH] ath10k: Fix interrupt storm

[Vasanthakumar Thiagarajan]

Promiscuous mode is enabled when wlan interface is added to
bridge. ath10k creates a monitor mode when promiscuous mode
is enabled. When monitor vdev is runing along with other
vdev(s) there is a huge number of interrupts generated
especially in noisy condition. Fix this by not enabling
promiscuous(monitor) mode when already a vdev is running.
This does not change the support of virtual interface of
type monitor along with other vdevs.

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index d6d2f0f..310e608 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -932,6 +932,14 @@ static int ath10k_vdev_start_restart(struct ath10k_vif *arvif, bool restart)
 		return ret;
 	}
 
+	if (ar->filter_flags & FIF_PROMISC_IN_BSS) {
+		ar->filter_flags &= ~FIF_PROMISC_IN_BSS;
+		ath10k_dbg(ar, ATH10K_DBG_MAC, "Disabling promiscuous mode when we start a vdev\n");
+		ret = ath10k_monitor_recalc(ar);
+		if (ret)
+			return ret;
+	}
+
 	ar->num_started_vdevs++;
 	ath10k_recalc_radar_detection(ar);
 
@@ -3369,6 +3377,14 @@ static void ath10k_configure_filter(struct ieee80211_hw *hw,
 
 	changed_flags &= SUPPORTED_FILTERS;
 	*total_flags &= SUPPORTED_FILTERS;
+	if (*total_flags & FIF_PROMISC_IN_BSS) {
+		if (ar->num_started_vdevs) {
+			ath10k_dbg(ar, ATH10K_DBG_MAC,
+				   "Not enabling promiscuous mode when already a vdev is running\n");
+			mutex_unlock(&ar->conf_mutex);
+			return;
+		}
+	}
 	ar->filter_flags = *total_flags;
 
 	ret = ath10k_monitor_recalc(ar);
-- 
1.7.9.5

Re: [PATCH] ath10k: Fix interrupt storm

[Michal Kazior]

On 12 February 2015 at 05:59, Vasanthakumar Thiagarajan
<vthiagar@qti.qualcomm.com> wrote:
> Promiscuous mode is enabled when wlan interface is added to
> bridge. ath10k creates a monitor mode when promiscuous mode
> is enabled. When monitor vdev is runing along with other
> vdev(s) there is a huge number of interrupts generated
> especially in noisy condition. Fix this by not enabling
> promiscuous(monitor) mode when already a vdev is running.
> This does not change the support of virtual interface of
> type monitor along with other vdevs.
>
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath10k/mac.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index d6d2f0f..310e608 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -932,6 +932,14 @@ static int ath10k_vdev_start_restart(struct ath10k_vif *arvif, bool restart)
>                 return ret;
>         }
>
> +       if (ar->filter_flags & FIF_PROMISC_IN_BSS) {
> +               ar->filter_flags &= ~FIF_PROMISC_IN_BSS;
> +               ath10k_dbg(ar, ATH10K_DBG_MAC, "Disabling promiscuous mode when we start a vdev\n");

The debug message style is: "mac <message>", i.e. "mac disabling
promiscuous mode because vdev is started".


> +               ret = ath10k_monitor_recalc(ar);
> +               if (ret)
> +                       return ret;

If you fail you should undo whatever the function did if it makes
sense. In this case it makes sense to stop the vdev.


> +       }
> +
>         ar->num_started_vdevs++;
>         ath10k_recalc_radar_detection(ar);
>
> @@ -3369,6 +3377,14 @@ static void ath10k_configure_filter(struct ieee80211_hw *hw,
>
>         changed_flags &= SUPPORTED_FILTERS;
>         *total_flags &= SUPPORTED_FILTERS;
> +       if (*total_flags & FIF_PROMISC_IN_BSS) {
> +               if (ar->num_started_vdevs) {
> +                       ath10k_dbg(ar, ATH10K_DBG_MAC,
> +                                  "Not enabling promiscuous mode when already a vdev is running\n");

Message style.


> +                       mutex_unlock(&ar->conf_mutex);
> +                       return;
> +               }
> +       }
>         ar->filter_flags = *total_flags;
>
>         ret = ath10k_monitor_recalc(ar);

Anyway the entire logic should go to ath10k_monitor_recalc(). It
already has access to it and just put calls to this function in
adequate callsites.

Moreover I'm pretty sure this patch breaks STA 4addr bridging. I'm
worried it also breaks other stuff (e.g  some IBSS usecases or even
some AP usecases) but I don't have time to check this now.


Michał

Re: [PATCH] ath10k: Fix interrupt storm

[Vasanthakumar Thiagarajan]

>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
>> index d6d2f0f..310e608 100644
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -932,6 +932,14 @@ static int ath10k_vdev_start_restart(struct ath10k_vif *arvif, bool restart)
>>                  return ret;
>>          }
>>
>> +       if (ar->filter_flags & FIF_PROMISC_IN_BSS) {
>> +               ar->filter_flags &= ~FIF_PROMISC_IN_BSS;
>> +               ath10k_dbg(ar, ATH10K_DBG_MAC, "Disabling promiscuous mode when we start a vdev\n");
>
> The debug message style is: "mac <message>", i.e. "mac disabling
> promiscuous mode because vdev is started".

Ok.

>
>
>> +               ret = ath10k_monitor_recalc(ar);
>> +               if (ret)
>> +                       return ret;
>
> If you fail you should undo whatever the function did if it makes
> sense. In this case it makes sense to stop the vdev.

Sure.

>
>
>> +       }
>> +
>>          ar->num_started_vdevs++;
>>          ath10k_recalc_radar_detection(ar);
>>
>> @@ -3369,6 +3377,14 @@ static void ath10k_configure_filter(struct ieee80211_hw *hw,
>>
>>          changed_flags &= SUPPORTED_FILTERS;
>>          *total_flags &= SUPPORTED_FILTERS;
>> +       if (*total_flags & FIF_PROMISC_IN_BSS) {
>> +               if (ar->num_started_vdevs) {
>> +                       ath10k_dbg(ar, ATH10K_DBG_MAC,
>> +                                  "Not enabling promiscuous mode when already a vdev is running\n");
>
> Message style.

Ok.

>
>
>> +                       mutex_unlock(&ar->conf_mutex);
>> +                       return;
>> +               }
>> +       }
>>          ar->filter_flags = *total_flags;
>>
>>          ret = ath10k_monitor_recalc(ar);
>
> Anyway the entire logic should go to ath10k_monitor_recalc(). It
> already has access to it and just put calls to this function in
> adequate callsites.

Ok.

>
> Moreover I'm pretty sure this patch breaks STA 4addr bridging. I'm
> worried it also breaks other stuff (e.g  some IBSS usecases or even
> some AP usecases) but I don't have time to check this now.

Can you please elaborate on this one?. I can review if we can for these
cases as well before disabling promiscuous mode.

Thanks,

Vasanth

Re: [PATCH] ath10k: Fix interrupt storm

[Vasanthakumar Thiagarajan]

>
>>
>> Moreover I'm pretty sure this patch breaks STA 4addr bridging. I'm
>> worried it also breaks other stuff (e.g  some IBSS usecases or even
>> some AP usecases) but I don't have time to check this now.
>
> Can you please elaborate on this one?. I can review if we can for these
> cases as well before disabling promiscuous mode.

Instead of making the fix specific to some interface type (AP), it will be
good to understand the issue with STA and IBSS if we disable promiscuous mode.
As you know, it will be uglier if the fix has to be interface specific.

Vasanth