rdisc: prevent solicitation loop for expiring DNS information (rh #1207730) (rh #1151665)

A solicitation loop could result for two cases:

1) a router sent DNS information, then removed that information without
sending it with lifetime=0
2) two routers exist, one sending DNS information and the other not, and
the first router which sends DNS information disappears

In these cases a solicitation would be generated when the DNS information
reached 1/2 its lifetime.  A router would then reply to the solicitation
without DNS information, which would then trigger another lifetime check,
which finds that the DNS info is still 1/2 lifetime.  Which triggers
another solicitation, etc.

Fix this by ensuring that a solicitation is never sent less than
rtr_solicitation_interval seconds after the last one.

src/rdisc/nm-fake-rdisc.c

@@ -333,6 +333,13 @@ start (NMRDisc *rdisc)
 	priv->receive_ra_id = g_timeout_add_seconds (ra->when, receive_ra, rdisc);
 }
 
+void
+nm_fake_rdisc_emit_new_ras (NMFakeRDisc *self)
+{
+	if (!NM_FAKE_RDISC_GET_PRIVATE (self)->receive_ra_id)
+		start (NM_RDISC (self));
+}
+
 /******************************************************************/
 
 NMRDisc *

src/rdisc/nm-fake-rdisc.h

@@ -87,6 +87,8 @@ void nm_fake_rdisc_add_dns_domain (NMFakeRDisc *self,
                                    guint32 timestamp,
                                    guint32 lifetime);
 
+void nm_fake_rdisc_emit_new_ras (NMFakeRDisc *self);
+
 gboolean nm_fake_rdisc_done (NMFakeRDisc *self);
 
 #endif /* __NETWORKMANAGER_FAKE_RDISC_H__ */

src/rdisc/nm-rdisc.c

@@ -35,6 +35,7 @@
 typedef struct {
 	int solicitations_left;
 	guint send_rs_id;
+	gint64 last_rs;
 	guint ra_timeout_id;  /* first RA timeout */
 	guint timeout_id;   /* prefix/dns/etc lifetime timeout */
 } NMRDiscPrivate;
@@ -285,6 +286,7 @@ send_rs (NMRDisc *rdisc)
 	if (klass->send_rs (rdisc))
 		priv->solicitations_left--;
 
+	priv->last_rs = nm_utils_get_monotonic_timestamp_s ();
 	if (priv->solicitations_left > 0) {
 		debug ("(%s): scheduling router solicitation retry in %d seconds.",
 		       rdisc->ifname, rdisc->rtr_solicitation_interval);
@@ -303,11 +305,16 @@ static void
 solicit (NMRDisc *rdisc)
 {
 	NMRDiscPrivate *priv = NM_RDISC_GET_PRIVATE (rdisc);
+	guint32 now = nm_utils_get_monotonic_timestamp_s ();
+	gint64 next;
 
 	if (!priv->send_rs_id) {
-		debug ("(%s): scheduling router solicitation.", rdisc->ifname);
-		priv->send_rs_id = g_idle_add ((GSourceFunc) send_rs, rdisc);
 		priv->solicitations_left = rdisc->rtr_solicitations;
+
+		next = CLAMP (priv->last_rs + rdisc->rtr_solicitation_interval - now, 0, G_MAXINT32);
+		debug ("(%s): scheduling explicit router solicitation request in %" G_GINT64_FORMAT " seconds.",
+		       rdisc->ifname, next);
+		priv->send_rs_id = g_timeout_add_seconds ((guint32) next, (GSourceFunc) send_rs, rdisc);
 	}
 }
 
@@ -590,6 +597,8 @@ dns_domain_free (gpointer data)
 static void
 nm_rdisc_init (NMRDisc *rdisc)
 {
+	NMRDiscPrivate *priv = NM_RDISC_GET_PRIVATE (rdisc);
+
 	rdisc->gateways = g_array_new (FALSE, FALSE, sizeof (NMRDiscGateway));
 	rdisc->addresses = g_array_new (FALSE, FALSE, sizeof (NMRDiscAddress));
 	rdisc->routes = g_array_new (FALSE, FALSE, sizeof (NMRDiscRoute));
@@ -597,6 +606,11 @@ nm_rdisc_init (NMRDisc *rdisc)
 	rdisc->dns_domains = g_array_new (FALSE, FALSE, sizeof (NMRDiscDNSDomain));
 	g_array_set_clear_func (rdisc->dns_domains, dns_domain_free);
 	rdisc->hop_limit = 64;
+
+	/* Start at very low number so that last_rs - rtr_solicitation_interval
+	 * is much lower than nm_utils_get_monotonic_timestamp_s() at startup.
+	 */
+	priv->last_rs = G_MININT32;
 }
 
 static void

src/rdisc/tests/test-rdisc-fake.c

@@ -107,6 +107,8 @@ typedef struct {
 	guint counter;
 	guint rs_counter;
 	guint32 timestamp1;
+	guint32 first_solicit;
+	guint32 timeout_id;
 } TestData;
 
 static void
@@ -344,6 +346,90 @@ test_preference (void)
 	g_main_loop_unref (data.loop);
 }
 
+static void
+test_dns_solicit_loop_changed (NMRDisc *rdisc, NMRDiscConfigMap changed, TestData *data)
+{
+	data->counter++;
+}
+
+static gboolean
+success_timeout (TestData *data)
+{
+	data->timeout_id = 0;
+	g_main_loop_quit (data->loop);
+	return G_SOURCE_REMOVE;
+}
+
+static void
+test_dns_solicit_loop_rs_sent (NMFakeRDisc *rdisc, TestData *data)
+{
+	guint32 now = nm_utils_get_monotonic_timestamp_s ();
+	guint id;
+
+	if (data->rs_counter > 0 && data->rs_counter < 6) {
+		if (data->rs_counter == 1) {
+			data->first_solicit = now;
+			/* Kill the test after 10 seconds if it hasn't failed yet */
+			data->timeout_id = g_timeout_add_seconds (10, (GSourceFunc) success_timeout, data);
+		}
+
+		/* On all but the first solicitation, which should be triggered by the
+		 * DNS servers reaching 1/2 lifetime, emit a new RA without the DNS
+		 * servers again.
+		 */
+		id = nm_fake_rdisc_add_ra (rdisc, 0, NM_RDISC_DHCP_LEVEL_NONE, 4, 1500);
+		g_assert (id);
+		nm_fake_rdisc_add_gateway (rdisc, id, "fe80::1", now, 10, NM_RDISC_PREFERENCE_MEDIUM);
+		nm_fake_rdisc_add_address (rdisc, id, "2001:db8:a:a::1", now, 10, 10);
+
+		nm_fake_rdisc_emit_new_ras (rdisc);
+	} else if (data->rs_counter >= 6) {
+		/* Fail if we've sent too many solicitations in the past 4 seconds */
+		g_assert_cmpint (now - data->first_solicit, >, 4);
+		g_source_remove (data->timeout_id);
+		g_main_loop_quit (data->loop);
+	}
+	data->rs_counter++;
+}
+
+static void
+test_dns_solicit_loop (void)
+{
+	NMFakeRDisc *rdisc = rdisc_new ();
+	guint32 now = nm_utils_get_monotonic_timestamp_s ();
+	TestData data = { g_main_loop_new (NULL, FALSE), 0, 0, now, 0 };
+	guint id;
+
+	/* Ensure that no solicitation loop happens when DNS servers or domains
+	 * stop being sent in advertisements.  This can happen if two routers
+	 * send RAs, but the one sending DNS info stops responding, or if one
+	 * router removes the DNS info from the RA without zero-lifetiming them
+	 * first.
+	 */
+
+	id = nm_fake_rdisc_add_ra (rdisc, 1, NM_RDISC_DHCP_LEVEL_NONE, 4, 1500);
+	g_assert (id);
+	nm_fake_rdisc_add_gateway (rdisc, id, "fe80::1", now, 10, NM_RDISC_PREFERENCE_LOW);
+	nm_fake_rdisc_add_address (rdisc, id, "2001:db8:a:a::1", now, 10, 10);
+	nm_fake_rdisc_add_dns_server (rdisc, id, "2001:db8:c:c::1", now, 6);
+
+	g_signal_connect (rdisc,
+	                  NM_RDISC_CONFIG_CHANGED,
+	                  G_CALLBACK (test_dns_solicit_loop_changed),
+	                  &data);
+	g_signal_connect (rdisc,
+	                  NM_FAKE_RDISC_RS_SENT,
+	                  G_CALLBACK (test_dns_solicit_loop_rs_sent),
+	                  &data);
+
+	nm_rdisc_start (NM_RDISC (rdisc));
+	g_main_loop_run (data.loop);
+	g_assert_cmpint (data.counter, ==, 3);
+
+	g_object_unref (rdisc);
+	g_main_loop_unref (data.loop);
+}
+
 NMTST_DEFINE ();
 
 int
@@ -361,6 +447,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/rdisc/simple", test_simple);
 	g_test_add_func ("/rdisc/everything-changed", test_everything);
 	g_test_add_func ("/rdisc/preference-changed", test_preference);
+	g_test_add_func ("/rdisc/dns-solicit-loop", test_dns_solicit_loop);
 
 	return g_test_run ();
 }