[PATCH] mac80211: use beacons for connection monitoring

[PATCH] mac80211: use beacons for connection monitoring

[Reinette Chatre]

From: Reinette Chatre <reinette.chatre@intel.com>

The connection monitor currently relies on probe requests paired
with probe responses to ensure that a connection is alive. This is
fragile in some environments where probe responses can get lost.
When we receive beacons we can also consider the connection to be
alive, so cancel connection poll instance when we receive a beacon.

The debug message "cancelling probereq poll due to a received beacon"
is removed as part of this change as this case is hit very often after
the above change and debug log receives significant number of these messages.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 net/mac80211/mlme.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e3b3156..9cf6153 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1743,14 +1743,10 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 	if (memcmp(bssid, mgmt->bssid, ETH_ALEN) != 0)
 		return;
 
-	if (ifmgd->flags & IEEE80211_STA_BEACON_POLL) {
-#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
-		if (net_ratelimit()) {
-			printk(KERN_DEBUG "%s: cancelling probereq poll due "
-			       "to a received beacon\n", sdata->dev->name);
-		}
-#endif
-		ifmgd->flags &= ~IEEE80211_STA_BEACON_POLL;
+	if (ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
+			    IEEE80211_STA_CONNECTION_POLL)) {
+		ifmgd->flags &= ~(IEEE80211_STA_BEACON_POLL |
+				  IEEE80211_STA_CONNECTION_POLL);
 		mutex_lock(&local->iflist_mtx);
 		ieee80211_recalc_ps(local, -1);
 		mutex_unlock(&local->iflist_mtx);
-- 
1.5.6.3

Re: [PATCH] mac80211: use beacons for connection monitoring

[Ben Greear]

On 07/29/2009 02:32 PM, Reinette Chatre wrote:
> From: Reinette Chatre<reinette.chatre@intel.com>
>
> The connection monitor currently relies on probe requests paired
> with probe responses to ensure that a connection is alive. This is
> fragile in some environments where probe responses can get lost.
> When we receive beacons we can also consider the connection to be
> alive, so cancel connection poll instance when we receive a beacon.

I'm chasing a similar problem while working on virtual STA device
support in ath5k.  I was also going to try sending up to 3 polls before
giving up on the connection.  Does that seems like a valid thing to do?

I'll try using this patch as well to see if that helps.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1248 bytes --]

On Wed, 2009-07-29 at 14:39 -0700, Ben Greear wrote:
> On 07/29/2009 02:32 PM, Reinette Chatre wrote:
> > From: Reinette Chatre<reinette.chatre@intel.com>
> >
> > The connection monitor currently relies on probe requests paired
> > with probe responses to ensure that a connection is alive. This is
> > fragile in some environments where probe responses can get lost.
> > When we receive beacons we can also consider the connection to be
> > alive, so cancel connection poll instance when we receive a beacon.
> 
> I'm chasing a similar problem while working on virtual STA device
> support in ath5k.  I was also going to try sending up to 3 polls before
> giving up on the connection.  Does that seems like a valid thing to do?

That seems somewhat better, yeah.

I suspect the real cause of the issue is something else though, like the
probe request not going out properly or the card being in a kind of
'temporary lock-up' state where it's not really doing anything for the
time being -- I've never had a problem on any other (than iwlwifi)
hardware.

Of course, the virtual STA device support stuff potentially has similar
issues, especially if it does channel switching like the ath9k virtual
wiphy stuff.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: use beacons for connection monitoring

[Ben Greear]

Johannes Berg wrote:
> On Wed, 2009-07-29 at 14:39 -0700, Ben Greear wrote:
>   
>> On 07/29/2009 02:32 PM, Reinette Chatre wrote:
>>     
>>> From: Reinette Chatre<reinette.chatre@intel.com>
>>>
>>> The connection monitor currently relies on probe requests paired
>>> with probe responses to ensure that a connection is alive. This is
>>> fragile in some environments where probe responses can get lost.
>>> When we receive beacons we can also consider the connection to be
>>> alive, so cancel connection poll instance when we receive a beacon.
>>>       
>> I'm chasing a similar problem while working on virtual STA device
>> support in ath5k.  I was also going to try sending up to 3 polls before
>> giving up on the connection.  Does that seems like a valid thing to do?
>>     
>
> That seems somewhat better, yeah.
>
> I suspect the real cause of the issue is something else though, like the
> probe request not going out properly or the card being in a kind of
> 'temporary lock-up' state where it's not really doing anything for the
> time being -- I've never had a problem on any other (than iwlwifi)
> hardware.
>
> Of course, the virtual STA device support stuff potentially has similar
> issues, especially if it does channel switching like the ath9k virtual
> wiphy stuff.
>   
We're not trying any channel switching, though it does seem to stop 
transmitting
and/or receiving for a bit during some operations.  The retransmit logic 
was helping,
but it still isn't quite right.  Hopefully we'll make more progress today.

Thanks,
Ben
> johannes
>   


-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] mac80211: use beacons for connection monitoring

[reinette chatre]

On Thu, 2009-07-30 at 03:30 -0700, Johannes Berg wrote:
> On Wed, 2009-07-29 at 14:39 -0700, Ben Greear wrote:
> > On 07/29/2009 02:32 PM, Reinette Chatre wrote:
> > > From: Reinette Chatre<reinette.chatre@intel.com>
> > >
> > > The connection monitor currently relies on probe requests paired
> > > with probe responses to ensure that a connection is alive. This is
> > > fragile in some environments where probe responses can get lost.
> > > When we receive beacons we can also consider the connection to be
> > > alive, so cancel connection poll instance when we receive a beacon.
> > 
> > I'm chasing a similar problem while working on virtual STA device
> > support in ath5k.  I was also going to try sending up to 3 polls before
> > giving up on the connection.  Does that seems like a valid thing to do?
> 
> That seems somewhat better, yeah.
> 
> I suspect the real cause of the issue is something else though, like the
> probe request not going out properly or the card being in a kind of
> 'temporary lock-up' state where it's not really doing anything for the
> time being -- I've never had a problem on any other (than iwlwifi)
> hardware.

I captured the exchange when this problem occurs using a third system.
The card does indeed try several times to send a probe request and they
are making it out over the air. The AP never responds. 

Is there perhaps anything in this exchange that can help us understand
what is going on?

No.     Time        Source                Destination           Protocol Info
 163883 401.891153  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=980, FN=0, Flags=........, SSID="XXX"
 163885 401.891482  00:14:f2:XX:XX:XX     00:16:ea:XX:XX:XX     IEEE 802.11 Probe Response, SN=3557, FN=0, Flags=........, BI=100, SSID="XXX"
 165523 404.884405  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=981, FN=0, Flags=........, SSID="XXX"
 165525 404.885265  00:14:f2:XX:XX:XX     00:16:ea:XX:XX:XX     IEEE 802.11 Probe Response, SN=3827, FN=0, Flags=....R..., BI=100, SSID="XXX"
 166981 407.884161  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=982, FN=0, Flags=........, SSID="XXX"
 166983 407.884555  00:14:f2:XX:XX:XX     00:16:ea:XX:XX:XX     IEEE 802.11 Probe Response, SN=4034, FN=0, Flags=........, BI=100, SSID="XXX"
 168133 410.884167  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=........, SSID="XXX"
 168134 410.884430  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168135 410.884643  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168136 410.884904  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168137 410.885073  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168138 410.885330  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168139 410.885580  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168140 410.885769  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168141 410.886618  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168142 410.887087  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168143 410.887308  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168144 410.887532  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
 168191 411.084889  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Deauthentication, SN=984, FN=0, Flags=........


Reinette

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 3310 bytes --]

On Thu, 2009-07-30 at 10:45 -0700, reinette chatre wrote:

> I captured the exchange when this problem occurs using a third system.
> The card does indeed try several times to send a probe request and they
> are making it out over the air. The AP never responds. 
> 
> Is there perhaps anything in this exchange that can help us understand
> what is going on?
> 
> No.     Time        Source                Destination           Protocol Info
>  163883 401.891153  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=980, FN=0, Flags=........, SSID="XXX"
>  163885 401.891482  00:14:f2:XX:XX:XX     00:16:ea:XX:XX:XX     IEEE 802.11 Probe Response, SN=3557, FN=0, Flags=........, BI=100, SSID="XXX"

It's responding here,

>  165523 404.884405  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=981, FN=0, Flags=........, SSID="XXX"
>  165525 404.885265  00:14:f2:XX:XX:XX     00:16:ea:XX:XX:XX     IEEE 802.11 Probe Response, SN=3827, FN=0, Flags=....R..., BI=100, SSID="XXX"

and here,

>  166981 407.884161  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=982, FN=0, Flags=........, SSID="XXX"
>  166983 407.884555  00:14:f2:XX:XX:XX     00:16:ea:XX:XX:XX     IEEE 802.11 Probe Response, SN=4034, FN=0, Flags=........, BI=100, SSID="XXX"

and here,

>  168133 410.884167  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=........, SSID="XXX"
>  168134 410.884430  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168135 410.884643  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168136 410.884904  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168137 410.885073  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168138 410.885330  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168139 410.885580  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168140 410.885769  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168141 410.886618  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168142 410.887087  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168143 410.887308  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"
>  168144 410.887532  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Probe Request, SN=983, FN=0, Flags=....R..., SSID="XXX"

but then it stops -- try to send that frame 12 times in quick
succession.

>  168191 411.084889  00:16:ea:XX:XX:XX     00:14:f2:XX:XX:XX     IEEE 802.11 Deauthentication, SN=984, FN=0, Flags=........

And then give up. Oddly, that frame is not retried?! I would expect that
to be retried too if the AP has gone south!?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
> From: Reinette Chatre <reinette.chatre@intel.com>
> 
> The connection monitor currently relies on probe requests paired
> with probe responses to ensure that a connection is alive. This is
> fragile in some environments where probe responses can get lost.
> When we receive beacons we can also consider the connection to be
> alive, so cancel connection poll instance when we receive a beacon.
> 
> The debug message "cancelling probereq poll due to a received beacon"
> is removed as part of this change as this case is hit very often after
> the above change and debug log receives significant number of these messages.

Not doing that was actually intentional -- we want to know whether or
not the connection is working both ways, not just whether we can receive
frames. This can certainly be improved, but I'm not sure this is the
best approach.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: use beacons for connection monitoring

[reinette chatre]

On Wed, 2009-07-29 at 14:45 -0700, Johannes Berg wrote:
> On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
> > From: Reinette Chatre <reinette.chatre@intel.com>
> > 
> > The connection monitor currently relies on probe requests paired
> > with probe responses to ensure that a connection is alive. This is
> > fragile in some environments where probe responses can get lost.
> > When we receive beacons we can also consider the connection to be
> > alive, so cancel connection poll instance when we receive a beacon.
> > 
> > The debug message "cancelling probereq poll due to a received beacon"
> > is removed as part of this change as this case is hit very often after
> > the above change and debug log receives significant number of these messages.
> 
> Not doing that was actually intentional -- we want to know whether or
> not the connection is working both ways, not just whether we can receive
> frames. This can certainly be improved, but I'm not sure this is the
> best approach.

I'd be happy to try out other approaches because my connection is
dropping every few minutes. 

Reinette

Re: [PATCH] mac80211: use beacons for connection monitoring

[Maxim Levitsky]

On Wed, 2009-07-29 at 14:50 -0700, reinette chatre wrote:
> On Wed, 2009-07-29 at 14:45 -0700, Johannes Berg wrote:
> > On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
> > > From: Reinette Chatre <reinette.chatre@intel.com>
> > > 
> > > The connection monitor currently relies on probe requests paired
> > > with probe responses to ensure that a connection is alive. This is
> > > fragile in some environments where probe responses can get lost.
> > > When we receive beacons we can also consider the connection to be
> > > alive, so cancel connection poll instance when we receive a beacon.
> > > 
> > > The debug message "cancelling probereq poll due to a received beacon"
> > > is removed as part of this change as this case is hit very often after
> > > the above change and debug log receives significant number of these messages.
> > 
> > Not doing that was actually intentional -- we want to know whether or
> > not the connection is working both ways, not just whether we can receive
> > frames. This can certainly be improved, but I'm not sure this is the
> > best approach.
> 
> I'd be happy to try out other approaches because my connection is
> dropping every few minutes. 


Mine too, and it even happens when I am a meter away from the AP.
My device is 3945.

Best regards,
	Maxim Levitsky

Re: [PATCH] mac80211: use beacons for connection monitoring

[Ben Greear]

On 07/29/2009 02:45 PM, Johannes Berg wrote:
> On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
>> From: Reinette Chatre<reinette.chatre@intel.com>
>>
>> The connection monitor currently relies on probe requests paired
>> with probe responses to ensure that a connection is alive. This is
>> fragile in some environments where probe responses can get lost.
>> When we receive beacons we can also consider the connection to be
>> alive, so cancel connection poll instance when we receive a beacon.
>>
>> The debug message "cancelling probereq poll due to a received beacon"
>> is removed as part of this change as this case is hit very often after
>> the above change and debug log receives significant number of these messages.
>
> Not doing that was actually intentional -- we want to know whether or
> not the connection is working both ways, not just whether we can receive
> frames. This can certainly be improved, but I'm not sure this is the
> best approach.
>
> johannes


While poking in this code, I think I found a logic bug:

In mlme.c ieee80211_associated, there are several places that do a mod-timer,
and then goto out.  But, out: just does a mod timer again blindly.

Am I missing something, or is this just wrong?

....
		mod_timer(&ifmgd->timer, jiffies + IEEE80211_PROBE_WAIT);
		goto out;
	}

	if (time_after(jiffies, last_rx + IEEE80211_PROBE_IDLE_TIME)) {
		ifmgd->flags |= IEEE80211_STA_PROBEREQ_POLL;
		mutex_lock(&local->iflist_mtx);
		ieee80211_recalc_ps(local, -1);
		mutex_unlock(&local->iflist_mtx);
		ieee80211_send_probe_req(sdata, ifmgd->bssid, ifmgd->ssid,
					 ifmgd->ssid_len, NULL, 0);
	}

  out:
	if (!disassoc)
		mod_timer(&ifmgd->timer,
			  jiffies + IEEE80211_MONITORING_INTERVAL);


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

On Wed, 2009-07-29 at 15:10 -0700, Ben Greear wrote:

> While poking in this code, I think I found a logic bug:
> 
> In mlme.c ieee80211_associated, there are several places that do a mod-timer,
> and then goto out.  But, out: just does a mod timer again blindly.
> 
> Am I missing something, or is this just wrong?

It seems wrong, but it's been rewritten anyway since the version you're
looking at :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: use beacons for connection monitoring

[Marcel Holtmann]

Hi Johannes,

> > The connection monitor currently relies on probe requests paired
> > with probe responses to ensure that a connection is alive. This is
> > fragile in some environments where probe responses can get lost.
> > When we receive beacons we can also consider the connection to be
> > alive, so cancel connection poll instance when we receive a beacon.
> > 
> > The debug message "cancelling probereq poll due to a received beacon"
> > is removed as part of this change as this case is hit very often after
> > the above change and debug log receives significant number of these messages.
> 
> Not doing that was actually intentional -- we want to know whether or
> not the connection is working both ways, not just whether we can receive
> frames. This can certainly be improved, but I'm not sure this is the
> best approach.

this patch might be a band-aid and needs improvement or a total
different solution, but it does help my disconnect problems. It makes my
connection fully stable again. So it is not a wrong approach in general,
because otherwise I have to be always close to me AP. And then using
WiFi is bloody pointless since I could just put an Ethernet cable into
the wall socket directly.

Regards

Marcel

Re: [PATCH] mac80211: use beacons for connection monitoring

[Helmut Schaa]

Am Mittwoch, 29. Juli 2009 schrieb Johannes Berg:
> On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
> > From: Reinette Chatre <reinette.chatre@intel.com>
> > 
> > The connection monitor currently relies on probe requests paired
> > with probe responses to ensure that a connection is alive. This is
> > fragile in some environments where probe responses can get lost.
> > When we receive beacons we can also consider the connection to be
> > alive, so cancel connection poll instance when we receive a beacon.
> > 
> > The debug message "cancelling probereq poll due to a received beacon"
> > is removed as part of this change as this case is hit very often after
> > the above change and debug log receives significant number of these messages.
> 
> Not doing that was actually intentional -- we want to know whether or
> not the connection is working both ways, not just whether we can receive
> frames. This can certainly be improved, but I'm not sure this is the
> best approach.

Just out of curiosity, do direct probe requests get ACKed?

If not, the hardware wouldn't have a possibility to retry the frame and the
probability of loosing a probe request frame would be much higher then of
a data frame.

Helmut

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

On Thu, 2009-07-30 at 11:36 +0200, Helmut Schaa wrote:
> Am Mittwoch, 29. Juli 2009 schrieb Johannes Berg:
> > On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
> > > From: Reinette Chatre <reinette.chatre@intel.com>
> > > 
> > > The connection monitor currently relies on probe requests paired
> > > with probe responses to ensure that a connection is alive. This is
> > > fragile in some environments where probe responses can get lost.
> > > When we receive beacons we can also consider the connection to be
> > > alive, so cancel connection poll instance when we receive a beacon.
> > > 
> > > The debug message "cancelling probereq poll due to a received beacon"
> > > is removed as part of this change as this case is hit very often after
> > > the above change and debug log receives significant number of these messages.
> > 
> > Not doing that was actually intentional -- we want to know whether or
> > not the connection is working both ways, not just whether we can receive
> > frames. This can certainly be improved, but I'm not sure this is the
> > best approach.
> 
> Just out of curiosity, do direct probe requests get ACKed?

Yes, they are normal directed management frames.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: use beacons for connection monitoring

[Maxim Levitsky]

On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
> From: Reinette Chatre <reinette.chatre@intel.com>
> 
> The connection monitor currently relies on probe requests paired
> with probe responses to ensure that a connection is alive. This is
> fragile in some environments where probe responses can get lost.
> When we receive beacons we can also consider the connection to be
> alive, so cancel connection poll instance when we receive a beacon.
> 
> The debug message "cancelling probereq poll due to a received beacon"
> is removed as part of this change as this case is hit very often after
> the above change and debug log receives significant number of these messages.

In my opinion this is the correct solution
I did plenty of wireless monitoring, and I see that nobody sends probe
requests at that rate (1 per second it seems). This is ridiculous.

According to the monitor on my iwl3945, (which might though be
incorrect), sometimes AP doesn't respond in time, but then later it
sends a storm (about 5 or so) probe responses at once (after the deauth)
I think that while wireless is fast most AP aren't, (mine is 150 Mhz
mips system for example), thus it can delay the response for more that
200 ms.

I think that the right solution is to send probes every minute or so,
and retry at last few times, before concluding that connection is lost.
(and if beacons are lost for a prolonged period it might make a sense to
send a probe earlier)

Best regards,
	Maxim Levitsky

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]

On Fri, 2009-07-31 at 10:08 +0300, Maxim Levitsky wrote:
> On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
> > From: Reinette Chatre <reinette.chatre@intel.com>
> > 
> > The connection monitor currently relies on probe requests paired
> > with probe responses to ensure that a connection is alive. This is
> > fragile in some environments where probe responses can get lost.
> > When we receive beacons we can also consider the connection to be
> > alive, so cancel connection poll instance when we receive a beacon.
> > 
> > The debug message "cancelling probereq poll due to a received beacon"
> > is removed as part of this change as this case is hit very often after
> > the above change and debug log receives significant number of these messages.
> 
> In my opinion this is the correct solution
> I did plenty of wireless monitoring, and I see that nobody sends probe
> requests at that rate (1 per second it seems). This is ridiculous.

Fix it then. Offer an implementation of your superior solution. We've
done pretty much exactly what you're all pointing out _forever_ in
mac80211, ever since it was initially merged. Now that it's more
explicit in the code, people are complaining?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

On Fri, 2009-07-31 at 09:43 +0200, Johannes Berg wrote:

> Fix it then. Offer an implementation of your superior solution.

I just don't think that this patch is a solution because it means that
we'll be connected to the AP forever, without knowing anything is wrong,
if the AP can reach us but we can't reach the AP (maybe because we have
a good Intel card with great sensitivity).

Sure -- it's possible to factor in transmit status reports (but remember
that not all hardware gives those reliably) or do things on a different
schedule etc. But as long as nobody wants to really improve things I
don't see why we should take the regression and _remove_ the current
behaviour.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: use beacons for connection monitoring

[Maxim Levitsky]

On Fri, 2009-07-31 at 09:58 +0200, Johannes Berg wrote:
> On Fri, 2009-07-31 at 09:43 +0200, Johannes Berg wrote:
> 
> > Fix it then. Offer an implementation of your superior solution.
> 
> I just don't think that this patch is a solution because it means that
> we'll be connected to the AP forever, without knowing anything is wrong,
> if the AP can reach us but we can't reach the AP (maybe because we have
> a good Intel card with great sensitivity).
> 
> Sure -- it's possible to factor in transmit status reports (but remember
> that not all hardware gives those reliably) or do things on a different
> schedule etc. But as long as nobody wants to really improve things I
> don't see why we should take the regression and _remove_ the current
> behaviour.
> 
> johannes


It is ok to send probes every minute or so, but not each second, and 
probes must be retried.

Because of these probes, wireless literally reconnects every 5 seconds,
here, and I am 10 meters from the AP.

Maxim

Re: [PATCH] mac80211: use beacons for connection monitoring

[Marcel Holtmann]

Hi Maxim,

> > > Fix it then. Offer an implementation of your superior solution.
> > 
> > I just don't think that this patch is a solution because it means that
> > we'll be connected to the AP forever, without knowing anything is wrong,
> > if the AP can reach us but we can't reach the AP (maybe because we have
> > a good Intel card with great sensitivity).
> > 
> > Sure -- it's possible to factor in transmit status reports (but remember
> > that not all hardware gives those reliably) or do things on a different
> > schedule etc. But as long as nobody wants to really improve things I
> > don't see why we should take the regression and _remove_ the current
> > behaviour.
> 
> It is ok to send probes every minute or so, but not each second, and 
> probes must be retried.
> 
> Because of these probes, wireless literally reconnects every 5 seconds,
> here, and I am 10 meters from the AP.

even with Reinette's patch, I am seeing this:

[18594.423855] wlan0: cancelling probereq poll due to a received beacon
[18629.544841] No probe response from AP 00:1c:f0:xx:xx:xx after 200ms, disconnecting.

So that is with the patch, being 1m away from the AP, in the 18th floor
of a building and with only another 10 APs around me.

The Bluetooth world calls this link supervision timeout and the default
value for that timeout is 30 seconds. I think it is the best to make
this value configurable and increase it to at least 30 seconds by
default. Within Bluetooth you can change it and runtime.

Johannes, I don't care how good or bad some hardware is. How sensitive
or not. This is a clear regression and you are overdoing it here with
only waiting for 200ms.

Regards

Marcel

Re: [PATCH] mac80211: use beacons for connection monitoring

[Maxim Levitsky]

[-- Attachment #1: Type: text/plain, Size: 5771 bytes --]

On Fri, 2009-07-31 at 09:43 +0200, Johannes Berg wrote:
> On Fri, 2009-07-31 at 10:08 +0300, Maxim Levitsky wrote:
> > On Wed, 2009-07-29 at 14:32 -0700, Reinette Chatre wrote:
> > > From: Reinette Chatre <reinette.chatre@intel.com>
> > > 
> > > The connection monitor currently relies on probe requests paired
> > > with probe responses to ensure that a connection is alive. This is
> > > fragile in some environments where probe responses can get lost.
> > > When we receive beacons we can also consider the connection to be
> > > alive, so cancel connection poll instance when we receive a beacon.
> > > 
> > > The debug message "cancelling probereq poll due to a received beacon"
> > > is removed as part of this change as this case is hit very often after
> > > the above change and debug log receives significant number of these messages.
> > 
> > In my opinion this is the correct solution
> > I did plenty of wireless monitoring, and I see that nobody sends probe
> > requests at that rate (1 per second it seems). This is ridiculous.
> 
> Fix it then. Offer an implementation of your superior solution. We've
> done pretty much exactly what you're all pointing out _forever_ in
> mac80211, ever since it was initially merged. Now that it's more
> explicit in the code, people are complaining?
> 
> johannes


Here it is:

First patch makes the probe requests be retried (and with it and only
it, my connection is very stable, it never retries more that 3 times,
and I set max retries to 5)

Second patch, trivial one bumps up the timeouts (30 for ping, and 1/2
for response, so device won't send out frames too often)

I attach the patches, since I afraid my mailer would mangle them.

Here they are in-line too:


-------------------------------------------------------------------------------------------------

[MAC80211] Retry probe request few times, before assuming that AP is out of range

From: Maxim Levitsky <maximlevitsky@gmail.com>

Fixes very often reconnects while connected to nearby APs

Signed-off-by: Maxim Levitky <maximlevitsky@gmail.com>
---

 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   27 ++++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)


diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index aec6853..ac2489b 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -280,6 +280,7 @@ struct ieee80211_if_managed {
 	struct work_struct beacon_loss_work;
 
 	unsigned long probe_timeout;
+	unsigned long probe_miss_count;
 
 	struct mutex mtx;
 	struct ieee80211_bss *associated;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index ee83125..38ef7f2 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -31,6 +31,7 @@
 #define IEEE80211_AUTH_MAX_TRIES 3
 #define IEEE80211_ASSOC_TIMEOUT (HZ / 5)
 #define IEEE80211_ASSOC_MAX_TRIES 3
+#define IEEE80211_ASSOC_RETRIES 5
 
 /*
  * beacon loss detection timeout
@@ -1209,6 +1210,7 @@ static void ieee80211_mgd_probe_ap(struct ieee80211_sub_if_data *sdata,
 	ieee80211_recalc_ps(sdata->local, -1);
 	mutex_unlock(&sdata->local->iflist_mtx);
 
+	sdata->u.mgd.probe_miss_count = 0;
 	ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
 	ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
 				 ssid + 2, ssid[1], NULL, 0);
@@ -2072,17 +2074,36 @@ static void ieee80211_sta_work(struct work_struct *work)
 	if (ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
 			    IEEE80211_STA_CONNECTION_POLL) &&
 	    ifmgd->associated) {
+
+		u8 bssid[ETH_ALEN];
+		const u8 *ssid;
+
+		memcpy(bssid, ifmgd->associated->cbss.bssid, ETH_ALEN);
+
 		if (time_is_after_jiffies(ifmgd->probe_timeout))
 			run_again(ifmgd, ifmgd->probe_timeout);
-		else {
-			u8 bssid[ETH_ALEN];
+
+		else if (ifmgd->probe_miss_count < IEEE80211_ASSOC_RETRIES) {
+
+			printk(KERN_DEBUG "No probe response from AP %pM"
+				" after %dms, try %d\n", bssid,
+				(1000 * IEEE80211_PROBE_WAIT)/HZ,
+				(int)ifmgd->probe_miss_count);
+
+			ifmgd->probe_miss_count++;
+			ifmgd->probe_timeout = jiffies + IEEE80211_PROBE_WAIT;
+			run_again(ifmgd, ifmgd->probe_timeout);
+
+			ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
+			ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
+				ssid + 2, ssid[1], NULL, 0);
+		} else {
 			/*
 			 * We actually lost the connection ... or did we?
 			 * Let's make sure!
 			 */
 			ifmgd->flags &= ~(IEEE80211_STA_CONNECTION_POLL |
 					  IEEE80211_STA_BEACON_POLL);
-			memcpy(bssid, ifmgd->associated->cbss.bssid, ETH_ALEN);
 			printk(KERN_DEBUG "No probe response from AP %pM"
 				" after %dms, disconnecting.\n",
 				bssid, (1000 * IEEE80211_PROBE_WAIT)/HZ);





------------------------------------------------------------------------------------------

[MAC80211] Increase timeouts for AP polling

From: Maxim Levitsky <maximlevitsky@gmail.com>

Do a poll every 30 seconds, and wait for response half a second

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---

 net/mac80211/mlme.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 38ef7f2..a8ab40c 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -42,13 +42,13 @@
  * Time the connection can be idle before we probe
  * it to see if we can still talk to the AP.
  */
-#define IEEE80211_CONNECTION_IDLE_TIME	(2 * HZ)
+#define IEEE80211_CONNECTION_IDLE_TIME	(30 * HZ)
 /*
  * Time we wait for a probe response after sending
  * a probe request because of beacon loss or for
  * checking the connection still works.
  */
-#define IEEE80211_PROBE_WAIT		(HZ / 5)
+#define IEEE80211_PROBE_WAIT		(HZ / 2)
 
 #define TMR_RUNNING_TIMER	0
 #define TMR_RUNNING_CHANSW	1

[-- Attachment #2: 0001-mac80211-retry-probe-requests --]
[-- Type: text/plain, Size: 2895 bytes --]

[MAC80211] Retry probe request few times, before assuming that AP is out of range

From: Maxim Levitsky <maximlevitsky@gmail.com>

Fixes very often reconnects while connected to nearby APs

Signed-off-by: Maxim Levitky <maximlevitsky@gmail.com>
---

 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   27 ++++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)


diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index aec6853..ac2489b 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -280,6 +280,7 @@ struct ieee80211_if_managed {
 	struct work_struct beacon_loss_work;
 
 	unsigned long probe_timeout;
+	unsigned long probe_miss_count;
 
 	struct mutex mtx;
 	struct ieee80211_bss *associated;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index ee83125..38ef7f2 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -31,6 +31,7 @@
 #define IEEE80211_AUTH_MAX_TRIES 3
 #define IEEE80211_ASSOC_TIMEOUT (HZ / 5)
 #define IEEE80211_ASSOC_MAX_TRIES 3
+#define IEEE80211_ASSOC_RETRIES 5
 
 /*
  * beacon loss detection timeout
@@ -1209,6 +1210,7 @@ static void ieee80211_mgd_probe_ap(struct ieee80211_sub_if_data *sdata,
 	ieee80211_recalc_ps(sdata->local, -1);
 	mutex_unlock(&sdata->local->iflist_mtx);
 
+	sdata->u.mgd.probe_miss_count = 0;
 	ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
 	ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
 				 ssid + 2, ssid[1], NULL, 0);
@@ -2072,17 +2074,36 @@ static void ieee80211_sta_work(struct work_struct *work)
 	if (ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
 			    IEEE80211_STA_CONNECTION_POLL) &&
 	    ifmgd->associated) {
+
+		u8 bssid[ETH_ALEN];
+		const u8 *ssid;
+
+		memcpy(bssid, ifmgd->associated->cbss.bssid, ETH_ALEN);
+
 		if (time_is_after_jiffies(ifmgd->probe_timeout))
 			run_again(ifmgd, ifmgd->probe_timeout);
-		else {
-			u8 bssid[ETH_ALEN];
+
+		else if (ifmgd->probe_miss_count < IEEE80211_ASSOC_RETRIES) {
+
+			printk(KERN_DEBUG "No probe response from AP %pM"
+				" after %dms, try %d\n", bssid,
+				(1000 * IEEE80211_PROBE_WAIT)/HZ,
+				(int)ifmgd->probe_miss_count);
+
+			ifmgd->probe_miss_count++;
+			ifmgd->probe_timeout = jiffies + IEEE80211_PROBE_WAIT;
+			run_again(ifmgd, ifmgd->probe_timeout);
+
+			ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
+			ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
+				ssid + 2, ssid[1], NULL, 0);
+		} else {
 			/*
 			 * We actually lost the connection ... or did we?
 			 * Let's make sure!
 			 */
 			ifmgd->flags &= ~(IEEE80211_STA_CONNECTION_POLL |
 					  IEEE80211_STA_BEACON_POLL);
-			memcpy(bssid, ifmgd->associated->cbss.bssid, ETH_ALEN);
 			printk(KERN_DEBUG "No probe response from AP %pM"
 				" after %dms, disconnecting.\n",
 				bssid, (1000 * IEEE80211_PROBE_WAIT)/HZ);

[-- Attachment #3: 0002-mac80211-increase-timeouts-for --]
[-- Type: text/plain, Size: 956 bytes --]

[MAC80211] Increase timeouts for AP polling

From: Maxim Levitsky <maximlevitsky@gmail.com>

Do a poll every 30 seconds, and wait for response half a second

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---

 net/mac80211/mlme.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 38ef7f2..a8ab40c 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -42,13 +42,13 @@
  * Time the connection can be idle before we probe
  * it to see if we can still talk to the AP.
  */
-#define IEEE80211_CONNECTION_IDLE_TIME	(2 * HZ)
+#define IEEE80211_CONNECTION_IDLE_TIME	(30 * HZ)
 /*
  * Time we wait for a probe response after sending
  * a probe request because of beacon loss or for
  * checking the connection still works.
  */
-#define IEEE80211_PROBE_WAIT		(HZ / 5)
+#define IEEE80211_PROBE_WAIT		(HZ / 2)
 
 #define TMR_RUNNING_TIMER	0
 #define TMR_RUNNING_CHANSW	1

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 3573 bytes --]

On Fri, 2009-07-31 at 16:19 +0300, Maxim Levitsky wrote:

> First patch makes the probe requests be retried (and with it and only
> it, my connection is very stable, it never retries more that 3 times,
> and I set max retries to 5)
> 
> Second patch, trivial one bumps up the timeouts (30 for ping, and 1/2
> for response, so device won't send out frames too often)

Thank you!

> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index ee83125..38ef7f2 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -31,6 +31,7 @@
>  #define IEEE80211_AUTH_MAX_TRIES 3
>  #define IEEE80211_ASSOC_TIMEOUT (HZ / 5)
>  #define IEEE80211_ASSOC_MAX_TRIES 3
> +#define IEEE80211_ASSOC_RETRIES 5

I'd prefer that be named PROBE_TRIES or something like that?

> @@ -1209,6 +1210,7 @@ static void ieee80211_mgd_probe_ap(struct ieee80211_sub_if_data *sdata,
>  	ieee80211_recalc_ps(sdata->local, -1);
>  	mutex_unlock(&sdata->local->iflist_mtx);
>  
> +	sdata->u.mgd.probe_miss_count = 0;
>  	ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
>  	ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
>  				 ssid + 2, ssid[1], NULL, 0);

That appears to be in the wrong place. Shouldn't that be in the
_set_associated function? Otherwise you're always setting it to 0 before
sending the probe?

> @@ -2072,17 +2074,36 @@ static void ieee80211_sta_work(struct work_struct *work)
>  	if (ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
>  			    IEEE80211_STA_CONNECTION_POLL) &&
>  	    ifmgd->associated) {
> +
> +		u8 bssid[ETH_ALEN];

drop that empty line please

> +		const u8 *ssid;
> +
> +		memcpy(bssid, ifmgd->associated->cbss.bssid, ETH_ALEN);
> +
>  		if (time_is_after_jiffies(ifmgd->probe_timeout))
>  			run_again(ifmgd, ifmgd->probe_timeout);
> -		else {
> -			u8 bssid[ETH_ALEN];
> +
> +		else if (ifmgd->probe_miss_count < IEEE80211_ASSOC_RETRIES) {
> +
> +			printk(KERN_DEBUG "No probe response from AP %pM"

and that one too

> +				" after %dms, try %d\n", bssid,
> +				(1000 * IEEE80211_PROBE_WAIT)/HZ,
> +				(int)ifmgd->probe_miss_count);
> +
> +			ifmgd->probe_miss_count++;
> +			ifmgd->probe_timeout = jiffies + IEEE80211_PROBE_WAIT;
> +			run_again(ifmgd, ifmgd->probe_timeout);
> +
> +			ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
> +			ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
> +				ssid + 2, ssid[1], NULL, 0);
> +		} else {

Shouldn't this just call _probe_ap() again? After you move the setting
to 0 out of it, that is?

> From: Maxim Levitsky <maximlevitsky@gmail.com>
> 
> Do a poll every 30 seconds, and wait for response half a second
> 
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
> 
>  net/mac80211/mlme.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 38ef7f2..a8ab40c 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -42,13 +42,13 @@
>   * Time the connection can be idle before we probe
>   * it to see if we can still talk to the AP.
>   */
> -#define IEEE80211_CONNECTION_IDLE_TIME	(2 * HZ)
> +#define IEEE80211_CONNECTION_IDLE_TIME	(30 * HZ)
>  /*
>   * Time we wait for a probe response after sending
>   * a probe request because of beacon loss or for
>   * checking the connection still works.
>   */
> -#define IEEE80211_PROBE_WAIT		(HZ / 5)
> +#define IEEE80211_PROBE_WAIT		(HZ / 2)


Fine with me.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: use beacons for connection monitoring

[Maxim Levitsky]

On Fri, 2009-07-31 at 15:39 +0200, Johannes Berg wrote:
> On Fri, 2009-07-31 at 16:19 +0300, Maxim Levitsky wrote:
> 
> > First patch makes the probe requests be retried (and with it and only
> > it, my connection is very stable, it never retries more that 3 times,
> > and I set max retries to 5)
> > 
> > Second patch, trivial one bumps up the timeouts (30 for ping, and 1/2
> > for response, so device won't send out frames too often)
> 
> Thank you!
> 
> > diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> > index ee83125..38ef7f2 100644
> > --- a/net/mac80211/mlme.c
> > +++ b/net/mac80211/mlme.c
> > @@ -31,6 +31,7 @@
> >  #define IEEE80211_AUTH_MAX_TRIES 3
> >  #define IEEE80211_ASSOC_TIMEOUT (HZ / 5)
> >  #define IEEE80211_ASSOC_MAX_TRIES 3
> > +#define IEEE80211_ASSOC_RETRIES 5
> 
> I'd prefer that be named PROBE_TRIES or something like that?
Fine

> 
> > @@ -1209,6 +1210,7 @@ static void ieee80211_mgd_probe_ap(struct ieee80211_sub_if_data *sdata,
> >  	ieee80211_recalc_ps(sdata->local, -1);
> >  	mutex_unlock(&sdata->local->iflist_mtx);
> >  
> > +	sdata->u.mgd.probe_miss_count = 0;
> >  	ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
> >  	ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
> >  				 ssid + 2, ssid[1], NULL, 0);
> 
> That appears to be in the wrong place. Shouldn't that be in the
> _set_associated function? Otherwise you're always setting it to 0 before
> sending the probe?
I thought about this too, but I think this is the right place.
ieee80211_mgd_probe_ap is called to start a probe on an AP, it will
ignore following calls till ether AP responds or we reconnect.
What I did is not to fail after first timeout, but resend the assoc
request once more, thus this function won't be called again.

> 
> > @@ -2072,17 +2074,36 @@ static void ieee80211_sta_work(struct work_struct *work)
> >  	if (ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
> >  			    IEEE80211_STA_CONNECTION_POLL) &&
> >  	    ifmgd->associated) {
> > +
> > +		u8 bssid[ETH_ALEN];
> 
> drop that empty line please
no problem
> 
> > +		const u8 *ssid;
> > +
> > +		memcpy(bssid, ifmgd->associated->cbss.bssid, ETH_ALEN);
> > +
> >  		if (time_is_after_jiffies(ifmgd->probe_timeout))
> >  			run_again(ifmgd, ifmgd->probe_timeout);
> > -		else {
> > -			u8 bssid[ETH_ALEN];
> > +
> > +		else if (ifmgd->probe_miss_count < IEEE80211_ASSOC_RETRIES) {
> > +
> > +			printk(KERN_DEBUG "No probe response from AP %pM"
> 
> and that one too
same here
> 
> > +				" after %dms, try %d\n", bssid,
> > +				(1000 * IEEE80211_PROBE_WAIT)/HZ,
> > +				(int)ifmgd->probe_miss_count);
> > +
> > +			ifmgd->probe_miss_count++;
> > +			ifmgd->probe_timeout = jiffies + IEEE80211_PROBE_WAIT;
> > +			run_again(ifmgd, ifmgd->probe_timeout);
> > +
> > +			ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
> > +			ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
> > +				ssid + 2, ssid[1], NULL, 0);
> > +		} else {
> 
> Shouldn't this just call _probe_ap() again? After you move the setting
> to 0 out of it, that is?
No, first of all I tried it :-). It takes the ifmgd->mtx first of all,
and besides this way it is more logical, ieee80211_mgd_probe_ap is
called once per AP ping, and here we continue it.
Since I have copied these 3 lines from ieee80211_mgd_probe_ap I have no
objection to put them in a new function.

> 
> > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > 
> > Do a poll every 30 seconds, and wait for response half a second
> > 
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> > ---
> > 
> >  net/mac80211/mlme.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> > index 38ef7f2..a8ab40c 100644
> > --- a/net/mac80211/mlme.c
> > +++ b/net/mac80211/mlme.c
> > @@ -42,13 +42,13 @@
> >   * Time the connection can be idle before we probe
> >   * it to see if we can still talk to the AP.
> >   */
> > -#define IEEE80211_CONNECTION_IDLE_TIME	(2 * HZ)
> > +#define IEEE80211_CONNECTION_IDLE_TIME	(30 * HZ)
> >  /*
> >   * Time we wait for a probe response after sending
> >   * a probe request because of beacon loss or for
> >   * checking the connection still works.
> >   */
> > -#define IEEE80211_PROBE_WAIT		(HZ / 5)
> > +#define IEEE80211_PROBE_WAIT		(HZ / 2)
> 
> 
> Fine with me.
> 
> johannes

Best regards,
	Maxim Levitsky

Re: [PATCH] mac80211: use beacons for connection monitoring

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]

On Fri, 2009-07-31 at 16:56 +0300, Maxim Levitsky wrote:

> > That appears to be in the wrong place. Shouldn't that be in the
> > _set_associated function? Otherwise you're always setting it to 0 before
> > sending the probe?
> I thought about this too, but I think this is the right place.
> ieee80211_mgd_probe_ap is called to start a probe on an AP, it will
> ignore following calls till ether AP responds or we reconnect.
> What I did is not to fail after first timeout, but resend the assoc
> request once more, thus this function won't be called again.

Ok, that makes sense.

> > > +		else if (ifmgd->probe_miss_count < IEEE80211_ASSOC_RETRIES) {
> > > +
> > > +			printk(KERN_DEBUG "No probe response from AP %pM"
> > 
> > and that one too
> same here
> > 
> > > +				" after %dms, try %d\n", bssid,
> > > +				(1000 * IEEE80211_PROBE_WAIT)/HZ,
> > > +				(int)ifmgd->probe_miss_count);
> > > +
> > > +			ifmgd->probe_miss_count++;
> > > +			ifmgd->probe_timeout = jiffies + IEEE80211_PROBE_WAIT;
> > > +			run_again(ifmgd, ifmgd->probe_timeout);
> > > +
> > > +			ssid = ieee80211_bss_get_ie(&ifmgd->associated->cbss, WLAN_EID_SSID);
> > > +			ieee80211_send_probe_req(sdata, ifmgd->associated->cbss.bssid,
> > > +				ssid + 2, ssid[1], NULL, 0);
> > > +		} else {
> > 
> > Shouldn't this just call _probe_ap() again? After you move the setting
> > to 0 out of it, that is?

> No, first of all I tried it :-). 

:)

> It takes the ifmgd->mtx first of all,
> and besides this way it is more logical, ieee80211_mgd_probe_ap is
> called once per AP ping, and here we continue it.
> Since I have copied these 3 lines from ieee80211_mgd_probe_ap I have no
> objection to put them in a new function.

Don't think it really matters, it's just two function calls anyway...
But maybe it would make sense to put this whole block into a
_mgd_probe_ap_retry function or so? On the other hand, it's just a
handful lines of code, so this is fine too.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]