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 <[email protected]>
---
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)
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
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.
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
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
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
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
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
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 <[email protected]>
> ---
>
> 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)
>
>