iproute2 / match / meta bug?

iproute2 / match / meta bug?

[Denys Fedoryshchenko]

Kernel 2.6.26, iproute2 latest release. All required modules is loaded.
What did i miss or doing wrong?

tc filter add dev ifb0 protocol all basic match "meta(rt_iif eq tap0)" classid 1:100 

tc -s -d filter show dev ifb0
filter parent 1: protocol all pref 49152 basic
filter parent 1: protocol all pref 49152 basic handle 0x1 flowid 1:100
  meta(-1 eq -1)

Whatever i try - it doesn't work
./tc filter add dev ifb0 protocol all basic match meta \(rt_iif eq tap0\) classid 1:100

But if i do
tc filter add dev ifb0 protocol all basic match u32\(u32 0x01020300 0xFFFFFF00 at 12\) classid 1:100

i will get everything "right"

filter parent 1: protocol all pref 49151 basic
filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
  u32(01020300/ffffff00 at 12)

Sure i will try to debug and find bug by myself, but i am not sure i can do that :-)

Re: iproute2 / match / meta bug?

[Patrick McHardy]

Denys Fedoryshchenko wrote:
> Kernel 2.6.26, iproute2 latest release. All required modules is loaded.
> What did i miss or doing wrong?
>
> tc filter add dev ifb0 protocol all basic match "meta(rt_iif eq tap0)" classid 1:100 
>
> tc -s -d filter show dev ifb0
> filter parent 1: protocol all pref 49152 basic
> filter parent 1: protocol all pref 49152 basic handle 0x1 flowid 1:100
>   meta(-1 eq -1)
>
> Whatever i try - it doesn't work
> ./tc filter add dev ifb0 protocol all basic match meta \(rt_iif eq tap0\) classid 1:100
>   

You probably need to add support for skb->iif in case the
packet didn't went through IPv4.

Re: iproute2 / match / meta bug?

[Denys Fedoryshchenko]

Well, the issue not with ifb0 only. It is just not parsing rules correctly i think:

rich tc # ./tc filter add dev ifb0 protocol all basic match meta\(protocol eq 1234\) classid 1:100
rich tc # ./tc filter add dev ifb0 protocol all basic match meta\(data_len eq 1234\) classid 1:100

filter parent 1: protocol all pref 49151 basic
filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
  meta(-1 eq 1234)

filter parent 1: protocol all pref 49152 basic
filter parent 1: protocol all pref 49152 basic handle 0x1 flowid 1:100
  meta(-1 eq 1234)



On Tuesday 29 July 2008, Patrick McHardy wrote:
> Denys Fedoryshchenko wrote:
> > Kernel 2.6.26, iproute2 latest release. All required modules is loaded.
> > What did i miss or doing wrong?
> >
> > tc filter add dev ifb0 protocol all basic match "meta(rt_iif eq tap0)" classid 1:100 
> >
> > tc -s -d filter show dev ifb0
> > filter parent 1: protocol all pref 49152 basic
> > filter parent 1: protocol all pref 49152 basic handle 0x1 flowid 1:100
> >   meta(-1 eq -1)
> >
> > Whatever i try - it doesn't work
> > ./tc filter add dev ifb0 protocol all basic match meta \(rt_iif eq tap0\) classid 1:100
> >   
> 
> You probably need to add support for skb->iif in case the
> packet didn't went through IPv4.
> 

Re: iproute2 / match / meta bug?

[Patrick McHardy]

Denys Fedoryshchenko wrote:
> Well, the issue not with ifb0 only. It is just not parsing rules correctly i think:
>
> rich tc # ./tc filter add dev ifb0 protocol all basic match meta\(protocol eq 1234\) classid 1:100
> rich tc # ./tc filter add dev ifb0 protocol all basic match meta\(data_len eq 1234\) classid 1:100
>
> filter parent 1: protocol all pref 49151 basic
> filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
>   meta(-1 eq 1234)
>
> filter parent 1: protocol all pref 49152 basic
> filter parent 1: protocol all pref 49152 basic handle 0x1 flowid 1:100
>   meta(-1 eq 1234)
>   

Works fine here with a fresh git checkout:

# tc filter add dev dummy0 protocol all parent 1: basic match 
meta\(protocol eq 1234\) classid 1:100
# tc -s -d filter show dev dummy0
filter parent 1: protocol all pref 49151 basic
filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
  meta(protocol mask 0x00000000 eq 1234)

filter parent 1: protocol all pref 49152 basic

Re: iproute2 / match / meta bug?

[Denys Fedoryshchenko]

According man of stroul

RETURN VALUE
       The strtoul() function returns either the result of the conversion or, if there was a leading minus sign, the negation of the result
       of the conversion represented as an unsigned value, unless the original (non-negated) value would overflow; in the latter case, str-
       toul() returns ULONG_MAX and sets the global variable errno to ERANGE.  Precisely the same holds  for  strtoull()  (with  ULLONG_MAX
       instead of ULONG_MAX).

ULONG_MAX is NOT LONG_MAX

So it must not work i guess (2.6.26 release).
Maybe in git it is fixed, dunno, i will check it now.


On Tuesday 29 July 2008, Patrick McHardy wrote:
> Denys Fedoryshchenko wrote:
> > Well, the issue not with ifb0 only. It is just not parsing rules correctly i think:
> >
> > rich tc # ./tc filter add dev ifb0 protocol all basic match meta\(protocol eq 1234\) classid 1:100
> > rich tc # ./tc filter add dev ifb0 protocol all basic match meta\(data_len eq 1234\) classid 1:100
> >
> > filter parent 1: protocol all pref 49151 basic
> > filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
> >   meta(-1 eq 1234)
> >
> > filter parent 1: protocol all pref 49152 basic
> > filter parent 1: protocol all pref 49152 basic handle 0x1 flowid 1:100
> >   meta(-1 eq 1234)
> >   
> 
> Works fine here with a fresh git checkout:
> 
> # tc filter add dev dummy0 protocol all parent 1: basic match 
> meta\(protocol eq 1234\) classid 1:100
> # tc -s -d filter show dev dummy0
> filter parent 1: protocol all pref 49151 basic
> filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
>   meta(protocol mask 0x00000000 eq 1234)
> 
> filter parent 1: protocol all pref 49152 basic
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: iproute2 / match / meta bug?

[Denys Fedoryshchenko]

Yes, there is typo seems, i check git also, bug is there.
No idea how it works with you :-\ Maybe on your arch ULONG_MAX == LONG_MAX

So at least patch looks like: (maybe it is incorrect, i will resend if required from work as attachment)

diff -Naur iproute2/tc/em_meta.c b/tc/em_meta.c
--- iproute2/tc/em_meta.c       2008-07-29 06:31:51.000000000 +0000
+++ b/tc/em_meta.c      2008-07-29 06:31:38.000000000 +0000
@@ -262,7 +262,7 @@
        }

        num = bstrtoul(arg);
-       if (num != LONG_MAX) {
+       if (num != ULONG_MAX) {
                obj->kind = TCF_META_TYPE_INT << 12;
                obj->kind |= TCF_META_ID_VALUE;
                *dst = (unsigned long) num;
@@ -320,7 +320,7 @@
                        a = bstr_next(a);

                        shift = bstrtoul(a);
-                       if (shift == LONG_MAX) {
+                       if (shift == ULONG_MAX) {
                                PARSE_ERR(a, "meta: invalid shift, must " \
                                    "be numeric");
                                return PARSE_FAILURE;
@@ -338,7 +338,7 @@
                        a = bstr_next(a);

                        mask = bstrtoul(a);
-                       if (mask == LONG_MAX) {
+                       if (mask == ULONG_MAX) {
                                PARSE_ERR(a, "meta: invalid mask, must be " \
                                    "numeric");
                                return PARSE_FAILURE;


because 

unsigned long bstrtoul(const struct bstr *b)
{
        char *inv = NULL;
        unsigned long l;
        char buf[b->len+1];

        memcpy(buf, b->data, b->len);
        buf[b->len] = '\0';

        l = strtoul(buf, &inv, 0);
        if (l == ULONG_MAX || inv == buf)
                return ULONG_MAX;

        return l;
}





On Tuesday 29 July 2008, Denys Fedoryshchenko wrote:
> According man of stroul
> 
> RETURN VALUE
>        The strtoul() function returns either the result of the conversion or, if there was a leading minus sign, the negation of the result
>        of the conversion represented as an unsigned value, unless the original (non-negated) value would overflow; in the latter case, str-
>        toul() returns ULONG_MAX and sets the global variable errno to ERANGE.  Precisely the same holds  for  strtoull()  (with  ULLONG_MAX
>        instead of ULONG_MAX).
> 
> ULONG_MAX is NOT LONG_MAX
> 
> So it must not work i guess (2.6.26 release).
> Maybe in git it is fixed, dunno, i will check it now.
> 
> 
> On Tuesday 29 July 2008, Patrick McHardy wrote:
> > Denys Fedoryshchenko wrote:
> > > Well, the issue not with ifb0 only. It is just not parsing rules correctly i think:
> > >
> > > rich tc # ./tc filter add dev ifb0 protocol all basic match meta\(protocol eq 1234\) classid 1:100
> > > rich tc # ./tc filter add dev ifb0 protocol all basic match meta\(data_len eq 1234\) classid 1:100
> > >
> > > filter parent 1: protocol all pref 49151 basic
> > > filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
> > >   meta(-1 eq 1234)
> > >
> > > filter parent 1: protocol all pref 49152 basic
> > > filter parent 1: protocol all pref 49152 basic handle 0x1 flowid 1:100
> > >   meta(-1 eq 1234)
> > >   
> > 
> > Works fine here with a fresh git checkout:
> > 
> > # tc filter add dev dummy0 protocol all parent 1: basic match 
> > meta\(protocol eq 1234\) classid 1:100
> > # tc -s -d filter show dev dummy0
> > filter parent 1: protocol all pref 49151 basic
> > filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
> >   meta(protocol mask 0x00000000 eq 1234)
> > 
> > filter parent 1: protocol all pref 49152 basic
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: iproute2 / match / meta bug?

[Denys Fedoryshchenko]

It seems that parser broken there (em_meta.c) too.

It won't recognize "quoted"/string parameter. For example:

'meta(dev eq "tap0")' will not work
but 
'meta(dev eq 1234)' will work

and so on... 
example is broken too, there is no more such parameter as indev i guess.

I am trying now to make it understand rt_iif as name, but seems i am stuck with parser, because it doesn't work even with things it must recognize.
And because of my weak programming skills seems it is trivial for someone to fix and implement rt_iif recognition by interface name (using ll_name_to_index i guess?)
, but it's near impossible for me.

Anybody can help? 
If not - i will try to do by myself, but it will take ages.

On Tuesday 29 July 2008, Denys Fedoryshchenko wrote:
> Yes, there is typo seems, i check git also, bug is there.
> No idea how it works with you :-\ Maybe on your arch ULONG_MAX == LONG_MAX

Re: iproute2 / match / meta bug?

[Denys Fedoryshchenko]

Code is a bit out of my understanding, but

tc/em_meta.c
       num = bstrtoul(arg);
        if (num != LONG_MAX) {
                obj->kind = TCF_META_TYPE_INT << 12;
                obj->kind |= TCF_META_ID_VALUE;
                *dst = (unsigned long) num;
                return bstr_next(arg);
        }

num is always -1 in my case

If i change
        num = bstrtoul(arg);

        if (num != -1) {
                obj->kind = TCF_META_TYPE_INT << 12;
                obj->kind |= TCF_META_ID_VALUE;
                *dst = (unsigned long) num;
                return bstr_next(arg);
        }

Everything works correctly.
example:
filter parent 1: protocol all pref 49151 basic
filter parent 1: protocol all pref 49151 basic handle 0x1 flowid 1:100
  meta(rt_iif mask 0x00000000 eq 1234)



 Seems just typo?

On Tuesday 29 July 2008, Patrick McHardy wrote:
> Denys Fedoryshchenko wrote:
> > Kernel 2.6.26, iproute2 latest release. All required modules is loaded.
> > What did i miss or doing wrong?
> >
> > tc filter add dev ifb0 protocol all basic match "meta(rt_iif eq tap0)" classid 1:100 
> >
> > tc -s -d filter show dev ifb0
> > filter parent 1: protocol all pref 49152 basic
> > filter parent 1: protocol all pref 49152 basic handle 0x1 flowid 1:100
> >   meta(-1 eq -1)
> >
> > Whatever i try - it doesn't work
> > ./tc filter add dev ifb0 protocol all basic match meta \(rt_iif eq tap0\) classid 1:100
> >   
> 
> You probably need to add support for skb->iif in case the
> packet didn't went through IPv4.
>