пт, 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

Reply via email to