d80211: How does TX flow control work?

d80211: How does TX flow control work?

[Jan Kiszka]

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

Hi,

can someone explain how TX flow control in d80211 is supposed to work? I
failed to understand the full design so far.

What I (think to) understand is that a low-level drivers call
ieee80211_stop_queue() if they run out of buffers. That flips a
per-queue bit (IEEE80211_LINK_STATE_XOFF), prevents that any further
frame is passed to the low-level TX routine, and can cause that up to
*one* packet per queue is stored in
ieee80211_local::pending_packets[queue]. But it looks to me like nothing
prevents ieee80211_tx() being invoked even in case that there is already
some stuff in that single-packet storage.

That in turn triggers WARN_ONs in ieee80211_tx() under high load for me
(with rt2500usb). And it should also cause orphaned skbs because the
storage is overwritten in that case. Either I'm blind or something is
fishy...

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, 02 Jan 2007 14:08:21 +0100, Jan Kiszka wrote:
> What I (think to) understand is that a low-level drivers call
> ieee80211_stop_queue() if they run out of buffers. That flips a
> per-queue bit (IEEE80211_LINK_STATE_XOFF), prevents that any further
> frame is passed to the low-level TX routine,

Correct.

> and can cause that up to
> *one* packet per queue is stored in
> ieee80211_local::pending_packets[queue].

This is needed due to fragmented frames. After resume, passing of
fragments to the driver has to continue where it was stopped. Returning
the half-sent fragmented frame to the 802.11 qdisc wasn't possible
until recently (I think the conversion of master interface to native
802.11 type could allow that now - but it's probably not worth the
effort).

> But it looks to me like nothing
> prevents ieee80211_tx() being invoked even in case that there is already
> some stuff in that single-packet storage.

The 802.11 qdisc (see wme_qdiscop_dequeue) takes care of that.

> That in turn triggers WARN_ONs in ieee80211_tx() under high load for me
> (with rt2500usb). And it should also cause orphaned skbs because the
> storage is overwritten in that case. Either I'm blind or something is
> fishy...

You are most likely hitting some bug. Could you post more information
please?

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Jiri Benc wrote:

> On Tue, 02 Jan 2007 14:08:21 +0100, Jan Kiszka wrote:
>   
>> What I (think to) understand is that a low-level drivers call
>> ieee80211_stop_queue() if they run out of buffers. That flips a
>> per-queue bit (IEEE80211_LINK_STATE_XOFF), prevents that any further
>> frame is passed to the low-level TX routine,
>>     
>
> Correct.
>
>   
>> and can cause that up to
>> *one* packet per queue is stored in
>> ieee80211_local::pending_packets[queue].
>>     
>
> This is needed due to fragmented frames. After resume, passing of
> fragments to the driver has to continue where it was stopped. Returning
> the half-sent fragmented frame to the 802.11 qdisc wasn't possible
> until recently (I think the conversion of master interface to native
> 802.11 type could allow that now - but it's probably not worth the
> effort).
>
>   
>> But it looks to me like nothing
>> prevents ieee80211_tx() being invoked even in case that there is already
>> some stuff in that single-packet storage.
>>     
>
> The 802.11 qdisc (see wme_qdiscop_dequeue) takes care of that.
>
>   
Ahh, that is an interesting new piece in the puzzle.


>> That in turn triggers WARN_ONs in ieee80211_tx() under high load for me
>> (with rt2500usb). And it should also cause orphaned skbs because the
>> storage is overwritten in that case. Either I'm blind or something is
>> fishy...
>>     
>
> You are most likely hitting some bug. Could you post more information
> please?
>
>   
Test scenario is rt2500usb from the rt2x00 CVS (+my currently half-pending
series), an ASUS WL167g USB stick, and hostapd driving that stick in master
mode. As soon as I trigger the AP to send out some longer TCP stream, I get
these warnings:

BUG: warning at /usr/src/rt2x00/rt2x00/ieee80211/ieee80211.c:1256/ieee80211_tx()
 <cfa02245> ieee80211_master_start_xmit+0x105/0x430 [80211]  <c024e35d> __ip_ct_refresh_acct+0x4d/0x60
 <c024fd11> tcp_packet+0x941/0x970  <c0217442> qdisc_restart+0x92/0x100
 <c020d43d> dev_queue_xmit+0xbd/0x1a0  <cfa050d8> ieee80211_subif_start_xmit+0x468/0x480 [80211]
 <c0207dca> skb_clone+0x3a/0x1a0  <c021d16d> nf_hook_slow+0x4d/0xc0
 <c020d495> dev_queue_xmit+0x115/0x1a0  <c0226a63> ip_output+0x1c3/0x200
 <c0225740> ip_finish_output+0x0/0x180  <c022628b> ip_queue_xmit+0x36b/0x3b0
 <c0224130> dst_output+0x0/0x10  <ce9bae7d> usb_hcd_giveback_urb+0x2d/0x60 [usbcore]
 <c0237da2> tcp_v4_send_check+0x82/0xd0  <c0237da2> tcp_v4_send_check+0x82/0xd0
 <c0233244> tcp_transmit_skb+0x5e4/0x610  <c0234b36> __tcp_push_pending_frames+0x676/0x740
 <c0207f81> __alloc_skb+0x51/0x100  <c022b817> tcp_sendmsg+0x897/0x980
 <c0153fa9> core_sys_select+0x1b9/0x2b0  <c0241f1d> inet_sendmsg+0x3d/0x50
 <c0202a8f> do_sock_write+0x8f/0xa0  <c020301f> sock_aio_write+0x5f/0x70
 <c01443d3> do_sync_write+0xc3/0x100  <c01247f0> autoremove_wake_function+0x0/0x40
 <c0144ca1> vfs_write+0xa1/0x140  <c01451d3> sys_write+0x43/0x70
 <c0102ae7> syscall_call+0x7/0xb

Does it tell you anything already? Is there something I may instrument? What
could the driver do wrong to trigger such bug?

Jan



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

Re: d80211: How does TX flow control work?

[Jiri Benc]

On Wed, 03 Jan 2007 19:10:01 +0100, Jan Kiszka wrote:
> BUG: warning at /usr/src/rt2x00/rt2x00/ieee80211/ieee80211.c:1256/ieee80211_tx()
>  <cfa02245> ieee80211_master_start_xmit+0x105/0x430 [80211]  <c024e35d> __ip_ct_refresh_acct+0x4d/0x60
>  <c024fd11> tcp_packet+0x941/0x970  <c0217442> qdisc_restart+0x92/0x100
>  <c020d43d> dev_queue_xmit+0xbd/0x1a0  <cfa050d8> ieee80211_subif_start_xmit+0x468/0x480 [80211]
>  <c0207dca> skb_clone+0x3a/0x1a0  <c021d16d> nf_hook_slow+0x4d/0xc0
>  <c020d495> dev_queue_xmit+0x115/0x1a0  <c0226a63> ip_output+0x1c3/0x200
>  <c0225740> ip_finish_output+0x0/0x180  <c022628b> ip_queue_xmit+0x36b/0x3b0
>  <c0224130> dst_output+0x0/0x10  <ce9bae7d> usb_hcd_giveback_urb+0x2d/0x60 [usbcore]
>  <c0237da2> tcp_v4_send_check+0x82/0xd0  <c0237da2> tcp_v4_send_check+0x82/0xd0
>  <c0233244> tcp_transmit_skb+0x5e4/0x610  <c0234b36> __tcp_push_pending_frames+0x676/0x740
>  <c0207f81> __alloc_skb+0x51/0x100  <c022b817> tcp_sendmsg+0x897/0x980
>  <c0153fa9> core_sys_select+0x1b9/0x2b0  <c0241f1d> inet_sendmsg+0x3d/0x50
>  <c0202a8f> do_sock_write+0x8f/0xa0  <c020301f> sock_aio_write+0x5f/0x70
>  <c01443d3> do_sync_write+0xc3/0x100  <c01247f0> autoremove_wake_function+0x0/0x40
>  <c0144ca1> vfs_write+0xa1/0x140  <c01451d3> sys_write+0x43/0x70
>  <c0102ae7> syscall_call+0x7/0xb
> 
> Does it tell you anything already? Is there something I may instrument? What
> could the driver do wrong to trigger such bug?

Do you have CONFIG_NET_SCHED enabled?

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Jiri Benc wrote:
> On Wed, 03 Jan 2007 19:10:01 +0100, Jan Kiszka wrote:
>> BUG: warning at /usr/src/rt2x00/rt2x00/ieee80211/ieee80211.c:1256/ieee80211_tx()
>>  <cfa02245> ieee80211_master_start_xmit+0x105/0x430 [80211]  <c024e35d> __ip_ct_refresh_acct+0x4d/0x60
>>  <c024fd11> tcp_packet+0x941/0x970  <c0217442> qdisc_restart+0x92/0x100
>>  <c020d43d> dev_queue_xmit+0xbd/0x1a0  <cfa050d8> ieee80211_subif_start_xmit+0x468/0x480 [80211]
>>  <c0207dca> skb_clone+0x3a/0x1a0  <c021d16d> nf_hook_slow+0x4d/0xc0
>>  <c020d495> dev_queue_xmit+0x115/0x1a0  <c0226a63> ip_output+0x1c3/0x200
>>  <c0225740> ip_finish_output+0x0/0x180  <c022628b> ip_queue_xmit+0x36b/0x3b0
>>  <c0224130> dst_output+0x0/0x10  <ce9bae7d> usb_hcd_giveback_urb+0x2d/0x60 [usbcore]
>>  <c0237da2> tcp_v4_send_check+0x82/0xd0  <c0237da2> tcp_v4_send_check+0x82/0xd0
>>  <c0233244> tcp_transmit_skb+0x5e4/0x610  <c0234b36> __tcp_push_pending_frames+0x676/0x740
>>  <c0207f81> __alloc_skb+0x51/0x100  <c022b817> tcp_sendmsg+0x897/0x980
>>  <c0153fa9> core_sys_select+0x1b9/0x2b0  <c0241f1d> inet_sendmsg+0x3d/0x50
>>  <c0202a8f> do_sock_write+0x8f/0xa0  <c020301f> sock_aio_write+0x5f/0x70
>>  <c01443d3> do_sync_write+0xc3/0x100  <c01247f0> autoremove_wake_function+0x0/0x40
>>  <c0144ca1> vfs_write+0xa1/0x140  <c01451d3> sys_write+0x43/0x70
>>  <c0102ae7> syscall_call+0x7/0xb
>>
>> Does it tell you anything already? Is there something I may instrument? What
>> could the driver do wrong to trigger such bug?
> 
> Do you have CONFIG_NET_SCHED enabled?
> 

Yes. Would it make a difference /wrt to that warning when I switch it off?

Jan


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

Re: d80211: How does TX flow control work?

[Jan Kiszka]

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

Jan Kiszka wrote:
> Jiri Benc wrote:
>> On Wed, 03 Jan 2007 19:10:01 +0100, Jan Kiszka wrote:
>>> BUG: warning at /usr/src/rt2x00/rt2x00/ieee80211/ieee80211.c:1256/ieee80211_tx()
>>>  <cfa02245> ieee80211_master_start_xmit+0x105/0x430 [80211]  <c024e35d> __ip_ct_refresh_acct+0x4d/0x60
>>>  <c024fd11> tcp_packet+0x941/0x970  <c0217442> qdisc_restart+0x92/0x100
>>>  <c020d43d> dev_queue_xmit+0xbd/0x1a0  <cfa050d8> ieee80211_subif_start_xmit+0x468/0x480 [80211]
>>>  <c0207dca> skb_clone+0x3a/0x1a0  <c021d16d> nf_hook_slow+0x4d/0xc0
>>>  <c020d495> dev_queue_xmit+0x115/0x1a0  <c0226a63> ip_output+0x1c3/0x200
>>>  <c0225740> ip_finish_output+0x0/0x180  <c022628b> ip_queue_xmit+0x36b/0x3b0
>>>  <c0224130> dst_output+0x0/0x10  <ce9bae7d> usb_hcd_giveback_urb+0x2d/0x60 [usbcore]
>>>  <c0237da2> tcp_v4_send_check+0x82/0xd0  <c0237da2> tcp_v4_send_check+0x82/0xd0
>>>  <c0233244> tcp_transmit_skb+0x5e4/0x610  <c0234b36> __tcp_push_pending_frames+0x676/0x740
>>>  <c0207f81> __alloc_skb+0x51/0x100  <c022b817> tcp_sendmsg+0x897/0x980
>>>  <c0153fa9> core_sys_select+0x1b9/0x2b0  <c0241f1d> inet_sendmsg+0x3d/0x50
>>>  <c0202a8f> do_sock_write+0x8f/0xa0  <c020301f> sock_aio_write+0x5f/0x70
>>>  <c01443d3> do_sync_write+0xc3/0x100  <c01247f0> autoremove_wake_function+0x0/0x40
>>>  <c0144ca1> vfs_write+0xa1/0x140  <c01451d3> sys_write+0x43/0x70
>>>  <c0102ae7> syscall_call+0x7/0xb
>>>
>>> Does it tell you anything already? Is there something I may instrument? What
>>> could the driver do wrong to trigger such bug?
>> Do you have CONFIG_NET_SCHED enabled?
>>

Sorry, this was most probably false alarm for the official stack. The
problem now appears to be related to a patch against d80211 that is only
present in the rt2x00 CVS.

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: 2366 bytes --]

Jan Kiszka wrote:
> Jan Kiszka wrote:
>> Jiri Benc wrote:
>>> On Wed, 03 Jan 2007 19:10:01 +0100, Jan Kiszka wrote:
>>>> BUG: warning at /usr/src/rt2x00/rt2x00/ieee80211/ieee80211.c:1256/ieee80211_tx()
>>>>  <cfa02245> ieee80211_master_start_xmit+0x105/0x430 [80211]  <c024e35d> __ip_ct_refresh_acct+0x4d/0x60
>>>>  <c024fd11> tcp_packet+0x941/0x970  <c0217442> qdisc_restart+0x92/0x100
>>>>  <c020d43d> dev_queue_xmit+0xbd/0x1a0  <cfa050d8> ieee80211_subif_start_xmit+0x468/0x480 [80211]
>>>>  <c0207dca> skb_clone+0x3a/0x1a0  <c021d16d> nf_hook_slow+0x4d/0xc0
>>>>  <c020d495> dev_queue_xmit+0x115/0x1a0  <c0226a63> ip_output+0x1c3/0x200
>>>>  <c0225740> ip_finish_output+0x0/0x180  <c022628b> ip_queue_xmit+0x36b/0x3b0
>>>>  <c0224130> dst_output+0x0/0x10  <ce9bae7d> usb_hcd_giveback_urb+0x2d/0x60 [usbcore]
>>>>  <c0237da2> tcp_v4_send_check+0x82/0xd0  <c0237da2> tcp_v4_send_check+0x82/0xd0
>>>>  <c0233244> tcp_transmit_skb+0x5e4/0x610  <c0234b36> __tcp_push_pending_frames+0x676/0x740
>>>>  <c0207f81> __alloc_skb+0x51/0x100  <c022b817> tcp_sendmsg+0x897/0x980
>>>>  <c0153fa9> core_sys_select+0x1b9/0x2b0  <c0241f1d> inet_sendmsg+0x3d/0x50
>>>>  <c0202a8f> do_sock_write+0x8f/0xa0  <c020301f> sock_aio_write+0x5f/0x70
>>>>  <c01443d3> do_sync_write+0xc3/0x100  <c01247f0> autoremove_wake_function+0x0/0x40
>>>>  <c0144ca1> vfs_write+0xa1/0x140  <c01451d3> sys_write+0x43/0x70
>>>>  <c0102ae7> syscall_call+0x7/0xb
>>>>
>>>> Does it tell you anything already? Is there something I may instrument? What
>>>> could the driver do wrong to trigger such bug?
>>> Do you have CONFIG_NET_SCHED enabled?
>>>
> 
> Sorry, this was most probably false alarm for the official stack. The
> problem now appears to be related to a patch against d80211 that is only
> present in the rt2x00 CVS.

Well, I said "most probably"...

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.

Jan


PS: Tests performed on a 2.6.17 kernel, but I don't see a reason why
newer kernels should be immune.


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

Re: d80211: How does TX flow control work?

[Jiri Benc]

On Mon, 08 Jan 2007 21:18:48 +0100, Jan Kiszka 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.

Hm, we probably need a way to tell the kernel not to remove 802.11
qdisc. Jouni, Simon, is that possible or do we need to patch NET_SCHED
code?

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

RE: d80211: How does TX flow control work?

[Simon Barber]

Scratches head -- this is from memory when I was thinking about this
problem a long time ago... I think we can return an error in the qdisc
destructor function - making sure legitimate interface removal is not
the cause of the qdisc deletion first of course.

Simon 

-----Original Message-----
From: Jiri Benc [mailto:jbenc@suse.cz] 
Sent: Wednesday, January 10, 2007 6:20 PM
To: Jan Kiszka
Cc: netdev@vger.kernel.org; Ivo Van Doorn;
rt2400-devel@lists.sourceforge.net; Jouni Malinen; Simon Barber
Subject: Re: d80211: How does TX flow control work?

On Mon, 08 Jan 2007 21:18:48 +0100, Jan Kiszka 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.

Hm, we probably need a way to tell the kernel not to remove 802.11
qdisc. Jouni, Simon, is that possible or do we need to patch NET_SCHED
code?

Thanks,

 Jiri

--
Jiri Benc
SUSE Labs

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 --]