svn commit: r354864 - head/usr.sbin/bhyve

[Vincenzo Maffione]

Author: vmaffione
Date: Tue Nov 19 21:10:44 2019
New Revision: 354864
URL: https://svnweb.freebsd.org/changeset/base/354864

Log:
  bhyve: virtio-net: disable receive until features are negotiated
  
  This patch fixes a race condition where the receive callback is called
  while the device is being reset. Since the rx_merge variable may change
  during reset, the receive callback may operate inconsistently with what
  the guest expects.
  Also, get rid of the unused rx_vhdrlen variable.
  
  PR:	242023
  Reported by:	aleksandr.fedorov at itglobal.com
  Reviewed by:	markj, jhb
  MFC with:	r354552
  Differential Revision:	https://reviews.freebsd.org/D22440

Modified:
  head/usr.sbin/bhyve/pci_virtio_net.c

Modified: head/usr.sbin/bhyve/pci_virtio_net.c
==============================================================================
--- head/usr.sbin/bhyve/pci_virtio_net.c	Tue Nov 19 21:08:18 2019	(r354863)
+++ head/usr.sbin/bhyve/pci_virtio_net.c	Tue Nov 19 21:10:44 2019	(r354864)
@@ -109,7 +109,6 @@ struct pci_vtnet_softc {
 	uint64_t	vsc_features;	/* negotiated features */
 	
 	pthread_mutex_t	rx_mtx;
-	unsigned int	rx_vhdrlen;
 	int		rx_merge;	/* merged rx bufs in use */
 
 	pthread_t 	tx_tid;
@@ -149,6 +148,16 @@ pci_vtnet_reset(void *vsc)
 	/* Acquire the RX lock to block RX processing. */
 	pthread_mutex_lock(&sc->rx_mtx);
 
+	/*
+	 * Make sure receive operation is disabled at least until we
+	 * re-negotiate the features, since receive operation depends
+	 * on the value of sc->rx_merge and the header length, which
+	 * are both set in pci_vtnet_neg_features().
+	 * Receive operation will be enabled again once the guest adds
+	 * the first receive buffers and kicks us.
+	 */
+	netbe_rx_disable(sc->vsc_be);
+
 	/* Set sc->resetting and give a chance to the TX thread to stop. */
 	pthread_mutex_lock(&sc->tx_mtx);
 	sc->resetting = 1;
@@ -158,9 +167,6 @@ pci_vtnet_reset(void *vsc)
 		pthread_mutex_lock(&sc->tx_mtx);
 	}
 
-	sc->rx_merge = 1;
-	sc->rx_vhdrlen = sizeof(struct virtio_net_rxhdr);
-
 	/*
 	 * Now reset rings, MSI-X vectors, and negotiated capabilities.
 	 * Do that with the TX lock held, since we need to reset
@@ -512,8 +518,7 @@ pci_vtnet_init(struct vmctx *ctx, struct pci_devinst *
 
 	sc->resetting = 0;
 
-	sc->rx_merge = 1;
-	sc->rx_vhdrlen = sizeof(struct virtio_net_rxhdr);
+	sc->rx_merge = 0;
 	pthread_mutex_init(&sc->rx_mtx, NULL); 
 
 	/* 
@@ -568,18 +573,24 @@ static void
 pci_vtnet_neg_features(void *vsc, uint64_t negotiated_features)
 {
 	struct pci_vtnet_softc *sc = vsc;
+	unsigned int rx_vhdrlen;
 
 	sc->vsc_features = negotiated_features;
 
-	if (!(negotiated_features & VIRTIO_NET_F_MRG_RXBUF)) {
+	if (negotiated_features & VIRTIO_NET_F_MRG_RXBUF) {
+		rx_vhdrlen = sizeof(struct virtio_net_rxhdr);
+		sc->rx_merge = 1;
+	} else {
+		/*
+		 * Without mergeable rx buffers, virtio-net header is 2
+		 * bytes shorter than sizeof(struct virtio_net_rxhdr).
+		 */
+		rx_vhdrlen = sizeof(struct virtio_net_rxhdr) - 2;
 		sc->rx_merge = 0;
-		/* Without mergeable rx buffers, virtio-net header is 2
-		 * bytes shorter than sizeof(struct virtio_net_rxhdr). */
-		sc->rx_vhdrlen = sizeof(struct virtio_net_rxhdr) - 2;
 	}
 
 	/* Tell the backend to enable some capabilities it has advertised. */
-	netbe_set_cap(sc->vsc_be, negotiated_features, sc->rx_vhdrlen);
+	netbe_set_cap(sc->vsc_be, negotiated_features, rx_vhdrlen);
 }
 
 static struct pci_devemu pci_de_vnet = {