Page MenuHomeFreeBSD

Add a SPD cache to speed up lookups
ClosedPublic

Authored by emeric.poupon_stormshield.eu on Apr 12 2018, 1:11 PM.
Referenced Files
Unknown Object (File)
Thu, Nov 28, 7:14 PM
Unknown Object (File)
Thu, Nov 28, 7:14 PM
Unknown Object (File)
Thu, Nov 28, 7:14 PM
Unknown Object (File)
Thu, Nov 28, 7:13 PM
Unknown Object (File)
Thu, Nov 28, 7:13 PM
Unknown Object (File)
Thu, Nov 28, 7:13 PM
Unknown Object (File)
Thu, Nov 21, 6:42 AM
Unknown Object (File)
Sat, Nov 16, 11:18 AM
Subscribers

Details

Reviewers
ae
Group Reviewers
network
Commits
rS334054: Add a SPD cache to speed up lookups.
Summary

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 eat
  • too much contention on multi socket systems, since we use a single shared lock.

Main changes:

  • added the sysctl tree 'net.key.spdcache' to control the SPD cache (disabled by default)
  • cache the sp indexes that are used to perform SP lookups.
  • use a range of dedicated mutexes to protect the cache lines.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Can you provide some numbers what benefits you have with this patch?

In D15050#317087, @ae wrote:

Can you provide some numbers what benefits you have with this patch?

Sure!
Performance increased from 3gpbs to 12gbps using a 2*6 cores Xeon system
SPD size: 1024
8192 different UDP flows
AES-CBC-HMAC-SHA1

Update the maxentries sysctl once the hashtable has been init in order to reflect the actual size used.

sys/netipsec/ipsec.h
223 ↗(On Diff #41402)

I think this breaks the ABI and makes the change a non-MFCable.

sys/netipsec/key.c
204 ↗(On Diff #41402)

It looks like key_spidxhash() is similar to key_saidxhash(). Maybe it makes sense to rework it like:

static uint32_t key_addrprotohash(const union sockaddr_union *src, const union sockaddr_union *dst,
     const uint8_t *proto);

#define SPDCACHE_HASHVAL(idx)  \
     (key_addrprotohash(&idx->src, &idx->dst, &idx->ul_proto) & V_spdcachehash_mask)
#define SAHADDRHASH_HASHVAL(idx)   \
     (key_addrprotohash(&idx->src, &idx->dst, &idx->proto) & V_sahaddrhash_mask)
245 ↗(On Diff #41402)

This confuses the reader, I think it is better to not hide & under the macro. Also it is not clear to me, why we can not just use NULL pointer for this purpose.

549 ↗(On Diff #41402)

I think this should be SYSCTL_PROC to correctly handle user's input.

8273 ↗(On Diff #41402)

hashinit() already did LISTs initialization.

emeric.poupon_stormshield.eu added inline comments.
sys/netipsec/ipsec.h
223 ↗(On Diff #41402)

Yes indeed. Is it a problem to be non-MFCable?

sys/netipsec/key.c
245 ↗(On Diff #41402)

Yes you're right. This is some old code from the first version of this cache, NULL is ok for the purpose.

549 ↗(On Diff #41402)

I am not sure to understand what you mean.
Do you mean it is badly handled (bug?) or would you like the cache size to be changed during runtime?

sys/netipsec/key.c
549 ↗(On Diff #41402)

I meant that user can specify wrong values and it is bad. I'm not sure that value 8 or 9 is good enough to test the cache. Ah, I understand, SYSCTL_PROC is wrong suggestion, since it is RDTUN, I meant that we need to check the value and probably do not accept values that are below some default.

emeric.poupon_stormshield.eu added inline comments.
sys/netipsec/key.c
549 ↗(On Diff #41402)

A small cache would be probably inefficient, but how can we decide the minimum size to be set? Do you have suggestions? 128, 1024, ...?

8 would be probably OK if you have a huge SPD but very few traffic flows involved at the same time (a huge SPD usually means many different traffic flows though)

I think we can try to test this with wider auditory.

sys/netipsec/ipsec.h
223 ↗(On Diff #41402)

I propose to reuse place of the following two counters, that are unused, for SPD cache. This will keep the structure size and also reduce number of unused counters :)

This revision is now accepted and ready to land.May 13 2018, 1:06 PM
sys/netipsec/ipsec.h
223 ↗(On Diff #41402)

Just to make sure: you want to remove ips_mbcoalesced and ips_clcoalesced?

sys/netipsec/ipsec.h
223 ↗(On Diff #41402)

Yes, they seem to be unused.

This revision now requires review to proceed.May 15 2018, 9:08 AM
This revision was not accepted when it landed; it landed in state Needs Review.May 22 2018, 3:54 PM
This revision was automatically updated to reflect the committed changes.