svn commit: r334054 - in head: sys/kern sys/netipsec tools/tools/crypto usr.bin/netstat

[Emeric POUPON]

----- Original Message -----
> From: "Mateusz Guzik" <mjguzik at gmail.com>
> To: "Fabien Thomas" <fabient at freebsd.org>
> Cc: svn-src-head at freebsd.org, svn-src-all at freebsd.org, "src-committers" <src-committers at freebsd.org>
> Sent: Tuesday, 22 May, 2018 18:45:32
> Subject: Re: svn commit: r334054 - in head: sys/kern sys/netipsec tools/tools/crypto usr.bin/netstat

> On Tue, May 22, 2018 at 5:54 PM, Fabien Thomas <fabient at freebsd.org> wrote:
> 
>> Author: fabient
>> Date: Tue May 22 15:54:25 2018
>> New Revision: 334054
>> URL: https://svnweb.freebsd.org/changeset/base/334054
>>
>> Log:
>>   Add a SPD cache to speed up lookups.
>>
>>   When large SPDs are used, we face two problems:
>>
>>   - too many CPU cycles are spent during the linear searches in the SPD
>>     for each packet
>>   - too much contention on multi socket systems, since we use a single
>>     shared lock.
>>
>>
>>  void
>> +spdcache_init(void)
>> +{
>> +       int i;
>> +
>> +       TUNABLE_INT_FETCH("net.key.spdcache.maxentries",
>> +           &V_key_spdcache_maxentries);
>> +       TUNABLE_INT_FETCH("net.key.spdcache.threshold",
>> +           &V_key_spdcache_threshold);
>> +
>> +       if (V_key_spdcache_maxentries) {
>> +               V_key_spdcache_maxentries = MAX(V_key_spdcache_maxentries,
>> +                   SPDCACHE_MAX_ENTRIES_PER_HASH);
>> +               V_spdcachehashtbl = hashinit(V_key_spdcache_maxentries /
>> +                   SPDCACHE_MAX_ENTRIES_PER_HASH,
>> +                   M_IPSEC_SPDCACHE, &V_spdcachehash_mask);
>> +               V_key_spdcache_maxentries = (V_spdcachehash_mask + 1)
>> +                   * SPDCACHE_MAX_ENTRIES_PER_HASH;
>> +
>> +               V_spdcache_lock = malloc(sizeof(struct mtx) *
>> +                   (V_spdcachehash_mask + 1),
>> +                   M_IPSEC_SPDCACHE, M_WAITOK|M_ZERO);
>> +
>> +               for (i = 0; i < V_spdcachehash_mask + 1; ++i)
>> +                       SPDCACHE_LOCK_INIT(i);
>> +       }
>> +}
>> +
>>
> 
> This ends up putting two locks per cacheline and sharing bucket heads
> across other lines.
> 
> Unless you got a good reason not to, you should define a struct a which has
> both the lock and the list.
> 
> An example of this can be found in kern/kern_lockf.c

Fortunately the SPD lookup time is negligible compared to crypto operations time with this patch.
But you are right, it should have been done your way: we will change that in the next step.

Thanks for pointing this out.

> 
> --
> Mateusz Guzik <mjguzik gmail.com>
> _______________________________________________
> svn-src-all at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"