Skip to contentSkip to conversation

d80211: How does TX flow control work?

DISCUSSION Jan Kiszka 5 participants 21 messages Jan 2, 2007 – May 1, 2007

Subsystems (3)
Networking Network drivers mac80211
lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

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

#1 lore raw
Quoted 4 lines · Jan Kiszka

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.

Quoted 3 lines

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).

Quoted 3 lines

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.

Quoted 4 lines

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

Signature · 2 lines
Jiri Benc
SUSE Labs
author
#2 lore raw
Quoted 24 lines · Jiri Benc
Quoted 5 lines · Jan Kiszka

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.

Quoted 4 lines

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).

Quoted 8 lines
Quoted 4 lines

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.

Quoted 10 lines
Quoted 5 lines

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

#3 lore raw
Quoted 17 lines · Jan Kiszka
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
Quoted 2 lines

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

Signature · 2 lines
Jiri Benc
SUSE Labs
author
#4 lore raw
Quoted 18 lines · Jiri Benc
Quoted 17 lines · Jan Kiszka
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
Quoted 5 lines
Quoted 2 lines

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

author
#5 lore raw
Quoted 19 lines · Jan Kiszka
Quoted 18 lines · Jiri Benc
Quoted 17 lines · Jan Kiszka
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
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines

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

author
#6 lore raw
Quoted 20 lines · Jan Kiszka
Quoted 19 lines · Jan Kiszka
Quoted 18 lines · Jiri Benc
Quoted 17 lines · Jan Kiszka
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
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines

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.

#7 lore raw
Quoted 6 lines · Jan Kiszka

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

Signature · 2 lines
Jiri Benc
SUSE Labs
#8 lore raw

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:[email protected]]
Sent: Wednesday, January 10, 2007 6:20 PM
To: Jan Kiszka
Cc: [email protected]; Ivo Van Doorn; [email protected]; Jouni Malinen; Simon Barber
Subject: Re: d80211: How does TX flow control work?

Quoted 7 lines · Jan Kiszka

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

maintainer
#9 lore raw
In reply to Jan Kiszka
Quoted 7 lines

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

author
#10 lore raw
Quoted 9 lines · Johannes Berg
Quoted 6 lines

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:

Quoted 8 lines
# 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

author
#11 lore raw
Quoted 15 lines · Jan Kiszka
Quoted 14 lines · Johannes Berg
Quoted 6 lines

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
Quoted 8 lines
Quoted 2 lines

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

#12 lore raw
Quoted 4 lines · Jan Kiszka

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.

Quoted 8 lines

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

Signature · 2 lines
Jiri Benc
SUSE Labs
author
#13 lore raw
Quoted 8 lines · Jiri Benc
Quoted 7 lines · Jan Kiszka

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:

Quoted 7 lines
Quoted 5 lines

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

author
#15 lore raw
Quoted 7 lines · Johannes Berg
Quoted 2 lines · Jan Kiszka

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

maintainer
#16 lore raw
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Johannes Berg
Quoted 2 lines · Jan Kiszka

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.

Quoted 6 lines
Quoted 2 lines

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

author
#17 lore raw
Quoted 8 lines · Johannes Berg
Quoted 7 lines · Jan Kiszka
Quoted 6 lines · Johannes Berg
Quoted 2 lines · Jan Kiszka

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

Quoted 8 lines
Quoted 3 lines

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

#18 lore raw
Quoted 3 lines · Jan Kiszka

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 <[email protected]>
---
 net/mac80211/ieee80211.c |    9 +++++++++
 1 files changed, 9 insertions(+)
net/mac80211/ieee80211.c +9 -0
@@ -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
 	 */
Signature · 2 lines
Jiri Benc
SUSE Labs
#19 lore raw
Quoted 7 lines · Jiri Benc
Quoted 3 lines · Jan Kiszka

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

author
#20 lore raw
Quoted 8 lines · Michael Wu
Quoted 6 lines · Jiri Benc
Quoted 3 lines · Jan Kiszka

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.

Quoted 7 lines

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