]> granicus.if.org Git - ipset/commitdiff
list:set timeout variant fixes
authorJozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Wed, 23 Mar 2011 20:10:16 +0000 (21:10 +0100)
committerJozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Wed, 23 Mar 2011 20:10:16 +0000 (21:10 +0100)
- the timeout value was actually not set
- the garbage collector was broken

The variant is fixed, the tests to the testsuite are added.

kernel/net/netfilter/ipset/ip_set_list_set.c
tests/setlist.t
tests/setlist.t.restore [new file with mode: 0644]

index a47c32982f06233b48b37c386b32842cdf122614..f4a46c0d25f34e2d1d07bc5d41657865b8e9b21f 100644 (file)
@@ -43,14 +43,19 @@ struct list_set {
 static inline struct set_elem *
 list_set_elem(const struct list_set *map, u32 id)
 {
-       return (struct set_elem *)((char *)map->members + id * map->dsize);
+       return (struct set_elem *)((void *)map->members + id * map->dsize);
+}
+
+static inline struct set_telem *
+list_set_telem(const struct list_set *map, u32 id)
+{
+       return (struct set_telem *)((void *)map->members + id * map->dsize);
 }
 
 static inline bool
 list_set_timeout(const struct list_set *map, u32 id)
 {
-       const struct set_telem *elem =
-               (const struct set_telem *) list_set_elem(map, id);
+       const struct set_telem *elem = list_set_telem(map, id);
 
        return ip_set_timeout_test(elem->timeout);
 }
@@ -58,19 +63,11 @@ list_set_timeout(const struct list_set *map, u32 id)
 static inline bool
 list_set_expired(const struct list_set *map, u32 id)
 {
-       const struct set_telem *elem =
-               (const struct set_telem *) list_set_elem(map, id);
+       const struct set_telem *elem = list_set_telem(map, id);
 
        return ip_set_timeout_expired(elem->timeout);
 }
 
-static inline int
-list_set_exist(const struct set_telem *elem)
-{
-       return elem->id != IPSET_INVALID_ID &&
-              !ip_set_timeout_expired(elem->timeout);
-}
-
 /* Set list without and with timeout */
 
 static int
@@ -146,11 +143,11 @@ list_elem_tadd(struct list_set *map, u32 i, ip_set_id_t id,
        struct set_telem *e;
 
        for (; i < map->size; i++) {
-               e = (struct set_telem *)list_set_elem(map, i);
+               e = list_set_telem(map, i);
                swap(e->id, id);
+               swap(e->timeout, timeout);
                if (e->id == IPSET_INVALID_ID)
                        break;
-               swap(e->timeout, timeout);
        }
 }
 
@@ -164,7 +161,7 @@ list_set_add(struct list_set *map, u32 i, ip_set_id_t id,
                /* Last element replaced: e.g. add new,before,last */
                ip_set_put_byindex(e->id);
        if (with_timeout(map->timeout))
-               list_elem_tadd(map, i, id, timeout);
+               list_elem_tadd(map, i, id, ip_set_timeout_set(timeout));
        else
                list_elem_add(map, i, id);
 
@@ -172,11 +169,11 @@ list_set_add(struct list_set *map, u32 i, ip_set_id_t id,
 }
 
 static int
-list_set_del(struct list_set *map, ip_set_id_t id, u32 i)
+list_set_del(struct list_set *map, u32 i)
 {
        struct set_elem *a = list_set_elem(map, i), *b;
 
-       ip_set_put_byindex(id);
+       ip_set_put_byindex(a->id);
 
        for (; i < map->size - 1; i++) {
                b = list_set_elem(map, i + 1);
@@ -308,11 +305,11 @@ list_set_uadt(struct ip_set *set, struct nlattr *tb[],
                                 (before == 0 ||
                                  (before > 0 &&
                                   next_id_eq(map, i, refid))))
-                               ret = list_set_del(map, id, i);
+                               ret = list_set_del(map, i);
                        else if (before < 0 &&
                                 elem->id == refid &&
                                 next_id_eq(map, i, id))
-                               ret = list_set_del(map, id, i + 1);
+                               ret = list_set_del(map, i + 1);
                }
                break;
        default:
@@ -460,17 +457,15 @@ list_set_gc(unsigned long ul_set)
        struct list_set *map = set->data;
        struct set_telem *e;
        u32 i;
-
-       /* We run parallel with other readers (test element)
-        * but adding/deleting new entries is locked out */
-       read_lock_bh(&set->lock);
-       for (i = map->size - 1; i >= 0; i--) {
-               e = (struct set_telem *) list_set_elem(map, i);
-               if (e->id != IPSET_INVALID_ID &&
-                   list_set_expired(map, i))
-                       list_set_del(map, e->id, i);
+       
+       /* nfnl_lock should be called */
+       write_lock_bh(&set->lock);
+       for (i = 0; i < map->size; i++) {
+               e = list_set_telem(map, i);
+               if (e->id != IPSET_INVALID_ID && list_set_expired(map, i))
+                       list_set_del(map, i);
        }
-       read_unlock_bh(&set->lock);
+       write_unlock_bh(&set->lock);
 
        map->gc.expires = jiffies + IPSET_GC_PERIOD(map->timeout) * HZ;
        add_timer(&map->gc);
index b5e060d1ac924c3be246cabfe22c40b8855eb280..6d9371d7f2f3c3732f7f72549ac68b551339c860 100644 (file)
@@ -29,7 +29,7 @@
 # Test foo,after,bar
 1 ipset -T test foo,after,bar
 # Save sets
-0 ipset -S > setlist.t.restore
+0 ipset -S > setlist.t.r
 # Delete bar,before,foo
 1 ipset -D test bar,before,foo
 # Delete foo,after,bar
@@ -43,7 +43,7 @@
 # Delete all sets
 0 ipset -X
 # Restore saved sets
-0 ipset -R < setlist.t.restore
+0 ipset -R < setlist.t.r
 # List set
 0 ipset -L test > .foo
 # Check listing
 # Flush all sets
 0 ipset -F
 # Delete all sets
-0 ipset -X && rm setlist.t.restore
+0 ipset -X && rm setlist.t.r
+# Restore list:set with timeout
+0 ipset -R < setlist.t.restore
+# Add set "before" last one
+0 ipset add test e before d
+# Check reference number of the pushed off set
+0 ref=`ipset list d | grep References | sed 's/References: //'` && test $ref -eq 0
+# Try to add already added set
+1 ipset add test a
+# Check reference number of added set
+0 ref=`ipset list a | grep References | sed 's/References: //'` && test $ref -eq 1
+# Try to add already added set with exist flag
+0 ipset add test a -!
+# Check reference number of added set
+0 ref=`ipset list a | grep References | sed 's/References: //'` && test $ref -eq 1
+# Delete set from the set
+0 ipset del test a
+# Check reference number of deleted set
+0 ref=`ipset list a | grep References | sed 's/References: //'` && test $ref -eq 0
+# Sleep 10s so that entries can time out
+0 sleep 10
+# Check reference numbers of the sets
+0 ref=`ipset list | grep 'References: 1' | wc -l` && test $ref -eq 0
+# Delete all sets
+0 ipset -x
 # eof
diff --git a/tests/setlist.t.restore b/tests/setlist.t.restore
new file mode 100644 (file)
index 0000000..55c145b
--- /dev/null
@@ -0,0 +1,10 @@
+create a hash:ip
+create b hash:ip
+create c hash:ip
+create d hash:ip
+create e hash:ip
+create test list:set timeout 5 size 4
+add test a
+add test b
+add test c
+add test d