Re: [PATCH] ath10k: Fix interrupt storm

[Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com>]

>> 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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k