From owner-svn-src-all@FreeBSD.ORG Thu May 3 09:04:55 2012 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx2.freebsd.org (mx2.freebsd.org [69.147.83.53]) by hub.freebsd.org (Postfix) with ESMTP id 9CB30106566C; Thu, 3 May 2012 09:04:55 +0000 (UTC) (envelope-from melifaro@FreeBSD.org) Received: from dhcp170-36-red.yandex.net (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx2.freebsd.org (Postfix) with ESMTP id D20951508CC; Thu, 3 May 2012 09:04:53 +0000 (UTC) Message-ID: <4FA24920.4080809@FreeBSD.org> Date: Thu, 03 May 2012 13:00:16 +0400 From: "Alexander V. Chernikov" User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:8.0) Gecko/20111117 Thunderbird/8.0 MIME-Version: 1.0 To: Luigi Rizzo References: <201204301022.q3UAMNcq060049@svn.freebsd.org> <20120430114836.GA62284@onelab2.iet.unipi.it> <20120430113610.GC18777@FreeBSD.org> <4F9E82DA.4030406@FreeBSD.org> <20120430133904.GA65792@onelab2.iet.unipi.it> In-Reply-To: <20120430133904.GA65792@onelab2.iet.unipi.it> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 May 2012 09:04:55 -0000 On 30.04.2012 17:39, Luigi Rizzo wrote: > 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. Yup. I'll make another patch > > 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. Reverted in r234946 > >>> 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. Ok :) > > cheers > luigi > -- WBR, Alexander