From owner-freebsd-ipfw@FreeBSD.ORG Fri Feb 29 09:49:03 2008 Return-Path: Delivered-To: freebsd-ipfw@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E0804106566C for ; Fri, 29 Feb 2008 09:49:03 +0000 (UTC) (envelope-from piso@southcross.wired.org) Received: from mail.oltrelinux.com (krisma.oltrelinux.com [194.242.226.43]) by mx1.freebsd.org (Postfix) with ESMTP id 992358FC16 for ; Fri, 29 Feb 2008 09:49:03 +0000 (UTC) (envelope-from piso@southcross.wired.org) Received: from southcross.wired.org (host-62-10-20-127.cust-adsl.tiscali.it [62.10.20.127]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.oltrelinux.com (Postfix) with ESMTP id 1BB7411AE87; Fri, 29 Feb 2008 10:48:56 +0100 (CET) Received: (from piso@localhost) by southcross.wired.org (8.14.2/8.14.1/Submit) id m1T9po74079745; Fri, 29 Feb 2008 10:51:51 +0100 (CET) (envelope-from piso) Date: Fri, 29 Feb 2008 10:51:50 +0100 From: Paolo Pisati To: Vadim Goncharov Message-ID: <20080229095150.GA76592@tin.it> References: <20080228151134.GA73358@tin.it> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) X-Virus-Scanned: by amavisd-new-20030616-p10 (Debian) at krisma.oltrelinux.com Cc: freebsd-ipfw@FreeBSD.org Subject: Re: [patch] ipfw_nat as a kld module X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Feb 2008 09:49:04 -0000 On Fri, Feb 29, 2008 at 05:21:35AM +0000, Vadim Goncharov wrote: > Hi Paolo Pisati! > > On Thu, 28 Feb 2008 16:11:34 +0100; Paolo Pisati wrote about '[patch] ipfw_nat as a kld module': > > > http://people.freebsd.org/~piso/ipfw_nat_module.patch > > Any objection if i commit it? > > Some comments: > > * //comments are not in out style(9) yes, i left some stuff around... :P > * IPFW_NAT_LOADED - again style(9), CAPSLOCK is used for constants and macros... > * lookup_nat() duplication - it is short, may be turn to #define macro in .h? that's doable > * struct ip_fw_chain moved to .h and no longer static, is this good? > I suggest to move into it's own static chain in module, see next the symbol is used outside it's originating file > * Instead of returning IP_FW_NAT function is called immediately from > ipfw_chk(). This inconsistent with other modules of this sort, like divert > and dummynet, where ipfw_chk() simply returns value and cookie to > ipfw_check_*() functions in _pfil.c. If it is done like that, ip_fw2.c > is dependent on modules in minimal way, as many of structures and code > as possible should be moved to modules. This allows to change module > without recompiling main ipfw - for example, your lookup_nat() and > LIST_HEAD from ip_fw_chain could reside entirely in module - then it would > be possible to easily switch from LIST to hash of some kind (imagine 500 > NAT instances). And so on. that's something i thought about, but i didn't see any tangible improvement to this modification, cause part of ipfw_nat would still be called from ipfw2.c (see ipfw_ctl). Anyway, i'll fix a couple of nits and commit as it is. bye, P.