svn commit: r362022 - stable/12/sys/dev/virtio/network

[Vincenzo Maffione]

Author: vmaffione
Date: Wed Jun 10 20:02:38 2020
New Revision: 362022
URL: https://svnweb.freebsd.org/changeset/base/362022

Log:
  MFC r361745
  
  netmap: vtnet: call netmap_rx_irq() under VQ lock
  
  The netmap_rx_irq() function normally wakes up user-space threads
  waiting for more packets. In this case, it is not necessary to
  call it under the driver queue lock. However, if the interface is
  attached to a VALE switch, netmap_rx_irq() ends up calling rxsync
  on the interface (see netmap_bwrap_intr_notify()). Although
  concurrent rxsyncs are serialized through the kring lock
  (see nm_kr_tryget()), the lock acquire operation is not blocking.
  As a result, it may happen that netmap_rx_irq() is called on
  an RX ring while another instance is running, causing the
  second call to fail, and received packets stall in the receive VQ.
  We fix this issue by calling netmap_irx_irq() under the VQ lock.

Modified:
  stable/12/sys/dev/virtio/network/if_vtnet.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/dev/virtio/network/if_vtnet.c
==============================================================================
--- stable/12/sys/dev/virtio/network/if_vtnet.c	Wed Jun 10 19:59:11 2020	(r362021)
+++ stable/12/sys/dev/virtio/network/if_vtnet.c	Wed Jun 10 20:02:38 2020	(r362022)
@@ -1873,9 +1873,20 @@ vtnet_rx_vq_intr(void *xrxq)
 		return;
 	}
 
+	VTNET_RXQ_LOCK(rxq);
+
 #ifdef DEV_NETMAP
+	/*
+	 * We call netmap_rx_irq() under lock to prevent concurrent calls.
+	 * This is not necessary to serialize the access to the RX vq, but
+	 * rather to avoid races that may happen if this interface is
+	 * attached to a VALE switch, which would cause received packets
+	 * to stall in the RX queue (nm_kr_tryget() could find the kring
+	 * busy when called from netmap_bwrap_intr_notify()).
+	 */
 	nmirq = netmap_rx_irq(ifp, rxq->vtnrx_id, &more);
 	if (nmirq != NM_IRQ_PASS) {
+		VTNET_RXQ_UNLOCK(rxq);
 		if (nmirq == NM_IRQ_RESCHED) {
 			taskqueue_enqueue(rxq->vtnrx_tq, &rxq->vtnrx_intrtask);
 		}
@@ -1883,8 +1894,6 @@ vtnet_rx_vq_intr(void *xrxq)
 	}
 #endif /* DEV_NETMAP */
 
-	VTNET_RXQ_LOCK(rxq);
-
 again:
 	if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
 		VTNET_RXQ_UNLOCK(rxq);
@@ -1924,17 +1933,18 @@ vtnet_rxq_tq_intr(void *xrxq, int pending)
 	sc = rxq->vtnrx_sc;
 	ifp = sc->vtnet_ifp;
 
+	VTNET_RXQ_LOCK(rxq);
+
 #ifdef DEV_NETMAP
 	nmirq = netmap_rx_irq(ifp, rxq->vtnrx_id, &more);
 	if (nmirq != NM_IRQ_PASS) {
+		VTNET_RXQ_UNLOCK(rxq);
 		if (nmirq == NM_IRQ_RESCHED) {
 			taskqueue_enqueue(rxq->vtnrx_tq, &rxq->vtnrx_intrtask);
 		}
 		return;
 	}
 #endif /* DEV_NETMAP */
-
-	VTNET_RXQ_LOCK(rxq);
 
 	if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
 		VTNET_RXQ_UNLOCK(rxq);