Date: Thu, 16 Nov 2000 09:46:28 +1100 From: "Lachlan O'Dea" <lodea@vet.com.au> To: freebsd-stable@FreeBSD.ORG Subject: Re: Bridging code in 4.2RC1 still not fixed Message-ID: <20001116094627.B1761@vet.com.au> In-Reply-To: <3A12777E.DEA187E0@pcx.si>; from cuk@pcx.si on Wed, Nov 15, 2000 at 12:46:06PM %2B0100 References: <3A12777E.DEA187E0@pcx.si>
next in thread | previous in thread | raw e-mail | index | archive | help
--+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 <cuk@pcx.si>
> 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: <Pine.BSF.4.21.0011150544060.68267-100000@titanic.medinet.si>
> 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 <tmoestl@gmx.net>
> > 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 <mailto:lodea@vet.com.au> 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 <mailto:lodea@vet.com.au> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001116094627.B1761>