From 42026f9fb3d42bad9ebc91d3a43a1f9a1315732f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Nov 2019 11:29:13 +0100 Subject: [PATCH 1/3] shared: add nm_utils_monotonic_timestamp_from_boottime() util We sometimes have a CLOCK_BOOTTIME and need to convert it to NetworkManager's monotonic timestamps. --- shared/nm-glib-aux/nm-time-utils.c | 46 ++++++++++++++++++++++++++++++ shared/nm-glib-aux/nm-time-utils.h | 1 + src/tests/test-core-with-expect.c | 3 ++ 3 files changed, 50 insertions(+) diff --git a/shared/nm-glib-aux/nm-time-utils.c b/shared/nm-glib-aux/nm-time-utils.c index 9e3fdf4e83..20d663bc02 100644 --- a/shared/nm-glib-aux/nm-time-utils.c +++ b/shared/nm-glib-aux/nm-time-utils.c @@ -265,6 +265,52 @@ nm_utils_monotonic_timestamp_as_boottime (gint64 timestamp, gint64 timestamp_ns_ return timestamp - offset; } +/** + * nm_utils_monotonic_timestamp_from_boottime: + * @boottime: the timestamp from CLOCK_BOOTTIME (or CLOCK_MONOTONIC, if + * kernel does not support CLOCK_BOOTTIME and monotonic timestamps are based + * on CLOCK_MONOTONIC). + * @timestamp_ns_per_tick: the scale in which @boottime is. If @boottime is in + * nano seconds, this should be 1. If it is in milli seconds, this should be + * %NM_UTILS_NS_PER_SECOND/1000, etc. + * + * Returns: the same timestamp in monotonic timestamp scale. + * + * Note that commonly monotonic timestamps are positive. But they may not + * be positive in this case. That's when boottime is taken from a time before + * the monotonic timestamps started counting. So, that means a zero or negative + * value is still a valid timestamp. + * + * This is the inverse of nm_utils_monotonic_timestamp_as_boottime(). + */ +gint64 +nm_utils_monotonic_timestamp_from_boottime (guint64 boottime, gint64 timestamp_ns_per_tick) +{ + const GlobalState *p; + gint64 offset; + + /* only support ns-per-tick being a multiple of 10. */ + g_return_val_if_fail (timestamp_ns_per_tick == 1 + || (timestamp_ns_per_tick > 0 && + timestamp_ns_per_tick <= NM_UTILS_NS_PER_SECOND && + timestamp_ns_per_tick % 10 == 0), + -1); + + p = _t_get_global_state (); + + nm_assert (p->offset_sec <= 0); + + /* calculate the offset of monotonic-timestamp to boottime. offset_s is <= 1. */ + offset = p->offset_sec * (NM_UTILS_NS_PER_SECOND / timestamp_ns_per_tick); + + nm_assert (offset <= 0 && offset > G_MININT64); + + /* check for overflow (note that offset is non-positive). */ + g_return_val_if_fail (boottime < G_MAXINT64, G_MAXINT64); + + return (gint64) boottime + offset; +} + gint64 nm_utils_clock_gettime_ns (clockid_t clockid) { diff --git a/shared/nm-glib-aux/nm-time-utils.h b/shared/nm-glib-aux/nm-time-utils.h index 7d2c1cf3f7..8bf41b96a1 100644 --- a/shared/nm-glib-aux/nm-time-utils.h +++ b/shared/nm-glib-aux/nm-time-utils.h @@ -27,6 +27,7 @@ gint64 nm_utils_get_monotonic_timestamp_us (void); gint64 nm_utils_get_monotonic_timestamp_ms (void); gint32 nm_utils_get_monotonic_timestamp_s (void); gint64 nm_utils_monotonic_timestamp_as_boottime (gint64 timestamp, gint64 timestamp_ticks_per_ns); +gint64 nm_utils_monotonic_timestamp_from_boottime (guint64 boottime, gint64 timestamp_ns_per_tick); static inline gint64 nm_utils_get_monotonic_timestamp_ns_cached (gint64 *cache_now) diff --git a/src/tests/test-core-with-expect.c b/src/tests/test-core-with-expect.c index 41f1c13c40..74e1043b33 100644 --- a/src/tests/test-core-with-expect.c +++ b/src/tests/test-core-with-expect.c @@ -44,10 +44,13 @@ test_nm_utils_monotonic_timestamp_as_boottime (void) g_assert_cmpint (now_boottime_2, >=, now_boottime); g_assert_cmpint (now_boottime_2 - now_boottime, <=, NM_UTILS_NS_PER_SECOND / 10); + g_assert_cmpint (now, ==, nm_utils_monotonic_timestamp_from_boottime (now_boottime_2, 1)); + for (timestamp_ns_per_tick = 1; timestamp_ns_per_tick <= NM_UTILS_NS_PER_SECOND; timestamp_ns_per_tick *= 10) { now_boottime_3 = nm_utils_monotonic_timestamp_as_boottime (now / timestamp_ns_per_tick, timestamp_ns_per_tick); g_assert_cmpint (now_boottime_2 / timestamp_ns_per_tick, ==, now_boottime_3); + g_assert_cmpint (now / timestamp_ns_per_tick, ==, nm_utils_monotonic_timestamp_from_boottime (now_boottime_3, timestamp_ns_per_tick)); } } } From a8d46492b308f786770aa7a896f2b34ccc978cfc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Sep 2019 16:14:05 +0200 Subject: [PATCH 2/3] dhcp/nettools: don't trim the "expiry" timestamp to 32 bit The "expiry" is the Unix timestamp when the lease expires. This is not at all a useful parameter, in particular because the system's clock can be reset. Instead, we should expose the lease receive time stamp (in CLOCK_BOOTTIME), and the lease lifetime. Anyway. So, we somehow need to express infinite lifetimes. Previously, we would use the special value 4294967295 (2^32-1). However, that value does not seem so great, because it's also the Unix timestamp of 2106-02-07T06:28:15+0000. While that is quite far in the future, it's a valid timestamp still. Of course, the code worked around that by never setting a timestamp larger than 4294967295-1, but it still limits the range of what we can expose. Note that for the lifetime "dhcp_lease_time", we do express infinity with 4294967295. That's fine, it also does not contradict what we receive in the DHCP lease on the wire because the lifetime there is expressed by a 32 bit integer. Instead, for the "expiry" timestamp, don't perform such triming. The expiry timestamp is just the start timestamp plus the lease lifetime. If that is larger than 2106-02-07, so be it. On the other hand, express infinity by omitting the "expiry" field. --- src/dhcp/nm-dhcp-nettools.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index e2719f8557..228bbc8bd1 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -372,7 +372,7 @@ lease_parse_address (NDhcp4ClientLease *lease, */ if (nettools_lifetime == G_MAXUINT64) { a_lifetime = NM_PLATFORM_LIFETIME_PERMANENT; - a_expiry = NM_PLATFORM_LIFETIME_PERMANENT; + a_expiry = -1; } else { gint64 ts_time = time (NULL); @@ -383,10 +383,7 @@ lease_parse_address (NDhcp4ClientLease *lease, else if (a_lifetime > NM_PLATFORM_LIFETIME_PERMANENT) a_lifetime = NM_PLATFORM_LIFETIME_PERMANENT - 1; - if (ts_time > NM_PLATFORM_LIFETIME_PERMANENT - a_lifetime) - a_expiry = NM_PLATFORM_LIFETIME_PERMANENT - 1; - else - a_expiry = ts_time + a_lifetime; + a_expiry = ts_time + a_lifetime; } if (!lease_get_in_addr (lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &a_netmask)) { @@ -411,10 +408,12 @@ lease_parse_address (NDhcp4ClientLease *lease, NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME, (guint64) a_lifetime); - nm_dhcp_option_add_option_u64 (options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NM_EXPIRY, - (guint64) a_expiry); + if (a_expiry != -1) { + nm_dhcp_option_add_option_u64 (options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_NM_EXPIRY, + (guint64) a_expiry); + } n_dhcp4_client_lease_get_siaddr (lease, &a_next_server); From 0108d7486642d5210a6b913efc4ca50f1846c7f6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Nov 2019 11:19:06 +0100 Subject: [PATCH 3/3] dhcp/nettools: exactly calculate lease lifetimes Now that we can not only get the expiry timestamp of the lease (n_dhcp4_client_lease_get_lifetime()), but also the base timestamp, we can calculate the lifetime exactly. Previously, we had to guess the base time by assuming that we just received the lease *now*. This wasn't exact. --- src/dhcp/nm-dhcp-nettools.c | 62 ++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index 228bbc8bd1..170d25e967 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -348,15 +348,14 @@ lease_parse_address (NDhcp4ClientLease *lease, GError **error) { char addr_str[NM_UTILS_INET_ADDRSTRLEN]; - const gint64 ts = nm_utils_get_monotonic_timestamp_ns (); - const gint64 ts_clock_boottime = nm_utils_monotonic_timestamp_as_boottime (ts, 1); struct in_addr a_address; struct in_addr a_netmask; struct in_addr a_next_server; guint32 a_plen; guint64 nettools_lifetime; - gint64 a_lifetime; - gint64 a_expiry; + guint32 a_lifetime; + guint32 a_timestamp; + guint64 a_expiry; n_dhcp4_client_lease_get_yiaddr (lease, &a_address); if (a_address.s_addr == INADDR_ANY) { @@ -364,26 +363,47 @@ lease_parse_address (NDhcp4ClientLease *lease, return FALSE; } - /* n_dhcp4_client_lease_get_lifetime() never fails */ n_dhcp4_client_lease_get_lifetime (lease, &nettools_lifetime); - /* FIXME: n_dhcp4_client_lease_get_lifetime() returns the time in nsec of CLOCK_BOOTTIME. - * We want to retrieve the original lifetime value in seconds, so we approximate it in a_lifetime. - * Use a nettools API to retrieve the original value as passed by the server. - */ + if (nettools_lifetime == G_MAXUINT64) { + a_timestamp = 0; a_lifetime = NM_PLATFORM_LIFETIME_PERMANENT; - a_expiry = -1; + a_expiry = G_MAXUINT64; } else { - gint64 ts_time = time (NULL); + guint64 nettools_basetime; + guint64 lifetime; + gint64 ts; - a_lifetime = ((gint64) nettools_lifetime - ts_clock_boottime + (NM_UTILS_NS_PER_SECOND / 2)) / NM_UTILS_NS_PER_SECOND; - /* A lease time of 0 is allowed on some dhcp servers, so, let's accept it. */ - if (a_lifetime < 0) - a_lifetime = 0; - else if (a_lifetime > NM_PLATFORM_LIFETIME_PERMANENT) - a_lifetime = NM_PLATFORM_LIFETIME_PERMANENT - 1; + n_dhcp4_client_lease_get_basetime (lease, &nettools_basetime); - a_expiry = ts_time + a_lifetime; + /* usually we shouldn't assert against external libraries like n-dhcp4. + * Here we still do it... it seems safe enough. */ + nm_assert (nettools_basetime > 0); + nm_assert (nettools_lifetime >= nettools_basetime); + nm_assert (((nettools_lifetime - nettools_basetime) % NM_UTILS_NS_PER_SECOND) == 0); + nm_assert ((nettools_lifetime - nettools_basetime) <= G_MAXUINT32); + + if (nettools_lifetime <= nettools_basetime) { + /* A lease time of 0 is allowed on some dhcp servers, so, let's accept it. */ + lifetime = 0; + } else { + lifetime = nettools_lifetime - nettools_basetime; + + /* we "ceil" the value to the next second. In practice, we don't expect any sub-second values + * from n-dhcp4 anyway, so this should have no effect. */ + lifetime += NM_UTILS_NS_PER_SECOND - 1; + } + + ts = nm_utils_monotonic_timestamp_from_boottime (nettools_basetime, 1); + + /* the timestamp must be positive, because we only started nettools DHCP client + * after obtaining the first monotonic timestamp. Hence, the lease must have been + * received afterwards. */ + nm_assert (ts >= NM_UTILS_NS_PER_SECOND); + + a_timestamp = ts / NM_UTILS_NS_PER_SECOND; + a_lifetime = NM_MIN (lifetime / NM_UTILS_NS_PER_SECOND, NM_PLATFORM_LIFETIME_PERMANENT - 1); + a_expiry = time (NULL) + ((lifetime - (nm_utils_clock_gettime_ns (CLOCK_BOOTTIME) - nettools_basetime)) / NM_UTILS_NS_PER_SECOND); } if (!lease_get_in_addr (lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &a_netmask)) { @@ -408,11 +428,11 @@ lease_parse_address (NDhcp4ClientLease *lease, NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME, (guint64) a_lifetime); - if (a_expiry != -1) { + if (a_expiry != G_MAXUINT64) { nm_dhcp_option_add_option_u64 (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_NM_EXPIRY, - (guint64) a_expiry); + a_expiry); } @@ -431,7 +451,7 @@ lease_parse_address (NDhcp4ClientLease *lease, .peer_address = a_address.s_addr, .plen = a_plen, .addr_source = NM_IP_CONFIG_SOURCE_DHCP, - .timestamp = ts / NM_UTILS_NS_PER_SECOND, + .timestamp = a_timestamp, .lifetime = a_lifetime, .preferred = a_lifetime, }));