From owner-cvs-all@FreeBSD.ORG Sat Nov 15 00:29:22 2003 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id CC4D516A4CE; Sat, 15 Nov 2003 00:29:22 -0800 (PST) Received: from xorpc.icir.org (xorpc.icir.org [192.150.187.68]) by mx1.FreeBSD.org (Postfix) with ESMTP id E1F5643FA3; Sat, 15 Nov 2003 00:29:21 -0800 (PST) (envelope-from rizzo@xorpc.icir.org) Received: from xorpc.icir.org (localhost [127.0.0.1]) by xorpc.icir.org (8.12.9p1/8.12.3) with ESMTP id hAF8TLFw073787; Sat, 15 Nov 2003 00:29:21 -0800 (PST) (envelope-from rizzo@xorpc.icir.org) Received: (from rizzo@localhost) by xorpc.icir.org (8.12.9p1/8.12.3/Submit) id hAF8TL9f073786; Sat, 15 Nov 2003 00:29:21 -0800 (PST) (envelope-from rizzo) Date: Sat, 15 Nov 2003 00:29:21 -0800 From: Luigi Rizzo To: Andre Oppermann Message-ID: <20031115002921.B68056@xorpc.icir.org> References: <200311142102.hAEL2Nen073186@repoman.freebsd.org> <20031114153145.A54064@xorpc.icir.org> <3FB593F5.1053E7E2@pipeline.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <3FB593F5.1053E7E2@pipeline.ch>; from oppermann@pipeline.ch on Sat, Nov 15, 2003 at 03:48:21AM +0100 cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet in_var.h ip_fastfwd.c ip_flow.c ip_flow.h ip_input.c ip_output.c src/sys/sys mbuf.h src/sys/conf files src/sys/net if_arcsubr.c if_ef.c if_ethersubr.c if_fddisubr.c if_iso88025subr.c if_ppp.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Nov 2003 08:29:23 -0000 [mu On Sat, Nov 15, 2003 at 03:48:21AM +0100, Andre Oppermann wrote: > Luigi Rizzo wrote: ... > > Given that there are large segments of common code between > > ip_fastforward() and ip_input()/ip_output() (i am thinking of the > > entire ipfw handling code, for one, and also some basic integrity > > checks, the fragmentation code, etc.) I also wonder if it wouldn't > > be beneficial to put the optimizations into the standard path rather > > than create a new (partial) replica of the same code, with the > > potential of introducing bugs, and with some substantial I-cache > > pollution which might well destroy the benefits of minor optimizations. > > I don't see much cache pollution here. Normally you use ip_fastforward i said I-cache, not data cache. Even a routed does some substantial amount of local communication (bgp and routing processes etc.) so i am pretty sure that in any non-trivial case you will end up having both the slow path and the fast path conflicting for the instruction cache. Merging them might help -- i have seen many cases where inlining code as opposed to explicit function calls makes things slower for this precise reason. > > Minor comments on the code: > > > > + one of the initial comments in the new code states > > > > ... The only part of the packet we touch with the CPU is the > > IP header. ... > > > > this is not true if you use ipfw because that code touches many > > places in the packet (and can also do some expensive computation > > like trying to locate the uid/gid of a packet; the fact that we > > only deal with packets not for us does not prevent the existence > > of such firewall rules). > > Well, as I said, everybody is free to shoot himself with such highly > complex firewall rules. I'd say the ipfw code could be optimized with > some of the ideas I've specified earlier. I don't think the ipfw code > would do a uid/gid lookup if neither the destination nor source is i was just saying that the comment is untrue. > > + could you clarify the divert logic ? I am a bit rusty with that > > part of the code, but i am under the impression that in > > ip_fastforward() you are passing along args.divert_rule and > > losing track of divert_info which is instead what you need too. > > It's not you being rusty, the code is indeed hard to follow. :-/ > divert_info is used for ip packet reassembly. ip_divert() is then > just using it to determine whether the packet was catched on the > way into the machine or out of it. It seems to have it's largest > significance for the ip reassembly. I've tested that too with an > earlier version of my code. However I will redo those tests to be > sure it is working as expected. ok, the specific case where i think it fails is when you divert a fragmented packet -- your code seems to store the divert_info (the port you divert to) into divert_rule, and lose track of the former. cheers luigi