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