On Mon, 2006-13-03 at 14:44 +1000, Russell Stuart wrote: 
> On Sat, 2006-03-11 at 08:11 -0500, jamal wrote: 
> > > On my machine tc does not parse filter "sample" for the u32
> > > filter.  Eg:
> > > 
> > > tc filter add dev eth2 parent 1:0 protocol ip prio 1 u32 ht 801: \ 
> > >     classid 1:3 \
> > >     sample ip protocol 1 0xff match ip protocol 1 0xff
> > >   Illegal "sample"
> > > 
> > 
> > The syntax is correct but ht 801: must exist - that is why it is being
> > rejected.. So there is absolutely categorically _no way in hell_ your
> > memset would have fixed it ;-> Apologies for being overdramatic ;->
> 
> You are wrong on both counts.

I am wrong on why it is being rejected - but what you are seeing is
worse than i thought initially. 

Lets put it this way:
The only you will _ever_ get that message is if you had made a syntax
error (which you did not). Please look at the code on where that message
appears and:

a) tell me how you would have got that message to begin with using
perfectly legal syntax.
a) tell me how a memset would have fixed that.


> Firstly, the error message came from tc when it parsed
> the line.  If tc gets an error talking to the kernel it 
> says, among other things:
>   "We have an error talking to the kernel"

Yes, I added that message to tc - I should know. And thats what you
should have got because nothing is wrong with your syntax.

Again, it does not make sense whatsover that you got a message saying it
was "illegal sample" (dont make me go overdramatic again ;->), heres my
laptop:

# tc -V
tc utility, iproute2-ss041019
# tc qdisc add dev eth0 ingress

# tc filter add dev eth0 parent ffff: protocol ip prio 1 u32 ht 801:
classid 1:3 sample ip protocol 1 0xff match ip protocol 1 0xff
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
#tc filter add dev eth0 parent ffff: protocol ip prio 10 handle 801::
u32 divisor 256
# tc filter add dev eth0 parent ffff: protocol ip prio 1 u32 ht 801:
classid 1:3 sample ip protocol 1 0xff match ip protocol 1 0xff

now it works.

Are you sure you didnt have your own changes in there that might have
contributed that?

> Of course on some machines (perhaps yours?)
> that random crap might be 0, and then it would work.  
> That is why I said at the start of my patch "On my 
> machine tc does not ...".  On other machines the bug
> may not appear.
> 

That maybe a different bug not what you stated. Please be a little more
reasonable - look at the code.
Just send the memset fix to Stephen with a different reason. Your
current reason is _wrong_ and i really dont have much time to have this
kind of discussion.  
If you had said "I added that memset there because it looks like the
right thing to do" then we would not have had this discussion.

You made claims you fixed a bug. It cant possibly be the bug you fixed.
Was it some other bug perhaps and you mixed up the two?

> Re: "sample never worked 100% of the time with that 
> hash".  Can you give an example?  As far as I know it
> always worked.
> 

Off top of my head i was using something along 3 bits mask.
I have to find/look at my notes for an example from about 2 years ago
when i did the SUCON presentation
[http://www.suug.ch/sucon/04/slides/pkt_cls.pdf]. Both sample and
hashkey depend on the hash - I used both at the time for that
presentation; if i find my notes i will give you a precise answer.

> Re: "it will work some of the time as it is right now 
> with 2.6.x".  Yes - it will work when you are sampling
> one byte. &#-1;&#-1; 
> I am sampling port numbers, which are two
> bytes.  It will not work in any case where there are
> two non-zero bytes.
> 

yes, this is another problem. So the issue was it works great if you had
an 8 bit selection but was unpredictable if you didnt.
The end goal is you want the hash to spread incoming selection evenly
into different buckets.

> Re: "Can you post the output of tc -s filter ls on 2.6 
> with and without your user space change?".  Here it is:
> 
>   With my change:
>     tc qdisc add dev eth0 root handle 1: htb
>     tc filter add dev eth0 parent 1:0 prio 1 protocol ip u32 ht 801: divisor 
> 256
>     tc filter add dev eth0 parent 1:0 protocol ip prio 1 u32 ht 801: classid 
> 1:0 sample tcp src 0x369 0xffff match tcp src 0x0369 0xffff
>     tc -s filter show dev eth0
>     filter parent 1: protocol ip pref 1 u32
>     filter parent 1: protocol ip pref 1 u32 fh 801: ht divisor 256
>     filter parent 1: protocol ip pref 1 u32 fh 801:3:800 order 2048 key ht 
> 801 bkt 3 flowid 1:
>       match 03690000/ffff0000 at nexthdr+0
>     filter parent 1: protocol ip pref 1 u32 fh 800: ht divisor 1
> 
>   On the orginal "tc" shipped with debian "sarge":
>     tc qdisc add dev eth0 root handle 1: htb
>     tc filter add dev eth0 parent 1:0 prio 1 protocol ip u32 ht 801: divisor 
> 256
>     tc filter add dev eth0 parent 1:0 protocol ip prio 1 u32 ht 801: classid 
> 1:0 sample tcp src 0x369 0xffff match tcp src 0x0369 0xffff
>     Illegal "sample"
>   Ooops.  Looks like I can't get out of this without patching
>   and compiling:

Now we may be getting to the core of the problem -  whatever is being
shipped by sarge maybe the problem. Can you check what version you have?
And whether someone did make any changes to it.

Heres what i have on my laptop (same version as before):
[EMAIL PROTECTED]:~ # tc qdisc add dev eth0 root handle 1: htb
[EMAIL PROTECTED]:~ # tc filter add dev eth0 parent 1:0 prio 1 protocol ip u32
ht 801: divisor 256
[EMAIL PROTECTED]:~ # tc filter add dev eth0 parent 1:0 protocol ip prio 1 u32
ht 801: classid 1:0 sample tcp src 0x369 0xffff match tcp src 0x0369
0xffff

[EMAIL PROTECTED]:~ # tc -s filter show dev eth0
filter parent 1: protocol ip pref 1 u32
filter parent 1: protocol ip pref 1 u32 fh 801: ht divisor 256
filter parent 1: protocol ip pref 1 u32 fh 801:6a:800 order 2048 key ht
801 bkt 6a flowid 1:
  match 03690000/ffff0000 at nexthdr+0
filter parent 1: protocol ip pref 1 u32 fh 800: ht divisor 1
[EMAIL PROTECTED]:~ #

And believe me i dont have your patch ;->

>     tc qdisc add dev eth0 root handle 1: htb
>     tc filter add dev eth0 parent 1:0 prio 1 protocol ip u32 ht 801: divisor 
> 256
>     tc filter add dev eth0 parent 1:0 protocol ip prio 1 u32 ht 801: classid 
> 1:0 sample tcp src 0x369 0xffff match tcp src 0x0369 0xffff
>     tc -s filter show dev eth0filter parent 1: protocol ip pref 1 u32
>     filter parent 1: protocol ip pref 1 u32 fh 801: ht divisor 256
>     filter parent 1: protocol ip pref 1 u32 fh 801:6a:800 order 2048 key ht 
> 801 bkt 6a flowid 1:
>       match 03690000/ffff0000 at nexthdr+0
>     filter parent 1: protocol ip pref 1 u32 fh 800: ht divisor 1
> 
> Note: this also answers you request in your other email re:
> "can you give me an example that doesn't work".
> 

I think it boils down to the version you are using.
The error you are getting has to do with the syntax 
"tcp src 0x369 0xffff" being rejected. It has nothing to do with
"sample". I bet you would have the same problem with "match" using that
syntax. 

I dont have time to read the rest of your email. [I normally have a
budget for time to respond to emails in the morning and i am afraid you
have just consumed all that time].

Lets resolve the issue above first and then we can come back to the rest
of your email. I dont think you have found the real cause for what you
are seeing.

Get the latest iproute2 and try to see if you see the same thing. I will
get it as well then we can sync.
I would like to help, but lets take it one step at a time.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to