From owner-svn-src-head@FreeBSD.ORG Mon Apr 30 13:19:17 2012 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id E419B106564A; Mon, 30 Apr 2012 13:19:17 +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 631B68FC14; Mon, 30 Apr 2012 13:19:17 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id A0D627300A; Mon, 30 Apr 2012 15:39:04 +0200 (CEST) Date: Mon, 30 Apr 2012 15:39:04 +0200 From: Luigi Rizzo To: "Alexander V. Chernikov" Message-ID: <20120430133904.GA65792@onelab2.iet.unipi.it> References: <201204301022.q3UAMNcq060049@svn.freebsd.org> <20120430114836.GA62284@onelab2.iet.unipi.it> <20120430113610.GC18777@FreeBSD.org> <4F9E82DA.4030406@FreeBSD.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F9E82DA.4030406@FreeBSD.org> User-Agent: Mutt/1.4.2.3i Cc: svn-src-head@FreeBSD.org, ae@FreeBSD.org, svn-src-all@FreeBSD.org, Gleb Smirnoff , src-committers@FreeBSD.org Subject: Re: svn commit: r234834 - in head/sys: contrib/pf/net net netinet netinet/ipfw ofed/drivers/infiniband/ulp/ipoib X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Apr 2012 13:19:18 -0000 On Mon, Apr 30, 2012 at 04:17:30PM +0400, Alexander V. Chernikov wrote: > On 30.04.2012 15:36, Gleb Smirnoff wrote: > >On Mon, Apr 30, 2012 at 01:48:36PM +0200, Luigi Rizzo wrote: > >L> On Mon, Apr 30, 2012 at 10:22:23AM +0000, Alexander V. Chernikov wrote: > >L> > Author: melifaro > >L> > Date: Mon Apr 30 10:22:23 2012 > >L> > New Revision: 234834 > >L> > URL: http://svn.freebsd.org/changeset/base/234834 > >L> > > >L> > Log: > >L> > Move several enums and structures required for L2 filtering from > >ip_fw_private.h to ip_fw.h. > >L> > >L> I would be really grateful if you could revert this back and discuss > >L> what you wanted to achieve with this change other than saving one > >L> entry in the list of includes. > Changing something inside ip_fw_private.h (for example, locking change) > requires changes in several totally unrelated subsystems, which is > clearly bad. There are certainly good reasons to split things even further (as Gleb suggested) so if you want to follow that route and have patches for review i will be glad to discuss that. I am the first one to say that the name ip_fw_private.h is confusing, but at the time i did not feel brave enough to move from a single header (ip_fw.h) to the four (ip_fw.h, ip_fw_user.h, ip_fw_kernel.h, ip_fw_kernel_private.h) that would be needed for proper confinement of information. This said, the change you have made introduce a worse form of header pollution and this is why i am requesting a backout. > >L> > >L> As clearly mentioned in the commit logs > >L> > >L> http://svnweb.freebsd.org/base?view=revision&revision=200580 > Maybe there are some other possibilities documenting preferred layout > other than commit log? Searching 2+yrs commit history is not the best > way of finding information. sure, you could have asked people involved with ipfw development to review this change. Also don't expect a single policy in style(9) to hold for all possible situations: the kernel is huge, it has parts that come from multiple origins in time and space, and sometimes is shared with other OSs as well. Refactoring (such as your changes) should keep that in mind. Let's not make a big deal of this thing: we all make mistakes or have different opinions on how things should be done, and the only way to avoid them is to discuss thing in advance, or be open to resolve things when problems arise. No hard feelings. cheers luigi