Reduce overhead of IPSEC on socket creation and destruction
AbandonedPublic
Actions

Authored by eri on Jul 7 2015, 7:15 PM.

Details

Reviewers

ae
rrs
gnn

Group Reviewers

network

Summary

With IPSEC enabled in GENERIC kernels the socket creation and destruction takes a penalty on delay since the IPSEC environment preparation is done unconditionally even though the socket did not ask to encapsulate its traffic with IPSEC.

Eliminate this overhead/latency induction by not preparing this structures unless IP_IPSEC_POLICY socket option is triggered.
Introduce an EVENTHANDLER for destorying or duplicating as in TCP syncache code any IPSEC related information attached with a socket, if any.

With this patch most of the IPSEC code sprinkled around PCB is removed and the various #ifdef IPSEC are really not needed but just left to identify that code is related to it.
Also this is a step forward on making IPSEC a module.

The SCTP changes i have not really tested but looking at the comments its state is unknown.

NOTE: This changes sizeof(struct inpcb) as well. NOTE2: Might also be good to split all this IPSEC PCB code into its own file.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

eri updated this revision to Diff 6760.Jul 7 2015, 7:15 PM

eri retitled this revision from to Reduce overhead of IPSEC on socket creation and destruction.

eri updated this object.

eri edited the test plan for this revision. (Show Details)

eri added reviewers: gnn, ae.

eri set the repository for this revision to rS FreeBSD src repository - subversion.

eri added a project: network.

Herald added a subscriber: imp. · View Herald TranscriptJul 7 2015, 7:15 PM

Actually the sizeof(struct inpcb) is the same. So, there is no ABI breakage.
But dividing SPD into two pieces leads to the problems. The new socket's SPD doesn't support some features, e.g. expiration time. Also, can you explain why did you use event handlers there? I see no advantage of this choice.

Really this is a weird use case though the expiry will not be supported on SPD but SAs will still be the same way usable.
The code also needs a follow-up patch to properly make the SPD matched only by the socket that configured from the policy, today code tries to do something that i am almost complete its broken in behaviour by overriding the policy that might have been applied from application.

The event handler usage is to allow TCP code to not depend on IPSEC code.
This allows later on to have the code contained into netipsec and make it a module.
The only difficulty for making an ipsec module after is rewriting TCP_MD5/SIGNATURE code to not rely on netipsec since really was a bad choice at the time.

gnn added a reviewer: rrs.Jul 27 2015, 1:54 PM

gnn added a reviewer: network.Sep 22 2015, 3:00 PM

So there are a few issues here, but I have to dig deeper on some of the issues. Please respond to the smaller questions in line while I, and others, do that.

sys/netinet/in_pcb.h
194	Why did you remove the type from this? I can see no reason for this change.
sys/netipsec/ipsec.c
474	Don't you mean && here?

See my replies!

sys/netinet/in_pcb.h
194	Just to avoid the forward declaration.
sys/netipsec/ipsec.c
474	No its meant as \|\|

I think the SCTP change are ok, I will flag this to Michael when you commit it (the code
in the FreeBSD netinet directory is not the base code..). The key here is its *very* doubtful
that SCTP and IPSEC can work. This is due to the multi-homing and the fact that once you
turn on IPSEC you would not be able to do un-ordered or partial reliability. There is
nothing in the SCTP stack to restrict this so, if a user tried this the association would fail.
DTLS is the right answer for SCTP not IPSEC.. as exemplified in all the deployment of
SCTP over DTLS over UDP over Stun/Ice in firefox/chrome using our SCTP code ;-)

eri abandoned this revision.Feb 20 2017, 11:43 PM