vtnet: Retry feature negotiation without offloads

A device is permitted to reject an otherwise valid subset of its
offered features by refusing to accept FEATURES_OK (VirtIO v1.3,
2.2.2).  Apple's Virtualization.framework does this in practice;
it treats the offered CSUM/TSO offloads as all-or-nothing, while
vtnet's default request contains only part of that group because
of hw.vtnet.lro_disable that would drop the guest TSO bits, thus
negotiation fails and the device does not attach.

If FEATURES_OK is rejected, retry the negotiation once with every
offload-related feature stripped.  Changing the feature set after
a failed FEATURES_OK requires re-initialising from device reset
(VirtIO v1.3, 3.1.1), so the retry goes through virtio_reinit().

A NIC without offloads is preferable to no NIC at all.  Devices
that accept the initial feature set are unaffected, while those
that also reject the reduced set continue to fail attachment as
before.

Signed-off-by:	Faraz Vahedi <kfv@kfv.io>
Reviewed by:	adrian
Pull Request:	https://github.com/freebsd/freebsd-src/pull/2322

2 files changed, 30 insertions, 2 deletions

diff --git a/sys/dev/virtio/network/if_vtnet.c b/sys/dev/virtio/network/if_vtnet.c
index ad642052d06d..ac5c08f60a0e 100644
--- a/sys/dev/virtio/network/if_vtnet.c
+++ b/sys/dev/virtio/network/if_vtnet.c
@@ -758,7 +758,7 @@ vtnet_negotiate_features(struct vtnet_softc *sc)
 {
 	device_t dev;
 	uint64_t features, negotiated_features;
-	int no_csum;
+	int error, no_csum;
 
 	dev = sc->vtnet_dev;
 	features = virtio_bus_is_modern(dev) ? VTNET_MODERN_FEATURES :
@@ -836,7 +836,19 @@ vtnet_negotiate_features(struct vtnet_softc *sc)
 	sc->vtnet_features = negotiated_features;
 	sc->vtnet_negotiated_features = negotiated_features;
 
-	return (virtio_finalize_features(dev));
+	error = virtio_finalize_features(dev);
+	if (error != 0 && (features & VTNET_OFFLOAD_FEATURES) != 0) {
+		device_printf(dev,
+		    "retrying feature negotiation without offloads\n");
+		features &= ~VTNET_OFFLOAD_FEATURES;
+		negotiated_features &= ~VTNET_OFFLOAD_FEATURES;
+		sc->vtnet_flags &= ~VTNET_FLAG_LRO_NOMRG;
+		sc->vtnet_features = negotiated_features;
+		sc->vtnet_negotiated_features = negotiated_features;
+		error = virtio_reinit(dev, features);
+	}
+
+	return (error);
 }
 
 static int
diff --git a/sys/dev/virtio/network/if_vtnetvar.h b/sys/dev/virtio/network/if_vtnetvar.h
index e445bdf6d6cb..77a7680e5b43 100644
--- a/sys/dev/virtio/network/if_vtnetvar.h
+++ b/sys/dev/virtio/network/if_vtnetvar.h
@@ -369,6 +369,22 @@ CTASSERT(sizeof(struct vtnet_mac_filter) <= PAGE_SIZE);
 #define VTNET_LRO_FEATURES (VIRTIO_NET_F_GUEST_TSO4 | \
     VIRTIO_NET_F_GUEST_TSO6 | VIRTIO_NET_F_GUEST_ECN)
 
+/*
+ * Union of the offload-related features offered by the driver.  As per spec,
+ * a device is permitted to reject an otherwise valid subset of its offered
+ * features by failing FEATURES_OK (v1.3 §2.2.2).  Offloads are where this
+ * happens in practice, so feature negotiation retries without this entire
+ * group when the device rejects the first feature set.
+ *
+ * Must cover every offload-related bit in VTNET_COMMON_FEATURES.
+ */
+#define VTNET_OFFLOAD_FEATURES \
+    (VIRTIO_NET_F_CSUM			| \
+     VIRTIO_NET_F_GUEST_CSUM		| \
+     VIRTIO_NET_F_CTRL_GUEST_OFFLOADS	| \
+     VTNET_TSO_FEATURES			| \
+     VTNET_LRO_FEATURES)
+
 #define VTNET_MIN_MTU		68
 #define VTNET_MAX_MTU		65536
 #define VTNET_MAX_RX_SIZE	65550