Skip to contentSkip to conversation

tc filter not show last u32 filter

RFC PATCH In review Vitaly E. Lavrov 3 participants 4 messages Apr 21, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 tc filter not show last u32 filter

"tc filter show" does not show last U32 filter on 32-bit systems (tested on x86).

Additional condition: filter does not have action and CONFIG_NET_CLS_ACT=y

Example: tc filter add dev eth0 parent 1:0 protocol ip prio 100 u32 match ip dst 10.200.2.2 flowid 1:20

Function tcf_action_copy_stats() returns an error because the (struct tc_action *)a->priv is NULL (for 32bit systems).

The sequence of calls:

u32_dump()
cls_u32.c:1009 if (tcf_exts_dump_stats(skb, &n->exts) < 0) goto nla_put_failure;

  tcf_exts_dump_stats()
     cls_api.c:606  if (tcf_action_copy_stats(skb, a, 1) < 0) return -1;
     tcf_action_copy_stats()
        act_api.c:604 struct tcf_common *p = a->priv;
        act_api.c:606 if (p == NULL) goto errout; // return -1;

One of variants correcting this error is a verify the existence of action before calling tcf_action_copy_stats().

Patch for kernel 3.18.10

net/sched/cls_api.c +5 -3
index aad6a67..8e7ad61 100644
@@ -602,9 +602,11 @@ EXPORT_SYMBOL(tcf_exts_dump);
 int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts)
 {
 #ifdef CONFIG_NET_CLS_ACT
-       struct tc_action *a = tcf_exts_first_act(exts);
-       if (tcf_action_copy_stats(skb, a, 1) < 0)
-               return -1;
+       if(tcf_exts_is_available(exts)) {
+               struct tc_action *a = tcf_exts_first_act(exts);
+               if (tcf_action_copy_stats(skb, a, 1) < 0)
+                       return -1;
+       }
 #endif
        return 0;
 }

This will fix the bug 84661 https://bugzilla.kernel.org/show_bug.cgi?id=84661

In 64bit system a->priv is not NULL, but is not a valid pointer, but because of a->type == 0 and compat_mode == 1 returns a value 0.

"tc filter show dev eth0".
------------------------
tc filter add dev eth0 parent 1:0 protocol ip prio 100 u32 match ip dst 10.200.2.2 flowid 1:20 tc filter add dev eth0 parent 1:0 protocol ip prio 100 u32 match ip dst 10.200.2.3 flowid 1:30 tc filter show dev eth0
....
(1) filter parent 1: protocol ip pref 100 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:20 match 0ac80202/ffffffff at 16
(2) filter parent 1: protocol ip pref 100 u32 fh 800::801 order 2049 key ht 800 bkt 0 flowid 1:30 match 0ac80203/ffffffff at 16
------------------------
64bit system
(1) a->priv == 0x80000800
(2) a->priv == 0x80000801

32bit system
(1) a->priv == 0x0
(2) a->priv == 0x0

I could not understand the reason for this difference between 32-bit and 64-bit systems.

#1 lore raw

Hello.

Quoted 19 lines · Vitaly E. Lavrov

"tc filter show" does not show last U32 filter on 32-bit systems (tested on x86).

Additional condition: filter does not have action and CONFIG_NET_CLS_ACT=y

Example: tc filter add dev eth0 parent 1:0 protocol ip prio 100 u32 match ip dst 10.200.2.2 flowid 1:20

Function tcf_action_copy_stats() returns an error because the (struct tc_action *)a->priv is NULL (for 32bit systems).

The sequence of calls:

u32_dump()
cls_u32.c:1009 if (tcf_exts_dump_stats(skb, &n->exts) < 0) goto
nla_put_failure;

   tcf_exts_dump_stats()
      cls_api.c:606  if (tcf_action_copy_stats(skb, a, 1) < 0) return -1;
Quoted 8 lines
      tcf_action_copy_stats()
         act_api.c:604 struct tcf_common *p = a->priv;
         act_api.c:606 if (p == NULL) goto errout; // return -1;

One of variants correcting this error is a verify the existence of action before calling tcf_action_copy_stats().

Patch for kernel 3.18.10

You need to send patches against the latest kernel. Look for DaveM's
'net.git' repo on git.kernel.org.

net/sched/cls_api.c EXPORT_SYMBOL(tcf_exts_dump); · line 605

Quoted 12 lines
net/sched/cls_api.c +1 -3
index aad6a67..8e7ad61 100644
@@ -602,9 +602,11 @@ EXPORT_SYMBOL(tcf_exts_dump);
  int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts)
  {
  #ifdef CONFIG_NET_CLS_ACT
-       struct tc_action *a = tcf_exts_first_act(exts);
-       if (tcf_action_copy_stats(skb, a, 1) < 0)
-               return -1;
+       if(tcf_exts_is_available(exts)) {

Please run your patches thru scripts/chackpatch.pl; space is needed after
*if*.

net/sched/cls_api.c EXPORT_SYMBOL(tcf_exts_dump); · line 606

Quoted 1 line
net/sched/cls_api.c +1 -0
+               struct tc_action *a = tcf_exts_first_act(exts);

Empty line needed here.

WBR, Sergei

#2 lore raw
Quoted 6 lines · Sergei Shtylyov

Hello.

Quoted 1 line · Vitaly E. Lavrov

"tc filter show" does not show last U32 filter on 32-bit systems (tested on x86).

Quoted 8 lines
Quoted 4 lines

Additional condition: filter does not have action and CONFIG_NET_CLS_ACT=y

Example: tc filter add dev eth0 parent 1:0 protocol ip prio 100 u32 match ip dst 10.200.2.2 flowid 1:20

You need to send patches against the latest kernel. Look for DaveM's 'net.git' repo on git.kernel.org.
Please run your patches thru scripts/chackpatch.pl; space is needed after *if*.

Thanks for the reminder about the rules.
Sorry that have not checked all branches of the kernel.

A similar patch is already exists in the git.kernel.org/linux-stable.git/master (commit b057df24a7536cce6c372efe9d0e3d1558afedf4) and linux-4.0.y.

The patch can be applied to branches of the kernel 3.14, 3.18 and 3.19. Why is this not done at once?

#3 lore raw
Quoted 9 lines · Vitaly E. Lavrov

A similar patch is already exists in the
git.kernel.org/linux-stable.git/master (commit
b057df24a7536cce6c372efe9d0e3d1558afedf4)
and linux-4.0.y.

The patch can be applied to branches of the kernel 3.14, 3.18 and 3.19. Why is this not done at once?

That commit should already have been backported to all stable kernels, so why do we need a different one?