lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <33ccbd2770b069da8144aa1b94134f7d6464f4eb.camel@redhat.com> Date: Fri, 11 Nov 2022 09:53:31 +0100 From: Paolo Abeni <pabeni@...hat.com> To: Kuniyuki Iwashima <kuniyu@...zon.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org> Cc: Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org Subject: Re: [PATCH v2 net-next 6/6] udp: Introduce optional per-netns hash table. On Thu, 2022-11-10 at 20:00 -0800, Kuniyuki Iwashima wrote: > @@ -408,6 +409,28 @@ static int proc_tcp_ehash_entries(struct ctl_table *table, int write, > return proc_dointvec(&tbl, write, buffer, lenp, ppos); > } > > +static int proc_udp_hash_entries(struct ctl_table *table, int write, > + void *buffer, size_t *lenp, loff_t *ppos) > +{ > + struct net *net = container_of(table->data, struct net, > + ipv4.sysctl_udp_child_hash_entries); > + int udp_hash_entries; > + struct ctl_table tbl; > + > + udp_hash_entries = net->ipv4.udp_table->mask + 1; > + > + /* A negative number indicates that the child netns > + * shares the global udp_table. > + */ > + if (!net_eq(net, &init_net) && net->ipv4.udp_table == &udp_table) > + udp_hash_entries *= -1; > + > + tbl.data = &udp_hash_entries; > + tbl.maxlen = sizeof(int); I see the procfs code below will only use tbl.data and tbl.maxlen, but perhaps is cleaner intially explicitly memset tbl to 0 > > + > + return proc_dointvec(&tbl, write, buffer, lenp, ppos); > +} > + > #ifdef CONFIG_IP_ROUTE_MULTIPATH > static int proc_fib_multipath_hash_policy(struct ctl_table *table, int write, > void *buffer, size_t *lenp, [...] > @@ -3308,22 +3308,112 @@ u32 udp_flow_hashrnd(void) > } > EXPORT_SYMBOL(udp_flow_hashrnd); > > -static int __net_init udp_sysctl_init(struct net *net) > +static void __net_init udp_sysctl_init(struct net *net) > { > - net->ipv4.udp_table = &udp_table; > - > net->ipv4.sysctl_udp_rmem_min = PAGE_SIZE; > net->ipv4.sysctl_udp_wmem_min = PAGE_SIZE; > > #ifdef CONFIG_NET_L3_MASTER_DEV > net->ipv4.sysctl_udp_l3mdev_accept = 0; > #endif > +} > + > +static struct udp_table __net_init *udp_pernet_table_alloc(unsigned int hash_entries) > +{ > + unsigned long hash_size, bitmap_size; > + struct udp_table *udptable; > + int i; > + > + udptable = kmalloc(sizeof(*udptable), GFP_KERNEL); > + if (!udptable) > + goto out; > + > + udptable->log = ilog2(hash_entries); > + udptable->mask = hash_entries - 1; > + > + hash_size = L1_CACHE_ALIGN(hash_entries * 2 * sizeof(struct udp_hslot)); > + bitmap_size = hash_entries * > + BITS_TO_LONGS(udp_bitmap_size(udptable)) * sizeof(unsigned long); Ouch, I'm very sorry. I did not realize we need a bitmap per hash bucket. This leads to a constant 8k additional memory overhead per netns, undependently from arch long bitsize. I guess it's still acceptable, but perhaps worth mentioning in the commit message? Again sorry for the back && forth, I'm reconsidering all the above given my dumb misunderstanding. I see that a minumum size of 256 hash buckets does not match your use case, still... if lower values of the per netns hash size are inflated to e.g. 128 and we keep the bitmap on stack (should be 64 bytes wide, I guess still an acceptable value), the per netns memory overhead will be 128*2*<hash bucket size> = 8K, less that what we get the above schema and any smaller hash - a single hash bucket leads to a 32 + 8K memory overhead. TL;DR: what about accepting any per netns hash table size, but always allocate at least 128 buckets and keep the bitmap on the stack? Thanks, Paolo
Powered by blists - more mailing lists