Re: d80211: How does TX flow control work?

Re: d80211: How does TX flow control work?

[Johannes Berg]

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


> The actual problem was meanwhile identified: shorewall happened to
> overwrite the queueing discipline of wmaster0 with pfifo_fast. I found
> the magic knob to tell shorewall to no longer do this (at least until I
> want to manage traffic control that way...), but I still wonder if it is
> an acceptable situation. Currently, the user can intentionally or
> accidentally screw up the stack this way.

I don't seem to be able to do that:

# tc qdisc change dev wmaster0 pfifo
RTNETLINK answers: Invalid argument

# tc qdisc replace dev wmaster0 pfifo
RTNETLINK answers: Invalid argument

what exactly does shorewall do?

johannes

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

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Johannes Berg wrote:
>> The actual problem was meanwhile identified: shorewall happened to
>> overwrite the queueing discipline of wmaster0 with pfifo_fast. I found
>> the magic knob to tell shorewall to no longer do this (at least until I
>> want to manage traffic control that way...), but I still wonder if it is
>> an acceptable situation. Currently, the user can intentionally or
>> accidentally screw up the stack this way.
> 
> I don't seem to be able to do that:
> 
> # tc qdisc change dev wmaster0 pfifo
> RTNETLINK answers: Invalid argument
> 
> # tc qdisc replace dev wmaster0 pfifo
> RTNETLINK answers: Invalid argument
> 
> what exactly does shorewall do?
> 

Don't recall... need to re-test... lacking time. :(

Just one note: I observed this on a 2.6.19 kernel - in case there is a
difference to the latest.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Jan Kiszka wrote:
> Johannes Berg wrote:
>>> The actual problem was meanwhile identified: shorewall happened to
>>> overwrite the queueing discipline of wmaster0 with pfifo_fast. I found
>>> the magic knob to tell shorewall to no longer do this (at least until I
>>> want to manage traffic control that way...), but I still wonder if it is
>>> an acceptable situation. Currently, the user can intentionally or
>>> accidentally screw up the stack this way.
>> I don't seem to be able to do that:
>>
>> # tc qdisc change dev wmaster0 pfifo
>> RTNETLINK answers: Invalid argument
>>
>> # tc qdisc replace dev wmaster0 pfifo
>> RTNETLINK answers: Invalid argument
>>
>> what exactly does shorewall do?
>>
> 
> Don't recall... need to re-test... lacking time. :(
> 
> Just one note: I observed this on a 2.6.19 kernel - in case there is a
> difference to the latest.
> 

Now I came across this issue once again. It is still present, I just
observed it over the latest rt2x00 tree after updating shorewall and
forgetting to fix its config.

I redirected tc to some logging filter, and this is what shorewall does
when "CLEAR_TC=Yes" is set in shorewall.conf:

...
tc qdisc del dev wmaster0 root
tc qdisc del dev wmaster0 ingress
tc qdisc del dev wlan0 root
tc qdisc del dev wlan0 ingress

HTH,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: d80211: How does TX flow control work?

[Jiri Benc]

On Tue, 01 May 2007 12:41:34 +0200, Jan Kiszka wrote:
> Now I came across this issue once again. It is still present, I just
> observed it over the latest rt2x00 tree after updating shorewall and
> forgetting to fix its config.
> 
> I redirected tc to some logging filter, and this is what shorewall does
> when "CLEAR_TC=Yes" is set in shorewall.conf:
> 
> ...
> tc qdisc del dev wmaster0 root
> tc qdisc del dev wmaster0 ingress
> tc qdisc del dev wlan0 root
> tc qdisc del dev wlan0 ingress

Could you retest with the latest wireless-dev? It should be fixed there.

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Jiri Benc wrote:
> On Tue, 01 May 2007 12:41:34 +0200, Jan Kiszka wrote:
>> Now I came across this issue once again. It is still present, I just
>> observed it over the latest rt2x00 tree after updating shorewall and
>> forgetting to fix its config.
>>
>> I redirected tc to some logging filter, and this is what shorewall does
>> when "CLEAR_TC=Yes" is set in shorewall.conf:
>>
>> ...
>> tc qdisc del dev wmaster0 root
>> tc qdisc del dev wmaster0 ingress
>> tc qdisc del dev wlan0 root
>> tc qdisc del dev wlan0 ingress
> 
> Could you retest with the latest wireless-dev? It should be fixed there.

Could you point me to the patch(es) in question? I only have 2MBit/s
downlink, so that downloading only the patch would speed things up here.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: d80211: How does TX flow control work?

[Johannes Berg]

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

On Tue, 2007-05-01 at 12:50 +0200, Jan Kiszka wrote:

> Could you point me to the patch(es) in question? I only have 2MBit/s
> downlink, so that downloading only the patch would speed things up here.

http://git.kernel.org/?p=linux/kernel/git/linville/wireless-dev.git;a=commit;h=00a908826e778b39a802013729f7826c2d575360

johannes

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

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Johannes Berg wrote:
> On Tue, 2007-05-01 at 12:50 +0200, Jan Kiszka wrote:
> 
>> Could you point me to the patch(es) in question? I only have 2MBit/s
>> downlink, so that downloading only the patch would speed things up here.
> 
> http://git.kernel.org/?p=linux/kernel/git/linville/wireless-dev.git;a=commit;h=00a908826e778b39a802013729f7826c2d575360
> 

Nope, doesn't help. And I wonder how this piece should do so: It just
performs a sanity check on ifconfig up, right? But the "tc qdisc del"
runs here _after_ wlan0 got configured.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: d80211: How does TX flow control work?

[Johannes Berg]

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

On Tue, 2007-05-01 at 15:07 +0200, Jan Kiszka wrote:
> Johannes Berg wrote:
> > On Tue, 2007-05-01 at 12:50 +0200, Jan Kiszka wrote:
> > 
> >> Could you point me to the patch(es) in question? I only have 2MBit/s
> >> downlink, so that downloading only the patch would speed things up here.
> > 
> > http://git.kernel.org/?p=linux/kernel/git/linville/wireless-dev.git;a=commit;h=00a908826e778b39a802013729f7826c2d575360
> > 
> 
> Nope, doesn't help. And I wonder how this piece should do so: It just
> performs a sanity check on ifconfig up, right? But the "tc qdisc del"
> runs here _after_ wlan0 got configured.

I never managed to reproduce that part anyway. I hadn't tried removing
the qdisc when the device is down, but when it's up I never managed
to... odd

johannes

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

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Johannes Berg wrote:
> On Tue, 2007-05-01 at 15:07 +0200, Jan Kiszka wrote:
>> Johannes Berg wrote:
>>> On Tue, 2007-05-01 at 12:50 +0200, Jan Kiszka wrote:
>>>
>>>> Could you point me to the patch(es) in question? I only have 2MBit/s
>>>> downlink, so that downloading only the patch would speed things up here.
>>> http://git.kernel.org/?p=linux/kernel/git/linville/wireless-dev.git;a=commit;h=00a908826e778b39a802013729f7826c2d575360
>>>
>> Nope, doesn't help. And I wonder how this piece should do so: It just
>> performs a sanity check on ifconfig up, right? But the "tc qdisc del"
>> runs here _after_ wlan0 got configured.
> 
> I never managed to reproduce that part anyway. I hadn't tried removing
> the qdisc when the device is down, but when it's up I never managed
> to... odd
> 

Already tried this lethal command? Works "fine" here...

#tc qdisc del dev wmaster0 root

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: d80211: How does TX flow control work?

[Jiri Benc]

On Tue, 01 May 2007 16:18:52 +0200, Jan Kiszka wrote:
> Already tried this lethal command? Works "fine" here...
> 
> #tc qdisc del dev wmaster0 root

I cannot reproduce it too.

Could you please (_without_ applying the attached patch):

- reproduce it with the latest wireless-dev (sorry, there were some changes
  in the networking core, we need to be sure this is not fixed already)
- send the dmesg output and/or any relevant logs?

If the problem still persists, could you please try again with the
following patch applied?

(Btw, you don't need to clone wireless-dev, just make another branch in
your local git tree and pull wireless-dev into it. That way you should
download a few hundred kilobytes only.)

Thanks,

 Jiri


Subject: [PATCH] mac80211: disallow transmitting when 802.11 qdisc is removed

Signed-off-by: Jiri Benc <jbenc@suse.cz>

---
 net/mac80211/ieee80211.c |    9 +++++++++
 1 files changed, 9 insertions(+)

--- mac80211.orig/net/mac80211/ieee80211.c
+++ mac80211/net/mac80211/ieee80211.c
@@ -1435,6 +1435,15 @@ static int ieee80211_master_start_xmit(s
 	int headroom;
 	int ret;
 
+	if (unlikely(!ieee80211_qdisc_installed(dev))) {
+		netif_stop_queue(dev);
+		if (net_ratelimit())
+			printk(KERN_ERR "%s: ieee80211 qdisc not installed\n",
+			       dev->name);
+		dev_kfree_skb(skb);
+		return 0;
+	}
+
 	/*
 	 * copy control out of the skb so other people can use skb->cb
 	 */


-- 
Jiri Benc
SUSE Labs

Re: d80211: How does TX flow control work?

[Michael Wu]

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

On Tuesday 01 May 2007 11:25, Jiri Benc wrote:
> On Tue, 01 May 2007 16:18:52 +0200, Jan Kiszka wrote:
> > Already tried this lethal command? Works "fine" here...
> >
> > #tc qdisc del dev wmaster0 root
>
> I cannot reproduce it too.
>
I can reproduce it. I thought qdiscs couldn't be changed without taking the 
interface down first.. which is not true. It merely waits for things to quiet 
down before swapping out the qdisc.

Some code in ieee80211_master_start_xmit probably will be needed to correctly 
handle things if the ieee80211 qdisc isn't installed. I don't see an easy way 
to prevent things from working all the time if the qdisc is removed.

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Michael Wu wrote:
> On Tuesday 01 May 2007 11:25, Jiri Benc wrote:
>> On Tue, 01 May 2007 16:18:52 +0200, Jan Kiszka wrote:
>>> Already tried this lethal command? Works "fine" here...
>>>
>>> #tc qdisc del dev wmaster0 root
>> I cannot reproduce it too.
>>
> I can reproduce it.

Fine, that save /me some time.

> I thought qdiscs couldn't be changed without taking the 
> interface down first.. which is not true. It merely waits for things to quiet 
> down before swapping out the qdisc.
> 
> Some code in ieee80211_master_start_xmit probably will be needed to correctly 
> handle things if the ieee80211 qdisc isn't installed. I don't see an easy way 
> to prevent things from working all the time if the qdisc is removed.

I didn't followed the discussion in details, but wasn't there once a
suggestion to hide netdevices from the user so that there would be no
chance to stumble over this issue?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]