From owner-freebsd-current@FreeBSD.ORG Tue Jun 22 12:00:51 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 038EA16A4E4 for ; Tue, 22 Jun 2004 12:00:51 +0000 (GMT) Received: from mailtoaster1.pipeline.ch (mailtoaster1.pipeline.ch [62.48.0.70]) by mx1.FreeBSD.org (Postfix) with ESMTP id 379EA43D54 for ; Tue, 22 Jun 2004 12:00:50 +0000 (GMT) (envelope-from andre@freebsd.org) Received: (qmail 74307 invoked from network); 22 Jun 2004 12:00:49 -0000 Received: from unknown (HELO freebsd.org) ([62.48.0.53]) (envelope-sender ) by mailtoaster1.pipeline.ch (qmail-ldap-1.03) with SMTP for ; 22 Jun 2004 12:00:49 -0000 Message-ID: <40D81F76.296C2A4@freebsd.org> Date: Tue, 22 Jun 2004 14:00:54 +0200 From: Andre Oppermann X-Mailer: Mozilla 4.8 [en] (Windows NT 5.0; U) X-Accept-Language: en MIME-Version: 1.0 To: Luigi Rizzo References: <40D754D5.1070805@freebsd.org> <20040622025054.B18617@xorpc.icir.org> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: freebsd-net@freebsd.org cc: freebsd-current@freebsd.org Subject: Re: New preview patch for ipfw to pfil_hooks conversion X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Jun 2004 12:00:51 -0000 Luigi Rizzo wrote: > > On Mon, Jun 21, 2004 at 11:36:21PM +0200, Andre Oppermann wrote: > > Here is the next preview patch for the ipfw to pfil_hooks conversion: > > > > http://www.nrg4u.com/freebsd/ipfw-pfilhooks-and-more-20040621.diff > > > > This patch significantly cleans up ip_input.c and ip_output.c. > > > > assorted comments: > - understood that this is WIP, but as someone else suggested it would > be a lot better to split the patch in logical chunks and apply them > one at a time. Especially because they seem to have different importance, > e.g. (as far as i can tell): Sure thing that I will split it up. It's just more convinient for me to diff up one big patch at the moment. > + ipfw pfil stuff > i surely consider this extremely useful and welcome, but would > prefer to put the hooks in ip_fw2.c instead of using a > separate file -- both to keep ipfw stuff confined, and to > use stricter compiler checks (e.g. define stuff static as > much as possible, avoid exporting internal APIs, etc.) > The only motivation against it would be if we plan to > backport this stuff to 4.x where we still have the option > of using ipfw1. I don't have any plans to backport it to 4.x. I chose to work with an external file at the moment for convinience. This way I don't have to scroll through thousands of lines each time. > + ip_reass replacement > > + ip options processing > on this particular one i am a bit unsure -- what is the point > for just moving stuff to a separate file instead of leaving > it where it is (ip_input/ip_output) so that many functions > that are only used there can be declared static as they are now ? > I'd rather just apply bugfixes. There are no bugs except one static variable which is used for packets in transit. I'll convert that to an m_tag before it goes in. Generally the ip_input.c and ip_output.c files have become very large and moving the ip options related functions out to their own file is makes it a lot more readable and sorted/ordered. IP options are only rarely used these days. Most of the functions either take an mbuf or a socket so there is not much to break. > generally, i have become a big fan of very strict compiler checks -- lately > they have saved me a huge amount of time in identifying dead code, > inconsistent interfaces and other bugs, so when in doubt between two > alternatives i tend to privilege the one that gives more chances to > the compiler to check things. Yea, on the hand we don't want to have everything in one file because that would make the checks as useful as not doing it. Normally in my coding style I prefer the safe variants, like bzero'ing entire structs instead of doing it piece-meal throughout the code that follows. Because one day someone will change the order of some code parts and *poof* things break in subtile and hard to debug ways. > > o ipfw forward is not yet implemented again (comes next) > > o ipfw layer2 is not yet implemented again (comes next) > > of course it is fundamental to preserve the entire existing > functionality before the commit Sure. As I said, this is WIP. And when I reach the point of current functionality == new functionality then I'll go and commit it. -- Andre