[libvirt] [PATCH] network: don't use dhcp-authoritative on static networks

[Laine Stump]

On 12/16/2016 11:58 AM, Martin Wilck wrote:
> "Static" DHCP networks are those where no dynamic DHCP range is
> defined, only a list of host entries is used to serve permanent
> IP addresses. On such networks, we don't want dnsmasq to reply
> to other requests than those statically defined. But
> "dhcp-authoritative" will cause dnsmasq to do just that.
> Therefore we can't use "dhcp-authoritative" for static networks.

Not surprising that this simple change would have unexpected 
consequences - that seems to be a basic law of the universe :-)

ACK to this, but it has me wondering 1) what is the range for which it 
returns a positive response? Is it for anything within the IP 
address/netmask of the interface it's listening on? Or something larger 
than that? (Does it just blindly ACK any request it gets?) and 2) Do we 
know for certain that the same thing doesn't happen when there is also a 
dhcp range defined?

I'll wait for an answer to these before I push.

>
> Fixes: 4ac20b3ae "network: add dnsmasq option 'dhcp-authoritative'"
> Signed-off-by: Martin Wilck <mwilck at suse.com>
> ---
>   src/network/bridge_driver.c                             | 9 ++++++++-
>   tests/networkxml2confdata/dhcp6host-routed-network.conf | 1 -
>   2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index ae1589d8c..17c6f3a0f 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -1355,7 +1355,14 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
>           if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) {
>               if (ipdef->nranges || ipdef->nhosts) {
>                   virBufferAddLit(&configbuf, "dhcp-no-override\n");
> -                virBufferAddLit(&configbuf, "dhcp-authoritative\n");
> +		/*
> +                 * Use "dhcp-authoritative" only for dynamic DHCP.
> +                 * In a static-only network, it would cause dnsmasq
> +                 * to reply to requests from other hosts than those
> +                 * statically defined.
> +                 */
> +		if (ipdef->nranges || !ipdef->nhosts)
> +                    virBufferAddLit(&configbuf, "dhcp-authoritative\n");
>               }
>   
>               if (ipdef->tftproot) {
> diff --git a/tests/networkxml2confdata/dhcp6host-routed-network.conf b/tests/networkxml2confdata/dhcp6host-routed-network.conf
> index 87a149880..5728ee430 100644
> --- a/tests/networkxml2confdata/dhcp6host-routed-network.conf
> +++ b/tests/networkxml2confdata/dhcp6host-routed-network.conf
> @@ -10,7 +10,6 @@ bind-dynamic
>   interface=virbr1
>   dhcp-range=192.168.122.1,static
>   dhcp-no-override
> -dhcp-authoritative
>   dhcp-range=2001:db8:ac10:fd01::1,static,64
>   dhcp-hostsfile=/var/lib/libvirt/dnsmasq/local.hostsfile
>   addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts