diff mbox series

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

Message ID 1539626250-769-3-git-send-email-tamizhr@codeaurora.org (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series cfg80211/mac80211: Add support to configure and monitor station's rssi threshold | expand

Commit Message

Tamizh chelvam Oct. 15, 2018, 5:57 p.m. UTC 
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 <tamizhr@codeaurora.org>
---
 net/mac80211/cfg.c      |   91 +++++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/sta_info.c |    1 +
 net/mac80211/sta_info.h |   19 ++++++++++
 3 files changed, 111 insertions(+)

Comments

Johannes Berg Nov. 9, 2018, 11:49 a.m. UTC | #1 
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
Tamizh chelvam Nov. 11, 2018, 1:57 p.m. UTC | #2 
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.
diff mbox series

Patch

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(&sta->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(&local->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(&local->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 lenient (to prevent starvation of slow stations). This
@@ -482,6 +485,14 @@  struct ieee80211_sta_rx_stats {
  * @pcpu_rx_stats: per-CPU RX statistics, assigned only if the driver needs
  *	this (by advertising the USES_RSS hw flag)
  * @status_stats: TX status statistics
+ * @n_rssi_tholds: Number of thresholds passed by user
+ * @rssi_tholds: RSSI threshold limit passed by the user
+ * @rssi_low: RSSI lower threshold for this station, a zero value implies
+ *	disabled
+ * @rssi_high: RSSI upper threshold for this station
+ * @rssi_hyst: RSSI hysteresis for this station
+ * @last_rssi_event_value: Last RSSI value for this station triggered the
+ *	RSSI cross event.
  */
 struct sta_info {
 	/* General information, mostly static */
@@ -583,6 +594,13 @@  struct sta_info {
 
 	struct cfg80211_chan_def tdls_chandef;
 
+	int n_rssi_tholds;
+	const s32 *rssi_tholds;
+	s32 rssi_low;
+	s32 rssi_high;
+	u32 rssi_hyst;
+	s32 last_rssi_event_value;
+
 	/* keep last! */
 	struct ieee80211_sta sta;
 };
@@ -720,6 +738,7 @@  int sta_info_destroy_addr(struct ieee80211_sub_if_data *sdata,
 			  const u8 *addr);
 int sta_info_destroy_addr_bss(struct ieee80211_sub_if_data *sdata,
 			      const u8 *addr);
+void sta_mon_rssi_config_free(struct sta_info *sta);
 
 void sta_info_recalc_tim(struct sta_info *sta);