netdev
[Top] [All Lists]

Re: iptables breakage WAS(Re: dummy as IMQ replacement

To: hadi@xxxxxxxxxx
Subject: Re: iptables breakage WAS(Re: dummy as IMQ replacement
From: Patrick McHardy <kaber@xxxxxxxxx>
Date: Fri, 25 Mar 2005 21:07:24 +0100
Cc: Andy Furniss <andy.furniss@xxxxxxxxxxxxx>, Harald Welte <laforge@xxxxxxxxxxxx>, Remus <rmocius@xxxxxxxxxxxxxx>, netdev <netdev@xxxxxxxxxxx>, Nguyen Dinh Nam <nguyendinhnam@xxxxxxxxx>, Andre Tomt <andre@xxxxxxxx>, syrius.ml@xxxxxxxxxx, Damion de Soto <damion@xxxxxxxxxxxx>
In-reply-to: <42445FFE.6040408@xxxxxxxxx>
References: <1107123123.8021.80.camel@xxxxxxxxxxxxxxxx> <423B7BCB.10400@xxxxxxxxxxxxx> <1111410890.1092.195.camel@xxxxxxxxxxxxxxxx> <423F41AD.3010902@xxxxxxxxxxxxx> <1111444869.1072.51.camel@xxxxxxxxxxxxxxxx> <423F71C2.8040802@xxxxxxxxxxxxx> <1111462263.1109.6.camel@xxxxxxxxxxxxxxxx> <42408998.5000202@xxxxxxxxxxxxx> <1111550254.1089.21.camel@xxxxxxxxxxxxxxxx> <4241C478.5030309@xxxxxxxxxxxxx> <1111607112.1072.48.camel@xxxxxxxxxxxxxxxx> <4241D764.2030306@xxxxxxxxxxxxx> <1111612042.1072.53.camel@xxxxxxxxxxxxxxxx> <4241F1D2.9050202@xxxxxxxxxxxxx> <4241F7F0.2010403@xxxxxxxxxxxxx> <1111625608.1037.16.camel@xxxxxxxxxxxxxxxx> <424212F7.10106@xxxxxxxxxxxxx> <1111663947.1037.24.camel@xxxxxxxxxxxxxxxx> <1111665450.1037.27.camel@xxxxxxxxxxxxxxxx> <4242DFB5.9040802@xxxxxxxxxxxxx> <1111749220.1092.457.camel@xxxxxxxxxxxxxxxx> <1111754346.1092.480.camel@xxxxxxxxxxxxxxxx> <42444A14.3090809@xxxxxxxxx> <1111775660.1092.571.camel@xxxxxxxxxxxxxxxx> <42445FFE.6040408@xxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.6) Gecko/20050324 Debian/1.7.6-1
Patrick McHardy wrote:
tcf_dump_walker() doesn't save the number of skipped entries, but
the last order dumped, so it could dump the same entries again
and again when they exceed the room in the skb.

How about this patch? It fixes two problems:

- off-by-one while skipping entries: index is incremented before the
  comparison with s_i, so it will start dumping at entry s_i-1 instead
  of s_i
- problem described above. n_i doesn't include how many empty hash
  chains were skipped, so adding it to cb->args[0] is incorrect

Regards
Patrick
===== include/net/pkt_act.h 1.10 vs edited =====
--- 1.10/include/net/pkt_act.h  2005-01-10 22:54:01 +01:00
+++ edited/include/net/pkt_act.h        2005-03-25 20:58:28 +01:00
@@ -102,20 +102,21 @@
                p = tcf_ht[tcf_hash(i)];
 
                for (; p; p = p->next) {
-                       index++;
-                       if (index < s_i)
+                       if (index < s_i) {
+                               index++;
                                continue;
+                       }
                        a->priv = p;
                        a->order = n_i;
                        r = (struct rtattr*) skb->tail;
                        RTA_PUT(skb, a->order, 0, NULL);
                        err = tcf_action_dump_1(skb, a, 0, 0);
                        if (0 > err) {
-                               index--;
                                skb_trim(skb, (u8*)r - skb->data);
                                goto done;
                        }
                        r->rta_len = skb->tail - (u8*)r;
+                       index++;
                        n_i++;
                        if (n_i >= TCA_ACT_MAX_PRIO) {
                                goto done;
@@ -124,8 +125,7 @@
        }
 done:
        read_unlock(&tcf_t_lock);
-       if (n_i)
-               cb->args[0] += n_i;
+       cb->args[0] = index;
        return n_i;
 
 rtattr_failure:
<Prev in Thread] Current Thread [Next in Thread>