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@redhat.com>
---
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>
--
2.45.2On 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@redhat.com>
> ---
> 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
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.
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 :|
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@redhat.com> >> --- >> 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. > 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 :-)
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@redhat.com>
> > > ---
> > > 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 :|
On 8/16/24 11:30 AM, Daniel P. Berrangé wrote: > 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@redhat.com> >>>> --- >>>> 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> > I'm pretty sure dnsmasq doesn't allow that configuration. And even if it did, libvirt itself would give this error and refuse the config: error: internal error: range 192.168.42.2 - 192.168.42.254 is not entirely within network 192.168.122.1 > 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. Fortunately we already enforce that for the DHCP range. But not for static host entries (I tried that and it was accepted by both libvirt and dnsmasq). I'm guessing it was an oversight that this check wasn't done for static host entries, although I can't think of a reasonable use case for it (and maybe there *is* one :-)). And thinking about the tftpserver, I suppose it *is* valid to have a tftpserver on a different subnet, so maybe we should leave that untouched. Or wait! What if we update static host / tftpserver IPs that were on the original (cached/suggested) subnet, but if they already don't match anyway, then we leave them alone? This way someone who had setup, e.g. a guest attached to the network in question as a tftpserver for other guests on the network could do that by a) adding a <host> entry for the tftpserver's interface MAC address that was on the same subnet as the "suggested" <ip> of the network, b) adding a tftpserver with that same IP to the network's config, and c) configuring said tftpserver guest to get its IP by DHCP. If libvirt had to find a different subnet, the tftpserver would get an address on the new subnet, and the other guests would be told to look to that new address for a tftpserver. But on the other hand, if they had a physcial tftpserver somewhere else on the network (by definition another subnet), they could just enter in that tftpserver address (on a different subnet), libvirt wouldn't touch that IP if it had to change the network's subnet, and the other guests would get an IP (and default route) on whatever subnet libvirt found, with the tftpserver IP sent in the DHCP response always the same. I think doing it like this would avoid surprises, while allowing for as much functionality as possible. > >> >>> 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.
© 2016 - 2024 Red Hat, Inc.