Re: [Dnsmasq-discuss] [PATCH] lease: prune lease as soon as expired

2019-02-27 Thread Simon Kelley
Nice catch.

Patch applied.

Thanks for your work with that one.


Cheers,

Simon.



On 11/02/2019 16:04, Florent Fourcot wrote:
> We detected a performance issue on a dnsmasq running many dhcp sessions
> (more than 10 000). At the end of the day, the server was only releasing
> old DHCP leases but was consuming a lot of CPU.
> 
> It looks like curent dhcp pruning:
>  1) it's pruning old sessions (iterate on all current leases). It's
>  important to note that it's only pruning session expired since more
>  than one second
>  2) it's looking for next lease to expire (iterate on all current leases
>  again)
>  3) it launchs an alarm to catch next expiration found in step 2). This
>  value can be zero for leases just expired (but not pruned).
> 
> So, for a second, dnsmasq could fall in a "prune loop" by doing:
>  * Not pruning anything, since difftime() is not > 0
>  * Run alarm again with zero as argument
> 
> On a server with very large number of leases and releasing often
> sessions, that can waste a very big CPU time.
> 
> Signed-off-by: Florent Fourcot 
> ---
>  src/lease.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/lease.c b/src/lease.c
> index 97bee21..7299227 100644
> --- a/src/lease.c
> +++ b/src/lease.c
> @@ -563,7 +563,7 @@ void lease_prune(struct dhcp_lease *target, time_t now)
>for (lease = leases, up =  lease; lease = tmp)
>  {
>tmp = lease->next;
> -  if ((lease->expires != 0 && difftime(now, lease->expires) > 0) || 
> lease == target)
> +  if ((lease->expires != 0 && difftime(now, lease->expires) >= 0) || 
> lease == target)
>   {
> file_dirty = 1;
> if (lease->hostname)
> 


___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


[Dnsmasq-discuss] [PATCH] lease: prune lease as soon as expired

2019-02-11 Thread Florent Fourcot
We detected a performance issue on a dnsmasq running many dhcp sessions
(more than 10 000). At the end of the day, the server was only releasing
old DHCP leases but was consuming a lot of CPU.

It looks like curent dhcp pruning:
 1) it's pruning old sessions (iterate on all current leases). It's
 important to note that it's only pruning session expired since more
 than one second
 2) it's looking for next lease to expire (iterate on all current leases
 again)
 3) it launchs an alarm to catch next expiration found in step 2). This
 value can be zero for leases just expired (but not pruned).

So, for a second, dnsmasq could fall in a "prune loop" by doing:
 * Not pruning anything, since difftime() is not > 0
 * Run alarm again with zero as argument

On a server with very large number of leases and releasing often
sessions, that can waste a very big CPU time.

Signed-off-by: Florent Fourcot 
---
 src/lease.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lease.c b/src/lease.c
index 97bee21..7299227 100644
--- a/src/lease.c
+++ b/src/lease.c
@@ -563,7 +563,7 @@ void lease_prune(struct dhcp_lease *target, time_t now)
   for (lease = leases, up =  lease; lease = tmp)
 {
   tmp = lease->next;
-  if ((lease->expires != 0 && difftime(now, lease->expires) > 0) || lease 
== target)
+  if ((lease->expires != 0 && difftime(now, lease->expires) >= 0) || lease 
== target)
{
  file_dirty = 1;
  if (lease->hostname)
-- 
2.11.0


___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss