[PATCH 0/7] cfg80211/mac80211: Add support to configure and monitor rssi threshold

[PATCH 0/7] cfg80211/mac80211: Add support to configure and monitor rssi threshold

[Tamizh chelvam]

This patchsets introduced new NL command and api to support
configuring rssi for the connected stations and api to notify
userspace application upon crossing the configured threshold.
This will be useful for the application which requires
station's current capability change information.

Tamizh chelvam (7):
  wireless: Change single cqm_config to rssi config list
  cfg80211: Add new NL command to configure peer specific rssi
    threshold
  mac80211: Add api to support configuring station specific rssi
    threshold
  cfg80211: Add support to notify station's rssi level crossing
  mac80211: Implement functionality to monitor station's rssi cross
    event
  cfg80211: Accept multiple RSSI threholds for STA_MON command
  mac80211: Add api to configure low and high RSSI threshold

 include/net/cfg80211.h       |   42 ++++-
 include/net/mac80211.h       |    6 +
 include/uapi/linux/nl80211.h |   60 +++++++
 net/mac80211/cfg.c           |   75 +++++++++
 net/mac80211/rx.c            |   65 +++++++-
 net/mac80211/sta_info.h      |   22 +++
 net/wireless/core.c          |   29 +++-
 net/wireless/core.h          |    6 +-
 net/wireless/nl80211.c       |  377 ++++++++++++++++++++++++++++++++++++------
 net/wireless/rdev-ops.h      |   31 ++++
 net/wireless/trace.h         |   73 ++++++++
 11 files changed, 723 insertions(+), 63 deletions(-)

-- 
1.7.9.5

[PATCH 1/7] wireless: Change single cqm_config to rssi config list

[Tamizh chelvam]

This patch changes single cqm_config into mac address based
rssi config list. This way the same structure can be
utilized by AP mode as well.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 include/net/cfg80211.h |    8 ++++--
 net/wireless/core.c    |   29 +++++++++++++++----
 net/wireless/core.h    |    6 ++--
 net/wireless/nl80211.c |   74 ++++++++++++++++++++++++++++++++----------------
 4 files changed, 82 insertions(+), 35 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5fbfe61..3e123a3 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4139,7 +4139,7 @@ static inline struct wiphy *wiphy_new(const struct cfg80211_ops *ops,
 struct cfg80211_conn;
 struct cfg80211_internal_bss;
 struct cfg80211_cached_keys;
-struct cfg80211_cqm_config;
+struct cfg80211_rssi_config;
 
 /**
  * struct wireless_dev - wireless device state
@@ -4204,7 +4204,8 @@ static inline struct wiphy *wiphy_new(const struct cfg80211_ops *ops,
  * @event_lock: (private) lock for event list
  * @owner_nlportid: (private) owner socket port ID
  * @nl_owner_dead: (private) owner socket went away
- * @cqm_config: (private) nl80211 RSSI monitor state
+ * @rssi_config: (private) nl80211 RSSI monitor state
+ * @rssi_config_list: (private) peer based list for rssi config
  */
 struct wireless_dev {
 	struct wiphy *wiphy;
@@ -4275,7 +4276,8 @@ struct wireless_dev {
 	} wext;
 #endif
 
-	struct cfg80211_cqm_config *cqm_config;
+	struct cfg80211_rssi_config *rssi_config;
+	struct list_head rssi_config_list;
 };
 
 static inline u8 *wdev_address(struct wireless_dev *wdev)
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 5fe35aa..62e496e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -994,10 +994,28 @@ void wiphy_rfkill_set_hw_state(struct wiphy *wiphy, bool blocked)
 }
 EXPORT_SYMBOL(wiphy_rfkill_set_hw_state);
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev)
+void cfg80211_rssi_config_free(struct wireless_dev *wdev, const u8 *peer)
 {
-	kfree(wdev->cqm_config);
-	wdev->cqm_config = NULL;
+	struct cfg80211_rssi_config *rssi_config, *tmp;
+
+	if (list_empty(&wdev->rssi_config_list))
+		goto free;
+
+	list_for_each_entry_safe(rssi_config, tmp, &wdev->rssi_config_list,
+				 list) {
+		if (peer && memcmp(rssi_config->addr, peer, ETH_ALEN))
+			continue;
+
+		list_del(&rssi_config->list);
+		kfree(rssi_config);
+		if (list_empty(&wdev->rssi_config_list) || peer)
+			goto out;
+	}
+
+free:
+	kfree(wdev->rssi_config);
+out:
+	wdev->rssi_config = NULL;
 }
 
 void cfg80211_unregister_wdev(struct wireless_dev *wdev)
@@ -1027,7 +1045,7 @@ void cfg80211_unregister_wdev(struct wireless_dev *wdev)
 		break;
 	}
 
-	cfg80211_cqm_config_free(wdev);
+	cfg80211_rssi_config_free(wdev, NULL);
 }
 EXPORT_SYMBOL(cfg80211_unregister_wdev);
 
@@ -1163,6 +1181,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		spin_lock_init(&wdev->event_lock);
 		INIT_LIST_HEAD(&wdev->mgmt_registrations);
 		spin_lock_init(&wdev->mgmt_registrations_lock);
+		INIT_LIST_HEAD(&wdev->rssi_config_list);
 
 		/*
 		 * We get here also when the interface changes network namespaces,
@@ -1292,7 +1311,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 			kzfree(wdev->wext.keys);
 #endif
 			flush_work(&wdev->disconnect_wk);
-			cfg80211_cqm_config_free(wdev);
+			cfg80211_rssi_config_free(wdev, NULL);
 		}
 		/*
 		 * synchronise (so that we won't find this netdev
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 63eb1b5..170a31b 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -260,10 +260,12 @@ struct cfg80211_beacon_registration {
 	u32 nlportid;
 };
 
-struct cfg80211_cqm_config {
+struct cfg80211_rssi_config {
+	struct list_head list;
 	u32 rssi_hyst;
 	s32 last_rssi_event_value;
 	int n_rssi_thresholds;
+	u8 addr[ETH_ALEN];
 	s32 rssi_thresholds[0];
 };
 
@@ -514,6 +516,6 @@ void cfg80211_stop_nan(struct cfg80211_registered_device *rdev,
 #define CFG80211_DEV_WARN_ON(cond)	({bool __r = (cond); __r; })
 #endif
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev);
+void cfg80211_rssi_config_free(struct wireless_dev *wdev, const u8 *peer);
 
 #endif /* __NET_WIRELESS_CORE_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 07514ca..2fb2e97 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3200,6 +3200,7 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
 		spin_lock_init(&wdev->event_lock);
 		INIT_LIST_HEAD(&wdev->mgmt_registrations);
 		spin_lock_init(&wdev->mgmt_registrations_lock);
+		INIT_LIST_HEAD(&wdev->rssi_config_list);
 
 		wdev->identifier = ++rdev->wdev_id;
 		list_add_rcu(&wdev->list, &rdev->wiphy.wdev_list);
@@ -10140,7 +10141,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	int err;
 
 	/* RSSI reporting disabled? */
-	if (!wdev->cqm_config)
+	if (!wdev->rssi_config)
 		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
 
 	/*
@@ -10149,7 +10150,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	 * connection is established and enough beacons received to calculate
 	 * the average.
 	 */
-	if (!wdev->cqm_config->last_rssi_event_value && wdev->current_bss &&
+	if (!wdev->rssi_config->last_rssi_event_value && wdev->current_bss &&
 	    rdev->ops->get_station) {
 		struct station_info sinfo = {};
 		u8 *mac_addr;
@@ -10161,26 +10162,56 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 			return err;
 
 		if (sinfo.filled & BIT(NL80211_STA_INFO_BEACON_SIGNAL_AVG))
-			wdev->cqm_config->last_rssi_event_value =
+			wdev->rssi_config->last_rssi_event_value =
 				(s8) sinfo.rx_beacon_signal_avg;
 	}
 
-	last = wdev->cqm_config->last_rssi_event_value;
-	hyst = wdev->cqm_config->rssi_hyst;
-	n = wdev->cqm_config->n_rssi_thresholds;
+	last = wdev->rssi_config->last_rssi_event_value;
+	hyst = wdev->rssi_config->rssi_hyst;
+	n = wdev->rssi_config->n_rssi_thresholds;
 
 	for (i = 0; i < n; i++)
-		if (last < wdev->cqm_config->rssi_thresholds[i])
+		if (last < wdev->rssi_config->rssi_thresholds[i])
 			break;
 
 	low = i > 0 ?
-		(wdev->cqm_config->rssi_thresholds[i - 1] - hyst) : S32_MIN;
+		(wdev->rssi_config->rssi_thresholds[i - 1] - hyst) : S32_MIN;
 	high = i < n ?
-		(wdev->cqm_config->rssi_thresholds[i] + hyst - 1) : S32_MAX;
+		(wdev->rssi_config->rssi_thresholds[i] + hyst - 1) : S32_MAX;
 
 	return rdev_set_cqm_rssi_range_config(rdev, dev, low, high);
 }
 
+static struct cfg80211_rssi_config *
+cfg80211_get_rssi_config(struct wireless_dev *wdev, const s32 *thresholds,
+			 int n_thresholds, u32 hysteresis, const u8 *peer)
+{
+	struct cfg80211_rssi_config *rssi_config;
+
+	if (!peer)
+		return NULL;
+
+	if (list_empty(&wdev->rssi_config_list))
+		goto new;
+
+	list_for_each_entry(rssi_config, &wdev->rssi_config_list, list) {
+		if (!memcmp(rssi_config->addr, peer, ETH_ALEN))
+			goto found;
+	}
+
+new:
+	rssi_config = kzalloc(sizeof(struct cfg80211_rssi_config) +
+			      n_thresholds * sizeof(s32), GFP_KERNEL);
+	list_add(&rssi_config->list, &wdev->rssi_config_list);
+found:
+	rssi_config->rssi_hyst = hysteresis;
+	rssi_config->n_rssi_thresholds = n_thresholds;
+	memcpy(rssi_config->addr, peer, ETH_ALEN);
+	memcpy(rssi_config->rssi_thresholds, thresholds,
+	       n_thresholds * sizeof(s32));
+	return rssi_config;
+}
+
 static int nl80211_set_cqm_rssi(struct genl_info *info,
 				const s32 *thresholds, int n_thresholds,
 				u32 hysteresis)
@@ -10204,7 +10235,7 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		return -EOPNOTSUPP;
 
 	wdev_lock(wdev);
-	cfg80211_cqm_config_free(wdev);
+	cfg80211_rssi_config_free(wdev, NULL);
 	wdev_unlock(wdev);
 
 	if (n_thresholds <= 1 && rdev->ops->set_cqm_rssi_config) {
@@ -10224,21 +10255,14 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 
 	wdev_lock(wdev);
 	if (n_thresholds) {
-		struct cfg80211_cqm_config *cqm_config;
-
-		cqm_config = kzalloc(sizeof(struct cfg80211_cqm_config) +
-				     n_thresholds * sizeof(s32), GFP_KERNEL);
-		if (!cqm_config) {
+		wdev->rssi_config = cfg80211_get_rssi_config(
+						wdev, thresholds,
+						n_thresholds, hysteresis,
+						wdev->current_bss->pub.bssid);
+		if (!wdev->rssi_config) {
 			err = -ENOMEM;
 			goto unlock;
 		}
-
-		cqm_config->rssi_hyst = hysteresis;
-		cqm_config->n_rssi_thresholds = n_thresholds;
-		memcpy(cqm_config->rssi_thresholds, thresholds,
-		       n_thresholds * sizeof(s32));
-
-		wdev->cqm_config = cqm_config;
 	}
 
 	err = cfg80211_cqm_rssi_update(rdev, dev);
@@ -15053,13 +15077,13 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 		    rssi_event != NL80211_CQM_RSSI_THRESHOLD_EVENT_HIGH))
 		return;
 
-	if (wdev->cqm_config) {
-		wdev->cqm_config->last_rssi_event_value = rssi_level;
+	if (wdev->rssi_config) {
+		wdev->rssi_config->last_rssi_event_value = rssi_level;
 
 		cfg80211_cqm_rssi_update(rdev, dev);
 
 		if (rssi_level == 0)
-			rssi_level = wdev->cqm_config->last_rssi_event_value;
+			rssi_level = wdev->rssi_config->last_rssi_event_value;
 	}
 
 	msg = cfg80211_prepare_cqm(dev, NULL, gfp);
-- 
1.7.9.5

Re: [PATCH 1/7] wireless: Change single cqm_config to rssi config list

[Johannes Berg]

On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 5fbfe61..3e123a3 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -4139,7 +4139,7 @@ static inline struct wiphy *wiphy_new(const struct cfg80211_ops *ops,
>  struct cfg80211_conn;
>  struct cfg80211_internal_bss;
>  struct cfg80211_cached_keys;
> -struct cfg80211_cqm_config;
> +struct cfg80211_rssi_config;

I think something like rssi_mon_config would be better? Just _rssi_
could be interpreted in many ways.
 
> -	struct cfg80211_cqm_config *cqm_config;
> +	struct cfg80211_rssi_config *rssi_config;
> +	struct list_head rssi_config_list;

Why do you need both now? Perhaps instead you should allow a NULL/all-
ones MAC address for where you have the direct pointer now, and remove
that? You anyway need to pass something to the peer argument in

> -void cfg80211_cqm_config_free(struct wireless_dev *wdev)
> +void cfg80211_rssi_config_free(struct wireless_dev *wdev, const u8 *peer)

even when the pointer is used.

> -	kfree(wdev->cqm_config);
> -	wdev->cqm_config = NULL;
> +	struct cfg80211_rssi_config *rssi_config, *tmp;
> +
> +	if (list_empty(&wdev->rssi_config_list))
> +		goto free;
> +
> +	list_for_each_entry_safe(rssi_config, tmp, &wdev->rssi_config_list,
> +				 list) {
> +		if (peer && memcmp(rssi_config->addr, peer, ETH_ALEN))
> +			continue;
> +
> +		list_del(&rssi_config->list);
> +		kfree(rssi_config);
> +		if (list_empty(&wdev->rssi_config_list) || peer)
> +			goto out;
> +	}

That logic could use some comments if we even decide to keep it this
way.

NULL means "free all"?

What's the locking scheme for this? It's way more complex now so perhaps
stick ASSERT_RTNL() in there or so?

> @@ -10140,7 +10141,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
>  	int err;
>  
>  	/* RSSI reporting disabled? */
> -	if (!wdev->cqm_config)
> +	if (!wdev->rssi_config)
>  		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);


So I guess the wdev->rssi_config is used for the case of "always use
this config for any AP you're connected to" or so - but maybe it'd be
better to track the AP as a station? Then again, I guess we can't force
userspace to change that.

> +static struct cfg80211_rssi_config *
> +cfg80211_get_rssi_config(struct wireless_dev *wdev, const s32 *thresholds,
> +			 int n_thresholds, u32 hysteresis, const u8 *peer)
> +{
> +	struct cfg80211_rssi_config *rssi_config;
> +
> +	if (!peer)
> +		return NULL;
> +
> +	if (list_empty(&wdev->rssi_config_list))
> +		goto new;
> +
> +	list_for_each_entry(rssi_config, &wdev->rssi_config_list, list) {
> +		if (!memcmp(rssi_config->addr, peer, ETH_ALEN))
> +			goto found;
> +	}
> +
> +new:
> +	rssi_config = kzalloc(sizeof(struct cfg80211_rssi_config) +
> +			      n_thresholds * sizeof(s32), GFP_KERNEL);
> +	list_add(&rssi_config->list, &wdev->rssi_config_list);

Why does "get" always imply "create"?

>  	wdev_lock(wdev);
>  	if (n_thresholds) {
> -		struct cfg80211_cqm_config *cqm_config;
> -
> -		cqm_config = kzalloc(sizeof(struct cfg80211_cqm_config) +
> -				     n_thresholds * sizeof(s32), GFP_KERNEL);
> -		if (!cqm_config) {
> +		wdev->rssi_config = cfg80211_get_rssi_config(
> +						wdev, thresholds,
> +						n_thresholds, hysteresis,
> +						wdev->current_bss->pub.bssid);

Here you link it to the BSSID anyway, but do we always have a
current_bss at this point?

johannes

Re: [PATCH 1/7] wireless: Change single cqm_config to rssi config list

[Tamizh chelvam]

On 2018-06-29 14:56, Johannes Berg wrote:
> On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
>> 
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index 5fbfe61..3e123a3 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -4139,7 +4139,7 @@ static inline struct wiphy *wiphy_new(const 
>> struct cfg80211_ops *ops,
>>  struct cfg80211_conn;
>>  struct cfg80211_internal_bss;
>>  struct cfg80211_cached_keys;
>> -struct cfg80211_cqm_config;
>> +struct cfg80211_rssi_config;
> 
> I think something like rssi_mon_config would be better? Just _rssi_
> could be interpreted in many ways.
> 
Okay sure
>> -	struct cfg80211_cqm_config *cqm_config;
>> +	struct cfg80211_rssi_config *rssi_config;
>> +	struct list_head rssi_config_list;
> 
> Why do you need both now? Perhaps instead you should allow a NULL/all-
> ones MAC address for where you have the direct pointer now, and remove
> that? You anyway need to pass something to the peer argument in
> 
In the current cqm/sta_mon implementation the range_config will be 
updated before notify event posted to userspace.
To update the range config for a specific station we need to have this 
peer addr based configuration list which holds the thresholds info
to choose the next rssi range.
Or do you want me to handle and store the rssi_config structure in 
mac80211 and update it in mac80211 itself ? And simply pass the 
notification event to userspace application ?

>> -void cfg80211_cqm_config_free(struct wireless_dev *wdev)
>> +void cfg80211_rssi_config_free(struct wireless_dev *wdev, const u8 
>> *peer)
> 
> even when the pointer is used.
> 
>> -	kfree(wdev->cqm_config);
>> -	wdev->cqm_config = NULL;
>> +	struct cfg80211_rssi_config *rssi_config, *tmp;
>> +
>> +	if (list_empty(&wdev->rssi_config_list))
>> +		goto free;
>> +
>> +	list_for_each_entry_safe(rssi_config, tmp, &wdev->rssi_config_list,
>> +				 list) {
>> +		if (peer && memcmp(rssi_config->addr, peer, ETH_ALEN))
>> +			continue;
>> +
>> +		list_del(&rssi_config->list);
>> +		kfree(rssi_config);
>> +		if (list_empty(&wdev->rssi_config_list) || peer)
>> +			goto out;
>> +	}
> 
> That logic could use some comments if we even decide to keep it this
> way.
> 
> NULL means "free all"?
> 
Yes.
> What's the locking scheme for this? It's way more complex now so 
> perhaps
> stick ASSERT_RTNL() in there or so?
> 
Isn't wdev_lock enough ?
>> @@ -10140,7 +10141,7 @@ static int cfg80211_cqm_rssi_update(struct 
>> cfg80211_registered_device *rdev,
>>  	int err;
>> 
>>  	/* RSSI reporting disabled? */
>> -	if (!wdev->cqm_config)
>> +	if (!wdev->rssi_config)
>>  		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
> 
> 
> So I guess the wdev->rssi_config is used for the case of "always use
> this config for any AP you're connected to" or so - but maybe it'd be
> better to track the AP as a station? Then again, I guess we can't force
> userspace to change that.
> 
Sorry I didn't get your point:( I didn't change anything in the station 
mode. Functionality remains
same for the station mode.

>> +static struct cfg80211_rssi_config *
>> +cfg80211_get_rssi_config(struct wireless_dev *wdev, const s32 
>> *thresholds,
>> +			 int n_thresholds, u32 hysteresis, const u8 *peer)
>> +{
>> +	struct cfg80211_rssi_config *rssi_config;
>> +
>> +	if (!peer)
>> +		return NULL;
>> +
>> +	if (list_empty(&wdev->rssi_config_list))
>> +		goto new;
>> +
>> +	list_for_each_entry(rssi_config, &wdev->rssi_config_list, list) {
>> +		if (!memcmp(rssi_config->addr, peer, ETH_ALEN))
>> +			goto found;
>> +	}
>> +
>> +new:
>> +	rssi_config = kzalloc(sizeof(struct cfg80211_rssi_config) +
>> +			      n_thresholds * sizeof(s32), GFP_KERNEL);
>> +	list_add(&rssi_config->list, &wdev->rssi_config_list);
> 
> Why does "get" always imply "create"?
> 
I'll rename the function.

>>  	wdev_lock(wdev);
>>  	if (n_thresholds) {
>> -		struct cfg80211_cqm_config *cqm_config;
>> -
>> -		cqm_config = kzalloc(sizeof(struct cfg80211_cqm_config) +
>> -				     n_thresholds * sizeof(s32), GFP_KERNEL);
>> -		if (!cqm_config) {
>> +		wdev->rssi_config = cfg80211_get_rssi_config(
>> +						wdev, thresholds,
>> +						n_thresholds, hysteresis,
>> +						wdev->current_bss->pub.bssid);
> 
> Here you link it to the BSSID anyway, but do we always have a
> current_bss at this point?
> 
Oops! I didn't think about that condition. I'll fix that in the next 
version.

Re: [PATCH 1/7] wireless: Change single cqm_config to rssi config list

[Johannes Berg]

On Wed, 2018-07-04 at 23:46 +0530, Tamizh chelvam wrote:

> > > -	struct cfg80211_cqm_config *cqm_config;
> > > +	struct cfg80211_rssi_config *rssi_config;
> > > +	struct list_head rssi_config_list;
> > 
> > Why do you need both now? Perhaps instead you should allow a NULL/all-
> > ones MAC address for where you have the direct pointer now, and remove
> > that? You anyway need to pass something to the peer argument in
> > 
> 
> In the current cqm/sta_mon implementation the range_config will be 
> updated before notify event posted to userspace.
> To update the range config for a specific station we need to have this 
> peer addr based configuration list which holds the thresholds info
> to choose the next rssi range.
> Or do you want me to handle and store the rssi_config structure in 
> mac80211 and update it in mac80211 itself ? And simply pass the 
> notification event to userspace application ?

I don't have any issues with storing it in mac80211 - could attach it to
the station entry there?

Come to think of it, that might also clarify the lifetime rules. As it
is now, what if the station is removed? What are the lifetime rules for
a per-station configuration?

It would almost look like it stays here (or maybe I missed when you
remove it from the list when a station is removed), but that doesn't
seem like a good idea.

Please document the lifetime rules also in the API, and make sure you
implement them properly.

> > What's the locking scheme for this? It's way more complex now so 
> > perhaps stick ASSERT_RTNL() in there or so?
> > 
> 
> Isn't wdev_lock enough ?

I guess it should be :-) Maybe assert on that, also to document the
locking rules.

> > >  	/* RSSI reporting disabled? */
> > > -	if (!wdev->cqm_config)
> > > +	if (!wdev->rssi_config)
> > >  		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
> > 
> > 
> > So I guess the wdev->rssi_config is used for the case of "always use
> > this config for any AP you're connected to" or so - but maybe it'd be
> > better to track the AP as a station? Then again, I guess we can't force
> > userspace to change that.

> Sorry I didn't get your point:( I didn't change anything in the station 
> mode. Functionality remains same for the station mode.

Right, that sort of goes back to my thoughts about lifetime rules too.
The rule for stations would be to have it deleted when the station is
deleted, but I guess the rule for client-mode is to keep the config
active even when roaming?

I was just thinking that you don't need to *store* it like that, you
could still - in client mode - store the configuration treating the AP
as a (peer) station, perhaps with an ff:ff:ff:ff:ff:ff MAC address or
something (instead of using the BSSID, to get lifetime rules adjusted
right).

But now that I'm thinking about how the lifetime rules differ ... that's
probably not worth it.

> > >  	wdev_lock(wdev);
> > >  	if (n_thresholds) {
> > > -		struct cfg80211_cqm_config *cqm_config;
> > > -
> > > -		cqm_config = kzalloc(sizeof(struct cfg80211_cqm_config) +
> > > -				     n_thresholds * sizeof(s32), GFP_KERNEL);
> > > -		if (!cqm_config) {
> > > +		wdev->rssi_config = cfg80211_get_rssi_config(
> > > +						wdev, thresholds,
> > > +						n_thresholds, hysteresis,
> > > +						wdev->current_bss->pub.bssid);
> > 
> > Here you link it to the BSSID anyway, but do we always have a
> > current_bss at this point?
> > 
> 
> Oops! I didn't think about that condition. I'll fix that in the next 
> version.

Again though - see above. I'm not sure it's even correct to link it to
the BSSID since until now, I think the configuration would remain across
roaming?

johannes

Re: [PATCH 1/7] wireless: Change single cqm_config to rssi config list

[Tamizh chelvam]

On 2018-07-06 17:16, Johannes Berg wrote:
> On Wed, 2018-07-04 at 23:46 +0530, Tamizh chelvam wrote:
> 
>> > > -	struct cfg80211_cqm_config *cqm_config;
>> > > +	struct cfg80211_rssi_config *rssi_config;
>> > > +	struct list_head rssi_config_list;
>> >
>> > Why do you need both now? Perhaps instead you should allow a NULL/all-
>> > ones MAC address for where you have the direct pointer now, and remove
>> > that? You anyway need to pass something to the peer argument in
>> >
>> 
>> In the current cqm/sta_mon implementation the range_config will be
>> updated before notify event posted to userspace.
>> To update the range config for a specific station we need to have this
>> peer addr based configuration list which holds the thresholds info
>> to choose the next rssi range.
>> Or do you want me to handle and store the rssi_config structure in
>> mac80211 and update it in mac80211 itself ? And simply pass the
>> notification event to userspace application ?
> 
> I don't have any issues with storing it in mac80211 - could attach it 
> to
> the station entry there?
yes.
> 
> Come to think of it, that might also clarify the lifetime rules. As it
> is now, what if the station is removed? What are the lifetime rules for
> a per-station configuration?
If we go with mac80211 based approach then the lifetime of the 
configuration would be for the current connection.
> 
> It would almost look like it stays here (or maybe I missed when you
> remove it from the list when a station is removed), but that doesn't
> seem like a good idea.
> 
> Please document the lifetime rules also in the API, and make sure you
> implement them properly.
In AP mode, the lifetime would be for the station's current connection. 
It will be removed in case of roaming/disconnected.
> 
>> > What's the locking scheme for this? It's way more complex now so
>> > perhaps stick ASSERT_RTNL() in there or so?
>> >
>> 
>> Isn't wdev_lock enough ?
> 
> I guess it should be :-) Maybe assert on that, also to document the
> locking rules.
Sure
> 
>> > >  	/* RSSI reporting disabled? */
>> > > -	if (!wdev->cqm_config)
>> > > +	if (!wdev->rssi_config)
>> > >  		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
>> >
>> >
>> > So I guess the wdev->rssi_config is used for the case of "always use
>> > this config for any AP you're connected to" or so - but maybe it'd be
>> > better to track the AP as a station? Then again, I guess we can't force
>> > userspace to change that.
> 
>> Sorry I didn't get your point:( I didn't change anything in the 
>> station
>> mode. Functionality remains same for the station mode.
> 
> Right, that sort of goes back to my thoughts about lifetime rules too.
> The rule for stations would be to have it deleted when the station is
> deleted, but I guess the rule for client-mode is to keep the config
> active even when roaming?
> 
> I was just thinking that you don't need to *store* it like that, you
> could still - in client mode - store the configuration treating the AP
> as a (peer) station, perhaps with an ff:ff:ff:ff:ff:ff MAC address or
> something (instead of using the BSSID, to get lifetime rules adjusted
> right).
> 
Going with mac80211 based storing configuration will remove these list 
implementation in the cfg80211.
So, there won't be any MAC address stored for client-mode in cqm_config 
structure.

> But now that I'm thinking about how the lifetime rules differ ... 
> that's
> probably not worth it.
> 
>> > >  	wdev_lock(wdev);
>> > >  	if (n_thresholds) {
>> > > -		struct cfg80211_cqm_config *cqm_config;
>> > > -
>> > > -		cqm_config = kzalloc(sizeof(struct cfg80211_cqm_config) +
>> > > -				     n_thresholds * sizeof(s32), GFP_KERNEL);
>> > > -		if (!cqm_config) {
>> > > +		wdev->rssi_config = cfg80211_get_rssi_config(
>> > > +						wdev, thresholds,
>> > > +						n_thresholds, hysteresis,
>> > > +						wdev->current_bss->pub.bssid);
>> >
>> > Here you link it to the BSSID anyway, but do we always have a
>> > current_bss at this point?
>> >
>> 
>> Oops! I didn't think about that condition. I'll fix that in the next
>> version.
> 
> Again though - see above. I'm not sure it's even correct to link it to
> the BSSID since until now, I think the configuration would remain 
> across
> roaming?
> 
As mentioned above storing configuration parameters in mac80211 will 
avoid this list implementation here.
So, no need of worrying about those roaming scenario know ?

Thanks,
Tamizh.

Re: [PATCH 1/7] wireless: Change single cqm_config to rssi config list

[Johannes Berg]

On Wed, 2018-07-11 at 12:24 +0530, Tamizh chelvam wrote:
> 
> > Again though - see above. I'm not sure it's even correct to link it to
> > the BSSID since until now, I think the configuration would remain 
> > across
> > roaming?
> > 
> 
> As mentioned above storing configuration parameters in mac80211 will 
> avoid this list implementation here.
> So, no need of worrying about those roaming scenario know ?

I think we should tie it to the station lifetime and have it
reconfigured on roaming etc., and if that means it's easier by storing
in mac80211 I have no objection.

johannes

[PATCH 2/7] cfg80211: Add new NL command to configure peer specific rssi threshold

[Tamizh chelvam]

This patch add support to configure peer specific rssi configuration
to monitor station's signal strength variation.
New NL80211_CMD_SET_STA_MON introduced to configure rssi threshold
and hysteresis info using NL80211_ATTR_STA_MON_RSSI_THOLD and
NL80211_ATTR_STA_MON_RSSI_HYST along with NL80211_ATTR_MAC.
Driver supporting this configuration should advertise
NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG.
Monitoring station's signal strength variation will be useful
for the application like steering.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 include/net/cfg80211.h       |    9 +++++
 include/uapi/linux/nl80211.h |   55 ++++++++++++++++++++++++++++++
 net/wireless/nl80211.c       |   77 ++++++++++++++++++++++++++++++++++++++++++
 net/wireless/rdev-ops.h      |   16 +++++++++
 net/wireless/trace.h         |   25 ++++++++++++++
 5 files changed, 182 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 3e123a3..584697b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3024,6 +3024,11 @@ struct cfg80211_external_auth_params {
  *
  * @tx_control_port: TX a control port frame (EAPoL).  The noencrypt parameter
  *	tells the driver that the frame should not be encrypted.
+ * @set_sta_mon_rssi_config: Configure  RSSI threshold for a station.
+ *	After configuration, the driver should (soon) send an event indicating
+ *	the current level of a station is above/below the configured threshold;
+ *	this may need some care when the configuration is changed
+ *	(without first being disabled.)
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3329,6 +3334,10 @@ struct cfg80211_ops {
 				   const u8 *buf, size_t len,
 				   const u8 *dest, const __be16 proto,
 				   const bool noencrypt);
+	int	(*set_sta_mon_rssi_config)(struct wiphy *wiphy,
+					   struct net_device *dev,
+					   const u8 *addr,
+					   s32 rssi_thold, u32 rssi_hyst);
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 28b3654..f13ad07 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1033,6 +1033,9 @@
  *	&NL80211_ATTR_CHANNEL_WIDTH,&NL80211_ATTR_NSS attributes with its
  *	address(specified in &NL80211_ATTR_MAC).
  *
+ * @NL80211_CMD_SET_STA_MON: This command is used to configure station's
+ *	connection monitoring notification trigger levels.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1245,6 +1248,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_CONTROL_PORT_FRAME,
 
+	NL80211_CMD_SET_STA_MON,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -2238,6 +2243,9 @@ enum nl80211_commands {
  * @NL80211_ATTR_TXQ_QUANTUM: TXQ scheduler quantum (bytes). Number of bytes
  *      a flow is assigned on each round of the DRR scheduler.
  *
+ * @NL80211_ATTR_STA_MON: Station's connection monitor configuration in a
+ *	nested attribute with %NL80211_ATTR_STA_MON_* sub-attributes.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2677,6 +2685,8 @@ enum nl80211_attrs {
 	NL80211_ATTR_TXQ_MEMORY_LIMIT,
 	NL80211_ATTR_TXQ_QUANTUM,
 
+	NL80211_ATTR_STA_MON,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -4276,6 +4286,46 @@ enum nl80211_ps_state {
 };
 
 /**
+ * enum nl80211_attr_sta_mon - Attributes to monitor station's connection
+ * @NL80211_ATTR_STA_MON_RSSI_THOLD: RSSI threshold in dBm. This value specifies
+ *	the threshold for the RSSI level at which an event will be sent. Zero
+ *	to disable.  Alternatively, if %NL80211_EXT_FEATURE_STA_MON_RSSI_LIST is
+ *	set, multiple values can be supplied as a low-to-high sorted array of\
+ *	threshold values in dBm.  Events will be sent when the RSSI value
+ *	crosses any of the thresholds. This threshold values are station
+ *	specific.
+ * @NL80211_ATTR_STA_MON_RSSI_HYST: RSSI hysteresis in dBm. This value specifies
+ *	the minimum amount the RSSI level must change after an event before a
+ *	new event may be issued (to reduce effects of RSSI oscillation).
+ * @NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT: RSSI threshold event
+ * @NL80211_ATTR_STA_MON_RSSI_LEVEL: the RSSI value in dBm that triggered the
+ *	RSSI threshold event.
+ */
+enum nl80211_attr_sta_mon {
+	__NL80211_ATTR_STA_MON_INVALID,
+	NL80211_ATTR_STA_MON_RSSI_THOLD,
+	NL80211_ATTR_STA_MON_RSSI_HYST,
+	NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT,
+	NL80211_ATTR_STA_MON_RSSI_LEVEL,
+
+	/* keep last */
+	__NL80211_ATTR_STA_MON_AFTER_LAST,
+	NL80211_ATTR_STA_MON_MAX = __NL80211_ATTR_STA_MON_AFTER_LAST - 1,
+};
+
+/**
+ * enum nl80211_sta_mon_rssi_threshold_event - RSSI threshold event
+ * @NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW: The RSSI level is lower than the
+ *	configured threshold
+ * @NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH: The RSSI is higher than the
+ *	configured threshold
+ */
+enum nl80211_sta_mon_rssi_threshold_event {
+	NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW,
+	NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH,
+};
+
+/**
  * enum nl80211_attr_cqm - connection quality monitor attributes
  * @__NL80211_ATTR_CQM_INVALID: invalid
  * @NL80211_ATTR_CQM_RSSI_THOLD: RSSI threshold in dBm. This value specifies
@@ -5134,6 +5184,10 @@ enum nl80211_feature_flags {
  * @NL80211_EXT_FEATURE_TXQS: Driver supports FQ-CoDel-enabled intermediate
  *      TXQs.
  *
+ * @NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG: With this driver can set
+ *	rssi threshold using %NL80211_ATTR_STA_MON_RSSI_THOLD attribute
+ *	for a connected station.
+ *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
  */
@@ -5167,6 +5221,7 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211,
 	NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT,
 	NL80211_EXT_FEATURE_TXQS,
+	NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2fb2e97..587a5cb1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -428,6 +428,7 @@ enum nl80211_multicast_groups {
 	[NL80211_ATTR_TXQ_LIMIT] = { .type = NLA_U32 },
 	[NL80211_ATTR_TXQ_MEMORY_LIMIT] = { .type = NLA_U32 },
 	[NL80211_ATTR_TXQ_QUANTUM] = { .type = NLA_U32 },
+	[NL80211_ATTR_STA_MON] = { .type = NLA_NESTED },
 };
 
 /* policy for the key attributes */
@@ -10131,6 +10132,14 @@ static int nl80211_set_cqm_txe(struct genl_info *info,
 	return rdev_set_cqm_txe_config(rdev, dev, rate, pkts, intvl);
 }
 
+static const struct nla_policy
+nl80211_attr_sta_mon_policy[NL80211_ATTR_STA_MON_MAX + 1] = {
+	[NL80211_ATTR_STA_MON_RSSI_THOLD] = { .type = NLA_U32 },
+	[NL80211_ATTR_STA_MON_RSSI_HYST] = { .type = NLA_U32 },
+	[NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT] = { .type = NLA_U32 },
+	[NL80211_ATTR_STA_MON_RSSI_LEVEL] = { .type = NLA_S32 },
+};
+
 static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 				    struct net_device *dev)
 {
@@ -12912,6 +12921,66 @@ static int nl80211_tx_control_port(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+static int nl80211_set_sta_mon_rssi(struct genl_info *info,
+				    const u8 *peer, s32 threshold,
+				    u32 hysteresis)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+
+	if (threshold > 0)
+		return -EINVAL;
+
+	if (threshold == 0)
+		hysteresis = 0;
+
+	if (!rdev->ops->set_sta_mon_rssi_config)
+		return -EOPNOTSUPP;
+
+	if ((wdev->iftype != NL80211_IFTYPE_AP &&
+	     wdev->iftype != NL80211_IFTYPE_P2P_GO &&
+	     wdev->iftype != NL80211_IFTYPE_AP_VLAN) ||
+	    !wiphy_ext_feature_isset(&rdev->wiphy,
+				NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG))
+		return -EOPNOTSUPP;
+
+	return rdev_set_sta_mon_rssi_config(rdev, dev, peer,
+					    threshold, hysteresis);
+}
+
+static int nl80211_sta_mon(struct sk_buff *skb, struct genl_info *info)
+{
+	struct nlattr *attrs[NL80211_ATTR_STA_MON_MAX + 1];
+	struct nlattr *sta_mon;
+	u8 *addr = NULL;
+	int err;
+
+	sta_mon = info->attrs[NL80211_ATTR_STA_MON];
+	if (!sta_mon || !(info->attrs[NL80211_ATTR_MAC]))
+		return -EINVAL;
+
+	err = nla_parse_nested(attrs, NL80211_ATTR_STA_MON_MAX,
+			       sta_mon, nl80211_attr_sta_mon_policy,
+			       info->extack);
+
+	if (err)
+		return err;
+
+	addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
+	if (attrs[NL80211_ATTR_STA_MON_RSSI_THOLD] &&
+	    attrs[NL80211_ATTR_STA_MON_RSSI_HYST]) {
+		s32 threshold =
+			nla_get_s32(attrs[NL80211_ATTR_STA_MON_RSSI_THOLD]);
+		u32 hysteresis =
+			nla_get_u32(attrs[NL80211_ATTR_STA_MON_RSSI_HYST]);
+
+		return nl80211_set_sta_mon_rssi(info, addr, threshold,
+						hysteresis);
+	}
+	return -EINVAL;
+}
+
 #define NL80211_FLAG_NEED_WIPHY		0x01
 #define NL80211_FLAG_NEED_NETDEV	0x02
 #define NL80211_FLAG_NEED_RTNL		0x04
@@ -13823,6 +13892,14 @@ static void nl80211_post_doit(const struct genl_ops *ops, struct sk_buff *skb,
 		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
 				  NL80211_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL80211_CMD_SET_STA_MON,
+		.doit = nl80211_sta_mon,
+		.policy = nl80211_policy,
+		.flags = GENL_UNS_ADMIN_PERM,
+		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+				  NL80211_FLAG_NEED_RTNL,
+	},
 };
 
 static struct genl_family nl80211_fam __ro_after_init = {
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 364f5d6..faccfae 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1232,4 +1232,20 @@ static inline int rdev_del_pmk(struct cfg80211_registered_device *rdev,
 	return ret;
 }
 
+static inline int
+rdev_set_sta_mon_rssi_config(struct cfg80211_registered_device *rdev,
+			     struct net_device *dev, const u8 *peer,
+			     s32 rssi_thold, u32 rssi_hyst)
+
+{
+	int ret;
+
+	trace_rdev_set_sta_mon_rssi_config(&rdev->wiphy, dev, peer,
+					   rssi_thold, rssi_hyst);
+	ret = rdev->ops->set_sta_mon_rssi_config(&rdev->wiphy, dev, peer,
+						 rssi_thold, rssi_hyst);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
+
 #endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 2b417a2..361ec08 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3257,6 +3257,31 @@
 	),
 	TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT, WIPHY_PR_ARG, WDEV_PR_ARG)
 );
+
+TRACE_EVENT(rdev_set_sta_mon_rssi_config,
+	TP_PROTO(struct wiphy *wiphy,
+		 struct net_device *netdev, const u8 *peer,
+		 s32 rssi_thold, u32 rssi_hyst),
+	TP_ARGS(wiphy, netdev, peer, rssi_thold, rssi_hyst),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		NETDEV_ENTRY
+		MAC_ENTRY(peer)
+		__field(s32, rssi_thold)
+		__field(u32, rssi_hyst)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		NETDEV_ASSIGN;
+		MAC_ASSIGN(peer, peer);
+		__entry->rssi_thold = rssi_thold;
+		__entry->rssi_hyst = rssi_hyst;
+	),
+	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", " MAC_PR_FMT
+		  ", rssi_thold: %d, rssi_hyst: %u ",
+		  WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(peer),
+		  __entry->rssi_thold, __entry->rssi_hyst)
+);
 #endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
1.7.9.5

Re: [PATCH 2/7] cfg80211: Add new NL command to configure peer specific rssi threshold

[Johannes Berg]

On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
> 
> + * @NL80211_ATTR_STA_MON: Station's connection monitor configuration in a
> + *	nested attribute with %NL80211_ATTR_STA_MON_* sub-attributes.

Can't we reuse the existing attributes in this nesting?

> +/**
> + * enum nl80211_sta_mon_rssi_threshold_event - RSSI threshold event
> + * @NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW: The RSSI level is lower than the
> + *	configured threshold
> + * @NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH: The RSSI is higher than the
> + *	configured threshold
> + */
> +enum nl80211_sta_mon_rssi_threshold_event {
> +	NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW,
> +	NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH,
> +};

It seems like this should already exist also for the CQM? Can we not
reuse it?

> +static int nl80211_set_sta_mon_rssi(struct genl_info *info,
> +				    const u8 *peer, s32 threshold,
> +				    u32 hysteresis)
> +{
> +	struct cfg80211_registered_device *rdev = info->user_ptr[0];
> +	struct net_device *dev = info->user_ptr[1];
> +	struct wireless_dev *wdev = dev->ieee80211_ptr;
> +
> +	if (threshold > 0)
> +		return -EINVAL;
> +
> +	if (threshold == 0)
> +		hysteresis = 0;
> +
> +	if (!rdev->ops->set_sta_mon_rssi_config)
> +		return -EOPNOTSUPP;
> +
> +	if ((wdev->iftype != NL80211_IFTYPE_AP &&
> +	     wdev->iftype != NL80211_IFTYPE_P2P_GO &&
> +	     wdev->iftype != NL80211_IFTYPE_AP_VLAN) ||
> +	    !wiphy_ext_feature_isset(&rdev->wiphy,
> +				NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG))
> +		return -EOPNOTSUPP;
> +
> +	return rdev_set_sta_mon_rssi_config(rdev, dev, peer,
> +					    threshold, hysteresis);
> +}

Wait, you went to all this effort in the first patch and now you don't
add this to the list?

johannes

Re: [PATCH 2/7] cfg80211: Add new NL command to configure peer specific rssi threshold

[Tamizh chelvam]

On 2018-06-29 14:59, Johannes Berg wrote:
> On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
>> 
>> + * @NL80211_ATTR_STA_MON: Station's connection monitor configuration 
>> in a
>> + *	nested attribute with %NL80211_ATTR_STA_MON_* sub-attributes.
> 
> Can't we reuse the existing attributes in this nesting?
This new attributes introduced as per the previous discussion.
https://patchwork.kernel.org/patch/10240697/
Is my understanding correct ?
> 
>> +/**
>> + * enum nl80211_sta_mon_rssi_threshold_event - RSSI threshold event
>> + * @NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW: The RSSI level is lower 
>> than the
>> + *	configured threshold
>> + * @NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH: The RSSI is higher 
>> than the
>> + *	configured threshold
>> + */
>> +enum nl80211_sta_mon_rssi_threshold_event {
>> +	NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW,
>> +	NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH,
>> +};
> 
> It seems like this should already exist also for the CQM? Can we not
> reuse it?
> 
>> +static int nl80211_set_sta_mon_rssi(struct genl_info *info,
>> +				    const u8 *peer, s32 threshold,
>> +				    u32 hysteresis)
>> +{
>> +	struct cfg80211_registered_device *rdev = info->user_ptr[0];
>> +	struct net_device *dev = info->user_ptr[1];
>> +	struct wireless_dev *wdev = dev->ieee80211_ptr;
>> +
>> +	if (threshold > 0)
>> +		return -EINVAL;
>> +
>> +	if (threshold == 0)
>> +		hysteresis = 0;
>> +
>> +	if (!rdev->ops->set_sta_mon_rssi_config)
>> +		return -EOPNOTSUPP;
>> +
>> +	if ((wdev->iftype != NL80211_IFTYPE_AP &&
>> +	     wdev->iftype != NL80211_IFTYPE_P2P_GO &&
>> +	     wdev->iftype != NL80211_IFTYPE_AP_VLAN) ||
>> +	    !wiphy_ext_feature_isset(&rdev->wiphy,
>> +				NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG))
>> +		return -EOPNOTSUPP;
>> +
>> +	return rdev_set_sta_mon_rssi_config(rdev, dev, peer,
>> +					    threshold, hysteresis);
>> +}
> 
> Wait, you went to all this effort in the first patch and now you don't
> add this to the list?
> 
No, the list and the rssi_config structure is for multi threshold value 
and not for single threshold.

Thanks,
Tamizh.

Re: [PATCH 2/7] cfg80211: Add new NL command to configure peer specific rssi threshold

[Johannes Berg]

On Wed, 2018-07-04 at 11:39 +0530, Tamizh chelvam wrote:
> On 2018-06-29 14:59, Johannes Berg wrote:
> > On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
> > > 
> > > + * @NL80211_ATTR_STA_MON: Station's connection monitor configuration 
> > > in a
> > > + *	nested attribute with %NL80211_ATTR_STA_MON_* sub-attributes.
> > 
> > Can't we reuse the existing attributes in this nesting?
> 
> This new attributes introduced as per the previous discussion.
> https://patchwork.kernel.org/patch/10240697/
> Is my understanding correct ?

Yeah ok ... I'm thinking perhaps we could share some part of it, but
it's not really worth making it more complex due to that.

> > Wait, you went to all this effort in the first patch and now you don't
> > add this to the list?
> > 
> No, the list and the rssi_config structure is for multi threshold value 
> and not for single threshold.

Yeah, though I'm still not sure why we need the single threshold thing
at all. List should always be a superset.

johannes

Re: [PATCH 2/7] cfg80211: Add new NL command to configure peer specific rssi threshold

[Tamizh chelvam]

On 2018-07-06 17:10, Johannes Berg wrote:
> On Wed, 2018-07-04 at 11:39 +0530, Tamizh chelvam wrote:
>> On 2018-06-29 14:59, Johannes Berg wrote:
>> > On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
>> > >
>> > > + * @NL80211_ATTR_STA_MON: Station's connection monitor configuration
>> > > in a
>> > > + *	nested attribute with %NL80211_ATTR_STA_MON_* sub-attributes.
>> >
>> > Can't we reuse the existing attributes in this nesting?
>> 
>> This new attributes introduced as per the previous discussion.
>> https://patchwork.kernel.org/patch/10240697/
>> Is my understanding correct ?
> 
> Yeah ok ... I'm thinking perhaps we could share some part of it, but
> it's not really worth making it more complex due to that.
> 
Sure, i'll reuse those cqm attributes and event notification enum as 
well.

>> > Wait, you went to all this effort in the first patch and now you don't
>> > add this to the list?
>> >
>> No, the list and the rssi_config structure is for multi threshold 
>> value
>> and not for single threshold.
> 
> Yeah, though I'm still not sure why we need the single threshold thing
> at all.
For taking steering kind of decision single threshold value can be used 
to monitor
the station's rssi.

> List should always be a superset.
> 
I believe superset you meant will accept a signle threshold 
configuration also from user ?
If so, we can remove the single threshold api implementation.

Thanks,
Tamizh.

Re: [PATCH 2/7] cfg80211: Add new NL command to configure peer specific rssi threshold

[Johannes Berg]

On Wed, 2018-07-11 at 12:34 +0530, Tamizh chelvam wrote:

> I believe superset you meant will accept a signle threshold 
> configuration also from user ?

Yes.

> If so, we can remove the single threshold api implementation.

I think here we can remove the single threshold API - we have it in the
old only because some drivers still implement only a single threshold
(AFAICT), but for a new API I think we can reasonably require multiple
to be implemented.

johannes

[PATCH 3/7] mac80211: Add api to support configuring station specific rssi threshold

[Tamizh chelvam]

This patch add set_sta_mon_rssi_config api to configure rssi and
hysteresis threshold value for a connected station. This configuration
will be applied only for the connected station.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 net/mac80211/cfg.c      |   35 +++++++++++++++++++++++++++++++++++
 net/mac80211/sta_info.h |   12 ++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index bdf6fa7..80ced50 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3803,6 +3803,40 @@ static int ieee80211_get_txq_stats(struct wiphy *wiphy,
 	return ret;
 }
 
+static int ieee80211_set_sta_mon_rssi_config(struct wiphy *wiphy,
+					     struct net_device *dev,
+					     const u8 *peer, s32 rssi_thold,
+					     u32 rssi_hyst)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct sta_info *sta;
+
+	if (sdata->vif.type == NL80211_IFTYPE_AP &&
+	    (!sdata->vif.bss_conf.enable_beacon ||
+	     !wiphy_ext_feature_isset(sdata->local->hw.wiphy,
+				 NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG)))
+		return -EOPNOTSUPP;
+
+	mutex_lock(&sdata->local->sta_mtx);
+
+	sta = sta_info_get_bss(sdata, peer);
+	if (!sta) {
+		mutex_unlock(&sdata->local->sta_mtx);
+		return -ENOENT;
+	}
+
+	if (sta->rssi_thold == rssi_thold &&
+	    sta->rssi_hyst == rssi_hyst)
+		goto unlock;
+
+	sta->rssi_thold = rssi_thold;
+	sta->rssi_hyst = rssi_hyst;
+	sta->last_sta_mon_event_signal = 0;
+unlock:
+	mutex_unlock(&sdata->local->sta_mtx);
+	return 0;
+}
+
 const struct cfg80211_ops mac80211_config_ops = {
 	.add_virtual_intf = ieee80211_add_iface,
 	.del_virtual_intf = ieee80211_del_iface,
@@ -3897,4 +3931,5 @@ static int ieee80211_get_txq_stats(struct wiphy *wiphy,
 	.set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
 	.tx_control_port = ieee80211_tx_control_port,
 	.get_txq_stats = ieee80211_get_txq_stats,
+	.set_sta_mon_rssi_config = ieee80211_set_sta_mon_rssi_config,
 };
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 81b35f6..8c459a7 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -482,6 +482,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
+ * @rssi_thold: RSSI threshold to monitor station's signal strength, a zero
+ *	value implies disabled. As with the cfg80211 callback, a change here
+ *	should cause an event to be sent indicating where the current value
+ *	is in relation to the newly configured threshold
+ * @rssi_hyst: Station's RSSI hysteresis
+ * @last_sta_mon_event_signal: Last signal strength average for a station
+ *	that triggered a sta_mon event. 0 indicates that no event has been
+ *	generated for the current association
  */
 struct sta_info {
 	/* General information, mostly static */
@@ -583,6 +591,10 @@ struct sta_info {
 
 	struct cfg80211_chan_def tdls_chandef;
 
+	s32 rssi_thold;
+	u32 rssi_hyst;
+	int last_sta_mon_event_signal;
+
 	/* keep last! */
 	struct ieee80211_sta sta;
 };
-- 
1.7.9.5

Re: [PATCH 3/7] mac80211: Add api to support configuring station specific rssi threshold

[Johannes Berg]

The subjects are a bit confusing - I think you should align the cfg80211
and mac80211 ones so it's clearer that this one is implementing the
previous one's API. Peer vs. station, and mac80211 doesn't add API but
implements it.

This patch doesn't really do anything though, so I guess you need to
combine it with patch 5.

johannes

Re: [PATCH 3/7] mac80211: Add api to support configuring station specific rssi threshold

[Tamizh chelvam]

On 2018-06-29 15:01, Johannes Berg wrote:
> The subjects are a bit confusing - I think you should align the 
> cfg80211
> and mac80211 ones so it's clearer that this one is implementing the
> previous one's API. Peer vs. station, and mac80211 doesn't add API but
> implements it.
> 
> This patch doesn't really do anything though, so I guess you need to
> combine it with patch 5.
> 
Okay sure

[PATCH 4/7] cfg80211: Add support to notify station's rssi level crossing

[Tamizh chelvam]

Add cfg80211_sta_mon_rssi_notify api to update user space upon
crossing the configured rssi threshold of a station.
NL80211_CMD_NOTIFY_STA_MON introduced to send this event to
userspace along with NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT,
NL80211_ATTR_MAC and NL80211_ATTR_STA_MON_RSSI_LEVEL info.
Userspace application can make a decision depends on this
notification.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 include/net/cfg80211.h       |   16 +++++++
 include/uapi/linux/nl80211.h |    1 +
 net/wireless/nl80211.c       |   98 ++++++++++++++++++++++++++++++++++++++++++
 net/wireless/trace.h         |   22 ++++++++++
 4 files changed, 137 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 584697b..7dcf9b9 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5916,6 +5916,22 @@ void cfg80211_cqm_txe_notify(struct net_device *dev, const u8 *peer,
 void cfg80211_cqm_beacon_loss_notify(struct net_device *dev, gfp_t gfp);
 
 /**
+ * cfg80211_sta_mon_rssi_notify - Station's rssi out of range event
+ * @dev: network device
+ * @peer: Station's mac address
+ * @rssi_event: the triggered RSSI event
+ * @rssi_level: new RSSI level value or 0 if not available
+ * @gfp: context flags
+ *
+ * This function is called when a configured rssi threshold reached event
+ * occurs for a station.
+ */
+void
+cfg80211_sta_mon_rssi_notify(struct net_device *dev, const u8 *peer,
+		enum nl80211_sta_mon_rssi_threshold_event rssi_event,
+		s32 rssi_level, gfp_t gfp);
+
+/**
  * cfg80211_radar_event - radar detection event
  * @wiphy: the wiphy
  * @chandef: chandef for the current channel
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f13ad07..9d47ee6 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1249,6 +1249,7 @@ enum nl80211_commands {
 	NL80211_CMD_CONTROL_PORT_FRAME,
 
 	NL80211_CMD_SET_STA_MON,
+	NL80211_CMD_NOTIFY_STA_MON,
 
 	/* add new commands above here */
 
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 587a5cb1..021e55a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -15364,6 +15364,104 @@ void cfg80211_pmksa_candidate_notify(struct net_device *dev, int index,
 }
 EXPORT_SYMBOL(cfg80211_pmksa_candidate_notify);
 
+static struct sk_buff *cfg80211_prepare_sta_mon(struct net_device *dev,
+						const char *mac, gfp_t gfp)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct sk_buff *msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp);
+	void **cb;
+
+	if (!msg)
+		return NULL;
+
+	cb = (void **)msg->cb;
+
+	cb[0] = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_NOTIFY_STA_MON);
+	if (!cb[0]) {
+		nlmsg_free(msg);
+		return NULL;
+	}
+
+	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
+	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, dev->ifindex))
+		goto nla_put_failure;
+
+	if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, mac))
+		goto nla_put_failure;
+
+	cb[1] = nla_nest_start(msg, NL80211_ATTR_STA_MON);
+	if (!cb[1])
+		goto nla_put_failure;
+
+	cb[2] = rdev;
+
+	return msg;
+nla_put_failure:
+	nlmsg_free(msg);
+	return NULL;
+}
+
+static void cfg80211_send_sta_mon(struct sk_buff *msg, gfp_t gfp)
+{
+	void **cb = (void **)msg->cb;
+	struct cfg80211_registered_device *rdev = cb[2];
+
+	nla_nest_end(msg, cb[1]);
+	genlmsg_end(msg, cb[0]);
+
+	memset(msg->cb, 0, sizeof(msg->cb));
+
+	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
+				NL80211_MCGRP_MLME, gfp);
+}
+
+void cfg80211_sta_mon_rssi_notify(struct net_device *dev, const u8 *peer,
+		enum nl80211_sta_mon_rssi_threshold_event rssi_event,
+		s32 rssi_level, gfp_t gfp)
+{
+	struct sk_buff *msg;
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct cfg80211_rssi_config *rssi_config;
+
+	if (WARN_ON(!peer))
+		return;
+
+	if (WARN_ON(rssi_event != NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW &&
+		    rssi_event != NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH))
+		return;
+
+	trace_cfg80211_sta_mon_rssi_notify(dev, peer, rssi_event, rssi_level);
+
+	list_for_each_entry(rssi_config, &wdev->rssi_config_list, list) {
+		if (!memcmp(rssi_config->addr, peer, ETH_ALEN)) {
+			wdev->rssi_config = rssi_config;
+			wdev->rssi_config->last_rssi_event_value = rssi_level;
+			break;
+		}
+	}
+
+	msg = cfg80211_prepare_sta_mon(dev, peer, gfp);
+	if (!msg)
+		return;
+
+	if (nla_put_u32(msg, NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT,
+			rssi_event))
+		goto nla_put_failure;
+
+	if (rssi_level && nla_put_s32(msg, NL80211_ATTR_STA_MON_RSSI_LEVEL,
+				      rssi_level))
+		goto nla_put_failure;
+
+	cfg80211_send_sta_mon(msg, gfp);
+
+	return;
+
+nla_put_failure:
+	nlmsg_free(msg);
+}
+EXPORT_SYMBOL(cfg80211_sta_mon_rssi_notify);
+
 static void nl80211_ch_switch_notify(struct cfg80211_registered_device *rdev,
 				     struct net_device *netdev,
 				     struct cfg80211_chan_def *chandef,
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 361ec08..5454c57 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3282,6 +3282,28 @@
 		  WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(peer),
 		  __entry->rssi_thold, __entry->rssi_hyst)
 );
+TRACE_EVENT(cfg80211_sta_mon_rssi_notify,
+	TP_PROTO(struct net_device *netdev, const u8 *peer,
+		 enum nl80211_sta_mon_rssi_threshold_event rssi_event,
+		 s32 rssi_level),
+	TP_ARGS(netdev, peer, rssi_event, rssi_level),
+	TP_STRUCT__entry(
+		NETDEV_ENTRY
+		MAC_ENTRY(peer)
+		__field(enum nl80211_sta_mon_rssi_threshold_event, rssi_event)
+		__field(s32, rssi_level)
+	),
+	TP_fast_assign(
+		NETDEV_ASSIGN;
+		MAC_ASSIGN(peer, peer);
+		__entry->rssi_event = rssi_event;
+		__entry->rssi_level = rssi_level;
+	),
+	TP_printk(NETDEV_PR_FMT ", peer: " MAC_PR_FMT
+		  ", rssi event: %d, rssi : %d",
+		  NETDEV_PR_ARG, MAC_PR_ARG(peer),
+		  __entry->rssi_event, __entry->rssi_level)
+);
 #endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
1.7.9.5

Re: [PATCH 4/7] cfg80211: Add support to notify station's rssi level crossing

[Johannes Berg]

On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
> Add cfg80211_sta_mon_rssi_notify api to update user space upon
> crossing the configured rssi threshold of a station.
> NL80211_CMD_NOTIFY_STA_MON introduced to send this event to
> userspace along with NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT,
> NL80211_ATTR_MAC and NL80211_ATTR_STA_MON_RSSI_LEVEL info.
> Userspace application can make a decision depends on this
> notification.

I guess you should also combine this with patch 2, it's a bit weird to
only have one side and not be able to use it in one patch, and both
aren't all that big.


> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1249,6 +1249,7 @@ enum nl80211_commands {
>  	NL80211_CMD_CONTROL_PORT_FRAME,
>  
>  	NL80211_CMD_SET_STA_MON,
> +	NL80211_CMD_NOTIFY_STA_MON,

Missing documentation - but again, why not unify it with the existing
event? Or can't we because that might confuse older applications?
Perhaps anyway we should unicast these notifications? Not sure though,
perhaps one app could set them up and the other might care?

> +static struct sk_buff *cfg80211_prepare_sta_mon(struct net_device *dev,
> +						const char *mac, gfp_t gfp)
> +{
> +	struct wireless_dev *wdev = dev->ieee80211_ptr;
> +	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
> +	struct sk_buff *msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp);
> +	void **cb;
> +
> +	if (!msg)
> +		return NULL;
> +
> +	cb = (void **)msg->cb;

Uh, what's that? Please use a structure.

> +	cb[0] = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_NOTIFY_STA_MON);
> 
[...]
> +
> +	cb[1] = nla_nest_start(msg, NL80211_ATTR_STA_MON);
> +	if (!cb[1])
> +		goto nla_put_failure;
> +
> +	cb[2] = rdev;

Yeah, definitely use a struct instead of three magic array indices.

> +
> +	msg = cfg80211_prepare_sta_mon(dev, peer, gfp);
> +	if (!msg)
> +		return;
> +
> +	if (nla_put_u32(msg, NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT,
> +			rssi_event))
> +		goto nla_put_failure;
> +
> +	if (rssi_level && nla_put_s32(msg, NL80211_ATTR_STA_MON_RSSI_LEVEL,
> +				      rssi_level))
> +		goto nla_put_failure;
> +
> +	cfg80211_send_sta_mon(msg, gfp);

In fact, perhaps better to have something like

struct nl80211_sta_mon_prep {
	struct sk_buff *skb;
	....
};

struct nl80211_sta_mon_prep prep;

if (cfg80211_prepare_sta_mon(&prep))
	return;
...
cfg80211_send_sta_mon(&prep, gfp);

or so?

johannes

Re: [PATCH 4/7] cfg80211: Add support to notify station's rssi level crossing

[Tamizh chelvam]

On 2018-06-29 15:05, Johannes Berg wrote:
> On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
>> Add cfg80211_sta_mon_rssi_notify api to update user space upon
>> crossing the configured rssi threshold of a station.
>> NL80211_CMD_NOTIFY_STA_MON introduced to send this event to
>> userspace along with NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT,
>> NL80211_ATTR_MAC and NL80211_ATTR_STA_MON_RSSI_LEVEL info.
>> Userspace application can make a decision depends on this
>> notification.
> 
> I guess you should also combine this with patch 2, it's a bit weird to
> only have one side and not be able to use it in one patch, and both
> aren't all that big.
Sure
> 
> 
>> --- a/include/uapi/linux/nl80211.h
>> +++ b/include/uapi/linux/nl80211.h
>> @@ -1249,6 +1249,7 @@ enum nl80211_commands {
>>  	NL80211_CMD_CONTROL_PORT_FRAME,
>> 
>>  	NL80211_CMD_SET_STA_MON,
>> +	NL80211_CMD_NOTIFY_STA_MON,
> 
> Missing documentation - but again, why not unify it with the existing
> event? Or can't we because that might confuse older applications?
> Perhaps anyway we should unicast these notifications? Not sure though,
> perhaps one app could set them up and the other might care?
> 
This new command introduced as per the previous discussion
https://patchwork.kernel.org/patch/10168685/
https://patchwork.kernel.org/patch/10240697/

Is my understanding correct ?

>> +static struct sk_buff *cfg80211_prepare_sta_mon(struct net_device 
>> *dev,
>> +						const char *mac, gfp_t gfp)
>> +{
>> +	struct wireless_dev *wdev = dev->ieee80211_ptr;
>> +	struct cfg80211_registered_device *rdev = 
>> wiphy_to_rdev(wdev->wiphy);
>> +	struct sk_buff *msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp);
>> +	void **cb;
>> +
>> +	if (!msg)
>> +		return NULL;
>> +
>> +	cb = (void **)msg->cb;
> 
> Uh, what's that? Please use a structure.

I just referred this code from cfg80211_prepare_cqm. Do you want me to 
change this to struct based?
> 
>> +	cb[0] = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_NOTIFY_STA_MON);
>> 
> [...]
>> +
>> +	cb[1] = nla_nest_start(msg, NL80211_ATTR_STA_MON);
>> +	if (!cb[1])
>> +		goto nla_put_failure;
>> +
>> +	cb[2] = rdev;
> 
> Yeah, definitely use a struct instead of three magic array indices.
> 
>> +
>> +	msg = cfg80211_prepare_sta_mon(dev, peer, gfp);
>> +	if (!msg)
>> +		return;
>> +
>> +	if (nla_put_u32(msg, NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT,
>> +			rssi_event))
>> +		goto nla_put_failure;
>> +
>> +	if (rssi_level && nla_put_s32(msg, NL80211_ATTR_STA_MON_RSSI_LEVEL,
>> +				      rssi_level))
>> +		goto nla_put_failure;
>> +
>> +	cfg80211_send_sta_mon(msg, gfp);
> 
> In fact, perhaps better to have something like
> 
> struct nl80211_sta_mon_prep {
> 	struct sk_buff *skb;
> 	....
> };
> 
> struct nl80211_sta_mon_prep prep;
> 
> if (cfg80211_prepare_sta_mon(&prep))
> 	return;
> ...
> cfg80211_send_sta_mon(&prep, gfp);
> 
> or so?

Tamizh.

Re: [PATCH 4/7] cfg80211: Add support to notify station's rssi level crossing

[Johannes Berg]

On Wed, 2018-07-04 at 11:43 +0530, Tamizh chelvam wrote:

> > > --- a/include/uapi/linux/nl80211.h
> > > +++ b/include/uapi/linux/nl80211.h
> > > @@ -1249,6 +1249,7 @@ enum nl80211_commands {
> > >  	NL80211_CMD_CONTROL_PORT_FRAME,
> > > 
> > >  	NL80211_CMD_SET_STA_MON,
> > > +	NL80211_CMD_NOTIFY_STA_MON,
> > 
> > Missing documentation - but again, why not unify it with the existing
> > event? Or can't we because that might confuse older applications?
> > Perhaps anyway we should unicast these notifications? Not sure though,
> > perhaps one app could set them up and the other might care?
> > 
> 
> This new command introduced as per the previous discussion
> https://patchwork.kernel.org/patch/10168685/
> https://patchwork.kernel.org/patch/10240697/
> 
> Is my understanding correct ?

I guess you're right. To my defense, this was like almost half a year
ago :)

> I just referred this code from cfg80211_prepare_cqm. Do you want me to 
> change this to struct based?

Please, I guess we can fix up cqm too later.

johannes

Re: [PATCH 4/7] cfg80211: Add support to notify station's rssi level crossing

[Tamizh chelvam]

On 2018-07-06 17:09, Johannes Berg wrote:
> On Wed, 2018-07-04 at 11:43 +0530, Tamizh chelvam wrote:
> 
>> > > --- a/include/uapi/linux/nl80211.h
>> > > +++ b/include/uapi/linux/nl80211.h
>> > > @@ -1249,6 +1249,7 @@ enum nl80211_commands {
>> > >  	NL80211_CMD_CONTROL_PORT_FRAME,
>> > >
>> > >  	NL80211_CMD_SET_STA_MON,
>> > > +	NL80211_CMD_NOTIFY_STA_MON,
>> >
>> > Missing documentation - but again, why not unify it with the existing
>> > event? Or can't we because that might confuse older applications?
>> > Perhaps anyway we should unicast these notifications? Not sure though,
>> > perhaps one app could set them up and the other might care?
>> >
>> 
>> This new command introduced as per the previous discussion
>> https://patchwork.kernel.org/patch/10168685/
>> https://patchwork.kernel.org/patch/10240697/
>> 
>> Is my understanding correct ?
> 
> I guess you're right. To my defense, this was like almost half a year
> ago :)
Yeah;)
> 
>> I just referred this code from cfg80211_prepare_cqm. Do you want me to
>> change this to struct based?
> 
> Please, I guess we can fix up cqm too later.
> 
Sure, I'll fix it in the next version.

Thanks,
Tamizh.

[PATCH 5/7] mac80211: Implement functionality to monitor station's rssi cross event

[Tamizh chelvam]

Triggers cfg80211_sta_mon_rssi_notify with the corresponding event when
station signal goes out of configured threshold. It uses rx data signal
to check against rssi threshold configured by the user.
This event will be useful for the application like steering to take
decision on any station depends on its current capability.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 include/net/mac80211.h  |    6 ++++++
 net/mac80211/rx.c       |   42 +++++++++++++++++++++++++++++++++++++++++-
 net/mac80211/sta_info.h |    4 ++++
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 851a5e1..96fb9a8 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -802,6 +802,12 @@ enum mac80211_rate_control_flags {
 	IEEE80211_TX_RC_160_MHZ_WIDTH		= BIT(10),
 };
 
+/*
+ * How many frames need to have been used in average station's
+ * signal strength before checking against the threshold
+ */
+#define IEEE80211_STA_SIGNAL_AVE_MIN_COUNT	4
+
 
 /* there are 40 bytes if you don't need the rateset to be kept */
 #define IEEE80211_TX_INFO_DRIVER_DATA_SIZE 40
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 0a38cc1..06a6111 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1534,6 +1534,43 @@ void ieee80211_sta_uapsd_trigger(struct ieee80211_sta *pubsta, u8 tid)
 	return RX_CONTINUE;
 }
 
+static void ieee80211_sta_rx_signal_thold_check(struct ieee80211_rx_data *rx)
+{
+	struct sta_info *sta = rx->sta;
+	struct ieee80211_bss_conf *bss_conf = &rx->sdata->vif.bss_conf;
+
+	if (!wiphy_ext_feature_isset(rx->local->hw.wiphy,
+				NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG))
+		return;
+
+	sta->count_rx_signal++;
+	if (sta->count_rx_signal < IEEE80211_STA_SIGNAL_AVE_MIN_COUNT)
+		return;
+
+	if (sta->rssi_thold && bss_conf->enable_beacon) {
+		int last_event = sta->last_sta_mon_event_signal;
+		int thold = sta->rssi_thold;
+		int hyst = sta->rssi_hyst;
+		int sig = -ewma_signal_read(&sta->rx_stats_avg.signal);
+
+		if (sig < thold &&
+		    (last_event == 0 || sig < last_event - hyst)) {
+			sta->last_sta_mon_event_signal = sig;
+			cfg80211_sta_mon_rssi_notify(
+				rx->sdata->dev, sta->addr,
+				NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW,
+				sig, GFP_ATOMIC);
+		} else if (sig > thold &&
+			   (last_event == 0 || sig > last_event + hyst)) {
+			sta->last_sta_mon_event_signal = sig;
+			cfg80211_sta_mon_rssi_notify(
+				rx->sdata->dev, sta->addr,
+				NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH,
+				sig, GFP_ATOMIC);
+		}
+	}
+}
+
 static ieee80211_rx_result debug_noinline
 ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 {
@@ -1589,6 +1626,7 @@ void ieee80211_sta_uapsd_trigger(struct ieee80211_sta *pubsta, u8 tid)
 	if (!(status->flag & RX_FLAG_NO_SIGNAL_VAL)) {
 		sta->rx_stats.last_signal = status->signal;
 		ewma_signal_add(&sta->rx_stats_avg.signal, -status->signal);
+		ieee80211_sta_rx_signal_thold_check(rx);
 	}
 
 	if (status->chains) {
@@ -4030,9 +4068,11 @@ static bool ieee80211_invoke_fast_rx(struct ieee80211_rx_data *rx,
 	/* statistics part of ieee80211_rx_h_sta_process() */
 	if (!(status->flag & RX_FLAG_NO_SIGNAL_VAL)) {
 		stats->last_signal = status->signal;
-		if (!fast_rx->uses_rss)
+		if (!fast_rx->uses_rss) {
 			ewma_signal_add(&sta->rx_stats_avg.signal,
 					-status->signal);
+			ieee80211_sta_rx_signal_thold_check(rx);
+		}
 	}
 
 	if (status->chains) {
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 8c459a7..e130f6d 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -490,6 +490,9 @@ struct ieee80211_sta_rx_stats {
  * @last_sta_mon_event_signal: Last signal strength average for a station
  *	that triggered a sta_mon event. 0 indicates that no event has been
  *	generated for the current association
+ * @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 */
@@ -594,6 +597,7 @@ struct sta_info {
 	s32 rssi_thold;
 	u32 rssi_hyst;
 	int last_sta_mon_event_signal;
+	unsigned int count_rx_signal;
 
 	/* keep last! */
 	struct ieee80211_sta sta;
-- 
1.7.9.5

Re: [PATCH 5/7] mac80211: Implement functionality to monitor station's rssi cross event

[Johannes Berg]

On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
> 
> +	if (sta->rssi_thold && bss_conf->enable_beacon) {
> +		int last_event = sta->last_sta_mon_event_signal;
> +		int thold = sta->rssi_thold;
> +		int hyst = sta->rssi_hyst;
> +		int sig = -ewma_signal_read(&sta->rx_stats_avg.signal);
> +
> +		if (sig < thold &&
> +		    (last_event == 0 || sig < last_event - hyst)) {
> +			sta->last_sta_mon_event_signal = sig;
> +			cfg80211_sta_mon_rssi_notify(
> +				rx->sdata->dev, sta->addr,
> +				NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW,
> +				sig, GFP_ATOMIC);
> +		} else if (sig > thold &&
> +			   (last_event == 0 || sig > last_event + hyst)) {
> +			sta->last_sta_mon_event_signal = sig;
> +			cfg80211_sta_mon_rssi_notify(
> +				rx->sdata->dev, sta->addr,
> +				NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH,
> +				sig, GFP_ATOMIC);
> +		}
> +	}

This seems really familiar - perhaps if you unify the cfg80211 API for
sta_mon and "normal" CQM (distinguish based on the peer address?) you
can unify this block of code too?

johannes

Re: [PATCH 5/7] mac80211: Implement functionality to monitor station's rssi cross event

[Tamizh chelvam]

On 2018-06-29 15:06, Johannes Berg wrote:
> On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
>> 
>> +	if (sta->rssi_thold && bss_conf->enable_beacon) {
>> +		int last_event = sta->last_sta_mon_event_signal;
>> +		int thold = sta->rssi_thold;
>> +		int hyst = sta->rssi_hyst;
>> +		int sig = -ewma_signal_read(&sta->rx_stats_avg.signal);
>> +
>> +		if (sig < thold &&
>> +		    (last_event == 0 || sig < last_event - hyst)) {
>> +			sta->last_sta_mon_event_signal = sig;
>> +			cfg80211_sta_mon_rssi_notify(
>> +				rx->sdata->dev, sta->addr,
>> +				NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW,
>> +				sig, GFP_ATOMIC);
>> +		} else if (sig > thold &&
>> +			   (last_event == 0 || sig > last_event + hyst)) {
>> +			sta->last_sta_mon_event_signal = sig;
>> +			cfg80211_sta_mon_rssi_notify(
>> +				rx->sdata->dev, sta->addr,
>> +				NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH,
>> +				sig, GFP_ATOMIC);
>> +		}
>> +	}
> 
> This seems really familiar - perhaps if you unify the cfg80211 API for
> sta_mon and "normal" CQM (distinguish based on the peer address?) you
> can unify this block of code too?
> 
Sure I'll check and update it in the next version of the patch.

[PATCH 6/7] cfg80211: Accept multiple RSSI threholds for STA_MON command

[Tamizh chelvam]

Change the NL80211_CMD_STA_MON RSSI threshold attribut to
accept any number of thresholds as a sorted array. If
user send the configuration with single RSSI threshold then
the old mechanism is enabled. Same netlink event will be
generated in both cases. This patch introduced
set_sta_mon_rssi_range_config to configure high and low
value.
Driver supporting this feature should advertise
NL80211_EXT_FEATURE_STA_MON_RSSI_LIST.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 include/net/cfg80211.h       |   10 +++
 include/uapi/linux/nl80211.h |    4 +
 net/wireless/nl80211.c       |  176 ++++++++++++++++++++++++++++++------------
 net/wireless/rdev-ops.h      |   15 ++++
 net/wireless/trace.h         |   26 +++++++
 5 files changed, 180 insertions(+), 51 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7dcf9b9..4c49cc5 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3029,6 +3029,12 @@ struct cfg80211_external_auth_params {
  *	the current level of a station is above/below the configured threshold;
  *	this may need some care when the configuration is changed
  *	(without first being disabled.)
+ * @set_sta_mon_rssi_range_config: Configure two RSSI thresholds in the
+ *	station's rssi monitor.  An event is to be sent only when the
+ *	signal level of a station is found to be outside the two values.
+ *	The driver should advertise %NL80211_EXT_FEATURE_STA_MON_RSSI_LIST if
+ *	this method is implemented. If it is provided then there's no point
+ *	providing @set_sta_mon_rssi_config
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3338,6 +3344,10 @@ struct cfg80211_ops {
 					   struct net_device *dev,
 					   const u8 *addr,
 					   s32 rssi_thold, u32 rssi_hyst);
+	int	(*set_sta_mon_rssi_range_config)(struct wiphy *wiphy,
+						 struct net_device *dev,
+						 const u8 *addr,
+						 s32 rssi_low, s32 rssi_high);
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9d47ee6..8e2a84b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -5188,6 +5188,9 @@ enum nl80211_feature_flags {
  * @NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG: With this driver can set
  *	rssi threshold using %NL80211_ATTR_STA_MON_RSSI_THOLD attribute
  *	for a connected station.
+ * @NL80211_EXT_FEATURE_STA_MON_RSSI_LIST: With this driver the
+ *	%NL80211_ATTR_STA_MON_RSSI_THOLD attribute accepts a list of zero or
+ *	more RSSI threshold values to monitor rather than exactly one threshold.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -5223,6 +5226,7 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT,
 	NL80211_EXT_FEATURE_TXQS,
 	NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG,
+	NL80211_EXT_FEATURE_STA_MON_RSSI_LIST,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 021e55a..c0fccb4 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10134,61 +10134,101 @@ static int nl80211_set_cqm_txe(struct genl_info *info,
 
 static const struct nla_policy
 nl80211_attr_sta_mon_policy[NL80211_ATTR_STA_MON_MAX + 1] = {
-	[NL80211_ATTR_STA_MON_RSSI_THOLD] = { .type = NLA_U32 },
+	[NL80211_ATTR_STA_MON_RSSI_THOLD] = { .type = NLA_BINARY },
 	[NL80211_ATTR_STA_MON_RSSI_HYST] = { .type = NLA_U32 },
 	[NL80211_ATTR_STA_MON_RSSI_THRESHOLD_EVENT] = { .type = NLA_U32 },
 	[NL80211_ATTR_STA_MON_RSSI_LEVEL] = { .type = NLA_S32 },
 };
 
-static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
-				    struct net_device *dev)
+static int cfg80211_set_rssi_range(struct cfg80211_registered_device *rdev,
+				   struct net_device *dev, const u8 *mac_addr,
+				   bool get_last_rssi)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	s32 last, low, high;
+	s32 low, high, last;
 	u32 hyst;
-	int i, n;
-	int err;
-
-	/* RSSI reporting disabled? */
-	if (!wdev->rssi_config)
-		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
+	int i, n, err;
 
-	/*
-	 * Obtain current RSSI value if possible, if not and no RSSI threshold
-	 * event has been received yet, we should receive an event after a
-	 * connection is established and enough beacons received to calculate
-	 * the average.
-	 */
-	if (!wdev->rssi_config->last_rssi_event_value && wdev->current_bss &&
-	    rdev->ops->get_station) {
+	if (get_last_rssi && mac_addr) {
 		struct station_info sinfo = {};
-		u8 *mac_addr;
-
-		mac_addr = wdev->current_bss->pub.bssid;
 
 		err = rdev_get_station(rdev, dev, mac_addr, &sinfo);
 		if (err)
 			return err;
 
-		if (sinfo.filled & BIT(NL80211_STA_INFO_BEACON_SIGNAL_AVG))
-			wdev->rssi_config->last_rssi_event_value =
-				(s8) sinfo.rx_beacon_signal_avg;
+		if (wdev->iftype != NL80211_IFTYPE_STATION &&
+		    wdev->iftype != NL80211_IFTYPE_P2P_CLIENT) {
+			if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL_AVG))
+				wdev->rssi_config->last_rssi_event_value =
+					(s8)sinfo.signal_avg;
+		} else {
+			if (sinfo.filled &
+			    BIT(NL80211_STA_INFO_BEACON_SIGNAL_AVG))
+				wdev->rssi_config->last_rssi_event_value =
+					(s8)sinfo.rx_beacon_signal_avg;
+		}
 	}
 
 	last = wdev->rssi_config->last_rssi_event_value;
 	hyst = wdev->rssi_config->rssi_hyst;
 	n = wdev->rssi_config->n_rssi_thresholds;
 
-	for (i = 0; i < n; i++)
+	for (i = 0; i < n; i++) {
 		if (last < wdev->rssi_config->rssi_thresholds[i])
 			break;
+	}
 
 	low = i > 0 ?
 		(wdev->rssi_config->rssi_thresholds[i - 1] - hyst) : S32_MIN;
 	high = i < n ?
 		(wdev->rssi_config->rssi_thresholds[i] + hyst - 1) : S32_MAX;
 
-	return rdev_set_cqm_rssi_range_config(rdev, dev, low, high);
+	if (wdev->iftype == NL80211_IFTYPE_STATION ||
+	    wdev->iftype == NL80211_IFTYPE_P2P_CLIENT)
+		return rdev_set_cqm_rssi_range_config(rdev, dev, low, high);
+
+	return rdev_set_sta_mon_rssi_range_config(rdev, dev, mac_addr,
+						  low, high);
+}
+
+static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
+				    struct net_device *dev)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	u8 *mac_addr = NULL;
+
+	/* RSSI reporting disabled? */
+	if (!wdev->rssi_config)
+		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
+
+	/*
+	 * Obtain current RSSI value if possible, if not and no RSSI threshold
+	 * event has been received yet, we should receive an event after a
+	 * connection is established and enough beacons received to calculate
+	 * the average.
+	 */
+	if (!wdev->rssi_config->last_rssi_event_value && wdev->current_bss &&
+	    rdev->ops->get_station)
+		mac_addr = wdev->current_bss->pub.bssid;
+
+	return cfg80211_set_rssi_range(rdev, dev, mac_addr,
+				!wdev->rssi_config->last_rssi_event_value);
+}
+
+static int nl80211_validate_rssi_tholds(const s32 *thresholds, int n_thresholds)
+{
+	int i;
+	s32 prev = S32_MIN;
+
+	/* Check all values negative and sorted */
+	for (i = 0; i < n_thresholds; i++) {
+		if (thresholds[i] > 0 || thresholds[i] <= prev)
+			return -EINVAL;
+
+		prev = thresholds[i];
+	}
+
+	return 0;
 }
 
 static struct cfg80211_rssi_config *
@@ -10228,16 +10268,11 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	int i, err;
-	s32 prev = S32_MIN;
-
-	/* Check all values negative and sorted */
-	for (i = 0; i < n_thresholds; i++) {
-		if (thresholds[i] > 0 || thresholds[i] <= prev)
-			return -EINVAL;
+	int err;
 
-		prev = thresholds[i];
-	}
+	err = nl80211_validate_rssi_tholds(thresholds, n_thresholds);
+	if (err)
+		return err;
 
 	if (wdev->iftype != NL80211_IFTYPE_STATION &&
 	    wdev->iftype != NL80211_IFTYPE_P2P_CLIENT)
@@ -10275,6 +10310,8 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 	}
 
 	err = cfg80211_cqm_rssi_update(rdev, dev);
+	if (err)
+		cfg80211_rssi_config_free(wdev, NULL);
 
 unlock:
 	wdev_unlock(wdev);
@@ -12922,21 +12959,13 @@ static int nl80211_tx_control_port(struct sk_buff *skb, struct genl_info *info)
 }
 
 static int nl80211_set_sta_mon_rssi(struct genl_info *info,
-				    const u8 *peer, s32 threshold,
-				    u32 hysteresis)
+				    const u8 *peer, const s32 *thresholds,
+				    int n_thresholds, u32 hysteresis)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-
-	if (threshold > 0)
-		return -EINVAL;
-
-	if (threshold == 0)
-		hysteresis = 0;
-
-	if (!rdev->ops->set_sta_mon_rssi_config)
-		return -EOPNOTSUPP;
+	int err;
 
 	if ((wdev->iftype != NL80211_IFTYPE_AP &&
 	     wdev->iftype != NL80211_IFTYPE_P2P_GO &&
@@ -12945,8 +12974,46 @@ static int nl80211_set_sta_mon_rssi(struct genl_info *info,
 				NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG))
 		return -EOPNOTSUPP;
 
-	return rdev_set_sta_mon_rssi_config(rdev, dev, peer,
-					    threshold, hysteresis);
+	err = nl80211_validate_rssi_tholds(thresholds, n_thresholds);
+	if (err)
+		return err;
+
+	if (n_thresholds <= 1 && rdev->ops->set_sta_mon_rssi_config) {
+		if (n_thresholds == 0 || thresholds[0] == 0)
+			return rdev_set_sta_mon_rssi_config(rdev, dev,
+							    peer, 0, 0);
+
+		return rdev_set_sta_mon_rssi_config(rdev, dev, peer,
+						    thresholds[0], hysteresis);
+	}
+
+	if (!rdev->ops->set_sta_mon_rssi_range_config ||
+	    !wiphy_ext_feature_isset(&rdev->wiphy,
+				     NL80211_EXT_FEATURE_STA_MON_RSSI_LIST))
+		return -EOPNOTSUPP;
+
+	/* Disabling */
+	if (!n_thresholds || (n_thresholds == 1 && thresholds[0] == 0))
+		return rdev_set_sta_mon_rssi_range_config(rdev, dev,
+							  peer, 0, 0);
+
+	wdev_lock(wdev);
+	wdev->rssi_config = cfg80211_get_rssi_config(wdev, thresholds,
+						     n_thresholds, hysteresis,
+						     peer);
+	if (!wdev->rssi_config) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = cfg80211_set_rssi_range(rdev, dev, peer,
+				!wdev->rssi_config->last_rssi_event_value);
+	if (err)
+		cfg80211_rssi_config_free(wdev, peer);
+
+unlock:
+	wdev_unlock(wdev);
+	return err;
 }
 
 static int nl80211_sta_mon(struct sk_buff *skb, struct genl_info *info)
@@ -12970,12 +13037,16 @@ static int nl80211_sta_mon(struct sk_buff *skb, struct genl_info *info)
 	addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
 	if (attrs[NL80211_ATTR_STA_MON_RSSI_THOLD] &&
 	    attrs[NL80211_ATTR_STA_MON_RSSI_HYST]) {
-		s32 threshold =
-			nla_get_s32(attrs[NL80211_ATTR_STA_MON_RSSI_THOLD]);
+		const s32 *tholds =
+			nla_data(attrs[NL80211_ATTR_STA_MON_RSSI_THOLD]);
 		u32 hysteresis =
 			nla_get_u32(attrs[NL80211_ATTR_STA_MON_RSSI_HYST]);
+		int len = nla_len(attrs[NL80211_ATTR_STA_MON_RSSI_THOLD]);
+
+		if (len % 4)
+			return -EINVAL;
 
-		return nl80211_set_sta_mon_rssi(info, addr, threshold,
+		return nl80211_set_sta_mon_rssi(info, addr, tholds, len / 4,
 						hysteresis);
 	}
 	return -EINVAL;
@@ -15422,6 +15493,7 @@ void cfg80211_sta_mon_rssi_notify(struct net_device *dev, const u8 *peer,
 {
 	struct sk_buff *msg;
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
 	struct cfg80211_rssi_config *rssi_config;
 
 	if (WARN_ON(!peer))
@@ -15437,6 +15509,8 @@ void cfg80211_sta_mon_rssi_notify(struct net_device *dev, const u8 *peer,
 		if (!memcmp(rssi_config->addr, peer, ETH_ALEN)) {
 			wdev->rssi_config = rssi_config;
 			wdev->rssi_config->last_rssi_event_value = rssi_level;
+			cfg80211_set_rssi_range(rdev, dev, peer,
+				!wdev->rssi_config->last_rssi_event_value);
 			break;
 		}
 	}
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index faccfae..1184e4a 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1248,4 +1248,19 @@ static inline int rdev_del_pmk(struct cfg80211_registered_device *rdev,
 	return ret;
 }
 
+static inline int
+rdev_set_sta_mon_rssi_range_config(struct cfg80211_registered_device *rdev,
+				   struct net_device *dev, const u8 *peer,
+				   s32 low, s32 high)
+{
+	int ret;
+
+	trace_rdev_set_sta_mon_rssi_range_config(&rdev->wiphy, dev, peer,
+						 low, high);
+	ret = rdev->ops->set_sta_mon_rssi_range_config(&rdev->wiphy, dev, peer,
+						       low, high);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
+
 #endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 5454c57..76c422c 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3282,6 +3282,32 @@
 		  WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(peer),
 		  __entry->rssi_thold, __entry->rssi_hyst)
 );
+
+TRACE_EVENT(rdev_set_sta_mon_rssi_range_config,
+	TP_PROTO(struct wiphy *wiphy,
+		 struct net_device *netdev, const u8 *peer,
+		 s32 low, s32 high),
+	TP_ARGS(wiphy, netdev, peer, low, high),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		NETDEV_ENTRY
+		MAC_ENTRY(peer)
+		__field(s32, rssi_low)
+		__field(s32, rssi_high)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		NETDEV_ASSIGN;
+		MAC_ASSIGN(peer, peer);
+		__entry->rssi_low = low;
+		__entry->rssi_high = high;
+	),
+	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", " MAC_PR_FMT
+		  ", range: %d - %d ",
+		  WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(peer),
+		  __entry->rssi_low, __entry->rssi_high)
+);
+
 TRACE_EVENT(cfg80211_sta_mon_rssi_notify,
 	TP_PROTO(struct net_device *netdev, const u8 *peer,
 		 enum nl80211_sta_mon_rssi_threshold_event rssi_event,
-- 
1.7.9.5

Re: [PATCH 6/7] cfg80211: Accept multiple RSSI threholds for STA_MON command

[Johannes Berg]

On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
> Change the NL80211_CMD_STA_MON RSSI threshold attribut to

This seems weird - you just introduced it a few patches back, and now
you change it and even worry about compatibility and have both cfg80211
callbacks etc. Just remove set_sta_mon_rssi_config() and ask that
drivers/mac80211 implement set_sta_mon_rssi_range_config?

>   * @NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG: With this driver can set
>   *	rssi threshold using %NL80211_ATTR_STA_MON_RSSI_THOLD attribute
>   *	for a connected station.
> + * @NL80211_EXT_FEATURE_STA_MON_RSSI_LIST: With this driver the
> + *	%NL80211_ATTR_STA_MON_RSSI_THOLD attribute accepts a list of zero or
> + *	more RSSI threshold values to monitor rather than exactly one threshold.

And maybe not have two bits here either?

Or do you expect somebody to actually really need the single threshold
in the near future? It seems you're implementing it for mac80211 only,
which doesn't care.

johannes

Re: [PATCH 6/7] cfg80211: Accept multiple RSSI threholds for STA_MON command

[Tamizh chelvam]

On 2018-06-29 15:09, Johannes Berg wrote:
> On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
>> Change the NL80211_CMD_STA_MON RSSI threshold attribut to
> 
> This seems weird - you just introduced it a few patches back, and now
> you change it and even worry about compatibility and have both cfg80211
> callbacks etc. Just remove set_sta_mon_rssi_config() and ask that
> drivers/mac80211 implement set_sta_mon_rssi_range_config?
> 
First I've done patch to have single threshold value for AP mode and as 
per the previous discussion extended to multiple thresholds also. Here 
the intention is to accept one or more than one RSSI thresholds to 
monitor. Any thought ?

>>   * @NL80211_EXT_FEATURE_STA_MON_RSSI_CONFIG: With this driver can set
>>   *	rssi threshold using %NL80211_ATTR_STA_MON_RSSI_THOLD attribute
>>   *	for a connected station.
>> + * @NL80211_EXT_FEATURE_STA_MON_RSSI_LIST: With this driver the
>> + *	%NL80211_ATTR_STA_MON_RSSI_THOLD attribute accepts a list of zero 
>> or
>> + *	more RSSI threshold values to monitor rather than exactly one 
>> threshold.
> 
> And maybe not have two bits here either?
> 
> Or do you expect somebody to actually really need the single threshold
> in the near future? It seems you're implementing it for mac80211 only,
> which doesn't care.
> 

Thanks,
Tamizh.

Re: [PATCH 6/7] cfg80211: Accept multiple RSSI threholds for STA_MON command

[Johannes Berg]

On Thu, 2018-07-05 at 13:07 +0530, Tamizh chelvam wrote:

> First I've done patch to have single threshold value for AP mode and as 
> per the previous discussion extended to multiple thresholds also. Here 
> the intention is to accept one or more than one RSSI thresholds to 
> monitor. Any thought ?

Is there any point in even offering the API for a single one though? I
mean - we only implement it in mac80211 now, and there we clearly
implement multiple, so single would never be used.

Like I said:

> > Or do you expect somebody to actually really need the single threshold
> > in the near future?

johannes

[PATCH 7/7] mac80211: Add api to configure low and high RSSI threshold

[Tamizh chelvam]

Add set_sta_mon_rssi_range_config api to support
configuring low and high RSSI threshold values
for a connected station. And trigger an event if a
station's signal strength goes out of configured
range.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 net/mac80211/cfg.c      |   40 ++++++++++++++++++++++++++++++++++++++++
 net/mac80211/rx.c       |   23 +++++++++++++++++++++++
 net/mac80211/sta_info.h |    6 ++++++
 3 files changed, 69 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 80ced50..5cdd8a3 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3831,12 +3831,51 @@ static int ieee80211_set_sta_mon_rssi_config(struct wiphy *wiphy,
 
 	sta->rssi_thold = rssi_thold;
 	sta->rssi_hyst = rssi_hyst;
+	sta->rssi_low = 0;
+	sta->rssi_high = 0;
 	sta->last_sta_mon_event_signal = 0;
 unlock:
 	mutex_unlock(&sdata->local->sta_mtx);
 	return 0;
 }
 
+static int ieee80211_set_sta_mon_rssi_range_cfg(struct wiphy *wiphy,
+						struct net_device *dev,
+						const u8 *peer,
+						s32 rssi_low, s32 rssi_high)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct sta_info *sta;
+
+	if (sdata->vif.type == NL80211_IFTYPE_AP &&
+	    (!sdata->vif.bss_conf.enable_beacon ||
+	    !wiphy_ext_feature_isset(sdata->local->hw.wiphy,
+				     NL80211_EXT_FEATURE_STA_MON_RSSI_LIST)))
+		return -EOPNOTSUPP;
+
+	mutex_lock(&sdata->local->sta_mtx);
+
+	sta = sta_info_get_bss(sdata, peer);
+	if (!sta) {
+		mutex_unlock(&sdata->local->sta_mtx);
+		return -ENOENT;
+	}
+
+	if (sta->rssi_low == rssi_low &&
+	    sta->rssi_high == rssi_high)
+		goto unlock;
+
+	sta->rssi_thold = 0;
+	sta->rssi_hyst = 0;
+	sta->rssi_low = rssi_low;
+	sta->rssi_high = rssi_high;
+	sta->last_sta_mon_event_signal = 0;
+
+unlock:
+	mutex_unlock(&sdata->local->sta_mtx);
+	return 0;
+}
+
 const struct cfg80211_ops mac80211_config_ops = {
 	.add_virtual_intf = ieee80211_add_iface,
 	.del_virtual_intf = ieee80211_del_iface,
@@ -3932,4 +3971,5 @@ static int ieee80211_set_sta_mon_rssi_config(struct wiphy *wiphy,
 	.tx_control_port = ieee80211_tx_control_port,
 	.get_txq_stats = ieee80211_get_txq_stats,
 	.set_sta_mon_rssi_config = ieee80211_set_sta_mon_rssi_config,
+	.set_sta_mon_rssi_range_config = ieee80211_set_sta_mon_rssi_range_cfg,
 };
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 06a6111..fddb5c6 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1569,6 +1569,29 @@ static void ieee80211_sta_rx_signal_thold_check(struct ieee80211_rx_data *rx)
 				sig, GFP_ATOMIC);
 		}
 	}
+
+	if (sta->rssi_low) {
+		int last_event = sta->last_sta_mon_event_signal;
+		int sig = -ewma_signal_read(&sta->rx_stats_avg.signal);
+		int low = sta->rssi_low;
+		int high = sta->rssi_high;
+
+		if (sig < low &&
+		    (last_event == 0 || last_event >= low)) {
+			sta->last_sta_mon_event_signal = sig;
+			cfg80211_sta_mon_rssi_notify(
+				rx->sdata->dev, sta->addr,
+				NL80211_STA_MON_RSSI_THRESHOLD_EVENT_LOW,
+				sig, GFP_ATOMIC);
+		} else if (sig > high &&
+			   (last_event == 0 || last_event <= high)) {
+			sta->last_sta_mon_event_signal = sig;
+			cfg80211_sta_mon_rssi_notify(
+				rx->sdata->dev, sta->addr,
+				NL80211_STA_MON_RSSI_THRESHOLD_EVENT_HIGH,
+				sig, GFP_ATOMIC);
+		}
+	}
 }
 
 static ieee80211_rx_result debug_noinline
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index e130f6d..05d68f8 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -487,6 +487,10 @@ struct ieee80211_sta_rx_stats {
  *	should cause an event to be sent indicating where the current value
  *	is in relation to the newly configured threshold
  * @rssi_hyst: Station's RSSI hysteresis
+ * @rssi_low: RSSI lower threshold to monitor station's signal strength, a zero
+ *	value implies disabled.  This is an alternative mechanism to the single
+ *	threshold event and can't be enabled simultaneously with it
+ * @rssi_high: RSSI upper threshold for station
  * @last_sta_mon_event_signal: Last signal strength average for a station
  *	that triggered a sta_mon event. 0 indicates that no event has been
  *	generated for the current association
@@ -596,6 +600,8 @@ struct sta_info {
 
 	s32 rssi_thold;
 	u32 rssi_hyst;
+	s32 rssi_low;
+	s32 rssi_high;
 	int last_sta_mon_event_signal;
 	unsigned int count_rx_signal;
 
-- 
1.7.9.5

Re: [PATCH 7/7] mac80211: Add api to configure low and high RSSI threshold

[Johannes Berg]

On Wed, 2018-06-13 at 16:15 +0530, Tamizh chelvam wrote:
> Add set_sta_mon_rssi_range_config api to support
> configuring low and high RSSI threshold values
> for a connected station. And trigger an event if a
> station's signal strength goes out of configured
> range.

Same here, let's not worry about the "single threshold" thing - it
simplifies the code.

johannes