Re: [Qemu-devel] [PATCH v2 1/2] slirp: Add "query-usernet" QMP command

[Eric Blake <eblake@redhat.com>]

On 02/26/2018 01:58 AM, Fam Zheng wrote:
> HMP "info usernet" has been available but it isn't ideal for programed

s/programed/programmed/

> use cases. This closes the gap in QMP by adding a counterpart
> "query-usernet" command. It is basically translated from
> the HMP slirp_connection_info() loop, which now calls the QMP
> implementation and prints the data, just like other HMP info_* commands.
> 
> The TCPS_* macros are now defined as a QAPI enum.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   net/slirp.c      |  26 +++++++
>   qapi/net.json    | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   slirp/libslirp.h |   1 +
>   slirp/misc.c     | 156 +++++++++++++++++++++++++++++-------------
>   slirp/tcp.h      |  15 -----
>   5 files changed, 339 insertions(+), 60 deletions(-)
> 

> +++ b/net/slirp.c
> @@ -36,6 +36,7 @@
>   #include "monitor/monitor.h"
>   #include "qemu/error-report.h"
>   #include "qemu/sockets.h"
> +#include "slirp/slirp.h"
>   #include "slirp/libslirp.h"
>   #include "slirp/ip6.h"
>   #include "chardev/char-fe.h"
> @@ -43,6 +44,7 @@
>   #include "qemu/cutils.h"
>   #include "qapi/error.h"
>   #include "qapi/qmp/qdict.h"
> +#include "qmp-commands.h"

Needs rebasing to master, although off-hand I think it is probably as 
simple as a switch to qapi/qapi-net-commands.h

> +++ b/qapi/net.json
> @@ -706,3 +706,204 @@
>   ##
>   { 'event': 'NIC_RX_FILTER_CHANGED',
>     'data': { '*name': 'str', 'path': 'str' } }
> +
> +##
> +# @TCPS:
> +#
> +# TCP States of a SLIRP connection.
> +#
> +# - States where connections are not established: none, closed, listen, syn_sent,
> +#   syn_received
> +#
> +# - States where user has closed: fin_wait_1, closing, last_ack, fin_wait_2,
> +#   time_wait
> +#
> +# - States await ACK of FIN: fin_wait_1, closing, last_ack

s/await/awaiting/

> +#
> +# 'none' state is used only when host forwarding
> +#
> +# Since 2.12
> +#
> +##
> +{ 'enum': 'TCPS',
> +  'data':
> +   ['closed',
> +    'listen',
> +    'syn_sent',

Probably nicer to spell this 'syn-sent'; it results in the same C code, 
but is more consistent with modern QMP naming.

> +    'syn_received',
> +    'established',
> +    'close_wait',
> +    'fin_wait_1',
> +    'closing',
> +    'last_ack',
> +    'fin_wait_2',
> +    'time_wait',

Likewise for these _.

> +    'none'
> +   ] }
> +
> +##
> +# @UsernetTCPConnection:
> +#
> +# SLIRP TCP information.
> +#
> +# @state: tcp connection state
> +#
> +# @hostfwd: whether this connection has host port forwarding
> +#
> +# @fd: the file descriptor of the connection
> +#
> +# @src_addr: source address of host port forwarding

Likewise, for src-addr and friends.

> +#
> +# @src_port: source port of host port forwarding
> +#
> +# @dest_addr: destination address of host port forwarding
> +#
> +# @dest_port: destination port of host port forwarding
> +#
> +# @recv_buffered: number of bytes queued in the receive buffer
> +#
> +# @send_buffered: number of bytes queued in the send buffer
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'UsernetTCPConnection',
> +  'data': {
> +    'state': 'TCPS',
> +    'hostfwd': 'bool',
> +    'fd': 'int',
> +    'src_addr': 'str',
> +    'src_port': 'int',
> +    'dest_addr': 'str',
> +    'dest_port': 'int',
> +    'recv_buffered': 'int',
> +    'send_buffered': 'int'
> +  } }
> +
> +##
> +# @UsernetUDPConnection:
> +#
> +# SLIRP UDP information.
> +#
> +# @hostfwd: whether this connection has host port forwarding
> +#
> +# @expire_time_ms: time in microseconds after which this connection will expire

And again.

> +#
> +# @fd: the file descriptor of the connection
> +#
> +# @src_addr: source address of host port forwarding
> +#
> +# @src_port: source port of host port forwarding
> +#
> +# @dest_addr: destination address of host port forwarding
> +#
> +# @dest_port: destination port of host port forwarding
> +#
> +# @recv_buffered: number of bytes queued in the receive buffer
> +#
> +# @send_buffered: number of bytes queued in the send buffer
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'UsernetUDPConnection',
> +  'data': {
> +    'hostfwd': 'bool',
> +    'expire_time_ms': 'int',
> +    'fd': 'int',
> +    'src_addr': 'str',
> +    'src_port': 'int',
> +    'dest_addr': 'str',
> +    'dest_port': 'int',
> +    'recv_buffered': 'int',
> +    'send_buffered': 'int'
> +    } }
> +
> +##
> +# @UsernetICMPConnection:
> +#
> +# SLIRP ICMP information.
> +#
> +# @expire_time_ms: time in microseconds after which this connection will expire

Etc.

> +#
> +# @fd: the file descriptor of the connection
> +#
> +# @src_addr: source address of host port forwarding
> +#
> +# @dest_addr: destination address of host port forwarding
> +#
> +# @recv_buffered: number of bytes queued in the receive buffer
> +#
> +# @send_buffered: number of bytes queued in the send buffer
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'UsernetICMPConnection',
> +  'data': {
> +    'expire_time_ms': 'int',
> +    'fd': 'int',
> +    'src_addr': 'str',
> +    'dest_addr': 'str',
> +    'recv_buffered': 'int',
> +    'send_buffered': 'int'
> +    } }
> +
> +##
> +# @UsernetType:
> +#
> +# Available netdev drivers.
> +#
> +# Since: 2.7

Since 2.7? Why not 2.12?

> +##
> +{ 'enum': 'UsernetType',
> +  'data': [ 'tcp', 'udp', 'icmp' ] }
> +
> +##
> +# @UsernetConnection:
> +#
> +# SLIRP usernet connection information.
> +#
> +# Since: 2.12
> +##
> +{ 'union': 'UsernetConnection',
> +  'discriminator': 'type',
> +  'base': { 'type': 'UsernetType' },
> +  'data': {
> +    'tcp':     'UsernetTCPConnection',
> +    'udp':     'UsernetUDPConnection',
> +    'icmp':    'UsernetICMPConnection'
> +    } }

Looks good.

> +
> +##
> +# @UsernetInfo:
> +#
> +# SLIRP usernet information.
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'UsernetInfo',
> +  'data': {
> +    'id':              'str',
> +    'vlan':            'int',
> +    'connections':     ['UsernetConnection']
> +} }
> +
> +##
> +# @query-usernet:
> +#
> +# Return SLIRP network information.
> +#
> +# Since: 2.11

2.12

> +#
> +# Example:
> +#
> +# -> { "execute": "query-usernet", "arguments": { } }
> +# <- { "return": [
> +#         {
> +#             "promiscuous": true,
> +#             "name": "vnet0",
> +#         }

Trailing comma is not valid JSON; is the example incomplete?  And 
doesn't match the UsernetInfo struct, which would have "id", "vlan", and 
"connections" instead of "promiscuous" and "name".

> +#       ]
> +#    }
> +#
> +##
> +{ 'command': 'query-usernet',
> +  'returns': ['UsernetInfo'] }

The idea makes sense, but you'll need a v3.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org