]> granicus.if.org Git - ipset/commitdiff
Remove unnecessary integer RCU handling and fix sparse warnings
authorJozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Thu, 27 Nov 2014 16:54:52 +0000 (17:54 +0100)
committerJozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Thu, 27 Nov 2014 16:54:52 +0000 (17:54 +0100)
kernel/include/linux/netfilter/ipset/ip_set.h
kernel/include/linux/netfilter/ipset/ip_set_compat.h.in
kernel/include/linux/netfilter/ipset/ip_set_timeout.h
kernel/net/netfilter/ipset/ip_set_bitmap_gen.h
kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c
kernel/net/netfilter/ipset/ip_set_hash_gen.h

index e640c62ec0ff2073be47e9ea7aba880dd6da26dc..08dd689c177002b25696f07719b3fafc20f93485 100644 (file)
@@ -114,10 +114,10 @@ struct ip_set_comment {
 };
 
 struct ip_set_skbinfo {
-       u32 __rcu skbmark;
-       u32 __rcu skbmarkmask;
-       u32 __rcu skbprio;
-       u16 __rcu skbqueue;
+       u32 skbmark;
+       u32 skbmarkmask;
+       u32 skbprio;
+       u16 skbqueue;
 };
 
 struct ip_set;
@@ -326,38 +326,6 @@ ip_set_update_counter(struct ip_set_counter *counter,
        }
 }
 
-/* RCU-safe assign value */
-#define IP_SET_RCU_ASSIGN(ptr, value)  \
-do {                                   \
-       /* Safe assign numeric types */ \
-       smp_wmb();                      \
-       *(ptr) = value;                 \
-} while (0)
-
-static inline void
-ip_set_rcu_assign_ulong(unsigned long *v, unsigned long value)
-{
-       IP_SET_RCU_ASSIGN(v, value);
-}
-
-static inline void
-ip_set_rcu_assign_u32(u32 *v, u32 value)
-{
-       IP_SET_RCU_ASSIGN(v, value);
-}
-
-static inline void
-ip_set_rcu_assign_u16(u16 *v, u16 value)
-{
-       IP_SET_RCU_ASSIGN(v, value);
-}
-
-static inline void
-ip_set_rcu_assign_u8(u8 *v, u8 value)
-{
-       IP_SET_RCU_ASSIGN(v, value);
-}
-
 #define ip_set_rcu_deref(t)            \
        rcu_dereference_index_check(t,  \
                rcu_read_lock_held() || rcu_read_lock_bh_held())
@@ -367,32 +335,25 @@ ip_set_get_skbinfo(struct ip_set_skbinfo *skbinfo,
                      const struct ip_set_ext *ext,
                      struct ip_set_ext *mext, u32 flags)
 {
-               mext->skbmark = ip_set_rcu_deref(skbinfo->skbmark);
-               mext->skbmarkmask = ip_set_rcu_deref(skbinfo->skbmarkmask);
-               mext->skbprio = ip_set_rcu_deref(skbinfo->skbprio);
-               mext->skbqueue = ip_set_rcu_deref(skbinfo->skbqueue);
+               mext->skbmark = skbinfo->skbmark;
+               mext->skbmarkmask = skbinfo->skbmarkmask;
+               mext->skbprio = skbinfo->skbprio;
+               mext->skbqueue = skbinfo->skbqueue;
 }
 static inline bool
 ip_set_put_skbinfo(struct sk_buff *skb, struct ip_set_skbinfo *skbinfo)
 {
-       u32 skbmark, skbmarkmask, skbprio;
-       u16 skbqueue;
-
-       skbmark = ip_set_rcu_deref(skbinfo->skbmark);
-       skbmarkmask = ip_set_rcu_deref(skbinfo->skbmarkmask);
-       skbprio = ip_set_rcu_deref(skbinfo->skbprio);
-       skbqueue = ip_set_rcu_deref(skbinfo->skbqueue);
        /* Send nonzero parameters only */
-       return ((skbmark || skbmarkmask) &&
+       return ((skbinfo->skbmark || skbinfo->skbmarkmask) &&
                nla_put_net64(skb, IPSET_ATTR_SKBMARK,
-                             cpu_to_be64((u64)skbmark << 32 |
-                                         skbmarkmask))) ||
-              (skbprio &&
+                             cpu_to_be64((u64)skbinfo->skbmark << 32 |
+                                         skbinfo->skbmarkmask))) ||
+              (skbinfo->skbprio &&
                nla_put_net32(skb, IPSET_ATTR_SKBPRIO,
-                             cpu_to_be32(skbprio))) ||
-              (skbqueue &&
+                             cpu_to_be32(skbinfo->skbprio))) ||
+              (skbinfo->skbqueue &&
                nla_put_net16(skb, IPSET_ATTR_SKBQUEUE,
-                            cpu_to_be16(skbqueue)));
+                            cpu_to_be16(skbinfo->skbqueue)));
 
 }
 
@@ -400,10 +361,10 @@ static inline void
 ip_set_init_skbinfo(struct ip_set_skbinfo *skbinfo,
                    const struct ip_set_ext *ext)
 {
-       ip_set_rcu_assign_u32(&skbinfo->skbmark, ext->skbmark);
-       ip_set_rcu_assign_u32(&skbinfo->skbmarkmask, ext->skbmarkmask);
-       ip_set_rcu_assign_u32(&skbinfo->skbprio, ext->skbprio);
-       ip_set_rcu_assign_u16(&skbinfo->skbqueue, ext->skbqueue);
+       skbinfo->skbmark = ext->skbmark;
+       skbinfo->skbmarkmask = ext->skbmarkmask;
+       skbinfo->skbprio = ext->skbprio;
+       skbinfo->skbqueue = ext->skbqueue;
 }
 
 static inline bool
index 862504bb3e909c0c2038b34a533d102c2ffae8c2..375a18a39eb5ea025a71d668ac575dd25211e3b6 100644 (file)
 #endif
 
 #ifndef rcu_dereference_protected
-#define rcu_dereference_protected(p, c)        rcu_dereference(p)
+#define rcu_dereference_protected(p, c)        (p)
+#endif
+
+#ifndef rcu_dereference_bh_check
+#define rcu_dereference_bh_check(p, c) rcu_dereference_bh(p)
 #endif
 
 #ifndef __rcu
 #define        __rcu
 #endif
 
+#ifndef RCU_INITIALIZER
+#define RCU_INITIALIZER(v)     (typeof(*(v)) __force __rcu *)(v)
+#define RCU_INIT_POINTER(p, v) \
+       do { \
+               p = RCU_INITIALIZER(v); \
+       } while (0)
+#endif
+
 #ifdef CHECK_KCONFIG
 #ifndef CONFIG_SPARSE_RCU_POINTER
 #error "CONFIG_SPARSE_RCU_POINTER must be enabled"
index 9e300315d824209d5e7e71d75c03a52fbf8e491e..cfe8a3f9aef3c08be6736ba562dd9c26cf564038 100644 (file)
@@ -40,23 +40,9 @@ ip_set_timeout_uget(struct nlattr *tb)
 }
 
 static inline bool
-__ip_set_timeout_expired(unsigned long t)
+ip_set_timeout_expired(unsigned long *t)
 {
-       return t != IPSET_ELEM_PERMANENT && time_is_before_jiffies(t);
-}
-
-static inline bool
-ip_set_timeout_expired_rcu(unsigned long *timeout)
-{
-       unsigned long t = ip_set_rcu_deref(*timeout);
-
-       return __ip_set_timeout_expired(t);
-}
-
-static inline bool
-ip_set_timeout_expired(unsigned long *timeout)
-{
-       return __ip_set_timeout_expired(*timeout);
+       return *t != IPSET_ELEM_PERMANENT && time_is_before_jiffies(*t);
 }
 
 static inline void
@@ -64,23 +50,23 @@ ip_set_timeout_set(unsigned long *timeout, u32 value)
 {
        unsigned long t;
 
-       if (!value)
-               return ip_set_rcu_assign_ulong(timeout, IPSET_ELEM_PERMANENT);
+       if (!value) {
+               *timeout = IPSET_ELEM_PERMANENT;
+               return;
+       }
 
        t = msecs_to_jiffies(value * 1000) + jiffies;
        if (t == IPSET_ELEM_PERMANENT)
                /* Bingo! :-) */
                t--;
-       ip_set_rcu_assign_ulong(timeout, t);
+       *timeout = t;
 }
 
 static inline u32
 ip_set_timeout_get(unsigned long *timeout)
 {
-       unsigned long t = ip_set_rcu_deref(*timeout);
-
-       return t == IPSET_ELEM_PERMANENT ? 0 :
-               jiffies_to_msecs(t - jiffies)/1000;
+       return *timeout == IPSET_ELEM_PERMANENT ? 0 :
+               jiffies_to_msecs(*timeout - jiffies)/1000;
 }
 
 #endif /* __KERNEL__ */
index 8d919cc9534f1643f516424f0c7ca0f9b6b64a07..136f20bc52c62d192260ec30da96a79245db0ba5 100644 (file)
@@ -124,7 +124,7 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,
        if (ret <= 0)
                return ret;
        if (SET_WITH_TIMEOUT(set) &&
-           ip_set_timeout_expired_rcu(ext_timeout(x, set)))
+           ip_set_timeout_expired(ext_timeout(x, set)))
                return 0;
        if (SET_WITH_COUNTER(set))
                ip_set_update_counter(ext_counter(x, set), ext, mext, flags);
@@ -216,7 +216,7 @@ mtype_list(const struct ip_set *set,
 #ifdef IP_SET_BITMAP_STORED_TIMEOUT
                     mtype_is_filled((const struct mtype_elem *) x) &&
 #endif
-                    ip_set_timeout_expired_rcu(ext_timeout(x, set))))
+                    ip_set_timeout_expired(ext_timeout(x, set))))
                        continue;
                nested = ipset_nest_start(skb, IPSET_ATTR_DATA);
                if (!nested) {
index 5e20c26dbacdcb731bc64537bad6a4cc6b4a85e0..86104744b00ff67339f78db238f093874b70845a 100644 (file)
@@ -134,7 +134,7 @@ bitmap_ipmac_add_timeout(unsigned long *timeout,
                if (e->ether)
                        ip_set_timeout_set(timeout, t);
                else
-                       ip_set_rcu_assign_ulong(timeout, t);
+                       *timeout = t;
        }
        return 0;
 }
index 6c8fc0b9073b1b9845ce018483308ab63cdb1e57..885105bbde2191466e0b931e4788eb0dc912cf2d 100644 (file)
 #include <linux/jhash.h>
 #include <linux/types.h>
 #include <linux/netfilter/ipset/ip_set_timeout.h>
-#ifndef rcu_dereference_bh_check
-#define rcu_dereference_bh_check(p, c) rcu_dereference_bh(p)
-#endif
+
+#define __ipset_dereference_protected(p, c)    rcu_dereference_protected(p, c)
+#define ipset_dereference_protected(p, set) \
+       __ipset_dereference_protected(p, spin_is_locked(&(set)->lock))
 
 #define rcu_dereference_bh_nfnl(p)     rcu_dereference_bh_check(p, 1)
 
@@ -78,7 +79,7 @@ struct htable {
        atomic_t ref;           /* References for resizing */
        atomic_t uref;          /* References for dumping */
        u8 htable_bits;         /* size of hash table == 2^htable_bits */
-       struct hbucket * __rcu bucket[0]; /* hashtable buckets */
+       struct hbucket __rcu * bucket[0]; /* hashtable buckets */
 };
 
 #define hbucket(h, i)          ((h)->bucket[i])
@@ -362,9 +363,9 @@ mtype_flush(struct ip_set *set)
        struct hbucket *n;
        u32 i;
 
-       t = h->table;
+       t = ipset_dereference_protected(h->table, set);
        for (i = 0; i < jhash_size(t->htable_bits); i++) {
-               n = hbucket(t, i);
+               n = __ipset_dereference_protected(hbucket(t, i), 1);
                if (n == NULL)
                        continue;
                if (set->extensions & IPSET_EXT_DESTROY)
@@ -387,7 +388,7 @@ mtype_ahash_destroy(struct ip_set *set, struct htable *t, bool ext_destroy)
        u32 i;
 
        for (i = 0; i < jhash_size(t->htable_bits); i++) {
-               n = hbucket(t, i);
+               n = __ipset_dereference_protected(hbucket(t, i), 1);
                if (n == NULL)
                        continue;
                if (set->extensions & IPSET_EXT_DESTROY && ext_destroy)
@@ -408,7 +409,8 @@ mtype_destroy(struct ip_set *set)
        if (set->extensions & IPSET_EXT_TIMEOUT)
                del_timer_sync(&h->gc);
 
-       mtype_ahash_destroy(set, h->table, true);
+       mtype_ahash_destroy(set,
+               __ipset_dereference_protected(h->table, 1), true);
        kfree(h);
 
        set->data = NULL;
@@ -458,10 +460,9 @@ mtype_expire(struct ip_set *set, struct htype *h, u8 nets_length, size_t dsize)
        u8 k;
 #endif
 
-       BUG_ON(!spin_is_locked(&set->lock));
-       t = h->table;
+       t = ipset_dereference_protected(h->table, set);
        for (i = 0; i < jhash_size(t->htable_bits); i++) {
-               n = hbucket(t, i);
+               n = __ipset_dereference_protected(hbucket(t, i), 1);
                if (n == NULL)
                        continue;
                for (j = 0, d = 0; j < n->pos; j++) {
@@ -533,8 +534,7 @@ mtype_resize(struct ip_set *set, bool retried)
        size_t dsize = set->dsize;
 #ifdef IP_SET_HASH_WITH_NETS
        u8 flags;
-       unsigned char _tmp[dsize];
-       struct mtype_elem *tmp = (struct mtype_elem *) &_tmp;
+       struct mtype_elem *tmp;
 #endif
        struct mtype_elem *data;
        struct mtype_elem *d;
@@ -542,6 +542,11 @@ mtype_resize(struct ip_set *set, bool retried)
        u32 i, j, key;
        int ret;
 
+#ifdef IP_SET_HASH_WITH_NETS
+       tmp = kmalloc(dsize, GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+#endif
        rcu_read_lock_bh();
        orig = rcu_dereference_bh_nfnl(h->table);
        htable_bits = orig->htable_bits;
@@ -565,13 +570,13 @@ retry:
        t->htable_bits = htable_bits;
 
        spin_lock_bh(&set->lock);
-       orig = h->table;
+       orig = __ipset_dereference_protected(h->table, 1);
        atomic_set(&orig->ref, 1);
        atomic_inc(&orig->uref);
        pr_debug("attempt to resize set %s from %u to %u, t %p\n",
                 set->name, orig->htable_bits, htable_bits, orig);
        for (i = 0; i < jhash_size(orig->htable_bits); i++) {
-               n = hbucket(orig, i);
+               n = __ipset_dereference_protected(hbucket(orig, i), 1);
                if (n == NULL)
                        continue;
                for (j = 0; j < n->pos; j++) {
@@ -588,7 +593,7 @@ retry:
                        mtype_data_reset_flags(data, &flags);
 #endif
                        key = HKEY(data, h->initval, htable_bits);
-                       m = hbucket(t, key);
+                       m = __ipset_dereference_protected(hbucket(t, key), 1);
                        if (!m) {
                                m = kzalloc(sizeof(struct hbucket) +
                                            AHASH_INIT_SIZE * dsize,
@@ -596,18 +601,18 @@ retry:
                                if (!m)
                                        ret = -ENOMEM;
                                m->size = AHASH_INIT_SIZE;
-                               hbucket(t, key) = m;
+                               RCU_INIT_POINTER(hbucket(t, key), m);
                        } else if (m->pos >= m->size) {
-                               struct hbucket *tmp;
+                               struct hbucket *ht;
 
                                if (m->size >= AHASH_MAX(h))
                                        ret = -EAGAIN;
                                else {
-                                       tmp = kzalloc(sizeof(struct hbucket) +
+                                       ht = kzalloc(sizeof(struct hbucket) +
                                                (m->size + AHASH_INIT_SIZE)
                                                * dsize,
                                                GFP_ATOMIC);
-                                       if (!tmp)
+                                       if (!ht)
                                                ret = -ENOMEM;
                                }
                                if (ret < 0) {
@@ -617,13 +622,14 @@ retry:
                                        mtype_ahash_destroy(set, t, false);
                                        if (ret == -EAGAIN)
                                                goto retry;
-                                       return ret;
+                                       goto out;
                                }
-                               memcpy(tmp, m, sizeof(struct hbucket) +
-                                              m->size * dsize);
-                               tmp->size = m->size + AHASH_INIT_SIZE;
+                               memcpy(ht, m, sizeof(struct hbucket) +
+                                             m->size * dsize);
+                               ht->size = m->size + AHASH_INIT_SIZE;
                                kfree(m);
-                               m = hbucket(t, key) = tmp;
+                               m = ht;
+                               RCU_INIT_POINTER(hbucket(t, key), ht);
                        }
                        d = ahash_data(m, m->pos, dsize);
                        memcpy(d, data, dsize);
@@ -648,11 +654,12 @@ retry:
                mtype_ahash_destroy(set, orig, false);
        }
 
-       return 0;
-
 out:
-       spin_unlock_bh(&set->lock);
+#ifdef IP_SET_HASH_WITH_NETS
+       kfree(tmp);
+#endif
        return ret;
+
 }
 
 /* Add an element to a hash and update the internal counters when succeeded,
@@ -679,9 +686,9 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
                        forceadd = true;
        }
 
-       t = h->table;
+       t = ipset_dereference_protected(h->table, set);
        key = HKEY(value, h->initval, t->htable_bits);
-       n = hbucket(t, key);
+       n = __ipset_dereference_protected(hbucket(t, key), 1);
        if (n == NULL) {
                if (forceadd) {
                        if (net_ratelimit())
@@ -815,9 +822,9 @@ mtype_del(struct ip_set *set, void *value, const struct ip_set_ext *ext,
        u32 key, multi = 0;
        size_t dsize = set->dsize;
 
-       t = h->table;
+       t = ipset_dereference_protected(h->table, set);
        key = HKEY(value, h->initval, t->htable_bits);
-       n = hbucket(t, key);
+       n = __ipset_dereference_protected(hbucket(t, key), 1);
        if (!n)
                goto out;
        for (i = 0, k = 0; i < n->pos; i++) {
@@ -1091,7 +1098,7 @@ mtype_list(const struct ip_set *set,
        for (; cb->args[IPSET_CB_ARG0] < jhash_size(t->htable_bits);
             cb->args[IPSET_CB_ARG0]++) {
                incomplete = skb_tail_pointer(skb);
-               n = hbucket(t, cb->args[IPSET_CB_ARG0]);
+               n = rcu_dereference(hbucket(t, cb->args[IPSET_CB_ARG0]));
                pr_debug("cb->arg bucket: %lu, t %p n %p\n",
                         cb->args[IPSET_CB_ARG0], t, n);
                if (n == NULL)
@@ -1199,12 +1206,14 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
 
        if (unlikely(!ip_set_optattr_netorder(tb, IPSET_ATTR_HASHSIZE) ||
                     !ip_set_optattr_netorder(tb, IPSET_ATTR_MAXELEM) ||
-#ifdef IP_SET_HASH_WITH_MARKMASK
-                    !ip_set_optattr_netorder(tb, IPSET_ATTR_MARKMASK) ||
-#endif
                     !ip_set_optattr_netorder(tb, IPSET_ATTR_TIMEOUT) ||
                     !ip_set_optattr_netorder(tb, IPSET_ATTR_CADT_FLAGS)))
                return -IPSET_ERR_PROTOCOL;
+#ifdef IP_SET_HASH_WITH_MARKMASK
+       /* Separated condition in order to avoid directive in argument list */
+       if (unlikely(!ip_set_optattr_netorder(tb, IPSET_ATTR_MARKMASK)))
+               return -IPSET_ERR_PROTOCOL;
+#endif
 
        if (tb[IPSET_ATTR_HASHSIZE]) {
                hashsize = ip_set_get_h32(tb[IPSET_ATTR_HASHSIZE]);
@@ -1227,7 +1236,7 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
 #endif
 #ifdef IP_SET_HASH_WITH_MARKMASK
        if (tb[IPSET_ATTR_MARKMASK]) {
-               markmask = ntohl(nla_get_u32(tb[IPSET_ATTR_MARKMASK]));
+               markmask = ntohl(nla_get_be32(tb[IPSET_ATTR_MARKMASK]));
 
                if (markmask == 0)
                        return -IPSET_ERR_INVALID_MARKMASK;