From 68ade8303ff94cc10586298997a0474b513ddc61 Mon Sep 17 00:00:00 2001 From: Jozsef Kadlecsik Date: Thu, 27 Nov 2014 17:54:52 +0100 Subject: Remove unnecessary integer RCU handling and fix sparse warnings --- kernel/include/linux/netfilter/ipset/ip_set.h | 77 +++++--------------- .../linux/netfilter/ipset/ip_set_compat.h.in | 14 +++- .../include/linux/netfilter/ipset/ip_set_timeout.h | 32 +++------ kernel/net/netfilter/ipset/ip_set_bitmap_gen.h | 4 +- kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 +- kernel/net/netfilter/ipset/ip_set_hash_gen.h | 83 ++++++++++++---------- 6 files changed, 90 insertions(+), 122 deletions(-) (limited to 'kernel') diff --git a/kernel/include/linux/netfilter/ipset/ip_set.h b/kernel/include/linux/netfilter/ipset/ip_set.h index e640c62..08dd689 100644 --- a/kernel/include/linux/netfilter/ipset/ip_set.h +++ b/kernel/include/linux/netfilter/ipset/ip_set.h @@ -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 diff --git a/kernel/include/linux/netfilter/ipset/ip_set_compat.h.in b/kernel/include/linux/netfilter/ipset/ip_set_compat.h.in index 862504b..375a18a 100644 --- a/kernel/include/linux/netfilter/ipset/ip_set_compat.h.in +++ b/kernel/include/linux/netfilter/ipset/ip_set_compat.h.in @@ -53,13 +53,25 @@ #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" diff --git a/kernel/include/linux/netfilter/ipset/ip_set_timeout.h b/kernel/include/linux/netfilter/ipset/ip_set_timeout.h index 9e30031..cfe8a3f 100644 --- a/kernel/include/linux/netfilter/ipset/ip_set_timeout.h +++ b/kernel/include/linux/netfilter/ipset/ip_set_timeout.h @@ -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__ */ diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h b/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h index 8d919cc..136f20b 100644 --- a/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h @@ -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) { diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c index 5e20c26..8610474 100644 --- a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c @@ -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; } diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h index 6c8fc0b..885105b 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_gen.h +++ b/kernel/net/netfilter/ipset/ip_set_hash_gen.h @@ -12,9 +12,10 @@ #include #include #include -#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; -- cgit v1.2.3