esp_input() and esp_output() allocated a cryptop from the crypto layer,
then malloc'ed the xform_data the callback needs, and with ESN a third
buffer for the separate AAD. The callback freed all three. That is
two or three allocation pairs for every packet.

Put the three in one structure, allocated once per packet from a
dedicated UMA zone. The crypto layer provides crypto_initreq() and
crypto_destroyreq() for an embedded cryptop, as wg(4) and ktls use.
The AAD buffer is sized for the ESP header, the longest IV and the
high-order half of the sequence number, which is all the separate AAD
can hold.

No functional change intended.

MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

rcm created this revision.Tue, Nov 10, 3:46 PM

Herald added subscribers: glebius, melifaro, ae, imp. · View Herald TranscriptTue, Nov 10, 3:46 PM

rcm requested review of this revision.Tue, Nov 10, 3:46 PM

markj added inline comments.Wed, Nov 11, 8:02 PM

sys/netipsec/xform_esp.c
147	You aren't including space for the high-order half of the sequence counter here.
159	Is it worth using a dedicated UMA zone here? That would shave off a bit more work per packet. You'd also get better memory efficiency, though I'm not sure that matters very much given that these structures have a fairly short lifetime.
448	Can you just pass the `req` structure here instead? Then you don't need to use containerof in esp_freereq() when freeing the structure from the callback, you can free it directly.

Size the AAD buffer for the high-order half of the sequence number as well.
Allocate struct esp_req from a dedicated UMA zone, created in esp_attach() and destroyed in esp_detach() after xform_detach(). M_ESP is gone.
Pass the esp_req as crp_opaque, so esp_freereq() takes it directly and no longer needs __containerof().

rcm added inline comments.Wed, Nov 11, 8:47 PM

sys/netipsec/xform_esp.c
147	ah yep, thanks!
159	yeah, that seemed to shave a bit more work per packet. cool
448	doh, yes. thanks

markj accepted this revision.Wed, Nov 11, 8:52 PM

markj added inline comments.

sys/netipsec/xform_esp.c
1096	These blank lines at the beginning of functions aren't needed.
1113	It's nicer to keep these macros paired with the function they are calling when possible, so static void esp_attach(void arg __unused) { ... } SYSINIT(...); static void esp_detach(void arg __unused) { ... } SYSUNINIT(...);