From owner-freebsd-net@FreeBSD.ORG Mon Jul 21 15:20:57 2008 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id AD2851065670 for ; Mon, 21 Jul 2008 15:20:57 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from ebb.errno.com (ebb.errno.com [69.12.149.25]) by mx1.freebsd.org (Postfix) with ESMTP id 4B8088FC14 for ; Mon, 21 Jul 2008 15:20:57 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from trouble.errno.com (trouble.errno.com [10.0.0.248]) (authenticated bits=0) by ebb.errno.com (8.13.6/8.12.6) with ESMTP id m6LFKs9w021931 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 21 Jul 2008 08:20:55 -0700 (PDT) (envelope-from sam@freebsd.org) Message-ID: <4884A956.5060108@freebsd.org> Date: Mon, 21 Jul 2008 08:20:54 -0700 From: Sam Leffler Organization: FreeBSD Project User-Agent: Thunderbird 2.0.0.9 (X11/20071125) MIME-Version: 1.0 To: "Bjoern A. Zeeb" References: <20080630040103.94730.qmail@mailgate.gta.com> <486A45AB.2080609@freebsd.org> <487EC62A.3070301@freebsd.org> <20080721085325.B57089@maildrop.int.zabbadoz.net> In-Reply-To: <20080721085325.B57089@maildrop.int.zabbadoz.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-DCC-sonic.net-Metrics: ebb.errno.com; whitelist Cc: freebsd-net@freebsd.org, vanhu_bsd@zeninc.net, Larry Baird Subject: Re: FreeBSD NAT-T patch integration [CFR/CFT] X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Jul 2008 15:20:57 -0000 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@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