svn commit: r362554 - in stable/12/sys/dev: netmap virtio/network
Vincenzo Maffione
vmaffione at FreeBSD.org
Tue Jun 23 20:41:11 UTC 2020
Author: vmaffione
Date: Tue Jun 23 20:41:10 2020
New Revision: 362554
URL: https://svnweb.freebsd.org/changeset/base/362554
Log:
MFC r362183
netmap: vtnet: fix races in vtnet_netmap_reg()
The nm_register callback needs to call nm_set_native_flags()
or nm_clear_native_flags() once the device has been stopped.
However, in the current implementation this is not true,
as the device is stopped by vtnet_init_locked(). This causes
race conditions where the driver crashes as soon as it
dequeues netmap buffers assuming they are mbufs (or the other
way around).
To fix the issue, we extend vtnet_init_locked() with a second
argument that, if not zero, will set/clear the netmap flags.
This results in a huge simplification of the nm_register
callback itself.
Also, use netmap_reset() to check if a ring is going to be
re-initialized in netmap mode.
Modified:
stable/12/sys/dev/netmap/if_vtnet_netmap.h
stable/12/sys/dev/virtio/network/if_vtnet.c
stable/12/sys/dev/virtio/network/if_vtnetvar.h
Directory Properties:
stable/12/ (props changed)
Modified: stable/12/sys/dev/netmap/if_vtnet_netmap.h
==============================================================================
--- stable/12/sys/dev/netmap/if_vtnet_netmap.h Tue Jun 23 20:23:56 2020 (r362553)
+++ stable/12/sys/dev/netmap/if_vtnet_netmap.h Tue Jun 23 20:41:10 2020 (r362554)
@@ -39,52 +39,18 @@ vtnet_netmap_reg(struct netmap_adapter *na, int state)
{
struct ifnet *ifp = na->ifp;
struct vtnet_softc *sc = ifp->if_softc;
- int success;
- int i;
- /* Drain the taskqueues to make sure that there are no worker threads
- * accessing the virtqueues. */
- vtnet_drain_taskqueues(sc);
-
+ /*
+ * Trigger a device reinit, asking vtnet_init_locked() to
+ * also enter or exit netmap mode.
+ */
VTNET_CORE_LOCK(sc);
-
- /* We need nm_netmap_on() to return true when called by
- * vtnet_init_locked() below. */
- if (state)
- nm_set_native_flags(na);
-
- /* We need to trigger a device reset in order to unexpose guest buffers
- * published to the host. */
- ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
- /* Get pending used buffers. The way they are freed depends on whether
- * they are netmap buffer or they are mbufs. We can tell apart the two
- * cases by looking at kring->nr_mode, before this is possibly updated
- * in the loop below. */
- for (i = 0; i < sc->vtnet_act_vq_pairs; i++) {
- struct vtnet_txq *txq = &sc->vtnet_txqs[i];
- struct vtnet_rxq *rxq = &sc->vtnet_rxqs[i];
-
- VTNET_TXQ_LOCK(txq);
- vtnet_txq_free_mbufs(txq);
- VTNET_TXQ_UNLOCK(txq);
-
- VTNET_RXQ_LOCK(rxq);
- vtnet_rxq_free_mbufs(rxq);
- VTNET_RXQ_UNLOCK(rxq);
- }
- vtnet_init_locked(sc);
- success = (ifp->if_drv_flags & IFF_DRV_RUNNING) ? 0 : ENXIO;
-
- if (state) {
- netmap_krings_mode_commit(na, state);
- } else {
- nm_clear_native_flags(na);
- netmap_krings_mode_commit(na, state);
- }
-
+ ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
+ vtnet_init_locked(sc, state ? VTNET_INIT_NETMAP_ENTER
+ : VTNET_INIT_NETMAP_EXIT);
VTNET_CORE_UNLOCK(sc);
- return success;
+ return 0;
}
@@ -245,15 +211,13 @@ vtnet_netmap_rxq_populate(struct vtnet_rxq *rxq)
{
struct netmap_adapter *na = NA(rxq->vtnrx_sc->vtnet_ifp);
struct netmap_kring *kring;
+ struct netmap_slot *slot;
int error;
- if (!nm_native_on(na) || rxq->vtnrx_id >= na->num_rx_rings)
+ slot = netmap_reset(na, NR_RX, rxq->vtnrx_id, 0);
+ if (slot == NULL)
return -1;
-
kring = na->rx_rings[rxq->vtnrx_id];
- if (!(nm_kring_pending_on(kring) ||
- kring->nr_pending_mode == NKR_NETMAP_ON))
- return -1;
/* Expose all the RX netmap buffers we can. In case of no indirect
* buffers, the number of netmap slots in the RX ring matches the
Modified: stable/12/sys/dev/virtio/network/if_vtnet.c
==============================================================================
--- stable/12/sys/dev/virtio/network/if_vtnet.c Tue Jun 23 20:23:56 2020 (r362553)
+++ stable/12/sys/dev/virtio/network/if_vtnet.c Tue Jun 23 20:41:10 2020 (r362554)
@@ -180,7 +180,7 @@ static int vtnet_init_tx_queues(struct vtnet_softc *);
static int vtnet_init_rxtx_queues(struct vtnet_softc *);
static void vtnet_set_active_vq_pairs(struct vtnet_softc *);
static int vtnet_reinit(struct vtnet_softc *);
-static void vtnet_init_locked(struct vtnet_softc *);
+static void vtnet_init_locked(struct vtnet_softc *, int);
static void vtnet_init(void *);
static void vtnet_free_ctrl_vq(struct vtnet_softc *);
@@ -512,7 +512,7 @@ vtnet_resume(device_t dev)
VTNET_CORE_LOCK(sc);
if (ifp->if_flags & IFF_UP)
- vtnet_init_locked(sc);
+ vtnet_init_locked(sc, 0);
sc->vtnet_flags &= ~VTNET_FLAG_SUSPENDED;
VTNET_CORE_UNLOCK(sc);
@@ -1065,7 +1065,7 @@ vtnet_change_mtu(struct vtnet_softc *sc, int new_mtu)
if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
- vtnet_init_locked(sc);
+ vtnet_init_locked(sc, 0);
}
return (0);
@@ -1109,7 +1109,7 @@ vtnet_ioctl(struct ifnet *ifp, u_long cmd, caddr_t dat
}
}
} else
- vtnet_init_locked(sc);
+ vtnet_init_locked(sc, 0);
if (error == 0)
sc->vtnet_if_flags = ifp->if_flags;
@@ -1167,7 +1167,7 @@ vtnet_ioctl(struct ifnet *ifp, u_long cmd, caddr_t dat
if (reinit && (ifp->if_drv_flags & IFF_DRV_RUNNING)) {
ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
- vtnet_init_locked(sc);
+ vtnet_init_locked(sc, 0);
}
VTNET_CORE_UNLOCK(sc);
@@ -2716,7 +2716,7 @@ vtnet_tick(void *xsc)
if (timedout != 0) {
ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
- vtnet_init_locked(sc);
+ vtnet_init_locked(sc, 0);
} else
callout_schedule(&sc->vtnet_tick_ch, hz);
}
@@ -3001,6 +3001,9 @@ vtnet_init_tx_queues(struct vtnet_softc *sc)
for (i = 0; i < sc->vtnet_act_vq_pairs; i++) {
txq = &sc->vtnet_txqs[i];
txq->vtntx_watchdog = 0;
+#ifdef DEV_NETMAP
+ netmap_reset(NA(sc->vtnet_ifp), NR_TX, i, 0);
+#endif /* DEV_NETMAP */
}
return (0);
@@ -3084,7 +3087,7 @@ vtnet_reinit(struct vtnet_softc *sc)
}
static void
-vtnet_init_locked(struct vtnet_softc *sc)
+vtnet_init_locked(struct vtnet_softc *sc, int init_mode)
{
device_t dev;
struct ifnet *ifp;
@@ -3099,6 +3102,18 @@ vtnet_init_locked(struct vtnet_softc *sc)
vtnet_stop(sc);
+#ifdef DEV_NETMAP
+ /* Once stopped we can update the netmap flags, if necessary. */
+ switch (init_mode) {
+ case VTNET_INIT_NETMAP_ENTER:
+ nm_set_native_flags(NA(ifp));
+ break;
+ case VTNET_INIT_NETMAP_EXIT:
+ nm_clear_native_flags(NA(ifp));
+ break;
+ }
+#endif /* DEV_NETMAP */
+
/* Reinitialize with the host. */
if (vtnet_virtio_reinit(sc) != 0)
goto fail;
@@ -3125,7 +3140,7 @@ vtnet_init(void *xsc)
sc = xsc;
VTNET_CORE_LOCK(sc);
- vtnet_init_locked(sc);
+ vtnet_init_locked(sc, 0);
VTNET_CORE_UNLOCK(sc);
}
Modified: stable/12/sys/dev/virtio/network/if_vtnetvar.h
==============================================================================
--- stable/12/sys/dev/virtio/network/if_vtnetvar.h Tue Jun 23 20:23:56 2020 (r362553)
+++ stable/12/sys/dev/virtio/network/if_vtnetvar.h Tue Jun 23 20:41:10 2020 (r362554)
@@ -365,4 +365,10 @@ CTASSERT(((VTNET_MAX_TX_SEGS - 1) * MCLBYTES) >= VTNET
"VTNET Core Lock", MTX_DEF); \
} while (0)
+/*
+ * Values for the init_mode argument of vtnet_init_locked().
+ */
+#define VTNET_INIT_NETMAP_ENTER 1
+#define VTNET_INIT_NETMAP_EXIT 2
+
#endif /* _IF_VTNETVAR_H */
More information about the svn-src-stable
mailing list