Re: [PATCH RFC] net: introduce support for lazy initialization of secret keys

[Eric Dumazet <eric.dumazet@gmail.com>]

On Wed, 2013-09-25 at 23:34 +0200, Hannes Frederic Sowa wrote:
> net_get_random_once is a new macro which handles the initialization of
> secret keys at use-site. It is possible to call it in the fast path. Only
> the initialization depends on the spinlock and is rather slow. Otherwise
> it should get used just before the key is used to delay the entropy
> extration as late as possible to get better randomness. It returns true
> if the key got initialized.

So you don't like cmpxchg() ;)

> +/* BE CAREFUL: this function is not interrupt safe */
> +#define net_get_random_once(buf, nbytes)				\
> +	({								\
> +		static DEFINE_SPINLOCK(__lock);				\
> +		static bool __done = false;				\
> +		bool __ret = false;					\
> +		if (unlikely(!__done))					\
> +			__ret = __net_get_random_once(buf,		\
> +						    nbytes,		\
> +						    &__done,		\
> +						    &__lock);		\
> +		__ret;							\
> +	})
> +
>  

No idea why its needed to have one spinlock per call point.

A single lock should be more than enough.

The spinlock could be private to __net_get_random_once()

+bool __net_get_random_once(void *buf, int nbytes, bool *done,
+                           spinlock_t *lock)
+{
+       spin_lock_bh(lock);
+       if (*done) {
+               spin_unlock_bh(lock);
+               return false;
+       }
+
+       get_random_bytes(buf, nbytes);

I think you might need a memory barrier here.

(smp_wmb();)

+       *done = true;
+       spin_unlock_bh(lock);


BTW, build_ehash_secret() is called like that :

if (unlikely(!inet_ehash_secret))
    if (sock->type != SOCK_RAW && sock->type != SOCK_DGRAM)
        build_ehash_secret();

So it would be better to make sure inet_ehash_secret is not 0 by
accident.