FreeBSD NAT-T patch integration [CFR/CFT]

[Bjoern A. Zeeb]

On Wed, 16 Jul 2008, Sam Leffler wrote:

Hi,

> Please test/review the following patch against HEAD:
>
> http://people.freebsd.org/~sam/nat_t-20080616.patch
>
> This adds only the kernel portion of the NAT-T support; you must provide the 
> user-level code from another place.
>
> The main difference from the patches floating around are in the ctloutput 
> path (adding proper locking for HEAD) and decap of ESP-in-UDP frames. 
> Assuming folks are ok w/ these changes I'll commit to HEAD.  Once this stuff 
> goes in we can look at getting the user-mode mods into the tree.

I have skipped through the patch.

My main concern at the moment is the API (pfkey stuff) to userland as
Yvan had stated in <20080626075307.GA1401 at zen.inc>.

I know that at the moment there seems to be one public (pseudo) reference
implementation this all works together but there might be/are other
people not using libipsec from ipsec-tools.

The point is changing the API once this hits the tree will be hard to
detect at a later point if at all (unless with a __FreeBSD_version or
(another) library version bump/sym versioning).


We are still missing other things I think not mentioned elswhere like
partial checksum recalculation.
I still wonder if we'd have all the information (at the right place) in
the kernel so we could easily add support for that at a later time
w/o having to change APIs again. Considering that it seems noone using
this patch in products has implemented this .. I dunno.
It's something that is already mentioned in the introduction of RFC 3947
and in 3.1.2. of 3948 and thus should be very obvious to anyone ever
seriously thought of finishing a proper more than "it works for me"
version of the patch.


Some minor things I had seen not reported so far:

I have seen two printfs that should be changed to proper logging, ...
 	/NAT-T OA present

s,bave,have, in "...in the SPD: This means we bave a non-generated"
but maybe change the entire comment. "non-generated SPD" is kind of
wrong wording.


I'd happily go through another patch once the missing/to be corrected
things were addressed.

/bz

-- 
Bjoern A. Zeeb              Stop bit received. Insert coin for new game.