Page MenuHomeFreeBSD

Remove IPSEC from GENERIC due to performance issues
ClosedPublic

Authored by gallatin on May 5 2019, 6:48 PM.

Details

Summary

Having IPSEC compiled into the kernel imposes a non-trivial performance penalty on multi-threaded workloads due to IPSEC refcounting.

In my benchmarks of multi-threaded UDP transmit (connected sockets), I've seen a roughly 20% performance penalty when the IPSEC option is included in the kernel (16.8Mpps vs 13.8Mpps with 32 senders on a 14 core / 28 HTT Xeon 2697v3)). This is largely due to key_addref() incrementing and decrementing an atomic reference count on the default policy. This cause all CPUs to stall on the same cacheline, as it bounces between different CPUs.

Given that relatively few users use ipsec, and that it can be loaded as a module, it seems reasonable to ask those users to load the ipsec module so as to avoid imposing this penalty on the GENERIC kernel. Its my hope that this will make FreeBSD look better in "out of the box" benchmark comparisons with other operating systems.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gallatin created this revision.May 5 2019, 6:48 PM

To make this change less painful for users, it seems like we should enable ifconfig to auto-load ipsec when ifconfig is invoked with ipsecX. ifconfig will currently try to load if_ipsec, and this does not work because the kernel module is ipsec.ko. So it seems like there are 3 obvious workarounds:

  1. teach ifconfig that ipsec is "special", and avoid prepending "if_" to its name when trying to load the kld.
  2. rename ipsec to if_ipsec. That would break users configs, and I don't like it.
  3. link if_ipsec.ko to ipsec.ko. I have heard the existing linking mechanism for em/igb causes issues.

Anything I'm not considering? I tend to think (1) is the simplest option.

cem accepted this revision.May 5 2019, 7:09 PM

No objection to moving IPSEC out of GENERIC (but continuing to build it as a module). Check with gnn@, as he originally added it.

I agree that teaching ifconfig about ipsec (option 1) seems the most straightforward way to provide compatibility through that interface.

This revision is now accepted and ready to land.May 5 2019, 7:09 PM
ae added a comment.May 5 2019, 10:47 PM

I think there are too few users of if_ipsec, to make assumption that all users who use IPsec will use ifconfig(8). AFAIR, it is not the problem, you can just add symlink if_ipsec.ko -> ipsec.ko. But you also need some tweaks that will load ipsec,ko when ipsec_enable is "YES".

jpaetzel accepted this revision.May 5 2019, 11:10 PM
gallatin added a reviewer: gnn.May 6 2019, 1:16 PM
In D20163#434345, @cem wrote:

No objection to moving IPSEC out of GENERIC (but continuing to build it as a module). Check with gnn@, as he originally added it.
I agree that teaching ifconfig about ipsec (option 1) seems the most straightforward way to provide compatibility through that interface.

Thanks. I've added @gnn to the review.

delphij accepted this revision.May 6 2019, 5:57 PM

No objection from me.

jhb added a comment.May 6 2019, 6:18 PM

FWIW, my limited testing of IPsec doesn't use if_ipsec, but instead I used setkey. I think having the rc.d scripts for 'ipsec_enable' autoloading ipsec.ko is reasonable. Maybe if there are ports scripts for IKE daemons (raccoon, strongswan?) those rc.d scripts should also try to load modules they need.

ae added a comment.May 6 2019, 6:51 PM
In D20163#434567, @jhb wrote:

FWIW, my limited testing of IPsec doesn't use if_ipsec, but instead I used setkey. I think having the rc.d scripts for 'ipsec_enable' autoloading ipsec.ko is reasonable.

I added this today in rS347178.

Maybe if there are ports scripts for IKE daemons (raccoon, strongswan?) those rc.d scripts should also try to load modules they need.

I think this should be done by port maintainers.

gnn accepted this revision.May 8 2019, 8:40 AM
In D20163#434567, @jhb wrote:

FWIW, my limited testing of IPsec doesn't use if_ipsec, but instead I used setkey. I think having the rc.d scripts for 'ipsec_enable' autoloading ipsec.ko is reasonable. Maybe if there are ports scripts for IKE daemons (raccoon, strongswan?) those rc.d scripts should also try to load modules they need.

Now that @ae has added this feature to autoload ipsec.ko, are you OK with this change?

jhb added a comment.May 8 2019, 8:11 PM
In D20163#434567, @jhb wrote:

FWIW, my limited testing of IPsec doesn't use if_ipsec, but instead I used setkey. I think having the rc.d scripts for 'ipsec_enable' autoloading ipsec.ko is reasonable. Maybe if there are ports scripts for IKE daemons (raccoon, strongswan?) those rc.d scripts should also try to load modules they need.

Now that @ae has added this feature to autoload ipsec.ko, are you OK with this change?

Oh, yes, sorry I wasn't objecting to the change at all. I was intending to refer to ae@'s commit that I had already seen when I made my original comment as being sufficient.

jhb accepted this revision.May 8 2019, 8:11 PM
gallatin added a reviewer: cy.May 9 2019, 12:33 PM
cy accepted this revision.May 9 2019, 1:18 PM
This revision was automatically updated to reflect the committed changes.