Re: [PATCH 2/3] mac80211: Implement API to configure station specific rssi threshold

[Tamizh chelvam]

On 2018-11-09 17:19, Johannes Berg wrote:

On Mon, 2018-10-15 at 23:27 +0530, Tamizh chelvam wrote:


+   sta_mon_rssi_config_free(sta);
+   sta->rssi_hyst = rssi_hyst;
+   if (fixed_thold) {
+   if (n_rssi_tholds > 2) {
+   ret = -EINVAL;
+   goto out;
+   }


This might be slightly wrong, you free and then can still return an
error.


Sure. I'll move the free part after the validation check.


+   if (n_rssi_tholds == 1) {
+   sta->rssi_low = rssi_tholds[0];
+   sta->rssi_high = rssi_tholds[0];
+   } else {
+   sta->rssi_low = rssi_tholds[0];
+   sta->rssi_high = rssi_tholds[1];
+   }
+   } else {
+   const s32 *rssi_tholds;
+
+   rssi_tholds = kmemdup(rssi_tholds,
+ n_rssi_tholds * sizeof(s32),
+ GFP_KERNEL);
+   if (!rssi_tholds) {
+   ret = -ENOMEM;
+   goto out;
+   }


Similarly here, I guess you should do the allocation (and other error
checking) before freeing.


ditto, Sure. I'll move the free part after the validation check.


+   sta->rssi_tholds = rssi_tholds;
+   sta->n_rssi_tholds = n_rssi_tholds;
+   ieee80211_update_rssi_config(sta);





+struct sta_mon_rssi_config {
+};


Huh?

oops:( I have kept all configuring parameters in sta_info itself, 
mistakenly didn't remove this struct:(


The commit log also makes it sounds like mac80211 actually *supports*
this, but clearly that's not the case. However you don't give any data
to the driver either, did you lose a patch along the way? Previously 
you

had patch 5 ("mac80211: Implement functionality to monitor station's
rssi cross event") and if I remember correctly I said you should squash
some, but now that's not here?



Thanks,
Tamizh.

Re: [PATCH 2/3] mac80211: Implement API to configure station specific rssi threshold

[Johannes Berg]

On Mon, 2018-10-15 at 23:27 +0530, Tamizh chelvam wrote:
> 
> + sta_mon_rssi_config_free(sta);
> + sta->rssi_hyst = rssi_hyst;
> + if (fixed_thold) {
> + if (n_rssi_tholds > 2) {
> + ret = -EINVAL;
> + goto out;
> + }

This might be slightly wrong, you free and then can still return an
error.

> + if (n_rssi_tholds == 1) {
> + sta->rssi_low = rssi_tholds[0];
> + sta->rssi_high = rssi_tholds[0];
> + } else {
> + sta->rssi_low = rssi_tholds[0];
> + sta->rssi_high = rssi_tholds[1];
> + }
> + } else {
> + const s32 *rssi_tholds;
> +
> + rssi_tholds = kmemdup(rssi_tholds,
> +   n_rssi_tholds * sizeof(s32),
> +   GFP_KERNEL);
> + if (!rssi_tholds) {
> + ret = -ENOMEM;
> + goto out;
> + }

Similarly here, I guess you should do the allocation (and other error
checking) before freeing.

> + sta->rssi_tholds = rssi_tholds;
> + sta->n_rssi_tholds = n_rssi_tholds;
> + ieee80211_update_rssi_config(sta);



> +struct sta_mon_rssi_config {
> +};

Huh?


The commit log also makes it sounds like mac80211 actually *supports*
this, but clearly that's not the case. However you don't give any data
to the driver either, did you lose a patch along the way? Previously you
had patch 5 ("mac80211: Implement functionality to monitor station's
rssi cross event") and if I remember correctly I said you should squash
some, but now that's not here?

johannes

[PATCH 2/3] mac80211: Implement API to configure station specific rssi threshold

[Tamizh chelvam]

Implement set_sta_mon_rssi_config API to configure station
specific rssi threshold value to monitor change in connected
station's signal strength.

Signed-off-by: Tamizh chelvam 
---
 net/mac80211/cfg.c  |   91 +++
 net/mac80211/sta_info.c |1 +
 net/mac80211/sta_info.h |   19 ++
 3 files changed, 111 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5162233..72d2b07 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3849,6 +3849,96 @@ static int ieee80211_get_txq_stats(struct wiphy *wiphy,
return drv_get_ftm_responder_stats(local, sdata, ftm_stats);
 }
 
+void sta_mon_rssi_config_free(struct sta_info *sta)
+{
+   kfree(sta->rssi_tholds);
+   sta->rssi_tholds = NULL;
+   sta->n_rssi_tholds = 0;
+}
+
+static void ieee80211_update_rssi_config(struct sta_info *sta)
+{
+   s32 last;
+   u32 hyst;
+   int i, n;
+
+   if (!sta->n_rssi_tholds)
+   return;
+
+   if (!sta->last_rssi_event_value)
+   sta->last_rssi_event_value =
+   -ewma_signal_read(>rx_stats_avg.signal);
+
+   last = sta->last_rssi_event_value;
+   hyst = sta->rssi_hyst;
+   n = sta->n_rssi_tholds;
+
+   for (i = 0; i < n; i++)
+   if (last < sta->rssi_tholds[i])
+   break;
+
+   sta->rssi_low = i > 0 ? (sta->rssi_tholds[i - 1] - hyst) : S32_MIN;
+   sta->rssi_high = i < n ? (sta->rssi_tholds[i] + hyst - 1) : S32_MAX;
+}
+
+static int ieee80211_set_sta_mon_rssi_config(struct wiphy *wiphy,
+struct net_device *dev,
+const u8 *mac_addr,
+const s32 *rssi_tholds,
+u32 rssi_hyst, int n_rssi_tholds,
+bool fixed_thold)
+{
+   struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+   struct ieee80211_local *local = sdata->local;
+   struct sta_info *sta;
+   int ret = 0;
+
+   mutex_lock(>sta_mtx);
+
+   if (mac_addr) {
+   sta = sta_info_get_bss(sdata, mac_addr);
+   if (!sta) {
+   ret = -ENOENT;
+   goto out;
+   }
+
+   sta_mon_rssi_config_free(sta);
+   sta->rssi_hyst = rssi_hyst;
+   if (fixed_thold) {
+   if (n_rssi_tholds > 2) {
+   ret = -EINVAL;
+   goto out;
+   }
+
+   if (n_rssi_tholds == 1) {
+   sta->rssi_low = rssi_tholds[0];
+   sta->rssi_high = rssi_tholds[0];
+   } else {
+   sta->rssi_low = rssi_tholds[0];
+   sta->rssi_high = rssi_tholds[1];
+   }
+   } else {
+   const s32 *rssi_tholds;
+
+   rssi_tholds = kmemdup(rssi_tholds,
+ n_rssi_tholds * sizeof(s32),
+ GFP_KERNEL);
+   if (!rssi_tholds) {
+   ret = -ENOMEM;
+   goto out;
+   }
+
+   sta->rssi_tholds = rssi_tholds;
+   sta->n_rssi_tholds = n_rssi_tholds;
+   ieee80211_update_rssi_config(sta);
+   }
+   }
+
+out:
+   mutex_unlock(>sta_mtx);
+   return ret;
+}
+
 const struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -3944,4 +4034,5 @@ static int ieee80211_get_txq_stats(struct wiphy *wiphy,
.tx_control_port = ieee80211_tx_control_port,
.get_txq_stats = ieee80211_get_txq_stats,
.get_ftm_responder_stats = ieee80211_get_ftm_responder_stats,
+   .set_sta_mon_rssi_config = ieee80211_set_sta_mon_rssi_config,
 };
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index fb8c225..28e9a6b 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1020,6 +1020,7 @@ static void __sta_info_destroy_part2(struct sta_info *sta)
 
rate_control_remove_sta_debugfs(sta);
ieee80211_sta_debugfs_remove(sta);
+   sta_mon_rssi_config_free(sta);
 
cleanup_single_sta(sta);
 }
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 9a04327..acbad98 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -411,6 +411,9 @@ struct ieee80211_sta_rx_stats {
u64 msdu[IEEE80211_NUM_TIDS + 1];
 };
 
+struct sta_mon_rssi_config {
+};
+
 /*
  * The bandwidth threshold below which the per-station CoDel parameters will be
  * scaled to be more