2008-09-08 15:37:22

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 4/4] mac80211: stop queues before carrier off

During testing of the disassociation fixes, Tomas noticed that it
was possible to run into a situation where you'd suddenly get a
few "wlan0: dropped frame to <AP> (unauthorized port)" messages
and I found this to be due to the AP's sta_info having been
removed but netif_carrier_off not having removed/stopped traffic
yet. To avoid that, stop the queue for the interface (and avoid
bringing them up when another vif scans when they weren't up.)

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/mlme.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)

--- everything.orig/net/mac80211/mlme.c 2008-09-08 16:39:59.000000000 +0200
+++ everything/net/mac80211/mlme.c 2008-09-08 16:40:03.000000000 +0200
@@ -429,6 +429,7 @@ static void ieee80211_set_associated(str
sdata->bss_conf.assoc = 1;
ieee80211_bss_info_change_notify(sdata, changed);

+ netif_tx_start_all_queues(sdata->dev);
netif_carrier_on(sdata->dev);

ieee80211_sta_send_apinfo(sdata, ifsta);
@@ -850,6 +851,7 @@ static void ieee80211_set_disassoc(struc
ifsta->assoc_scan_tries = 0;
ifsta->assoc_tries = 0;

+ netif_tx_stop_all_queues(sdata->dev);
netif_carrier_off(sdata->dev);

ieee80211_sta_tear_down_BA_sessions(sdata, sta->addr);
@@ -3269,6 +3271,7 @@ static void ieee80211_sta_reset_auth(str
ifsta->direct_probe_tries = 0;
ifsta->auth_tries = 0;
ifsta->assoc_tries = 0;
+ netif_tx_stop_all_queues(sdata->dev);
netif_carrier_off(sdata->dev);
}

@@ -3745,13 +3748,15 @@ void ieee80211_scan_completed(struct iee
rcu_read_lock();
list_for_each_entry_rcu(sdata, &local->interfaces, list) {
/* Tell AP we're back */
- if (sdata->vif.type == IEEE80211_IF_TYPE_STA &&
- sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED)
- ieee80211_send_nullfunc(local, sdata, 0);
+ if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
+ if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
+ ieee80211_send_nullfunc(local, sdata, 0);
+ netif_tx_wake_all_queues(sdata->dev);
+ }
+ } else
+ netif_tx_wake_all_queues(sdata->dev);

ieee80211_restart_sta_timer(sdata);
-
- netif_wake_queue(sdata->dev);
}
rcu_read_unlock();

@@ -3909,10 +3914,13 @@ static int ieee80211_sta_start_scan(stru

rcu_read_lock();
list_for_each_entry_rcu(sdata, &local->interfaces, list) {
- netif_stop_queue(sdata->dev);
- if (sdata->vif.type == IEEE80211_IF_TYPE_STA &&
- (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED))
- ieee80211_send_nullfunc(local, sdata, 1);
+ if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
+ if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
+ netif_tx_stop_all_queues(sdata->dev);
+ ieee80211_send_nullfunc(local, sdata, 1);
+ }
+ } else
+ netif_tx_stop_all_queues(sdata->dev);
}
rcu_read_unlock();


--



2008-09-08 23:32:37

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: stop queues before carrier off

On Tue, Sep 9, 2008 at 1:29 AM, Matthew Garrett <[email protected]> wrote:
> On Tue, Sep 09, 2008 at 01:08:17AM +0300, Tomas Winkler wrote:
>> It has to be other way around.
>
> [Citation needed]\

This is how any other connection manager I know works. Maybe I'm
trapped in old concepts but I don't find current NM paradigm
friendly.
Tomas

2008-09-08 23:37:12

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: stop queues before carrier off

On Tue, Sep 09, 2008 at 02:32:35AM +0300, Tomas Winkler wrote:
> On Tue, Sep 9, 2008 at 1:29 AM, Matthew Garrett <[email protected]> wrote:
> > On Tue, Sep 09, 2008 at 01:08:17AM +0300, Tomas Winkler wrote:
> >> It has to be other way around.
> >
> > [Citation needed]\
>
> This is how any other connection manager I know works. Maybe I'm
> trapped in old concepts but I don't find current NM paradigm
> friendly.

It's the way it's been in NM since 2004, so I think changing it at this
point would be unfriendly. The aim isn't to build a Windows clone.

--
Matthew Garrett | [email protected]

2008-09-08 21:44:10

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: stop queues before carrier off

On Tue, 2008-09-09 at 00:40 +0300, Tomas Winkler wrote:
> On Mon, Sep 8, 2008 at 6:33 PM, Johannes Berg <[email protected]> wrote:
> > During testing of the disassociation fixes, Tomas noticed that it
> > was possible to run into a situation where you'd suddenly get a
> > few "wlan0: dropped frame to <AP> (unauthorized port)" messages
> > and I found this to be due to the AP's sta_info having been
> > removed but netif_carrier_off not having removed/stopped traffic
> > yet. To avoid that, stop the queue for the interface (and avoid
> > bringing them up when another vif scans when they weren't up.)
> >
> > Signed-off-by: Johannes Berg <[email protected]>
>
> It worked as far as I tested... ACK then.
> I foolishly tried to switch APs with NM... With sadness I have to say that
> we are now where other operating systems were 5 years ago

need... cfg80211... Anyway, was there some problem with NM here?

Dan

> Tomas
>
>
> > ---
> > net/mac80211/mlme.c | 26 +++++++++++++++++---------
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > --- everything.orig/net/mac80211/mlme.c 2008-09-08 16:39:59.000000000 +0200
> > +++ everything/net/mac80211/mlme.c 2008-09-08 16:40:03.000000000 +0200
> > @@ -429,6 +429,7 @@ static void ieee80211_set_associated(str
> > sdata->bss_conf.assoc = 1;
> > ieee80211_bss_info_change_notify(sdata, changed);
> >
> > + netif_tx_start_all_queues(sdata->dev);
> > netif_carrier_on(sdata->dev);
> >
> > ieee80211_sta_send_apinfo(sdata, ifsta);
> > @@ -850,6 +851,7 @@ static void ieee80211_set_disassoc(struc
> > ifsta->assoc_scan_tries = 0;
> > ifsta->assoc_tries = 0;
> >
> > + netif_tx_stop_all_queues(sdata->dev);
> > netif_carrier_off(sdata->dev);
> >
> > ieee80211_sta_tear_down_BA_sessions(sdata, sta->addr);
> > @@ -3269,6 +3271,7 @@ static void ieee80211_sta_reset_auth(str
> > ifsta->direct_probe_tries = 0;
> > ifsta->auth_tries = 0;
> > ifsta->assoc_tries = 0;
> > + netif_tx_stop_all_queues(sdata->dev);
> > netif_carrier_off(sdata->dev);
> > }
> >
> > @@ -3745,13 +3748,15 @@ void ieee80211_scan_completed(struct iee
> > rcu_read_lock();
> > list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> > /* Tell AP we're back */
> > - if (sdata->vif.type == IEEE80211_IF_TYPE_STA &&
> > - sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED)
> > - ieee80211_send_nullfunc(local, sdata, 0);
> > + if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
> > + if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
> > + ieee80211_send_nullfunc(local, sdata, 0);
> > + netif_tx_wake_all_queues(sdata->dev);
> > + }
> > + } else
> > + netif_tx_wake_all_queues(sdata->dev);
> >
> > ieee80211_restart_sta_timer(sdata);
> > -
> > - netif_wake_queue(sdata->dev);
> > }
> > rcu_read_unlock();
> >
> > @@ -3909,10 +3914,13 @@ static int ieee80211_sta_start_scan(stru
> >
> > rcu_read_lock();
> > list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> > - netif_stop_queue(sdata->dev);
> > - if (sdata->vif.type == IEEE80211_IF_TYPE_STA &&
> > - (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED))
> > - ieee80211_send_nullfunc(local, sdata, 1);
> > + if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
> > + if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
> > + netif_tx_stop_all_queues(sdata->dev);
> > + ieee80211_send_nullfunc(local, sdata, 1);
> > + }
> > + } else
> > + netif_tx_stop_all_queues(sdata->dev);
> > }
> > rcu_read_unlock();
> >
> >
> > --
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2008-09-08 21:40:49

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: stop queues before carrier off

On Mon, Sep 8, 2008 at 6:33 PM, Johannes Berg <[email protected]> wrote:
> During testing of the disassociation fixes, Tomas noticed that it
> was possible to run into a situation where you'd suddenly get a
> few "wlan0: dropped frame to <AP> (unauthorized port)" messages
> and I found this to be due to the AP's sta_info having been
> removed but netif_carrier_off not having removed/stopped traffic
> yet. To avoid that, stop the queue for the interface (and avoid
> bringing them up when another vif scans when they weren't up.)
>
> Signed-off-by: Johannes Berg <[email protected]>

It worked as far as I tested... ACK then.
I foolishly tried to switch APs with NM... With sadness I have to say that
we are now where other operating systems were 5 years ago
Tomas


> ---
> net/mac80211/mlme.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> --- everything.orig/net/mac80211/mlme.c 2008-09-08 16:39:59.000000000 +0200
> +++ everything/net/mac80211/mlme.c 2008-09-08 16:40:03.000000000 +0200
> @@ -429,6 +429,7 @@ static void ieee80211_set_associated(str
> sdata->bss_conf.assoc = 1;
> ieee80211_bss_info_change_notify(sdata, changed);
>
> + netif_tx_start_all_queues(sdata->dev);
> netif_carrier_on(sdata->dev);
>
> ieee80211_sta_send_apinfo(sdata, ifsta);
> @@ -850,6 +851,7 @@ static void ieee80211_set_disassoc(struc
> ifsta->assoc_scan_tries = 0;
> ifsta->assoc_tries = 0;
>
> + netif_tx_stop_all_queues(sdata->dev);
> netif_carrier_off(sdata->dev);
>
> ieee80211_sta_tear_down_BA_sessions(sdata, sta->addr);
> @@ -3269,6 +3271,7 @@ static void ieee80211_sta_reset_auth(str
> ifsta->direct_probe_tries = 0;
> ifsta->auth_tries = 0;
> ifsta->assoc_tries = 0;
> + netif_tx_stop_all_queues(sdata->dev);
> netif_carrier_off(sdata->dev);
> }
>
> @@ -3745,13 +3748,15 @@ void ieee80211_scan_completed(struct iee
> rcu_read_lock();
> list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> /* Tell AP we're back */
> - if (sdata->vif.type == IEEE80211_IF_TYPE_STA &&
> - sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED)
> - ieee80211_send_nullfunc(local, sdata, 0);
> + if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
> + if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
> + ieee80211_send_nullfunc(local, sdata, 0);
> + netif_tx_wake_all_queues(sdata->dev);
> + }
> + } else
> + netif_tx_wake_all_queues(sdata->dev);
>
> ieee80211_restart_sta_timer(sdata);
> -
> - netif_wake_queue(sdata->dev);
> }
> rcu_read_unlock();
>
> @@ -3909,10 +3914,13 @@ static int ieee80211_sta_start_scan(stru
>
> rcu_read_lock();
> list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> - netif_stop_queue(sdata->dev);
> - if (sdata->vif.type == IEEE80211_IF_TYPE_STA &&
> - (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED))
> - ieee80211_send_nullfunc(local, sdata, 1);
> + if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
> + if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
> + netif_tx_stop_all_queues(sdata->dev);
> + ieee80211_send_nullfunc(local, sdata, 1);
> + }
> + } else
> + netif_tx_stop_all_queues(sdata->dev);
> }
> rcu_read_unlock();
>
>
> --
>
>

2008-09-09 00:51:40

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: stop queues before carrier off

On Tue, Sep 9, 2008 at 2:37 AM, Matthew Garrett <[email protected]> wrote:
> On Tue, Sep 09, 2008 at 02:32:35AM +0300, Tomas Winkler wrote:
>> On Tue, Sep 9, 2008 at 1:29 AM, Matthew Garrett <[email protected]> wrote:
>> > On Tue, Sep 09, 2008 at 01:08:17AM +0300, Tomas Winkler wrote:
>> >> It has to be other way around.
>> >
>> > [Citation needed]\
>>
>> This is how any other connection manager I know works. Maybe I'm
>> trapped in old concepts but I don't find current NM paradigm
>> friendly.
>
> It's the way it's been in NM since 2004, so I think changing it at this
> point would be unfriendly.

As I see it NM is evolving lately, which is good It didn't have
connection editor before. Now they is just missing <connect> button
along add, edit and delete to make me happy.

The aim isn't to build a Windows clone.

I didn't suggested this and wireless profile is a general concept
nothing to do with Windows.

Another example you live big building where everybody has an AP. So
your scan list has 50 entries.
Now you have to search by eye all this 50 entries instead of directly
requesting connection to your particular AP.
What I want to see is 3 APs my home, work and favorite coffee house. I
would access scan list only if I cannot find any of these three.

But we should probably move this conversation to different mailing
list. Second I didn't plan to criticize anyone I didn't do anything
actively to change it. I just got little frustrated when testing
switching between APs

Thanks
Tomas

2008-09-08 22:08:22

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: stop queues before carrier off

On Tue, Sep 9, 2008 at 12:44 AM, Dan Williams <[email protected]> wrote:
> On Tue, 2008-09-09 at 00:40 +0300, Tomas Winkler wrote:
>> On Mon, Sep 8, 2008 at 6:33 PM, Johannes Berg <[email protected]> wrote:
>> > During testing of the disassociation fixes, Tomas noticed that it
>> > was possible to run into a situation where you'd suddenly get a
>> > few "wlan0: dropped frame to <AP> (unauthorized port)" messages
>> > and I found this to be due to the AP's sta_info having been
>> > removed but netif_carrier_off not having removed/stopped traffic
>> > yet. To avoid that, stop the queue for the interface (and avoid
>> > bringing them up when another vif scans when they weren't up.)
>> >
>> > Signed-off-by: Johannes Berg <[email protected]>
>>
>> It worked as far as I tested... ACK then.
>> I foolishly tried to switch APs with NM... With sadness I have to say that
>> we are now where other operating systems were 5 years ago
>
> need... cfg80211...
Correct

Anyway, was there some problem with NM here?

I've added manually two APs one with security enabled
(WPA2-Personal/CCMP) second open.
I was not able to connect to the open AP with NM it ask for a key.

When I want to connect to hidden ssid I have to manually scan for it
with iwlist wlan0 essid <SSID>, but that's rather old problem I guess.

The GUI has conceptual problem that it trying to connect something in
scan list (Wireless Networks - right click) instead to what we call a
profile (managed list of netrwork connection in NM language)
It has to be other way around. It should be possible to create a
profile form an entry in thes scan list and connect this specific
profile and not clicking on the entry of an scan list.

Even wpa supplicant works with profiles so why not the GUI?

Thanks
Tomas

>
> Dan
>
>> Tomas
>>
>>
>> > ---
>> > net/mac80211/mlme.c | 26 +++++++++++++++++---------
>> > 1 file changed, 17 insertions(+), 9 deletions(-)
>> >
>> > --- everything.orig/net/mac80211/mlme.c 2008-09-08 16:39:59.000000000 +0200
>> > +++ everything/net/mac80211/mlme.c 2008-09-08 16:40:03.000000000 +0200
>> > @@ -429,6 +429,7 @@ static void ieee80211_set_associated(str
>> > sdata->bss_conf.assoc = 1;
>> > ieee80211_bss_info_change_notify(sdata, changed);
>> >
>> > + netif_tx_start_all_queues(sdata->dev);
>> > netif_carrier_on(sdata->dev);
>> >
>> > ieee80211_sta_send_apinfo(sdata, ifsta);
>> > @@ -850,6 +851,7 @@ static void ieee80211_set_disassoc(struc
>> > ifsta->assoc_scan_tries = 0;
>> > ifsta->assoc_tries = 0;
>> >
>> > + netif_tx_stop_all_queues(sdata->dev);
>> > netif_carrier_off(sdata->dev);
>> >
>> > ieee80211_sta_tear_down_BA_sessions(sdata, sta->addr);
>> > @@ -3269,6 +3271,7 @@ static void ieee80211_sta_reset_auth(str
>> > ifsta->direct_probe_tries = 0;
>> > ifsta->auth_tries = 0;
>> > ifsta->assoc_tries = 0;
>> > + netif_tx_stop_all_queues(sdata->dev);
>> > netif_carrier_off(sdata->dev);
>> > }
>> >
>> > @@ -3745,13 +3748,15 @@ void ieee80211_scan_completed(struct iee
>> > rcu_read_lock();
>> > list_for_each_entry_rcu(sdata, &local->interfaces, list) {
>> > /* Tell AP we're back */
>> > - if (sdata->vif.type == IEEE80211_IF_TYPE_STA &&
>> > - sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED)
>> > - ieee80211_send_nullfunc(local, sdata, 0);
>> > + if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
>> > + if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
>> > + ieee80211_send_nullfunc(local, sdata, 0);
>> > + netif_tx_wake_all_queues(sdata->dev);
>> > + }
>> > + } else
>> > + netif_tx_wake_all_queues(sdata->dev);
>> >
>> > ieee80211_restart_sta_timer(sdata);
>> > -
>> > - netif_wake_queue(sdata->dev);
>> > }
>> > rcu_read_unlock();
>> >
>> > @@ -3909,10 +3914,13 @@ static int ieee80211_sta_start_scan(stru
>> >
>> > rcu_read_lock();
>> > list_for_each_entry_rcu(sdata, &local->interfaces, list) {
>> > - netif_stop_queue(sdata->dev);
>> > - if (sdata->vif.type == IEEE80211_IF_TYPE_STA &&
>> > - (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED))
>> > - ieee80211_send_nullfunc(local, sdata, 1);
>> > + if (sdata->vif.type == IEEE80211_IF_TYPE_STA) {
>> > + if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
>> > + netif_tx_stop_all_queues(sdata->dev);
>> > + ieee80211_send_nullfunc(local, sdata, 1);
>> > + }
>> > + } else
>> > + netif_tx_stop_all_queues(sdata->dev);
>> > }
>> > rcu_read_unlock();
>> >
>> >
>> > --
>> >
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>

2008-09-08 15:48:33

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: stop queues before carrier off

Oops, sorry, quilt ate the attribution (From header line), can you add
those for the first three patches? I had

From: Tomas Winkler <[email protected]>

as in the original mails Tomas sent. Alternatively, I can resend?

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-08 22:29:07

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 4/4] mac80211: stop queues before carrier off

On Tue, Sep 09, 2008 at 01:08:17AM +0300, Tomas Winkler wrote:
> It has to be other way around.

[Citation needed]
--
Matthew Garrett | [email protected]