2010-11-20 01:54:32

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v3 1/2] mac80211: use nullfunc instead of probe request for connection monitoring

nullfunc frames are better for connection monitoring, because probe requests
are answered even if the AP has already dropped the connection, whereas
nullfunc frames from an unassociated station will trigger a disassoc/deauth
frame from the AP (WLAN_REASON_CLASS3_FRAME_FROM_NONASSOC_STA), which allows
the station to reconnect immediately instead of waiting until it attempts to
transmit the next unicast frame.

This only works on hardware with reliable tx ACK reporting, any other hardware
needs to fall back to the probe request method.

Signed-off-by: Felix Fietkau <[email protected]>
---
net/mac80211/ieee80211_i.h | 2 +
net/mac80211/mlme.c | 88 +++++++++++++++++++++++++++++++------------
net/mac80211/status.c | 4 ++
3 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 3598abf..7bb9a62 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1264,6 +1264,8 @@ void ieee80211_send_nullfunc(struct ieee80211_local *local,
int powersave);
void ieee80211_sta_rx_notify(struct ieee80211_sub_if_data *sdata,
struct ieee80211_hdr *hdr);
+void ieee80211_sta_tx_notify(struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_hdr *hdr);
void ieee80211_beacon_connection_loss_work(struct work_struct *work);

void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index dfc4a31..18c0e42 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1026,6 +1026,50 @@ void ieee80211_sta_rx_notify(struct ieee80211_sub_if_data *sdata,
ieee80211_sta_reset_conn_monitor(sdata);
}

+static void ieee80211_reset_ap_probe(struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+
+ if (!(ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
+ IEEE80211_STA_CONNECTION_POLL)))
+ return;
+
+ ifmgd->flags &= ~(IEEE80211_STA_CONNECTION_POLL |
+ IEEE80211_STA_BEACON_POLL);
+ mutex_lock(&sdata->local->iflist_mtx);
+ ieee80211_recalc_ps(sdata->local, -1);
+ mutex_unlock(&sdata->local->iflist_mtx);
+
+ if (sdata->local->hw.flags & IEEE80211_HW_CONNECTION_MONITOR)
+ return;
+
+ /*
+ * We've received a probe response, but are not sure whether
+ * we have or will be receiving any beacons or data, so let's
+ * schedule the timers again, just in case.
+ */
+ ieee80211_sta_reset_beacon_monitor(sdata);
+
+ mod_timer(&ifmgd->conn_mon_timer,
+ round_jiffies_up(jiffies +
+ IEEE80211_CONNECTION_IDLE_TIME));
+}
+
+void ieee80211_sta_tx_notify(struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_hdr *hdr)
+{
+ if (!ieee80211_is_data(hdr->frame_control) &&
+ !ieee80211_is_nullfunc(hdr->frame_control))
+ return;
+
+ ieee80211_sta_reset_conn_monitor(sdata);
+
+ if (ieee80211_is_nullfunc(hdr->frame_control)) {
+ sdata->u.mgd.probe_send_count = 0;
+ ieee80211_queue_work(&sdata->local->hw, &sdata->work);
+ }
+}
+
static void ieee80211_mgd_probe_ap_send(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
@@ -1041,8 +1085,19 @@ static void ieee80211_mgd_probe_ap_send(struct ieee80211_sub_if_data *sdata)
if (ifmgd->probe_send_count >= unicast_limit)
dst = NULL;

- ssid = ieee80211_bss_get_ie(ifmgd->associated, WLAN_EID_SSID);
- ieee80211_send_probe_req(sdata, dst, ssid + 2, ssid[1], NULL, 0);
+ /*
+ * When the hardware reports an accurate Tx ACK status, it's
+ * better to send a nullfunc frame instead of a probe request,
+ * as it will kick us off the AP quickly if we aren't associated
+ * anymore. The timeout will be reset if the frame is ACKed by
+ * the AP.
+ */
+ if (sdata->local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS)
+ ieee80211_send_nullfunc(sdata->local, sdata, 0);
+ else {
+ ssid = ieee80211_bss_get_ie(ifmgd->associated, WLAN_EID_SSID);
+ ieee80211_send_probe_req(sdata, dst, ssid + 2, ssid[1], NULL, 0);
+ }

ifmgd->probe_send_count++;
ifmgd->probe_timeout = jiffies + IEEE80211_PROBE_WAIT;
@@ -1509,29 +1564,8 @@ static void ieee80211_rx_mgmt_probe_resp(struct ieee80211_sub_if_data *sdata,
ieee80211_rx_bss_info(sdata, mgmt, len, rx_status, &elems, false);

if (ifmgd->associated &&
- memcmp(mgmt->bssid, ifmgd->associated->bssid, ETH_ALEN) == 0 &&
- ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
- IEEE80211_STA_CONNECTION_POLL)) {
- ifmgd->flags &= ~(IEEE80211_STA_CONNECTION_POLL |
- IEEE80211_STA_BEACON_POLL);
- mutex_lock(&sdata->local->iflist_mtx);
- ieee80211_recalc_ps(sdata->local, -1);
- mutex_unlock(&sdata->local->iflist_mtx);
-
- if (sdata->local->hw.flags & IEEE80211_HW_CONNECTION_MONITOR)
- return;
-
- /*
- * We've received a probe response, but are not sure whether
- * we have or will be receiving any beacons or data, so let's
- * schedule the timers again, just in case.
- */
- ieee80211_sta_reset_beacon_monitor(sdata);
-
- mod_timer(&ifmgd->conn_mon_timer,
- round_jiffies_up(jiffies +
- IEEE80211_CONNECTION_IDLE_TIME));
- }
+ memcmp(mgmt->bssid, ifmgd->associated->bssid, ETH_ALEN) == 0)
+ ieee80211_reset_ap_probe(sdata);
}

/*
@@ -1882,6 +1916,10 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
ifmgd->associated) {
u8 bssid[ETH_ALEN];

+ /* ACK received for nullfunc probing frame */
+ if (!ifmgd->probe_send_count)
+ ieee80211_reset_ap_probe(sdata);
+
memcpy(bssid, ifmgd->associated->bssid, ETH_ALEN);
if (time_is_after_jiffies(ifmgd->probe_timeout))
run_again(ifmgd, ifmgd->probe_timeout);
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 3153c19..8695cd1 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -155,6 +155,10 @@ static void ieee80211_frame_acked(struct sta_info *sta, struct sk_buff *skb)

ieee80211_queue_work(&local->hw, &local->recalc_smps);
}
+
+ if ((sdata->vif.type == NL80211_IFTYPE_STATION) &&
+ (local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS))
+ ieee80211_sta_tx_notify(sdata, (void *) skb->data);
}

void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
--
1.7.3.2



2010-11-20 19:33:53

by Paul Stewart

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] mac80211: use nullfunc instead of probe request for connection monitoring

A couple of issues here, I think.

On Fri, Nov 19, 2010 at 5:54 PM, Felix Fietkau <[email protected]> wrote:
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
[...]
> +void ieee80211_sta_tx_notify(struct ieee80211_sub_if_data *sdata,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct ieee80211_hdr *hdr)
> +{
> + ? ? ? if (!ieee80211_is_data(hdr->frame_control) &&
> + ? ? ? ? ? !ieee80211_is_nullfunc(hdr->frame_control))
> + ? ? ? ? ? return;
> +
> + ? ? ? ieee80211_sta_reset_conn_monitor(sdata);
> +
> + ? ? ? if (ieee80211_is_nullfunc(hdr->frame_control)) {

Could you also add a "&& sdata->u.mgd.probe_send_count = 0" condition
here, or the BEACON/CONNECTION poll test to reduce the number of times
we queue work here? Or should this work be done on every ACKed
nullfunc?

> + ? ? ? ? ? ? ? sdata->u.mgd.probe_send_count = 0;
> + ? ? ? ? ? ? ? ieee80211_queue_work(&sdata->local->hw, &sdata->work);
> + ? ? ? }
> +}
> +
> ?static void ieee80211_mgd_probe_ap_send(struct ieee80211_sub_if_data *sdata)
> ?{
> ? ? ? ?struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
> @@ -1882,6 +1916,10 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
> ? ? ? ? ? ?ifmgd->associated) {
> ? ? ? ? ? ? ? ?u8 bssid[ETH_ALEN];
>
> + ? ? ? ? ? ? ? /* ACK received for nullfunc probing frame */
> + ? ? ? ? ? ? ? if (!ifmgd->probe_send_count)
> + ? ? ? ? ? ? ? ? ? ? ? ieee80211_reset_ap_probe(sdata);
> +

Yeah, but then it looks like we'd end up falling through to the
"ifmgd->probe_send_count < IEEE80211_MAX_PROBE_TRIES" case later in
this block, and send another probe! Not good.

> ? ? ? ? ? ? ? ?memcpy(bssid, ifmgd->associated->bssid, ETH_ALEN);
> ? ? ? ? ? ? ? ?if (time_is_after_jiffies(ifmgd->probe_timeout))
> ? ? ? ? ? ? ? ? ? ? ? ?run_again(ifmgd, ifmgd->probe_timeout);

--
Paul

2010-11-20 01:54:36

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v3 2/2] mac80211: reduce the number of retries for nullfunc probing

Since nullfunc frames are transmitted as unicast frames, they're more
reliable than the broadcast probe requests, so we need fewer retries
to figure out whether the AP is really gone.

Signed-off-by: Felix Fietkau <[email protected]>
---
net/mac80211/mlme.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 18c0e42..0c0f9fa 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -28,6 +28,7 @@
#include "rate.h"
#include "led.h"

+#define IEEE80211_MAX_NULLFUNC_TRIES 2
#define IEEE80211_MAX_PROBE_TRIES 5

/*
@@ -1915,16 +1916,22 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
IEEE80211_STA_CONNECTION_POLL) &&
ifmgd->associated) {
u8 bssid[ETH_ALEN];
+ int max_tries;

/* ACK received for nullfunc probing frame */
if (!ifmgd->probe_send_count)
ieee80211_reset_ap_probe(sdata);

+ if (local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS)
+ max_tries = IEEE80211_MAX_NULLFUNC_TRIES;
+ else
+ max_tries = IEEE80211_MAX_PROBE_TRIES;
+
memcpy(bssid, ifmgd->associated->bssid, ETH_ALEN);
if (time_is_after_jiffies(ifmgd->probe_timeout))
run_again(ifmgd, ifmgd->probe_timeout);

- else if (ifmgd->probe_send_count < IEEE80211_MAX_PROBE_TRIES) {
+ else if (ifmgd->probe_send_count < max_tries) {
#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
wiphy_debug(local->hw.wiphy,
"%s: No probe response from AP %pM"
--
1.7.3.2