Re: [PATCH 6/7] network: turn on autoaddr in default network

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 07, 2024 at 02:15:16PM -0400, Laine Stump wrote:
> On 8/7/24 1:45 PM, Daniel P. Berrangé wrote:
> > On Wed, Aug 07, 2024 at 01:16:02PM -0400, Laine Stump wrote:
> > > With autoaddr enabled, the subnet to be used for the default network
> > > will be verified/changed at the time the network starts.
> > > 
> > > Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
> > > ---
> > >   src/network/default.xml.in | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/src/network/default.xml.in b/src/network/default.xml.in
> > > index 08a3632eb6..a01c6d30ae 100644
> > > --- a/src/network/default.xml.in
> > > +++ b/src/network/default.xml.in
> > > @@ -2,7 +2,7 @@
> > >     <name>default</name>
> > >     <bridge name='virbr0'/>
> > >     <forward/>
> > > -  <ip address='192.168.122.1' netmask='255.255.255.0'>
> > > +  <ip autoaddr='yes' address='192.168.122.1' netmask='255.255.255.0'>
> > >       <dhcp>
> > >         <range start='192.168.122.2' end='192.168.122.254'/>
> > >       </dhcp>
> > 
> > What I find unsettling is that we're providing an address + netmask
> > here, along with a DHCP range, but there's no guarantee any of these
> > are within the start+end addresses in network.conf
> 
> The code removes the network part of any existing dhcp range, static host,
> or bootpserver, and replaces that with the network part of the newly chosen
> network, which puts them into the same subnet, so actually it is guaranteed.
> 
> While I agree that it might be unusual for someone to have static host
> addresses configured in a network where they wanted to use autoaddr, it's
> not difficult to support, and makes the handling consistent with the way
> that the network's IP address, and also the DHCP range and bootp server
> addresses are handled - basically every IP address associated with the
> network is moved to the new subnet.

Hmmm, it would be wierd for users to have the DHCP range / static
hosts under a different network from the primary host IP, but users
are known todo wierd things.

So if someonme has a config like:

  <ip autoaddr='yes' address='192.168.122.1' netmask='255.255.255.0'>
      <dhcp>
         <range start='192.168.42.2' end='192.168.42.254'/>
      </dhcp>

And we detect a clash for 192.168.122.0/24, IIUC, you're saying we'll
cyhang the DHCP range to 192.168.123.2->192.168.123.254, even though
the DHCP range was on a different subnet originally. That'd be quite
susprising to me.

I think we should enforce that if you have autoaddr=yes, that all
DHCP/static host IPs are on the same subnet as the primary network
IP.

> 
> > I'm thinking that perhaps autoaddr='yes' should be mutually exclusive
> > with existence of an explicit address + DHCP range. ie only permit
> > 
> >    <ip autoaddr='yes'>
> >        <dhcp/>
> >    </ip>
> > 
> > on the basis that if someone wants explicit control over the DHCP
> > range, then they probably shouldn't be relying on auto-addr usage.
> 
> That simplifies it, but would require removing the code that saves the
> current chosen subnet (so that it can be tried first when the network is
> next started), making it more likely that addresses would change each time
> the network is started. I see you've brought up exactly that topic on your
> response to Patch 7/7 :-)

Yeah, removing the addrs doesn't actually simplify, since we need to
then store the exact same info somewhere else. So what you've done is
better, if we can define sane semantics when the DHCP/static hosts are
on different subnets by forbidding that config.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux