Core TeamGroup
ActivePublic

Watchers

  • This project does not have any watchers.

Recent Activity

Nov 25 2016

fabient closed D8130: Split arc4random mutexes to improve performance on IPSec traffic by committing rS309143: In a dual processor system (2*6 cores) during IPSec throughput tests,.
Nov 25 2016, 1:49 PM · Core Team

Nov 15 2016

delphij accepted D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Sorry for the delay (des@ have pinged me today). Consider this as a so@ approval as we haven't heard other objections so far.

Nov 15 2016, 6:07 PM · Core Team

Nov 9 2016

emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

delphij: is that OK for you?

Nov 9 2016, 10:41 AM · Core Team

Nov 7 2016

ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

By the second thought, no problem:) Since you mention that per-CPU locking is not your goal, in worst case the same harmless multiple stiring can happens since numruns belongs to the same arc4 data set to which stiring applied.

Nov 7 2016, 11:41 AM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Unfortunately this is a problem that is already present in the current version.

In current version it is not a problem (see my previous comment about multiple stiring above), but in case each arc4 instance can run on different arc4 data set, it can be a problem.

Nov 7 2016, 11:19 AM · Core Team
emeric.poupon_stormshield.eu updated the diff for D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Updated diff using full context, changed code according to delphij's comment

Nov 7 2016, 10:01 AM · Core Team
emeric.poupon_stormshield.eu added inline comments to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
Nov 7 2016, 9:57 AM · Core Team

Nov 4 2016

ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

unlocked read of arc4->numruns

Will lead to several arc4 stiring in worst case (which IMHO is non-practical case). Any number of stiring does not harm arc4, at least one stiring should be enough. But all this for one CPU case, I don't know about per-CPU locking in details so can't answer the next note.

Nov 4 2016, 10:05 PM · Core Team
delphij added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

I have some comments inline -- please fix the style change.

Nov 4 2016, 9:13 PM · Core Team

Oct 7 2016

ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Yes.

Oct 7 2016, 4:16 PM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Is that OK for you?

Oct 7 2016, 3:43 PM · Core Team
emeric.poupon_stormshield.eu updated the diff for D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Updated diff according to ache's remarks

Oct 7 2016, 8:11 AM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

This is nearly what I proposed before, but why would you reseed all the instances in case the user calls arc4rand with reseed = 1?

Because from current API point of view there is no separate CPU argument, so the call with reseed == 1 can come from any random CPU.
In general any random CPU may be not equal to unknown CPU for which this operation was made.

Oct 7 2016, 7:32 AM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
if (atomic_cmpset_int(&arc4rand_iniseed_state, ARC4_ENTR_HAVE, ARC4_ENTR_SEED) ||
    reseed) {
        ARC4_FOREACH(arc4)
                arc4_randomstir(arc4);
}
...
if (arc4->numruns > ARC4_RESEED_BYTES || 
    tv.tv_sec > arc4->t_reseed)
        arc4_randomstir(arc4);

This is nearly what I proposed before, but why would you reseed all the instances in case the user calls arc4rand with reseed = 1?

Oct 7 2016, 7:21 AM · Core Team

Oct 6 2016

ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Not sure this is a big security concern or not?

Maybe I am wrong, but I don't see big deal here - the moment of good randomness available is unpredictable in the time intervals we consider, and if some threads got it a bit later (on the next call), I don't see the problem.

Oct 6 2016, 4:49 PM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
In D8130#169388, @ache wrote:

There can be small window when arc4rand_iniseed_state is already set to SEED, but arc4_randomstir() is not called yet. And right in this time another thread calls the code. Well, we miss only single reinitialization per-CPU (more are time-consuming and can't fit between the check and immediate function call), on the next call they will be already reinitialized. Dou mean another scenario?

Yes that is what I meant. Not sure this is a big security concern or not?

Oct 6 2016, 4:13 PM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
In D8130#169387, @ache wrote:

The problem is that another thread may call the arc4rand() function and get some unsafe random bytes before the stir actually occurs and after the state was set to SEED.

Please show how it can happen in steps by steps. In old code if the state was SEED, it surely reinitialized, and reinitialization itself keeps a lock, so RNG can't move further and wait for reinitialization (from any thread).

Oct 6 2016, 4:10 PM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

There can be small window when arc4rand_iniseed_state is already set to SEED, but arc4_randomstir() is not called yet. And right in this time another thread calls the code. Well, we miss only single reinitialization per-CPU (more are time-consuming and can't fit between the check and immediate function call), on the next call they will be already reinitialized. Dou mean another scenario?
We can move both check and function call under lock, but it will slow things down, i.e. the thing you fight against.

Oct 6 2016, 3:19 PM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

The problem is that another thread may call the arc4rand() function and get some unsafe random bytes before the stir actually occurs and after the state was set to SEED.

Please show how it can happen in steps by steps. In old code if the state was SEED, it surely reinitialized, and reinitialization itself keeps a lock, so RNG can't move further and wait for reinitialization (from any thread).

Here is what I propose:

In your code 'reseed' variable should be move to the same 'if' as 'arc4rand_iniseed_state' placed, we never know what we cpu we reseed otherwise.
The next thing I don't like much is possible double initialization, if this condition additionaly meet in the same time 'if ((arc4->numruns > ARC4_RESEED_BYTES) || (tv.tv_sec > arc4->t_reseed))
All you need is simple local 'reseedall' variable and set it where ARC4_FOREACH(arc4) is.

Oct 6 2016, 2:59 PM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
In D8130#169376, @ache wrote:

I meant the arc4rand_iniseed_state variable.

I too. Without locking or atomic either yet one additional seeding can happens or no seeding can happens at all (CPU writing to another half of word, which is already checked).

I do agree with that, but on the very first call of arc4rand(), we can make sure readomstir() is called,

Not all stiring are equal) Non random stiring is done to just not block arc4 on early boot phase.
When good randomnes becomes available (which arc4rand_iniseed_state indicates) it must be reinitialized immediately on the next call.

Oct 6 2016, 12:11 PM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

I meant the arc4rand_iniseed_state variable.

I too. Without locking or atomic either yet one additional seeding can happens or no seeding can happens at all (CPU writing to another half of word, which is already checked).

I do agree with that, but on the very first call of arc4rand(), we can make sure readomstir() is called,

Not all stiring are equal) Non random stiring is done to just not block arc4 on early boot phase.
When good randomnes becomes available (which arc4rand_iniseed_state indicates) it must be reinitialized immediately on the next call.

Oct 6 2016, 11:04 AM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
In D8130#169374, @ache wrote:

But I am not sure to see the actual benefit of this atomic?

It allows to not play with locking.

I meant the arc4rand_iniseed_state variable.

Oct 6 2016, 10:47 AM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

But I am not sure to see the actual benefit of this atomic?

It allows to not play with locking.

I guess the read_random() call is blocking until the underlying entropy processor becomes secure?

Yes. Then it allows arc4 reseed. There is no point to reseed it before it happens with non random data.

Oct 6 2016, 10:38 AM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

That's a good point indeed.

Oct 6 2016, 10:28 AM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

reseed arg without specifying particular CPU means nothing but stir all CPUs too. And specifying that CPU is impossible on API level - nobody know from which CPU this code will be called.

Oct 6 2016, 9:32 AM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

The problem is that arc4rand_iniseed_state which is reset on the first use with atomic currently asssume that all RNGs are stired, but really only first one.

Oct 6 2016, 9:25 AM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Do you have other remarks on this patch?

Oct 6 2016, 9:13 AM · Core Team

Oct 5 2016

emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Thanks for working on this, but I have some different ideas which I don't really have the time to sit down and implement at this time that I feel like sharing and may be useful:

I think we could take a different approach: now that we have the pseudo-random state per-CPU, perhaps we can make them CPU-bound and completely eliminate the locks and use critical sections instead?

Oct 5 2016, 7:07 AM · Core Team

Oct 4 2016

delphij added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Thanks for working on this, but I have some different ideas which I don't really have the time to sit down and implement at this time that I feel like sharing and may be useful:

Oct 4 2016, 4:18 PM · Core Team
emeric.poupon_stormshield.eu updated the diff for D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Changed malloc test to KASSERT

Oct 4 2016, 1:29 PM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
In D8130#168678, @ache wrote:

Sorry, my prev. comment was sent non-edited. Please forget it if you got it through the mail and re-read here instead.

No problem!
Since the call is very unlikely to fail, what about a KASSERT on failure then? That seems to be an acceptable compromise.

Oct 4 2016, 9:39 AM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
In D8130#168678, @ache wrote:

Sorry, my prev. comment was sent non-edited. Please forget it if you got it through the mail and re-read here instead.

Oct 4 2016, 8:47 AM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Sorry, my prev. comment was sent non-edited. Please forget it if you got it through the mail and re-read here instead.

Oct 4 2016, 8:29 AM · Core Team
markm added a reviewer for D8130: Split arc4random mutexes to improve performance on IPSec traffic: delphij.

Add SO

Oct 4 2016, 8:17 AM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
In D8130#168667, @ache wrote:

It will be better to use non-failing malloc flag. arc4_init() can't fail.

You mean a M_WAITOK flag? I was wondering: are you sure it is really safe to sleep in this context?

Oct 4 2016, 8:14 AM · Core Team
emeric.poupon_stormshield.eu added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.
In D8130#168667, @ache wrote:

It will be better to use non-failing malloc flag. arc4_init() can't fail.

Oct 4 2016, 7:41 AM · Core Team
ache added a comment to D8130: Split arc4random mutexes to improve performance on IPSec traffic.

It will be better to use non-failing malloc flag. arc4_init() can't fail.

Oct 4 2016, 6:56 AM · Core Team

Oct 3 2016

emeric.poupon_stormshield.eu updated the diff for D8130: Split arc4random mutexes to improve performance on IPSec traffic.

Sorry, forgot to remove a commented line

Oct 3 2016, 12:10 PM · Core Team
emeric.poupon_stormshield.eu retitled D8130: Split arc4random mutexes to improve performance on IPSec traffic from to Split arc4random mutexes to improve performance on IPSec traffic.
Oct 3 2016, 12:02 PM · Core Team

Jul 26 2016

robak removed a member for Core Team: peter.
Jul 26 2016, 7:59 AM
robak removed a member for Core Team: rwatson.
Jul 26 2016, 7:59 AM
robak removed a member for Core Team: glebius.
Jul 26 2016, 7:59 AM
robak removed a member for Core Team: theraven.
Jul 26 2016, 7:59 AM
robak removed a member for Core Team: gavin.
Jul 26 2016, 7:58 AM
robak added members for Core Team: allanjude, bcr, benno, jhb, kmoore.
Jul 26 2016, 7:58 AM