[iproute2 PATCH] tc: handle is not used without containing ':'

[iproute2 PATCH] tc: handle is not used without containing ':'

[Philipp Schönberger]

Handle is not used if it is not containing a ':' . This is
reproducible with these commands.

/sbin/tc qdisc add dev lo parent 0xffffffff handle 0x10000 pfifo limit 10

/sbin/tc qdisc show dev lo
qdisc pfifo 8001: root refcnt 2 limit 10p

Signed-off-by: Philipp Schönberger <ph.schoenberger@googlemail.com>
--- iproute2-2.6.33/tc/tc_util.c    2010-02-25 04:56:50.000000000 +0100
+++ /home/pschoen/tc_util.c    2013-06-13 18:31:00.347135625 +0200
@@ -50,9 +50,14 @@
     maj = strtoul(str, &p, 16);
     if (p == str)
         return -1;
-    maj <<= 16;
-    if (*p != ':' && *p!=0)
-        return -1;
+    if (*p != ':')
+    {
+        if(*p!=0)
+            return -1;
+        else if (TC_H_MIN(maj) != 0)
+            return -1;
+    }else
+        maj <<= 16;
 ok:
     *h = maj;
     return 0;

Re: [iproute2 PATCH] tc: handle is not used without containing ':'

[Sergei Shtylyov]

Hello.

On 06/13/2013 09:20 PM, Philipp Schönberger wrote:

> Handle is not used if it is not containing a ':' . This is
> reproducible with these commands.

> /sbin/tc qdisc add dev lo parent 0xffffffff handle 0x10000 pfifo limit 10

> /sbin/tc qdisc show dev lo
> qdisc pfifo 8001: root refcnt 2 limit 10p

> Signed-off-by: Philipp Schönberger <ph.schoenberger@googlemail.com>
> --- iproute2-2.6.33/tc/tc_util.c    2010-02-25 04:56:50.000000000 +0100
> +++ /home/pschoen/tc_util.c    2013-06-13 18:31:00.347135625 +0200
> @@ -50,9 +50,14 @@
>       maj = strtoul(str, &p, 16);
>       if (p == str)
>           return -1;
> -    maj <<= 16;
> -    if (*p != ':' && *p!=0)
> -        return -1;
> +    if (*p != ':')
> +    {
> +        if(*p!=0)

    Maybe you should be more consistent in using space after *if*? :-)

> +            return -1;
> +        else if (TC_H_MIN(maj) != 0)
> +            return -1;
> +    }else

    Space after } seems a good idea too...

> +        maj <<= 16;
>   ok:
>       *h = maj;
>       return 0;

WBR, Sergei

Re: [iproute2 PATCH] tc: handle is not used without containing ':'

[Philipp Schönberger]

Did not notice it was not formatted correct sorry :-)

Signed-off-by: Philipp Schönberger <ph.schoenberger@googlemail.com>
--- iproute2-2.6.33/tc/tc_util.c    2010-02-25 04:56:50.000000000 +0100
+++ /home/pschoen/tc_util.c    2013-06-13 18:31:00.347135625 +0200
@@ -50,9 +50,14 @@
     maj = strtoul(str, &p, 16);
     if (p == str)
         return -1;
-    maj <<= 16;
-    if (*p != ':' && *p!=0)
-        return -1;
+    if (*p != ':')
+    {
+        if (*p!=0)
+            return -1;
+        else if (TC_H_MIN(maj) != 0)
+            return -1;
+    } else
+        maj <<= 16;
 ok:
     *h = maj;
     return 0;

2013/6/13 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> Hello.
>
>
> On 06/13/2013 09:20 PM, Philipp Schönberger wrote:
>
>> Handle is not used if it is not containing a ':' . This is
>> reproducible with these commands.
>
>
>> /sbin/tc qdisc add dev lo parent 0xffffffff handle 0x10000 pfifo limit 10
>
>
>> /sbin/tc qdisc show dev lo
>> qdisc pfifo 8001: root refcnt 2 limit 10p
>
>
>> Signed-off-by: Philipp Schönberger <ph.schoenberger@googlemail.com>
>> --- iproute2-2.6.33/tc/tc_util.c    2010-02-25 04:56:50.000000000 +0100
>> +++ /home/pschoen/tc_util.c    2013-06-13 18:31:00.347135625 +0200
>> @@ -50,9 +50,14 @@
>>       maj = strtoul(str, &p, 16);
>>       if (p == str)
>>           return -1;
>> -    maj <<= 16;
>> -    if (*p != ':' && *p!=0)
>> -        return -1;
>> +    if (*p != ':')
>> +    {
>> +        if(*p!=0)
>
>
>    Maybe you should be more consistent in using space after *if*? :-)
>
>
>> +            return -1;
>> +        else if (TC_H_MIN(maj) != 0)
>> +            return -1;
>> +    }else
>
>
>    Space after } seems a good idea too...
>
>
>> +        maj <<= 16;
>>   ok:
>>       *h = maj;
>>       return 0;
>
>
> WBR, Sergei
>
>

Re: [iproute2 PATCH] tc: handle is not used without containing ':'

[Stephen Hemminger]

On Thu, 13 Jun 2013 19:20:52 +0200
Philipp Schönberger  <ph.schoenberger@googlemail.com> wrote:

> Handle is not used if it is not containing a ':' . This is
> reproducible with these commands.
> 
> /sbin/tc qdisc add dev lo parent 0xffffffff handle 0x10000 pfifo limit 10
> 
> /sbin/tc qdisc show dev lo
> qdisc pfifo 8001: root refcnt 2 limit 10p
> 
> Signed-off-by: Philipp Schönberger <ph.schoenberger@googlemail.com>
> --- iproute2-2.6.33/tc/tc_util.c    2010-02-25 04:56:50.000000000 +0100
> +++ /home/pschoen/tc_util.c    2013-06-13 18:31:00.347135625 +0200
> @@ -50,9 +50,14 @@
>      maj = strtoul(str, &p, 16);
>      if (p == str)
>          return -1;
> -    maj <<= 16;
> -    if (*p != ':' && *p!=0)
> -        return -1;
> +    if (*p != ':')
> +    {
> +        if(*p!=0)
> +            return -1;
> +        else if (TC_H_MIN(maj) != 0)
> +            return -1;
> +    }else
> +        maj <<= 16;
>  ok:
>      *h = maj;
>      return 0;
> --
> 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

The syntax for tc has always reqired : after the handle.
 http://www.lartc.org/howto/lartc.qdisc.classful.html

One could argue the error checking could be better, but not worth changing
the syntax now.

Re: [iproute2 PATCH] tc: handle is not used without containing ':'

[Philipp Schönberger]

Then the interface of class and handle is not equal.
You take a look at the classid which is also represented in an '__u32'
with major and minor part  with 16 bit for each.
The following class Ids are pointing to the same class and working
correctly with the tc command

10:0010
1:10
1:0010
0x00100010

So correct me if i'm wrong but this should the same case for the
handle, only that the minor part has to be zero.
The code does not restrict to not have a ':' in the handle it only add
the possibility to also enter it without ':' like the classid and
check if the minor part is zero.

WBR, Philipp

2013/6/18 Stephen Hemminger <stephen@networkplumber.org>:
> On Thu, 13 Jun 2013 19:20:52 +0200
> Philipp Schönberger  <ph.schoenberger@googlemail.com> wrote:
>
>> Handle is not used if it is not containing a ':' . This is
>> reproducible with these commands.
>>
>> /sbin/tc qdisc add dev lo parent 0xffffffff handle 0x10000 pfifo limit 10
>>
>> /sbin/tc qdisc show dev lo
>> qdisc pfifo 8001: root refcnt 2 limit 10p
>>
>> Signed-off-by: Philipp Schönberger <ph.schoenberger@googlemail.com>
>> --- iproute2-2.6.33/tc/tc_util.c    2010-02-25 04:56:50.000000000 +0100
>> +++ /home/pschoen/tc_util.c    2013-06-13 18:31:00.347135625 +0200
>> @@ -50,9 +50,14 @@
>>      maj = strtoul(str, &p, 16);
>>      if (p == str)
>>          return -1;
>> -    maj <<= 16;
>> -    if (*p != ':' && *p!=0)
>> -        return -1;
>> +    if (*p != ':')
>> +    {
>> +        if(*p!=0)
>> +            return -1;
>> +        else if (TC_H_MIN(maj) != 0)
>> +            return -1;
>> +    }else
>> +        maj <<= 16;
>>  ok:
>>      *h = maj;
>>      return 0;
>> --
>> 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
>
> The syntax for tc has always reqired : after the handle.
>  http://www.lartc.org/howto/lartc.qdisc.classful.html
>
> One could argue the error checking could be better, but not worth changing
> the syntax now.