Re: [PATCH v3] net/macos: implement vmnet-based netdev

[Jason Wang <jasowang@redhat.com>]

在 2021/8/7 下午8:24, Markus Armbruster 写道:
> Jason, did this fall through the cracks?


Kind of, will review this week.

6.2 material though.

Thanks


>
> My review is for the QAPI schema only.
>
> Akihiko Odaki <akihiko.odaki@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.
>>
>> Rebased to commit 9aef0954195cc592e86846dbbe7f3c2c5603690a by Akihiko
>> Odaki.
>>
>> Signed-off-by: Phillip Tennen <phillip@axleos.com>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
>> Message-Id: <20210315103209.20870-1-akihiko.odaki@gmail.com>
> [...]
>
>> diff --git a/qapi/net.json b/qapi/net.json
>> index 7fab2e7cd8a..e3b67f174fc 100644
>> --- a/qapi/net.json
>> +++ b/qapi/net.json
>> @@ -452,6 +452,115 @@
>>       '*vhostdev':     'str',
>>       '*queues':       'int' } }
>>   
>> +##
>> +# @VmnetOperatingMode:
>> +#
>> +# The operating modes in which a vmnet netdev can run
>> +# Only available on macOS
> Generated qemu-qmp-ref.7 and .html show this as
>
>      The operating modes in which a vmnet netdev can run Only available
>      on macOS
>
> Please end your sentences with periods :)
>
> More of the same below.  Proof-reading the generated documentation is
> always a good idea, and often forgotten (I've been guilty of that, too).
>
>> +#
>> +# @host: the guest may communicate with the host
>> +#        and other guest network interfaces
>> +#
>> +# @shared: the guest may reach the Internet through a NAT,
> Scratch "a"?
>
>> +#          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
> "bridged width" or "bridged to"?  I'm not a networking guy...
>
>> +#
>> +# Since: 6.0
> 6.2
>
>> +##
>> +{ 'enum': 'VmnetOperatingMode',
>> +  'data': [ 'host', 'shared', 'bridged' ],
>> +  'if': 'defined(CONFIG_VMNET)' }
> I suspect we want 'defined(CONFIG_VMNET) && defined(CONFIG_DARWIN)',
> here and below.
>
>> +
>> +##
>> +# @NetdevVmnetModeOptionsBridged:
>> +#
>> +# Options for the vmnet-macos netdev
>> +# that are only available in 'bridged' mode
>> +# Only available on macOS
>> +#
>> +# @ifname: the physical network interface to bridge with
>> +#          (defaults to en0 if not specified)
> Scratch " if not specified".
>
>> +#
>> +# Since: 6.0
>> +##
>> +{ 'struct': 'NetdevVmnetModeOptionsBridged',
>> +  'data': { '*ifname':  'str' },
>> +  'if': 'defined(CONFIG_VMNET)' }
>> +
>> +##
>> +# @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.
>> +#                      (only valid with mode=host|shared)
>> +#                      (must be specified with dhcp-end-address and
>> +#                       dhcp-subnet-mask)
>> +#                      (allocated automatically if unset)
>> +#
>> +# @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)
> No IPv6.  Is it because the underlying OS feature can't do it, or is it
> merely not implemented in QEMU, yet?
>
>> +#
>> +# Since: 6.0
>> +##
>> +{ 'struct': 'NetdevVmnetModeOptionsHostOrShared',
>> +  'data': {
>> +    '*dhcp-start-address': 'str' ,
>> +    '*dhcp-end-address':   'str',
>> +    '*dhcp-subnet-mask':   'str' },
>> +  'if': 'defined(CONFIG_VMNET)' }
>> +
>> +##
>> +# @NetdevVmnetModeOptions:
>> +#
>> +# Options specific to different operating modes of a vmnet netdev
> Suggest
>
>     # Options for a vmnet network interface backend.
>
>> +# 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_VMNET)' }
>> +
>> +##
>> +# @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)
>> +#
>> +# Since: 6.0
>> +##
>> +{ 'struct': 'NetdevVmnetOptions',
>> +  'data': {'options': 'NetdevVmnetModeOptions' },
>> +  'if': 'defined(CONFIG_VMNET)' }
> Why do you need this wrapper struct?
>
>> +
>>   ##
>>   # @NetClientDriver:
>>   #
>> @@ -460,10 +569,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_VMNET)' } ] }
>>   
>>   ##
>>   # @Netdev:
>> @@ -477,6 +589,8 @@
>>   # Since: 1.2
>>   #
>>   #        'l2tpv3' - since 2.1
>> +#
>> +#        'vmnet-macos' since 6.0 (only available on macOS)
>>   ##
>>   { 'union': 'Netdev',
>>     'base': { 'id': 'str', 'type': 'NetClientDriver' },
>> @@ -492,7 +606,9 @@
>>       'hubport':  'NetdevHubPortOptions',
>>       'netmap':   'NetdevNetmapOptions',
>>       'vhost-user': 'NetdevVhostUserOptions',
>> -    'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
>> +    'vhost-vdpa': 'NetdevVhostVDPAOptions',
>> +    'vmnet-macos': { 'type': 'NetdevVmnetOptions',
>> +                     'if': 'defined(CONFIG_VMNET)' } } }
>>   
>>   ##
>>   # @RxState:
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index f7210779409..5ff872b3e84 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2609,6 +2609,15 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>>   #ifdef __linux__
>>       "-netdev vhost-vdpa,id=str,vhostdev=/path/to/dev\n"
>>       "                configure a vhost-vdpa network,Establish a vhost-vdpa netdev\n"
>> +#endif
>> +#ifdef CONFIG_VMNET
>> +    "-netdev vmnet-macos,id=str,mode=bridged[,ifname=ifname]\n"
>> +    "         configure a macOS-provided vmnet network in \"physical interface bridge\" mode\n"
>> +    "         the physical interface to bridge with defaults to en0 if unspecified\n"
>> +    "-netdev vmnet-macos,id=str,mode=host|shared\n"
>> +    "                     [,dhcp_start_address=addr,dhcp_end_address=addr,dhcp_subnet_mask=mask]\n"
>> +    "         configure a macOS-provided vmnet network in \"host\" or \"shared\" mode\n"
>> +    "         the DHCP configuration will be set automatically if unspecified\n"
>>   #endif
>>       "-netdev hubport,id=str,hubid=n[,netdev=nd]\n"
>>       "                configure a hub port on the hub with ID 'n'\n", QEMU_ARCH_ALL)