From 48509af4bc2a60c368c46f1351ddf5370e012bc0 Mon Sep 17 00:00:00 2001 From: Jozsef Kadlecsik Date: Wed, 23 Mar 2011 21:10:16 +0100 Subject: list:set timeout variant fixes - 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 | 53 +++++++++++++--------------- tests/setlist.t | 30 ++++++++++++++-- tests/setlist.t.restore | 10 ++++++ 3 files changed, 61 insertions(+), 32 deletions(-) create mode 100644 tests/setlist.t.restore diff --git a/kernel/net/netfilter/ipset/ip_set_list_set.c b/kernel/net/netfilter/ipset/ip_set_list_set.c index a47c329..f4a46c0 100644 --- a/kernel/net/netfilter/ipset/ip_set_list_set.c +++ b/kernel/net/netfilter/ipset/ip_set_list_set.c @@ -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); diff --git a/tests/setlist.t b/tests/setlist.t index b5e060d..6d9371d 100644 --- a/tests/setlist.t +++ b/tests/setlist.t @@ -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 @@ -51,5 +51,29 @@ # 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 index 0000000..55c145b --- /dev/null +++ b/tests/setlist.t.restore @@ -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 -- cgit v1.2.3