[PATCH] mac80211: don't drop null frames during software scan

[PATCH] mac80211: don't drop null frames during software scan

[Kalle Valo]

ieee80211_tx_h_check_assoc() was dropping everything else than probe
requests during software scan. So the null frame with the power save
bit was dropped and AP never received it. This meant that AP never
buffered any frames for the station during software scan.

Fix this by allowing to transmit both probe request and null frames
during software scan. Tested with stlc45xx.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

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

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index c3f0e95..f2494fc 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -193,7 +193,8 @@ ieee80211_tx_h_check_assoc(struct ieee80211_tx_data *tx)
 		return TX_CONTINUE;
 
 	if (unlikely(tx->local->sw_scanning) &&
-	    !ieee80211_is_probe_req(hdr->frame_control))
+	    !ieee80211_is_probe_req(hdr->frame_control) &&
+	    !ieee80211_is_nullfunc(hdr->frame_control))
 		return TX_DROP;
 
 	if (tx->sdata->vif.type == NL80211_IFTYPE_MESH_POINT)

Re: [PATCH] mac80211: don't drop null frames during software scan

[Johannes Berg]

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

On Sat, 2009-03-14 at 16:44 +0200, Kalle Valo wrote:
> ieee80211_tx_h_check_assoc() was dropping everything else than probe
> requests during software scan. So the null frame with the power save
> bit was dropped and AP never received it. This meant that AP never
> buffered any frames for the station during software scan.
> 
> Fix this by allowing to transmit both probe request and null frames
> during software scan. Tested with stlc45xx.

Would it make sense to reorder the scan code instead?

johannes

> Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
> ---
> 
>  net/mac80211/tx.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index c3f0e95..f2494fc 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -193,7 +193,8 @@ ieee80211_tx_h_check_assoc(struct ieee80211_tx_data *tx)
>  		return TX_CONTINUE;
>  
>  	if (unlikely(tx->local->sw_scanning) &&
> -	    !ieee80211_is_probe_req(hdr->frame_control))
> +	    !ieee80211_is_probe_req(hdr->frame_control) &&
> +	    !ieee80211_is_nullfunc(hdr->frame_control))
>  		return TX_DROP;
>  
>  	if (tx->sdata->vif.type == NL80211_IFTYPE_MESH_POINT)
> 
> 

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

Re: [PATCH] mac80211: don't drop null frames during software scan

[Kalle Valo]

Johannes Berg wrote:
> On Sat, 2009-03-14 at 16:44 +0200, Kalle Valo wrote:
>> ieee80211_tx_h_check_assoc() was dropping everything else than probe
>> requests during software scan. So the null frame with the power save
>> bit was dropped and AP never received it. This meant that AP never
>> buffered any frames for the station during software scan.
>>
>> Fix this by allowing to transmit both probe request and null frames
>> during software scan. Tested with stlc45xx.
> 
> Would it make sense to reorder the scan code instead?

Perhaps. I chose this path only because it was simple to implement :) 
I'll take a look at the scan code in more detail and fix it there.

Kalle

Re: [PATCH] mac80211: don't drop null frames during software scan

[Johannes Berg]

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

On Sat, 2009-03-14 at 17:32 +0200, Kalle Valo wrote:
> Johannes Berg wrote:
> > On Sat, 2009-03-14 at 16:44 +0200, Kalle Valo wrote:
> >> ieee80211_tx_h_check_assoc() was dropping everything else than probe
> >> requests during software scan. So the null frame with the power save
> >> bit was dropped and AP never received it. This meant that AP never
> >> buffered any frames for the station during software scan.
> >>
> >> Fix this by allowing to transmit both probe request and null frames
> >> during software scan. Tested with stlc45xx.
> > 
> > Would it make sense to reorder the scan code instead?
> 
> Perhaps. I chose this path only because it was simple to implement :) 
> I'll take a look at the scan code in more detail and fix it there.

You're kinda right too, we want to disable the queues first, then set
sw_scanning/notify the driver, and then send the nullfunc, I think. So
it's either this patch, or iterating the interface list twice.

johannes

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

Re: [PATCH] mac80211: don't drop null frames during software scan

[Johannes Berg]

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

On Sat, 2009-03-14 at 17:07 +0100, Johannes Berg wrote:
> On Sat, 2009-03-14 at 17:32 +0200, Kalle Valo wrote:
> > Johannes Berg wrote:
> > > On Sat, 2009-03-14 at 16:44 +0200, Kalle Valo wrote:
> > >> ieee80211_tx_h_check_assoc() was dropping everything else than probe
> > >> requests during software scan. So the null frame with the power save
> > >> bit was dropped and AP never received it. This meant that AP never
> > >> buffered any frames for the station during software scan.
> > >>
> > >> Fix this by allowing to transmit both probe request and null frames
> > >> during software scan. Tested with stlc45xx.
> > > 
> > > Would it make sense to reorder the scan code instead?
> > 
> > Perhaps. I chose this path only because it was simple to implement :) 
> > I'll take a look at the scan code in more detail and fix it there.
> 
> You're kinda right too, we want to disable the queues first, then set
> sw_scanning/notify the driver, and then send the nullfunc, I think. So
> it's either this patch, or iterating the interface list twice.

Similarly for the beacon disable part, that _needs_ to be done after
sw_scanning=true otherwise it won't actually work...

I think I prefer your patch over that complexity, but would like to have
a good comment explaining it, and possibly one in
scan.c:ieee80211_start_scan that points to that code.

johannes

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

Re: [PATCH] mac80211: don't drop null frames during software scan

[Kalle Valo]

Johannes Berg wrote:
> On Sat, 2009-03-14 at 17:07 +0100, Johannes Berg wrote:
>> On Sat, 2009-03-14 at 17:32 +0200, Kalle Valo wrote:
>>> Perhaps. I chose this path only because it was simple to implement :) 
>>> I'll take a look at the scan code in more detail and fix it there.
>> You're kinda right too, we want to disable the queues first, then set
>> sw_scanning/notify the driver, and then send the nullfunc, I think. So
>> it's either this patch, or iterating the interface list twice.
> 
> Similarly for the beacon disable part, that _needs_ to be done after
> sw_scanning=true otherwise it won't actually work...
> 
> I think I prefer your patch over that complexity, but would like to have
> a good comment explaining it

Ok. I'll add a comment in v2.

> and possibly one in
> scan.c:ieee80211_start_scan that points to that code.

Yeah, that's definitely needed. I was first really confused why the null 
frame got lost.

Kalle

Re: [PATCH] mac80211: don't drop null frames during software scan

[Kalle Valo]

Johannes Berg wrote:
> On Sat, 2009-03-14 at 17:07 +0100, Johannes Berg wrote:
>> On Sat, 2009-03-14 at 17:32 +0200, Kalle Valo wrote:
>>
>>> Perhaps. I chose this path only because it was simple to implement :) 
>>> I'll take a look at the scan code in more detail and fix it there.
>> You're kinda right too, we want to disable the queues first, then set
>> sw_scanning/notify the driver, and then send the nullfunc, I think. So
>> it's either this patch, or iterating the interface list twice.
> 
> Similarly for the beacon disable part, that _needs_ to be done after
> sw_scanning=true otherwise it won't actually work...
> 
> I think I prefer your patch over that complexity, but would like to have
> a good comment explaining it, and possibly one in
> scan.c:ieee80211_start_scan that points to that code.

I just sent v2. If you have time, please take a look and comment.

Kalle

Re: [PATCH] mac80211: don't drop null frames during software scan

[Johannes Berg]

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

On Sat, 2009-03-14 at 17:07 +0100, Johannes Berg wrote:
> On Sat, 2009-03-14 at 17:32 +0200, Kalle Valo wrote:
> > Johannes Berg wrote:
> > > On Sat, 2009-03-14 at 16:44 +0200, Kalle Valo wrote:
> > >> ieee80211_tx_h_check_assoc() was dropping everything else than probe
> > >> requests during software scan. So the null frame with the power save
> > >> bit was dropped and AP never received it. This meant that AP never
> > >> buffered any frames for the station during software scan.
> > >>
> > >> Fix this by allowing to transmit both probe request and null frames
> > >> during software scan. Tested with stlc45xx.
> > > 
> > > Would it make sense to reorder the scan code instead?
> > 
> > Perhaps. I chose this path only because it was simple to implement :) 
> > I'll take a look at the scan code in more detail and fix it there.
> 
> You're kinda right too, we want to disable the queues first, then set
> sw_scanning/notify the driver, and then send the nullfunc, I think. So
> it's either this patch, or iterating the interface list twice.

Related to this, shouldn't the driver notification:

        if (local->ops->sw_scan_start)
                local->ops->sw_scan_start(local_to_hw(local));

be moved to _after_ we stop the subif queues, disable beacons and send
the nullfunc? The sw_scan_complete is done before we re-enable
everything, so it seems logical that the start should be after we
disable it all.

Michael?

johannes

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

Re: [PATCH] mac80211: don't drop null frames during software scan

[Michael Buesch]

On Saturday 14 March 2009 17:12:49 Johannes Berg wrote:
> On Sat, 2009-03-14 at 17:07 +0100, Johannes Berg wrote:
> > On Sat, 2009-03-14 at 17:32 +0200, Kalle Valo wrote:
> > > Johannes Berg wrote:
> > > > On Sat, 2009-03-14 at 16:44 +0200, Kalle Valo wrote:
> > > >> ieee80211_tx_h_check_assoc() was dropping everything else than probe
> > > >> requests during software scan. So the null frame with the power save
> > > >> bit was dropped and AP never received it. This meant that AP never
> > > >> buffered any frames for the station during software scan.
> > > >>
> > > >> Fix this by allowing to transmit both probe request and null frames
> > > >> during software scan. Tested with stlc45xx.
> > > > 
> > > > Would it make sense to reorder the scan code instead?
> > > 
> > > Perhaps. I chose this path only because it was simple to implement :) 
> > > I'll take a look at the scan code in more detail and fix it there.
> > 
> > You're kinda right too, we want to disable the queues first, then set
> > sw_scanning/notify the driver, and then send the nullfunc, I think. So
> > it's either this patch, or iterating the interface list twice.
> 
> Related to this, shouldn't the driver notification:
> 
>         if (local->ops->sw_scan_start)
>                 local->ops->sw_scan_start(local_to_hw(local));
> 
> be moved to _after_ we stop the subif queues, disable beacons and send
> the nullfunc? The sw_scan_complete is done before we re-enable
> everything, so it seems logical that the start should be after we
> disable it all.

For b43 it doesn't matter.

-- 
Greetings, Michael.