Re: [PATCH RFC 1/4] net: Introduce qmp cmd "query-netdev"

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Nov 02, 2022 at 01:42:39PM +0800, Jason Wang wrote:
> On Tue, Nov 1, 2022 at 12:19 AM <huangy81@chinatelecom.cn> wrote:
> >
> > From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> >
> > For netdev device that can offload virtio-net dataplane to slave,
> > such as vhost-net, vhost-user and vhost-vdpa, exporting it's
> > capability information and acked features would be more friendly for
> > developers. These infomation can be analyzed and compare to slave
> > capability provided by, eg dpdk or other slaves directly, helping to
> > draw conclusions about if vm network interface works normally, if
> > it vm can be migrated to another feature-compatible destination or
> > whatever else.
> >
> > For developers who devote to offload virtio-net dataplane to DPU
> > and make efforts to migrate vm lively from software-based source
> > host to DPU-offload destination host smoothly, virtio-net feature
> > compatibility is an serious issue, exporting the key capability
> > and acked_features of netdev could also help to debug greatly.
> >
> > So we export out the key capabilities of netdev, which may affect
> > the final negotiated virtio-net features, meanwhile, backed-up
> > acked_features also exported, which is used to initialize or
> > restore features negotiated between qemu and vhost slave when
> > starting vhost_dev device.
> >
> > Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> > ---
> >  net/net.c     | 44 +++++++++++++++++++++++++++++++++++++++
> >  qapi/net.json | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 110 insertions(+)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 2db160e..5d11674 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -53,6 +53,7 @@
> >  #include "sysemu/runstate.h"
> >  #include "net/colo-compare.h"
> >  #include "net/filter.h"
> > +#include "net/vhost-user.h"
> >  #include "qapi/string-output-visitor.h"
> >
> >  /* Net bridge is currently not supported for W32. */
> > @@ -1224,6 +1225,49 @@ void qmp_netdev_del(const char *id, Error **errp)
> >      }
> >  }
> >
> > +static NetDevInfo *query_netdev(NetClientState *nc)
> > +{
> > +    NetDevInfo *info = NULL;
> > +
> > +    if (!nc || !nc->is_netdev) {
> > +        return NULL;
> > +    }
> > +
> > +    info = g_malloc0(sizeof(*info));
> > +    info->name = g_strdup(nc->name);
> > +    info->type = nc->info->type;
> > +    info->ufo = nc->info->has_ufo;
> > +    info->vnet_hdr = nc->info->has_vnet_hdr;
> > +    info->vnet_hdr_len = nc->info->has_vnet_hdr_len;
> 
> So all the fields are virtio specific, I wonder if it's better to
> rename the command as query-vhost or query-virtio?
> 
> Thanks

We have info virtio already. Seems to fit there logically.


> > +
> > +    if (nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > +        info->has_acked_features = true;
> > +        info->acked_features = vhost_user_get_acked_features(nc);
> > +    }
> > +
> > +    return info;
> > +}
> > +
> > +NetDevInfoList *qmp_query_netdev(Error **errp)
> > +{
> > +    NetClientState *nc;
> > +    NetDevInfo *info = NULL;
> > +    NetDevInfoList *head = NULL, **tail = &head;
> > +
> > +    QTAILQ_FOREACH(nc, &net_clients, next) {
> > +        if (nc->info->type == NET_CLIENT_DRIVER_NIC) {
> > +            continue;
> > +        }
> > +
> > +        info = query_netdev(nc);
> > +        if (info) {
> > +            QAPI_LIST_APPEND(tail, info);
> > +        }
> > +    }
> > +
> > +    return head;
> > +}
> > +
> >  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
> >  {
> >      char *str;
> > diff --git a/qapi/net.json b/qapi/net.json
> > index dd088c0..76a6513 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -631,6 +631,72 @@
> >                         'if': 'CONFIG_VMNET' } } }
> >
> >  ##
> > +# @NetDevInfo:
> > +#
> > +# NetDev information.  This structure describes a NetDev information, including
> > +# capabilities and negotiated features.
> > +#
> > +# @name: The NetDev name.
> > +#
> > +# @type: Type of NetDev.
> > +#
> > +# @ufo: True if NetDev has ufo capability.
> > +#
> > +# @vnet-hdr: True if NetDev has vnet_hdr.
> > +#
> > +# @vnet-hdr-len: True if given length can be assigned to NetDev.
> > +#
> > +# @acked-features: Negotiated features with vhost slave device if device support
> > +#                  dataplane offload.
> > +#
> > +# Since:  7.1
> > +##
> > +{'struct': 'NetDevInfo',
> > + 'data': {
> > +    'name': 'str',
> > +    'type': 'NetClientDriver',
> > +    'ufo':'bool',
> > +    'vnet-hdr':'bool',
> > +    'vnet-hdr-len':'bool',
> > +    '*acked-features': 'uint64' } }
> > +
> > +##
> > +# @query-netdev:
> > +#
> > +# Get a list of NetDevInfo for all virtual netdev peer devices.
> > +#
> > +# Returns: a list of @NetDevInfo describing each virtual netdev peer device.
> > +#
> > +# Since: 7.1
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-netdev" }
> > +# <- {
> > +#       "return":[
> > +#          {
> > +#             "name":"hostnet0",
> > +#             "type":"vhost-user",
> > +#             "ufo":true,
> > +#             "vnet-hdr":true,
> > +#             "vnet-hdr-len":true,
> > +#             "acked-features":"5111807907",
> > +#          },
> > +#          {
> > +#             "name":"hostnet1",
> > +#             "type":"vhost-user",
> > +#             "ufo":true,
> > +#             "vnet-hdr":true,
> > +#             "vnet-hdr-len":true,
> > +#             "acked-features":"5111807907",
> > +#          }
> > +#       ]
> > +#    }
> > +#
> > +##
> > +{ 'command': 'query-netdev', 'returns': ['NetDevInfo'] }
> > +
> > +##
> >  # @RxState:
> >  #
> >  # Packets receiving state
> > --
> > 1.8.3.1
> >