From owner-freebsd-stable Wed Nov 15 14:47:16 2000 Delivered-To: freebsd-stable@freebsd.org Received: from angmar.mel.vet.com.au (angmar.mel.vet.com.au [203.103.154.62]) by hub.freebsd.org (Postfix) with ESMTP id 3530037B479 for ; Wed, 15 Nov 2000 14:47:02 -0800 (PST) Received: from anduril.mel.vet.com.au (anduril.mel.vet.com.au [203.103.154.58]) by angmar.mel.vet.com.au (Postfix) with ESMTP id 2D82D5A1C3 for ; Thu, 16 Nov 2000 09:46:55 +1100 (EST) Received: (from ulmo@localhost) by anduril.mel.vet.com.au (8.9.3/8.9.3) id JAA02016 for freebsd-stable@FreeBSD.ORG; Thu, 16 Nov 2000 09:46:28 +1100 Date: Thu, 16 Nov 2000 09:46:28 +1100 From: "Lachlan O'Dea" To: freebsd-stable@FreeBSD.ORG Subject: Re: Bridging code in 4.2RC1 still not fixed Message-ID: <20001116094627.B1761@vet.com.au> Mail-Followup-To: freebsd-stable@FreeBSD.ORG References: <3A12777E.DEA187E0@pcx.si> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="+HP7ph2BbKc20aGI" Content-Disposition: inline User-Agent: Mutt/1.3.9i In-Reply-To: <3A12777E.DEA187E0@pcx.si>; from cuk@pcx.si on Wed, Nov 15, 2000 at 12:46:06PM +0100 X-Operating-System: Linux 2.2.14-5.0 i686 Sender: owner-freebsd-stable@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG --+HP7ph2BbKc20aGI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Nov 15, 2000 at 12:46:06PM +0100, Marko Cuk wrote: > I know, Bosko, that I still didn't send you debugging information about > it, but I'm trying to do that in near future. > > Read the following forward, please. Bridging and dummynet is definitely broken right now. The code in ip_dummynet.c referenced below is wrong, however it will only be executed if you actually add a dummynet pipe. I'm not sure if this is the same problem as the original poster was talking about. Luigi Rizzo is aware of the dummynet+bridging proplem, but he simply doesn't have time to work on it right now. I'm sure he'll fix it as soon as he is able to. I've attached an email to Luigi with my thoughts on the problem, with a patch that seemed to resolve the panic on my machine. However, I don't think the patch is actually correct. > Marko Cuk > Date: Wed, 15 Nov 2000 12:31:07 +0100 > From: Marko Cuk > To: tmoestl@gmx.net, bmilekic@dsuper.net > Subject: Re: bug in bridging/dummynet code - PR kern/19551 (fwd) > Message-ID: <3A1273FB.2C1C40A5@pcx.si> > Organization: Pcx computers d.o.o., Tehnika > X-Mailer: Mozilla 4.74 [en] (Windows NT 5.0; U) > X-Accept-Language: en > MIME-Version: 1.0 > References: > Content-Type: text/plain; charset=iso-8859-2 > Content-Transfer-Encoding: 7bit > > Hello !! > > I tried that "fix" and maschine is now stable and won't crash, but > dummynet wont work. When I insert rule > ipfw add 5000 pipe 1 tcp from any to any > > only ICMP packets goes through, but maschine remains stable. > > Can I test something more ? Do you have additional ideas ? > > Inform me ASAP. > > > Regards, Cuk > > > > > Date: Tue, 14 Nov 2000 22:01:17 +0100 > > From: Thomas Moestl > > To: freebsd-net@freebsd.org > > Cc: bmilekic@dsuper.net > > Subject: bug in bridging/dummynet code - PR kern/19551 > > > > Hi, > > > > I think I have spotted a bug in the bridge/dummynet code that might be > > responsible for some panics people have reported recently - see e.g. > > PR kern/19551. > > PR kern/21534 seems related and are probably about the same thing, > > PR kern/19488 goes in the same direction. > > > > Bosko, I'm CCing this to you because the PR is currently assigned to > > you. > > > > Here is the relevant section of code from netinet/ip_dummynet.c:402: > > > > #ifdef BRIDGE > > case DN_TO_BDG_FWD : { > > struct mbuf *m = (struct mbuf *)pkt ; > > struct ether_header hdr; > > > > if (m->m_len < ETHER_HDR_LEN > > && (m = m_pullup(m, ETHER_HDR_LEN)) == NULL) { > > m_freem(m); > > break; > > } > > bcopy(mtod(m, struct ether_header *), &hdr, ETHER_HDR_LEN); > > m_adj(m, ETHER_HDR_LEN); > > bdg_forward(&m, &hdr, pkt->ifp); > > if (m) > > m_freem(m); > > } > > break ; > > #endif > > > > Now, pkt is a malloc()ed structure, not an mbuf! Calling m_pullup() on it > > seems defective, at least because m_free may be called in m_pullup(), > > which leaks kernel memory if the freed structure is not an mbuf. > > And of course, the ethernet header should be in the mbuf in pkt->dn_m. > > Should it be: > > > > #ifdef BRIDGE > > case DN_TO_BDG_FWD : { > > struct mbuf *m = (struct mbuf *)pkt ; > > struct ether_header hdr; > > > > if (pkt->dn_m->m_len < ETHER_HDR_LEN > > && (pkt->dn_m = m_pullup(pkt->dn_m, ETHER_HDR_LEN)) == NULL) { > > m_freem(pkt->dn_m); > > break; > > } > > bcopy(mtod(pkt->dn_m, struct ether_header *), &hdr, ETHER_HDR_LEN); > > m_adj(pkt->dn_m, ETHER_HDR_LEN); > > bdg_forward(&m, &hdr, pkt->ifp); > > if (m) > > /* bdg_format will put pkt->dn_m into mbuf into m in our case */ > > m_freem(m); > > } > > break ; > > #endif > > > > Hmm, maybe I'm wrong here, but that seems odd to me. Please enlighten > > me! Unfortunetly, I have no machine I could use to test it at the moment. I'm not sure, but I don't think it will work. As far as I can tell, the mbuf chain starts with the IP header, not the ethernet header. In my patch, I prepended the ethernet header in bridge.c and then did something similar to this in ip_dummynet.c. -- Lachlan O'Dea Computer Associates Pty Ltd Webmaster Vet - Anti-Virus Software http://www.vet.com.au/ Vet 10.2 Forever! --+HP7ph2BbKc20aGI Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=junk Hi Luigi, I've been having a look at PR kern/22224 to see what I can do. It seems that dummnet does not work with bridging after Archie Cobbs moved bridging code from the drivers to ether_input() (revision 1.17 of bridge.c). As a result of these changes, bdg_forward() now expects the ethernet header to be passed to it. Archie put some code in ip_dummynet.c (revision 1.25) to get the ethernet header from the start of the mbuf chain. Unfortunately the code is wrong and that's what causes the panic reported in the PR. As far as I can tell, the ethernet header is not in the mbuf chain received by dummynet. The best idea I could come up with was to prepend the ethernet header to the mbuf chain before giving it to dummynet_io(), and then pulling it out when it's time to call bdg_forward(). I have attached my patches for this. This stopped the panic, and packets do seem to go through dummynet pipes now. The delay config setting seems to work fine, however setting bw has no effect, packets simply go through at the maximum bandwidth. Dummynet may be getting confused because it expects the mbuf chain to start with an IP header, but the only problem I could see was that the packet size would be 14 bytes too big, which I didn't think would make a huge difference. Anyway, if you get a chance to look at this, I'd appreciate any suggestions you may have. Thanks. P.S. I was just reviewing my patch and noticed it may have a bug if m_pullup() has to allocate a new mbuf. -- Lachlan O'Dea Computer Associates Pty Ltd Webmaster Vet - Anti-Virus Software http://www.vet.com.au/ Vet 10.2 Forever! --+HP7ph2BbKc20aGI Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="dummy.diff" Index: net/bridge.c =================================================================== RCS file: /usr/local/cvsrep/freebsd/src/sys/net/bridge.c,v retrieving revision 1.16.2.6 diff -u -r1.16.2.6 bridge.c --- net/bridge.c 2000/09/25 17:30:01 1.16.2.6 +++ net/bridge.c 2000/11/09 07:53:12 @@ -753,6 +769,14 @@ * pass the pkt to dummynet. Need to include m, dst, rule. * Dummynet consumes the packet in all cases. */ + + /* + * prepend ethernet header to mbuf chain. + */ + M_PREPEND(m, ETHER_HDR_LEN, M_DONTWAIT); + if (m == NULL) + return ENOBUFS; + bcopy(eh, mtod(m, struct ether_header *), ETHER_HDR_LEN); dummynet_io((off & 0xffff), DN_TO_BDG_FWD, m, dst, NULL, 0, rule, 0); if (canfree) /* dummynet has consumed the original one */ *m0 = NULL ; Index: netinet/ip_dummynet.c =================================================================== RCS file: /usr/local/cvsrep/freebsd/src/sys/netinet/ip_dummynet.c,v retrieving revision 1.24.2.4 diff -u -r1.24.2.4 ip_dummynet.c --- netinet/ip_dummynet.c 2000/07/17 20:06:16 1.24.2.4 +++ netinet/ip_dummynet.c 2000/11/09 07:53:12 @@ -402,15 +402,16 @@ #ifdef BRIDGE case DN_TO_BDG_FWD : { struct mbuf *m = (struct mbuf *)pkt ; + struct mbuf *mreal = pkt->dn_m; struct ether_header hdr; - if (m->m_len < ETHER_HDR_LEN - && (m = m_pullup(m, ETHER_HDR_LEN)) == NULL) { - m_freem(m); + if (mreal->m_len < ETHER_HDR_LEN + && (mreal = m_pullup(mreal, ETHER_HDR_LEN)) == NULL) { + m_freem(mreal); break; } - bcopy(mtod(m, struct ether_header *), &hdr, ETHER_HDR_LEN); - m_adj(m, ETHER_HDR_LEN); + bcopy(mtod(mreal, struct ether_header *), &hdr, ETHER_HDR_LEN); + m_adj(mreal, ETHER_HDR_LEN); bdg_forward(&m, &hdr, pkt->ifp); if (m) m_freem(m); --+HP7ph2BbKc20aGI-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-stable" in the body of the message