ndisc: merge branch 'th/ndisc-addr-lifetime'

https://github.com/NetworkManager/NetworkManager/pull/228

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/57
https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1796622

7 files changed, 163 insertions, 88 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index d1f059abd8..8bc98d6155 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -3545,7 +3545,8 @@ ndisc_set_router_config (NMNDisc *ndisc, NMDevice *self)
 		guint32 lifetime, preferred;
 		gint32 base;
 
-		if (IN6_IS_ADDR_LINKLOCAL (&addr->address))
+		if (   IN6_IS_ADDR_UNSPECIFIED (&addr->address)
+		    || IN6_IS_ADDR_LINKLOCAL (&addr->address))
 			continue;
 
 		if (   addr->n_ifa_flags & IFA_F_TENTATIVE
diff --git a/src/ndisc/nm-fake-ndisc.c b/src/ndisc/nm-fake-ndisc.c
index 15abee88f1..6e9a72c6f4 100644
--- a/src/ndisc/nm-fake-ndisc.c
+++ b/src/ndisc/nm-fake-ndisc.c
@@ -296,7 +296,7 @@ receive_ra (gpointer user_data)
 				.dad_counter = 0,
 			};
 
-			if (nm_ndisc_complete_and_add_address (ndisc, &address))
+			if (nm_ndisc_complete_and_add_address (ndisc, &address, now))
 				changed |= NM_NDISC_CONFIG_ADDRESSES;
 		}
 	}
diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c
index 8bb1812764..9f427d4d59 100644
--- a/src/ndisc/nm-lndp-ndisc.c
+++ b/src/ndisc/nm-lndp-ndisc.c
@@ -221,10 +221,10 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 				.preferred = ndp_msg_opt_prefix_preferred_time (msg, offset),
 			};
 
-			if (address.preferred > address.lifetime)
-				address.preferred = address.lifetime;
-			if (nm_ndisc_complete_and_add_address (ndisc, &address))
-				changed |= NM_NDISC_CONFIG_ADDRESSES;
+			if (address.preferred <= address.lifetime) {
+				if (nm_ndisc_complete_and_add_address (ndisc, &address, now))
+					changed |= NM_NDISC_CONFIG_ADDRESSES;
+			}
 		}
 	}
 	ndp_msg_opt_for_each_offset(offset, msg, NDP_MSG_OPT_ROUTE) {
diff --git a/src/ndisc/nm-ndisc-private.h b/src/ndisc/nm-ndisc-private.h
index bbecb01a43..ae03c0ee43 100644
--- a/src/ndisc/nm-ndisc-private.h
+++ b/src/ndisc/nm-ndisc-private.h
@@ -40,7 +40,7 @@ void nm_ndisc_ra_received (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed)
 void nm_ndisc_rs_received (NMNDisc *ndisc);
 
 gboolean nm_ndisc_add_gateway              (NMNDisc *ndisc, const NMNDiscGateway *new);
-gboolean nm_ndisc_complete_and_add_address (NMNDisc *ndisc, NMNDiscAddress *new);
+gboolean nm_ndisc_complete_and_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gint32 now_s);
 gboolean nm_ndisc_add_route                (NMNDisc *ndisc, const NMNDiscRoute *new);
 gboolean nm_ndisc_add_dns_server           (NMNDisc *ndisc, const NMNDiscDNSServer *new);
 gboolean nm_ndisc_add_dns_domain           (NMNDisc *ndisc, const NMNDiscDNSDomain *new);
diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index a6b4453891..1dd8398c75 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -124,53 +124,66 @@ _preference_to_priority (NMIcmpv6RouterPref pref)
 
 /*****************************************************************************/
 
-static gint32
+/* we rely on the fact, that _EXPIRY_INFINITY > any other valid gint64 timestamps. */
+#define _EXPIRY_INFINITY G_MAXINT64
+
+static gint64
 get_expiry_time (guint32 timestamp, guint32 lifetime)
 {
-	gint64 t;
-
-	/* timestamp is supposed to come from nm_utils_get_monotonic_timestamp_s().
-	 * It is expected to be within a certain range. */
 	nm_assert (timestamp > 0);
 	nm_assert (timestamp <= G_MAXINT32);
 
 	if (lifetime == NM_NDISC_INFINITY)
-		return G_MAXINT32;
-
-	t = (gint64) timestamp + (gint64) lifetime;
-	return CLAMP (t, 0, G_MAXINT32 - 1);
+		return _EXPIRY_INFINITY;
+	return ((gint64) timestamp) + ((gint64) lifetime);
 }
 
 #define get_expiry(item) \
 	({ \
 		typeof (item) _item = (item); \
 		nm_assert (_item); \
-		get_expiry_time ((_item->timestamp), (_item->lifetime)); \
+		get_expiry_time (_item->timestamp, _item->lifetime); \
 	})
 
 #define get_expiry_half(item) \
 	({ \
 		typeof (item) _item = (item); \
 		nm_assert (_item); \
-		get_expiry_time ((_item->timestamp),\
-		                 (_item->lifetime) == NM_NDISC_INFINITY \
-		                   ? NM_NDISC_INFINITY \
-		                   : (_item->lifetime) / 2); \
+		(_item->lifetime == NM_NDISC_INFINITY) \
+		  ? _EXPIRY_INFINITY \
+		  : get_expiry_time (_item->timestamp, _item->lifetime / 2); \
 	})
 
 #define get_expiry_preferred(item) \
 	({ \
 		typeof (item) _item = (item); \
 		nm_assert (_item); \
-		get_expiry_time ((_item->timestamp), (_item->preferred)); \
+		get_expiry_time (_item->timestamp, _item->preferred); \
 	})
 
+static gboolean
+expiry_next (gint32 now_s, gint64 expiry_timestamp, gint32 *nextevent)
+{
+	gint32 e;
+
+	if (expiry_timestamp == _EXPIRY_INFINITY)
+		return TRUE;
+	e = MIN (expiry_timestamp, ((gint64) (G_MAXINT32 - 1)));
+	if (now_s >= e)
+		return FALSE;
+	if (nextevent) {
+		if (*nextevent > e)
+			*nextevent = e;
+	}
+	return TRUE;
+}
+
 static const char *
-_get_exp (char *buf, gsize buf_size, gint64 now_ns, gint32 expiry_time)
+_get_exp (char *buf, gsize buf_size, gint64 now_ns, gint64 expiry_time)
 {
 	int l;
 
-	if (expiry_time == G_MAXINT32)
+	if (expiry_time == _EXPIRY_INFINITY)
 		return "permanent";
 	l = g_snprintf (buf, buf_size,
 	                "%.4f",
@@ -406,34 +419,101 @@ complete_address (NMNDisc *ndisc, NMNDiscAddress *addr)
 }
 
 static gboolean
-nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new)
+nm_ndisc_add_address (NMNDisc *ndisc,
+                      const NMNDiscAddress *new,
+                      gint32 now_s,
+                      gboolean from_ra)
 {
 	NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE (ndisc);
 	NMNDiscDataInternal *rdata = &priv->rdata;
+	NMNDiscAddress new2;
+	NMNDiscAddress *existing = NULL;
 	guint i;
 
 	nm_assert (new);
 	nm_assert (new->timestamp > 0 && new->timestamp < G_MAXINT32);
 	nm_assert (!IN6_IS_ADDR_UNSPECIFIED (&new->address));
 	nm_assert (!IN6_IS_ADDR_LINKLOCAL (&new->address));
+	nm_assert (new->preferred <= new->lifetime);
+	nm_assert (!from_ra || now_s > 0);
 
 	for (i = 0; i < rdata->addresses->len; i++) {
 		NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i);
 
-		if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) {
-			if (new->lifetime == 0) {
-				g_array_remove_index (rdata->addresses, i);
-				return TRUE;
+		if (from_ra) {
+			/* RFC4862 5.5.3.d, we find an existing address with the same prefix.
+			 * (note that all prefixes at this point have implicity length /64). */
+			if (memcmp (&item->address, &new->address, 8) == 0) {
+				existing = item;
+				break;
 			}
+		} else {
+			if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) {
+				existing = item;
+				break;
+			}
+		}
+	}
 
-			if (   item->dad_counter == new->dad_counter
-			    && get_expiry (item) == get_expiry (new)
-			    && get_expiry_preferred (item) == get_expiry_preferred (new))
-				return FALSE;
+	if (existing) {
+		if (from_ra) {
+			const gint32 NM_NDISC_PREFIX_LFT_MIN = 7200; /* seconds, RFC4862 5.5.3.e */
+			gint64 old_expiry_lifetime, old_expiry_preferred;
+
+			old_expiry_lifetime = get_expiry (existing);
+			old_expiry_preferred = get_expiry_preferred (existing);
+
+			if (new->lifetime == NM_NDISC_INFINITY)
+				existing->lifetime = NM_NDISC_INFINITY;
+			else {
+				gint64 new_lifetime, remaining_lifetime;
+
+				/* see RFC4862 5.5.3.e */
+				if (existing->lifetime == NM_NDISC_INFINITY)
+					remaining_lifetime = G_MAXINT64;
+				else
+					remaining_lifetime = ((gint64) existing->timestamp) + ((gint64) existing->lifetime) - ((gint64) now_s);
+				new_lifetime = ((gint64) new->timestamp) + ((gint64) new->lifetime) - ((gint64) now_s);
+
+				if (   new_lifetime > (gint64) NM_NDISC_PREFIX_LFT_MIN
+				    || new_lifetime > remaining_lifetime) {
+					existing->timestamp = now_s;
+					existing->lifetime = CLAMP (new_lifetime, (gint64) 0, (gint64) (G_MAXUINT32 - 1));
+				} else if (remaining_lifetime <= (gint64) NM_NDISC_PREFIX_LFT_MIN) {
+					/* keep the current lifetime. */
+				} else {
+					existing->timestamp = now_s;
+					existing->lifetime = NM_NDISC_PREFIX_LFT_MIN;
+				}
+			}
 
-			*item = *new;
+			if (new->preferred == NM_NDISC_INFINITY) {
+				nm_assert (existing->lifetime == NM_NDISC_INFINITY);
+				existing->preferred = new->preferred;
+			} else {
+				existing->preferred = NM_CLAMP (((gint64) new->timestamp) + ((gint64) new->preferred) - ((gint64) existing->timestamp),
+				                                0, G_MAXUINT32 - 1);
+				if (existing->lifetime != NM_NDISC_INFINITY)
+					existing->preferred = MIN (existing->preferred, existing->lifetime);
+			}
+
+			return    old_expiry_lifetime != get_expiry (existing)
+			       || old_expiry_preferred != get_expiry_preferred (existing);
+		}
+
+		if (new->lifetime == 0) {
+			g_array_remove_index (rdata->addresses, i);
 			return TRUE;
 		}
+
+		if (   get_expiry (existing) == get_expiry (new)
+		    && get_expiry_preferred (existing) == get_expiry_preferred (new))
+			return FALSE;
+
+		existing->timestamp = new->timestamp;
+		existing->lifetime = new->lifetime;
+		existing->preferred = new->preferred;
+		return TRUE;
 	}
 
 	/* we create at most max_addresses autoconf addresses. This is different from
@@ -444,18 +524,27 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new)
 	    && rdata->addresses->len >= priv->max_addresses)
 		return FALSE;
 
-	if (new->lifetime)
-		g_array_append_val (rdata->addresses, *new);
-	return !!new->lifetime;
+	if (new->lifetime == 0)
+		return FALSE;
+
+	if (from_ra) {
+		new2 = *new;
+		new2.dad_counter = 0;
+		if (!complete_address (ndisc, &new2))
+			return FALSE;
+		new = &new2;
+	}
+
+	g_array_append_val (rdata->addresses, *new);
+	return TRUE;
 }
 
 gboolean
-nm_ndisc_complete_and_add_address (NMNDisc *ndisc, NMNDiscAddress *new)
+nm_ndisc_complete_and_add_address (NMNDisc *ndisc,
+                                   const NMNDiscAddress *new,
+                                   gint32 now_s)
 {
-	if (!complete_address (ndisc, new))
-		return FALSE;
-
-	return nm_ndisc_add_address (ndisc, new);
+	return nm_ndisc_add_address (ndisc, new, now_s, TRUE);
 }
 
 gboolean
@@ -753,7 +842,7 @@ nm_ndisc_set_config (NMNDisc *ndisc,
 	guint i;
 
 	for (i = 0; i < addresses->len; i++) {
-		if (nm_ndisc_add_address (ndisc, &g_array_index (addresses, NMNDiscAddress, i)))
+		if (nm_ndisc_add_address (ndisc, &g_array_index (addresses, NMNDiscAddress, i), 0, FALSE))
 			changed = TRUE;
 	}
 
@@ -1000,17 +1089,12 @@ clean_gateways (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *n
 	for (i = 0; i < rdata->gateways->len; ) {
 		NMNDiscGateway *item = &g_array_index (rdata->gateways, NMNDiscGateway, i);
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-
-			if (now >= expiry) {
-				g_array_remove_index (rdata->gateways, i);
-				*changed |= NM_NDISC_CONFIG_GATEWAYS;
-				continue;
-			}
-			if (*nextevent > expiry)
-				*nextevent = expiry;
+		if (!expiry_next (now, get_expiry (item), nextevent)) {
+			g_array_remove_index (rdata->gateways, i);
+			*changed |= NM_NDISC_CONFIG_GATEWAYS;
+			continue;
 		}
+
 		i++;
 	}
 
@@ -1028,17 +1112,12 @@ clean_addresses (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *
 	for (i = 0; i < rdata->addresses->len; ) {
 		const NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i);
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-
-			if (now >= expiry) {
-				g_array_remove_index (rdata->addresses, i);
-				*changed |= NM_NDISC_CONFIG_ADDRESSES;
-				continue;
-			}
-			if (*nextevent > expiry)
-				*nextevent = expiry;
+		if (!expiry_next (now, get_expiry (item), nextevent)) {
+			g_array_remove_index (rdata->addresses, i);
+			*changed |= NM_NDISC_CONFIG_ADDRESSES;
+			continue;
 		}
+
 		i++;
 	}
 }
@@ -1054,17 +1133,12 @@ clean_routes (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nex
 	for (i = 0; i < rdata->routes->len; ) {
 		NMNDiscRoute *item = &g_array_index (rdata->routes, NMNDiscRoute, i);
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-
-			if (now >= expiry) {
-				g_array_remove_index (rdata->routes, i);
-				*changed |= NM_NDISC_CONFIG_ROUTES;
-				continue;
-			}
-			if (*nextevent > expiry)
-				*nextevent = expiry;
+		if (!expiry_next (now, get_expiry (item), nextevent)) {
+			g_array_remove_index (rdata->routes, i);
+			*changed |= NM_NDISC_CONFIG_ROUTES;
+			continue;
 		}
+
 		i++;
 	}
 }
@@ -1079,18 +1153,16 @@ clean_dns_servers (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32
 
 	for (i = 0; i < rdata->dns_servers->len; ) {
 		NMNDiscDNSServer *item = &g_array_index (rdata->dns_servers, NMNDiscDNSServer, i);
+		gint64 refresh;
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-			gint32 refresh;
-
-			if (now >= expiry) {
+		refresh = get_expiry_half (item);
+		if (refresh != _EXPIRY_INFINITY) {
+			if (!expiry_next (now, get_expiry (item), NULL)) {
 				g_array_remove_index (rdata->dns_servers, i);
 				*changed |= NM_NDISC_CONFIG_DNS_SERVERS;
 				continue;
 			}
 
-			refresh = get_expiry_half (item);
 			if (now >= refresh)
 				solicit_routers (ndisc);
 			else if (*nextevent > refresh)
@@ -1110,18 +1182,16 @@ clean_dns_domains (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32
 
 	for (i = 0; i < rdata->dns_domains->len; ) {
 		NMNDiscDNSDomain *item = &g_array_index (rdata->dns_domains, NMNDiscDNSDomain, i);
+		gint64 refresh;
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-			gint32 refresh;
-
-			if (now >= expiry) {
+		refresh = get_expiry_half (item);
+		if (refresh != _EXPIRY_INFINITY) {
+			if (!expiry_next (now, get_expiry (item), NULL)) {
 				g_array_remove_index (rdata->dns_domains, i);
 				*changed |= NM_NDISC_CONFIG_DNS_DOMAINS;
 				continue;
 			}
 
-			refresh = get_expiry_half (item);
 			if (now >= refresh)
 				solicit_routers (ndisc);
 			else if (*nextevent > refresh)
diff --git a/src/ndisc/nm-ndisc.h b/src/ndisc/nm-ndisc.h
index fdc5615f54..73eef368ed 100644
--- a/src/ndisc/nm-ndisc.h
+++ b/src/ndisc/nm-ndisc.h
@@ -58,6 +58,9 @@ typedef enum {
 	NM_NDISC_DHCP_LEVEL_MANAGED
 } NMNDiscDHCPLevel;
 
+/* we rely on the fact that NM_NDISC_INFINITY is the largest possible
+ * time duration (G_MAXUINT32) and that the range of finite values
+ * goes from 0 to G_MAXUINT32-1. */
 #define NM_NDISC_INFINITY  G_MAXUINT32
 
 struct _NMNDiscGateway {
diff --git a/src/ndisc/tests/test-ndisc-fake.c b/src/ndisc/tests/test-ndisc-fake.c
index e99d2fc55e..268f3b49b4 100644
--- a/src/ndisc/tests/test-ndisc-fake.c
+++ b/src/ndisc/tests/test-ndisc-fake.c
@@ -233,8 +233,9 @@ test_everything_changed (NMNDisc *ndisc, const NMNDiscData *rdata, guint changed
 
 		g_assert_cmpint (rdata->gateways_n, ==, 1);
 		match_gateway (rdata, 0, "fe80::2", data->timestamp1, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM);
-		g_assert_cmpint (rdata->addresses_n, ==, 1);
-		match_address (rdata, 0, "2001:db8:a:b::1", data->timestamp1, 10, 10);
+		g_assert_cmpint (rdata->addresses_n, ==, 2);
+		match_address (rdata, 0, "2001:db8:a:a::1", data->timestamp1, 10, 0);
+		match_address (rdata, 1, "2001:db8:a:b::1", data->timestamp1, 10, 10);
 		g_assert_cmpint (rdata->routes_n, ==, 1);
 		match_route (rdata, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp1, 10, 10);
 		g_assert_cmpint (rdata->dns_servers_n, ==, 1);
@@ -384,7 +385,7 @@ test_preference_changed_cb (NMNDisc *ndisc, const NMNDiscData *rdata, guint chan
 		match_gateway (rdata, 0, "fe80::1", data->timestamp1 + 2, 10, NM_ICMPV6_ROUTER_PREF_HIGH);
 		match_gateway (rdata, 1, "fe80::2", data->timestamp1 + 1, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM);
 		g_assert_cmpint (rdata->addresses_n, ==, 2);
-		match_address (rdata, 0, "2001:db8:a:a::1", data->timestamp1 + 2, 10, 10);
+		match_address (rdata, 0, "2001:db8:a:a::1", data->timestamp1 + 3, 9, 9);
 		match_address (rdata, 1, "2001:db8:a:b::1", data->timestamp1 + 1, 10, 10);
 		g_assert_cmpint (rdata->routes_n, ==, 2);
 		match_route (rdata, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1 + 2, 10, 15);