[PATCH] PKT_SCHED: Provide compat policer stats in action policer

[PATCH] PKT_SCHED: Provide compat policer stats in action policer

[Thomas Graf]

Dave,

This should go in before 2.6.10. It fixes a forgotten case to provide
police backward compatibility statistics for old iproute2 versions
running on a new kernel with actions enabled. Should make distributions
happy with older iproute2 versions and all-included kernel configs
since they probably favour actions over plain policer.

Testing results:
  iproute2-2.4.7 on 2.6.10-rc3-bk8:
  cls-police: police creation succeeded
  cls-police: Sending 10 ICMP echo requests
  cls-police: police dumping succeeded with output:
  filter protocol ip pref 10 u32 
  filter protocol ip pref 10 u32 fh 800: ht divisor 1 
  filter protocol ip pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 10:12 
  police 3 action drop rate 2Kbit burst 10Kb mtu 2Kb 
    match 00010000/00ff0000 at 8
   Sent 420 bytes 10 pkts (dropped 0, overlimits 0)  <-- This would have been missing
  cls-police: police deletion succeeded

 iproute2-2.6.9 on 2.6.10-rc3-bk8:
 ...
  filter protocol ip pref 10 u32 
  filter protocol ip pref 10 u32 fh 800: ht divisor 1 
  filter protocol ip pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 10:12  (rule hit 10 success 10)
    match 00010000/00ff0000 at 8 (success 10 ) 
   police 0x4 rate 2000bit burst 10Kb mtu 2Kb action drop 
  ref 1 bind 1
   Sent 420 bytes 10 pkts (dropped 0, overlimits 0) 
 ...

 (Same results for fw classifier)

Signed-off-by: Thomas Graf <tgraf@suug.ch>

--- linux-2.6.10-rc3-bk7.orig/net/sched/act_api.c	2004-12-14 14:24:34.000000000 +0100
+++ linux-2.6.10-rc3-bk7/net/sched/act_api.c	2004-12-14 16:15:13.000000000 +0100
@@ -418,6 +418,7 @@
 
 int tcf_action_copy_stats (struct sk_buff *skb,struct tc_action *a)
 {
+	int err;
 	struct gnet_dump d;
 	struct tcf_act_hdr *h = a->priv;
 	
@@ -428,7 +429,14 @@
 	if (NULL == h)
 		goto errout;
 
-	if (gnet_stats_start_copy(skb, TCA_ACT_STATS, h->stats_lock, &d) < 0)
+	if (a->type == TCA_OLD_COMPAT)
+		err = gnet_stats_start_copy_compat(skb, TCA_ACT_STATS,
+			TCA_STATS, TCA_XSTATS, h->stats_lock, &d);
+	else
+		err = gnet_stats_start_copy(skb, TCA_ACT_STATS,
+			h->stats_lock, &d);
+
+	if (err < 0)
 		goto errout;
 
 	if (NULL != a->ops && NULL != a->ops->get_stats)

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[jamal]

Hopefully this makes my point to Patrick earlier about regression
testing? ;-> Good effort Thomas.

cheers,
jamal

On Wed, 2004-12-15 at 08:01, Thomas Graf wrote:
> Dave,
> 
> This should go in before 2.6.10. It fixes a forgotten case to provide
> police backward compatibility statistics for old iproute2 versions
> running on a new kernel with actions enabled. Should make distributions
> happy with older iproute2 versions and all-included kernel configs
> since they probably favour actions over plain policer.
> 
> Testing results:
>   iproute2-2.4.7 on 2.6.10-rc3-bk8:
>   cls-police: police creation succeeded
>   cls-police: Sending 10 ICMP echo requests
>   cls-police: police dumping succeeded with output:
>   filter protocol ip pref 10 u32 
>   filter protocol ip pref 10 u32 fh 800: ht divisor 1 
>   filter protocol ip pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 10:12 
>   police 3 action drop rate 2Kbit burst 10Kb mtu 2Kb 
>     match 00010000/00ff0000 at 8
>    Sent 420 bytes 10 pkts (dropped 0, overlimits 0)  <-- This would have been missing
>   cls-police: police deletion succeeded
> 
>  iproute2-2.6.9 on 2.6.10-rc3-bk8:
>  ...
>   filter protocol ip pref 10 u32 
>   filter protocol ip pref 10 u32 fh 800: ht divisor 1 
>   filter protocol ip pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 10:12  (rule hit 10 success 10)
>     match 00010000/00ff0000 at 8 (success 10 ) 
>    police 0x4 rate 2000bit burst 10Kb mtu 2Kb action drop 
>   ref 1 bind 1
>    Sent 420 bytes 10 pkts (dropped 0, overlimits 0) 
>  ...
> 
>  (Same results for fw classifier)
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> 
> --- linux-2.6.10-rc3-bk7.orig/net/sched/act_api.c	2004-12-14 14:24:34.000000000 +0100
> +++ linux-2.6.10-rc3-bk7/net/sched/act_api.c	2004-12-14 16:15:13.000000000 +0100
> @@ -418,6 +418,7 @@
>  
>  int tcf_action_copy_stats (struct sk_buff *skb,struct tc_action *a)
>  {
> +	int err;
>  	struct gnet_dump d;
>  	struct tcf_act_hdr *h = a->priv;
>  	
> @@ -428,7 +429,14 @@
>  	if (NULL == h)
>  		goto errout;
>  
> -	if (gnet_stats_start_copy(skb, TCA_ACT_STATS, h->stats_lock, &d) < 0)
> +	if (a->type == TCA_OLD_COMPAT)
> +		err = gnet_stats_start_copy_compat(skb, TCA_ACT_STATS,
> +			TCA_STATS, TCA_XSTATS, h->stats_lock, &d);
> +	else
> +		err = gnet_stats_start_copy(skb, TCA_ACT_STATS,
> +			h->stats_lock, &d);
> +
> +	if (err < 0)
>  		goto errout;
>  
>  	if (NULL != a->ops && NULL != a->ops->get_stats)
> 
> 

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[Thomas Graf]

* jamal <1103119774.1077.74.camel@jzny.localdomain> 2004-12-15 09:09
> 
> Hopefully this makes my point to Patrick earlier about regression
> testing? ;-> Good effort Thomas.

I generally agree with you but I also understand patrick's point. The
testsuite doesn't replace manual testing for explict changes but rather
helps to catch the easly forgotten cases.

I made some significant changes to the testsuite and I'm still adding
lots of tests. I already found a few minor bugs such as unhandled policer
results, unhandled error cases in action configuration which I will
push patches for once 2.6.10 is out.

The architecture currently looks like this:

 qdisc specific and generic tests
 foreach classful-qdisc
   classifier specific and generic tests
   foreach classifier
     action/policer/indev tests

generic tests include:
 - add,dump,delete,add,delete basic tests
 - stress tests (adding/dumping/deleting a few hundred objects)
 - statistics tests

So for example indev is tested K*I*N*M times where K is the number of
kernels available in my testing enviroment, I is the number of
iproute2 versions, N is the number of classful qdiscs and M is the
number of classifiers supporting indev.  This helps to catch bugs
specific to a certain module and can be used to check consistency
between various iproute2 modules.

/proc/conf.gz gets splitted up and is provided to the test scripts
as enviroment variables so they can easly check if the kernel is
capable of for example indev and if not check if it properly returns
an error code.

I will publish the latest testsuite as soon as I've finished writing
the most urgent tests.

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[Patrick McHardy]

jamal wrote:

>Hopefully this makes my point to Patrick earlier about regression
>testing? ;-> Good effort Thomas.
>  
>
I value regression testing, but I prefer to use my eyes first. Since
this problem is not related to the policer oops fix it doesn't
convince me that my time would have been well invested doing the
tests you described.

Regards
Patrick

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[jamal]

On Wed, 2004-12-15 at 10:42, Patrick McHardy wrote:

> I value regression testing, but I prefer to use my eyes first. 
> Since
> this problem is not related to the policer oops fix it doesn't
> convince me that my time would have been well invested doing the
> tests you described.

But it is _absolutely_ related to the policer oops.
If those tests were run to begin with there would be no oops neither
this latest problem. 
Hopefully with the regression tests in place this will get better.

[You fear Murphy less than i - and thats a style difference. Your style
is actually more effective in Linux because you can distribute the
burden onto users. As a matter of fact it is within Daves tolerance
range (but not mine[1]). So you should do just fine]

cheers,
jamal

[1]I am not saying anyone can write perfect code (Alexey was close;->),
but if there are known things to check for, such as those i proposed
(and have been checking on for a long time), trusting your eyes maybe
insufficient.

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[Patrick McHardy]

jamal wrote:
> On Wed, 2004-12-15 at 10:42, Patrick McHardy wrote:
> 
>>Since
>>this problem is not related to the policer oops fix it doesn't
>>convince me that my time would have been well invested doing the
>>tests you described.
> 
> 
> But it is _absolutely_ related to the policer oops.
> If those tests were run to begin with there would be no oops neither
> this latest problem. 

I agree that this problem would have been avoided if the
regression tests were run when the change was made, and it
made sense to run them at that time. Unfortunately I missed
the patch when it went in, otherwise I would have objected
to using a field called "priv" and making assumptions about
the layout of the structure it points to in a file called
act_api anyway.

But reshuffling structure members of a structure not exposed
to userspace doesn't require testing tc, and doesn't require
testing old kernels. This was the only point I was trying to
make, I run tests when they make sense, but not because I might
find something unrelated by accident. As an exception to this,
I am willing to run unrelated tests if it is little overhead
(== fully automatic).

Even following your logic (We cant compromise quality by
handwaving on instinct. Famous last words: "that couldnt have
possibly caused a bug down there") I need to either test the
entire kernel after each change ("down there" could be
anywhere), or judge for myself which parts to test. Blindly
running regression tests isn't going to do much good.

> Hopefully with the regression tests in place this will get better.

On a side-note, you both seem to be inventing your own testing
framework and regression tests. tcng already includes lots of
regression tests for tc, tcng and the kernel. Unfortunately,
last time I checked, it didn't work with 2.6.

> [You fear Murphy less than i - and thats a style difference. Your style
> is actually more effective in Linux because you can distribute the
> burden onto users. As a matter of fact it is within Daves tolerance
> range (but not mine[1]). So you should do just fine]

I don't feel like I'm distributing burden onto anyone. As I
said, I run the tests I deem necessary, and I never send out
patches of whichs correctness I'm not convinced. So far, my
history of mistakes has been pretty good.

Regards
Patrick

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[Thomas Graf]

* Patrick McHardy <41C6A6CC.1050105@trash.net> 2004-12-20 11:17
> I agree that this problem would have been avoided if the
> regression tests were run when the change was made, and it
> made sense to run them at that time. Unfortunately I missed
> the patch when it went in, otherwise I would have objected
> to using a field called "priv" and making assumptions about
> the layout of the structure it points to in a file called
> act_api anyway.

Ifs and buts, it was solely and purely my fault. period.

> On a side-note, you both seem to be inventing your own testing
> framework and regression tests. tcng already includes lots of
> regression tests for tc, tcng and the kernel. Unfortunately,
> last time I checked, it didn't work with 2.6.

The tests are based on tcsim which might behave differently than
the kernel itself. My test framework primarly tries to cover
iproute - kernel incompatbilities and tries to trigger bugs
by running every bit of code in every possible combination.

> I don't feel like I'm distributing burden onto anyone. As I
> said, I run the tests I deem necessary, and I never send out
> patches of whichs correctness I'm not convinced. So far, my
> history of mistakes has been pretty good.

Agreed, the only bug which would have been easly found with the
testframework was the CBQ slab corruption bug due deleting
filters twice added with your generic filter deletion simplification.
Unfortunately, I'm more error-prone than most so I need a testframework.

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[jamal]

On Mon, 2004-12-20 at 09:03, Thomas Graf wrote:

> 
> Ifs and buts, it was solely and purely my fault. period.
> 

No fault on anyone - its a process of improving what we do.

> Agreed, the only bug which would have been easly found with the
> testframework was the CBQ slab corruption bug due deleting
> filters twice added with your generic filter deletion simplification.
> Unfortunately, I'm more error-prone than most so I need a testframework.

I need it to ;-> It will help me be less conservative ;->
Thomas, we need automation of these tests - I just relaized what you
have is not really in that category. Refer to sample example i posted.
Doesnt have to be exactly like that, but somewhere along those lines.

cheers,
jamal

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[jamal]

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

On Mon, 2004-12-20 at 05:17, Patrick McHardy wrote:

> > Hopefully with the regression tests in place this will get better.
> 
> On a side-note, you both seem to be inventing your own testing
> framework and regression tests. tcng already includes lots of
> regression tests for tc, tcng and the kernel. Unfortunately,
> last time I checked, it didn't work with 2.6.

I havent looked closely at tcng although Werner has showed it to me a
few times (may be under influence). We need to pick one or other test
setup. I dont care if its what I have, tcng or what Thomas has.
I just stared quickly at what Thomas has and realize its not really
automated. In my case it is easier because i can click on the proverbial
one-button and run 20 tests (including a subset of the policer ones)
and even capturing tcpdumps. I have attached a sample testcase. 
They are harder to create and require the environment i have.
But once you create them, you should be saying "go" - go do something
and come back and get results.
Whatever we end up having, my preference would be something along those
lines,

> > [You fear Murphy less than i - and thats a style difference. Your style
> > is actually more effective in Linux because you can distribute the
> > burden onto users. As a matter of fact it is within Daves tolerance
> > range (but not mine[1]). So you should do just fine]
> 
> I don't feel like I'm distributing burden onto anyone. As I
> said, I run the tests I deem necessary, and I never send out
> patches of whichs correctness I'm not convinced. So far, my
> history of mistakes has been pretty good.

You have been excellent. My statements are more generic than reflect on
you in person. We have probably the highest amount of visible bugs in a
longtime so i hope you understand my concerns.

cheers,
jamal

[-- Attachment #2: M-005-set.tcl --]
[-- Type: text/x-tcl, Size: 3094 bytes --]

#!/usr/bin/expect
#
# Pedit Main Test 5
#
# Purpose:  
# To test the pedit/munge/set capabilities.
#
# What it does:
# -Run tcpdump beforehand, and capture a hex dump of a packet.
# -Install a rule stating that any packet that arrives on loopback
# (ingress) and is destined for 127.0.0.1 will be edited.  Offset 0
# will be rewritten to 0x12345678
# -Run tcpdump afterwards, and check the differences between packets
#
# Expected result:
# -The ping will fail, and offset 0 of the packet will contain 0x12345678
#


# Set description for use with libtest
set libtest_description "Pedit action: setting a u32 range at offset 0."
 
# Source common test procedures and run set-up commands
source ./libtest.tcl

# Delete any ingress qdiscs associated with loopback in order to clear state
send "tc qdisc del dev lo ingress\r"

# Send initial pings to establish a base case
send "sleep 1;  ping 127.0.0.1 -c 3 &\r "

# Get a tcpdump hex output for a packet
send "tcpdump -i lo -s 96 -vvv -x host 127.0.0.1 and icmp\[icmptype\]=icmp-echo -c 1\r"
if [expect_tcpdump_results before_list] {
	output_test_failure "Error while expecting tcpdump results"
	return -1
} 

# Add an ingress qdisc
send "tc qdisc add dev lo ingress\r"
 
# Match IP destination 127.0.0.1, and edit these packets to set the 4 octets
# at offset 0 to be 0x12345678
send "tc filter add dev lo parent ffff: protocol ip prio 10 u32 match ip dst 127.0.0.1/32 flowid 1:1 action pedit munge offset 0 u32 set 0x12345678 pipe action mirred egress redirect dev dummy0\r"

# Send a few pings to 127.0.0.1 while tcpdump is running on dummy0 and capture
# the data.
expect -re ".*"
send "sleep 1;  ping 127.0.0.1 -c 2 &\r"

# Get a tcpdump hex output for a packet
send "tcpdump -i dummy0 -s 96 -vvv -x -c 1\r"
if [expect_tcpdump_results after_list] {
        output_test_failure "Error while expecting tcpdump results"
        return -1
} 

# Extra logging
output_test_message "Before: $before_list"
output_test_message "After:  $after_list"

# Check the results.  In this case, data at offsets 0-3 was changed.

# Offsets 0-1 should be 0x1234
if {[lindex $before_list 0] == [lindex $after_list 0]} {
	output_test_failure "Test failed at offsets 0-1"
	return -1
} else {
	if {[lindex $after_list 0] != "1234"} {
		output_test_failure "Test failed at offsets 0-1."
		return -1
	} else {
		output_test_message "Offsets 0-1 changed to 0x1234 (success)"
	}
}

# Offsets 2-3 should be 0x5678
if {[lindex $before_list 1] == [lindex $after_list 1]} {
	output_test_failure "Test failed at offsets 2-3"
	return -1
} else {
	if {[lindex $after_list 1] != "5678"} {
		output_test_failure "Test failed at offsets 2-3"
		return -1
	} else {
		output_test_message "Offsets 2-3 changed to 0x5678 (success)"
	}
}

# Ensure that all expected matching portions of the packet are identical
set exemptions [concat $DEFAULT_TCPDUMP_ICMP_EXEMPTION 0 1]
if [compare_lists $before_list $after_list $exemptions $DEFAULT_TCPDUMP_LENGTH mismatch_index] {
	output_test_failure "Test failed at $mismatch_index"
	return -1
} 

expect -re ".*"
output_test_success
return 0

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[Thomas Graf]

* jamal <1103552830.1049.355.camel@jzny.localdomain> 2004-12-20 09:27
> I just stared quickly at what Thomas has and realize its not really
> automated. In my case it is easier because i can click on the proverbial
> one-button and run 20 tests (including a subset of the policer ones)
> and even capturing tcpdumps. I have attached a sample testcase. 

I've put up a snapshot of what I have at:
http://people.suug.ch/~tgr/tmp/tc-testsuite.tar.gz

.. and a special for jamal not including iproute2 sources
so you can save a few bucks ;->
http://people.suug.ch/~tgr/tmp/tc-testsuite-jamal.tar.bz2

I'm not sure what you mean, it's all automated and actually
quite similar to what you've included. Actually, we could
simply run your testscript under my testsuite if you
replace the "tc" with a variable given to the script.

My indev test: (requires a patched iproute2 to correctly
print [ indev INDEV ] in usage text, patches follow later)

----
#!/bin/bash
# vim: ft=sh
#
source lib/generic.sh

TYPE=$1; shift
FILTER=$@

HELP=`$TC filter add ${TYPE} help 2>&1`

if [ "`echo $HELP | grep \"indev DEV\"`" ]; then
        if [ $CONFIG_NET_CLS_IND ]; then
                ts_tc "attr-indev" "filter addition" $FILTER indev "eth1"
        else
                ts_log "attr-indev: No INDEV support, checking error return value"
                RES=`$TC $FILTER indev "eth1" 2>&1`
                if [ "`echo $RES | head -1`" != \
                     "RTNETLINK answers: Operation not supported" ]; then
                        echo "Warning: Kernel did not report EOPNOTSUPP"
                fi
        fi
else
        # no support for indev, add filter anyway to not break deletion
        ts_log "ip version doesn't support indev, skipping"
        ts_tc "attr-indev" "dummy addition" $FILTER
fi
---

Here's an example run:

tgr:axs ~/dev/tc-testsuite make alltests
Running cls-testbed.t [iproute2-2.4.7/2.6.10-rc3-bk12]: FAILED
Running cls-testbed.t [iproute2-2.6.9/2.6.10-rc3-bk12]: PASS
Running dsmark.t [iproute2-2.4.7/2.6.10-rc3-bk12]: FAILED
Running dsmark.t [iproute2-2.6.9/2.6.10-rc3-bk12]: PASS
Running std-cbq.t [iproute2-2.4.7/2.6.10-rc3-bk12]: PASS
Running std-cbq.t [iproute2-2.6.9/2.6.10-rc3-bk12]: PASS
tgr:axs ~/dev/tc-testsuite 

So, 2 of them failed, let's see what happenend:
tgr:axs ~/dev/tc-testsuite cat results/cls-testbed.t.iproute2-2.4.7.out
...
Preparing classifier testbed with qdisc dsmark
cls-testbed: dsmark root qdisc creation failed:
command: iproute2/iproute2-2.4.7/tc/tc qdisc add dev lo root handle 10:0 dsmark indices 64 default_index 1 set_tc_index
stderr output:
Unknown qdisc "dsmark", hence option "indices" is unparsable
...

I'll take a closer look at tcng to see how we can put the best parts of both together.
It will definitely be no problem to integrate your expect based tests.

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[jamal]

Dont have time to look at it right now - If you can also have it
generate traffic in that framework and validate results that would
be similar to what i have.

cheers,
jamal

On Mon, 2004-12-20 at 19:16, Thomas Graf wrote:
> * jamal <1103552830.1049.355.camel@jzny.localdomain> 2004-12-20 09:27
> > I just stared quickly at what Thomas has and realize its not really
> > automated. In my case it is easier because i can click on the proverbial
> > one-button and run 20 tests (including a subset of the policer ones)
> > and even capturing tcpdumps. I have attached a sample testcase. 
> 
> I've put up a snapshot of what I have at:
> http://people.suug.ch/~tgr/tmp/tc-testsuite.tar.gz
> 
> .. and a special for jamal not including iproute2 sources
> so you can save a few bucks ;->
> http://people.suug.ch/~tgr/tmp/tc-testsuite-jamal.tar.bz2
> 
> I'm not sure what you mean, it's all automated and actually
> quite similar to what you've included. Actually, we could
> simply run your testscript under my testsuite if you
> replace the "tc" with a variable given to the script.
> 
> My indev test: (requires a patched iproute2 to correctly
> print [ indev INDEV ] in usage text, patches follow later)
> 
> ----
> #!/bin/bash
> # vim: ft=sh
> #
> source lib/generic.sh
> 
> TYPE=$1; shift
> FILTER=$@
> 
> HELP=`$TC filter add ${TYPE} help 2>&1`
> 
> if [ "`echo $HELP | grep \"indev DEV\"`" ]; then
>         if [ $CONFIG_NET_CLS_IND ]; then
>                 ts_tc "attr-indev" "filter addition" $FILTER indev "eth1"
>         else
>                 ts_log "attr-indev: No INDEV support, checking error return value"
>                 RES=`$TC $FILTER indev "eth1" 2>&1`
>                 if [ "`echo $RES | head -1`" != \
>                      "RTNETLINK answers: Operation not supported" ]; then
>                         echo "Warning: Kernel did not report EOPNOTSUPP"
>                 fi
>         fi
> else
>         # no support for indev, add filter anyway to not break deletion
>         ts_log "ip version doesn't support indev, skipping"
>         ts_tc "attr-indev" "dummy addition" $FILTER
> fi
> ---
> 
> Here's an example run:
> 
> tgr:axs ~/dev/tc-testsuite make alltests
> Running cls-testbed.t [iproute2-2.4.7/2.6.10-rc3-bk12]: FAILED
> Running cls-testbed.t [iproute2-2.6.9/2.6.10-rc3-bk12]: PASS
> Running dsmark.t [iproute2-2.4.7/2.6.10-rc3-bk12]: FAILED
> Running dsmark.t [iproute2-2.6.9/2.6.10-rc3-bk12]: PASS
> Running std-cbq.t [iproute2-2.4.7/2.6.10-rc3-bk12]: PASS
> Running std-cbq.t [iproute2-2.6.9/2.6.10-rc3-bk12]: PASS
> tgr:axs ~/dev/tc-testsuite 
> 
> So, 2 of them failed, let's see what happenend:
> tgr:axs ~/dev/tc-testsuite cat results/cls-testbed.t.iproute2-2.4.7.out
> ...
> Preparing classifier testbed with qdisc dsmark
> cls-testbed: dsmark root qdisc creation failed:
> command: iproute2/iproute2-2.4.7/tc/tc qdisc add dev lo root handle 10:0 dsmark indices 64 default_index 1 set_tc_index
> stderr output:
> Unknown qdisc "dsmark", hence option "indices" is unparsable
> ...
> 
> I'll take a closer look at tcng to see how we can put the best parts of both together.
> It will definitely be no problem to integrate your expect based tests.
> 

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[Thomas Graf]

* jamal <1103721013.1090.30.camel@jzny.localdomain> 2004-12-22 08:10
> Dont have time to look at it right now - If you can also have it
> generate traffic in that framework and validate results that would
> be similar to what i have.

Sure, no problem. Here's an example:

ts_tc "attr-police" "police creation" $FILTER police rate 2kbit buffer 10k drop

ts_log "cls-police: Sending 10 ICMP echo requests"
hping2 -c 10 -1 --fast ${DEST} &> /dev/null

ts_tc "cls-police" "police dumping" -s -s -d filter list dev $DEV parent 10:0

It could be done a lot better but I'm not aiming at replacing tcsim
but rather to complement it so we have tcsim to validate the
correctness of queueing and classification algorithms and my naive brutal
testing framework for regression and stress testing under real conditions.

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[jamal]

On Wed, 2004-12-22 at 08:32, Thomas Graf wrote:
> * jamal <1103721013.1090.30.camel@jzny.localdomain> 2004-12-22 08:10
> > Dont have time to look at it right now - If you can also have it
> > generate traffic in that framework and validate results that would
> > be similar to what i have.
> 
> Sure, no problem. Here's an example:
> 
> ts_tc "attr-police" "police creation" $FILTER police rate 2kbit buffer 10k drop
> 
> ts_log "cls-police: Sending 10 ICMP echo requests"
> hping2 -c 10 -1 --fast ${DEST} &> /dev/null
> 
> ts_tc "cls-police" "police dumping" -s -s -d filter list dev $DEV parent 10:0
> 

Good start. Now if there was two hooks which could be empty. First one
to say what to do with traffic - you have it going to /dev/null
Note that i had tcpdump capture it because i wanted to analyze it for
pedit sake at the second spot: after you finish testing.
In order to really tell whether test passed or not, you need to see
those counters and analyze them too ;-> (I dont - but it is valuable).

> It could be done a lot better but I'm not aiming at replacing tcsim
> but rather to complement it so we have tcsim to validate the
> correctness of queueing and classification algorithms and my naive brutal
> testing framework for regression and stress testing under real conditions.

I think tcsim for testing the infrastructure sounds reasonable. Whats
Mr. Almesberger's long term plans for it? CCed him.

cheers,
jamal

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[Patrick McHardy]

jamal wrote:

> I havent looked closely at tcng although Werner has showed it to me a
> few times (may be under influence). We need to pick one or other test
> setup. I dont care if its what I have, tcng or what Thomas has.
> I just stared quickly at what Thomas has and realize its not really
> automated. In my case it is easier because i can click on the proverbial
> one-button and run 20 tests (including a subset of the policer ones)
> and even capturing tcpdumps. I have attached a sample testcase. 
> They are harder to create and require the environment i have.
> But once you create them, you should be saying "go" - go do something
> and come back and get results.
> Whatever we end up having, my preference would be something along those
> lines,

tcsim has one major advantage, you can test the actual
scheduling algorithms for their behaviour under very
controlled conditions. It gave me a lot more confidence
when replacing the HFSC lists by rbtrees. But, as Thomas
notes, it does all its tests in userspace, which might
not be ideal for things besides scheduling algorithms.
So a combination of both seems to be best.

Regards
Patrick

Re: [PATCH] PKT_SCHED: Provide compat policer stats in action policer

[David S. Miller]

On Wed, 15 Dec 2004 14:01:28 +0100
Thomas Graf <tgraf@suug.ch> wrote:

> This should go in before 2.6.10. It fixes a forgotten case to provide
> police backward compatibility statistics for old iproute2 versions
> running on a new kernel with actions enabled. Should make distributions
> happy with older iproute2 versions and all-included kernel configs
> since they probably favour actions over plain policer.

Applied, thanks Thomas.

About testsuites.  Figure out what we want, and I can even put up a
BK repository for the testsuite on kernel.bkbits.net  Unlike what
some others appear to be suggesting, I see no problem with multiple
testsuites :-)