Re: [PATCH v2 1/4] qapi: net: Add query-netdevs command

[Alexey Kirillov <lekiravi@yandex-team.ru>]

ping

Sorry, I lost a point in discussion.

As I wrote early, main reason for mix frontend and backend devices is for
easy recreation of current state of network. As we have common structure
for all netdevs (`struct NetClientState`), I think, that it'll be good
idea to iterate using `net_clients`.
There is no such trouble to split `query-netdevs` by two, for example,
`query-front-netdevs` and `query-back-netdevs`, but in my opinion it'll
break consistancy in getting links between netdevs.

05.03.2020, 17:26, "Alexey Kirillov" <lekiravi@yandex-team.ru>:
> 05.03.2020, 15:03, "Markus Armbruster" <armbru@redhat.com>:
>>  Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>>
>>>   Add a qmp command that provides information about currently attached
>>>   network devices and their configuration.
>>
>>  Closes a gap in QMP; appreciated!
>>
>>>   Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>>
>>  [...]
>>>   diff --git a/qapi/net.json b/qapi/net.json
>>>   index 1cb9a7d782..4f329a1de0 100644
>>>   --- a/qapi/net.json
>>>   +++ b/qapi/net.json
>>>   @@ -750,3 +750,92 @@
>>>    ##
>>>    { 'event': 'FAILOVER_NEGOTIATED',
>>>      'data': {'device-id': 'str'} }
>>>   +
>>>   +##
>>>   +# @NetdevInfo:
>>>   +#
>>>   +# Configuration of a network device.
>>>   +#
>>>   +# @id: Device identifier.
>>>   +#
>>>   +# @type: Specify the driver used for interpreting remaining arguments.
>>>   +#
>>>   +# @peer: Connected network device.
>>
>>  @peer is optional. I assume its present when the device is connected
>>  (frontend to backend or vice versa). Correct?
>
> Yes, this field stores connected frontend/backend device @id.
>
>>>   +#
>>>   +# @queues-count: Number of queues.
>>
>>  We use plain @queues elsewhere in the schema.
>
> It can conflict with fields inside Netdev*Options, isn't it?
>
>>>   +#
>>>   +# @hub: hubid of hub, if connected to.
>>
>>  How @hub is related to @peer is not quite obvious to me. Can you give
>>  an example where @hub is present?
>
> NetdevHubPortOptions has an option @hubid. @hub gives that id, if
> netdev is connected to the hub via hubport. As example:
>
> HMP:
>
> hub 0
>  \ hub0port1: socket.0: index=0,type=socket,
>  \ hub0port0: virtio-net-pci.0: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
>
> QMP:
>
> [
>   {
>     "peer": "hub0port0",
>     "netdev": "hub0port0",
>     "hub": 0,
>     "model": "virtio-net-pci",
>     "macaddr": "52:54:00:12:34:56",
>     "type": "nic",
>     "queues-count": 1,
>     "id": "virtio-net-pci.0"
>   },
>   {
>     "peer": "hub0port1",
>     "listen": "127.0.0.1:90",
>     "hub": 0,
>     "type": "socket",
>     "queues-count": 1,
>     "id": "socket.0"
>   },
>   {
>     "peer": "socket.0",
>     "netdev": "socket.0",
>     "hub": 0,
>     "hubid": 0,
>     "type": "hubport",
>     "queues-count": 1,
>     "id": "hub0port1"
>   },
>   {
>     "peer": "virtio-net-pci.0",
>     "netdev": "virtio-net-pci.0",
>     "hub": 0,
>     "hubid": 0,
>     "type": "hubport",
>     "queues-count": 1,
>     "id": "hub0port0"
>   }
> ]
>
>>>   +#
>>>   +# @perm-mac: Original MAC address.
>>
>>  What does "perm-" mean?
>>
>>  It's optional. When exactly is it present?
>
> @perm-mac is the permanent (original) MAC address. It only used
> for nic, because most of nic realizations can change MAC at
> runtime and/or reset it to default (permanent) value.
>
>>>   +#
>>>   +# Since: 5.0
>>>   +##
>>>   +{ 'union': 'NetdevInfo',
>>>   + 'base': { 'id': 'str',
>>>   + 'type': 'NetClientDriver',
>>>   + '*peer': 'str',
>>>   + 'queues-count': 'int',
>>>   + '*hub': 'int',
>>>   + '*perm-mac': 'str' },
>>>   + 'discriminator': 'type',
>>>   + 'data': {
>>>   + 'nic': 'NetLegacyNicOptions',
>>>   + 'user': 'NetdevUserOptions',
>>>   + 'tap': 'NetdevTapOptions',
>>>   + 'l2tpv3': 'NetdevL2TPv3Options',
>>>   + 'socket': 'NetdevSocketOptions',
>>>   + 'vde': 'NetdevVdeOptions',
>>>   + 'bridge': 'NetdevBridgeOptions',
>>>   + 'hubport': 'NetdevHubPortOptions',
>>>   + 'netmap': 'NetdevNetmapOptions',
>>>   + 'vhost-user': 'NetdevVhostUserOptions' } }
>>
>>  This is a copy of union 'Netdev' with a few additional common members
>>  (@peer, @queues-count, @hub, @perm-mac). I can't see how to avoid the
>>  duplication without adding nesting on the wire.
>>
>>>   +
>>>   +##
>>>   +# @query-netdevs:
>>>   +#
>>>   +# Get a list of @NetdevInfo for all virtual network devices.
>>>   +#
>>>   +# Returns: a list of @NetdevInfo describing each virtual network device.
>>>   +#
>>>   +# Since: 5.0
>>>   +#
>>>   +# Example:
>>>   +#
>>>   +# -> { "execute": "query-netdevs" }
>>>   +# <- { "return": [
>>>   +# {
>>>   +# "peer": "netdev0",
>>>   +# "netdev": "netdev0",
>>>   +# "perm-mac": "52:54:00:12:34:56"
>>>   +# "model": "virtio-net-pci",
>>>   +# "macaddr": "52:54:00:12:34:56",
>>>   +# "queues-count": 1,
>>>   +# "type": "nic",
>>>   +# "id": "net0"
>>>   +# },
>>>   +# {
>>>   +# "peer": "net0",
>>>   +# "ipv6": true,
>>>   +# "ipv4": true,
>>>   +# "host": "10.0.2.2",
>>>   +# "queues-count": 1,
>>>   +# "ipv6-dns": "fec0::3",
>>>   +# "ipv6-prefix": "fec0::",
>>>   +# "net": "10.0.2.0/255.255.255.0",
>>>   +# "ipv6-host": "fec0::2",
>>>   +# "type": "user",
>>>   +# "dns": "10.0.2.3",
>>>   +# "hostfwd": [
>>>   +# {
>>>   +# "str": "tcp::20004-:22"
>>>   +# }
>>>   +# ],
>>>   +# "ipv6-prefixlen": 64,
>>>   +# "id": "netdev0",
>>>   +# "restrict": false
>>>   +# }
>>>   +# ]
>>>   +# }
>>>   +#
>>>   +##
>>>   +{ 'command': 'query-netdevs', 'returns': ['NetdevInfo'] }
>>
>>  Like HMP "info network" and -net, this mixes frontends ("type": "nic")
>>  and backends. Unlike query-chardev and query-block. Hmm.
>>
>>  A long time ago, all we had was -net: "-net nic" for configuring
>>  frontends, "-net none" for suppressing a default frontend + backend, and
>>  "-net anything-else" for configuring backends. "info network" showed
>>  the stuff set up with -net.
>>
>>  In v0.12, we got -device for configuring frontends, and -netdev for
>>  backends. -netdev is like -net less "none", "nic", and the hub
>>  weirdness. "info network" was extended to also show all this.
>>
>>  In v2.12, we got -nic, replacing -net nic.
>>
>>  Unless I'm missing something, -net is just for backward compatibility
>>  now.
>>
>>  What's the use case for query-networks reporting frontends?
>
> In my vision, new QMP command is the replacement for old
> HMP command. It must provide information about all
> network devices, mainly for recreate similar net topology.
> Currently, there are no differrence between fronted and
> backend devices in context of my command, because
> all of them use the same interface in NetClientState.
>
>>
>
> --
> Alexey Kirillov
> Yandex.Cloud

-- 
Alexey Kirillov
Yandex.Cloud