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 | 82 ++++++++++++++++++++++++++++++-------------
net/mac80211/status.c | 4 ++
3 files changed, 63 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..ccea1a1 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1026,6 +1026,48 @@ 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))
+ ieee80211_reset_ap_probe(sdata);
+}
+
static void ieee80211_mgd_probe_ap_send(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
@@ -1041,8 +1083,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 +1562,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);
}
/*
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 3153c19..a6dd792 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
On Fri, 2010-11-19 at 13:43 +0100, Felix Fietkau wrote:
> On 2010-11-19 6:58 AM, Johannes Berg wrote:
> > On Fri, 19 Nov 2010 05:42:36 +0100, Felix Fietkau <[email protected]> wrote:
> >> + if (sdata->local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS)
> >> + ieee80211_send_nullfunc(sdata->local, sdata, 0);
> >
> > Hmm, should that really be 0 always? Could it be racy either way?
> > I think that probably needs some analysis.
> Yes, it should be 0. While it's probing the AP,
> IEEE80211_STA_CONNECTION_POLL or IEEE80211_STA_BEACON_POLL is set, and
> before sending the first probe request or nullfunc frame,
> ieee80211_recalc_ps is called. That effectively pulls us out of
> powersave mode, so sending a nullfunc frame that indicates that we're
> awake should be safe.
Ah yes, and the same should be true for devices like iwlwifi or wl1271
that handle more of it in firmware. Unless there's a PS-poll mode
implemented in firmware, but I guess that's unlikely. Might be worth
adding a comment for though.
> >> + 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 +1562,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);
> >
> > Not sure I get this part...
> I just moved most of this code up to to the ieee80211_reset_ap_probe
> function to avoid duplicating it (since it's called from
> ieee80211_sta_tx_notify as well).
Oops, not paying attention, sorry.
johannes
On 2010-11-19 6:58 AM, Johannes Berg wrote:
> On Fri, 19 Nov 2010 05:42:36 +0100, Felix Fietkau <[email protected]> wrote:
>> + if (sdata->local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS)
>> + ieee80211_send_nullfunc(sdata->local, sdata, 0);
>
> Hmm, should that really be 0 always? Could it be racy either way?
> I think that probably needs some analysis.
Yes, it should be 0. While it's probing the AP,
IEEE80211_STA_CONNECTION_POLL or IEEE80211_STA_BEACON_POLL is set, and
before sending the first probe request or nullfunc frame,
ieee80211_recalc_ps is called. That effectively pulls us out of
powersave mode, so sending a nullfunc frame that indicates that we're
awake should be safe.
>> + 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 +1562,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);
>
> Not sure I get this part...
I just moved most of this code up to to the ieee80211_reset_ap_probe
function to avoid duplicating it (since it's called from
ieee80211_sta_tx_notify as well).
- Felix
On Fri, 19 Nov 2010 05:42:36 +0100, Felix Fietkau <[email protected]> wrote:
> + if (!(ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
> + IEEE80211_STA_CONNECTION_POLL)))
> + return;
Some whitespace issues here and some other places?
> + if (sdata->local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS)
> + ieee80211_send_nullfunc(sdata->local, sdata, 0);
Hmm, should that really be 0 always? Could it be racy either way?
I think that probably needs some analysis.
> + 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 +1562,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);
Not sure I get this part...
> + if ((sdata->vif.type == NL80211_IFTYPE_STATION) &&
> + (local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS))
> + ieee80211_sta_tx_notify(sdata, (void *) skb->data);
more whitespace weirdness
johannes