platform: merge branch 'th/bgo727382_platform_fix_addr_lifetime' (part 2)

https://bugzilla.gnome.org/show_bug.cgi?id=727382

Signed-off-by: Thomas Haller <thaller@redhat.com>

5 files changed, 157 insertions, 45 deletions

diff --git a/src/dhcp-manager/nm-dhcp-dhclient-utils.c b/src/dhcp-manager/nm-dhcp-dhclient-utils.c
index 3e7173aec3..8527e6c0fb 100644
--- a/src/dhcp-manager/nm-dhcp-dhclient-utils.c
+++ b/src/dhcp-manager/nm-dhcp-dhclient-utils.c
@@ -27,6 +27,7 @@
 #include "nm-dhcp-dhclient-utils.h"
 #include "nm-ip4-config.h"
 #include "nm-utils.h"
+#include "NetworkManagerUtils.h"
 
 #define CLIENTID_TAG            "send dhcp-client-identifier"
 #define CLIENTID_FORMAT         CLIENTID_TAG " \"%s\"; # added by NetworkManager"
@@ -530,6 +531,7 @@ nm_dhcp_dhclient_read_lease_ip_configs (const char *iface,
 	GSList *parsed = NULL, *iter, *leases = NULL;
 	char **line, **split = NULL;
 	GHashTable *hash = NULL;
+	gint32 now_monotonic_ts;
 
 	g_return_val_if_fail (contents != NULL, NULL);
 
@@ -570,6 +572,7 @@ nm_dhcp_dhclient_read_lease_ip_configs (const char *iface,
 		g_date_time_ref (now);
 	else
 		now = g_date_time_new_now_utc ();
+	now_monotonic_ts = nm_utils_get_monotonic_timestamp_s ();
 
 	for (iter = parsed; iter; iter = g_slist_next (iter)) {
 		NMIP4Config *ip4;
@@ -593,7 +596,7 @@ nm_dhcp_dhclient_read_lease_ip_configs (const char *iface,
 			continue;
 
 		/* scale expiry to seconds (and CLAMP into the range of guint32) */
-		expiry = CLAMP (expiry / G_TIME_SPAN_SECOND, 0, G_MAXUINT32-1);
+		expiry = CLAMP (expiry / G_TIME_SPAN_SECOND, 0, NM_PLATFORM_LIFETIME_PERMANENT-1);
 		if (expiry <= 0) {
 			/* the address is already expired. Don't even add it. */
 			continue;
@@ -624,6 +627,7 @@ nm_dhcp_dhclient_read_lease_ip_configs (const char *iface,
 		if (!address.plen)
 			address.plen = nm_utils_ip4_get_default_prefix (address.address);
 
+		address.timestamp = now_monotonic_ts;
 		address.lifetime = address.preferred = expiry;
 		address.source = NM_PLATFORM_SOURCE_DHCP;
 
diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c
index 09733a45a8..f5da671380 100644
--- a/src/platform/nm-linux-platform.c
+++ b/src/platform/nm-linux-platform.c
@@ -1032,6 +1032,92 @@ hack_empty_master_iff_lower_up (NMPlatform *platform, struct nl_object *object)
 	rtnl_link_unset_flags (rtnllink, IFF_LOWER_UP);
 }
 
+static guint32
+_get_remaining_time (guint32 start_timestamp, guint32 end_timestamp)
+{
+	/* Return the remaining time between @start_timestamp until @end_timestamp.
+	 *
+	 * If @end_timestamp is NM_PLATFORM_LIFETIME_PERMANENT, it returns
+	 * NM_PLATFORM_LIFETIME_PERMANENT. If @start_timestamp already passed
+	 * @end_timestamp it returns 0. Beware, NMPlatformIPAddress treats a @lifetime
+	 * of 0 as permanent.
+	 */
+	if (end_timestamp == NM_PLATFORM_LIFETIME_PERMANENT)
+		return NM_PLATFORM_LIFETIME_PERMANENT;
+	if (start_timestamp >= end_timestamp)
+		return 0;
+	return end_timestamp - start_timestamp;
+}
+
+/* _timestamp_nl_to_ms:
+ * @timestamp_nl: a timestamp from ifa_cacheinfo.
+ * @monotonic_ms: *now* in CLOCK_MONOTONIC. Needed to estimate the current
+ * uptime and how often timestamp_nl wrapped.
+ *
+ * Convert the timestamp from ifa_cacheinfo to CLOCK_MONOTONIC milliseconds.
+ * The ifa_cacheinfo fields tstamp and cstamp contains timestamps that counts
+ * with in 1/100th of a second of clock_gettime(CLOCK_MONOTONIC). However,
+ * the uint32 counter wraps every 497 days of uptime, so we have to compensate
+ * for that. */
+static gint64
+_timestamp_nl_to_ms (guint32 timestamp_nl, gint64 monotonic_ms)
+{
+	const gint64 WRAP_INTERVAL = (((gint64) G_MAXUINT32) + 1) * (1000 / 100);
+	gint64 timestamp_nl_ms;
+
+	/* convert timestamp from 1/100th of a second to msec. */
+	timestamp_nl_ms = ((gint64) timestamp_nl) * (1000 / 100);
+
+	/* timestamp wraps every 497 days. Try to compensate for that.*/
+	if (timestamp_nl_ms > monotonic_ms) {
+		/* timestamp_nl_ms is in the future. Truncate it to *now* */
+		timestamp_nl_ms = monotonic_ms;
+	} else if (monotonic_ms >= WRAP_INTERVAL) {
+		timestamp_nl_ms += (monotonic_ms / WRAP_INTERVAL) * WRAP_INTERVAL;
+		if (timestamp_nl_ms > monotonic_ms)
+			timestamp_nl_ms -= WRAP_INTERVAL;
+	}
+
+	return timestamp_nl_ms;
+}
+
+static guint32
+_rtnl_addr_last_update_time_to_nm (const struct rtnl_addr *rtnladdr)
+{
+	guint32 last_update_time = rtnl_addr_get_last_update_time ((struct rtnl_addr *) rtnladdr);
+	struct timespec tp;
+	gint64 now_nl, now_nm, result;
+
+	/* timestamp is unset. Default to 1. */
+	if (!last_update_time)
+		return 1;
+
+	/* do all the calculations in milliseconds scale */
+
+	clock_gettime (CLOCK_MONOTONIC, &tp);
+	now_nm = nm_utils_get_monotonic_timestamp_ms ();
+	now_nl = (((gint64) tp.tv_sec) * ((gint64) 1000)) +
+	         (tp.tv_nsec / (NM_UTILS_NS_PER_SECOND/1000));
+
+	result = now_nm - (now_nl - _timestamp_nl_to_ms (last_update_time, now_nl));
+
+	/* converting the last_update_time into nm_utils_get_monotonic_timestamp_ms() scale is
+	 * a good guess but fails in the following situations:
+	 *
+	 * - If the address existed before start of the process, the timestamp in nm scale would
+	 *   be negative or zero. In this case we default to 1.
+	 * - during hibernation, the CLOCK_MONOTONIC/last_update_time drifts from
+	 *   nm_utils_get_monotonic_timestamp_ms() scale.
+	 */
+	if (result <= 1000)
+		return 1;
+
+	if (result > now_nm)
+		return now_nm / 1000;
+
+	return result / 1000;
+}
+
 static void
 _init_ip_address_lifetime (NMPlatformIPAddress *address, const struct rtnl_addr *rtnladdr)
 {
@@ -1057,26 +1143,26 @@ _init_ip_address_lifetime (NMPlatformIPAddress *address, const struct rtnl_addr
 	          a_valid > 0 &&
 	          a_preferred > 0);
 
-	/* The correct timestamp value would probably be rtnl_addr_get_last_update_time()
-	 * (after converting into the proper time scale).
-	 * But this is relatively complicated to convert and we don't actually need it.
-	 * Especially, because rtnl_addr_get_last_update_time() might be negative in
-	 * nm_utils_get_monotonic_timestamp_s() scale -- which we want to avoid.
-	 *
-	 * Remember: timestamp has no meaning, beyond anchoring the relative lifetimes
-	 * at some point in time. We choose this point to be "1 second".
+	if (a_valid <= 1) {
+		/* Since we want to have positive @timestamp and @valid != 0,
+		 * we must handle this case special. */
+		address->timestamp = 1;
+		address->lifetime = 1; /* Extend the lifetime by one second */
+		address->preferred = 0; /* no longer preferred. */
+		return;
+	}
+
+	/* _rtnl_addr_last_update_time_to_nm() might be wrong, so don't rely on
+	 * timestamp to have any meaning beyond anchoring the relative durations
+	 * @lifetime and @preferred.
 	 */
-	address->timestamp = 1;
-
-	/* account for the timestamp==1 by incrementing valid/preferred -- unless
-	 * it is NM_PLATFORM_LIFETIME_PERMANENT or already NM_PLATFORM_LIFETIME_PERMANENT-1
-	 * (i.e. the largest non-permanent lifetime). */
-	if (a_valid < NM_PLATFORM_LIFETIME_PERMANENT - 1)
-		a_valid += 1;
-	if (a_preferred < NM_PLATFORM_LIFETIME_PERMANENT - 1)
-		a_preferred += 1;
-	address->lifetime = a_valid;
-	address->preferred = a_preferred;
+	address->timestamp = _rtnl_addr_last_update_time_to_nm (rtnladdr);
+
+	/* We would expect @timestamp to be less then @a_valid. Just to be sure,
+	 * fix it up. */
+	address->timestamp = MIN (address->timestamp, a_valid - 1);
+	address->lifetime = _get_remaining_time (address->timestamp, a_valid);
+	address->preferred = _get_remaining_time (address->timestamp, a_preferred);
 }
 
 static gboolean
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 71c60ca5bf..542b0faab2 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -1436,6 +1436,7 @@ nm_platform_ip4_address_add (int ifindex,
 		addr.address = address;
 		addr.peer_address = peer_address;
 		addr.plen = plen;
+		addr.timestamp = 0; /* set it at zero, which to_string will treat as *now* */
 		addr.lifetime = lifetime;
 		addr.preferred = preferred;
 		if (label)
@@ -1470,6 +1471,7 @@ nm_platform_ip6_address_add (int ifindex,
 		addr.address = address;
 		addr.peer_address = peer_address;
 		addr.plen = plen;
+		addr.timestamp = 0; /* set it to zero, which to_string will treat as *now* */
 		addr.lifetime = lifetime;
 		addr.preferred = preferred;
 		addr.flags = flags;
@@ -1571,6 +1573,17 @@ _rebase_relative_time_on_now (guint32 timestamp, guint32 duration, guint32 now,
 	if (duration == NM_PLATFORM_LIFETIME_PERMANENT)
 		return NM_PLATFORM_LIFETIME_PERMANENT;
 
+	if (timestamp == 0) {
+		/* if the @timestamp is zero, assume it was just left unset and that the relative
+		 * @duration starts counting from @now. This is convenient to construct an address
+		 * and print it in nm_platform_ip4_address_to_string().
+		 *
+		 * In general it does not make sense to set the @duration without anchoring at
+		 * @timestamp because you don't know the absolute expiration time when looking
+		 * at the address at a later moment. */
+		timestamp = now;
+	}
+
 	/* For timestamp > now, just accept it and calculate the expected(?) result. */
 	t = (gint64) timestamp + (gint64) duration - (gint64) now;
 
@@ -1597,13 +1610,18 @@ _address_get_lifetime (const NMPlatformIPAddress *address, guint32 now, guint32
 		if (!lifetime)
 			return FALSE;
 		preferred = _rebase_relative_time_on_now (address->timestamp, address->preferred, now, padding);
-		if (preferred > lifetime) {
-			g_warn_if_reached ();
-			preferred = lifetime;
-		}
 
 		*out_lifetime = lifetime;
-		*out_preferred = preferred;
+		*out_preferred = MIN (preferred, lifetime);
+
+		/* Assert that non-permanent addresses have a (positive) @timestamp. _rebase_relative_time_on_now()
+		 * treats addresses with timestamp 0 as *now*. Addresses passed to _address_get_lifetime() always
+		 * should have a valid @timestamp, otherwise on every re-sync, their lifetime will be extended anew.
+		 */
+		g_return_val_if_fail (   address->timestamp != 0
+		                      || (   address->lifetime  == NM_PLATFORM_LIFETIME_PERMANENT
+		                          && address->preferred == NM_PLATFORM_LIFETIME_PERMANENT), TRUE);
+		g_return_val_if_fail (preferred <= lifetime, TRUE);
 	}
 	return TRUE;
 }
@@ -2089,12 +2107,6 @@ _lifetime_to_string (guint32 timestamp, guint32 lifetime, gint32 now, char *buf,
 static const char *
 _lifetime_summary_to_string (gint32 now, guint32 timestamp, guint32 preferred, guint32 lifetime, char *buf, size_t buf_size)
 {
-	if (timestamp == 0) {
-		if (preferred == NM_PLATFORM_LIFETIME_PERMANENT && lifetime == NM_PLATFORM_LIFETIME_PERMANENT)
-			return "";
-		if (preferred == 0 && lifetime == 0)
-			return " lifetime unset";
-	}
 	g_snprintf (buf, buf_size, " lifetime %d-%u[%u,%u]",
 	            (signed) now, (unsigned) timestamp, (unsigned) preferred, (unsigned) lifetime);
 	return buf;
@@ -2494,31 +2506,32 @@ nm_platform_ip6_route_cmp (const NMPlatformIP6Route *a, const NMPlatformIP6Route
 int
 nm_platform_ip_address_cmp_expiry (const NMPlatformIPAddress *a, const NMPlatformIPAddress *b)
 {
-	gint64 ta, tb;
+	gint64 ta = 0, tb = 0;
 
 	_CMP_POINTER (a, b);
 
 	if (a->lifetime == NM_PLATFORM_LIFETIME_PERMANENT || a->lifetime == 0)
 		ta = G_MAXINT64;
-	else
+	else if (a->timestamp)
 		ta = ((gint64) a->timestamp) + a->lifetime;
 
 	if (b->lifetime == NM_PLATFORM_LIFETIME_PERMANENT || b->lifetime == 0)
 		tb = G_MAXINT64;
-	else
+	else if (b->timestamp)
 		tb = ((gint64) b->timestamp) + b->lifetime;
 
 	if (ta == tb) {
 		/* if the lifetime is equal, compare the preferred time. */
+		ta = tb = 0;
 
 		if (a->preferred == NM_PLATFORM_LIFETIME_PERMANENT || a->lifetime == 0 /* liftime==0 means permanent! */)
 			ta = G_MAXINT64;
-		else
+		else if (a->timestamp)
 			ta = ((gint64) a->timestamp) + a->preferred;
 
 		if (b->preferred == NM_PLATFORM_LIFETIME_PERMANENT|| b->lifetime == 0)
 			tb = G_MAXINT64;
-		else
+		else if (b->timestamp)
 			tb = ((gint64) b->timestamp) + b->preferred;
 
 		if (ta == tb)
diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h
index f7b14b7b9b..96b6cfabbd 100644
--- a/src/platform/nm-platform.h
+++ b/src/platform/nm-platform.h
@@ -167,10 +167,22 @@ typedef struct {
 	NMPlatformSource source; \
 	\
 	/* Timestamp in seconds in the reference system of nm_utils_get_monotonic_timestamp_*().
-	 * This value is mainly used to anchor the relative lifetime and preferred values.
-	 * For addresses originating from DHCP it might be set to nm_utils_get_monotonic_timestamp_s()
-	 * of when the lease was received. For addresses from platform/kernel it is set to 1.
-	 * For permanent addresses it is mostly set to 0.
+	 *
+	 * The rules are:
+	 * 1 @lifetime==0: @timestamp and @preferred is irrelevant (but mostly set to 0 too). Such addresses
+	 *   are permanent. This rule is so that unset addresses (calloc) are permanent by default.
+	 * 2 @lifetime==@preferred==NM_PLATFORM_LIFETIME_PERMANENT: @timestamp is irrelevant (but mostly
+	 *   set to 0). Such addresses are permanent.
+	 * 3 Non permanent addreses should (almost) always have @timestamp > 0. 0 is not a valid timestamp
+	 *   and never returned by nm_utils_get_monotonic_timestamp_s(). In this case @valid/@preferred
+	 *   is anchored at @timestamp.
+	 * 4 Non permanent addresses with @timestamp == 0 are implicitely anchored at *now*, thus the time
+	 *   moves as time goes by. This is usually not useful, except e.g. nm_platform_ip[46]_address_add().
+	 *
+	 * Non permanent addresses from DHCP/RA might have the @timestamp set to the moment of when the
+	 * lease was received. Addresses from kernel might have the @timestamp based on the last modification
+	 * time of the addresses. But don't rely on this behaviour, the @timestamp is only defined for anchoring
+	 * @lifetime and @preferred.
 	 */ \
 	guint32 timestamp; \
 	guint32 lifetime;   /* seconds since timestamp */ \
diff --git a/src/platform/tests/dump.c b/src/platform/tests/dump.c
index 9276d18439..e97ef138cb 100644
--- a/src/platform/tests/dump.c
+++ b/src/platform/tests/dump.c
@@ -13,7 +13,6 @@ dump_interface (NMPlatformLink *link)
 	GArray *ip4_addresses;
 	const NMPlatformIP6Address *ip6_address;
 	const NMPlatformIP4Address *ip4_address;
-	char addrstr[INET6_ADDRSTRLEN];
 	GArray *ip6_routes;
 	GArray *ip4_routes;
 	const NMPlatformIP6Route *ip6_route;
@@ -73,14 +72,12 @@ dump_interface (NMPlatformLink *link)
 
 	for (i = 0; i < ip4_addresses->len; i++) {
 		ip4_address = &g_array_index (ip4_addresses, NMPlatformIP4Address, i);
-		inet_ntop (AF_INET, &ip4_address->address, addrstr, sizeof (addrstr));
-		printf ("    ip4-address %s/%d lifetime %u preferred %u\n", addrstr, ip4_address->plen, ip4_address->lifetime, ip4_address->preferred);
+		printf ("    ip4-address %s\n", nm_platform_ip4_address_to_string (ip4_address));
 	}
 
 	for (i = 0; i < ip6_addresses->len; i++) {
 		ip6_address = &g_array_index (ip6_addresses, NMPlatformIP6Address, i);
-		inet_ntop (AF_INET6, &ip6_address->address, addrstr, sizeof (addrstr));
-		printf ("    ip6-address %s/%d lifetime %u preferred %u\n", addrstr, ip6_address->plen, ip6_address->lifetime, ip6_address->preferred);
+		printf ("    ip6-address %s\n", nm_platform_ip6_address_to_string (ip6_address));
 	}
 
 	g_array_unref (ip4_addresses);