From owner-freebsd-net@FreeBSD.ORG Fri Sep 14 08:14:07 2012 Return-Path: Delivered-To: net@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 740461065673; Fri, 14 Sep 2012 08:14:07 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id EFE3A8FC18; Fri, 14 Sep 2012 08:14:05 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 69ADE7300A; Fri, 14 Sep 2012 10:33:40 +0200 (CEST) Date: Fri, 14 Sep 2012 10:33:40 +0200 From: Luigi Rizzo To: Gleb Smirnoff Message-ID: <20120914083340.GA31420@onelab2.iet.unipi.it> References: <20120912123457.GC85604@glebius.int.ru> <20120912211726.GB10974@onelab2.iet.unipi.it> <20120913174105.GA22571@onelab2.iet.unipi.it> <20120913193125.GO85604@glebius.int.ru> <20120913205721.GA24570@onelab2.iet.unipi.it> <20120914072350.GQ85604@glebius.int.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120914072350.GQ85604@glebius.int.ru> User-Agent: Mutt/1.4.2.3i Cc: luigi@FreeBSD.org, "Bjoern A. Zeeb" , net@FreeBSD.org Subject: Re: moving pfil consumers to sys/netpfil X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Sep 2012 08:14:07 -0000 On Fri, Sep 14, 2012 at 11:23:50AM +0400, Gleb Smirnoff wrote: > On Thu, Sep 13, 2012 at 10:57:21PM +0200, Luigi Rizzo wrote: > L> On Thu, Sep 13, 2012 at 11:31:25PM +0400, Gleb Smirnoff wrote: > L> > On Thu, Sep 13, 2012 at 07:41:05PM +0200, Luigi Rizzo wrote: > L> > L> Second: > L> > L> What i contest is the fact that you classify ipfw as a "pfil client", > L> > L> when pfil is just a tiny adaptation layer to access ipfw. > L> > L> I mentioned the three alternative APIs (netmap, netfilter, ndis) > L> > L> to witness the fact that pfil is irrelevant to ipfw's operation. > L> > > L> > In FreeBSD ipfw is a pfil client. Dot. > L> > > L> > In Linux it is netfilter client, and if they want to they may or may not > L> > put it under netfilter/ipfw. We don't care. Same for Windows and ndis. > L> > L> This is the second time a "we don't care" statement appears in > L> this discussion and I find them extremely non constructive. > > Don't take "we don't care" out of context! We don't care where ipfw > resides in Linux or Windows. I don't see why we should to. We do care > where pfil consumers reside in FreeBSD. i am not taking it out of context, i am saying that deliberately ignoring that this (or other) code is used elsewhere is not a sensible thing to do -- it makes maintainance harder, reduces the chance of getting bugfixes from the outside, etc. Not long ago there was a thread on using wrappers to access ifnet fields so that third parties have an easier time in using our code, and this is the approach that we should follow. So rather than "we don't care" i'd like to read "let's see if we can find a solution that satisfies all". I take your offer to move the code also in stable/9 as a step in this direction. So, to somehow close the issue (as it seems a made decision) > L> I do not know who you consider as "we", but as one who maintains > L> the code under multiple platforms i do have an interest in avoiding > L> gratuitous changes that introduce differences between the various > L> versions. > > The movement is mergeable since userland doesn't depend on it. If you'd > like I could merge movement to stable/9, so you won't have any differencies > between maintained versions. deal. > L> please remain in context. > L> We were talking about the files in netinet/ipfw, which have > L> #include > L> in them, so when moved to the new location these 35 lines > L> need to be changed to > L> #include > L> Which is completely trivial, but introduces a difference between > L> the files in HEAD and those in stable/9 (and in the other versions > L> "we don't care" about) . > > Does ipfw resides in netinet/ipfw on other platforms: Linux, Windows? right now yes (it's an external module, so the only real concern is the hardwired netinet/ in the #includes). I dont mind relocating it there too, as long as it is done everywhere. > L> Now if your plan involves removing the netinet/ prefix and adding > L> ... compile-with "${NORMAL_C} -I$S/netpfil" > L> to the lines in HEAD:sys/conf/files, and > L> ... compile-with "${NORMAL_C} -I$S/netinet" > L> for those in stable/9, i can lift this objection. > > Don't see reason for -I. It compiles okay w/o it. See diff attached. It compiles because you have all these tiny changes: Index: sys/netpfil/ipfw/dn_heap.c =================================================================== --- sys/netpfil/ipfw/dn_heap.c (working copy) +++ sys/netpfil/ipfw/dn_heap.c (working copy) @@ -37,7 +37,7 @@ #include #include #include -#include +#include #ifndef log #define log(x, arg...) #endif and not just a plain 'svn mv'. > L> I still think that a subsystem should not be classified by looking > L> at one of the APIs it uses, but rather based on overall functionality. > L> Hence in my opinion ipfw does not belong under netpfil/ more than > L> it belongs under netinet/ , and lacking a better place i consider > L> the relocation a gratuitous one. > > Ipfw overall functionality is packet filtering. Since packet filtering > can happen at different layers and protocols, none of the following > directories suit perfectly: net, netinet, netinet6. Thus, we (me + Bjoern) > decided to make a separate directory for packet filters. Since all > packet filters attach to kernel via pfil(9) we decided to name > the directory netpfil. Up until now you were only talking about "pfil consumers", and this is what i was disagreeing about. Now you say netpfil/ is the home for "packet filters", which is a different story, and one that makes sense to me (but it also means that it can be used as the home for other packet filters even if they do not use the pfil api). So, it seems we have reached a compromise: netpfil is the home for "packet filters", and ipfw goes there also in stable/9 (or, if it is simpler, we remove the netinet/ prefix from the #include and compile with -I ...). ok ? cheers luigi