Hello Simon and others.

In certain situations, dnsmasq DHCP will offer multiple different
clients single IP address. Later it ACKs the first client and NACK the
second. It relies on ability of those clients to retry, but it seems
netbooting software often cannot recover from such behaviour.

Attaching script I use to reproduce this issue. Just create some local
bridge, have a limited IP pool on its dhcp-range option. Just few
addresses above actual number instances. They start roughtly at the same
time.

There is also pcap file in linked bug with some other reports. Good
summary is in commetn 85 [3].

In attached patches, I introduced thing I call temporary leases. Those
leases are never saved into leased file. They have short time duration,
set to 30 s same as ping timeout. It ensures even with
dhcp-sequential-ip, different clients have reservations for different
addresses. It helps especially in case --no-ping is used. Without this
change it takes quite long to retry multiple times
discover-offer-request solutions. Because pings contain sort of
workaround for this deficiency, but will cover only 6 different
addresses in default configuration. Then it switches to overload,
similar to no-ping. Then it offers multiple clients the same address,
but when the 2nd client requests the lease, it denies it again.

I think I were able to find relative simple algorithm. I think IPv6
should receive similar approach. We side-stepped this by offering
different address in ACK in thread [4]. While it seems that works, I
think it would be better to not offer address it later rejects itself.
We test DHCP clients abilities for no good reason.

With this patches, even multiple clients without ping boot fast enough,
even when they start at similar time. Starting at similar time if common
thing on boot of cloud hosting instances, which may use dnsmasq for
local caching. OpenStack is example that recorded it, but it can happen
even in normal machines. For example in a classroom with 10 computers.

Would you look at it or test it, whether some issues with those changes
can be found?

Cheers,
Petr

3. https://bugzilla.redhat.com/show_bug.cgi?id=2028704#c85
4.
https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q3/015585.html

On 12/8/21 01:18, Petr Menšík wrote:
>
> Hi Simon and others,
>
> I am debugging strange issue, which happens inside OpenStack in
> certain situations. It seems under not precisely defined conditions
> dnsmasq returns "no address available" error even in situation, when
> not yet all leases are used.
>
> It seems do_icmp_ping is responsible for ruling out recently tried IP
> addresses. It seems a bit weird address allocation happens only for
> addresses recently not pinged. I have found another place which does
> do_icmp_ping, but does not use hash value computed from hardware
> address. Even when it is already known at that time. First patch
> attached adds hash also to second place. That should mean single
> address would use shared ping. The second patch simplifies a bit
> do_icmp_patch and its return value. Instead of artificially ensuring
> hash would match, just return correct value when hash matches. The
> second change is just optional optimization.
>
> Few details are at RH bug #2028704 [1]. Original tested version 2.79
> did not contain commit 0669ee7a69a
> <http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=0669ee7a69a004ce34fed41e50aa575f8e04427b>
> [2], which improves the situation. But I think there remain cases when
> ping is not accepted when it should be. Testing with latest release
> did not work according to report. I think the first patch may fix
> still missing part.
>
> Cheers,
> Petr
>
> 1. https://bugzilla.redhat.com/show_bug.cgi?id=2028704
> 2.
> http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=0669ee7a69a004ce34fed41e50aa575f8e04427b
>
> -- 
> Petr Menšík
> Software Engineer
> Red Hat, http://www.redhat.com/
> email: pemen...@redhat.com
> PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB

-- 
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB

Attachment: setup.sh
Description: application/shellscript

From ca3ae1a0a3a479ea42b0ff3183d271f7ee89a7c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Mon, 13 Dec 2021 17:45:29 +0100
Subject: [PATCH 6/6] Handle case when temporary lease is from different subnet

Force single temporary lease if IP pool is almost exahusted and
there remains only temporary leases from different subnet. Track
temporary count because it iterates lists multiple times and could be
processor intensive. Avoid iterating in void if none temporary leases
can be freed.
---
 src/dnsmasq.h |  2 ++
 src/lease.c   | 34 ++++++++++++++++++++++++++++++----
 src/rfc2131.c | 21 +++++++++++++++++----
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index d15e97b..8516f78 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1523,6 +1523,8 @@ struct dhcp_lease *lease_find_by_client(unsigned char *hwaddr, int hw_len, int h
 struct dhcp_lease *lease_find_by_addr(struct in_addr addr);
 struct in_addr lease_find_max_addr(struct dhcp_context *context);
 void lease_prune(struct dhcp_lease *target, time_t now);
+void lease_free_any_temporary(time_t now);
+int lease_have_temporary(void);
 void lease_update_from_configs(void);
 int do_script_run(time_t now);
 void rerun_scripts(void);
diff --git a/src/lease.c b/src/lease.c
index d9de0b9..2e50461 100644
--- a/src/lease.c
+++ b/src/lease.c
@@ -19,7 +19,7 @@
 #ifdef HAVE_DHCP
 
 static struct dhcp_lease *leases = NULL, *old_leases = NULL;
-static int dns_dirty, file_dirty, leases_left;
+static int dns_dirty, file_dirty, leases_left, lease_temporaries;
 
 static int read_leases(time_t now, FILE *leasestream)
 {
@@ -552,6 +552,8 @@ void lease_prune(struct dhcp_lease *target, time_t now)
 	{
 	  if ((lease->flags & LEASE_TEMP) == 0)
 	    file_dirty = 1;
+	  else
+	    lease_temporaries--;
 	  if (lease->hostname)
 	    dns_dirty = 1;
 
@@ -570,8 +572,26 @@ void lease_prune(struct dhcp_lease *target, time_t now)
 	up = &lease->next;
     }
 } 
-	
-  
+
+/* Find and release single temporary address. */
+void lease_free_any_temporary(time_t now)
+{
+  struct dhcp_lease *lease;
+
+  for (lease = leases; lease; lease = lease->next)
+    if (lease->flags & LEASE_TEMP)
+      {
+	lease_prune(lease, now);
+	break;
+      }
+}
+
+int lease_have_temporary(void)
+{
+  return (lease_temporaries > 0);
+}
+
+
 struct dhcp_lease *lease_find_by_client(unsigned char *hwaddr, int hw_len, int hw_type,
 					unsigned char *clid, int clid_len)
 {
@@ -623,6 +643,7 @@ struct dhcp_lease *lease_find_by_addr(struct in_addr addr)
   return NULL;
 }
 
+
 #ifdef HAVE_DHCP6
 /* find address for {CLID, IAID, address} */
 struct dhcp_lease *lease6_find(unsigned char *clid, int clid_len, 
@@ -774,6 +795,7 @@ static struct dhcp_lease *lease_allocate(int temp)
   else
     {
       lease->flags |= LEASE_TEMP;
+      lease_temporaries++;
     }
 
   return lease;
@@ -926,7 +948,11 @@ void lease_set_hwaddr(struct dhcp_lease *lease, const unsigned char *hwaddr,
 void lease_set_permanent(struct dhcp_lease *lease)
 {
   if (lease->flags & LEASE_TEMP)
-    set_modified(lease, LEASE_CHANGED);
+    {
+      lease->flags &= ~LEASE_TEMP;
+      set_modified(lease, LEASE_CHANGED);
+      lease_temporaries--;
+    }
 }
 
 static void kill_name(struct dhcp_lease *lease)
diff --git a/src/rfc2131.c b/src/rfc2131.c
index be9064a..0ae6a54 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -72,12 +72,25 @@ static struct dhcp_lease *temp_lease4_allocate(struct dhcp_context *context,
 		     struct dhcp_packet *mess, unsigned char *hwaddr, int hw_len,
 		     struct dhcp_netid *netids, time_t now, int loopback)
 {
-  struct dhcp_lease *lease;
+  struct dhcp_lease *lease = NULL;
   unsigned int hash = ping_hash(hwaddr, hw_len);
   if (!address_allocate(context, &mess->yiaddr, hash, netids, now, loopback))
-    /* When no unused address is available, reuse first temporary lease
-     * with matching address. */
-    lease = find_temp_lease(context, hash, netids);
+    {
+      if (lease_have_temporary())
+	{
+	  /* When no unused address is available, reuse first temporary lease
+	  * with matching address. */
+	  lease = find_temp_lease(context, hash, netids);
+	  if (!lease)
+	    {
+	      /* Not found temp lease in requested range.
+	       * Force free any temporary lease and retry. */
+	      lease_free_any_temporary(now);
+	      if (address_allocate(context, &mess->yiaddr, hash, netids, now, loopback))
+		lease = lease4_allocate(mess->yiaddr, 1);
+	    }
+	}
+    }
   else
     lease = lease4_allocate(mess->yiaddr, 1);
   if (lease)
-- 
2.31.1

From df3adfcd01eb26c069bbaa552ecc842e3eafc53e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Mon, 13 Dec 2021 15:12:04 +0100
Subject: [PATCH 5/6] Ensure temporary leases are ignored on save

Mark lease as permanent after ack. Ensure it is flagged as modified if
previous lease were temporary. Ensures lease script woud be run always
on ack only, temporary leases are not propagated to script or dbus.

Reuse repeated parts of lease write code, move common parts to
functions.
---
 src/dnsmasq.h |   1 +
 src/lease.c   | 150 ++++++++++++++++++++++++--------------------------
 src/rfc2131.c |   2 +-
 3 files changed, 75 insertions(+), 78 deletions(-)

diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 3c1f419..d15e97b 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1517,6 +1517,7 @@ void lease_set_hwaddr(struct dhcp_lease *lease, const unsigned char *hwaddr,
 void lease_set_hostname(struct dhcp_lease *lease, const char *name, int auth, char *domain, char *config_domain);
 void lease_set_expires(struct dhcp_lease *lease, unsigned int len, time_t now);
 void lease_set_interface(struct dhcp_lease *lease, int interface, time_t now);
+void lease_set_permanent(struct dhcp_lease *lease);
 struct dhcp_lease *lease_find_by_client(unsigned char *hwaddr, int hw_len, int hw_type,  
 					unsigned char *clid, int clid_len);
 struct dhcp_lease *lease_find_by_addr(struct in_addr addr);
diff --git a/src/lease.c b/src/lease.c
index 0b8f094..d9de0b9 100644
--- a/src/lease.c
+++ b/src/lease.c
@@ -248,11 +248,43 @@ static void ourprintf(int *errp, char *format, ...)
   va_end(ap);
 }
 
+static void lease_print_id(unsigned char *id, int id_len, const char *delim, int *errp)
+{
+  int i;
+
+  for (i = 0; i < id_len - 1; i++)
+    ourprintf(errp, "%.2x:", id[i]);
+  ourprintf(errp, "%.2x%s", id[i], delim);
+}
+
+static void lease_dump_begin(struct dhcp_lease *lease, int *errp)
+{
+#ifdef HAVE_BROKEN_RTC
+  ourprintf(errp, "%u ", lease->length);
+#else
+  ourprintf(errp, "%lu ", (unsigned long)lease->expires);
+#endif
+
+  if (lease->hwaddr_type != ARPHRD_ETHER || lease->hwaddr_len == 0)
+    ourprintf(errp, "%.2x-", lease->hwaddr_type);
+  lease_print_id(lease->hwaddr, lease->hwaddr_len, "", errp);
+}
+
+static void lease_dump_end(struct dhcp_lease *lease, int *errp)
+{
+  ourprintf(errp, "%s ", lease->hostname ? lease->hostname : "*");
+
+  if (lease->clid && lease->clid_len != 0)
+    lease_print_id(lease->clid, lease->clid_len, "\n", errp);
+  else
+    ourprintf(errp, "*\n");
+}
+
 void lease_update_file(time_t now)
 {
   struct dhcp_lease *lease;
   time_t next_event;
-  int i, err = 0;
+  int err = 0;
 
   if (file_dirty != 0 && daemon->lease_stream)
     {
@@ -263,79 +295,35 @@ void lease_update_file(time_t now)
       
       for (lease = leases; lease; lease = lease->next)
 	{
-
-	  if (lease->flags & LEASE_TEMP)
-	    continue;
 #ifdef HAVE_DHCP6
 	  if (lease->flags & (LEASE_TA | LEASE_NA))
 	    continue;
 #endif
 
-#ifdef HAVE_BROKEN_RTC
-	  ourprintf(&err, "%u ", lease->length);
-#else
-	  ourprintf(&err, "%lu ", (unsigned long)lease->expires);
-#endif
-
-	  if (lease->hwaddr_type != ARPHRD_ETHER || lease->hwaddr_len == 0) 
-	    ourprintf(&err, "%.2x-", lease->hwaddr_type);
-	  for (i = 0; i < lease->hwaddr_len; i++)
+	  if (!(lease->flags & LEASE_TEMP))
 	    {
-	      ourprintf(&err, "%.2x", lease->hwaddr[i]);
-	      if (i != lease->hwaddr_len - 1)
-		ourprintf(&err, ":");
+	      lease_dump_begin(lease, &err);
+	      inet_ntop(AF_INET, &lease->addr, daemon->addrbuff, ADDRSTRLEN);
+	      ourprintf(&err, " %s ", daemon->addrbuff);
+	      lease_dump_end(lease, &err);
 	    }
-	  
-	  inet_ntop(AF_INET, &lease->addr, daemon->addrbuff, ADDRSTRLEN); 
-
-	  ourprintf(&err, " %s ", daemon->addrbuff);
-	  ourprintf(&err, "%s ", lease->hostname ? lease->hostname : "*");
-	  	  
-	  if (lease->clid && lease->clid_len != 0)
-	    {
-	      for (i = 0; i < lease->clid_len - 1; i++)
-		ourprintf(&err, "%.2x:", lease->clid[i]);
-	      ourprintf(&err, "%.2x\n", lease->clid[i]);
-	    }
-	  else
-	    ourprintf(&err, "*\n");	  
 	}
       
 #ifdef HAVE_DHCP6  
       if (daemon->duid)
 	{
 	  ourprintf(&err, "duid ");
-	  for (i = 0; i < daemon->duid_len - 1; i++)
-	    ourprintf(&err, "%.2x:", daemon->duid[i]);
-	  ourprintf(&err, "%.2x\n", daemon->duid[i]);
-	  
-	  for (lease = leases; lease; lease = lease->next)
-	    {
-	      
-	      if (!(lease->flags & (LEASE_TA | LEASE_NA)))
-		continue;
+	  lease_print_id(daemon->duid, daemon->duid_len, "\n", &err);
 
-#ifdef HAVE_BROKEN_RTC
-	      ourprintf(&err, "%u ", lease->length);
-#else
-	      ourprintf(&err, "%lu ", (unsigned long)lease->expires);
-#endif
-    
-	      inet_ntop(AF_INET6, &lease->addr6, daemon->addrbuff, ADDRSTRLEN);
-	 
-	      ourprintf(&err, "%s%u %s ", (lease->flags & LEASE_TA) ? "T" : "",
-			lease->iaid, daemon->addrbuff);
-	      ourprintf(&err, "%s ", lease->hostname ? lease->hostname : "*");
-	      
-	      if (lease->clid && lease->clid_len != 0)
-		{
-		  for (i = 0; i < lease->clid_len - 1; i++)
-		    ourprintf(&err, "%.2x:", lease->clid[i]);
-		  ourprintf(&err, "%.2x\n", lease->clid[i]);
-		}
-	      else
-		ourprintf(&err, "*\n");	  
-	    }
+	  for (lease = leases; lease; lease = lease->next)
+	    if (!(lease->flags & LEASE_TEMP) && lease->flags & (LEASE_TA | LEASE_NA))
+	      {
+		lease_dump_begin(lease, &err);
+		inet_ntop(AF_INET6, &lease->addr6, daemon->addrbuff, ADDRSTRLEN);
+		ourprintf(&err, "%s%u %s ", (lease->flags & LEASE_TA) ? "T" : "",
+			  lease->iaid, daemon->addrbuff);
+		lease_dump_end(lease, &err);
+	      }
 	}
 #endif      
 	  
@@ -562,7 +550,8 @@ void lease_prune(struct dhcp_lease *target, time_t now)
       tmp = lease->next;
       if ((lease->expires != 0 && difftime(now, lease->expires) >= 0) || lease == target)
 	{
-	  file_dirty = 1;
+	  if ((lease->flags & LEASE_TEMP) == 0)
+	    file_dirty = 1;
 	  if (lease->hostname)
 	    dns_dirty = 1;
 
@@ -820,6 +809,13 @@ struct dhcp_lease *lease6_allocate(struct in6_addr *addrp, int lease_type)
 }
 #endif
 
+static void set_modified(struct dhcp_lease *lease, int flag)
+{
+  if ((lease->flags & LEASE_TEMP) == 0)
+    file_dirty = 1; /* run script on change */
+  lease->flags |= flag;
+}
+
 void lease_set_expires(struct dhcp_lease *lease, unsigned int len, time_t now)
 {
   time_t exp;
@@ -844,8 +840,7 @@ void lease_set_expires(struct dhcp_lease *lease, unsigned int len, time_t now)
       dns_dirty = 1;
       lease->expires = exp;
 #ifndef HAVE_BROKEN_RTC
-      lease->flags |= LEASE_AUX_CHANGED | LEASE_EXP_CHANGED;
-      file_dirty = 1;
+      set_modified(lease, LEASE_AUX_CHANGED | LEASE_EXP_CHANGED);
 #endif
     }
   
@@ -853,8 +848,7 @@ void lease_set_expires(struct dhcp_lease *lease, unsigned int len, time_t now)
   if (len != lease->length)
     {
       lease->length = len;
-      lease->flags |= LEASE_AUX_CHANGED;
-      file_dirty = 1; 
+      set_modified(lease, LEASE_AUX_CHANGED);
     }
 #endif
 } 
@@ -890,9 +884,7 @@ void lease_set_hwaddr(struct dhcp_lease *lease, const unsigned char *hwaddr,
 	memcpy(lease->hwaddr, hwaddr, hw_len);
       lease->hwaddr_len = hw_len;
       lease->hwaddr_type = hw_type;
-      lease->flags |= LEASE_CHANGED;
-      if ((lease->flags & LEASE_TEMP) != 0)
-	file_dirty = 1; /* run script on change */
+      set_modified(lease, LEASE_CHANGED);
     }
 
   /* only update clid when one is available, stops packets
@@ -905,8 +897,7 @@ void lease_set_hwaddr(struct dhcp_lease *lease, const unsigned char *hwaddr,
 
       if (lease->clid_len != clid_len)
 	{
-	  lease->flags |= LEASE_AUX_CHANGED;
-	  file_dirty = 1;
+	  set_modified(lease, LEASE_AUX_CHANGED);
 	  free(lease->clid);
 	  if (!(lease->clid = whine_malloc(clid_len)))
 	    return;
@@ -916,8 +907,7 @@ void lease_set_hwaddr(struct dhcp_lease *lease, const unsigned char *hwaddr,
 	}
       else if (memcmp(lease->clid, clid, clid_len) != 0)
 	{
-	  lease->flags |= LEASE_AUX_CHANGED;
-	  file_dirty = 1;
+	  set_modified(lease, LEASE_AUX_CHANGED);
 #ifdef HAVE_DHCP6
 	  change = 1;
 #endif	
@@ -933,6 +923,12 @@ void lease_set_hwaddr(struct dhcp_lease *lease, const unsigned char *hwaddr,
 #endif
 }
 
+void lease_set_permanent(struct dhcp_lease *lease)
+{
+  if (lease->flags & LEASE_TEMP)
+    set_modified(lease, LEASE_CHANGED);
+}
+
 static void kill_name(struct dhcp_lease *lease)
 {
   /* run script to say we lost our old name */
@@ -1044,9 +1040,8 @@ void lease_set_hostname(struct dhcp_lease *lease, const char *name, int auth, ch
   if (auth)
     lease->flags |= LEASE_AUTH_NAME;
   
-  file_dirty = 1;
   dns_dirty = 1; 
-  lease->flags |= LEASE_CHANGED; /* run script on change */
+  set_modified(lease, LEASE_CHANGED);
 }
 
 void lease_set_interface(struct dhcp_lease *lease, int interface, time_t now)
@@ -1145,9 +1140,10 @@ int do_script_run(time_t now)
       }
   
   for (lease = leases; lease; lease = lease->next)
-    if ((lease->flags & (LEASE_NEW | LEASE_CHANGED)) || 
-	((lease->flags & LEASE_AUX_CHANGED) && option_bool(OPT_LEASE_RO)) ||
-	((lease->flags & LEASE_EXP_CHANGED) && option_bool(OPT_LEASE_RENEW)))
+    if ((lease->flags & (LEASE_TEMP)) == 0 &&
+	((lease->flags & (LEASE_NEW | LEASE_CHANGED)) ||
+	 ((lease->flags & LEASE_AUX_CHANGED) && option_bool(OPT_LEASE_RO)) ||
+	 ((lease->flags & LEASE_EXP_CHANGED) && option_bool(OPT_LEASE_RENEW))))
       {
 #ifdef HAVE_SCRIPT
 	queue_script((lease->flags & LEASE_NEW) ? ACTION_ADD : ACTION_OLD, lease, 
diff --git a/src/rfc2131.c b/src/rfc2131.c
index 3c2ad29..be9064a 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -1493,7 +1493,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 	    }
 	  
 	  time = calc_time(context, config, option_find(mess, sz, OPTION_LEASE_TIME, 4));
-	  lease->flags &= ~LEASE_TEMP;
+	  lease_set_permanent(lease);
 	  lease_set_hwaddr(lease, mess->chaddr, clid, mess->hlen, mess->htype, clid_len, now, do_classes);
 	  
 	  /* if all the netids in the ignore_name list are present, ignore client-supplied name */
-- 
2.31.1

From 8df6409f07d0a92581afea801c5e1c6f92a37a0d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Sat, 11 Dec 2021 05:52:49 +0100
Subject: [PATCH 4/6] Reuse temporary leases if no free address remains

When more clients sends DISCOVER message than we have free addresses, we
need to offer addresses already offered to someone else. It allows
offering at least something when address pool is almost empty. Someone
will probably do not receive address in the end, but that is best
attempt possible.
---
 src/dhcp.c    | 195 +++++++++++++++++++++++++++++++++-----------------
 src/dnsmasq.h |   4 +-
 src/rfc2131.c |  14 ++--
 3 files changed, 140 insertions(+), 73 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index 24d50f1..1ae7b8f 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -772,96 +772,157 @@ int do_icmp_ping(time_t now, struct in_addr addr, unsigned int hash, int loopbac
   return 0; /* address in use or no memory. */
 }
 
+static struct in_addr dhcp_context_start_addr(struct dhcp_context *c, unsigned int hash)
+{
+  struct in_addr start;
+  if (option_bool(OPT_CONSEC_ADDR))
+    /* seed is largest extant lease addr in this context */
+    start = lease_find_max_addr(c);
+  else
+    /* pick a seed based on hwaddr */
+    start.s_addr = htonl(ntohl(c->start.s_addr) +
+			  ((hash + c->addr_epoch) % (1 + ntohl(c->end.s_addr) - ntohl(c->start.s_addr))));
+  return start;
+}
+
+static int dhcp_context_my_addr(const struct dhcp_context *context, struct in_addr addr)
+{
+  const struct dhcp_context *d;
+
+  /* eliminate addresses in use by the server. */
+  for (d = context; d; d = d->current)
+    if (addr.s_addr == d->router.s_addr)
+      return 1;
+  return 0;
+}
+
+static struct in_addr dhcp_context_next_addr(const struct dhcp_context *c,
+					     struct in_addr addr)
+{
+  if (addr.s_addr == c->end.s_addr)
+    addr = c->start;
+  else
+    addr.s_addr = htonl(ntohl(addr.s_addr) + 1);
+  return addr;
+}
+
+/* Addresses which end in .255 and .0 are broken in Windows even when using
+   supernetting. ie dhcp-range=192.168.0.1,192.168.1.254,255,255,254.0
+   then 192.168.0.255 is a valid IP address, but not for Windows as it's
+   in the class C range. See  KB281579. We therefore don't allocate these
+   addresses to avoid hard-to-diagnose problems. Thanks Bill. */
+static int bug_kb281579(struct in_addr addr)
+{
+  unsigned int h;
+  return (IN_CLASSC(ntohl(addr.s_addr)) &&
+	  ((h=ntohl(addr.s_addr) & 0xff) == 0xff || h == 0x0));
+}
+
+static int address_free(struct dhcp_context *c,
+		     struct in_addr addr, unsigned int j,
+		     time_t now, int loopback)
+{
+  if (	!lease_find_by_addr(addr) &&
+	!config_find_by_address(daemon->dhcp_conf, addr) &&
+	!bug_kb281579(addr))
+      {
+	/* in consec-ip mode, skip addresses equal to
+	   the number of addresses rejected by clients. This
+	   should avoid the same client being offered the same
+	   address after it has rjected it. */
+	if (option_bool(OPT_CONSEC_ADDR) && c->addr_epoch)
+	  c->addr_epoch--;
+	else
+	  {
+	    int r = do_icmp_ping(now, addr, j, loopback);
+
+	    if (r)
+	      {
+		/* consec-ip mode: we offered this address for another client
+		   (different hash) recently, don't offer it to this one. */
+		if (!option_bool(OPT_CONSEC_ADDR) || r > 1)
+		  return 1;
+	      }
+	    else
+	      {
+		/* address in use: perturb address selection so that we are
+		   less likely to try this address again. */
+		if (!option_bool(OPT_CONSEC_ADDR))
+		  c->addr_epoch++;
+	      }
+	  }
+      }
+  return 0;
+}
+
 int address_allocate(struct dhcp_context *context,
-		     struct in_addr *addrp, unsigned char *hwaddr, int hw_len, 
-		     struct dhcp_netid *netids, time_t now, int loopback)   
+		     struct in_addr *addrp, unsigned int hash,
+		     struct dhcp_netid *netids, time_t now, int loopback)
 {
   /* Find a free address: exclude anything in use and anything allocated to
      a particular hwaddr/clientid/hostname in our configuration.
      Try to return from contexts which match netids first. */
 
   struct in_addr start, addr;
-  struct dhcp_context *c, *d;
+  struct dhcp_context *c;
   int pass;
-  unsigned int j; 
-
-  j = ping_hash(hwaddr, hw_len);
 
   for (pass = 0; pass <= 1; pass++)
     for (c = context; c; c = c->current)
       if (c->flags & (CONTEXT_STATIC | CONTEXT_PROXY))
 	continue;
-      else if (!match_netid(c->filter, netids, pass))
-	continue;
-      else
+      else if (match_netid(c->filter, netids, pass))
 	{
-	  if (option_bool(OPT_CONSEC_ADDR))
-	    /* seed is largest extant lease addr in this context */
-	    start = lease_find_max_addr(c);
-	  else
-	    /* pick a seed based on hwaddr */
-	    start.s_addr = htonl(ntohl(c->start.s_addr) + 
-				 ((j + c->addr_epoch) % (1 + ntohl(c->end.s_addr) - ntohl(c->start.s_addr))));
-
 	  /* iterate until we find a free address. */
-	  addr = start;
-	  
-	  do {
-	    /* eliminate addresses in use by the server. */
-	    for (d = context; d; d = d->current)
-	      if (addr.s_addr == d->router.s_addr)
-		break;
+	  addr = start = dhcp_context_start_addr(c, hash);
 
-	    /* Addresses which end in .255 and .0 are broken in Windows even when using 
-	       supernetting. ie dhcp-range=192.168.0.1,192.168.1.254,255,255,254.0
-	       then 192.168.0.255 is a valid IP address, but not for Windows as it's
-	       in the class C range. See  KB281579. We therefore don't allocate these 
-	       addresses to avoid hard-to-diagnose problems. Thanks Bill. */	    
-	    if (!d &&
-		!lease_find_by_addr(addr) && 
-		!config_find_by_address(daemon->dhcp_conf, addr) &&
-		(!IN_CLASSC(ntohl(addr.s_addr)) || 
-		 ((ntohl(addr.s_addr) & 0xff) != 0xff && ((ntohl(addr.s_addr) & 0xff) != 0x0))))
+	  do {
+	    if (!dhcp_context_my_addr(context, addr) &&
+		address_free(c, addr, hash, now, loopback))
 	      {
-		/* in consec-ip mode, skip addresses equal to
-		   the number of addresses rejected by clients. This
-		   should avoid the same client being offered the same
-		   address after it has rjected it. */
-		if (option_bool(OPT_CONSEC_ADDR) && c->addr_epoch)
-		  c->addr_epoch--;
-		else
-		  {
-		    int r = do_icmp_ping(now, addr, j, loopback);
-		    
-		    if (r)
-		      {
-			/* consec-ip mode: we offered this address for another client
-			   (different hash) recently, don't offer it to this one. */
-			if (!option_bool(OPT_CONSEC_ADDR) || r > 1)
-			  {
-			    *addrp = addr;
-			    return 1;
-			  }
-		      }
-		    else
-		      {
-			/* address in use: perturb address selection so that we are
-			   less likely to try this address again. */
-			if (!option_bool(OPT_CONSEC_ADDR))
-			  c->addr_epoch++;
-		      }
-		  }
+		*addrp = addr;
+		return 1;
 	      }
+	    addr = dhcp_context_next_addr(c, addr);
+	  } while (addr.s_addr != start.s_addr);
+	}
 
-	    if (addr.s_addr == c->end.s_addr)
-	      addr = c->start;
-	    else
-	      addr.s_addr = htonl(ntohl(addr.s_addr) + 1);
+  return 0;
+}
+
+struct dhcp_lease *find_temp_lease(struct dhcp_context *context, unsigned int hash,
+		     struct dhcp_netid *netids)
+{
+  /* Find a free address: exclude anything in use and anything allocated to
+     a particular hwaddr/clientid/hostname in our configuration.
+     Try to return from contexts which match netids first. */
+
+  struct in_addr start, addr;
+  struct dhcp_context *c;
+  int pass;
+  struct dhcp_lease *lease;
+
+  for (pass = 0; pass <= 1; pass++)
+    for (c = context; c; c = c->current)
+      if (c->flags & (CONTEXT_STATIC | CONTEXT_PROXY))
+	continue;
+      else if (match_netid(c->filter, netids, pass))
+	{
+	  /* iterate until we find a free address. */
+	  addr = start = dhcp_context_start_addr(c, hash);
+
+	  do {
+	    if (!dhcp_context_my_addr(context, addr) &&
+		!config_find_by_address(daemon->dhcp_conf, addr) &&
+		(lease = lease_find_by_addr(addr)) &&
+		(lease->flags & LEASE_TEMP))
+	      return lease;
 
+	    addr = dhcp_context_next_addr(c, addr);
 	  } while (addr.s_addr != start.s_addr);
 	}
 
-  return 0;
+  return NULL;
 }
 
 void dhcp_read_ethers(void)
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index c992965..3c1f419 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1482,8 +1482,10 @@ unsigned int ping_hash(unsigned char *hwaddr, int hw_len);
 int do_icmp_ping(time_t now, struct in_addr addr,
 		 unsigned int hash, int loopback);
 int address_allocate(struct dhcp_context *context,
-		     struct in_addr *addrp, unsigned char *hwaddr, int hw_len,
+		     struct in_addr *addrp, unsigned int ping_hash,
 		     struct dhcp_netid *netids, time_t now, int loopback);
+struct dhcp_lease *find_temp_lease(struct dhcp_context *context,
+				   unsigned int hash, struct dhcp_netid *netids);
 void dhcp_read_ethers(void);
 struct dhcp_config *config_find_by_address(struct dhcp_config *configs, struct in_addr addr);
 char *host_from_dns(struct in_addr addr);
diff --git a/src/rfc2131.c b/src/rfc2131.c
index 9ec76c7..3c2ad29 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -73,9 +73,13 @@ static struct dhcp_lease *temp_lease4_allocate(struct dhcp_context *context,
 		     struct dhcp_netid *netids, time_t now, int loopback)
 {
   struct dhcp_lease *lease;
-  if (!address_allocate(context, &mess->yiaddr, hwaddr, hw_len, netids, now, loopback))
-    return NULL;
-  lease = lease4_allocate(mess->yiaddr, 1);
+  unsigned int hash = ping_hash(hwaddr, hw_len);
+  if (!address_allocate(context, &mess->yiaddr, hash, netids, now, loopback))
+    /* When no unused address is available, reuse first temporary lease
+     * with matching address. */
+    lease = find_temp_lease(context, hash, netids);
+  else
+    lease = lease4_allocate(mess->yiaddr, 1);
   if (lease)
     {
       lease_set_hwaddr(lease, mess->chaddr, NULL, mess->hlen, mess->htype, 0, now, 0);
@@ -639,8 +643,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 		       lease_prune(lease, now);
 		       lease = NULL;
 		     }
-		   if (!address_allocate(context, &mess->yiaddr, mess->chaddr,
-					 mess->hlen, tagif_netid, now, loopback))
+		   if (!address_allocate(context, &mess->yiaddr, ping_hash(mess->chaddr, mess->hlen),
+					 tagif_netid, now, loopback))
 		     message = _("no address available");
 		}
 	      else
-- 
2.31.1

From c7bf53755643ce79ff7967f2a566a356d72dfcaf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Fri, 10 Dec 2021 20:18:20 +0100
Subject: [PATCH 3/6] Create temporary leases on DISCOVER message

Previously only ping cache contained hash of few last pinged addresses.
They allowed skipping of address proposed to different host in
allocate_address. If no-ping were used together with dhcp-sequential-ip,
nothing would prevent offering single address to multiple clients.

Use temporary leases to store clients interested right when DHCPDISCOVER is
received. It makes sure that address is 'reserved' for that client when
he requests it. Uses short expiration time.
---
 src/dbus.c    |  2 +-
 src/dnsmasq.h |  3 ++-
 src/lease.c   | 25 +++++++++++++++++--------
 src/rfc2131.c | 30 ++++++++++++++++++++++++------
 4 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/src/dbus.c b/src/dbus.c
index d746b9a..dac767f 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -534,7 +534,7 @@ static DBusMessage *dbus_add_lease(DBusMessage* message)
 				      "ia_id and is_temporary must be zero for IPv4 lease");
       
       if (!(lease = lease_find_by_addr(addr.addr4)))
-    	lease = lease4_allocate(addr.addr4);
+    	lease = lease4_allocate(addr.addr4, 0);
     }
 #ifdef HAVE_DHCP6
   else if (inet_pton(AF_INET6, ipaddr, &addr.addr6))
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index aa63580..c992965 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -788,6 +788,7 @@ struct frec {
 #define LEASE_TA            64  /* IPv6 temporary lease */
 #define LEASE_HAVE_HWADDR  128  /* Have set hwaddress */
 #define LEASE_EXP_CHANGED  256  /* Lease expiry time changed */
+#define LEASE_TEMP       0x200  /* Lease contains just DISCOVER reservation */
 
 struct dhcp_lease {
   int clid_len;          /* length of client identifier */
@@ -1493,7 +1494,7 @@ char *host_from_dns(struct in_addr addr);
 void lease_update_file(time_t now);
 void lease_update_dns(int force);
 void lease_init(time_t now);
-struct dhcp_lease *lease4_allocate(struct in_addr addr);
+struct dhcp_lease *lease4_allocate(struct in_addr addr, int temp);
 #ifdef HAVE_DHCP6
 struct dhcp_lease *lease6_allocate(struct in6_addr *addrp, int lease_type);
 struct dhcp_lease *lease6_find(unsigned char *clid, int clid_len, 
diff --git a/src/lease.c b/src/lease.c
index 1a9f1c6..0b8f094 100644
--- a/src/lease.c
+++ b/src/lease.c
@@ -69,7 +69,7 @@ static int read_leases(time_t now, FILE *leasestream)
 		
 	if (inet_pton(AF_INET, daemon->namebuff, &addr.addr4))
 	  {
-	    if ((lease = lease4_allocate(addr.addr4)))
+	    if ((lease = lease4_allocate(addr.addr4, 0)))
 	      domain = get_domain(lease->addr);
 	    
 	    hw_len = parse_hex(daemon->dhcp_buff2, (unsigned char *)daemon->dhcp_buff2, DHCP_CHADDR_MAX, NULL, &hw_type);
@@ -264,6 +264,8 @@ void lease_update_file(time_t now)
       for (lease = leases; lease; lease = lease->next)
 	{
 
+	  if (lease->flags & LEASE_TEMP)
+	    continue;
 #ifdef HAVE_DHCP6
 	  if (lease->flags & (LEASE_TA | LEASE_NA))
 	    continue;
@@ -760,7 +762,7 @@ struct in_addr lease_find_max_addr(struct dhcp_context *context)
   return addr;
 }
 
-static struct dhcp_lease *lease_allocate(void)
+static struct dhcp_lease *lease_allocate(int temp)
 {
   struct dhcp_lease *lease;
   if (!leases_left || !(lease = whine_malloc(sizeof(struct dhcp_lease))))
@@ -775,16 +777,22 @@ static struct dhcp_lease *lease_allocate(void)
   lease->hwaddr_len = 256; /* illegal value */
   lease->next = leases;
   leases = lease;
-  
-  file_dirty = 1;
   leases_left--;
+  if (!temp)
+    {
+      file_dirty = 1;
+    }
+  else
+    {
+      lease->flags |= LEASE_TEMP;
+    }
 
   return lease;
 }
 
-struct dhcp_lease *lease4_allocate(struct in_addr addr)
+struct dhcp_lease *lease4_allocate(struct in_addr addr, int temp)
 {
-  struct dhcp_lease *lease = lease_allocate();
+  struct dhcp_lease *lease = lease_allocate(temp);
   if (lease)
     {
       lease->addr = addr;
@@ -797,7 +805,7 @@ struct dhcp_lease *lease4_allocate(struct in_addr addr)
 #ifdef HAVE_DHCP6
 struct dhcp_lease *lease6_allocate(struct in6_addr *addrp, int lease_type)
 {
-  struct dhcp_lease *lease = lease_allocate();
+  struct dhcp_lease *lease = lease_allocate(0);
 
   if (lease)
     {
@@ -883,7 +891,8 @@ void lease_set_hwaddr(struct dhcp_lease *lease, const unsigned char *hwaddr,
       lease->hwaddr_len = hw_len;
       lease->hwaddr_type = hw_type;
       lease->flags |= LEASE_CHANGED;
-      file_dirty = 1; /* run script on change */
+      if ((lease->flags & LEASE_TEMP) != 0)
+	file_dirty = 1; /* run script on change */
     }
 
   /* only update clid when one is available, stops packets
diff --git a/src/rfc2131.c b/src/rfc2131.c
index a47174b..9ec76c7 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -68,6 +68,22 @@ static int pxe_uefi_workaround(int pxe_arch, struct dhcp_netid *netid, struct dh
 static void apply_delay(u32 xid, time_t recvtime, struct dhcp_netid *netid);
 static int is_pxe_client(struct dhcp_packet *mess, size_t sz, const char **pxe_vendor);
 
+static struct dhcp_lease *temp_lease4_allocate(struct dhcp_context *context,
+		     struct dhcp_packet *mess, unsigned char *hwaddr, int hw_len,
+		     struct dhcp_netid *netids, time_t now, int loopback)
+{
+  struct dhcp_lease *lease;
+  if (!address_allocate(context, &mess->yiaddr, hwaddr, hw_len, netids, now, loopback))
+    return NULL;
+  lease = lease4_allocate(mess->yiaddr, 1);
+  if (lease)
+    {
+      lease_set_hwaddr(lease, mess->chaddr, NULL, mess->hlen, mess->htype, 0, now, 0);
+      lease_set_expires(lease, PING_CACHE_TIME, now);
+    }
+  return lease;
+}
+
 size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 		  size_t sz, time_t now, int unicast_dest, int loopback,
 		  int *is_inform, int pxe, struct in_addr fallback, time_t recvtime)
@@ -623,7 +639,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 		       lease_prune(lease, now);
 		       lease = NULL;
 		     }
-		   if (!address_allocate(context, &mess->yiaddr, mess->chaddr, mess->hlen, tagif_netid, now, loopback))
+		   if (!address_allocate(context, &mess->yiaddr, mess->chaddr,
+					 mess->hlen, tagif_netid, now, loopback))
 		     message = _("no address available");
 		}
 	      else
@@ -651,7 +668,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 
 	  if (!message && 
 	      !lease && 
-	      (!(lease = lease4_allocate(mess->yiaddr))))
+	      (!(lease = lease4_allocate(mess->yiaddr, 0))))
 	    message = _("no leases left");
 	  
 	  if (!message)
@@ -662,7 +679,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 	      if (hostname)
 		lease_set_hostname(lease, hostname, 1, get_domain(lease->addr), domain); 
 	      /* infinite lease unless nailed in dhcp-host line. */
-	      lease_set_expires(lease,  
+	      lease_set_expires(lease,
 				have_config(config, CONFIG_TIME) ? config->lease_time : 0xffffffff, 
 				now); 
 	      lease_set_interface(lease, int_index, now);
@@ -1138,8 +1155,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 	    mess->yiaddr = addr;
 	  else if (emac_len == 0)
 	    message = _("no unique-id");
-	  else if (!address_allocate(context, &mess->yiaddr, emac, emac_len, tagif_netid, now, loopback))
-	    message = _("no address available");      
+	  else if (!temp_lease4_allocate(context, mess, emac, emac_len, tagif_netid, now, loopback))
+	    message = _("no address available");
 	}
       
       daemon->metrics[METRIC_DHCPDISCOVER]++;
@@ -1341,7 +1358,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 	      
 	      else if (!lease)
 		{	     
-		  if ((lease = lease4_allocate(mess->yiaddr)))
+		  if ((lease = lease4_allocate(mess->yiaddr, 0)))
 		    do_classes = 1;
 		  else
 		    message = _("no leases left");
@@ -1472,6 +1489,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 	    }
 	  
 	  time = calc_time(context, config, option_find(mess, sz, OPTION_LEASE_TIME, 4));
+	  lease->flags &= ~LEASE_TEMP;
 	  lease_set_hwaddr(lease, mess->chaddr, clid, mess->hlen, mess->htype, clid_len, now, do_classes);
 	  
 	  /* if all the netids in the ignore_name list are present, ignore client-supplied name */
-- 
2.31.1

From b120eaefd9910d5dbeeffb54152c9c68236d8822 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Wed, 8 Dec 2021 00:11:46 +0100
Subject: [PATCH 2/6] Simplify ICMP ping from dhcp

Do not export whole record with hash. Instead return just value
signalling when hash has matched.

Make a bit simpler search of free addresses in dhcp.
---
 src/dhcp.c    | 33 ++++++++++++++-------------------
 src/dnsmasq.h |  4 ++--
 2 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index 1614929..24d50f1 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -721,9 +721,8 @@ unsigned int ping_hash(unsigned char *hwaddr, int hw_len)
    This wrapper handles a cache and load-limiting.
    Return is NULL is address in use, or a pointer to a cache entry
    recording that it isn't. */
-struct ping_result *do_icmp_ping(time_t now, struct in_addr addr, unsigned int hash, int loopback)
+int do_icmp_ping(time_t now, struct in_addr addr, unsigned int hash, int loopback)
 {
-  static struct ping_result dummy;
   struct ping_result *r, *victim = NULL;
   int count, max = (int)(0.6 * (((float)PING_CACHE_TIME)/
 				((float)PING_WAIT)));
@@ -741,19 +740,14 @@ struct ping_result *do_icmp_ping(time_t now, struct in_addr addr, unsigned int h
       {
 	count++;
 	if (r->addr.s_addr == addr.s_addr)
-	  return r;
+	  return (1 + (hash == r->hash));
       }
   
   /* didn't find cached entry */
   if ((count >= max) || option_bool(OPT_NO_PING) || loopback)
-    {
       /* overloaded, or configured not to check, loopback interface, return "not in use" */
-      dummy.hash = hash;
-      return &dummy;
-    }
-  else if (icmp_ping(addr))
-    return NULL; /* address in use. */
-  else
+    return 2;
+  else if (!icmp_ping(addr))
     {
       /* at this point victim may hold an expired record */
       if (!victim)
@@ -772,9 +766,10 @@ struct ping_result *do_icmp_ping(time_t now, struct in_addr addr, unsigned int h
 	  victim->addr = addr;
 	  victim->time = now;
 	  victim->hash = hash;
+	  return 2;
 	}
-      return victim;
     }
+  return 0; /* address in use or no memory. */
 }
 
 int address_allocate(struct dhcp_context *context,
@@ -836,13 +831,13 @@ int address_allocate(struct dhcp_context *context,
 		  c->addr_epoch--;
 		else
 		  {
-		    struct ping_result *r;
+		    int r = do_icmp_ping(now, addr, j, loopback);
 		    
-		    if ((r = do_icmp_ping(now, addr, j, loopback)))
+		    if (r)
 		      {
 			/* consec-ip mode: we offered this address for another client
 			   (different hash) recently, don't offer it to this one. */
-			if (!option_bool(OPT_CONSEC_ADDR) || r->hash == j)
+			if (!option_bool(OPT_CONSEC_ADDR) || r > 1)
 			  {
 			    *addrp = addr;
 			    return 1;
@@ -857,12 +852,12 @@ int address_allocate(struct dhcp_context *context,
 		      }
 		  }
 	      }
-	    
-	    addr.s_addr = htonl(ntohl(addr.s_addr) + 1);
-	    
-	    if (addr.s_addr == htonl(ntohl(c->end.s_addr) + 1))
+
+	    if (addr.s_addr == c->end.s_addr)
 	      addr = c->start;
-	    
+	    else
+	      addr.s_addr = htonl(ntohl(addr.s_addr) + 1);
+
 	  } while (addr.s_addr != start.s_addr);
 	}
 
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index c1dbe98..aa63580 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1478,8 +1478,8 @@ struct dhcp_context *narrow_context(struct dhcp_context *context,
 				    struct in_addr taddr,
 				    struct dhcp_netid *netids);
 unsigned int ping_hash(unsigned char *hwaddr, int hw_len);
-struct ping_result *do_icmp_ping(time_t now, struct in_addr addr,
-				 unsigned int hash, int loopback);
+int do_icmp_ping(time_t now, struct in_addr addr,
+		 unsigned int hash, int loopback);
 int address_allocate(struct dhcp_context *context,
 		     struct in_addr *addrp, unsigned char *hwaddr, int hw_len,
 		     struct dhcp_netid *netids, time_t now, int loopback);
-- 
2.31.1

From 50e998196cec11d6899f89924672f76f75a2866f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Wed, 8 Dec 2021 00:39:30 +0100
Subject: [PATCH 1/6] Add icmp ping hash also when pinging requested address

Previously requested address were pinged always with hash 0, but pinged
from address_allocate with different hash. Try using the same hash for
the same client, regardless what source place it were used.
---
 src/dhcp.c    | 26 +++++++++++++++++---------
 src/dnsmasq.h |  1 +
 src/rfc2131.c |  3 ++-
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index e500bc2..1614929 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -702,6 +702,21 @@ struct dhcp_config *config_find_by_address(struct dhcp_config *configs, struct i
   return NULL;
 }
 
+unsigned int ping_hash(unsigned char *hwaddr, int hw_len)
+{
+  int i;
+  unsigned int j = 0;
+  /* hash hwaddr: use the SDBM hashing algorithm.  Seems to give good
+     dispersal even with similarly-valued "strings". */
+  for (i = 0; i < hw_len; i++)
+    j = hwaddr[i] + (j << 6) + (j << 16) - j;
+
+  /* j == 0 is marker */
+  if (j == 0)
+    j = 1;
+  return j;
+}
+
 /* Check if and address is in use by sending ICMP ping.
    This wrapper handles a cache and load-limiting.
    Return is NULL is address in use, or a pointer to a cache entry
@@ -772,18 +787,11 @@ int address_allocate(struct dhcp_context *context,
 
   struct in_addr start, addr;
   struct dhcp_context *c, *d;
-  int i, pass;
+  int pass;
   unsigned int j; 
 
-  /* hash hwaddr: use the SDBM hashing algorithm.  Seems to give good
-     dispersal even with similarly-valued "strings". */ 
-  for (j = 0, i = 0; i < hw_len; i++)
-    j = hwaddr[i] + (j << 6) + (j << 16) - j;
+  j = ping_hash(hwaddr, hw_len);
 
-  /* j == 0 is marker */
-  if (j == 0)
-    j = 1;
-  
   for (pass = 0; pass <= 1; pass++)
     for (c = context; c; c = c->current)
       if (c->flags & (CONTEXT_STATIC | CONTEXT_PROXY))
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 05c1743..c1dbe98 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1477,6 +1477,7 @@ struct dhcp_context *address_available(struct dhcp_context *context,
 struct dhcp_context *narrow_context(struct dhcp_context *context, 
 				    struct in_addr taddr,
 				    struct dhcp_netid *netids);
+unsigned int ping_hash(unsigned char *hwaddr, int hw_len);
 struct ping_result *do_icmp_ping(time_t now, struct in_addr addr,
 				 unsigned int hash, int loopback);
 int address_allocate(struct dhcp_context *context,
diff --git a/src/rfc2131.c b/src/rfc2131.c
index c902eb7..a47174b 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -1133,7 +1133,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 		   !config_find_by_address(daemon->dhcp_conf, lease->addr))
 	    mess->yiaddr = lease->addr;
 	  else if (opt && address_available(context, addr, tagif_netid) && !lease_find_by_addr(addr) && 
-		   !config_find_by_address(daemon->dhcp_conf, addr) && do_icmp_ping(now, addr, 0, loopback))
+		   !config_find_by_address(daemon->dhcp_conf, addr) &&
+		   do_icmp_ping(now, addr, ping_hash(emac, emac_len), loopback))
 	    mess->yiaddr = addr;
 	  else if (emac_len == 0)
 	    message = _("no unique-id");
-- 
2.31.1

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss

Reply via email to