pf: Send syncookies from the receiving thread

[bnovkov]

Summary

pf sends outbound packets by offloading them to a single per-vnet SWI handler
through the V_pf_sendqueue mbuf queue. A large DDoS attack may overwhelm
that per-vnet queue with syncookie packets and cause contention in the SWI
handler that negatively affects other pf operations.

Fix this by sending the initial syncookie challenge from the context
of the receiving thread. This avoids the syncookie-induced contention on
the pf_intr mbuf queue.

Sponsored by: Klara, Inc.
Sponsored by: Entersekt
MFC after: 3 weeks

Test Plan

I've tested this patch by flooding a host using hping3. The two attached pmcstat-generated flamegraphs show that the patch is successfully removing the load from the SWI handler.

baseline.svg113 KBDownload

patched.svg110 KBDownload

[bofh]

Can you check or host the results somewhere else? Maybe freefall?

[bnovkov]

In D59068#1354800, @bofh wrote:

Can you check or host the results somewhere else? Maybe freefall?

Sorry, I forgot that phabricator doesn't automatically attach the referenced files, they should be visible now.

In case they're not, I've also uploaded them to freefall.

[kp]

sys/netpfil/pf/pf.h

756

Why? It doesn't seem to be used anywhere outside of pf.c

757

Pretty much everything in pf works on mbufs. It seems rather pointless to include that in the function name.

I would name this pf_send_ip_direct() so that it's clear that this function will emit the packet directly (as opposed to say pf_send(), pf_send_icmp(), ... which use the swi).

[kp]

I'm also struggling to make sense of the baseline flame graph. It seems to be spending a lot of time removing interfaces (5% of samples in pf_kkif_free()!) and I don't see any calls to pf_test(), which we should see if we were handling traffic.
Maybe the address decoding went wrong, because those stacks just look wrong.

[bnovkov]

Address @kp 's comments

[bnovkov]

sys/netpfil/pf/pf.h

756

thanks for catching this, this is a leftover from a previous iteration of this patch.

[bnovkov]

In D59068#1356482, @kp wrote:

I'm also struggling to make sense of the baseline flame graph. It seems to be spending a lot of time removing interfaces (5% of samples in pf_kkif_free()!) and I don't see any calls to pf_test(), which we should see if we were handling traffic.
Maybe the address decoding went wrong, because those stacks just look wrong.

Hm, that's odd. I'll try to find out what went wrong.

[markj]

In D59068#1356482, @kp wrote:

I'm also struggling to make sense of the baseline flame graph. It seems to be spending a lot of time removing interfaces (5% of samples in pf_kkif_free()!) and I don't see any calls to pf_test(), which we should see if we were handling traffic.
Maybe the address decoding went wrong, because those stacks just look wrong.

It also looks like the samples were taken from a kernel with INVARIANTS enabled. (trash_ctor() wouldn't appear in the profile otherwise.)

[kp]

I think the patch is fine now, but I would like to see the flame graphs anyway, so we're sure this does what we think it does.

[bnovkov]

In D59068#1357257, @bnovkov wrote:

In D59068#1356482, @kp wrote:

I'm also struggling to make sense of the baseline flame graph. It seems to be spending a lot of time removing interfaces (5% of samples in pf_kkif_free()!) and I don't see any calls to pf_test(), which we should see if we were handling traffic.
Maybe the address decoding went wrong, because those stacks just look wrong.

Hm, that's odd. I'll try to find out what went wrong.

I have indeed managed to fumble the kernel config while testing, sorry about that.
I've generated new flamegraphs that have decoded properly.