netdev - Re: [PATCH 1/8] udp: add a counter into udp

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4AF80040.6060501@gmail.com>
Date:	Mon, 09 Nov 2009 12:42:56 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Andi Kleen <andi@...stfloor.org>
CC:	"David S. Miller" <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Lucian Adrian Grijincu <lgrijincu@...acom.com>,
	Octavian Purdila <opurdila@...acom.com>
Subject: Re: [PATCH 1/8] udp: add a counter into udp_hslot

Andi Kleen a écrit :
> Eric Dumazet <eric.dumazet@...il.com> writes:
>>  
>> +/**
>> + *	struct udp_hslot - UDP hash slot
>> + *
>> + *	@head:	head of list of sockets
>> + *	@count:	number of sockets in 'head' list
>> + *	@lock:	spinlock protecting changes to head/count
>> + */
>>  struct udp_hslot {
>>  	struct hlist_nulls_head	head;
>> +	int			count;
> 
> Do you really need an int? On 64bit it's free due to the alignment, 
> but on 32bit x86 it's costly and you blow up the table considerably,
> increasing cache misses.

Even a short (16 bits) might be too small for IXIACOM :)

On 32bit x86, size of hash table is 512 slots max.
(one slot per 2MB of LOWMEM, rounded to power of two)

You are speaking of <= 4096 bytes overhead :)

>  
> Again it would be nicer if that was a separate smaller table together
> with the spinlock.

Nice for space, not nice for fast path, because this means additional
cache miss to get the spinlock (multicast rx still needs to take spinlock),
and some guys want really fast (low latency) multicast rx.

> 
> In theory could also put a short counter into the low level alignment
> bits of the pointer and perhaps convert the spinlock to a bitlock?
> Then all could collapse into a single pointer.
> 

Not enough bits in low level alignment unfortunatly. We only could give a
hint (one bit is enough) of possibly long chain, but not allowing precise 
choice of shortest chain.

Once multicast is converted to RCU, then we wont need one spinlock per slot
(it wont be used in fast path, only at bind()/close() time)
and yes, we can use a separate small array to contain hashed spinlocks,
or even a single spinlock for CONFIG_BASE_SMALL :)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html