FreeBSD NAT-T patch integration [CFR/CFT]

[Sam Leffler]

Bjoern A. Zeeb wrote:
> 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.

Please send me your specific issues; I haven't seen any comments about 
"partial checksum recalculations".

> 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.

I don't see any of the above blocking this work going in.  Forcing 
people to maintain out-of-tree patches for years because of vague 
concerns is unproductive.  This code is used by at least 2 vendors 
shipping products.

>
>
> 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.
>
Please apply your changes to the p4 branch or fix 'em when the code hits 
CVS.  I've see no concrete rationale for holding this work out.

    Sam