unless someone fixes mclgeti in this driver in the next 24 hours, this should
go in.

this has my ok on august 31.

On 28/08/2010, at 4:07 AM, Thordur I Bjornsson wrote:

> As seen on misc@, and also by myself sometime ago, but I forgot about
> it as work got hectic and I was moving.
>
> Anyways, vr(4) will not even surivie a few ping -f's, before the pools
> become corrupt, this has (obviously) something todo with how MCLGETI
> takes and puts buf's of the rings;
>
> Reverting MCLGETI "fixes" the issue. I've attached a diff, I remember
> testing it, and it survives fine.
>
> I did spent some time back then trying to figure this out but to no avail.
> The problem is the card is still messing with mbufs apperently after they
> have been taken of the ring (This is "kind of", similar to rev1.94 I
think).
>
> Index: dev/pci/if_vr.c
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/pci/if_vr.c,v
> retrieving revision 1.105
> diff -u -p -r1.105 if_vr.c
> --- dev/pci/if_vr.c   19 May 2010 15:27:35 -0000      1.105
> +++ dev/pci/if_vr.c   5 Aug 2010 15:59:05 -0000
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: if_vr.c,v 1.105 2010/05/19 15:27:35 oga Exp $ */
> +/*   $OpenBSD: if_vr.c,v 1.95 2009/06/04 16:56:20 sthen Exp $        */
>
> /*
>  * Copyright (c) 1997, 1998
> @@ -135,10 +135,9 @@ void vr_setcfg(struct vr_softc *, int);
> void vr_iff(struct vr_softc *);
> void vr_reset(struct vr_softc *);
> int vr_list_rx_init(struct vr_softc *);
> -void vr_fill_rx_ring(struct vr_softc *);
> int vr_list_tx_init(struct vr_softc *);
>
> -int vr_alloc_mbuf(struct vr_softc *, struct vr_chain_onefrag *);
> +int vr_alloc_mbuf(struct vr_softc *, struct vr_chain_onefrag *, struct mbuf
*);
>
> /*
>  * Supported devices & quirks
> @@ -664,7 +663,6 @@ vr_attach(struct device *parent, struct
>       /*
>        * Call MI attach routines.
>        */
> -     m_clsetwms(ifp, MCLBYTES, 2, VR_RX_LIST_CNT - 1);
>       if_attach(ifp);
>       ether_ifattach(ifp);
>       return;
> @@ -749,6 +747,9 @@ vr_list_rx_init(struct vr_softc *sc)
>                   sc->sc_listmap->dm_segs[0].ds_addr +
>                   offsetof(struct vr_list_data, vr_rx_list[i]);
>
> +             if (vr_alloc_mbuf(sc, &cd->vr_rx_chain[i], NULL))
> +                     return (ENOBUFS);
> +
>               if (i == (VR_RX_LIST_CNT - 1))
>                       nexti = 0;
>               else
> @@ -760,30 +761,11 @@ vr_list_rx_init(struct vr_softc *sc)
>                   offsetof(struct vr_list_data, vr_rx_list[nexti]));
>       }
>
> -     cd->vr_rx_prod = cd->vr_rx_cons = &cd->vr_rx_chain[0];
> -     cd->vr_rx_cnt = 0;
> -     vr_fill_rx_ring(sc);
> +     cd->vr_rx_head = &cd->vr_rx_chain[0];
>
>       return (0);
> }
>
> -void
> -vr_fill_rx_ring(struct vr_softc *sc)
> -{
> -     struct vr_chain_data    *cd;
> -     struct vr_list_data     *ld;
> -
> -     cd = &sc->vr_cdata;
> -     ld = sc->vr_ldata;
> -
> -     while (cd->vr_rx_cnt < VR_RX_LIST_CNT) {
> -             if (vr_alloc_mbuf(sc, cd->vr_rx_prod))
> -                     break;
> -             cd->vr_rx_prod = cd->vr_rx_prod->vr_nextdesc;
> -             cd->vr_rx_cnt++;
> -     }
> -}
> -
> /*
>  * A frame has been uploaded: pass the resulting mbuf chain up to
>  * the higher level protocols.
> @@ -791,7 +773,7 @@ vr_fill_rx_ring(struct vr_softc *sc)
> void
> vr_rxeof(struct vr_softc *sc)
> {
> -     struct mbuf             *m;
> +     struct mbuf             *m0, *m;
>       struct ifnet            *ifp;
>       struct vr_chain_onefrag *cur_rx;
>       int                     total_len = 0;
> @@ -799,21 +781,20 @@ vr_rxeof(struct vr_softc *sc)
>
>       ifp = &sc->arpcom.ac_if;
>
> -     while(sc->vr_cdata.vr_rx_cnt > 0) {
> +     for (;;) {
> +
>               bus_dmamap_sync(sc->sc_dmat, sc->sc_listmap,
>                   0, sc->sc_listmap->dm_mapsize,
>                   BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
> -             rxstat = letoh32(sc->vr_cdata.vr_rx_cons->vr_ptr->vr_status);
> +             rxstat = letoh32(sc->vr_cdata.vr_rx_head->vr_ptr->vr_status);
>               if (rxstat & VR_RXSTAT_OWN)
>                       break;
>
> -             rxctl = letoh32(sc->vr_cdata.vr_rx_cons->vr_ptr->vr_ctl);
> +             rxctl = letoh32(sc->vr_cdata.vr_rx_head->vr_ptr->vr_ctl);
>
> -             cur_rx = sc->vr_cdata.vr_rx_cons;
> -             m = cur_rx->vr_mbuf;
> -             cur_rx->vr_mbuf = NULL;
> -             sc->vr_cdata.vr_rx_cons = cur_rx->vr_nextdesc;
> -             sc->vr_cdata.vr_rx_cnt--;
> +             m0 = NULL;
> +             cur_rx = sc->vr_cdata.vr_rx_head;
> +             sc->vr_cdata.vr_rx_head = cur_rx->vr_nextdesc;
>
>               /*
>                * If an error occurs, update stats, clear the
> @@ -843,13 +824,24 @@ vr_rxeof(struct vr_softc *sc)
>                       printf("\n");
> #endif
>
> -                     m_freem(m);
> +                     /* Reinitialize descriptor */
> +                     cur_rx->vr_ptr->vr_status = htole32(VR_RXSTAT);
> +                     cur_rx->vr_ptr->vr_data =
> +                         htole32(cur_rx->vr_map->dm_segs[0].ds_addr +
> +                         sizeof(u_int64_t));
> +                     cur_rx->vr_ptr->vr_ctl = htole32(VR_RXCTL | VR_RXLEN);
> +                     bus_dmamap_sync(sc->sc_dmat, sc->sc_listmap,
> +                         0, sc->sc_listmap->dm_mapsize,
> +                         BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
>                       continue;
>               }
>
>               /* No errors; receive the packet. */
>               total_len = VR_RXBYTES(letoh32(cur_rx->vr_ptr->vr_status));
>
> +             m = cur_rx->vr_mbuf;
> +             cur_rx->vr_mbuf = NULL;
> +
>               bus_dmamap_sync(sc->sc_dmat, cur_rx->vr_map, 0,
>                   cur_rx->vr_map->dm_mapsize, BUS_DMASYNC_POSTREAD);
>               bus_dmamap_unload(sc->sc_dmat, cur_rx->vr_map);
> @@ -861,22 +853,22 @@ vr_rxeof(struct vr_softc *sc)
>                */
>               total_len -= ETHER_CRC_LEN;
>
> -#ifdef __STRICT_ALIGNMENT
> +#ifndef __STRICT_ALIGNMENT
> +             if (vr_alloc_mbuf(sc, cur_rx, NULL) == 0) {
> +                     m->m_pkthdr.rcvif = ifp;
> +                     m->m_pkthdr.len = m->m_len = total_len;
> +             } else
> +#endif
>               {
> -                     struct mbuf *m0;
>                       m0 = m_devget(mtod(m, caddr_t), total_len,
>                           ETHER_ALIGN, ifp, NULL);
> -                     m_freem(m);
> +                     vr_alloc_mbuf(sc, cur_rx, m);
>                       if (m0 == NULL) {
>                               ifp->if_ierrors++;
>                               continue;
>                       }
>                       m = m0;
> -             }
> -#else
> -             m->m_pkthdr.rcvif = ifp;
> -             m->m_pkthdr.len = m->m_len = total_len;
> -#endif
> +             }
>
>               ifp->if_ipackets++;
>               if (sc->vr_quirks & VR_Q_CSUM &&
> @@ -902,8 +894,6 @@ vr_rxeof(struct vr_softc *sc)
>               ether_input_mbuf(ifp, m);
>       }
>
> -     vr_fill_rx_ring(sc);
> -
>       bus_dmamap_sync(sc->sc_dmat, sc->sc_listmap,
>           0, sc->sc_listmap->dm_mapsize,
>           BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
> @@ -935,7 +925,7 @@ vr_rxeoc(struct vr_softc *sc)
>
>       vr_rxeof(sc);
>
> -     CSR_WRITE_4(sc, VR_RXADDR, sc->vr_cdata.vr_rx_cons->vr_paddr);
> +     CSR_WRITE_4(sc, VR_RXADDR, sc->vr_cdata.vr_rx_head->vr_paddr);
>       VR_SETBIT16(sc, VR_COMMAND, VR_CMD_RX_ON);
>       VR_SETBIT16(sc, VR_COMMAND, VR_CMD_RX_GO);
> }
> @@ -1345,7 +1335,7 @@ vr_init(void *xsc)
>       /*
>        * Load the address of the RX list.
>        */
> -     CSR_WRITE_4(sc, VR_RXADDR, sc->vr_cdata.vr_rx_cons->vr_paddr);
> +     CSR_WRITE_4(sc, VR_RXADDR, sc->vr_cdata.vr_rx_head->vr_paddr);
>
>       /* Enable receiver and transmitter. */
>       CSR_WRITE_2(sc, VR_COMMAND, VR_CMD_TX_NOPOLL|VR_CMD_START|
> @@ -1534,23 +1524,34 @@ vr_stop(struct vr_softc *sc)
> }
>
> int
> -vr_alloc_mbuf(struct vr_softc *sc, struct vr_chain_onefrag *r)
> +vr_alloc_mbuf(struct vr_softc *sc, struct vr_chain_onefrag *r, struct mbuf
*mb)
> {
>       struct vr_desc  *d;
>       struct mbuf     *m;
>
> -     if (r == NULL)
> -             return (EINVAL);
> +     if (mb == NULL) {
> +             MGETHDR(m, M_DONTWAIT, MT_DATA);
> +             if (m == NULL)
> +                     return (ENOBUFS);
>
> -     m = MCLGETI(NULL, M_DONTWAIT, &sc->arpcom.ac_if, MCLBYTES);
> -     if (!m)
> -             return (ENOBUFS);
> +             MCLGET(m, M_DONTWAIT);
> +             if (!(m->m_flags & M_EXT)) {
> +                     m_free(m);
> +                     return (ENOBUFS);
> +             }
> +     } else  {
> +             m = mb;
> +             m->m_data = m->m_ext.ext_buf;
> +     }
>
>       m->m_len = m->m_pkthdr.len = MCLBYTES;
> +     r->vr_mbuf = m;
> +
>       m_adj(m, sizeof(u_int64_t));
>
> -     if (bus_dmamap_load_mbuf(sc->sc_dmat, r->vr_map, m, BUS_DMA_NOWAIT)) {
> -             m_free(m);
> +     if (bus_dmamap_load_mbuf(sc->sc_dmat, r->vr_map, r->vr_mbuf,
> +         BUS_DMA_NOWAIT)) {
> +             m_freem(r->vr_mbuf);
>               return (ENOBUFS);
>       }
>
> @@ -1558,7 +1559,6 @@ vr_alloc_mbuf(struct vr_softc *sc, struc
>           BUS_DMASYNC_PREREAD);
>
>       /* Reinitialize the RX descriptor */
> -     r->vr_mbuf = m;
>       d = r->vr_ptr;
>       d->vr_data = htole32(r->vr_map->dm_segs[0].ds_addr);
>       d->vr_ctl = htole32(VR_RXCTL | VR_RXLEN);
> Index: dev/pci/if_vrreg.h
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/pci/if_vrreg.h,v
> retrieving revision 1.27
> diff -u -p -r1.27 if_vrreg.h
> --- dev/pci/if_vrreg.h        18 Jun 2009 17:48:15 -0000      1.27
> +++ dev/pci/if_vrreg.h        5 Aug 2010 15:59:05 -0000
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: if_vrreg.h,v 1.27 2009/06/18 17:48:15 claudio Exp $   */
> +/*   $OpenBSD: if_vrreg.h,v 1.26 2009/05/12 13:30:56 sthen Exp $     */
>
> /*
>  * Copyright (c) 1997, 1998
> @@ -435,9 +435,7 @@ struct vr_chain_data {
>       struct vr_chain_onefrag vr_rx_chain[VR_RX_LIST_CNT];
>       struct vr_chain         vr_tx_chain[VR_TX_LIST_CNT];
>
> -     struct vr_chain_onefrag *vr_rx_cons;
> -     struct vr_chain_onefrag *vr_rx_prod;
> -     int                      vr_rx_cnt;
> +     struct vr_chain_onefrag *vr_rx_head;
>
>       struct vr_chain         *vr_tx_cons;
>       struct vr_chain         *vr_tx_prod;

Reply via email to