Re: [PATCH v2 05/10] net: Consolidate vhost feature bits into NetClientInfo

[Jason Wang <jasowang@redhat.com>]

On Wed, Jun 18, 2025 at 11:57 PM Laurent Vivier <lvivier@redhat.com> wrote:
>
> Previously, the vhost_net_get_feature_bits() function in
> hw/net/vhost_net.c used a large switch statement to determine
> the appropriate feature bits based on the NetClientDriver type.
>
> This created unnecessary coupling between the generic vhost layer
> and specific network backends (like TAP, vhost-user, and
> vhost-vdpa).
>
> This patch moves the definition of vhost feature bits directly into the
> NetClientInfo structure for each relevant network client.
>
> This refactoring centralizes feature bit definitions where they're
> needed, making code easier to add new vhost-enabled network backends
> in the future without modifying core vhost logic.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/net/vhost_net.c       | 80 ++--------------------------------------
>  include/net/net.h        |  1 +
>  include/net/vhost-vdpa.h |  2 -
>  net/tap.c                | 19 ++++++++++
>  net/vhost-user.c         | 43 +++++++++++++++++++++
>  net/vhost-vdpa.c         |  4 +-
>  6 files changed, 70 insertions(+), 79 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 16dadd022e75..3dff819d2dbd 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -36,86 +36,14 @@
>  #include "hw/virtio/virtio-bus.h"
>  #include "linux-headers/linux/vhost.h"
>
> -
> -/* Features supported by host kernel. */
> -static const int kernel_feature_bits[] = {
> -    VIRTIO_F_NOTIFY_ON_EMPTY,
> -    VIRTIO_RING_F_INDIRECT_DESC,
> -    VIRTIO_RING_F_EVENT_IDX,
> -    VIRTIO_NET_F_MRG_RXBUF,
> -    VIRTIO_F_VERSION_1,
> -    VIRTIO_NET_F_MTU,
> -    VIRTIO_F_IOMMU_PLATFORM,
> -    VIRTIO_F_RING_PACKED,
> -    VIRTIO_F_RING_RESET,
> -    VIRTIO_F_IN_ORDER,
> -    VIRTIO_F_NOTIFICATION_DATA,
> -    VIRTIO_NET_F_RSC_EXT,
> -    VIRTIO_NET_F_HASH_REPORT,
> -    VHOST_INVALID_FEATURE_BIT
> -};
> -
> -/* Features supported by others. */
> -static const int user_feature_bits[] = {
> -    VIRTIO_F_NOTIFY_ON_EMPTY,
> -    VIRTIO_F_NOTIFICATION_DATA,
> -    VIRTIO_RING_F_INDIRECT_DESC,
> -    VIRTIO_RING_F_EVENT_IDX,
> -
> -    VIRTIO_F_ANY_LAYOUT,
> -    VIRTIO_F_VERSION_1,
> -    VIRTIO_NET_F_CSUM,
> -    VIRTIO_NET_F_GUEST_CSUM,
> -    VIRTIO_NET_F_GSO,
> -    VIRTIO_NET_F_GUEST_TSO4,
> -    VIRTIO_NET_F_GUEST_TSO6,
> -    VIRTIO_NET_F_GUEST_ECN,
> -    VIRTIO_NET_F_GUEST_UFO,
> -    VIRTIO_NET_F_HOST_TSO4,
> -    VIRTIO_NET_F_HOST_TSO6,
> -    VIRTIO_NET_F_HOST_ECN,
> -    VIRTIO_NET_F_HOST_UFO,
> -    VIRTIO_NET_F_MRG_RXBUF,
> -    VIRTIO_NET_F_MTU,
> -    VIRTIO_F_IOMMU_PLATFORM,
> -    VIRTIO_F_RING_PACKED,
> -    VIRTIO_F_RING_RESET,
> -    VIRTIO_F_IN_ORDER,
> -    VIRTIO_NET_F_RSS,
> -    VIRTIO_NET_F_RSC_EXT,
> -    VIRTIO_NET_F_HASH_REPORT,
> -    VIRTIO_NET_F_GUEST_USO4,
> -    VIRTIO_NET_F_GUEST_USO6,
> -    VIRTIO_NET_F_HOST_USO,
> -
> -    /* This bit implies RARP isn't sent by QEMU out of band */
> -    VIRTIO_NET_F_GUEST_ANNOUNCE,
> -
> -    VIRTIO_NET_F_MQ,
> -
> -    VHOST_INVALID_FEATURE_BIT
> -};
> -
>  static const int *vhost_net_get_feature_bits(struct vhost_net *net)
>  {
> -    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> -        return kernel_feature_bits;
> -    }
> -
> -    if (qemu_is_vhost_user(net->nc)) {
> -        return user_feature_bits;
> +    if (net->nc->info->vhost_feature_bits == NULL) {
> +        error_report("Feature bits not defined for this type: %d",
> +                     net->nc->info->type);
>      }
>
> -#ifdef CONFIG_VHOST_NET_VDPA
> -    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> -        return vdpa_feature_bits;
> -    }
> -#endif
> -
> -    error_report("Feature bits not defined for this type: %d",
> -                 net->nc->info->type);
> -
> -    return 0;
> +    return net->nc->info->vhost_feature_bits;
>  }
>
>  uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
> diff --git a/include/net/net.h b/include/net/net.h
> index 8a62cd6e8aab..dd11be11a39f 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -94,6 +94,7 @@ typedef struct NetClientInfo {
>      NetAnnounce *announce;
>      SetSteeringEBPF *set_steering_ebpf;
>      NetCheckPeerType *check_peer_type;
> +    const int *vhost_feature_bits;
>      IsVHostUser *is_vhost_user;
>      GetVHostNet *get_vhost_net;

Assuming we had already had get_vhost_net, it looks to me it's better
to move/reuse the vhost_feature_bits there?

Thanks