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

From: Patrick McHardy <[email-masked]>
Date: Fri, 25 Mar 2005 21:07:24 +0100

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: