svn commit: r361745 - head/sys/dev/virtio/network

[Vincenzo Maffione]

Author: vmaffione
Date: Wed Jun  3 05:27:29 2020
New Revision: 361745
URL: https://svnweb.freebsd.org/changeset/base/361745

Log:
  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.
  
  MFC after:	1 week

Modified:
  head/sys/dev/virtio/network/if_vtnet.c

Modified: head/sys/dev/virtio/network/if_vtnet.c
==============================================================================
--- head/sys/dev/virtio/network/if_vtnet.c	Wed Jun  3 05:09:33 2020	(r361744)
+++ head/sys/dev/virtio/network/if_vtnet.c	Wed Jun  3 05:27:29 2020	(r361745)
@@ -1936,9 +1936,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);
 		}
@@ -1946,8 +1957,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);
@@ -1987,17 +1996,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);