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

[Hyman]

在 2022/11/2 14:41, Michael S. Tsirkin 写道:
> 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.

Ok, it seems that 'x-query-virtio-netdev' is a good option.

> > > +
> > > +    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