From owner-freebsd-net Wed Nov 22 21:13:45 2000 Delivered-To: freebsd-net@freebsd.org Received: from iguana.aciri.org (iguana.aciri.org [192.150.187.36]) by hub.freebsd.org (Postfix) with ESMTP id D141637B4CF for ; Wed, 22 Nov 2000 21:13:42 -0800 (PST) Received: (from rizzo@localhost) by iguana.aciri.org (8.11.1/8.11.1) id eAN5DJh01421; Wed, 22 Nov 2000 21:13:19 -0800 (PST) (envelope-from rizzo) From: Luigi Rizzo Message-Id: <200011230513.eAN5DJh01421@iguana.aciri.org> Subject: Re: PATCH REVIEW Re: bug in bridging/dummynet code - PR kern/19551 In-Reply-To: from Bosko Milekic at "Nov 22, 2000 4: 1:52 pm" To: bmilekic@technokratis.com (Bosko Milekic) Date: Wed, 22 Nov 2000 21:13:19 -0800 (PST) Cc: tmoestl@gmx.net, cuk@cuk.nu, rizzo@aciri.org, freebsd-net@freebsd.org X-Mailer: ELM [version 2.4ME+ PL43 (25)] MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org > Thomas, I have looked at and reviewed your patch, and have added one > hunk to bridge.c, please take a look at the "almost ready for commit" > version here: > > http://people.FreeBSD.org/~bmilekic/dumbridge.diff sounds ok to me (modulo testing). > Basically, I don't like the fact that we have to copy the ethernet > header back into the mbuf for dummynet in bdg_forward(), especially since > we just removed it before calling bdg_forward(). But, this is no fault of this was also part of my criticism to the original changes. I posted a suggestion for keeping the ethernet header together with the mbuf when calling ether_input() (in case, define a new function ether_input2() if backward compatibility was a concern) to avoid the need for M_PREPEND, and also (probably) to save some code in the in the individual drivers where the ethernet splitting is replicated and could be centralized in ether_input2(). But this is some change that now would require a lot of work in touching the individual drivers. thanks Bosko and Thomas for your work cheers luigi ----------------------------------+----------------------------------------- Luigi RIZZO, luigi@iet.unipi.it . ACIRI/ICSI (on leave from Univ. di Pisa) http://www.iet.unipi.it/~luigi/ . 1947 Center St, Berkeley CA 94704 Phone: (510) 666 2927 ----------------------------------+----------------------------------------- > yours and since dummynet does expect it, I agree with the present fix. > > I'd like to ask net@ and Luigi to also review this before I commit > it, and I'd like to ask Marko Cuk (and others?) to test it and confirm > once again that it indeed fixes their problems, just so I can close all > the PRs along with it. > > Thanks, Thomas! > > On Wed, 22 Nov 2000, Thomas Moestl wrote: > > > Well, the first attempt I sent last week did apparently fix the panics, but > > there was another logical error, this time in bridge.c: dummynet expects > > an ehternet header at the start of the mbuf, but the bridging code did > > not place it there. I wrote a second patch a week ago, and sent it to > > Marko Cuk, who had asked for it. Looking into freebsd-net, I see that > > Lachlan O'Dea has come to the same conclusion. > > > > Marko Cuk has apparently not tested the patch yet, but term@rmci.net has > > filed a bug report (kern/23010) today about very much the same thing. He > > has reported success with my patch, so I thought it might be time to > > try to get it in, so I have attached it ;-) > > > > This should close the PRs kern/21534, kern/23010 and (probably only partly) > > kern/19551. Marko Cuk's problem seems to be exactly the same to me: > > he also uses pipe firewall rules, and the panics happened when he activated > > the firewalling for bridging. He also reported partial success (no more > > panics) with my first patch. > > > > So, to come to a conclusion: could you please review (and perhaps commit) the > > patch or refer me to right person to bug about it? > > > > Thanks for your time, > > - Thomas > > Regards, > Bosko Milekic > bmilekic@technokratis.com > > > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message