[Dnsmasq-discuss] [PATCH] Change dhcp_release to use first address when no IP subnet matches

Simon Kelley simon at thekelleys.org.uk
Thu Aug 22 23:50:21 BST 2019


On 26/04/2019 21:03, Brian Haley wrote:
> Currently, dhcp_release will only send a 'fake' release
> when the address given is in the same subnet as an IP
> on the interface that was given.
> 
> This doesn't work in an environment where dnsmasq is
> managing leases for remote subnets via a DHCP relay, as
> running dhcp_release locally will just cause it to
> silently exit without doing anything, leaving the lease
> n the database.
> 
> Change it to save the first IP it finds on the interface,
> and if no matching subnet IP is found, use it as a fall-back.
> This fixes an issue we are seeing in certain Openstack
> deployments where we are using dnsmasq to provision baremetal
> systems in a datacenter.
> 
> While using Dbus might have seemed like an obvious solution,
> because of our extensive use of network namespaces (which
> Dbus doesn't support), this seemed like a better solution
> than creating system.d policy files for each dnsmasq we
> might spawn and using --enable-dbus=$id in order to isolate
> messages to specific dnsmasq instances.


The address we're determining here is the address used for the "server
identifier" for the DHCP lease. In the case of a client on the other
side from a DHCP relay, dnsmasq uses one of the addresses of the
interface through which the packet arrived. This patch also uses one of
the addresses, but it doesn't choose it in the same way, so for an
interface with multiple addresses, dhcp_release may pick a different
address than the one dnsmasq did, the server_id in the release message
will not match, and the operation will silently fail again.

If the interface has only one address, a common case, then that has to
be chosen, and all works. For more robustness, it makes sense for this
patch to pick one of the addresses in the same way that the dnsmasq code
does, namely, by doing

ioctl(..., SIOCGIFADDR, ...)

in the case that none of the addresses match the address of the lease.


A patch to do that instead of using the first one returned by netlink
would be great.



Cheers,

Simon.



> 
> Signed-off-by: Brian Haley <haleyb.dev at gmail.com>
> ---
>  contrib/lease-tools/dhcp_release.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/lease-tools/dhcp_release.c b/contrib/lease-tools/dhcp_release.c
> index 59883d4..63a732f 100644
> --- a/contrib/lease-tools/dhcp_release.c
> +++ b/contrib/lease-tools/dhcp_release.c
> @@ -180,6 +180,7 @@ static int is_same_net(struct in_addr a, struct in_addr b, struct in_addr mask)
>  
>  static struct in_addr find_interface(struct in_addr client, int fd, unsigned int index)
>  {
> +  struct in_addr first_addr = {};
>    struct sockaddr_nl addr;
>    struct nlmsghdr *h;
>    ssize_t len;
> @@ -218,7 +219,11 @@ static struct in_addr find_interface(struct in_addr client, int fd, unsigned int
>  
>        for (h = (struct nlmsghdr *)iov.iov_base; NLMSG_OK(h, (size_t)len); h = NLMSG_NEXT(h, len))
>  	if (h->nlmsg_type == NLMSG_DONE)
> -	  exit(0);
> +          {
> +	    if (first_addr.s_addr)
> +	      return first_addr;
> +	    exit(0);
> +          }
>  	else if (h->nlmsg_type == RTM_NEWADDR)
>            {
>              struct ifaddrmsg *ifa = NLMSG_DATA(h);  
> @@ -234,7 +239,11 @@ static struct in_addr find_interface(struct in_addr client, int fd, unsigned int
>                  
>                  for (rta = IFA_RTA(ifa); RTA_OK(rta, len1); rta = RTA_NEXT(rta, len1))
>  		  if (rta->rta_type == IFA_LOCAL)
> -		    addr = *((struct in_addr *)(rta+1));
> +		    {
> +		      addr = *((struct in_addr *)(rta+1));
> +		      if (!first_addr.s_addr)
> +			first_addr = addr;
> +		    }
>  		
>                  if (addr.s_addr && is_same_net(addr, client, netmask))
>  		  return addr;
> 




More information about the Dnsmasq-discuss mailing list