* [PATCH 1/2] dhcp-server: Fix lease expiry time calculation
@ 2021-06-04 22:19 Andrew Zaborowski
2021-06-04 22:19 ` [PATCH 2/2] dhcp: Track lease lifetime and bind time separately Andrew Zaborowski
2021-06-07 18:21 ` [PATCH 1/2] dhcp-server: Fix lease expiry time calculation Denis Kenzior
0 siblings, 2 replies; 3+ messages in thread
From: Andrew Zaborowski @ 2021-06-04 22:19 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 1891 bytes --]
We were comparing the l_time_now() (usecs) to lease->lifetime (secs)
and then converting the result from usecs to secs, so the "diff" and the
"to_secs" operations need to be switched around. While there, use the
opportunity to replace the "diff" operation so that if lease->lifetime is
already in the past (e.g. because processing took to long), we schedule
the timeout in 1 millisec instead of the absolute value of the difference.
---
ell/dhcp-server.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/ell/dhcp-server.c b/ell/dhcp-server.c
index 50855fe..3d6a6a8 100644
--- a/ell/dhcp-server.c
+++ b/ell/dhcp-server.c
@@ -191,7 +191,6 @@ static void set_next_expire_timer(struct l_dhcp_server *server,
struct l_dhcp_lease *expired)
{
struct l_dhcp_lease *next;
- unsigned int next_timeout;
/*
* If this is an expiring lease put it into the expired queue, removing
@@ -214,15 +213,20 @@ static void set_next_expire_timer(struct l_dhcp_server *server,
return;
}
- next_timeout = l_time_to_secs(l_time_diff(l_time_now(),
- next->lifetime));
-
- if (server->next_expire)
- l_timeout_modify(server->next_expire, next_timeout);
- else
- server->next_expire = l_timeout_create(server->lease_seconds,
- lease_expired_cb,
- server, NULL);
+ if (server->next_expire) {
+ uint32_t now = l_time_to_secs(l_time_now());
+
+ if (l_time_after(next->lifetime, now)) {
+ unsigned int next_timeout = next->lifetime - now;
+
+ l_timeout_modify(server->next_expire, next_timeout);
+ } else
+ l_timeout_modify_ms(server->next_expire, 1);
+ } else
+ server->next_expire = l_timeout_create(
+ server->lease_seconds,
+ lease_expired_cb,
+ server, NULL);
}
static void lease_expired_cb(struct l_timeout *timeout, void *user_data)
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] dhcp: Track lease lifetime and bind time separately
2021-06-04 22:19 [PATCH 1/2] dhcp-server: Fix lease expiry time calculation Andrew Zaborowski
@ 2021-06-04 22:19 ` Andrew Zaborowski
2021-06-07 18:21 ` [PATCH 1/2] dhcp-server: Fix lease expiry time calculation Denis Kenzior
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Zaborowski @ 2021-06-04 22:19 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 5606 bytes --]
dhcp-server has been confusing lease->lifetime with the expiry time and
also reducing the expiry timestamp resolution and maximum to 32-bit
second count. To fix both problems, add microsecond-resultion 64-bit
l_dhcp_lease::bound_time (compatible with ell/time.h utils) and use
l_dhcp_lease::lifetime exclusively for the life time in seconds. Both
fields have the same usage between client and server now. A public
getter for the bound time isn't added for now for lack of evidence that
it's going to be useful.
---
ell/dhcp-private.h | 1 +
ell/dhcp-server.c | 45 +++++++++++++++++++++++++++++----------------
ell/dhcp.c | 2 ++
3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/ell/dhcp-private.h b/ell/dhcp-private.h
index 204e54e..c422322 100644
--- a/ell/dhcp-private.h
+++ b/ell/dhcp-private.h
@@ -158,6 +158,7 @@ struct l_dhcp_lease {
uint32_t lifetime;
uint32_t t1;
uint32_t t2;
+ uint64_t bound_time;
uint32_t router;
uint32_t *dns;
char *domain_name;
diff --git a/ell/dhcp-server.c b/ell/dhcp-server.c
index 3d6a6a8..4c31834 100644
--- a/ell/dhcp-server.c
+++ b/ell/dhcp-server.c
@@ -30,6 +30,7 @@
#include <arpa/inet.h>
#include <time.h>
#include <errno.h>
+#include <limits.h>
#include "private.h"
#include "time.h"
@@ -102,12 +103,14 @@ struct l_dhcp_server {
l_util_debug(server->debug_handler, server->debug_data, \
"%s:%i " fmt, __func__, __LINE__, ## args)
-static bool is_expired_lease(struct l_dhcp_lease *lease)
+static uint64_t get_lease_expiry_time(const struct l_dhcp_lease *lease)
{
- if (lease->lifetime < l_time_to_secs(l_time_now()))
- return true;
+ return lease->bound_time + lease->lifetime * L_USEC_PER_SEC;
+}
- return false;
+static bool is_expired_lease(const struct l_dhcp_lease *lease)
+{
+ return !l_time_after(get_lease_expiry_time(lease), l_time_now());
}
static bool match_lease_mac(const void *data, const void *user_data)
@@ -169,11 +172,12 @@ static int get_lease(struct l_dhcp_server *server, uint32_t yiaddr,
return 0;
}
-static int compare_lifetime_or_offering(const void *a, const void *b,
+static int compare_expiry_or_offering(const void *a, const void *b,
void *user_data)
{
const struct l_dhcp_lease *lease1 = a;
const struct l_dhcp_lease *lease2 = b;
+ int64_t diff;
/*
* Ensures offered but not active leases stay at the head of the queue.
@@ -182,7 +186,9 @@ static int compare_lifetime_or_offering(const void *a, const void *b,
if (lease1->offering)
return 1;
- return lease2->lifetime - lease1->lifetime;
+ diff = (int64_t) lease2->bound_time - lease1->bound_time +
+ ((int64_t) lease2->lifetime - lease1->lifetime) * L_USEC_PER_SEC;
+ return diff >= 0 ? diff > 0 ? 1 : 0 : -1;
}
static void lease_expired_cb(struct l_timeout *timeout, void *user_data);
@@ -214,14 +220,16 @@ static void set_next_expire_timer(struct l_dhcp_server *server,
}
if (server->next_expire) {
- uint32_t now = l_time_to_secs(l_time_now());
+ uint64_t expiry = get_lease_expiry_time(next);
+ uint64_t now = l_time_now();
+ uint64_t next_timeout = l_time_after(expiry, now) ?
+ expiry - now : 0;
+ uint64_t next_timeout_ms = l_time_to_msecs(next_timeout) ?: 1;
- if (l_time_after(next->lifetime, now)) {
- unsigned int next_timeout = next->lifetime - now;
+ if (next_timeout_ms > ULONG_MAX)
+ next_timeout_ms = ULONG_MAX;
- l_timeout_modify(server->next_expire, next_timeout);
- } else
- l_timeout_modify_ms(server->next_expire, 1);
+ l_timeout_modify_ms(server->next_expire, next_timeout_ms);
} else
server->next_expire = l_timeout_create(
server->lease_seconds,
@@ -234,6 +242,11 @@ static void lease_expired_cb(struct l_timeout *timeout, void *user_data)
struct l_dhcp_server *server = user_data;
struct l_dhcp_lease *lease = l_queue_peek_tail(server->lease_list);
+ if (!is_expired_lease(lease)) {
+ set_next_expire_timer(server, NULL);
+ return;
+ }
+
if (server->event_handler)
server->event_handler(server, L_DHCP_SERVER_EVENT_LEASE_EXPIRED,
server->user_data, lease);
@@ -258,24 +271,24 @@ static struct l_dhcp_lease *add_lease(struct l_dhcp_server *server,
lease->address = yiaddr;
lease->offering = offering;
- lease->lifetime = l_time_to_secs(l_time_now());
+ lease->bound_time = l_time_now();
if (!offering) {
- lease->lifetime += server->lease_seconds;
+ lease->lifetime = server->lease_seconds;
/*
* Insert into queue by lifetime (skipping any offered leases
* at the head)
*/
l_queue_insert(server->lease_list, lease,
- compare_lifetime_or_offering, NULL);
+ compare_expiry_or_offering, NULL);
/*
* This is a new (or renewed lease) so pass NULL for expired so
* the queue's are not modified, only the next_expired timer
*/
set_next_expire_timer(server, NULL);
} else {
- lease->lifetime += OFFER_TIME;
+ lease->lifetime = OFFER_TIME;
/* Push offered leases to head, active leases after those */
l_queue_push_head(server->lease_list, lease);
}
diff --git a/ell/dhcp.c b/ell/dhcp.c
index a4afa95..fff1645 100644
--- a/ell/dhcp.c
+++ b/ell/dhcp.c
@@ -860,6 +860,8 @@ static void dhcp_client_rx_message(const void *data, size_t len, void *userdata)
dhcp_client_event_notify(client, r);
+ client->lease->bound_time = l_time_now();
+
/*
* Start T1, once it expires we will start the T2 timer. If
* we renew the lease, we will end up back here.
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] dhcp-server: Fix lease expiry time calculation
2021-06-04 22:19 [PATCH 1/2] dhcp-server: Fix lease expiry time calculation Andrew Zaborowski
2021-06-04 22:19 ` [PATCH 2/2] dhcp: Track lease lifetime and bind time separately Andrew Zaborowski
@ 2021-06-07 18:21 ` Denis Kenzior
1 sibling, 0 replies; 3+ messages in thread
From: Denis Kenzior @ 2021-06-07 18:21 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 675 bytes --]
Hi Andrew,
On 6/4/21 5:19 PM, Andrew Zaborowski wrote:
> We were comparing the l_time_now() (usecs) to lease->lifetime (secs)
> and then converting the result from usecs to secs, so the "diff" and the
> "to_secs" operations need to be switched around. While there, use the
> opportunity to replace the "diff" operation so that if lease->lifetime is
> already in the past (e.g. because processing took to long), we schedule
> the timeout in 1 millisec instead of the absolute value of the difference.
> ---
> ell/dhcp-server.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
Both applied, thanks.
Regards,
-Denis
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-07 18:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-04 22:19 [PATCH 1/2] dhcp-server: Fix lease expiry time calculation Andrew Zaborowski
2021-06-04 22:19 ` [PATCH 2/2] dhcp: Track lease lifetime and bind time separately Andrew Zaborowski
2021-06-07 18:21 ` [PATCH 1/2] dhcp-server: Fix lease expiry time calculation Denis Kenzior
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).