Re: [PATCH v2] net/macos: implement vmnet-based network device

[Markus Armbruster]

Phillip, this doesn't apply anymore.  I'm reviewing the QAPI schema part
anyway.

Peter, there is a question for you below.  Search for "Sphinx".

phillip.ennen@gmail.com writes:

> From: Phillip Tennen <phillip@axleos.com>
>
> This patch implements a new netdev device, reachable via -netdev
> vmnet-macos, that’s backed by macOS’s vmnet framework.
>
> The vmnet framework provides native bridging support, and its usage in
> this patch is intended as a replacement for attempts to use a tap device
> via the tuntaposx kernel extension. Notably, the tap/tuntaposx approach
> never would have worked in the first place, as QEMU interacts with the
> tap device via poll(), and macOS does not support polling device files.
>
> vmnet requires either a special entitlement, granted via a provisioning
> profile, or root access. Otherwise attempts to create the virtual
> interface will fail with a “generic error” status code. QEMU may not
> currently be signed with an entitlement granted in a provisioning
> profile, as this would necessitate pre-signed binary build distribution,
> rather than source-code distribution. As such, using this netdev
> currently requires that qemu be run with root access. I’ve opened a
> feedback report with Apple to allow the use of the relevant entitlement
> with this use case:
> https://openradar.appspot.com/radar?id=5007417364447232
>
> vmnet offers three operating modes, all of which are supported by this
> patch via the “mode=host|shared|bridge” option:
>
> * "Host" mode: Allows the vmnet interface to communicate with other
> * vmnet
> interfaces that are in host mode and also with the native host.
> * "Shared" mode: Allows traffic originating from the vmnet interface to
> reach the Internet through a NAT. The vmnet interface can also
> communicate with the native host.
> * "Bridged" mode: Bridges the vmnet interface with a physical network
> interface.
>
> Each of these modes also provide some extra configuration that’s
> supported by this patch:
>
> * "Bridged" mode: The user may specify the physical interface to bridge
> with. Defaults to en0.
> * "Host" mode / "Shared" mode: The user may specify the DHCP range and
> subnet. Allocated by vmnet if not provided.
>
> vmnet also offers some extra configuration options that are not
> supported by this patch:
>
> * Enable isolation from other VMs using vmnet
> * Port forwarding rules
> * Enabling TCP segmentation offload
> * Only applicable in "shared" mode: specifying the NAT IPv6 prefix
> * Only available in "host" mode: specifying the IP address for the VM
> within an isolated network
>
> Note that this patch requires macOS 10.15 as a minimum, as this is when
> bridging support was implemented in vmnet.framework.
>
> Signed-off-by: Phillip Tennen <phillip@axleos.com>
> ---
[...]
> diff --git a/qapi/net.json b/qapi/net.json
> index c31748c87f..e4d4143243 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -450,6 +450,115 @@
>      '*vhostdev':     'str',
>      '*queues':       'int' } }
>  
> +##
> +# @VmnetOperatingMode:
> +#
> +# The operating modes in which a vmnet netdev can run
> +# Only available on macOS

Please end these sentences with a period.

I'm not sure we need "Only available on macOS".  Rendered documentation
shows the 'if' like

    If
    defined(CONFIG_DARWIN)

More of the same below.

> +#
> +# @host: the guest may communicate with the host 
> +#        and other guest network interfaces
> +#
> +# @shared: the guest may reach the Internet through a NAT, 
> +#          and may communicate with the host and other guest 
> +#          network interfaces
> +#
> +# @bridged: the guest's traffic is bridged with a 
> +#           physical network interface of the host
> +#
> +# Since: 6.0
> +##
> +{ 'enum': 'VmnetOperatingMode',
> +  'data': [ 'host', 'shared', 'bridged' ],
> +  'if': 'defined(CONFIG_DARWIN)' }
> +
> +##
> +# @NetdevVmnetModeOptionsBridged:
> +#
> +# Options for the vmnet-macos netdev
> +# that are only available in 'bridged' mode
> +# Only available on macOS

Likewise.

> +#
> +# @ifname: the physical network interface to bridge with 
> +#          (defaults to en0 if not specified)
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'NetdevVmnetModeOptionsBridged',
> +  'data': { '*ifname':  'str' },
> +  'if': 'defined(CONFIG_DARWIN)' }
> +
> +##
> +# @NetdevVmnetModeOptionsHostOrShared:
> +#
> +# Options for the vmnet-macos netdev
> +# that are only available in 'host' or 'shared' mode
> +# Only available on macOS
> +#
> +# @dhcp-start-address: the gateway address to use for the interface. 
> +#                      The range to dhcp_end_address is placed in the DHCP 
> pool.

Recommend to wrap lines around column 75.

> +#                      (only valid with mode=host|shared)

Isn't that trivial?  The type's only use is as union branch for modes
host and shared.

> +#                      (must be specified with dhcp-end-address and 
> +#                       dhcp-subnet-mask)

Does that mean you have to specify all three parameters or none?

Sphinx warns:

    qapi/net.json:492:Unexpected indentation.

Known restriction of our tooling: the position is less than useful.

Unindenting the second line like

   #                      (must be specified with dhcp-end-address and 
   #                      dhcp-subnet-mask)

here and similarly below makes Sphinx shut up.

I'm not 100% sure our tooling behaves as intended here.  Peter?

> +#                      (allocated automatically if unset)

How?

The sequence of (parenthesized sentences) looks pretty bad in rendered
documentation.  Please proof-read docs/manual/interop/qemu-qmp-ref.html.

> +#
> +# @dhcp-end-address: the DHCP IPv4 range end address to use for the 
> interface. 
> +#                      (only valid with mode=host|shared)
> +#                      (must be specified with dhcp-start-address and 
> +#                       dhcp-subnet-mask)
> +#                      (allocated automatically if unset)
> +#
> +# @dhcp-subnet-mask: the IPv4 subnet mask (string) to use on the interface.
> +#                    (only valid with mode=host|shared)
> +#                    (must be specified with dhcp-start-address and 
> +#                     dhcp-end-address)
> +#                    (allocated automatically if unset)
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'NetdevVmnetModeOptionsHostOrShared',
> +  'data': { 
> +    '*dhcp-start-address': 'str' ,
> +    '*dhcp-end-address':   'str',
> +    '*dhcp-subnet-mask':   'str' },
> +  'if': 'defined(CONFIG_DARWIN)' }
> +
> +##
> +# @NetdevVmnetModeOptions:
> +#
> +# Options specific to different operating modes of a vmnet netdev
> +# Only available on macOS
> +#
> +# @mode: the operating mode vmnet should run in
> +#
> +# Since: 6.0
> +##
> +{ 'union': 'NetdevVmnetModeOptions',
> +  'base': { 'mode': 'VmnetOperatingMode' },
> +  'discriminator': 'mode',
> +  'data': {
> +    'bridged':      'NetdevVmnetModeOptionsBridged',
> +    'host':         'NetdevVmnetModeOptionsHostOrShared',
> +    'shared':       'NetdevVmnetModeOptionsHostOrShared' },
> +  'if': 'defined(CONFIG_DARWIN)' }
> +
> +##
> +# @NetdevVmnetOptions:
> +#
> +# vmnet network backend
> +# Only available on macOS
> +#
> +# @options: a structure specifying the mode and mode-specific options
> +#           (once QAPI supports a union type as a branch to another union 
> type,
> +#            this structure can be changed to a union, and the contents of
> +#            NetdevVmnetModeOptions moved here)

Sphinx warns:

    qapi/net.json:546:Unexpected indentation.

> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'NetdevVmnetOptions',
> +  'data': {'options': 'NetdevVmnetModeOptions' },
> +  'if': 'defined(CONFIG_DARWIN)' }
> +

Awkward.

You can't use make NetdevVmnetModeOptions a branch of union Netdev,
because NetdevVmnetModeOptions is a union, and a branch must be a
struct.  To work around, you wrap struct NetdevVmnetOptions around union
NetdevVmnetModeOptions.

NetdevVmnetModeOptions has no common members other than the union
discriminator.  Why not add them as three branches to Netdev?

>  ##
>  # @NetClientDriver:
>  #
> @@ -458,10 +567,13 @@
>  # Since: 2.7
>  #
>  #        @vhost-vdpa since 5.1
> +#
> +#        @vmnet-macos since 6.0 (only available on macOS)
>  ##
>  { 'enum': 'NetClientDriver',
>    'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> -            'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa' ] }
> +            'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa',
> +            { 'name': 'vmnet-macos', 'if': 'defined(CONFIG_DARWIN)' } ] }
>  
>  ##
>  # @Netdev:
> @@ -475,6 +587,8 @@
>  # Since: 1.2
>  #
>  #        'l2tpv3' - since 2.1
> +#
> +#        'vmnet-macos' since 6.0 (only available on macOS)
>  ##
>  { 'union': 'Netdev',
>    'base': { 'id': 'str', 'type': 'NetClientDriver' },
> @@ -490,7 +604,9 @@
>      'hubport':  'NetdevHubPortOptions',
>      'netmap':   'NetdevNetmapOptions',
>      'vhost-user': 'NetdevVhostUserOptions',
> -    'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
> +    'vhost-vdpa': 'NetdevVhostVDPAOptions',
> +    'vmnet-macos': { 'type': 'NetdevVmnetOptions', 
> +                     'if': 'defined(CONFIG_DARWIN)' } } }
>  
>  ##
>  # @NetFilterDirection:
[...]