On Sat, May 02, 2015 at 07:27:52PM +0200, Jozsef Kadlecsik wrote:
> There's nothing much required because the bitmap types use atomic
> bit operations. However the logic of adding elements slightly changed:
> first the MAC address updated (which is not atomic), then the element
> activated (added).
>
> Signed-off-by: Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx>
> ---
> net/netfilter/ipset/ip_set_bitmap_gen.h | 14 ++++++++++----
> net/netfilter/ipset/ip_set_bitmap_ip.c | 3 ++-
> net/netfilter/ipset/ip_set_bitmap_ipmac.c | 13 +++++++++++--
> net/netfilter/ipset/ip_set_bitmap_port.c | 3 ++-
> 4 files changed, 25 insertions(+), 8 deletions(-)
>
[...]
> diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c b/net/netfilter/ipset/ip_set_bitmap_ip.c
> index 55b083e..487513b 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_ip.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_ip.c
> @@ -80,7 +80,7 @@ static inline int
> bitmap_ip_do_add(const struct bitmap_ip_adt_elem *e, struct bitmap_ip *map,
> u32 flags, size_t dsize)
> {
> - return !!test_and_set_bit(e->id, map->members);
> + return !!test_bit(e->id, map->members);
> }
>
> static inline int
> @@ -377,6 +377,7 @@ bitmap_ip_init(void)
> static void __exit
> bitmap_ip_fini(void)
> {
> + rcu_barrier();
You need the rcu_barrier() if you kfree_rcu() or call_rcu().
Basically, this makes sure that rcu gives a chance to deferred
callbacks to run. I don't see any of this in this patch, so this may
have slipped through or it may not necessary.
> diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> index 8610474..8f3f1cd 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> @@ -146,15 +146,23 @@ bitmap_ipmac_do_add(const struct bitmap_ipmac_adt_elem *e,
> struct bitmap_ipmac_elem *elem;
>
> elem = get_elem(map->extensions, e->id, dsize);
> - if (test_and_set_bit(e->id, map->members)) {
> + if (test_bit(e->id, map->members)) {
> if (elem->filled == MAC_FILLED) {
> - if (e->ether && (flags & IPSET_FLAG_EXIST))
> + if (e->ether &&
> + (flags & IPSET_FLAG_EXIST) &&
> + !ether_addr_equal(e->ether, elem->ether)) {
> + /* memcpy isn't atomic */
> + clear_bit(e->id, map->members);
> + smp_mb__after_atomic();
> memcpy(elem->ether, e->ether, ETH_ALEN);
> + }
> return IPSET_ADD_FAILED;
> } else if (!e->ether)
> /* Already added without ethernet address */
> return IPSET_ADD_FAILED;
> /* Fill the MAC address and trigger the timer activation */
> + clear_bit(e->id, map->members);
> + smp_mb__after_atomic();
> memcpy(elem->ether, e->ether, ETH_ALEN);
> elem->filled = MAC_FILLED;
> return IPSET_ADD_START_STORED_TIMEOUT;
> @@ -414,6 +422,7 @@ bitmap_ipmac_init(void)
> static void __exit
> bitmap_ipmac_fini(void)
> {
> + rcu_barrier();
Same thing here.
> ip_set_type_unregister(&bitmap_ipmac_type);
> }
>
> diff --git a/net/netfilter/ipset/ip_set_bitmap_port.c b/net/netfilter/ipset/ip_set_bitmap_port.c
> index 005dd36..e69619a 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_port.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_port.c
> @@ -73,7 +73,7 @@ static inline int
> bitmap_port_do_add(const struct bitmap_port_adt_elem *e,
> struct bitmap_port *map, u32 flags, size_t dsize)
> {
> - return !!test_and_set_bit(e->id, map->members);
> + return !!test_bit(e->id, map->members);
> }
>
> static inline int
> @@ -311,6 +311,7 @@ bitmap_port_init(void)
> static void __exit
> bitmap_port_fini(void)
> {
> + rcu_barrier();
And here.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
