[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 05/12] qapi: net: add stream and dgram netdevs
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v5 05/12] qapi: net: add stream and dgram netdevs |
Date: |
Wed, 29 Jun 2022 13:20:26 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Laurent Vivier <lvivier@redhat.com> writes:
> Copied from socket netdev file and modified to use SocketAddress
> to be able to introduce new features like unix socket.
>
> "udp" and "mcast" are squashed into dgram netdev, multicast is detected
> according to the IP address type.
> "listen" and "connect" modes are managed by stream netdev. An optional
> parameter "server" defines the mode (server by default)
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
> ---
[...]
> diff --git a/qapi/net.json b/qapi/net.json
> index d6f7cfd4d656..32a9b1a5ac6c 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -7,6 +7,7 @@
> ##
>
> { 'include': 'common.json' }
> +{ 'include': 'sockets.json' }
>
> ##
> # @set_link:
> @@ -566,6 +567,42 @@
> '*isolated': 'bool' },
> 'if': 'CONFIG_VMNET' }
>
> +##
> +# @NetdevStreamOptions:
> +#
> +# Configuration info for stream socket netdev
> +#
> +# @addr: socket address to listen on (server=true)
> +# or connect to (server=false)
Only some address types are valid, as discussed in review of v4.
Shouldn't we document that here?
> +# @server: create server socket (default: true)
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'NetdevStreamOptions',
> + 'data': {
> + 'addr': 'SocketAddress',
> + '*server': 'bool' } }
> +
> +##
> +# @NetdevDgramOptions:
> +#
> +# Configuration info for datagram socket netdev.
> +#
> +# @remote: remote address
> +# @local: local address
> +#
> +# The code checks there is at least one of these options and reports an error
> +# if not. If remote address is present and it's a multicast address, local
> +# address is optional. Otherwise local address is required and remote address
> +# is optional.
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'NetdevDgramOptions',
> + 'data': {
> + '*local': 'SocketAddress',
> + '*remote': 'SocketAddress' } }
Hard to see, but the space in "} }" is funny: it's U+00A0
(NO-BREAK-SPACE) encoded in UTF-8. Make it a plain old space, please.
Only some combinations of address types are valid, as discussed in
review of v4. Shouldn't we document that here?
> +
> ##
> # @NetClientDriver:
> #
> @@ -579,8 +616,9 @@
> # @vmnet-bridged since 7.1
> ##
> { 'enum': 'NetClientDriver',
> - 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> - 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa',
> + 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'stream',
> + 'dgram', 'vde', 'bridge', 'hubport', 'netmap', 'vhost-user',
> + 'vhost-vdpa',
> { 'name': 'vmnet-host', 'if': 'CONFIG_VMNET' },
> { 'name': 'vmnet-shared', 'if': 'CONFIG_VMNET' },
> { 'name': 'vmnet-bridged', 'if': 'CONFIG_VMNET' }] }
> @@ -610,6 +648,8 @@
> 'tap': 'NetdevTapOptions',
> 'l2tpv3': 'NetdevL2TPv3Options',
> 'socket': 'NetdevSocketOptions',
> + 'stream': 'NetdevStreamOptions',
> + 'dgram': 'NetdevDgramOptions',
> 'vde': 'NetdevVdeOptions',
> 'bridge': 'NetdevBridgeOptions',
> 'hubport': 'NetdevHubPortOptions',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 377d22fbd82f..2be0cbd2549b 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2722,6 +2722,18 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
> "-netdev socket,id=str[,fd=h][,udp=host:port][,localaddr=host:port]\n"
> " configure a network backend to connect to another
> network\n"
> " using an UDP tunnel\n"
> + "-netdev
> stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port\n"
> + "-netdev stream,id=str[,server=on|off],addr.type=fd,addr.str=h\n"
> + " configure a network backend to connect to another
> network\n"
> + " using a socket connection in stream mode.\n"
> + "-netdev
> dgram,id=str,remote.type=inet,remote.host=maddr,remote.port=port[,local.type=inet,local.host=addr]\n"
> + "-netdev
> dgram,id=str,remote.type=inet,remote.host=maddr,remote.port=port[,local.type=fd,local.str=h]\n"
> + " configure a network backend to connect to a multicast
> maddr and port\n"
> + " use 'local.host=addr' to specify the host address to
> send packets from\n"
I think we use ``local.host=addr`` markup.
> + "-netdev
> dgram,id=str,local.type=inet,local.host=addr,local.port=port[,remote.type=inet,remote.host=addr,remote.port=port]\n"
> + "-netdev dgram,id=str,local.type=fd,local.str=h\n"
> + " configure a network backend to connect to another
> network\n"
> + " using an UDP tunnel\n"
> #ifdef CONFIG_VDE
> "-netdev
> vde,id=str[,sock=socketpath][,port=n][,group=groupname][,mode=octalmode]\n"
> " configure a network backend to connect to port 'n' of a
> vde switch\n"
I intend to look at this again after we decide what to do about the doc
comments in net.json.
- [PATCH v5 00/12] qapi: net: add unix socket type support to netdev backend, Laurent Vivier, 2022/06/27
- [PATCH v5 02/12] net: remove the @errp argument of net_client_inits(), Laurent Vivier, 2022/06/27
- [PATCH v5 01/12] net: introduce convert_host_port(), Laurent Vivier, 2022/06/27
- [PATCH v5 03/12] net: simplify net_client_parse() error management, Laurent Vivier, 2022/06/27
- [PATCH v5 04/12] qapi: net: introduce a way to bypass qemu_opts_parse_noisily(), Laurent Vivier, 2022/06/27
- [PATCH v5 06/12] net: stream: Don't ignore EINVAL on netdev socket connection, Laurent Vivier, 2022/06/27
- [PATCH v5 05/12] qapi: net: add stream and dgram netdevs, Laurent Vivier, 2022/06/27
- [PATCH v5 07/12] net: stream: add unix socket, Laurent Vivier, 2022/06/27
- [PATCH v5 08/12] net: dgram: make dgram_dst generic, Laurent Vivier, 2022/06/27
- [PATCH v5 09/12] net: dgram: move mcast specific code from net_socket_fd_init_dgram(), Laurent Vivier, 2022/06/27
- [PATCH v5 10/12] net: dgram: add unix socket, Laurent Vivier, 2022/06/27
- [PATCH v5 11/12] qemu-sockets: introduce socket_uri(), Laurent Vivier, 2022/06/27
- [PATCH v5 12/12] net: stream: move to QIO, Laurent Vivier, 2022/06/27