Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 Feb 2008 05:21:35 +0000 (UTC)
From:      Vadim Goncharov <vadim_nuclight@mail.ru>
To:        freebsd-ipfw@freebsd.org
Subject:   Re: [patch] ipfw_nat as a kld module
Message-ID:  <slrnfsf5iv.17n8.vadim_nuclight@hostel.avtf.net>
References:  <20080228151134.GA73358@tin.it>

next in thread | previous in thread | raw e-mail | index | archive | help
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)
* IPFW_NAT_LOADED - again style(9), CAPSLOCK is used for constants
* lookup_nat() duplication - it is short, may be turn to #define macro in .h?
* 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
* 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.

Maybe I missed some points as I was looking briefly...

-- 
WBR, Vadim Goncharov. ICQ#166852181       mailto:vadim_nuclight@mail.ru
[Moderator of RU.ANTI-ECOLOGY][FreeBSD][http://antigreen.org][LJ:/nuclight]




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?slrnfsf5iv.17n8.vadim_nuclight>