Oh, umm, that patch is still here ...
I guess we can combine 2 and 3 too.
> + if (sta->rssi_low && bss_conf->enable_beacon) {
> + int last_event =
> + sta->last_rssi_event_value;
> + int sig = -ewma_signal_read(&sta->rx_stats_avg.signal);
> + int low = sta->rssi_low;
> + int high = sta->rssi_high;
This doesn't really support a list, like in patch 2 where you store
sta_info::rssi_tholds?
> + if (sig < low &&
> + (last_event == 0 || last_event >= low)) {
> + sta->last_rssi_event_value = sig;
> + cfg80211_sta_mon_rssi_notify(
> + rx->sdata->dev, sta->addr,
> + NL80211_CQM_RSSI_THRESHOLD_EVENT_LOW,
> + sig, GFP_ATOMIC);
> + rssi_cross = true;
> + } else if (sig > high &&
> + (last_event == 0 || last_event <= high)) {
> + sta->last_rssi_event_value = sig;
> + cfg80211_sta_mon_rssi_notify(
> + rx->sdata->dev, sta->addr,
> + NL80211_CQM_RSSI_THRESHOLD_EVENT_HIGH,
> + sig, GFP_ATOMIC);
> + rssi_cross = true;
> + }
> + }
> +
> + if (rssi_cross) {
> + ieee80211_update_rssi_config(sta);
> + rssi_cross = false;
> + }
> +}
Ah, but it does, just hard to understand.
However, this does mean what I suspected on the other patch is true -
you're calling ieee80211_update_rssi_config() here, and that uses the
sta->rssi_tholds array without any protection, while a concurrent change
of configuration can free the data.
You need to sort that out. I would suggest to stick all the sta->rssi_*
fields you add into a new struct (you even had an empty one), and
protect that struct using RCU. That also saves the memory in case it's
not assigned at all. Something like
struct sta_mon_rssi_config {
struct rcu_head rcu_head;
int n_thresholds;
s32 low, high;
u32 hyst;
s32 last_value;
s32 thresholds[];
};
then you can kfree_rcu() it, and just do a single allocation using
struct_size() for however many entries you need.
> + * @count_rx_signal: Number of data frames used in averaging station signal.
> + * This can be used to avoid generating less reliable station rssi cross
> + * events that would be based only on couple of received frames.
> */
> struct sta_info {
> /* General information, mostly static */
> @@ -600,6 +603,7 @@ struct sta_info {
> s32 rssi_high;
> u32 rssi_hyst;
> s32 last_rssi_event_value;
> + unsigned int count_rx_signal;
I guess that would also move into the new struct.
johannes
