Message ID | 70ff6b512f8a6e9fc70a0e2b001d6278d31a1c96.1558884459.git.sbrivio@redhat.com |
---|---|
State | Accepted |
Delegated to: | Jozsef Kadlecsik |
Headers | show |
Series | ipset: Fix memory accounting for hash types on resize | expand |
Hi Stefano, On Sun, 26 May 2019, Stefano Brivio wrote: > If a fresh array block is allocated during resize, the current in-memory > set size should be increased by the size of the block, not replaced by it. > > Before the fix, adding entries to a hash set type, leading to a table > resize, caused an inconsistent memory size to be reported. This becomes > more obvious when swapping sets with similar sizes: > > # cat hash_ip_size.sh > #!/bin/sh > FAIL_RETRIES=10 > > tries=0 > while [ ${tries} -lt ${FAIL_RETRIES} ]; do > ipset create t1 hash:ip > for i in `seq 1 4345`; do > ipset add t1 1.2.$((i / 255)).$((i % 255)) > done > t1_init="$(ipset list t1|sed -n 's/Size in memory: \(.*\)/\1/p')" > > ipset create t2 hash:ip > for i in `seq 1 4360`; do > ipset add t2 1.2.$((i / 255)).$((i % 255)) > done > t2_init="$(ipset list t2|sed -n 's/Size in memory: \(.*\)/\1/p')" > > ipset swap t1 t2 > t1_swap="$(ipset list t1|sed -n 's/Size in memory: \(.*\)/\1/p')" > t2_swap="$(ipset list t2|sed -n 's/Size in memory: \(.*\)/\1/p')" > > ipset destroy t1 > ipset destroy t2 > tries=$((tries + 1)) > > if [ ${t1_init} -lt 10000 ] || [ ${t2_init} -lt 10000 ]; then > echo "FAIL after ${tries} tries:" > echo "T1 size ${t1_init}, after swap ${t1_swap}" > echo "T2 size ${t2_init}, after swap ${t2_swap}" > exit 1 > fi > done > echo "PASS" > # echo -n 'func hash_ip4_resize +p' > /sys/kernel/debug/dynamic_debug/control > # ./hash_ip_size.sh > [ 2035.018673] attempt to resize set t1 from 10 to 11, t 00000000fe6551fa > [ 2035.078583] set t1 resized from 10 (00000000fe6551fa) to 11 (00000000172a0163) > [ 2035.080353] Table destroy by resize 00000000fe6551fa > FAIL after 4 tries: > T1 size 9064, after swap 71128 > T2 size 71128, after swap 9064 > > Reported-by: NOYB <JunkYardMail1@Frontier.com> > Fixes: 9e41f26a505c ("netfilter: ipset: Count non-static extension memory for userspace") > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Excellent, thanks! Patch is applied in ipset git tree and a new release is on the way. Best regards, Jozsef > --- > net/netfilter/ipset/ip_set_hash_gen.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h > index 01d51f775f12..623e0d675725 100644 > --- a/net/netfilter/ipset/ip_set_hash_gen.h > +++ b/net/netfilter/ipset/ip_set_hash_gen.h > @@ -625,7 +625,7 @@ mtype_resize(struct ip_set *set, bool retried) > goto cleanup; > } > m->size = AHASH_INIT_SIZE; > - extsize = ext_size(AHASH_INIT_SIZE, dsize); > + extsize += ext_size(AHASH_INIT_SIZE, dsize); > RCU_INIT_POINTER(hbucket(t, key), m); > } else if (m->pos >= m->size) { > struct hbucket *ht; > -- > 2.20.1 > > - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary
diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h index 01d51f775f12..623e0d675725 100644 --- a/net/netfilter/ipset/ip_set_hash_gen.h +++ b/net/netfilter/ipset/ip_set_hash_gen.h @@ -625,7 +625,7 @@ mtype_resize(struct ip_set *set, bool retried) goto cleanup; } m->size = AHASH_INIT_SIZE; - extsize = ext_size(AHASH_INIT_SIZE, dsize); + extsize += ext_size(AHASH_INIT_SIZE, dsize); RCU_INIT_POINTER(hbucket(t, key), m); } else if (m->pos >= m->size) { struct hbucket *ht;
If a fresh array block is allocated during resize, the current in-memory set size should be increased by the size of the block, not replaced by it. Before the fix, adding entries to a hash set type, leading to a table resize, caused an inconsistent memory size to be reported. This becomes more obvious when swapping sets with similar sizes: # cat hash_ip_size.sh #!/bin/sh FAIL_RETRIES=10 tries=0 while [ ${tries} -lt ${FAIL_RETRIES} ]; do ipset create t1 hash:ip for i in `seq 1 4345`; do ipset add t1 1.2.$((i / 255)).$((i % 255)) done t1_init="$(ipset list t1|sed -n 's/Size in memory: \(.*\)/\1/p')" ipset create t2 hash:ip for i in `seq 1 4360`; do ipset add t2 1.2.$((i / 255)).$((i % 255)) done t2_init="$(ipset list t2|sed -n 's/Size in memory: \(.*\)/\1/p')" ipset swap t1 t2 t1_swap="$(ipset list t1|sed -n 's/Size in memory: \(.*\)/\1/p')" t2_swap="$(ipset list t2|sed -n 's/Size in memory: \(.*\)/\1/p')" ipset destroy t1 ipset destroy t2 tries=$((tries + 1)) if [ ${t1_init} -lt 10000 ] || [ ${t2_init} -lt 10000 ]; then echo "FAIL after ${tries} tries:" echo "T1 size ${t1_init}, after swap ${t1_swap}" echo "T2 size ${t2_init}, after swap ${t2_swap}" exit 1 fi done echo "PASS" # echo -n 'func hash_ip4_resize +p' > /sys/kernel/debug/dynamic_debug/control # ./hash_ip_size.sh [ 2035.018673] attempt to resize set t1 from 10 to 11, t 00000000fe6551fa [ 2035.078583] set t1 resized from 10 (00000000fe6551fa) to 11 (00000000172a0163) [ 2035.080353] Table destroy by resize 00000000fe6551fa FAIL after 4 tries: T1 size 9064, after swap 71128 T2 size 71128, after swap 9064 Reported-by: NOYB <JunkYardMail1@Frontier.com> Fixes: 9e41f26a505c ("netfilter: ipset: Count non-static extension memory for userspace") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> --- net/netfilter/ipset/ip_set_hash_gen.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)