[PATCH] mac80211: Increment probe_send_count earlier

[PATCH] mac80211: Increment probe_send_count earlier

[Soumik DAS]

At times, null frame gets transmitted and acked before mac80211
gets to increment probe_send_count. This leads to a situation
where mac80211 waits for ack to null frame, but no ack comes
causing unnecessary disconnection.

Signed-off-by: Soumik Das <soumik.das@stericsson.com>
---
 net/mac80211/mlme.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index dbd4bd9..a1213e4 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1522,6 +1522,7 @@ static void ieee80211_mgd_probe_ap_send(struct ieee80211_sub_if_data *sdata)
 	 * anymore. The timeout will be reset if the frame is ACKed by
 	 * the AP.
 	 */
+	ifmgd->probe_send_count++;
 	if (sdata->local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS) {
 		ifmgd->nullfunc_failed = false;
 		ieee80211_send_nullfunc(sdata->local, sdata, 0);
@@ -1538,7 +1539,6 @@ static void ieee80211_mgd_probe_ap_send(struct ieee80211_sub_if_data *sdata)
 					 0, (u32) -1, true, false);
 	}
 
-	ifmgd->probe_send_count++;
 	ifmgd->probe_timeout = jiffies + msecs_to_jiffies(probe_wait_ms);
 	run_again(ifmgd, ifmgd->probe_timeout);
 	if (sdata->local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS)
-- 
1.7.5.4

Re: [PATCH] mac80211: Increment probe_send_count earlier

[Kalle Valo]

Hi Soumik,

Soumik DAS <soumik.das@stericsson.com> writes:

> At times, null frame gets transmitted and acked before mac80211
> gets to increment probe_send_count. This leads to a situation
> where mac80211 waits for ack to null frame, but no ack comes
> causing unnecessary disconnection.

Can explain a bit more about the scenario when this happens, please?
It's not clear for me from the patch.

-- 
Kalle Valo

RE: [PATCH] mac80211: Increment probe_send_count earlier

[Soumik DAS]

Hi Kalle,

Mac80211 tries to verify the existence of the current AP by probing or sending a NULL frame (function: ieee80211_mgd_probe_ap_send). It 1st sends a null frame to the AP increments probe_send_count and waits for the ACK to the NULL frame for a finite duration of time.

Normally, the sequence of events will be -
	a. call ieee80211_send_nullfunc()
	b. increment probe_send_count ( this indicates probing is underway)
	c. null frame gets transmitted and acked, so mac80211 resets probe_send_count and concludes that the current is still alive.

In some cases the sequence of events gets changed -
	a. call ieee80211_send_nullfunc()
		(current thread gets scheduled out/pre-empted at this point)
	b. null frame gets transmitted and acked
	c. probe_send_count is incremented
	d. mac80211 waits for some specified timeout and checks that probe_send_count is still > 0. It concludes that AP did not acknowledge the null frame and disassociates from the current AP.
	e. After a scan STA again re-associate with the same AP.

Btw, this verification of current AP by sending null frame occurs after every scan.

Regards,
Soumik

-----Original Message-----
From: Kalle Valo [mailto:kvalo@adurom.com] 
Sent: Thursday, May 10, 2012 12:41 PM
To: Soumik DAS
Cc: John Linville; linux-wireless; Johannes Berg
Subject: Re: [PATCH] mac80211: Increment probe_send_count earlier

Hi Soumik,

Soumik DAS <soumik.das@stericsson.com> writes:

> At times, null frame gets transmitted and acked before mac80211 gets 
> to increment probe_send_count. This leads to a situation where 
> mac80211 waits for ack to null frame, but no ack comes causing 
> unnecessary disconnection.

Can explain a bit more about the scenario when this happens, please?
It's not clear for me from the patch.

--
Kalle Valo

Re: [PATCH] mac80211: Increment probe_send_count earlier

[Kalle Valo]

Hi Soumik,

Soumik DAS <soumik.das@stericsson.com> writes:

> In some cases the sequence of events gets changed -
> 	a. call ieee80211_send_nullfunc()
> 		(current thread gets scheduled out/pre-empted at this point)
> 	b. null frame gets transmitted and acked
> 	c. probe_send_count is incremented
> 	d. mac80211 waits for some specified timeout and checks that probe_send_count is still > 0. It concludes that AP did not acknowledge the null frame and disassociates from the current AP.
> 	e. After a scan STA again re-associate with the same AP.

Ok, so there's a race in mac80211? You should describe the race in the
commit log so that a random developer understands what's happening. It
doesn't need to be anything fancy, usually just few sentences is enough
to give sufficient background for the patch. Where the race is, how
often it happens and how to fix it properly, that sort of things.

I repeat this a lot (sorry!) but the most important question the commit
log should answer is "Why?". The question "What?" isn't actually that
important as anyone can read the patch themselves.

-- 
Kalle Valo

Re: [PATCH] mac80211: Increment probe_send_count earlier

[John W. Linville]

On Thu, May 10, 2012 at 12:46:10PM +0300, Kalle Valo wrote:
> Hi Soumik,
> 
> Soumik DAS <soumik.das@stericsson.com> writes:
> 
> > In some cases the sequence of events gets changed -
> > 	a. call ieee80211_send_nullfunc()
> > 		(current thread gets scheduled out/pre-empted at this point)
> > 	b. null frame gets transmitted and acked
> > 	c. probe_send_count is incremented
> > 	d. mac80211 waits for some specified timeout and checks that probe_send_count is still > 0. It concludes that AP did not acknowledge the null frame and disassociates from the current AP.
> > 	e. After a scan STA again re-associate with the same AP.
> 
> Ok, so there's a race in mac80211? You should describe the race in the
> commit log so that a random developer understands what's happening. It
> doesn't need to be anything fancy, usually just few sentences is enough
> to give sufficient background for the patch. Where the race is, how
> often it happens and how to fix it properly, that sort of things.
> 
> I repeat this a lot (sorry!) but the most important question the commit
> log should answer is "Why?". The question "What?" isn't actually that
> important as anyone can read the patch themselves.
> 
> -- 
> Kalle Valo

Soumik,

Are you going to repost with a proper changelog?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.