moving pfil consumers to sys/netpfil

[Luigi Rizzo]

On Thu, Sep 13, 2012 at 11:31:25PM +0400, Gleb Smirnoff wrote:
> On Thu, Sep 13, 2012 at 07:41:05PM +0200, Luigi Rizzo wrote:
> L> Second:
> L>    What i contest is the fact that you classify ipfw as a "pfil client",
> L>    when pfil is just a tiny adaptation layer to access ipfw.
> L>    I mentioned the three alternative APIs (netmap, netfilter, ndis)
> L>    to witness the fact that pfil is irrelevant to ipfw's operation.
> 
> In FreeBSD ipfw is a pfil client. Dot.
> 
> In Linux it is netfilter client, and if they want to they may or may not
> put it under netfilter/ipfw. We don't care. Same for Windows and ndis.

This is the second time a "we don't care" statement appears in
this discussion and I find them extremely non constructive.
I do not know who you consider as "we", but as one who maintains
the code under multiple platforms i do have an interest in avoiding
gratuitous changes that introduce differences between the various
versions.

> ipfw isn't netinet specific, it also supports netinet6 and also supports
> layer2 pfil hooks.
> 
> Putting all pfil clients into one place puts things in order. It simplifies
> kernel overview for newcomer, it makes things easier for those who want
> to hack on the pfil itself.
> 
> L> Third:
> L>    any code relocation comes with some pain -- specifically, netinet/
> L>    is hardwired in many #include paths in the ipfw sources
> L> 
> L> 	> grep netinet/ipfw sys/netinet/ipfw/* | grep include | wc
> L> 	      35      70    1791
> 
> Numbers from your grep are impressive, but they are absolutely irrelevant.
> Actual diff for movement isn't that big. For example:
> 
> - sys/net has zero diff, no files modified
> - sys/netinet/* has zero diff, just ipfw directory removed, no files modified
> - sbin/ipfw has zero diff

please remain in context.
We were talking about the files in netinet/ipfw, which have
	#include <netinet/ipfw/something>
in them, so when moved to the new location these 35 lines
need to be changed to 
	#include <netpfil/ipfw/something>
Which is completely trivial, but introduces a difference between
the files in HEAD and those in stable/9 (and in the other versions
"we don't care" about) .

Now if your plan involves removing the netinet/ prefix and adding
	... compile-with "${NORMAL_C} -I$S/netpfil"
to the lines in HEAD:sys/conf/files, and
	... compile-with "${NORMAL_C} -I$S/netinet"
for those in stable/9, i can lift this objection.

I still think that a subsystem should not be classified by looking
at one of the APIs it uses, but rather based on overall functionality.
Hence in my opinion ipfw does not belong under netpfil/ more than
it belongs under netinet/ , and lacking a better place i consider
the relocation a gratuitous one.

> This is actual diff that is already universe-tested.

(attachment probably stripped by the mailing list ?)

cheers
luigi