[Fwd: IPFW PATCH: make the IPFW_DEFUALT_RULE number constant non private]

[Max Laier]

On Sunday 24 August 2008 01:18:37 Roman Kurakin wrote:
> Luigi Rizzo wrote:
> > On Sun, Aug 24, 2008 at 02:03:50AM +0400, Roman Kurakin wrote:
> > ...
> >
> >>> unless the tools you have in mind already include ip_fw.h (in which
> >>> case the change is harmless and I have no objection), i think it would
> >>> be better to export the value in a sysctl and let the tools fetch it
> >>> from there, so they do not need to include the header.
> >>
> >> In fact, I am talking about ipfw(8) and natd(8). The first one uses
> >> hard-coded value, the last one
> >> pass rulenumbers to libalias(3) without a check, libalias(3) flushes
> >
> > ...
> >
> >> IIRC the natd(8) doesn't include ip_fw.h, but I do not see why it would
> >> be better to export
> >> this value via sysctl, not compiled in via #include<> for it. The only
> >
> > because ip_fw.h has a lot of stuff in it which in turn is likely
> > to require more headers to build correctly. This is unnecessary bloat
> > at compile time, and also less practical than the sysctl-based approach
> > because it requires a binary update if the value ever changes.
> >
> > Besides, implementing the sysctl does not require to change the
> > headers (thus saving rebuilds of the objects that depend on it),
> > is one line in one file (ip_fw2.c) in the kernel, and one line to
> > call sysctlbyname() in each of the userland program, which you'd
> > need to change anyways; and because both ipfw(8) and natd(8) already
> > use sysctl/sysctlbyname to read/write other similar values from the
> > kernel, you are more consistent with the existing code and don't
> > even need additional headers.
>
> Agreed, but including ip_fw.h for natd.c does not require any additional
> headers.
> All that needed looks like already there.
> If some one point me to other utilities that need to be fixed I'll also
> a sysctl for them.
> These two already include enough of netinet includes, so if you not
> strictly against adding
> just one more include for natd, I'll prefer not to add a sysctl by right
> now.

There is no guarantee that the kernel or ipfw module you are running was built 
with the installed ip_fw.h or that IPFW_DEFAULT_RULE wasn't defined 
differently in the build environment.  So a sysctl (or the like) is the *only* 
correct approach.  Please fix it for real and don't just slap band-aid at it.  
Nice catch, by the way!

> rik
>
> > At runtime the sysctl is obviously more expensive than reading a
> > hardwired constant, but still a whole lot cheaper than the subsequent
> > action (adding a rule to the firewall) so one won't notice.
> >
> > 	cheers
> > 	luigi
> > _______________________________________________
> > freebsd-net at freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-net
> > To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
>
> _______________________________________________
> freebsd-net at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"

-- 
/"\  Best regards,                      | mlaier at freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News