From: Andrew Morton <hidden> Date: 2004-12-07 06:45:06
Thomas Cataldo [off-list ref] wrote:
Tonight I upgraded to 2.6.10-rc3. Everything was fine until I started
wondershaper to setup my Qos rules :
wondershaper eth0 255 16
And the machine freezed hard. No magic sysrq working, no oops in my
logs.
The computer is an x86 smp (dual p3)
wondershaper was working fine with 2.6.9.
Me too, with your .config:
Using http://lartc.org/wondershaper/wondershaper-1.1a.tar.gz
vmm:/home/akpm/wondershaper-1.1a# ./wshaper eth0 255 16
u32 classifier
Perfomance counters on
input device check on
Actions configured
Unable to handle kernel NULL pointer dereference at virtual address 00000000
printing eip:
c0290b58
*pde = 00000000
Oops: 0002 [#1]
SMP
Modules linked in: police sch_ingress cls_u32 sch_sfq sch_cbq usbcore
CPU: 1
EIP: 0060:[<c0290b58>] Not tainted VLI
EFLAGS: 00010206 (2.6.10-rc3)
EIP is at _spin_lock_bh+0x10/0x20
eax: cf690000 ebx: ce0d4d80 ecx: 00000008 edx: 00000000
esi: cf691ba0 edi: 00000000 ebp: 00000000 esp: cf691b48
ds: 007b es: 007b ss: 0068
Process tc (pid: 2743, threadinfo=cf690000 task=cf07b520)
Stack: c02372ab ce0d4d80 cd4d8800 cebd1c40 ce0d4d80 c0237346 ce0d4d80 00000008
00000000 00000000 00000000 cf691ba0 cf691ba0 c02478d6 ce0d4d80 00000008
00000000 cf691ba0 ce0d4d80 cf6a6070 30960094 cec44380 00000000 00019000
Call Trace:
[<c02372ab>] gnet_stats_start_copy_compat+0x1b/0x98
[<c0237346>] gnet_stats_start_copy+0x1e/0x24
[<c02478d6>] tcf_action_copy_stats+0x26/0xa0
[<c02473e2>] tcf_action_dump_old+0x36/0x3c
[<d0807174>] u32_dump+0x2c8/0x344 [cls_u32]
[<d08071a6>] u32_dump+0x2fa/0x344 [cls_u32]
[<c0246cf9>] tcf_fill_node+0x11d/0x170
[<c0246d9c>] tfilter_notify+0x50/0xa0
[<c0246bae>] tc_ctl_tfilter+0x542/0x570
[<c0240705>] rtnetlink_rcv+0x23d/0x360
[<c0249f08>] netlink_data_ready+0x1c/0x54
[<c02495c5>] netlink_sendskb+0x21/0x40
[<c024970b>] netlink_unicast+0xe3/0xec
[<c0249d04>] netlink_sendmsg+0x27c/0x28c
[<c02306d9>] sock_sendmsg+0xd5/0xf8
[<c02306d9>] sock_sendmsg+0xd5/0xf8
[<c01c0a70>] copy_from_user+0x30/0x60
[<c01c0a70>] copy_from_user+0x30/0x60
[<c0127cfc>] autoremove_wake_function+0x0/0x40
[<c0231db7>] sys_sendmsg+0x18f/0x1f4
[<c013c068>] handle_mm_fault+0x80/0x11c
[<c010fc3b>] do_page_fault+0x1a3/0x554
[<c01c0a70>] copy_from_user+0x30/0x60
[<c02321bc>] sys_socketcall+0x1d8/0x1f4
[<c01021cd>] sysenter_past_esp+0x52/0x71
Code: 3a 00 7e f9 fa eb e9 c3 8d 76 00 fa f0 fe 08 79 09 f3 90 80 38 00 7e f9 eb f2 c3 89 c2 b8 00 e0 ff ff 21 e0 81
<0>Kernel panic - not syncing: Fatal exception in interrupt
Somehow I don't think this is because "Performance" was misspelled ;)
tcf_act_hdr.stats_lock is NULL in tcf_action_copy_stats()
Can you do a:
tc -V
This seems to point to probably be a backward compat issue which was
overlooked in the stats.
cheers,
jamal
On Tue, 2004-12-07 at 01:44, Andrew Morton wrote:
Thomas Cataldo [off-list ref] wrote:
quoted
Tonight I upgraded to 2.6.10-rc3. Everything was fine until I started
wondershaper to setup my Qos rules :
wondershaper eth0 255 16
And the machine freezed hard. No magic sysrq working, no oops in my
logs.
The computer is an x86 smp (dual p3)
wondershaper was working fine with 2.6.9.
Me too, with your .config:
Using http://lartc.org/wondershaper/wondershaper-1.1a.tar.gz
vmm:/home/akpm/wondershaper-1.1a# ./wshaper eth0 255 16
u32 classifier
Perfomance counters on
input device check on
Actions configured
Unable to handle kernel NULL pointer dereference at virtual address 00000000
printing eip:
c0290b58
*pde = 00000000
Oops: 0002 [#1]
SMP
Modules linked in: police sch_ingress cls_u32 sch_sfq sch_cbq usbcore
CPU: 1
EIP: 0060:[<c0290b58>] Not tainted VLI
EFLAGS: 00010206 (2.6.10-rc3)
EIP is at _spin_lock_bh+0x10/0x20
eax: cf690000 ebx: ce0d4d80 ecx: 00000008 edx: 00000000
esi: cf691ba0 edi: 00000000 ebp: 00000000 esp: cf691b48
ds: 007b es: 007b ss: 0068
Process tc (pid: 2743, threadinfo=cf690000 task=cf07b520)
Stack: c02372ab ce0d4d80 cd4d8800 cebd1c40 ce0d4d80 c0237346 ce0d4d80 00000008
00000000 00000000 00000000 cf691ba0 cf691ba0 c02478d6 ce0d4d80 00000008
00000000 cf691ba0 ce0d4d80 cf6a6070 30960094 cec44380 00000000 00019000
Call Trace:
[<c02372ab>] gnet_stats_start_copy_compat+0x1b/0x98
[<c0237346>] gnet_stats_start_copy+0x1e/0x24
[<c02478d6>] tcf_action_copy_stats+0x26/0xa0
[<c02473e2>] tcf_action_dump_old+0x36/0x3c
[<d0807174>] u32_dump+0x2c8/0x344 [cls_u32]
[<d08071a6>] u32_dump+0x2fa/0x344 [cls_u32]
[<c0246cf9>] tcf_fill_node+0x11d/0x170
[<c0246d9c>] tfilter_notify+0x50/0xa0
[<c0246bae>] tc_ctl_tfilter+0x542/0x570
[<c0240705>] rtnetlink_rcv+0x23d/0x360
[<c0249f08>] netlink_data_ready+0x1c/0x54
[<c02495c5>] netlink_sendskb+0x21/0x40
[<c024970b>] netlink_unicast+0xe3/0xec
[<c0249d04>] netlink_sendmsg+0x27c/0x28c
[<c02306d9>] sock_sendmsg+0xd5/0xf8
[<c02306d9>] sock_sendmsg+0xd5/0xf8
[<c01c0a70>] copy_from_user+0x30/0x60
[<c01c0a70>] copy_from_user+0x30/0x60
[<c0127cfc>] autoremove_wake_function+0x0/0x40
[<c0231db7>] sys_sendmsg+0x18f/0x1f4
[<c013c068>] handle_mm_fault+0x80/0x11c
[<c010fc3b>] do_page_fault+0x1a3/0x554
[<c01c0a70>] copy_from_user+0x30/0x60
[<c02321bc>] sys_socketcall+0x1d8/0x1f4
[<c01021cd>] sysenter_past_esp+0x52/0x71
Code: 3a 00 7e f9 fa eb e9 c3 8d 76 00 fa f0 fe 08 79 09 f3 90 80 38 00 7e f9 eb f2 c3 89 c2 b8 00 e0 ff ff 21 e0 81
<0>Kernel panic - not syncing: Fatal exception in interrupt
Somehow I don't think this is because "Performance" was misspelled ;)
tcf_act_hdr.stats_lock is NULL in tcf_action_copy_stats()
From: Patrick McHardy <hidden> Date: 2004-12-07 17:00:30
jamal wrote:
Can you do a:
tc -V
This seems to point to probably be a backward compat issue which was
overlooked in the stats.
That's also what I thought at first. But the problem is in
tcf_action_copy_stats, it assumes a->priv has the same layout as
struct tcf_act_hdr, which is not true for struct tcf_police. This
patch rearranges struct tcf_police to match tcf_act_hdr.
Signed-off-by: Patrick McHardy <redacted>
From: Thomas Graf <tgraf@suug.ch> Date: 2004-12-07 17:07:55
* Patrick McHardy [off-list ref] 2004-12-07 17:59
jamal wrote:
quoted
Can you do a:
tc -V
This seems to point to probably be a backward compat issue which was
overlooked in the stats.
That's also what I thought at first. But the problem is in
tcf_action_copy_stats, it assumes a->priv has the same layout as
struct tcf_act_hdr, which is not true for struct tcf_police. This
patch rearranges struct tcf_police to match tcf_act_hdr.
Hehe, see my post a few minutes back. I came up with nearly the same
solution ;-> The only difference to my patch is that I don't touch
tcf_police if the action code isn't compiled.
From: Patrick McHardy <hidden> Date: 2004-12-07 17:24:06
Thomas Graf wrote:
* Patrick McHardy [off-list ref] 2004-12-07 17:59
quoted
That's also what I thought at first. But the problem is in
tcf_action_copy_stats, it assumes a->priv has the same layout as
struct tcf_act_hdr, which is not true for struct tcf_police. This
patch rearranges struct tcf_police to match tcf_act_hdr.
Hehe, see my post a few minutes back. I came up with nearly the same
solution ;-> The only difference to my patch is that I don't touch
tcf_police if the action code isn't compiled.
Either one is fine with me, although I would prefer to see
the number of ifdefs in this area going down, not up :)
Regards
Patrick
On Tue, 2004-12-07 at 12:23, Patrick McHardy wrote:
Either one is fine with me, although I would prefer to see
the number of ifdefs in this area going down, not up :)
You guys pick one or other or a mix. I run 4 base testcases for the
policer typically:
1) Old kernel, uptodate TC - MUST pass
2) old kernel, old tc (trivial - expected to pass).
3) New Kernel, uptodate TC - MUST pass
4) New Kernel, uptodate TC - MUST pass (although trivial)
Try both setting, dumping then deleting policies.
If these tests pass, please push patch to Dave.
cheers,
jamal
From: Jamal Hadi Salim <hidden> Date: 2004-12-08 04:42:06
BTW, old kernel in this case implies one that does not support tc
actions at all. So pick something like 2.4.28.
New is whatever 2.6.x with patch.
Old tc is something that for example ships with redhat
new tc is whatever one is patched.
Supplementary tests are: in 2.6.x to compile the policer
in two different ways a) via tc actions and b) using the old scheme
which is understood by "old" tc. Repeat the tests i described earlier
with b) pretending to be "old" kernel.
Infact come to think of it i would also prefer to have the suplementary
tests run as well.
If you guys have no cycles, please pass the patch to me and i will test
on the weekend.
cheers,
jamal
On Tue, 2004-12-07 at 23:27, jamal wrote:
On Tue, 2004-12-07 at 12:23, Patrick McHardy wrote:
quoted
Either one is fine with me, although I would prefer to see
the number of ifdefs in this area going down, not up :)
You guys pick one or other or a mix. I run 4 base testcases for the
policer typically:
1) Old kernel, uptodate TC - MUST pass
2) old kernel, old tc (trivial - expected to pass).
3) New Kernel, uptodate TC - MUST pass
4) New Kernel, uptodate TC - MUST pass (although trivial)
Try both setting, dumping then deleting policies.
If these tests pass, please push patch to Dave.
cheers,
jamal
From: Patrick McHardy <hidden> Date: 2004-12-08 05:17:35
Jamal Hadi Salim wrote:
BTW, old kernel in this case implies one that does not support tc
actions at all. So pick something like 2.4.28.
New is whatever 2.6.x with patch.
Old tc is something that for example ships with redhat
new tc is whatever one is patched.
Supplementary tests are: in 2.6.x to compile the policer
in two different ways a) via tc actions and b) using the old scheme
which is understood by "old" tc. Repeat the tests i described earlier
with b) pretending to be "old" kernel.
Infact come to think of it i would also prefer to have the suplementary
tests run as well.
If you guys have no cycles, please pass the patch to me and i will test
on the weekend.
I think these tests are a waste of time. struct tcf_police is not
userspace-visible, so it's highly unlikely that the tc version matters.
Why an old kernel needs to be tested is beyond me. For possible in-kernel
breakage caused by the restructuring, without CONFIG_NET_CLS_ACT,
struct tcf_police is only used in police.c, without any casts or
assumptions about layout, so I can't see what could break. With
CONFIG_NET_CLS_ACT, the only place where it is used outside of
police.c is tcf_action_copy_stats, and this is exactly what this patch
(tested) fixes.
If you still want to do these test, please use the attached patch.
Regards
Patrick
I can almost guarantee that one or more of those tests i outlined would
fail. So i would suggest a revert until the testing has been done.
cheers,
jamal
On Wed, 2004-12-08 at 00:30, David S. Miller wrote:
On Tue, 07 Dec 2004 18:23:46 +0100
Patrick McHardy [off-list ref] wrote:
quoted
Either one is fine with me, although I would prefer to see
the number of ifdefs in this area going down, not up :)
On Wed, 2004-12-08 at 00:17, Patrick McHardy wrote:
I think these tests are a waste of time. struct tcf_police is not
userspace-visible, so it's highly unlikely that the tc version matters.
Why an old kernel needs to be tested is beyond me.
Regression testing.
You need both backward and forward compatibility.
Old kernels must continue to work with new tc for the policer using the
old syntax.
new kernels must continue to work with old tc for policer management
using old syntax.
Policer existed before any tc action code was written and has a very
different layout of the structure. User tools and classifiers (accessed
from user tools) do touch that code.
These kind of tests constitute about 50% or more of my testing.
For possible in-kernel
breakage caused by the restructuring, without CONFIG_NET_CLS_ACT,
struct tcf_police is only used in police.c, without any casts or
assumptions about layout, so I can't see what could break. With
CONFIG_NET_CLS_ACT, the only place where it is used outside of
police.c is tcf_action_copy_stats, and this is exactly what this patch
(tested) fixes.
If you still want to do these test, please use the attached patch.
No rush now that its in (I also dont have time or equipment at the
moment). Lets hope no more freezes reported. When i get time i will look
into it.
cheers,
jamal
From: Thomas Graf <tgraf@suug.ch> Date: 2004-12-08 14:32:00
* jamal [off-list ref] 2004-12-08 07:31
On Wed, 2004-12-08 at 00:17, Patrick McHardy wrote:
quoted
I think these tests are a waste of time. struct tcf_police is not
userspace-visible, so it's highly unlikely that the tc version matters.
Why an old kernel needs to be tested is beyond me.
Regression testing.
You need both backward and forward compatibility.
Old kernels must continue to work with new tc for the policer using the
old syntax.
new kernels must continue to work with old tc for policer management
using old syntax.
Policer existed before any tc action code was written and has a very
different layout of the structure. User tools and classifiers (accessed
from user tools) do touch that code.
These kind of tests constitute about 50% or more of my testing.
I invested some time to ease testing since this was primarly my fault
by overlooking the special case of tcf_police.
I've put together a small testsuite allowing to easly run tests for
multiple versions of iproute2. It can be found at:
http://people.suug.ch/~tgr/iproute2/tc-testsuite.tar.gz
One simply extracts various iproute2 versions into iproute2/ and
sets KERNEL_INCLUDE if needed for older versions. 'make compile' on
the top level compiles all the versions.
The tests are defined in tests/ and are simple shell scripts and get
invoked for every iproute2 verison in iproute2 with $TC and $IP
set to the version currently being tested. The output of every test
run is stored in results/$TEST.$IPVERSION.out respectively .dmesg.
'make clean' removes all the results again.
'make liststests' lists all the available tests.
'make alltests' runs all the tests.
I've run all the tests on my patch with the following kernels and
iproute2 versions:
- 2.6.10-rc2-bk13 (actions compiled in)
- 2.6.10-rc2-bk13-no-act (old policer compiled in)
- 2.4.28-rc1-bk1
- iproute2-2.6.9-tgr (with all my patches in)
- iproute2-2.4.7
iproute-2.6.9 was sucessful with all kernels. I couldn't test with the
old 2.4.7 iproute2 yet since the syntax has changed and I need to adopt
the tests first. I will create better tests and run it on patrick's
patch when I get home.
From: Thomas Graf <tgraf@suug.ch> Date: 2004-12-08 14:59:09
* Thomas Graf [off-list ref] 2004-12-08 15:32
iproute-2.6.9 was sucessful with all kernels. I couldn't test with the
old 2.4.7 iproute2 yet since the syntax has changed and I need to adopt
the tests first. I will create better tests and run it on patrick's
patch when I get home.
I've updated the tests and all were successful:
tc-2.6.9-tgr tc-2.4.7
2.6.10-rc2-bk13* Y Y
2.6.10-rc2-bk13-no-act* Y Y
2.4.28-rc1-bk1 Y Y
* including tcf_police patch
Good stuff. Hopefully we can run these tests everytime we make changes
going forward. We cant compromise quality by handwaving on instinct.
Famous last words: "that couldnt have possibly caused a bug down there".
I will take a look at what you have and integrate my 20-30 testcases if
they are not covered over time - or may be adpat what you have to follow
how i do things or maybe i can send them to you and you can integrate
them.
iproute-2.6.9 was sucessful with all kernels. I couldn't test with the
old 2.4.7 iproute2 yet since the syntax has changed and I need to adopt
the tests first. I will create better tests and run it on patrick's
patch when I get home.
Thanks for your efforts Thomas.
cheers,
jamal
On Wed, 2004-12-08 at 09:59, Thomas Graf wrote:
* Thomas Graf [off-list ref] 2004-12-08 15:32
quoted
iproute-2.6.9 was sucessful with all kernels. I couldn't test with the
old 2.4.7 iproute2 yet since the syntax has changed and I need to adopt
the tests first. I will create better tests and run it on patrick's
patch when I get home.
I've updated the tests and all were successful:
tc-2.6.9-tgr tc-2.4.7
2.6.10-rc2-bk13* Y Y
2.6.10-rc2-bk13-no-act* Y Y
2.4.28-rc1-bk1 Y Y
* including tcf_police patch
From: Patrick McHardy <hidden> Date: 2004-12-08 16:57:56
jamal wrote:
I can almost guarantee that one or more of those tests i outlined would
fail. So i would suggest a revert until the testing has been done.
Please be more specific than an "almost guarantee" that
"one or more tests" may fail when asking to revert a patch
that fixes an easily triggerable crash. For example, point
to the code that makes you think it might fail.
Good stuff. Hopefully we can run these tests everytime we make changes
going forward. We cant compromise quality by handwaving on instinct.
Famous last words: "that couldnt have possibly caused a bug down there".
I will take a look at what you have and integrate my 20-30 testcases if
they are not covered over time - or may be adpat what you have to follow
how i do things or maybe i can send them to you and you can integrate
them.
I've only put a small cbq test case and the policer test in there for
now. I'd be happy to integrate your test cases if you send them to me.
I wil add more tests in the next days given time.
Our sysadmin had some cycles and set up a UML capable of booting
any kernel image by script so we can easly test all changes to iproute2
or the kernel on as many branches as we want. I've put in the latest
2.4/2.6 main releases, the latest bk snapshots of both and earlier releases
of both to ensure we keep some backward compatibility. All 2.6 branches
have 2 configs, one with action code and the other with the old policer.
I'm not quite sure which versions of iproute2 are being used by the
distributions so I've put the latest bk version and the ones of red hat,
suse and debian into it.
That's 36 kernel<->iproute2 combinations per test, given we add a few
dozen tests makes it hard to tell if something went wrong. I'll
probably need to add some more logic to it besides just checking if
the test script has written anything to stderr.
I'll be happy to put patches into the test cycle once they're posted
to netdev or alternatively add dave's bk tree to the list of branches
whichever is more reasonable.
From: Stephen Hemminger <hidden> Date: 2004-12-08 19:31:42
I don't know if this is related but something broke netem after 2.6.10-rc2
Now a simple request to delay 10ms ends up taking 1000ms.
Still narrowing down the changeset, but it isn't a change in netem
or tc utils since these didn't change.
On Wed, 2004-12-08 at 11:57, Patrick McHardy wrote:
jamal wrote:
quoted
I can almost guarantee that one or more of those tests i outlined would
fail. So i would suggest a revert until the testing has been done.
Please be more specific than an "almost guarantee" that
"one or more tests" may fail when asking to revert a patch
that fixes an easily triggerable crash. For example, point
to the code that makes you think it might fail.
I hope this is clear after you read the last email exchange i had with
Thomas and that you are not intentionaly trying to be annoying.
cheers,
jamal
Stephen,
Can we incoporate the tests from Thomas as part of iproute2?
I have about 20 or so i could add in. Maybe you could add some
for netem as well.
cheers,
jamal
On Wed, 2004-12-08 at 14:30, Stephen Hemminger wrote:
I don't know if this is related but something broke netem after 2.6.10-rc2
Now a simple request to delay 10ms ends up taking 1000ms.
Still narrowing down the changeset, but it isn't a change in netem
or tc utils since these didn't change.
From: Patrick McHardy <hidden> Date: 2004-12-08 21:21:54
Stephen Hemminger wrote:
I don't know if this is related but something broke netem after 2.6.10-rc2
Now a simple request to delay 10ms ends up taking 1000ms.
Still narrowing down the changeset, but it isn't a change in netem
or tc utils since these didn't change.
I can't find anything in the Changelog that looks related.
Can you give me a testcase ?
Regards
Patrick
From: Stephen Hemminger <hidden> Date: 2004-12-08 22:07:07
On Wed, 08 Dec 2004 22:21:54 +0100
Patrick McHardy [off-list ref] wrote:
Stephen Hemminger wrote:
quoted
I don't know if this is related but something broke netem after 2.6.10-rc2
Now a simple request to delay 10ms ends up taking 1000ms.
Still narrowing down the changeset, but it isn't a change in netem
or tc utils since these didn't change.
I can't find anything in the Changelog that looks related.
Can you give me a testcase ?
I think the problem was a missing poke (qdisc_restart) in the netem timer
routine, it probably worked earlier for me on other hardware because I was
using different hardware that was waking up and checking tx in response
to received packets.
From: Patrick McHardy <hidden> Date: 2004-12-08 22:26:46
Stephen Hemminger wrote:
I think the problem was a missing poke (qdisc_restart) in the netem timer
routine, it probably worked earlier for me on other hardware because I was
using different hardware that was waking up and checking tx in response
to received packets.
Yes, that seems likely. Since netem doesn't account for the delayed packets
in sch->q.qlen it will only be woken up while non-delayed packets are
queued.
Regards
Patrick