On 2010/05/26 11:46, Claudio Jeker wrote:
> On Wed, May 26, 2010 at 10:06:54AM +0100, Stuart Henderson wrote:
> > On 2010/05/21 23:54, Chris Bayly wrote:
> > > Okay, I believe I've finally simplified this down to something
> > > manageable,
> > > and hopefully a heck of alot more repeatable.
> > >
> > > I have a second Alix 3D3 board running -current as of the 19th.
> > >
> > > I can make it start leaking mbufs with the following setup.
> > >
> > > - Completely bone stock OpenBSD install.
> > > - At install time setup one interface (vr0) as dhcp
> > > - After install, issue the following commands:
> > >
> > > ifconfig vr1 up # Note: no cable connected on vr1
> > > ifconfig bridge0 add vr0
> > > ifconfig bridge0 add vr1
> > > ifconfig bridge0 up
> > >
> > > netstat -m # will now show an increasing number of mbuf 2048 byte clusters
> > > # being used
> >
> > in vr_start() [if_vr.c:1228] we have this
> >
> > if (ifp->if_flags & IFF_OACTIVE || sc->vr_link == 0)
> > return;
> >
> > the sc->vr_link check was added to prevent watchdog timeouts,
> > but because these frames aren't transmitted we have a leak.
> >
> > not sure if this is the best way, but it seems to do the trick:
> >
> > Index: if_vr.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
> > retrieving revision 1.105
> > diff -u -p -r1.105 if_vr.c
> > --- if_vr.c 19 May 2010 15:27:35 -0000 1.105
> > +++ if_vr.c 26 May 2010 09:05:51 -0000
> > @@ -1225,7 +1225,7 @@ vr_start(struct ifnet *ifp)
> >
> > sc = ifp->if_softc;
> >
> > - if (ifp->if_flags & IFF_OACTIVE || sc->vr_link == 0)
> > + if (ifp->if_flags & IFF_OACTIVE)
> > return;
> >
> > cur_tx = sc->vr_cdata.vr_tx_prod;
> > @@ -1233,6 +1233,11 @@ vr_start(struct ifnet *ifp)
> > IFQ_DEQUEUE(&ifp->if_snd, m_head);
> > if (m_head == NULL)
> > break;
> > + /* transmitting without link results in watchdog timeouts */
> > + if (sc->vr_link == 0) {
> > + m_freem(m_head);
> > + return;
> > + }
> >
> > /* Pack the data into the descriptor. */
> > if (vr_encap(sc, cur_tx, m_head)) {
> >
>
> Hmm. The IFQ should have a limit and when reaching that packets will be
> dropped. So after 256 packets no more packets should leak.
> Unless something is calling IF_ENQUEUE() without checking IF_QFULL() first.
> Ugh, the these ifq makros are stupid.
With vr+bridge, the leak seems to be unlimited, certainly more than
256.
If you send packets some other way (I use ping6 ff02::1%vr0) then the
leak is clamped.
> Still I think the current behaviour is strange and your diff makes more
> sense. Any idea why vr(4) is using IFQ_DEQUEUE() + IF_PREPEND instead of
> doing the IFQ_POLL(), IFQ_DEQUEUE() dance?
No ideas...
[Added chris@ to cc's.]
