Skip to contentSkip to conversation

compat-wireless: Connection quality monitor extensions

RFC In review Dmitry Tarnyagin 4 participants 5 messages Oct 12, 2011 – Oct 13, 2011

Subsystems (2)
mac80211 802.11 (including cfg80211/NL80211)
lore
Share & tools
lore

Dmitry Tarnyagin

author
#0 lore raw

RFC 2/7 compat-wireless: Connection quality monitor extensions

From: Bartosz Markowski <[email protected]>
Date: Tue, 12 Jul 2011 13:29:03 +0200
Subject: [PATCH 2/7] compat-wireless: CQM STE extensions

added:
* beacon miss threshold - This value specifies the threshold for the BEACON loss level
* tx fail - This value specifies the threshold for the TX loss level

Signed-off-by Bartosz Markowski <[email protected]>
---
 include/linux/compat-3.0.h |    2 +
 include/linux/nl80211.h    |   16 +++++
 include/net/cfg80211.h     |   34 ++++++++++
 include/net/mac80211.h     |   41 ++++++++++++
 net/mac80211/cfg.c         |   54 ++++++++++++++++
 net/mac80211/mlme.c        |   23 ++++++-
 net/wireless/mlme.c        |   25 +++++++
 net/wireless/nl80211.c     |  152  
+++++++++++++++++++++++++++++++++++++++++++-
 net/wireless/nl80211.h     |    9 +++
 9 files changed, 353 insertions(+), 3 deletions(-)
include/linux/compat-3.0.h +0 -0
index 8c8720e..961a3a6 100644
@@ -59,6 +59,7 @@ int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigne
 int __must_check kstrtou8_from_user(const char __user *s, size_t count,

unsigned int base, u8 *res);
int __must_check kstrtos8_from_user(const char __user *s, size_t count,
unsigned int base, s8 *res);

+/*
static inline int __must_check kstrtou64_from_user(const char __user *s,
size_t count, unsigned int base, u64 *res)
{
return kstrtoull_from_user(s, count, base, res);
@@ -78,6 +79,7 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
{
return kstrtoint_from_user(s, count, base, res);
}
+*/

#endif /* (LINUX_VERSION_CODE < KERNEL_VERSION(3,0,0)) */

include/linux/nl80211.h +16 -0
index be125a5..44911f6 100644
@@ -2311,6 +2311,14 @@ enum nl80211_ps_state {
  * @NL80211_ATTR_CQM_RSSI_THRESHOLD_EVENT: RSSI threshold event
  * @NL80211_ATTR_CQM_PKT_LOSS_EVENT: a u32 value indicating that this many
  *	consecutive packets were not acknowledged by the peer
+ * @NL80211_ATTR_CQM_BEACON_MISS_THOLD: BEACON threshold. This value specifies
+ *	the threshold for the BEACON loss level at which an event will be
+ *	sent. Zero to disable.
+ * @NL80211_ATTR_CQM_BEACON_MISS_THOLD_EVENT: BEACON miss event
+ * @NL80211_ATTR_CQM_TX_FAIL_THOLD: TX threshold. This value specifies the
+ *	the threshold for the TX loss level at which an event will be
+ *	sent. Zero to disable.
+ * @NL80211_ATTR_CQM_TX_FAIL_THOLD_EVENT: TX threshold event
  * @__NL80211_ATTR_CQM_AFTER_LAST: internal
  * @NL80211_ATTR_CQM_MAX: highest key attribute
  */

@@ -2321,6 +2329,14 @@ enum nl80211_attr_cqm {
 	NL80211_ATTR_CQM_RSSI_THRESHOLD_EVENT,
 	NL80211_ATTR_CQM_PKT_LOSS_EVENT,
 
+	/* Beacon Threshold */
+	NL80211_ATTR_CQM_BEACON_MISS_THOLD,
+	NL80211_ATTR_CQM_BEACON_MISS_THOLD_EVENT,
+
+	/* Tx Threshold */
+	NL80211_ATTR_CQM_TX_FAIL_THOLD,
+	NL80211_ATTR_CQM_TX_FAIL_THOLD_EVENT,
+
 	/* keep last */
 	__NL80211_ATTR_CQM_AFTER_LAST,
 	NL80211_ATTR_CQM_MAX = __NL80211_ATTR_CQM_AFTER_LAST - 1
include/net/cfg80211.h +0 -0
index 7fcb671..d2a9c21 100644
@@ -1403,6 +1403,9 @@ struct cfg80211_gtk_rekey_data {
  * @set_power_mgmt: Configure WLAN power management. A timeout value of -1
  *	allows the driver to adjust the dynamic ps timeout value.
  * @set_cqm_rssi_config: Configure connection quality monitor RSSI

threshold.
+ * @set_cqm_beacon_miss_config: Configure connection quality monitor BEACON
+ * threshold
+ * @set_cqm_tx_fail_config: Configure connection quality monitor TX threshold.
* @sched_scan_start: Tell the driver to start a scheduled scan.
* @sched_scan_stop: Tell the driver to stop an ongoing scheduled
* scan. The driver_initiated flag specifies whether the driver
@@ -1590,6 +1593,14 @@ struct cfg80211_ops {
struct net_device *dev,
s32 rssi_thold, u32 rssi_hyst);

+ int (*set_cqm_beacon_miss_config)(struct wiphy *wiphy,
+ struct net_device *dev,
+ u32 beacon_thold);
+
+ int (*set_cqm_tx_fail_config)(struct wiphy *wiphy,
+ struct net_device *dev,
+ u32 tx_thold);
+
void (*mgmt_frame_register)(struct wiphy *wiphy,
struct net_device *dev,
u16 frame_type, bool reg);
@@ -3185,6 +3196,29 @@ void cfg80211_gtk_rekey_notify(struct net_device *dev, const u8 *bssid,
void cfg80211_pmksa_candidate_notify(struct net_device *dev, int index,
const u8 *bssid, bool preauth, gfp_t gfp);

+/*
+ * cfg80211_cqm_beacon_miss_notify - connection quality monitoring beacon
+ * miss event
+ * @dev: network device
+ * @gfp: context flags
+ *
+ * This function is called when a configured connection quality monitoring
+ * beacon threshold reached event occurs.
+ */
+void cfg80211_cqm_beacon_miss_notify(struct net_device *dev,
+ gfp_t gfp);
+
+/**
+ * cfg80211_cqm_tx_fail_notify - connection quality monitoring tx failure event
+ * @dev: network device
+ * @gfp: context flags
+ *
+ * This function is called when a configured connection quality monitoring
+ * tx threshold reached event occurs.
+ */
+void cfg80211_cqm_tx_fail_notify(struct net_device *dev,
+ gfp_t gfp);
+
/* Logging, debugging and troubleshooting/diagnostic helpers. */

/* wiphy_printk helpers, similar to dev_printk */

include/net/mac80211.h +0 -0
index e9d5745..604464e 100644
@@ -244,6 +244,10 @@ enum ieee80211_rssi_event {
  * @cqm_rssi_thold: Connection quality monitor RSSI threshold, a zero

value
* implies disabled
* @cqm_rssi_hyst: Connection quality monitor RSSI hysteresis
+ * @cqm_beacon_miss_thold: Connection quality monitor beacon threshold, a zero
+ * value implies disabled
+ * @cqm_tx_fail_thold: Connection quality monitor tx threshold, a zero value
+ * implies disabled
* @arp_addr_list: List of IPv4 addresses for hardware ARP filtering. The
* may filter ARP queries targeted for other addresses than listed here.
* The driver must allow ARP queries targeted for all address listed here
@@ -280,6 +284,8 @@ struct ieee80211_bss_conf {
u16 ht_operation_mode;
s32 cqm_rssi_thold;
u32 cqm_rssi_hyst;
+ u32 cqm_beacon_miss_thold;
+ u32 cqm_tx_fail_thold;
enum nl80211_channel_type channel_type;
__be32 arp_addr_list[IEEE80211_BSS_ARP_ADDR_LIST_LEN];
u8 arp_addr_cnt;
@@ -1126,6 +1132,11 @@ enum sta_notify_cmd {
* @IEEE80211_HW_TX_AMPDU_SETUP_IN_HW: The device handles TX A-MPDU
session
* setup strictly in HW. mac80211 should not attempt to do this in
* software.
+ * @IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS
+ * Connection quality monitoring - beacon miss.
+ *
+ * @IEEE80211_HW_SUPPORTS_CQM_TX_FAIL
+ * Connection quality monitoring - tx failure.
*/
enum ieee80211_hw_flags {
IEEE80211_HW_HAS_RATE_CONTROL = 1<<0,
@@ -1152,6 +1163,8 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SUPPORTS_PER_STA_GTK = 1<<21,
IEEE80211_HW_AP_LINK_PS = 1<<22,
IEEE80211_HW_TX_AMPDU_SETUP_IN_HW = 1<<23,
+ IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS = 1<<24,
+ IEEE80211_HW_SUPPORTS_CQM_TX_FAIL = 1<<25,
};

/**
@@ -3376,6 +3389,34 @@ void ieee80211_cqm_rssi_notify(struct ieee80211_vif *vif,
*/
unsigned char ieee80211_get_operstate(struct ieee80211_vif *vif);

+/*
+ * ieee80211_cqm_beacon_miss_notify - inform a configured connection quality
+ * monitoring beacon miss threshold triggered
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @gfp: context flag
+ *
+ * When the %IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS is set, and a connection
+ * quality monitoring is configured with an beacon miss threshold, the driver
+ * will inform whenever the desired amount of consecutive beacons is missed.
+ */
+void ieee80211_cqm_beacon_miss_notify(struct ieee80211_vif *vif,
+ gfp_t gfp);
+
+/**
+ * ieee80211_cqm_tx_fail_notify - inform a configured connection quality
+ * monitoring beacon miss threshold triggered
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @gfp: context flag
+ *
+ * When the %IEEE80211_HW_SUPPORTS_CQM_TX_FAIL is set, and a connection
+ * quality monitoring is configured with an tx failure threshold, the driver
+ * whenever the desired amount of consecutive TX attempts is failed.
+ */
+void ieee80211_cqm_tx_fail_notify(struct ieee80211_vif *vif,
+ gfp_t gfp);
+
/**
* ieee80211_chswitch_done - Complete channel switch process
* @vif: &struct ieee80211_vif pointer from the add_interface callback.

net/mac80211/cfg.c +54 -0

66 lines not on this page · raw message

net/mac80211/mlme.c +22 -1
index 96f9fae..c3e2d3c 100644
@@ -2174,7 +2174,10 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
 				    ifmgd->probe_send_count, max_tries);
 #endif
 			ieee80211_mgd_probe_ap_send(sdata);
-		} else {
+		} else if (!(local->hw.flags &
+				IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS) &&
+			   !(local->hw.flags &
+				IEEE80211_HW_SUPPORTS_CQM_TX_FAIL)) {
 			/*
 			 * We actually lost the connection ... or did we?
 			 * Let's make sure!

@@ -2811,3 +2814,21 @@ unsigned char ieee80211_get_operstate(struct ieee80211_vif *vif)
 	return sdata->dev->operstate;
 }
 EXPORT_SYMBOL(ieee80211_get_operstate);
+
+void ieee80211_cqm_beacon_miss_notify(struct ieee80211_vif *vif,
+				      gfp_t gfp)
+{
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+
+	cfg80211_cqm_beacon_miss_notify(sdata->dev, gfp);
+}
+EXPORT_SYMBOL(ieee80211_cqm_beacon_miss_notify);
+
+void ieee80211_cqm_tx_fail_notify(struct ieee80211_vif *vif,
+				      gfp_t gfp)
+{
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+
+	cfg80211_cqm_tx_fail_notify(sdata->dev, gfp);
+}
+EXPORT_SYMBOL(ieee80211_cqm_tx_fail_notify);
net/wireless/mlme.c +25 -0
index 21fc970..4194b3e 100644
@@ -1097,6 +1097,7 @@ void cfg80211_gtk_rekey_notify(struct net_device *dev, const u8 *bssid,
 }
 EXPORT_SYMBOL(cfg80211_gtk_rekey_notify);
 
+
 void cfg80211_pmksa_candidate_notify(struct net_device *dev, int index,
 				     const u8 *bssid, bool preauth, gfp_t gfp)
 {

@@ -1107,3 +1108,27 @@ void cfg80211_pmksa_candidate_notify(struct net_device *dev, int index,
 	nl80211_pmksa_candidate_notify(rdev, dev, index, bssid, preauth, gfp);
 }
 EXPORT_SYMBOL(cfg80211_pmksa_candidate_notify);
+
+void cfg80211_cqm_beacon_miss_notify(struct net_device *dev,
+				     gfp_t gfp)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct wiphy *wiphy = wdev->wiphy;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+
+	/* Indicate roaming trigger event to user space */
+	nl80211_send_cqm_beacon_miss_notify(rdev, dev, gfp);
+}
+EXPORT_SYMBOL(cfg80211_cqm_beacon_miss_notify);
+
+void cfg80211_cqm_tx_fail_notify(struct net_device *dev,
+				 gfp_t gfp)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct wiphy *wiphy = wdev->wiphy;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+
+	/* Indicate roaming trigger event to user space */
+	nl80211_send_cqm_tx_fail_notify(rdev, dev, gfp);
+}
+EXPORT_SYMBOL(cfg80211_cqm_tx_fail_notify);
net/wireless/nl80211.c +150 -2

176 lines not on this page · raw message

net/wireless/nl80211.h +9 -0
index f24a1fb..920e19a 100644
@@ -108,6 +108,15 @@ void
 nl80211_send_cqm_pktloss_notify(struct cfg80211_registered_device *rdev,
 				struct net_device *netdev, const u8 *peer,
 				u32 num_packets, gfp_t gfp);
+void
+nl80211_send_cqm_beacon_miss_notify(struct cfg80211_registered_device *rdev,
+			       struct net_device *netdev,
+			       gfp_t gfp);
+
+void
+nl80211_send_cqm_tx_fail_notify(struct cfg80211_registered_device *rdev,
+			   struct net_device *netdev,
+			   gfp_t gfp);
 
 void nl80211_gtk_rekey_notify(struct cfg80211_registered_device *rdev,
 			      struct net_device *netdev, const u8 *bssid,
Signature · 1 line
1.7.1
#1 lore raw

Hi Dmitry,

Some minor comments on this patch:

Note that I'm not a maintainer and that these are comments on the patch itself, not the code it adds.

That said, it's nice to see someone updating compat-wireless when they update the main wireless repository.

Quoted 18 lines · Dmitry Tarnyagin

From: Bartosz Markowski <[email protected]>
Date: Tue, 12 Jul 2011 13:29:03 +0200
Subject: [PATCH 2/7] compat-wireless: CQM STE extensions

added:
* beacon miss threshold - This value specifies the threshold for the BEACON loss level
* tx fail - This value specifies the threshold for the TX loss level

Signed-off-by Bartosz Markowski <[email protected]>

---
 include/linux/compat-3.0.h |    2 +

 include/linux/nl80211.h    |   16 +++++
 include/net/cfg80211.h     |   34 ++++++++++
 include/net/mac80211.h     |   41 ++++++++++++
 net/mac80211/cfg.c         |   54 ++++++++++++++++
 net/mac80211/mlme.c        |   23 ++++++-
 net/wireless/mlme.c        |   25 +++++++
Quoted 3 lines

 net/wireless/nl80211.c     |  152
+++++++++++++++++++++++++++++++++++++++++++-
 net/wireless/nl80211.h     |    9 +++

You are changing a lot of files that are not actually part of the compat-wireless git repository - are you sure that this patch is correct?

Quoted 17 lines
include/linux/compat-3.0.h +0 -0
index 8c8720e..961a3a6 100644
@@ -59,6 +59,7 @@ int __must_check kstrtos16_from_user(const char __user *s,

size_t count, unsigne
 int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
 int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);

+/*
 static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
 {
       return kstrtoull_from_user(s, count, base, res);

include/linux/compat-3.0.h +0 -0
@@ -78,6 +79,7 @@ static inline int __must_check kstrtos32_from_user(const
Quoted 5 lines

char __user *s, size_t
 {
       return kstrtoint_from_user(s, count, base, res); }
+*/

Why are you commenting out this function?

Thanks,

Signature · 5 lines
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
maintainer
#2 lore raw
In reply to Dmitry Tarnyagin
Quoted 4 lines · Dmitry Tarnyagin

added:
* beacon miss threshold - This value specifies the threshold for the BEACON loss level
* tx fail - This value specifies the threshold for the TX loss level

That's a bit short for a changelog for such a big patch. The patch might stand to be split up into two and be explained better.

Also, as Julian pointed out -- get your compat vs. wireless tree in order. You don't want to be modifying the compat repository, it's even impossible, you need to be submitting patches against wireless-next or -testing. You also need to make sure they apply against those trees, which this patch obviously can't:

Quoted 1 line
  include/linux/compat-3.0.h |    2 +

as you can see easily :)

Quoted 9 lines

+++ b/include/linux/nl80211.h

include/linux/nl80211.h +1 -0
@@ -2311,6 +2311,14 @@ enum nl80211_ps_state {
   * @NL80211_ATTR_CQM_RSSI_THRESHOLD_EVENT: RSSI threshold event
   * @NL80211_ATTR_CQM_PKT_LOSS_EVENT: a u32 value indicating that this many
   *	consecutive packets were not acknowledged by the peer
+ * @NL80211_ATTR_CQM_BEACON_MISS_THOLD: BEACON threshold. This value

specifies
+ * the threshold for the BEACON loss level at which an event will be
+ * sent. Zero to disable.

Should it really be zero to disable rather than leaving it out to disable?

Quoted 5 lines

+ * @NL80211_ATTR_CQM_BEACON_MISS_THOLD_EVENT: BEACON miss event
+ * @NL80211_ATTR_CQM_TX_FAIL_THOLD: TX threshold. This value specifies the
+ * the threshold for the TX loss level at which an event will be
+ * sent. Zero to disable.
+ * @NL80211_ATTR_CQM_TX_FAIL_THOLD_EVENT: TX threshold event

How's this not the same as PKT_LOSS_EVENT?

Some advertising for whether this is supported or not is necessary.

Quoted 1 line

+++ b/include/net/mac80211.h

Again a fairly complex patch modifying both cfg80211 and mac80211 -- not good. Keep in mind that there are in fact drivers not using mac80211.

Quoted 11 lines
include/net/mac80211.h +0 -0
@@ -244,6 +244,10 @@ enum ieee80211_rssi_event {
   * @cqm_rssi_thold: Connection quality monitor RSSI threshold, a zero

value
* implies disabled
* @cqm_rssi_hyst: Connection quality monitor RSSI hysteresis
+ * @cqm_beacon_miss_thold: Connection quality monitor beacon threshold, a zero
+ * value implies disabled
+ * @cqm_tx_fail_thold: Connection quality monitor tx threshold, a zero value
+ * implies disabled

Your line-wrapping makes this hard to read :(

Quoted 14 lines

+++ b/net/mac80211/cfg.c

net/mac80211/cfg.c +8 -0
@@ -1751,6 +1751,58 @@ static int ieee80211_set_cqm_rssi_config(struct  
 wiphy *wiphy,
  	return 0;
  }
 
+static int ieee80211_set_cqm_beacon_miss_config(struct wiphy *wiphy,
+						struct net_device *dev,
+						u32 beacon_thold)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	struct ieee80211_vif *vif = &sdata->vif;
+	struct ieee80211_bss_conf *bss_conf = &vif->bss_conf;

net/mac80211/cfg.c static int ieee80211_set_cqm_rssi_config(struct · line 1772

Quoted 10 lines
net/mac80211/cfg.c +10 -0
+
+	if (beacon_thold == bss_conf->cqm_beacon_miss_thold)
+		return 0;
+
+	bss_conf->cqm_beacon_miss_thold = beacon_thold;
+
+	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS)) {
+		if (sdata->vif.type != NL80211_IFTYPE_STATION)
+			return -EOPNOTSUPP;
+		return 0;

This looks/is wrong in multiple ways.

Quoted 13 lines
net/mac80211/mlme.c +5 -1
+++ b/net/mac80211/mlme.c

@@ -2174,7 +2174,10 @@ void ieee80211_sta_work(struct  
 ieee80211_sub_if_data *sdata)
  				    ifmgd->probe_send_count, max_tries);
  #endif
  			ieee80211_mgd_probe_ap_send(sdata);
-		} else {
+		} else if (!(local->hw.flags &
+				IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS) &&
+			   !(local->hw.flags &
+				IEEE80211_HW_SUPPORTS_CQM_TX_FAIL)) {
  			/*
  			 * We actually lost the connection ... or did we?
Quoted 1 line

* Let's make sure!

explain.

net/wireless/mlme.c void cfg80211_gtk_rekey_notify(struct net_device · line 1102

Quoted 11 lines
net/wireless/mlme.c +1 -0
index 21fc970..4194b3e 100644
@@ -1097,6 +1097,7 @@ void cfg80211_gtk_rekey_notify(struct net_device  
 *dev, const u8 *bssid,
  }
  EXPORT_SYMBOL(cfg80211_gtk_rekey_notify);
 
+
  void cfg80211_pmksa_candidate_notify(struct net_device *dev, int index,

spurious whitespace change

johannes

#3 lore raw

Hello Johannes,

Generally we deliver this patch because is usefull in roaming cases. Speed up roaming when beacon miss will be detected (without disconnection in upper layer).
Some questions below.

2011/10/12 Johannes Berg <[email protected]>:

Quoted 21 lines · Dmitry Tarnyagin
Quoted 5 lines

added:
* beacon miss threshold - This value specifies the threshold for the BEACON loss level
* tx fail - This value specifies the threshold for the TX loss level

That's a bit short for a changelog for such a big patch. The patch might stand to be split up into two and be explained better.

Also, as Julian pointed out -- get your compat vs. wireless tree in order. You don't want to be modifying the compat repository, it's even impossible, you need to be submitting patches against wireless-next or -testing. You also need to make sure they apply against those trees, which this patch obviously can't:

Quoted 1 line

  include/linux/compat-3.0.h |    2 +

as you can see easily :)

Quoted 12 lines
Quoted 9 lines

+++ b/include/linux/nl80211.h

include/linux/nl80211.h +1 -0
@@ -2311,6 +2311,14 @@ enum nl80211_ps_state {
    * @NL80211_ATTR_CQM_RSSI_THRESHOLD_EVENT: RSSI threshold event
    * @NL80211_ATTR_CQM_PKT_LOSS_EVENT: a u32 value indicating that this many
    *  consecutive packets were not acknowledged by the peer
+ * @NL80211_ATTR_CQM_BEACON_MISS_THOLD: BEACON threshold. This value

specifies
+ *   the threshold for the BEACON loss level at which an event will be
+ *   sent. Zero to disable.

Should it really be zero to disable rather than leaving it out to disable?

Similar like in case of NL80211_ATTR_CQM_RSSI_ Usefull to disable in run-time also - when associated.

Quoted 8 lines
Quoted 6 lines

+ * @NL80211_ATTR_CQM_BEACON_MISS_THOLD_EVENT: BEACON miss event
+ * @NL80211_ATTR_CQM_TX_FAIL_THOLD: TX threshold. This value specifies the
+ *   the threshold for the TX loss level at which an event will be
+ *   sent. Zero to disable.
+ * @NL80211_ATTR_CQM_TX_FAIL_THOLD_EVENT: TX threshold event

How's this not the same as PKT_LOSS_EVENT?

Based on supplicant code - this is mapped to EVENT_STATION_LOW_ACK. Seems used only in case we are GO/AP and not configurable from user space. I think possible to reuse PKT_LOSS_EVENT.
But still new HW_FLAGS is required for that and counter configuration from user space could be usefull.

Quoted 40 lines

Some advertising for whether this is supported or not is necessary.

Quoted 1 line

+++ b/include/net/mac80211.h

Again a fairly complex patch modifying both cfg80211 and mac80211 -- not good. Keep in mind that there are in fact drivers not using mac80211.

Quoted 11 lines
include/net/mac80211.h +0 -0
@@ -244,6 +244,10 @@ enum ieee80211_rssi_event {
    * @cqm_rssi_thold: Connection quality monitor RSSI threshold, a zero

value
   *  implies disabled
   * @cqm_rssi_hyst: Connection quality monitor RSSI hysteresis
+ * @cqm_beacon_miss_thold: Connection quality monitor beacon threshold, a zero
+ *   value implies disabled
+ * @cqm_tx_fail_thold: Connection quality monitor tx threshold, a zero value
+ *   implies disabled

Your line-wrapping makes this hard to read :(

Quoted 18 lines

+++ b/net/mac80211/cfg.c

net/mac80211/cfg.c +12 -0
@@ -1751,6 +1751,58 @@ static int ieee80211_set_cqm_rssi_config(struct
 wiphy *wiphy,
       return 0;
   }
 
+static int ieee80211_set_cqm_beacon_miss_config(struct wiphy *wiphy,
+                                             struct net_device *dev,
+                                             u32 beacon_thold)
+{
+     struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+     struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+     struct ieee80211_vif *vif = &sdata->vif;
+     struct ieee80211_bss_conf *bss_conf = &vif->bss_conf;
+
+     if (beacon_thold == bss_conf->cqm_beacon_miss_thold)
+             return 0;
+
Quoted 8 lines

net/mac80211/cfg.c static int ieee80211_set_cqm_rssi_config(struct wiphy *wiphy, · line 1771

Quoted 6 lines
net/mac80211/cfg.c +6 -0
+     bss_conf->cqm_beacon_miss_thold = beacon_thold;
+
+     if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS)) {
+             if (sdata->vif.type != NL80211_IFTYPE_STATION)
+                     return -EOPNOTSUPP;
+             return 0;

This looks/is wrong in multiple ways.

Could you add more description here - this is almost the same like ieee80211_set_cqm_rssi_config() fuction.

Quoted 17 lines
Quoted 14 lines
net/mac80211/mlme.c +5 -1
+++ b/net/mac80211/mlme.c

@@ -2174,7 +2174,10 @@ void ieee80211_sta_work(struct
 ieee80211_sub_if_data *sdata)
                                   ifmgd->probe_send_count, max_tries);
   #endif
                       ieee80211_mgd_probe_ap_send(sdata);
-             } else {
+             } else if (!(local->hw.flags &
+                             IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS) &&
+                        !(local->hw.flags &
+                             IEEE80211_HW_SUPPORTS_CQM_TX_FAIL)) {
                       /*
                        * We actually lost the connection ... or did we?
Quoted 1 line

                       * Let's make sure!

explain.

In case of BEACON_MISS we should not report connection_loss to upper layer, we should only notify about beacon_miss and give a chance to roam without disconnection. Without that first we will see disconnection in wpa_supplicant. Next if roam will fail and driver support IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS should report this connection_lost by itself via ieee80211_connection_loss().

Quoted 15 lines

net/wireless/mlme.c void cfg80211_gtk_rekey_notify(struct net_device · line 1102

Quoted 12 lines
net/wireless/mlme.c +1 -0
index 21fc970..4194b3e 100644
@@ -1097,6 +1097,7 @@ void cfg80211_gtk_rekey_notify(struct net_device
 *dev, const u8 *bssid,
   }
   EXPORT_SYMBOL(cfg80211_gtk_rekey_notify);
 
+
   void cfg80211_pmksa_candidate_notify(struct net_device *dev, int index,

spurious whitespace change

Quoted 7 lines

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

BR
Janusz

maintainer
#4 lore raw

Hi,

Please trim your quotes. There's no point in quoting all of the email to reply to specific points.

Quoted 13 lines
Quoted 4 lines
Quoted 4 lines

+++ b/include/linux/nl80211.h

include/linux/nl80211.h +0 -0
@@ -2311,6 +2311,14 @@ enum nl80211_ps_state {
   * @NL80211_ATTR_CQM_RSSI_THRESHOLD_EVENT: RSSI threshold event
   * @NL80211_ATTR_CQM_PKT_LOSS_EVENT: a u32 value indicating that this many
Quoted 8 lines
Quoted 5 lines
(quoted diff) +1 -0
   *  consecutive packets were not acknowledged by the peer
+ * @NL80211_ATTR_CQM_BEACON_MISS_THOLD: BEACON threshold. This value

specifies
+ * the threshold for the BEACON loss level at which an event will be
+ * sent. Zero to disable.

Should it really be zero to disable rather than leaving it out to disable?

Quoted 2 lines

Similar like in case of NL80211_ATTR_CQM_RSSI_ Usefull to disable in run-time also - when associated.

I don't think you understood my question, but I think it's actually OK as is so you can change other config w/o disabling this.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

+ * @NL80211_ATTR_CQM_BEACON_MISS_THOLD_EVENT: BEACON miss event
+ * @NL80211_ATTR_CQM_TX_FAIL_THOLD: TX threshold. This value specifies the
+ * the threshold for the TX loss level at which an event will be
+ * sent. Zero to disable.
+ * @NL80211_ATTR_CQM_TX_FAIL_THOLD_EVENT: TX threshold event

Quoted 7 lines
Quoted 1 line

How's this not the same as PKT_LOSS_EVENT?

Based on supplicant code - this is mapped to EVENT_STATION_LOW_ACK. Seems used only in case we are GO/AP and not configurable from user space. I think possible to reuse PKT_LOSS_EVENT.
But still new HW_FLAGS is required for that and counter configuration from user space could be usefull.

Yeah so add the config for PKT_LOSS_EVENT.

Quoted 11 lines
Quoted 1 line
Quoted 1 line

+ if (!(local->hw.flags &

IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS)) {

Quoted 5 lines

net/mac80211/cfg.c static int ieee80211_set_cqm_rssi_config(struct wiphy *wiphy, · line 1771

Quoted 3 lines
net/mac80211/cfg.c +3 -0
+             if (sdata->vif.type != NL80211_IFTYPE_STATION)
+                     return -EOPNOTSUPP;
+             return 0;

This looks/is wrong in multiple ways.

Could you add more description here - this is almost the same like ieee80211_set_cqm_rssi_config() fuction.

Well so evidently this accepts the config if the HW supports it and the interface type isn't station -- that's bad.

Also, I haven't seen any evidence that you implemented support for this in mac80211 if the device doesn't support it, so you can't do it this way.

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
net/mac80211/mlme.c +4 -1
-             } else {
+             } else if (!(local->hw.flags &
+                             IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS) &&
+                        !(local->hw.flags &
+                             IEEE80211_HW_SUPPORTS_CQM_TX_FAIL)) {
                      /*
                       * We actually lost the connection ... or did we?
                       * Let's make sure!
Quoted 8 lines
Quoted 1 line

explain.

In case of BEACON_MISS we should not report connection_loss to upper layer, we should only notify about beacon_miss and give a chance to roam without disconnection. Without that first we will see disconnection in wpa_supplicant. Next if roam will fail and driver support IEEE80211_HW_SUPPORTS_CQM_BEACON_MISS should report this connection_lost by itself via ieee80211_connection_loss().

So then the test is wrong -- it has to be dependent on wpa_s actually having asked for this.

I really don't want to be reviewing this with so many basic errors. It looks like you blindly copied parts of the other CQM things without understanding what they do. Please find somebody else to review & explain things with you first, I can't do that at this point.

johannes