From owner-freebsd-ipfw Tue Mar 7 14:26:41 2000 Delivered-To: freebsd-ipfw@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 6F51D37B836 for ; Tue, 7 Mar 2000 14:26:36 -0800 (PST) (envelope-from robert@cyrus.watson.org) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.9.3/8.9.3) with SMTP id RAA21744; Tue, 7 Mar 2000 17:27:44 -0500 (EST) (envelope-from robert@cyrus.watson.org) Date: Tue, 7 Mar 2000 17:27:43 -0500 (EST) From: Robert Watson X-Sender: robert@fledge.watson.org Reply-To: Robert Watson To: Luigi Rizzo Cc: freebsd-ipfw@freebsd.org Subject: Re: fix to bridging code--m_dup instead of m_copypacket In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-ipfw@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On yet more inspection, it looks like the problem is that bdg_forward() makes mbuf-related calls that rearrange the mbuf buffers, alignment, etc, and if calling code from the interface driver has any pointers cast from the contents of the mbuf, then those pointers will be incorrect following the return of bdg_forward(). In the if_dc driver, a struct ether_header was retrieved using mtod(), and following the bridge call, it no longer pointed to the right data, resulting in the code that followed seeing it as a non-broadcast ethernet address and dropping it. if_xl.c and if_ed.c also use mtod before and after calls to bdg_forward and might suffer from the same problem. if_de.c seems to copy out the ethernet header before the call, and uses the copy for its call to ether_input() and friends; if_ed casts also, but based on its ring buffer, so I'm not sure what the results would be. Either way, the flawed assumption seems to be that the mbuf will remain somewhat the same in structure at the end of the bridge calls, which is clearly not true :-). Recasting following the bridge call eh = mtod(m, struct ether_header *); seems to clear things up. I don't have the various other ethernet devices, so am unable to test to see if this is an issue, but suspect it might be. The problem only arose after a fair amount of network use, so presumably has to do with long term mbuf-allocation trends, et al. Robert On Tue, 7 Mar 2000, Robert Watson wrote: > > On further inspection, the bug still seems to be occurring, although it > took more to break bridging (previously a short flood ping at the bridge > hosts's IP address would break it, now it takes a long one). > > The symptom is that the bridge host stops responding to broadcasts, and > inspection of the mbuf in kernel using a debugger shows that following > bdg_forward, before ether_input is called, the destination ethernet > address in the packet is rewritten, resulting in ether_input rejecting the > packet as not destined for the local host, although bridge_in() did return > BDG_BCAST. I had assumed that the interface output routines in > bdg_forward() were stomping the value (hence switching to m_duping before > sending the mbuf to the output routine), but the problem appears to > remain. > > I'm going to play with it some more, but was wondering if you had seen > this behavior before? > > > On Tue, 7 Mar 2000, Robert Watson wrote: > > > > > Luigi, > > > > I patched support for bridging into if_dc.c and discovered a > > problem--broadcast packets were being correctly bridged, but were not > > being delivered to the bridge itself (?). It looks like some interface > > output code modifies the packet in-place, and copies for per-interface > > delivery were made using m_copypacket instead of m_dup. Changing the code > > to use m_dup instead appeared to work correctly. I'm not sure why the > > packet is getting modified in-place, as it goes out on the bridged segment > > with the broadcast address. Also, it introduces an extra copy for each > > interface a packet is delivered to, which can be large if your MAC address > > working set exceeds the address hash, etc. However, it seems to make > > things much more stable from the perspective of communicating with the > > bridge box. > > > > Here's the patch against -current, which I'd like to commit before the > > release if it looks OK by you. I'd also like to commit bridge support for > > if_dc.c, which prompted me to discover this problem. Assuming this fix is > > good, I'll do that as a followup. > > > > Index: bridge.c > > =================================================================== > > RCS file: /home/ncvs/src/sys/net/bridge.c,v > > retrieving revision 1.16 > > diff -u -r1.16 bridge.c > > --- bridge.c 2000/02/08 14:53:55 1.16 > > +++ bridge.c 2000/03/07 20:16:33 > > @@ -788,10 +788,10 @@ > > if (canfree && once ) { /* no need to copy */ > > m = *m0 ; > > *m0 = NULL ; /* original is gone */ > > - } else /* on a P5-90, m_copypacket takes 540 ticks */ > > - m = m_copypacket(*m0, M_DONTWAIT); > > + } else > > + m = m_dup(*m0, M_DONTWAIT); > > if (m == NULL) { > > - printf("bdg_forward: sorry, m_copy failed!\n"); > > + printf("bdg_forward: sorry, m_dup failed!\n"); > > return ENOBUFS ; /* the original is still there... */ > > } > > /* > > > > > > Robert N M Watson > > > > robert@fledge.watson.org http://www.watson.org/~robert/ > > PGP key fingerprint: AF B5 5F FF A6 4A 79 37 ED 5F 55 E9 58 04 6A B1 > > TIS Labs at Network Associates, Safeport Network Services > > > > > > > Robert N M Watson > > robert@fledge.watson.org http://www.watson.org/~robert/ > PGP key fingerprint: AF B5 5F FF A6 4A 79 37 ED 5F 55 E9 58 04 6A B1 > TIS Labs at Network Associates, Safeport Network Services > > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-ipfw" in the body of the message > Robert N M Watson robert@fledge.watson.org http://www.watson.org/~robert/ PGP key fingerprint: AF B5 5F FF A6 4A 79 37 ED 5F 55 E9 58 04 6A B1 TIS Labs at Network Associates, Safeport Network Services To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-ipfw" in the body of the message