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. MFC after: 1 week
This commit is contained in:
parent
7312c97fa4
commit
16f224b5f8
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=362183
@ -39,52 +39,18 @@ vtnet_netmap_reg(struct netmap_adapter *na, int state)
|
|||||||
{
|
{
|
||||||
struct ifnet *ifp = na->ifp;
|
struct ifnet *ifp = na->ifp;
|
||||||
struct vtnet_softc *sc = ifp->if_softc;
|
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);
|
VTNET_CORE_LOCK(sc);
|
||||||
|
ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
|
||||||
/* We need nm_netmap_on() to return true when called by
|
vtnet_init_locked(sc, state ? VTNET_INIT_NETMAP_ENTER
|
||||||
* vtnet_init_locked() below. */
|
: VTNET_INIT_NETMAP_EXIT);
|
||||||
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);
|
|
||||||
}
|
|
||||||
|
|
||||||
VTNET_CORE_UNLOCK(sc);
|
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_adapter *na = NA(rxq->vtnrx_sc->vtnet_ifp);
|
||||||
struct netmap_kring *kring;
|
struct netmap_kring *kring;
|
||||||
|
struct netmap_slot *slot;
|
||||||
int error;
|
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;
|
return -1;
|
||||||
|
|
||||||
kring = na->rx_rings[rxq->vtnrx_id];
|
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
|
/* 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
|
* buffers, the number of netmap slots in the RX ring matches the
|
||||||
|
@ -181,7 +181,7 @@ static int vtnet_init_tx_queues(struct vtnet_softc *);
|
|||||||
static int vtnet_init_rxtx_queues(struct vtnet_softc *);
|
static int vtnet_init_rxtx_queues(struct vtnet_softc *);
|
||||||
static void vtnet_set_active_vq_pairs(struct vtnet_softc *);
|
static void vtnet_set_active_vq_pairs(struct vtnet_softc *);
|
||||||
static int vtnet_reinit(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_init(void *);
|
||||||
|
|
||||||
static void vtnet_free_ctrl_vq(struct vtnet_softc *);
|
static void vtnet_free_ctrl_vq(struct vtnet_softc *);
|
||||||
@ -523,7 +523,7 @@ vtnet_resume(device_t dev)
|
|||||||
|
|
||||||
VTNET_CORE_LOCK(sc);
|
VTNET_CORE_LOCK(sc);
|
||||||
if (ifp->if_flags & IFF_UP)
|
if (ifp->if_flags & IFF_UP)
|
||||||
vtnet_init_locked(sc);
|
vtnet_init_locked(sc, 0);
|
||||||
sc->vtnet_flags &= ~VTNET_FLAG_SUSPENDED;
|
sc->vtnet_flags &= ~VTNET_FLAG_SUSPENDED;
|
||||||
VTNET_CORE_UNLOCK(sc);
|
VTNET_CORE_UNLOCK(sc);
|
||||||
|
|
||||||
@ -1087,7 +1087,7 @@ vtnet_change_mtu(struct vtnet_softc *sc, int new_mtu)
|
|||||||
|
|
||||||
if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
|
if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
|
||||||
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);
|
return (0);
|
||||||
@ -1131,7 +1131,7 @@ vtnet_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else
|
} else
|
||||||
vtnet_init_locked(sc);
|
vtnet_init_locked(sc, 0);
|
||||||
|
|
||||||
if (error == 0)
|
if (error == 0)
|
||||||
sc->vtnet_if_flags = ifp->if_flags;
|
sc->vtnet_if_flags = ifp->if_flags;
|
||||||
@ -1189,7 +1189,7 @@ vtnet_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
|
|||||||
|
|
||||||
if (reinit && (ifp->if_drv_flags & IFF_DRV_RUNNING)) {
|
if (reinit && (ifp->if_drv_flags & IFF_DRV_RUNNING)) {
|
||||||
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);
|
VTNET_CORE_UNLOCK(sc);
|
||||||
@ -2783,7 +2783,7 @@ vtnet_tick(void *xsc)
|
|||||||
|
|
||||||
if (timedout != 0) {
|
if (timedout != 0) {
|
||||||
ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
|
ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
|
||||||
vtnet_init_locked(sc);
|
vtnet_init_locked(sc, 0);
|
||||||
} else
|
} else
|
||||||
callout_schedule(&sc->vtnet_tick_ch, hz);
|
callout_schedule(&sc->vtnet_tick_ch, hz);
|
||||||
}
|
}
|
||||||
@ -3068,6 +3068,9 @@ vtnet_init_tx_queues(struct vtnet_softc *sc)
|
|||||||
for (i = 0; i < sc->vtnet_act_vq_pairs; i++) {
|
for (i = 0; i < sc->vtnet_act_vq_pairs; i++) {
|
||||||
txq = &sc->vtnet_txqs[i];
|
txq = &sc->vtnet_txqs[i];
|
||||||
txq->vtntx_watchdog = 0;
|
txq->vtntx_watchdog = 0;
|
||||||
|
#ifdef DEV_NETMAP
|
||||||
|
netmap_reset(NA(sc->vtnet_ifp), NR_TX, i, 0);
|
||||||
|
#endif /* DEV_NETMAP */
|
||||||
}
|
}
|
||||||
|
|
||||||
return (0);
|
return (0);
|
||||||
@ -3151,7 +3154,7 @@ vtnet_reinit(struct vtnet_softc *sc)
|
|||||||
}
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
vtnet_init_locked(struct vtnet_softc *sc)
|
vtnet_init_locked(struct vtnet_softc *sc, int init_mode)
|
||||||
{
|
{
|
||||||
device_t dev;
|
device_t dev;
|
||||||
struct ifnet *ifp;
|
struct ifnet *ifp;
|
||||||
@ -3166,6 +3169,18 @@ vtnet_init_locked(struct vtnet_softc *sc)
|
|||||||
|
|
||||||
vtnet_stop(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. */
|
/* Reinitialize with the host. */
|
||||||
if (vtnet_virtio_reinit(sc) != 0)
|
if (vtnet_virtio_reinit(sc) != 0)
|
||||||
goto fail;
|
goto fail;
|
||||||
@ -3192,7 +3207,7 @@ vtnet_init(void *xsc)
|
|||||||
sc = xsc;
|
sc = xsc;
|
||||||
|
|
||||||
VTNET_CORE_LOCK(sc);
|
VTNET_CORE_LOCK(sc);
|
||||||
vtnet_init_locked(sc);
|
vtnet_init_locked(sc, 0);
|
||||||
VTNET_CORE_UNLOCK(sc);
|
VTNET_CORE_UNLOCK(sc);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -372,4 +372,10 @@ CTASSERT(((VTNET_MAX_TX_SEGS - 1) * MCLBYTES) >= VTNET_MAX_MTU);
|
|||||||
"VTNET Core Lock", MTX_DEF); \
|
"VTNET Core Lock", MTX_DEF); \
|
||||||
} while (0)
|
} 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 */
|
#endif /* _IF_VTNETVAR_H */
|
||||||
|
Loading…
Reference in New Issue
Block a user