[RFC PATCH] virtio-net: introduce strict peer feature check

Jason Wang posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251107020149.3223-1-jasowang@redhat.com
Maintainers: Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
hw/core/machine.c              |   1 +
hw/net/virtio-net.c            | 153 +++++++++++++++++++++++++--------
include/hw/virtio/virtio-net.h |   1 +
3 files changed, 119 insertions(+), 36 deletions(-)
[RFC PATCH] virtio-net: introduce strict peer feature check
Posted by Jason Wang 1 week ago
We used to clear features silently in virtio_net_get_features() even
if it is required. This complicates the live migration compatibility
as the management layer may think the feature is enabled but in fact
not.

Let's add a strict feature check to make sure if there's a mismatch
between the required feature and peer, fail the get_features()
immediately instead of waiting until the migration to fail. This
offload the migration compatibility completely to the management
layer.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/core/machine.c              |   1 +
 hw/net/virtio-net.c            | 153 +++++++++++++++++++++++++--------
 include/hw/virtio/virtio-net.h |   1 +
 3 files changed, 119 insertions(+), 36 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 681adbb7ac..a9e43c4990 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,6 +40,7 @@
 
 GlobalProperty hw_compat_10_1[] = {
     { TYPE_ACPI_GED, "x-has-hest-addr", "false" },
+    { TYPE_VIRTIO_NET, "strict-peer-feature-check", "false"},
 };
 const size_t hw_compat_10_1_len = G_N_ELEMENTS(hw_compat_10_1);
 
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 33116712eb..3acc5ed4a6 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3090,53 +3090,120 @@ static void virtio_net_get_features(VirtIODevice *vdev, uint64_t *features,
     virtio_add_feature_ex(features, VIRTIO_NET_F_MAC);
 
     if (!peer_has_vnet_hdr(n)) {
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_CSUM);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_TSO4);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_TSO6);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_ECN);
-
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_CSUM);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_TSO4);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_TSO6);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_ECN);
-
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_USO);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_USO4);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_USO6);
-
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO);
-        virtio_clear_feature_ex(features,
-                                VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM);
-        virtio_clear_feature_ex(features,
-                                VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM);
+        if (n->strict_peer_feature_check) {
+            if (virtio_has_feature_ex(features, VIRTIO_NET_F_CSUM) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_HOST_TSO4) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_HOST_TSO6) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_HOST_ECN) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_CSUM) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_TSO4) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_TSO6) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_ECN) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_HOST_USO) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_USO4) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_USO6) |
+                virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) |
+                virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO) |
+                virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM) |
+                virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM) |
+                virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_HASH_REPORT)) {
+                error_setg(errp, "virtio_net: peer doesn't support vnet hdr");
+                return;
+            }
+        } else {
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_CSUM);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_TSO4);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_TSO6);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_ECN);
+
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_CSUM);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_TSO4);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_TSO6);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_ECN);
+
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_USO);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_USO4);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_USO6);
+
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO);
+            virtio_clear_feature_ex(features,
+                                    VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM);
+            virtio_clear_feature_ex(features,
+                                    VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM);
 
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HASH_REPORT);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HASH_REPORT);
+        }
     }
 
     if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) {
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_UFO);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_UFO);
+        if (n->strict_peer_feature_check) {
+            if (virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_UFO) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_HOST_UFO)) {
+                error_setg(errp, "virtio_net: peer doesn't support UFO");
+                return;
+            }
+        } else {
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_UFO);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_UFO);
+        }
     }
     if (!peer_has_uso(n)) {
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_USO);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_USO4);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_USO6);
+        if (n->strict_peer_feature_check) {
+            if (virtio_has_feature_ex(features, VIRTIO_NET_F_HOST_USO) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_USO4) |
+                virtio_has_feature_ex(features, VIRTIO_NET_F_GUEST_USO6)) {
+                error_setg(errp, "virtio_net: peer doesn't support USO");
+                return;
+            }
+        } else {
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_USO);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_USO4);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_USO6);
+        }
     }
 
     if (!peer_has_tunnel(n)) {
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO);
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO);
-        virtio_clear_feature_ex(features,
-                                VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM);
-        virtio_clear_feature_ex(features,
-                                VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM);
+        if (n->strict_peer_feature_check) {
+            if (virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) |
+                virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO) |
+                virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM) |
+                virtio_has_feature_ex(features,
+                                      VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM)) {
+                error_setg(errp, "virtio_net: peer doesn't support tunnel GSO");
+                return;
+            }
+        } else {
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO);
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO);
+            virtio_clear_feature_ex(features,
+                                    VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM);
+            virtio_clear_feature_ex(features,
+                                    VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM);
+        }
     }
 
     if (!get_vhost_net(nc->peer)) {
         if (!use_own_hash) {
-            virtio_clear_feature_ex(features, VIRTIO_NET_F_HASH_REPORT);
-            virtio_clear_feature_ex(features, VIRTIO_NET_F_RSS);
+            if (n->strict_peer_feature_check) {
+                if (virtio_has_feature_ex(features, VIRTIO_NET_F_HASH_REPORT) |
+                    virtio_has_feature_ex(features, VIRTIO_NET_F_RSS)) {
+                    error_setg(errp,
+                               "virtio_net: peer doesn't support RSS/HASH_REPORT");
+                    return;
+                }
+            } else {
+                virtio_clear_feature_ex(features, VIRTIO_NET_F_HASH_REPORT);
+                virtio_clear_feature_ex(features, VIRTIO_NET_F_RSS);
+            }
         } else if (virtio_has_feature_ex(features, VIRTIO_NET_F_RSS)) {
             virtio_net_load_ebpf(n, errp);
         }
@@ -3145,14 +3212,26 @@ static void virtio_net_get_features(VirtIODevice *vdev, uint64_t *features,
     }
 
     if (!use_peer_hash) {
-        virtio_clear_feature_ex(features, VIRTIO_NET_F_HASH_REPORT);
+        if (n->strict_peer_feature_check &&
+            virtio_has_feature_ex(features, VIRTIO_NET_F_HASH_REPORT)) {
+            error_setg(errp, "virtio_net: peer doesn't HASH_REPORT");
+            return;
+        } else {
+            virtio_clear_feature_ex(features, VIRTIO_NET_F_HASH_REPORT);
+        }
 
         if (!use_own_hash || !virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
             if (!virtio_net_load_ebpf(n, errp)) {
                 return;
             }
 
-            virtio_clear_feature_ex(features, VIRTIO_NET_F_RSS);
+            if (n->strict_peer_feature_check &&
+                virtio_has_feature_ex(features, VIRTIO_NET_F_RSS)) {
+                error_setg(errp, "virtio_net: fail to attach eBPF for RSS");
+                return;
+            } else {
+                virtio_clear_feature_ex(features, VIRTIO_NET_F_RSS);
+            }
         }
     }
 
@@ -4313,6 +4392,8 @@ static const Property virtio_net_properties[] = {
                                host_features_ex,
                                VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM,
                                false),
+    DEFINE_PROP_BOOL("strict-peer-feature-check", VirtIONet,
+                     strict_peer_feature_check, true),
 };
 
 static void virtio_net_class_init(ObjectClass *klass, const void *data)
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 5b8ab7bda7..abd4ca4bb0 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -222,6 +222,7 @@ struct VirtIONet {
     /* primary failover device is hidden*/
     bool failover_primary_hidden;
     bool failover;
+    bool strict_peer_feature_check;
     DeviceListener primary_listener;
     QDict *primary_opts;
     bool primary_opts_from_json;
-- 
2.34.1