Date: Tue, 7 Mar 2000 17:27:43 -0500 (EST) From: Robert Watson <robert@cyrus.watson.org> To: Luigi Rizzo <luigi@info.iet.unipi.it> Cc: freebsd-ipfw@freebsd.org Subject: Re: fix to bridging code--m_dup instead of m_copypacket Message-ID: <Pine.NEB.3.96L.1000307172226.16458F-100000@fledge.watson.org> In-Reply-To: <Pine.NEB.3.96L.1000307153334.16458E-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1000307172226.16458F-100000>