[patch] ipfw_nat as a kld module
Paolo Pisati
piso at FreeBSD.org
Fri Feb 29 09:49:04 UTC 2008
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.
More information about the freebsd-ipfw
mailing list