Re: [Xen-devel] [PATCH v2 1/4] qapi: net: Add query-netdevs command
05.03.2020, 15:03, "Markus Armbruster" :
> Alexey Kirillov 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
>
> [...]
>> 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 -net
Re: [Xen-devel] [PATCH v2 1/4] qapi: net: Add query-netdevs command
04.03.2020, 18:57, "Laurent Vivier" :
> On 04/03/2020 14:06, Alexey Kirillov wrote:
>> Add a qmp command that provides information about currently attached
>> network devices and their configuration.
>>
>> Signed-off-by: Alexey Kirillov
>> ---
>> include/net/net.h | 1 +
>> net/hub.c | 8 +++
>> net/l2tpv3.c | 19 +++
>> net/net.c | 91 +
>> net/netmap.c | 13 +
>> net/slirp.c | 126 ++
>> net/socket.c | 71 ++
>> net/tap-win32.c | 9
>> net/tap.c | 103 +++--
>> net/vde.c | 26 ++
>> net/vhost-user.c | 18 +--
>> qapi/net.json | 89
>> 12 files changed, 566 insertions(+), 8 deletions(-)
>
> ...
>> diff --git a/net/net.c b/net/net.c
>> index 9e93c3f8a1..01e0548295 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -54,6 +54,7 @@
>> #include "sysemu/sysemu.h"
>> #include "net/filter.h"
>> #include "qapi/string-output-visitor.h"
>> +#include "qapi/clone-visitor.h"
>>
>> /* Net bridge is currently not supported for W32. */
>> #if !defined(_WIN32)
>> @@ -128,6 +129,12 @@ char *qemu_mac_strdup_printf(const uint8_t *macaddr)
>>
>> void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6])
>> {
>> + g_assert(nc->stored_config);
>> +
>> + g_free(nc->stored_config->u.nic.macaddr);
>> + nc->stored_config->u.nic.macaddr = g_strdup_printf(MAC_FMT,
>> + MAC_ARG(macaddr));
>> +
>
> Why do you use this rather than the qemu_mac_strdup_printf() function
> defined above?
>
> qemu_mac_strdup_printf():
> 890ee6abb385 ("net: add MAC address string printer")
>
> MAC_FMT/MAC_ARG:
> 6d1d4939a647 ("net: Add macros for MAC address tracing")
>
> MAC_FMT/MAC_ARG seems to be reserved for tracing.
>
> Thanks,
> Laurent
Somehow, I managed not to notice this feature.
Thank you for pointing this out, I will definitely fix this place.
--
Alexey Kirillov
Yandex.Cloud
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] qapi: net: Add query-netdevs command
Alexey Kirillov 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
[...]
> 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?
> +#
> +# @queues-count: Number of queues.
We use plain @queues elsewhere in the schema.
> +#
> +# @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?
> +#
> +# @perm-mac: Original MAC address.
What does "perm-" mean?
It's optional. When exactly is it present?
> +#
> +# 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?
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] qapi: net: Add query-netdevs command
On 04/03/2020 14:06, Alexey Kirillov wrote:
> Add a qmp command that provides information about currently attached
> network devices and their configuration.
>
> Signed-off-by: Alexey Kirillov
> ---
> include/net/net.h | 1 +
> net/hub.c | 8 +++
> net/l2tpv3.c | 19 +++
> net/net.c | 91 +
> net/netmap.c | 13 +
> net/slirp.c | 126 ++
> net/socket.c | 71 ++
> net/tap-win32.c | 9
> net/tap.c | 103 +++--
> net/vde.c | 26 ++
> net/vhost-user.c | 18 +--
> qapi/net.json | 89
> 12 files changed, 566 insertions(+), 8 deletions(-)
>
...
> diff --git a/net/net.c b/net/net.c
> index 9e93c3f8a1..01e0548295 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -54,6 +54,7 @@
> #include "sysemu/sysemu.h"
> #include "net/filter.h"
> #include "qapi/string-output-visitor.h"
> +#include "qapi/clone-visitor.h"
>
> /* Net bridge is currently not supported for W32. */
> #if !defined(_WIN32)
> @@ -128,6 +129,12 @@ char *qemu_mac_strdup_printf(const uint8_t *macaddr)
>
> void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6])
> {
> +g_assert(nc->stored_config);
> +
> +g_free(nc->stored_config->u.nic.macaddr);
> +nc->stored_config->u.nic.macaddr = g_strdup_printf(MAC_FMT,
> + MAC_ARG(macaddr));
> +
Why do you use this rather than the qemu_mac_strdup_printf() function
defined above?
qemu_mac_strdup_printf():
890ee6abb385 ("net: add MAC address string printer")
MAC_FMT/MAC_ARG:
6d1d4939a647 ("net: Add macros for MAC address tracing")
MAC_FMT/MAC_ARG seems to be reserved for tracing.
Thanks,
Laurent
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/4] qapi: net: Add query-netdevs command
Add a qmp command that provides information about currently attached
network devices and their configuration.
Signed-off-by: Alexey Kirillov
---
include/net/net.h | 1 +
net/hub.c | 8 +++
net/l2tpv3.c | 19 +++
net/net.c | 91 +
net/netmap.c | 13 +
net/slirp.c | 126 ++
net/socket.c | 71 ++
net/tap-win32.c | 9
net/tap.c | 103 +++--
net/vde.c | 26 ++
net/vhost-user.c | 18 +--
qapi/net.json | 89
12 files changed, 566 insertions(+), 8 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index e175ba9677..2c8956c0b3 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -92,6 +92,7 @@ struct NetClientState {
char *model;
char *name;
char info_str[256];
+NetdevInfo *stored_config;
unsigned receive_disabled : 1;
NetClientDestructor *destructor;
unsigned int queue_index;
diff --git a/net/hub.c b/net/hub.c
index 5795a678ed..37995b5517 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -148,6 +148,7 @@ static NetHubPort *net_hub_port_new(NetHub *hub, const char
*name,
NetHubPort *port;
int id = hub->num_ports++;
char default_name[128];
+NetdevHubPortOptions *stored;
if (!name) {
snprintf(default_name, sizeof(default_name),
@@ -160,6 +161,13 @@ static NetHubPort *net_hub_port_new(NetHub *hub, const
char *name,
port->id = id;
port->hub = hub;
+/* Store startup parameters */
+nc->stored_config = g_new0(NetdevInfo, 1);
+nc->stored_config->type = NET_CLIENT_DRIVER_HUBPORT;
+stored = &nc->stored_config->u.hubport;
+
+stored->hubid = hub->id;
+
QLIST_INSERT_HEAD(&hub->ports, port, next);
return port;
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 55fea17c0f..f4e45e7b28 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -535,6 +535,7 @@ int net_init_l2tpv3(const Netdev *netdev,
struct addrinfo hints;
struct addrinfo *result = NULL;
char *srcport, *dstport;
+NetdevL2TPv3Options *stored;
nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name);
@@ -726,6 +727,24 @@ int net_init_l2tpv3(const Netdev *netdev,
l2tpv3_read_poll(s, true);
+/* Store startup parameters */
+nc->stored_config = g_new0(NetdevInfo, 1);
+nc->stored_config->type = NET_CLIENT_DRIVER_L2TPV3;
+stored = &nc->stored_config->u.l2tpv3;
+
+memcpy(stored, l2tpv3, sizeof(NetdevL2TPv3Options));
+
+stored->src = g_strdup(l2tpv3->src);
+stored->dst = g_strdup(l2tpv3->dst);
+
+if (l2tpv3->has_srcport) {
+stored->srcport = g_strdup(l2tpv3->srcport);
+}
+
+if (l2tpv3->has_dstport) {
+stored->dstport = g_strdup(l2tpv3->dstport);
+}
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"l2tpv3: connected");
return 0;
diff --git a/net/net.c b/net/net.c
index 9e93c3f8a1..01e0548295 100644
--- a/net/net.c
+++ b/net/net.c
@@ -54,6 +54,7 @@
#include "sysemu/sysemu.h"
#include "net/filter.h"
#include "qapi/string-output-visitor.h"
+#include "qapi/clone-visitor.h"
/* Net bridge is currently not supported for W32. */
#if !defined(_WIN32)
@@ -128,6 +129,12 @@ char *qemu_mac_strdup_printf(const uint8_t *macaddr)
void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6])
{
+g_assert(nc->stored_config);
+
+g_free(nc->stored_config->u.nic.macaddr);
+nc->stored_config->u.nic.macaddr = g_strdup_printf(MAC_FMT,
+ MAC_ARG(macaddr));
+
snprintf(nc->info_str, sizeof(nc->info_str),
"model=%s,macaddr=%02x:%02x:%02x:%02x:%02x:%02x",
nc->model,
@@ -283,6 +290,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
NetClientState **peers = conf->peers.ncs;
NICState *nic;
int i, queues = MAX(1, conf->peers.queues);
+NetLegacyNicOptions *stored;
assert(info->type == NET_CLIENT_DRIVER_NIC);
assert(info->size >= sizeof(NICState));
@@ -298,6 +306,27 @@ NICState *qemu_new_nic(NetClientInfo *info,
nic->ncs[i].queue_index = i;
}
+/* Store startup parameters */
+nic->ncs[0].stored_config = g_new0(NetdevInfo, 1);
+nic->ncs[0].stored_config->type = NET_CLIENT_DRIVER_NIC;
+stored = &nic->ncs[0].stored_config->u.nic;
+
+/* Read-only in runtime */
+nic->ncs[0].stored_config->has_perm_mac = true;
+nic->ncs[0].stored_config->perm_mac = g_strdup_printf(MAC_FMT,
+MAC_ARG(conf->macaddr.a));
+
+if (peers[0]) {
+stored->has_netdev = true;
+stored->netdev = g_strdup(peers[0]->name);
+}
+
+stored->has_macaddr = true;
+stored->macaddr = g_strdup_printf(MAC_FMT, MAC_ARG(conf->macaddr.a));
+
+stored->has_model = true;
+stored->model = g_strdup(model);
+
return nic;
}
@@ -34
