On Wed, Nov 23, 2016 at 00:42 +0100, Stefan Fritsch wrote:
> On Wednesday, 23 November 2016 00:34:42 CET Mike Belopuhov wrote:
> > Yes, after looking closer at virtio code I agree with you.
> > However, stop/init is purely OpenBSD specific action. There
> > are no provisions or requirements from the hardware really.
> > Thus we can treat UP/DOWN as purely software state and don't
> > stop/reinit the PCI device.
>
> I guess we could do that. But then we cannot free the mbufs on DOWN
> until the device has used them.
Diff to this effect is below. Works on vmd and qemu (original
one didn't because I kept the virtio_reset).
> That sounds like an unnecessary waste of memory to me.
>
This is not so much memory we lose and then if you up it again
you're going to have it all back. We can revert to the present
behavior once vmd matures, in the meantime people won't have to
juggle diffs around in their trees :)
The diff is meant to be applied on top of the previous two.
Please note I've also added a virtio_stop_vq_intr to vio_stop.
diff --git sys/dev/pci/if_vio.c sys/dev/pci/if_vio.c
index 68e6636..156c16f 100644
--- sys/dev/pci/if_vio.c
+++ sys/dev/pci/if_vio.c
@@ -596,10 +596,13 @@ vio_attach(struct device *parent, struct device *self,
void *aux)
ifmedia_set(&sc->sc_media, IFM_ETHER | IFM_AUTO);
vsc->sc_config_change = vio_config_change;
timeout_set(&sc->sc_txtick, vio_txtick, &sc->sc_vq[VQTX]);
timeout_set(&sc->sc_rxtick, vio_rxtick, &sc->sc_vq[VQRX]);
+ if_rxr_init(&sc->sc_rx_ring, 2 * ((ifp->if_hardmtu / MCLBYTES) + 1),
+ sc->sc_vq[VQRX].vq_num);
+
if_attach(ifp);
ether_ifattach(ifp);
return;
@@ -665,19 +668,14 @@ vio_init(struct ifnet *ifp)
{
struct vio_softc *sc = ifp->if_softc;
struct virtio_softc *vsc = sc->sc_virtio;
vio_stop(ifp, 0);
- if_rxr_init(&sc->sc_rx_ring, 2 * ((ifp->if_hardmtu / MCLBYTES) + 1),
- sc->sc_vq[VQRX].vq_num);
- virtio_reinit_start(vsc);
- virtio_negotiate_features(vsc, vsc->sc_features, NULL);
virtio_start_vq_intr(vsc, &sc->sc_vq[VQRX]);
virtio_stop_vq_intr(vsc, &sc->sc_vq[VQTX]);
if (vsc->sc_nvqs >= 3)
virtio_start_vq_intr(vsc, &sc->sc_vq[VQCTL]);
- virtio_reinit_end(vsc);
vio_populate_rx_mbufs(sc);
ifp->if_flags |= IFF_RUNNING;
ifq_clr_oactive(&ifp->if_snd);
vio_iff(sc);
vio_link_state(ifp);
@@ -692,18 +690,17 @@ vio_stop(struct ifnet *ifp, int disable)
timeout_del(&sc->sc_txtick);
timeout_del(&sc->sc_rxtick);
ifp->if_flags &= ~IFF_RUNNING;
ifq_clr_oactive(&ifp->if_snd);
- /* only way to stop I/O and DMA is resetting... */
- virtio_reset(vsc);
vio_rxeof(sc);
if (vsc->sc_nvqs >= 3)
vio_ctrleof(&sc->sc_vq[VQCTL]);
- vio_tx_drain(sc);
- if (disable)
- vio_rx_drain(sc);
+
+ virtio_stop_vq_intr(vsc, &sc->sc_vq[VQRX]);
+ if (vsc->sc_nvqs >= 3)
+ virtio_stop_vq_intr(vsc, &sc->sc_vq[VQCTL]);
if (vsc->sc_nvqs >= 3) {
if (sc->sc_ctrl_inuse != FREE)
sc->sc_ctrl_inuse = RESET;
wakeup(&sc->sc_ctrl_inuse);
@@ -1029,22 +1026,26 @@ vio_rxeof(struct vio_softc *sc)
mlast = m;
bufs_left--;
}
if (bufs_left == 0) {
- ml_enqueue(&ml, m0);
+ if (ifp->if_flags & IFF_RUNNING)
+ ml_enqueue(&ml, m0);
+ else
+ m_freem(m0);
m0 = NULL;
}
}
if (m0 != NULL) {
DBGPRINT("expected %d buffers, got %d", (int)hdr->num_buffers,
(int)hdr->num_buffers - bufs_left);
ifp->if_ierrors++;
m_freem(m0);
}
- if_input(ifp, &ml);
+ if (ifp->if_flags & IFF_RUNNING)
+ if_input(ifp, &ml);
return r;
}
int
vio_rx_intr(struct virtqueue *vq)
