пт, 19 нояб. 2021 г. в 17:22, Markus Armbruster <arm...@redhat.com>:
> I apologize for taking so long to respond. I spotted v5, but haven't > reviewed it. Please read my response, then assess whether you need v6. > If not, let me know, so I can have a look at v5. > > Not a problem, v5 has not so much difference to v4. I think we can discuss qapi here, after I'll submit v6 and then you can review it skipping v5. > Vladislav Yaroshchuk <yaroshchuk2...@gmail.com> writes: > > > --000000000000325dde05d084e6e4 > > Content-Type: text/plain; charset="UTF-8" > > Content-Transfer-Encoding: quoted-printable > > > > =D0=B2=D1=82, 9 =D0=BD=D0=BE=D1=8F=D0=B1. 2021 =D0=B3. =D0=B2 08:41, > Markus= > > Armbruster <arm...@redhat.com>: > > > >> Vladislav Yaroshchuk <yaroshchuk2...@gmail.com> writes: > >> > >> > Create separate netdevs for each vmnet operating mode: > >> > - vmnet-host > >> > - vmnet-shared > >> > - vmnet-bridged > >> > > >> > Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2...@gmail.com> > >> > >> [...] > >> > >> > diff --git a/qapi/net.json b/qapi/net.json > >> > index 7fab2e7cd8..087cdf0546 100644 > >> > --- a/qapi/net.json > >> > +++ b/qapi/net.json > >> > @@ -452,6 +452,112 @@ > >> > '*vhostdev': 'str', > >> > '*queues': 'int' } } > >> > > >> > +## > >> > +# @NetdevVmnetHostOptions: > >> > +# > >> > +# vmnet (host mode) network backend. > >> > +# > >> > +# Allows the vmnet interface to communicate with other vmnet > >> > +# interfaces that are in host mode and also with the native host. > >> > +# > >> > +# @dhcpstart: The starting IPv4 address to use for the interface. > >> > +# Must be in the private IP range (RFC 1918). Must be > >> > +# specified along with @dhcpend and @subnetmask. > >> > +# This address is used as the gateway address. The > >> > +# subsequent address up to and including dhcpend are > >> > +# placed in the DHCP pool. > >> > >> In QMP, we separate words by dashes, like @dhcp-start. We may prefer > >> not to here, for consistency with NetdevUserOptions, ... > >> > >> +# > >> > +# @dhcpend: The DHCP IPv4 range end address to use for the > >> > +# interface. Must be in the private IP range (RFC 1918). > >> > +# Must be specified along with @dhcpstart and @subnetmask. > >> > >> ... and here, to match @dhcpstart. > >> > >> Yes, I did not use dashed exactly because of this: to be consistent > with NetdevUserOptions. > > > >> +# > >> > +# @subnetmask: The IPv4 subnet mask to use on the interface. Must > >> > +# be specified along with @dhcpstart and @subnetmask. > >> > >> @subnet-mask, please. > >> > > > > So I think we can leave `dhcpstart`, `dhcpend`, but add a dash to > > `subnet-mask`. > > Yes, please. > >> No support for IPv6? > >> > >> > > It's supported. vmnet-shared tested with link-local addresses. > > Also some configurable options exist, but only for 'shared' mode. > > I can try to add them in the next patch series version. > > Or a later patch. > > 'vmnet-shared' has NAT66 in addition to NAT44. It has configurable prefix for internal network, so I've added a new corresponding property in v5. >> +# > >> > +# @isolated: Enable isolation for this interface. Interface isolation > >> > +# ensures that network communication between multiple > >> > +# vmnet interfaces instances is not possible. > >> > +# > >> > +# @net-uuid: The identifier (UUID) to uniquely identify the network. > >> > +# If provided, no DHCP service is provided on this network > >> > +# and the interface is added to an isolated network with > >> > +# the specified identifier. > >> > >> Out of curiosity: identify to whom? > >> > >> If I set @net-uuid, I get an isolated network with a UUID, and if I set > >> "isolated": true, I get one without a UUID. When would I want the > >> former, and when the latter? > >> > >> > > `if I set "isolated": true, I get one without a UUID.` is an > > incorrect statement. > > > > vmnet.framework isolation features are designed a bit weirdly (just my > > opinion). > > Most things happen on the macOS side, and we just can provide some > > configuration > > options to vmnet.framework as the user provides options to QEMU. > > > > Firstly, if you set @net-uuid, you're putting the interface into an > > isolated network. > > You can set the same @net-uuid on multiple vmnet-host interfaces, also > > outside > > QEMU, it can be any vmnet interface started in 'host' mode, ex. xhyve's > > netdev > > if they support this. All of them can communicate with each other, but > only > > inside > > this net identified by the @net-uuid. They can't communicate with > > interfaces from > > network with id !=3D ${our set @net-uuid}. > > > > "@isolated" is a more prohibitive mode. "Isolated" interface is not able > to > > communicate > > with anyone except the host. > > > > What happens when I set "isolated": false together with @net-uuid? > > > > > > Let's compare (vmnet-host mode): > > > > 0. When nothing is provided, DHCP is enabled and uses default > > (macOS-configured) settings. > > Your interface can communicate with any other vmnet-host interface. > > > > 1. When provided `@isolated=3Don` only, DHCP is still enabled, but the > > interface can > > communicate only with the host. All the attempts to create another > > interface with > > the same subnet will fail with `VMNET_SHARING_SERVICE_BUSY`. If we > don't > > specify DHCP settings (start, end, mask), macOS adjusts them > > automatically to > > prevent collisions and the `VMNET_SHARING_SERVICE_BUSY` error. > > > > 2. When provided @net-uuid=3Duuid[,@isolated=3Doff], the interface can > > communicate to any > > other interface inside the 'uuid' network. Also with other VMs under > > other hypervisors. > > DHCP is disabled. > > > > 3. When provided @net-uuid=3Duuid,@isolated=3Don, the interface can > communi= > > cate > > only with > > the host. DHCP is disabled. All the attempts to create another > > interface with > > the @net-uuid will fail with `VMNET_SHARING_SERVICE_BUSY`. > > The doc comments don't make this clear, I'm afraid. Care to improve > them some? > > Ok, I'll add all that stuff to documentation. > >> When "no DCHCP service is provided", then @dhcpstart and @dhcpend become > >> misnomers. Compare NetdevUserOptions: it uses @net to specify the > >> address range, and dhcpstart to specify the DHCP range (@dhcpend is > >> implied). Should we aim for consistency between the two? > >> > > > > I think it is not our choice. All of these options are just mappings to > > vmnet interface > > params: > > https://developer.apple.com/documentation/vmnet/vmnet_constants > > I think it *is* our choice. We can choose to be internally consistent, > i.e. with NetdevUserOptions, or consistent with the external interface > we expose internally, i.e. with the vmnet interface. Tradeoff. > > I think the second one is better, let's stay consistent with vmnet. > @subnet-mask -> vmnet_subnet_mask_key > > @dhcpstart -> vmnet_start_address_key > > @dhcpend -> vmnet_end_address_key > > Note that the right hand side does not say "DHCP". I think the left > hand side should not, either. Why not @start-address and @end-address? > > I agree, these names sound much more suitable. > > More detailed description is provided in the `vmnet.h` header of macOS > SDK. > > There is a restriction that says: when the `vmnet_subnet_mask_key` > > (@subnet-mask) > > is set you "must also specify vmnet_start_address_key and > > vmnet_end_address_key". > > So, when "no DHCP service is provided" the`@subnet-mask` property also > > becomes > > a misnomer. > > I don't think so. vmnet_end_address_key, vmnet_end_address_key, > vmnet_subnet_mask_key all make sense regardless of DHCP service. > > The gateway address is hardwired to the first address in the range. If > DHCP is provided, it hands out addresses from the remainder of the > range. > > For some reason I thought that these params are DHCP-specific, while they are not. My fault, sorry. Will fix this too. > > I added a corresponding restriction: you cannot provide @net-uuid within > > any of DHCP > > options, this causes hard failure with `error_setg()` (see vmnet-host.c). > > > > > >> +# > >> > +# Since: 6.2 > >> > >> Missed 6.2, please adjust. More of the same below. > >> > >> > > The next one is 6.3, isn't it? > > 7.0 > >> +## > >> > +{ 'struct': 'NetdevVmnetHostOptions', > >> > + 'data': { > >> > + '*dhcpstart': 'str', > >> > + '*dhcpend': 'str', > >> > + '*subnetmask': 'str', > >> > + '*isolated': 'bool', > >> > + '*net-uuid': 'str' > >> > + }, > >> > + 'if': 'CONFIG_VMNET' } > >> > + > >> > +## > >> > +# @NetdevVmnetSharedOptions: > >> > +# > >> > +# vmnet (shared mode) network backend. > >> > +# > >> > +# Allows traffic originating from the vmnet interface to reach the > >> > +# Internet through a network address translator (NAT). The vmnet > >> > +# interface can also communicate with the native host. By default, > >> > +# the vmnet interface is able to communicate with other shared mode > >> > +# interfaces. If a subnet range is specified, the vmnet interface can > >> > >> Do you mean "subnet mask"? > >> > >> > > This phrase was just copied from Apple's `vmnet.h` from SDK: > > ``` > > * [...] VMNET_SHARED_MODE > > * Allows traffic originating from the vmnet interface to reach the > > * Internet through a network address translator (NAT). The vmnet > interface > > * can also communicate with the native host. By default, the vmnet > > interface > > * is able to communicate with other shared mode interfaces. If a subnet > > range > > * is specified, the vmnet interface can communicate with other shared > mode > > * interfaces on the same subnet. > > ``` > > But `subnet mask` sounds more suitable, so it's better to say: > > "... If a subnet mask and DHCP range is specified, the vmnet interface > can > > ..." > > > >> +# communicate with other shared mode interfaces on the same subnet. > >> > +# > >> > +# @dhcpstart: The starting IPv4 address to use for the interface. > >> > +# Must be in the private IP range (RFC 1918). Must be > >> > +# specified along with @dhcpend and @subnetmask. > >> > +# This address is used as the gateway address. The > >> > +# subsequent address up to and including dhcpend are > >> > +# placed in the DHCP pool. > >> > +# > >> > +# @dhcpend: The DHCP IPv4 range end address to use for the > >> > +# interface. Must be in the private IP range (RFC 1918). > >> > +# Must be specified along with @dhcpstart and @subnetmask. > >> > +# > >> > +# @subnetmask: The IPv4 subnet mask to use on the interface. Must > >> > +# be specified along with @dhcpstart and @subnetmask. > >> > +# > >> > +# @isolated: Enable isolation for this interface. Interface isolation > >> > +# ensures that network communication between multiple > >> > +# vmnet interfaces instances is not possible. > >> > +# > >> > +# Since: 6.2 > >> > +## > >> > +{ 'struct': 'NetdevVmnetSharedOptions', > >> > + 'data': { > >> > + '*dhcpstart': 'str', > >> > + '*dhcpend': 'str', > >> > + '*subnetmask': 'str', > >> > + '*isolated': 'bool' > >> > + }, > >> > + 'if': 'CONFIG_VMNET' } > >> > >> [...] > > > > As a conclusion, my TODOs for now: > > * fix `subnet range` in documentation > > * add dash to @subnet-mask > > * change version: 6.2 -> 6.3 > > * provide IPv6 configuration properties for vmnet-shared (optional). > > Already done in v5: * @subnetmask -> @subnet-mask * nat66 prefix property added * updated version 6.2 -> 7.0 Now in my TODO: * improve documentation, especially for isolation properties. * change @dhcpstart, @dhcpend to @start-address, @end-address. Please confirm, and I'll submit v6 with all these fixes. Thank you for the review! -- Best Regards, Vladislav Yaroshchuk