OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
Re: Backout mclgeti for vr(4).

From: David Gwynne (lokianimata.net)
Date: Sun Aug 29 2010 - 20:10:29 CDT


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;