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
From 18e49004782549068450cded3c12ff65e44a4308 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/2] 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 c3bdf99..b7cbe84 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -722,9 +722,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)));
@@ -742,19 +741,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)
@@ -773,9 +767,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,
@@ -837,13 +832,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;
@@ -858,12 +853,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 f39458f..db9106f 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1481,8 +1481,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 2b57d92a6495edfa94f2f005ba4b883afd885790 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/2] 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 d9bcd3d..c3bdf99 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -703,6 +703,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
@@ -773,18 +788,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 937c7c7..f39458f 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1480,6 +1480,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 f05fad7..abc1bd7 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -1185,7 +1185,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
