From 6b18fc252d1e31399b9aef3049a78d9a9b129e00 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 21 Jan 2021 18:37:38 +0100 Subject: [PATCH 01/11] shared: add nm_g_{idle,timeout}_add_source() helpers We have g_idle_add() and g_timeout_add(). But these return those odd guint source ids. That is totally pointless. The only potential benefit is that a guint is only 4 bytes while a pointer is 8 bytes (on 64 bit systems). Otherwise, it seems always preferable to have an actual GSource instance instead of an integer. It also saves the overhead in g_source_remove() which first needs to do a hash lookup to find the GSource. A GSource instance would theoretically work with multiple GMainContext instances, while g_source_remove() only works wit g_main_context_default(). On the other hand we have helper API like nm_g_idle_source_new() and nm_g_timeout_source_new(), which is fully flexible and sensible because it returns a reference to the GSource instance. However, it is a bit verbose to use in the common case. Add helper functions that simplify the use and are conceptionally similar to g_{idle,timeout}_add() (hence the name). --- shared/nm-glib-aux/nm-shared-utils.h | 53 ++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 8f0d50c9f5..c432b125b6 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -1364,6 +1364,59 @@ nm_g_source_attach(GSource *source, GMainContext *context) return source; } +static inline GSource * +nm_g_idle_add_source(GSourceFunc func, gpointer user_data) +{ + /* G convenience function to attach a new timeout source to the default GMainContext. + * In that sense it's very similar to g_idle_add() except that it returns a + * reference to the new source. */ + return nm_g_source_attach(nm_g_idle_source_new(G_PRIORITY_DEFAULT, func, user_data, NULL), + NULL); +} + +static inline GSource * +nm_g_timeout_add_source(guint timeout_msec, GSourceFunc func, gpointer user_data) +{ + /* G convenience function to attach a new timeout source to the default GMainContext. + * In that sense it's very similar to g_timeout_add() except that it returns a + * reference to the new source. */ + return nm_g_source_attach( + nm_g_timeout_source_new(timeout_msec, G_PRIORITY_DEFAULT, func, user_data, NULL), + NULL); +} + +static inline GSource * +nm_g_timeout_add_source_seconds(guint timeout_sec, GSourceFunc func, gpointer user_data) +{ + /* G convenience function to attach a new timeout source to the default GMainContext. + * In that sense it's very similar to g_timeout_add_seconds() except that it returns a + * reference to the new source. */ + return nm_g_source_attach( + nm_g_timeout_source_new_seconds(timeout_sec, G_PRIORITY_DEFAULT, func, user_data, NULL), + NULL); +} + +static inline GSource * +nm_g_timeout_add_source_approx(guint timeout_msec, + guint timeout_sec_threshold, + GSourceFunc func, + gpointer user_data) +{ + GSource *source; + + /* If timeout_msec is larger or equal than a threshold, then we use g_timeout_source_new_seconds() + * instead. */ + if (timeout_msec / 1000u >= timeout_sec_threshold) + source = nm_g_timeout_source_new_seconds(timeout_msec / 1000u, + G_PRIORITY_DEFAULT, + func, + user_data, + NULL); + else + source = nm_g_timeout_source_new(timeout_msec, G_PRIORITY_DEFAULT, func, user_data, NULL); + return nm_g_source_attach(source, NULL); +} + NM_AUTO_DEFINE_FCN0(GMainContext *, _nm_auto_unref_gmaincontext, g_main_context_unref); #define nm_auto_unref_gmaincontext nm_auto(_nm_auto_unref_gmaincontext) From 779ee32263c41855623433e23a545f6f0339b1d3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 22 Jan 2021 13:04:31 +0100 Subject: [PATCH 02/11] shared/tests: add nmtst_main_loop_run_assert() helper --- shared/nm-utils/nm-test-utils.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h index 83be77c6bf..66d5ae2796 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1194,6 +1194,14 @@ nmtst_main_loop_run(GMainLoop *loop, guint timeout_msec) return loopx != NULL; } +#define nmtst_main_loop_run_assert(loop, timeout_msec) \ + G_STMT_START \ + { \ + if (!nmtst_main_loop_run((loop), (timeout_msec))) \ + g_assert_not_reached(); \ + } \ + G_STMT_END + static inline void _nmtst_main_loop_quit_on_notify(GObject *object, GParamSpec *pspec, gpointer user_data) { From 8a3310043b15ba861b6821d005decf665784a9e8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 21 Jan 2021 08:14:37 +0100 Subject: [PATCH 03/11] ndisc: add static asserts to _route_preference_coerce() Our internal NMIcmpv6RouterPref defines must be numerically identical to the values in the protocol. Add a static assertion for that. --- src/ndisc/nm-lndp-ndisc.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c index 1d8becbfec..6f9ac9dcab 100644 --- a/src/ndisc/nm-lndp-ndisc.c +++ b/src/ndisc/nm-lndp-ndisc.c @@ -85,14 +85,26 @@ send_rs(NMNDisc *ndisc, GError **error) static NMIcmpv6RouterPref _route_preference_coerce(enum ndp_route_preference pref) { +#define _ASSERT_ENUM(v1, v2) \ + G_STMT_START \ + { \ + G_STATIC_ASSERT((NMIcmpv6RouterPref)(v1) == (v2)); \ + G_STATIC_ASSERT((enum ndp_route_preference)(v2) == (v1)); \ + G_STATIC_ASSERT((gint64)(v1) == (v2)); \ + G_STATIC_ASSERT((gint64)(v2) == (v1)); \ + } \ + G_STMT_END + switch (pref) { case NDP_ROUTE_PREF_LOW: - return NM_ICMPV6_ROUTER_PREF_LOW; case NDP_ROUTE_PREF_MEDIUM: - return NM_ICMPV6_ROUTER_PREF_MEDIUM; case NDP_ROUTE_PREF_HIGH: - return NM_ICMPV6_ROUTER_PREF_HIGH; + _ASSERT_ENUM(NDP_ROUTE_PREF_LOW, NM_ICMPV6_ROUTER_PREF_LOW); + _ASSERT_ENUM(NDP_ROUTE_PREF_MEDIUM, NM_ICMPV6_ROUTER_PREF_MEDIUM); + _ASSERT_ENUM(NDP_ROUTE_PREF_HIGH, NM_ICMPV6_ROUTER_PREF_HIGH); + return (NMIcmpv6RouterPref) pref; } + /* unexpected value must be treated as MEDIUM (RFC 4191). */ return NM_ICMPV6_ROUTER_PREF_MEDIUM; } From de9e570cb1f15050b92acde4c896afcfde943636 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 21 Jan 2021 08:56:43 +0100 Subject: [PATCH 04/11] ndisc: mark NMIcmpv6RouterPref enum as _nm_packed We embed values of this type in structs. Let's pack it to only use one byte. --- src/nm-core-utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h index f4fe5f673c..a3f47874fd 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -559,7 +559,7 @@ char * nm_utils_format_con_diff_for_audit(GHashTable *diff); /* this enum is compatible with ICMPV6_ROUTER_PREF_* (from , * the values for netlink attribute RTA_PREF) and "enum ndp_route_preference" * from . */ -typedef enum { +typedef enum _nm_packed { NM_ICMPV6_ROUTER_PREF_MEDIUM = 0x0, /* ICMPV6_ROUTER_PREF_MEDIUM */ NM_ICMPV6_ROUTER_PREF_LOW = 0x3, /* ICMPV6_ROUTER_PREF_LOW */ NM_ICMPV6_ROUTER_PREF_HIGH = 0x1, /* ICMPV6_ROUTER_PREF_HIGH */ From f892fce04ff4173d9b827d4214a95ebd49e2f20d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 22 Jan 2021 13:05:11 +0100 Subject: [PATCH 05/11] ndisc/tests: use nmtst_main_loop_run_assert() to ensure we terminate Otherwise, if there is a problem with the test they will run indefinitely. Sure, meson will kill them after a while, but I don't think autotools does, does it? Anyway, give a maximum time to wait. --- src/ndisc/tests/test-ndisc-fake.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ndisc/tests/test-ndisc-fake.c b/src/ndisc/tests/test-ndisc-fake.c index 95caec5bd4..62eb877720 100644 --- a/src/ndisc/tests/test-ndisc-fake.c +++ b/src/ndisc/tests/test-ndisc-fake.c @@ -181,7 +181,7 @@ test_simple(void) g_signal_connect(ndisc, NM_NDISC_CONFIG_RECEIVED, G_CALLBACK(test_simple_changed), &data); nm_ndisc_start(NM_NDISC(ndisc)); - g_main_loop_run(data.loop); + nmtst_main_loop_run_assert(data.loop, 15000); g_assert_cmpint(data.counter, ==, 1); g_object_unref(ndisc); @@ -273,7 +273,7 @@ test_everything(void) g_signal_connect(ndisc, NM_FAKE_NDISC_RS_SENT, G_CALLBACK(test_everything_rs_sent), &data); nm_ndisc_start(NM_NDISC(ndisc)); - g_main_loop_run(data.loop); + nmtst_main_loop_run_assert(data.loop, 15000); g_assert_cmpint(data.counter, ==, 2); g_assert_cmpint(data.rs_counter, ==, 1); @@ -335,7 +335,7 @@ test_preference_order(void) g_signal_connect(ndisc, NM_NDISC_CONFIG_RECEIVED, G_CALLBACK(test_preference_order_cb), &data); nm_ndisc_start(NM_NDISC(ndisc)); - g_main_loop_run(data.loop); + nmtst_main_loop_run_assert(data.loop, 15000); g_assert_cmpint(data.counter, ==, 2); g_object_unref(ndisc); @@ -421,7 +421,7 @@ test_preference_changed(void) &data); nm_ndisc_start(NM_NDISC(ndisc)); - g_main_loop_run(data.loop); + nmtst_main_loop_run_assert(data.loop, 15000); g_assert_cmpint(data.counter, ==, 3); g_object_unref(ndisc); @@ -506,7 +506,7 @@ test_dns_solicit_loop(void) &data); nm_ndisc_start(NM_NDISC(ndisc)); - g_main_loop_run(data.loop); + nmtst_main_loop_run_assert(data.loop, 20000); g_assert_cmpint(data.counter, ==, 3); g_object_unref(ndisc); From 03c6d8280cbe115c9214f431410e4bcd2cc65fbd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 21 Jan 2021 16:48:35 +0100 Subject: [PATCH 06/11] ndisc: don't call solicit_routers() from clean_dns_*() functions This was done since NDisc code was added to NetworkManager in commit c3a4656a68f9 ('rdisc: libndp implementation'). Note what it does: in clean_dns_*() we will call solicit_router() if the half-life of any entity is expired. That doesn't seem right. Why only for dns_servers and dns_domains, but not routes, addresses and gateways? Also, why would the timings for when we solicit depend on when elements expire. It is "normal" that some of them will expire. We should solicit based on other parameters, like keeping track of when and how to solicit. Note that there is a change in behavior here: if we stopped soliciting (either because we received our first RA or because we run out of retries), then we now will never start again. Previously this was a mechanism so that we would eventually start soliciting again. This will be fixed in a follow-up commit soon. --- src/ndisc/nm-fake-ndisc.c | 1 + src/ndisc/nm-ndisc.c | 45 +++++++------------------------ src/ndisc/tests/test-ndisc-fake.c | 3 +++ 3 files changed, 14 insertions(+), 35 deletions(-) diff --git a/src/ndisc/nm-fake-ndisc.c b/src/ndisc/nm-fake-ndisc.c index 3771b404fb..634a48c2f3 100644 --- a/src/ndisc/nm-fake-ndisc.c +++ b/src/ndisc/nm-fake-ndisc.c @@ -222,6 +222,7 @@ nm_fake_ndisc_done(NMFakeNDisc *self) static gboolean send_rs(NMNDisc *ndisc, GError **error) { + _LOGT("send_rs()"); g_signal_emit(ndisc, signals[RS_SENT], 0); return TRUE; } diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c index 44035ff909..0ac10b8808 100644 --- a/src/ndisc/nm-ndisc.c +++ b/src/ndisc/nm-ndisc.c @@ -241,15 +241,6 @@ get_expiry_time(guint32 timestamp, guint32 lifetime) get_expiry_time(_item->timestamp, _item->lifetime); \ }) -#define get_expiry_half(item) \ - ({ \ - typeof(item) _item = (item); \ - nm_assert(_item); \ - (_item->lifetime == NM_NDISC_INFINITY) \ - ? _EXPIRY_INFINITY \ - : get_expiry_time(_item->timestamp, _item->lifetime / 2); \ - }) - #define get_expiry_preferred(item) \ ({ \ typeof(item) _item = (item); \ @@ -1328,21 +1319,13 @@ 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; - 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; - } - - if (now >= refresh) - solicit_routers(ndisc); - else if (*nextevent > refresh) - *nextevent = refresh; + if (!expiry_next(now, get_expiry(item), nextevent)) { + g_array_remove_index(rdata->dns_servers, i); + *changed |= NM_NDISC_CONFIG_DNS_SERVERS; + continue; } + i++; } } @@ -1357,21 +1340,13 @@ 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; - 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; - } - - if (now >= refresh) - solicit_routers(ndisc); - else if (*nextevent > refresh) - *nextevent = refresh; + if (!expiry_next(now, get_expiry(item), nextevent)) { + g_array_remove_index(rdata->dns_domains, i); + *changed |= NM_NDISC_CONFIG_DNS_DOMAINS; + continue; } + i++; } } diff --git a/src/ndisc/tests/test-ndisc-fake.c b/src/ndisc/tests/test-ndisc-fake.c index 62eb877720..7deec0ca34 100644 --- a/src/ndisc/tests/test-ndisc-fake.c +++ b/src/ndisc/tests/test-ndisc-fake.c @@ -484,6 +484,9 @@ test_dns_solicit_loop(void) TestData data = {g_main_loop_new(NULL, FALSE), 0, 0, now, 0}; guint id; + g_test_skip("The solicitation behavior is wrong and need fixing. This test is not working too"); + return; + /* 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 From 4c2035347ec5eaaeee9320944ba07748755cd047 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 21 Jan 2021 18:19:15 +0100 Subject: [PATCH 07/11] ndisc: track expiry of Router Advertisements in milliseconds Elements of RAs have a lifetime. Previously we would track both the timestamp (when we received the RA) and the lifetime. However, we are mainly interested in the expiry time. So tracking the expiry in form of timestamp and lifetime is redundant and cumbersome to use. Consider also the cases nm_ndisc_add_address() were we mangle the expiry. In that case, the timestamp becomes meaningless or it's not clear what the timestamp should be. Also, there are no real cases where we actually need the receive timestamp. Note that when we convert the times to NMPlatformIP6Address, we again need to synthesize a base time stamp. But here too, it's NMPlatformIP6Address fault of doing this pointless split of timestamp and lifetime. While at it, increase the precision to milliseconds. As we receive lifetimes with seconds precision, one might think that seconds precision is enough for tracking the timeouts. However it just leads to ugly uncertainty about rounding, when we can track times with sufficient precision without downside. For example, before configuring an address in kernel, we also need to calculate a remaining lifetime with a lower precision. By having the exact values, we can do so more accurately. At least, in theory. Of course NMPlatformIP6Address itself has only precision of seconds, we already loose the information before. However, NMNDisc no longer has that problem. --- src/devices/nm-device.c | 59 ++- src/ndisc/nm-fake-ndisc.c | 137 ++++--- src/ndisc/nm-fake-ndisc.h | 14 +- src/ndisc/nm-lndp-ndisc.c | 128 ++++--- src/ndisc/nm-ndisc-private.h | 13 +- src/ndisc/nm-ndisc.c | 598 ++++++++++++++++-------------- src/ndisc/nm-ndisc.h | 96 +++-- src/ndisc/tests/test-ndisc-fake.c | 510 ++++++++++++++++--------- src/nm-ip6-config.c | 21 +- 9 files changed, 910 insertions(+), 666 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 2f4ad2ad24..574f8c6476 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -5148,26 +5148,26 @@ device_recheck_slave_status(NMDevice *self, const NMPlatformLink *plink) static void ndisc_set_router_config(NMNDisc *ndisc, NMDevice *self) { - NMDevicePrivate * priv = NM_DEVICE_GET_PRIVATE(self); - gint32 now; - GArray * addresses, *dns_servers, *dns_domains; - guint len, i; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); + gs_unref_array GArray *addresses = NULL; + gs_unref_array GArray *dns_servers = NULL; + gs_unref_array GArray * dns_domains = NULL; + guint len; + guint i; const NMDedupMultiHeadEntry *head_entry; NMDedupMultiIter ipconf_iter; if (nm_ndisc_get_node_type(ndisc) != NM_NDISC_NODE_TYPE_ROUTER) return; - now = nm_utils_get_monotonic_timestamp_sec(); - head_entry = nm_ip6_config_lookup_addresses(priv->ip_config_6); addresses = g_array_sized_new(FALSE, TRUE, sizeof(NMNDiscAddress), head_entry ? head_entry->len : 0); nm_dedup_multi_iter_for_each (&ipconf_iter, head_entry) { const NMPlatformIP6Address *addr = NMP_OBJECT_CAST_IP6_ADDRESS(ipconf_iter.current->obj); NMNDiscAddress * ndisc_addr; - guint32 lifetime, preferred; - gint32 base; + guint32 lifetime; + guint32 preferred; if (IN6_IS_ADDR_UNSPECIFIED(&addr->address) || IN6_IS_ADDR_LINKLOCAL(&addr->address)) continue; @@ -5178,31 +5178,21 @@ ndisc_set_router_config(NMNDisc *ndisc, NMDevice *self) if (addr->plen != 64) continue; - /* resolve the timestamps relative to a new base. - * - * Note that for convenience, platform @addr might have timestamp and/or - * lifetime unset. We don't allow that flexibility for ndisc and require - * well defined timestamps. */ - if (addr->timestamp) { - nm_assert(addr->timestamp < G_MAXINT32); - base = addr->timestamp; - } else - base = now; - lifetime = nm_utils_lifetime_get(addr->timestamp, addr->lifetime, addr->preferred, - base, + NM_NDISC_EXPIRY_BASE_TIMESTAMP / 1000, &preferred); if (!lifetime) continue; g_array_set_size(addresses, addresses->len + 1); - ndisc_addr = &g_array_index(addresses, NMNDiscAddress, addresses->len - 1); - ndisc_addr->address = addr->address; - ndisc_addr->timestamp = base; - ndisc_addr->lifetime = lifetime; - ndisc_addr->preferred = preferred; + ndisc_addr = &g_array_index(addresses, NMNDiscAddress, addresses->len - 1); + ndisc_addr->address = addr->address; + ndisc_addr->expiry_msec = + _nm_ndisc_lifetime_to_expiry(NM_NDISC_EXPIRY_BASE_TIMESTAMP, lifetime); + ndisc_addr->expiry_preferred_msec = + _nm_ndisc_lifetime_to_expiry(NM_NDISC_EXPIRY_BASE_TIMESTAMP, preferred); } len = nm_ip6_config_get_num_nameservers(priv->ip_config_6); @@ -5212,10 +5202,10 @@ ndisc_set_router_config(NMNDisc *ndisc, NMDevice *self) const struct in6_addr *nameserver = nm_ip6_config_get_nameserver(priv->ip_config_6, i); NMNDiscDNSServer * ndisc_nameserver; - ndisc_nameserver = &g_array_index(dns_servers, NMNDiscDNSServer, i); - ndisc_nameserver->address = *nameserver; - ndisc_nameserver->timestamp = now; - ndisc_nameserver->lifetime = NM_NDISC_ROUTER_LIFETIME; + ndisc_nameserver = &g_array_index(dns_servers, NMNDiscDNSServer, i); + ndisc_nameserver->address = *nameserver; + ndisc_nameserver->expiry_msec = + _nm_ndisc_lifetime_to_expiry(NM_NDISC_EXPIRY_BASE_TIMESTAMP, NM_NDISC_ROUTER_LIFETIME); } len = nm_ip6_config_get_num_searches(priv->ip_config_6); @@ -5225,16 +5215,13 @@ ndisc_set_router_config(NMNDisc *ndisc, NMDevice *self) const char * search = nm_ip6_config_get_search(priv->ip_config_6, i); NMNDiscDNSDomain *ndisc_search; - ndisc_search = &g_array_index(dns_domains, NMNDiscDNSDomain, i); - ndisc_search->domain = (char *) search; - ndisc_search->timestamp = now; - ndisc_search->lifetime = NM_NDISC_ROUTER_LIFETIME; + ndisc_search = &g_array_index(dns_domains, NMNDiscDNSDomain, i); + ndisc_search->domain = (char *) search; + ndisc_search->expiry_msec = + _nm_ndisc_lifetime_to_expiry(NM_NDISC_EXPIRY_BASE_TIMESTAMP, NM_NDISC_ROUTER_LIFETIME); } nm_ndisc_set_config(ndisc, addresses, dns_servers, dns_domains); - g_array_unref(addresses); - g_array_unref(dns_servers); - g_array_unref(dns_domains); } static void diff --git a/src/ndisc/nm-fake-ndisc.c b/src/ndisc/nm-fake-ndisc.c index 634a48c2f3..e9b329385e 100644 --- a/src/ndisc/nm-fake-ndisc.c +++ b/src/ndisc/nm-fake-ndisc.c @@ -30,11 +30,10 @@ typedef struct { typedef struct { struct in6_addr network; - int plen; struct in6_addr gateway; - guint32 timestamp; - guint32 lifetime; - guint32 preferred; + gint64 expiry_msec; + gint64 expiry_preferred_msec; + int plen; NMIcmpv6RouterPref preference; } FakePrefix; @@ -128,8 +127,7 @@ void nm_fake_ndisc_add_gateway(NMFakeNDisc * self, guint ra_id, const char * addr, - guint32 timestamp, - guint32 lifetime, + gint64 expiry_msec, NMIcmpv6RouterPref preference) { NMFakeNDiscPrivate *priv = NM_FAKE_NDISC_GET_PRIVATE(self); @@ -137,12 +135,12 @@ nm_fake_ndisc_add_gateway(NMFakeNDisc * self, NMNDiscGateway * gw; g_assert(ra); - g_array_set_size(ra->gateways, ra->gateways->len + 1); - gw = &g_array_index(ra->gateways, NMNDiscGateway, ra->gateways->len - 1); - g_assert(inet_pton(AF_INET6, addr, &gw->address) == 1); - gw->timestamp = timestamp; - gw->lifetime = lifetime; - gw->preference = preference; + + gw = nm_g_array_append_new(ra->gateways, NMNDiscGateway); + if (inet_pton(AF_INET6, addr, &gw->address) != 1) + g_assert_not_reached(); + gw->expiry_msec = expiry_msec; + gw->preference = preference; } void @@ -151,9 +149,8 @@ nm_fake_ndisc_add_prefix(NMFakeNDisc * self, const char * network, guint plen, const char * gateway, - guint32 timestamp, - guint32 lifetime, - guint32 preferred, + gint64 expiry_msec, + gint64 expiry_preferred_msec, NMIcmpv6RouterPref preference) { NMFakeNDiscPrivate *priv = NM_FAKE_NDISC_GET_PRIVATE(self); @@ -161,54 +158,52 @@ nm_fake_ndisc_add_prefix(NMFakeNDisc * self, FakePrefix * prefix; g_assert(ra); - g_array_set_size(ra->prefixes, ra->prefixes->len + 1); - prefix = &g_array_index(ra->prefixes, FakePrefix, ra->prefixes->len - 1); - memset(prefix, 0, sizeof(*prefix)); - g_assert(inet_pton(AF_INET6, network, &prefix->network) == 1); - g_assert(inet_pton(AF_INET6, gateway, &prefix->gateway) == 1); - prefix->plen = plen; - prefix->timestamp = timestamp; - prefix->lifetime = lifetime; - prefix->preferred = preferred; - prefix->preference = preference; + + prefix = nm_g_array_append_new(ra->prefixes, FakePrefix); + *prefix = (FakePrefix){ + .plen = plen, + .expiry_msec = expiry_msec, + .expiry_preferred_msec = expiry_preferred_msec, + .preference = preference, + }; + if (inet_pton(AF_INET6, network, &prefix->network) != 1) + g_assert_not_reached(); + if (inet_pton(AF_INET6, gateway, &prefix->gateway) != 1) + g_assert_not_reached(); } void nm_fake_ndisc_add_dns_server(NMFakeNDisc *self, guint ra_id, const char * address, - guint32 timestamp, - guint32 lifetime) + gint64 expiry_msec) { NMFakeNDiscPrivate *priv = NM_FAKE_NDISC_GET_PRIVATE(self); FakeRa * ra = find_ra(priv->ras, ra_id); NMNDiscDNSServer * dns; g_assert(ra); - g_array_set_size(ra->dns_servers, ra->dns_servers->len + 1); - dns = &g_array_index(ra->dns_servers, NMNDiscDNSServer, ra->dns_servers->len - 1); - g_assert(inet_pton(AF_INET6, address, &dns->address) == 1); - dns->timestamp = timestamp; - dns->lifetime = lifetime; + + dns = nm_g_array_append_new(ra->dns_servers, NMNDiscDNSServer); + + dns->expiry_msec = expiry_msec; + if (inet_pton(AF_INET6, address, &dns->address) != 1) + g_assert_not_reached(); } void -nm_fake_ndisc_add_dns_domain(NMFakeNDisc *self, - guint ra_id, - const char * domain, - guint32 timestamp, - guint32 lifetime) +nm_fake_ndisc_add_dns_domain(NMFakeNDisc *self, guint ra_id, const char *domain, gint64 expiry_msec) { NMFakeNDiscPrivate *priv = NM_FAKE_NDISC_GET_PRIVATE(self); FakeRa * ra = find_ra(priv->ras, ra_id); NMNDiscDNSDomain * dns; g_assert(ra); - g_array_set_size(ra->dns_domains, ra->dns_domains->len + 1); - dns = &g_array_index(ra->dns_domains, NMNDiscDNSDomain, ra->dns_domains->len - 1); - dns->domain = g_strdup(domain); - dns->timestamp = timestamp; - dns->lifetime = lifetime; + + dns = nm_g_array_append_new(ra->dns_domains, NMNDiscDNSDomain); + + dns->domain = g_strdup(domain); + dns->expiry_msec = expiry_msec; } gboolean @@ -230,13 +225,13 @@ send_rs(NMNDisc *ndisc, GError **error) static gboolean receive_ra(gpointer user_data) { - NMFakeNDisc * self = user_data; - NMFakeNDiscPrivate * priv = NM_FAKE_NDISC_GET_PRIVATE(self); - NMNDisc * ndisc = NM_NDISC(self); - NMNDiscDataInternal *rdata = ndisc->rdata; - FakeRa * ra = priv->ras->data; - NMNDiscConfigMap changed = 0; - gint32 now = nm_utils_get_monotonic_timestamp_sec(); + NMFakeNDisc * self = user_data; + NMFakeNDiscPrivate * priv = NM_FAKE_NDISC_GET_PRIVATE(self); + NMNDisc * ndisc = NM_NDISC(self); + NMNDiscDataInternal *rdata = ndisc->rdata; + FakeRa * ra = priv->ras->data; + NMNDiscConfigMap changed = 0; + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); guint i; NMNDiscDHCPLevel dhcp_level; @@ -251,53 +246,51 @@ receive_ra(gpointer user_data) } for (i = 0; i < ra->gateways->len; i++) { - NMNDiscGateway *item = &g_array_index(ra->gateways, NMNDiscGateway, i); + const NMNDiscGateway *item = &g_array_index(ra->gateways, NMNDiscGateway, i); - if (nm_ndisc_add_gateway(ndisc, item)) + if (nm_ndisc_add_gateway(ndisc, item, now_msec)) changed |= NM_NDISC_CONFIG_GATEWAYS; } for (i = 0; i < ra->prefixes->len; i++) { - FakePrefix * item = &g_array_index(ra->prefixes, FakePrefix, i); - NMNDiscRoute route = { - .network = item->network, - .plen = item->plen, - .gateway = item->gateway, - .timestamp = item->timestamp, - .lifetime = item->lifetime, - .preference = item->preference, + FakePrefix * item = &g_array_index(ra->prefixes, FakePrefix, i); + const NMNDiscRoute route = { + .network = item->network, + .plen = item->plen, + .gateway = item->gateway, + .expiry_msec = item->expiry_msec, + .preference = item->preference, }; g_assert(route.plen > 0 && route.plen <= 128); - if (nm_ndisc_add_route(ndisc, &route)) + if (nm_ndisc_add_route(ndisc, &route, now_msec)) changed |= NM_NDISC_CONFIG_ROUTES; if (item->plen == 64) { - NMNDiscAddress address = { - .address = item->network, - .timestamp = item->timestamp, - .lifetime = item->lifetime, - .preferred = item->preferred, - .dad_counter = 0, + const NMNDiscAddress address = { + .address = item->network, + .expiry_msec = item->expiry_msec, + .expiry_preferred_msec = item->expiry_preferred_msec, + .dad_counter = 0, }; - if (nm_ndisc_complete_and_add_address(ndisc, &address, now)) + if (nm_ndisc_complete_and_add_address(ndisc, &address, now_msec)) changed |= NM_NDISC_CONFIG_ADDRESSES; } } for (i = 0; i < ra->dns_servers->len; i++) { - NMNDiscDNSServer *item = &g_array_index(ra->dns_servers, NMNDiscDNSServer, i); + const NMNDiscDNSServer *item = &g_array_index(ra->dns_servers, NMNDiscDNSServer, i); - if (nm_ndisc_add_dns_server(ndisc, item)) + if (nm_ndisc_add_dns_server(ndisc, item, now_msec)) changed |= NM_NDISC_CONFIG_DNS_SERVERS; } for (i = 0; i < ra->dns_domains->len; i++) { - NMNDiscDNSDomain *item = &g_array_index(ra->dns_domains, NMNDiscDNSDomain, i); + const NMNDiscDNSDomain *item = &g_array_index(ra->dns_domains, NMNDiscDNSDomain, i); - if (nm_ndisc_add_dns_domain(ndisc, item)) + if (nm_ndisc_add_dns_domain(ndisc, item, now_msec)) changed |= NM_NDISC_CONFIG_DNS_DOMAINS; } @@ -314,7 +307,7 @@ receive_ra(gpointer user_data) priv->ras = g_slist_remove(priv->ras, priv->ras->data); fake_ra_free(ra); - nm_ndisc_ra_received(NM_NDISC(self), now, changed); + nm_ndisc_ra_received(NM_NDISC(self), now_msec, changed); /* Schedule next RA */ if (priv->ras) { diff --git a/src/ndisc/nm-fake-ndisc.h b/src/ndisc/nm-fake-ndisc.h index b393b593d3..677f15fcac 100644 --- a/src/ndisc/nm-fake-ndisc.h +++ b/src/ndisc/nm-fake-ndisc.h @@ -35,8 +35,7 @@ guint nm_fake_ndisc_add_ra(NMFakeNDisc * self, void nm_fake_ndisc_add_gateway(NMFakeNDisc * self, guint ra_id, const char * addr, - guint32 timestamp, - guint32 lifetime, + gint64 expiry_msec, NMIcmpv6RouterPref preference); void nm_fake_ndisc_add_prefix(NMFakeNDisc * self, @@ -44,22 +43,19 @@ void nm_fake_ndisc_add_prefix(NMFakeNDisc * self, const char * network, guint plen, const char * gateway, - guint32 timestamp, - guint32 lifetime, - guint32 preferred, + gint64 expiry_msec, + gint64 expiry_preferred_msec, NMIcmpv6RouterPref preference); void nm_fake_ndisc_add_dns_server(NMFakeNDisc *self, guint ra_id, const char * address, - guint32 timestamp, - guint32 lifetime); + gint64 expiry_msec); void nm_fake_ndisc_add_dns_domain(NMFakeNDisc *self, guint ra_id, const char * domain, - guint32 timestamp, - guint32 lifetime); + gint64 expiry_msec); void nm_fake_ndisc_emit_new_ras(NMFakeNDisc *self); diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c index 6f9ac9dcab..0a1a5f2274 100644 --- a/src/ndisc/nm-lndp-ndisc.c +++ b/src/ndisc/nm-lndp-ndisc.c @@ -117,7 +117,7 @@ receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) NMNDiscConfigMap changed = 0; struct ndp_msgra * msgra = ndp_msgra(msg); struct in6_addr gateway_addr; - gint32 now = nm_utils_get_monotonic_timestamp_sec(); + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); int offset; int hop_limit; guint32 val; @@ -133,7 +133,9 @@ receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) * single time when the configuration is finished and updates can * come at any time. */ - _LOGD("received router advertisement at %d", (int) now); + _LOGD("received router advertisement at timestamp %" G_GINT64_FORMAT ".%03d seconds", + now_msec / 1000, + (int) (now_msec % 1000)); gateway_addr = *ndp_msg_addrto(msg); if (IN6_IS_ADDR_UNSPECIFIED(&gateway_addr)) @@ -175,13 +177,18 @@ receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) */ { const NMNDiscGateway gateway = { - .address = gateway_addr, - .timestamp = now, - .lifetime = ndp_msgra_router_lifetime(msgra), - .preference = _route_preference_coerce(ndp_msgra_route_preference(msgra)), + .address = gateway_addr, + .expiry_msec = _nm_ndisc_lifetime_to_expiry(now_msec, ndp_msgra_router_lifetime(msgra)), + .preference = _route_preference_coerce(ndp_msgra_route_preference(msgra)), }; - if (nm_ndisc_add_gateway(ndisc, &gateway)) + /* https://tools.ietf.org/html/rfc2461#section-4.2 + * > A Lifetime of 0 indicates that the router is not a + * > default router and SHOULD NOT appear on the default + * > router list. + * We handle that by tracking a gateway that expires right now. */ + + if (nm_ndisc_add_gateway(ndisc, &gateway, now_msec)) changed |= NM_NDISC_CONFIG_GATEWAYS; } @@ -204,64 +211,71 @@ receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) if (ndp_msg_opt_prefix_flag_on_link(msg, offset)) { const NMNDiscRoute route = { - .network = r_network, - .plen = r_plen, - .timestamp = now, - .lifetime = ndp_msg_opt_prefix_valid_time(msg, offset), + .network = r_network, + .plen = r_plen, + .expiry_msec = + _nm_ndisc_lifetime_to_expiry(now_msec, + ndp_msg_opt_prefix_valid_time(msg, offset)), }; - if (nm_ndisc_add_route(ndisc, &route)) + if (nm_ndisc_add_route(ndisc, &route, now_msec)) changed |= NM_NDISC_CONFIG_ROUTES; } /* Address */ if (r_plen == 64 && ndp_msg_opt_prefix_flag_auto_addr_conf(msg, offset)) { - NMNDiscAddress address = { - .address = r_network, - .timestamp = now, - .lifetime = ndp_msg_opt_prefix_valid_time(msg, offset), - .preferred = ndp_msg_opt_prefix_preferred_time(msg, offset), + const guint32 valid_time = ndp_msg_opt_prefix_valid_time(msg, offset); + const guint32 preferred_time = + NM_MIN(ndp_msg_opt_prefix_preferred_time(msg, offset), valid_time); + const NMNDiscAddress address = { + .address = r_network, + .expiry_msec = _nm_ndisc_lifetime_to_expiry(now_msec, valid_time), + .expiry_preferred_msec = _nm_ndisc_lifetime_to_expiry(now_msec, preferred_time), }; - if (address.preferred <= address.lifetime) { - if (nm_ndisc_complete_and_add_address(ndisc, &address, now)) - changed |= NM_NDISC_CONFIG_ADDRESSES; - } + if (nm_ndisc_complete_and_add_address(ndisc, &address, now_msec)) + changed |= NM_NDISC_CONFIG_ADDRESSES; } } ndp_msg_opt_for_each_offset (offset, msg, NDP_MSG_OPT_ROUTE) { - NMNDiscRoute route = { - .gateway = gateway_addr, - .plen = ndp_msg_opt_route_prefix_len(msg, offset), - .timestamp = now, - .lifetime = ndp_msg_opt_route_lifetime(msg, offset), - .preference = _route_preference_coerce(ndp_msg_opt_route_preference(msg, offset)), - }; + guint8 plen = ndp_msg_opt_route_prefix_len(msg, offset); + struct in6_addr network; - if (route.plen == 0 || route.plen > 128) + if (plen == 0 || plen > 128) continue; - /* Routers through this particular gateway */ - nm_utils_ip6_address_clear_host_address(&route.network, + nm_utils_ip6_address_clear_host_address(&network, ndp_msg_opt_route_prefix(msg, offset), - route.plen); - if (nm_ndisc_add_route(ndisc, &route)) - changed |= NM_NDISC_CONFIG_ROUTES; + plen); + + { + const NMNDiscRoute route = { + .network = network, + .gateway = gateway_addr, + .plen = plen, + .expiry_msec = + _nm_ndisc_lifetime_to_expiry(now_msec, ndp_msg_opt_route_lifetime(msg, offset)), + .preference = _route_preference_coerce(ndp_msg_opt_route_preference(msg, offset)), + }; + + /* Routers through this particular gateway */ + if (nm_ndisc_add_route(ndisc, &route, now_msec)) + changed |= NM_NDISC_CONFIG_ROUTES; + } } - /* DNS information */ ndp_msg_opt_for_each_offset (offset, msg, NDP_MSG_OPT_RDNSS) { struct in6_addr *addr; int addr_index; ndp_msg_opt_rdnss_for_each_addr (addr, addr_index, msg, offset) { - NMNDiscDNSServer dns_server = { - .address = *addr, - .timestamp = now, - .lifetime = ndp_msg_opt_rdnss_lifetime(msg, offset), + const NMNDiscDNSServer dns_server = { + .address = *addr, + .expiry_msec = + _nm_ndisc_lifetime_to_expiry(now_msec, ndp_msg_opt_rdnss_lifetime(msg, offset)), }; - if (nm_ndisc_add_dns_server(ndisc, &dns_server)) + if (nm_ndisc_add_dns_server(ndisc, &dns_server, now_msec)) changed |= NM_NDISC_CONFIG_DNS_SERVERS; } } @@ -270,13 +284,13 @@ receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) int domain_index; ndp_msg_opt_dnssl_for_each_domain (domain, domain_index, msg, offset) { - NMNDiscDNSDomain dns_domain = { - .domain = domain, - .timestamp = now, - .lifetime = ndp_msg_opt_dnssl_lifetime(msg, offset), + const NMNDiscDNSDomain dns_domain = { + .domain = domain, + .expiry_msec = + _nm_ndisc_lifetime_to_expiry(now_msec, ndp_msg_opt_dnssl_lifetime(msg, offset)), }; - if (nm_ndisc_add_dns_domain(ndisc, &dns_domain)) + if (nm_ndisc_add_dns_domain(ndisc, &dns_domain, now_msec)) changed |= NM_NDISC_CONFIG_DNS_DOMAINS; } } @@ -316,7 +330,7 @@ receive_ra(struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) } } - nm_ndisc_ra_received(ndisc, now, changed); + nm_ndisc_ra_received(ndisc, now_msec, changed); return 0; } @@ -374,7 +388,6 @@ send_ra(NMNDisc *ndisc, GError **error) { NMLndpNDiscPrivate * priv = NM_LNDP_NDISC_GET_PRIVATE(ndisc); NMNDiscDataInternal * rdata = ndisc->rdata; - gint32 now = nm_utils_get_monotonic_timestamp_sec(); int errsv; struct in6_addr * addr; struct ndp_msg * msg; @@ -404,18 +417,9 @@ send_ra(NMNDisc *ndisc, GError **error) /* The device let us know about all addresses that the device got * whose prefixes are suitable for delegating. Let's announce them. */ for (i = 0; i < rdata->addresses->len; i++) { - const NMNDiscAddress *address = &g_array_index(rdata->addresses, NMNDiscAddress, i); - guint32 age = NM_CLAMP((gint64) now - (gint64) address->timestamp, 0, G_MAXUINT32 - 1); - guint32 lifetime = address->lifetime; - guint32 preferred = address->preferred; + const NMNDiscAddress * address = &g_array_index(rdata->addresses, NMNDiscAddress, i); struct nd_opt_prefix_info *prefix; - /* Clamp the life times if they're not forever. */ - if (lifetime != NM_NDISC_INFINITY) - lifetime = lifetime > age ? lifetime - age : 0; - if (preferred != NM_NDISC_INFINITY) - preferred = preferred > age ? preferred - age : 0; - prefix = _ndp_msg_add_option(msg, sizeof(*prefix)); if (!prefix) { /* Maybe we could sent separate RAs, but why bother... */ @@ -428,8 +432,14 @@ send_ra(NMNDisc *ndisc, GError **error) prefix->nd_opt_pi_prefix_len = 64; prefix->nd_opt_pi_flags_reserved |= ND_OPT_PI_FLAG_ONLINK; prefix->nd_opt_pi_flags_reserved |= ND_OPT_PI_FLAG_AUTO; - prefix->nd_opt_pi_valid_time = htonl(lifetime); - prefix->nd_opt_pi_preferred_time = htonl(preferred); + prefix->nd_opt_pi_valid_time = + htonl(_nm_ndisc_lifetime_from_expiry(NM_NDISC_EXPIRY_BASE_TIMESTAMP, + address->expiry_msec, + TRUE)); + prefix->nd_opt_pi_preferred_time = + htonl(_nm_ndisc_lifetime_from_expiry(NM_NDISC_EXPIRY_BASE_TIMESTAMP, + address->expiry_preferred_msec, + TRUE)); prefix->nd_opt_pi_prefix.s6_addr32[0] = address->address.s6_addr32[0]; prefix->nd_opt_pi_prefix.s6_addr32[1] = address->address.s6_addr32[1]; prefix->nd_opt_pi_prefix.s6_addr32[2] = 0; diff --git a/src/ndisc/nm-ndisc-private.h b/src/ndisc/nm-ndisc-private.h index e06820f94e..047391243b 100644 --- a/src/ndisc/nm-ndisc-private.h +++ b/src/ndisc/nm-ndisc-private.h @@ -21,14 +21,15 @@ struct _NMNDiscDataInternal { typedef struct _NMNDiscDataInternal NMNDiscDataInternal; -void nm_ndisc_ra_received(NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed); +void nm_ndisc_ra_received(NMNDisc *ndisc, gint64 now_msec, 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, 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); +gboolean nm_ndisc_add_gateway(NMNDisc *ndisc, const NMNDiscGateway *new_item, gint64 now_msec); +gboolean +nm_ndisc_complete_and_add_address(NMNDisc *ndisc, const NMNDiscAddress *new_item, gint64 now_msec); +gboolean nm_ndisc_add_route(NMNDisc *ndisc, const NMNDiscRoute *new_item, gint64 now_msec); +gboolean nm_ndisc_add_dns_server(NMNDisc *ndisc, const NMNDiscDNSServer *new_item, gint64 now_msec); +gboolean nm_ndisc_add_dns_domain(NMNDisc *ndisc, const NMNDiscDNSDomain *new_item, gint64 now_msec); /*****************************************************************************/ diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c index 0ac10b8808..a86853c081 100644 --- a/src/ndisc/nm-ndisc.c +++ b/src/ndisc/nm-ndisc.c @@ -41,7 +41,9 @@ struct _NMNDiscPrivate { gint32 last_rs; gint32 last_ra; }; - guint timeout_id; /* prefix/dns/etc lifetime timeout */ + + GSource *timeout_expire_source; + NMUtilsIPv6IfaceId iid; /* immutable values: */ @@ -84,7 +86,8 @@ G_DEFINE_TYPE(NMNDisc, nm_ndisc, G_TYPE_OBJECT) /*****************************************************************************/ -static void _config_changed_log(NMNDisc *ndisc, NMNDiscConfigMap changed); +static void _config_changed_log(NMNDisc *ndisc, NMNDiscConfigMap changed); +static gboolean timeout_expire_cb(gpointer user_data); /*****************************************************************************/ @@ -102,6 +105,7 @@ nm_ndisc_data_to_l3cd(NMDedupMultiIndex * multi_idx, guint32 ifa_flags; guint8 plen; guint i; + const gint32 now_sec = nm_utils_get_monotonic_timestamp_sec(); l3cd = nm_l3_config_data_new(multi_idx, ifindex); @@ -129,12 +133,17 @@ nm_ndisc_data_to_l3cd(NMDedupMultiIndex * multi_idx, NMPlatformIP6Address a; a = (NMPlatformIP6Address){ - .ifindex = ifindex, - .address = ndisc_addr->address, - .plen = plen, - .timestamp = ndisc_addr->timestamp, - .lifetime = ndisc_addr->lifetime, - .preferred = MIN(ndisc_addr->lifetime, ndisc_addr->preferred), + .ifindex = ifindex, + .address = ndisc_addr->address, + .plen = plen, + .timestamp = now_sec, + .lifetime = _nm_ndisc_lifetime_from_expiry(((gint64) now_sec) * 1000, + ndisc_addr->expiry_msec, + TRUE), + .preferred = _nm_ndisc_lifetime_from_expiry( + ((gint64) now_sec) * 1000, + NM_MIN(ndisc_addr->expiry_msec, ndisc_addr->expiry_preferred_msec), + TRUE), .addr_source = NM_IP_CONFIG_SOURCE_NDISC, .n_ifa_flags = ifa_flags, }; @@ -220,68 +229,40 @@ _preference_to_priority(NMIcmpv6RouterPref pref) /*****************************************************************************/ -/* 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) -{ - nm_assert(timestamp > 0); - nm_assert(timestamp <= G_MAXINT32); - - if (lifetime == NM_NDISC_INFINITY) - 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); \ - }) - -#define get_expiry_preferred(item) \ - ({ \ - typeof(item) _item = (item); \ - nm_assert(_item); \ - get_expiry_time(_item->timestamp, _item->preferred); \ - }) - static gboolean -expiry_next(gint32 now_s, gint64 expiry_timestamp, gint32 *nextevent) +expiry_next(gint64 now_msec, gint64 expiry_msec, gint64 *next_msec) { - gint32 e; - - if (expiry_timestamp == _EXPIRY_INFINITY) + if (expiry_msec == NM_NDISC_EXPIRY_INFINITY) return TRUE; - e = MIN(expiry_timestamp, ((gint64)(G_MAXINT32 - 1))); - if (now_s >= e) + + if (expiry_msec <= now_msec) { + /* expired. */ return FALSE; - if (nextevent) { - if (*nextevent > e) - *nextevent = e; } + + if (next_msec) { + if (*next_msec > expiry_msec) + *next_msec = expiry_msec; + } + + /* the timestamp is good (not yet expired) */ return TRUE; } static const char * -_get_exp(char *buf, gsize buf_size, gint64 now_ns, gint64 expiry_time) +_get_exp(char *buf, gsize buf_size, gint64 now_msec, gint64 expiry_time) { int l; - if (expiry_time == _EXPIRY_INFINITY) + if (expiry_time == NM_NDISC_EXPIRY_INFINITY) return "permanent"; - l = g_snprintf(buf, - buf_size, - "%.4f", - ((double) ((expiry_time * NM_UTILS_NSEC_PER_SEC) - now_ns)) - / ((double) NM_UTILS_NSEC_PER_SEC)); + l = g_snprintf(buf, buf_size, "%.3f", ((double) (expiry_time - now_msec)) / 1000); nm_assert(l < buf_size); return buf; } -#define get_exp(buf, now_ns, item) _get_exp((buf), G_N_ELEMENTS(buf), (now_ns), (get_expiry(item))) +#define get_exp(buf, now_msec, item) \ + _get_exp((buf), G_N_ELEMENTS(buf), (now_msec), (item)->expiry_msec) /*****************************************************************************/ @@ -352,17 +333,16 @@ _ASSERT_data_gateways(const NMNDiscDataInternal *data) const NMNDiscGateway *item = &g_array_index(data->gateways, NMNDiscGateway, i); nm_assert(!IN6_IS_ADDR_UNSPECIFIED(&item->address)); - nm_assert(item->timestamp > 0 && item->timestamp <= G_MAXINT32); for (j = 0; j < i; j++) { const NMNDiscGateway *item2 = &g_array_index(data->gateways, NMNDiscGateway, j); nm_assert(!IN6_ARE_ADDR_EQUAL(&item->address, &item2->address)); } - nm_assert(item->lifetime > 0); - if (i > 0) + if (i > 0) { nm_assert(_preference_to_priority(item_prev->preference) >= _preference_to_priority(item->preference)); + } item_prev = item; } @@ -408,7 +388,7 @@ nm_ndisc_emit_config_change(NMNDisc *self, NMNDiscConfigMap changed) /*****************************************************************************/ gboolean -nm_ndisc_add_gateway(NMNDisc *ndisc, const NMNDiscGateway *new) +nm_ndisc_add_gateway(NMNDisc *ndisc, const NMNDiscGateway *new_item, gint64 now_msec) { NMNDiscDataInternal *rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; guint i; @@ -417,41 +397,43 @@ nm_ndisc_add_gateway(NMNDisc *ndisc, const NMNDiscGateway *new) for (i = 0; i < rdata->gateways->len;) { NMNDiscGateway *item = &g_array_index(rdata->gateways, NMNDiscGateway, i); - if (IN6_ARE_ADDR_EQUAL(&item->address, &new->address)) { - if (new->lifetime == 0) { + if (IN6_ARE_ADDR_EQUAL(&item->address, &new_item->address)) { + if (new_item->expiry_msec <= now_msec) { g_array_remove_index(rdata->gateways, i); _ASSERT_data_gateways(rdata); return TRUE; } - if (item->preference != new->preference) { + if (item->preference != new_item->preference) { g_array_remove_index(rdata->gateways, i); continue; } - if (get_expiry(item) == get_expiry(new)) + if (item->expiry_msec == new_item->expiry_msec) return FALSE; - *item = *new; + item->expiry_msec = new_item->expiry_msec; _ASSERT_data_gateways(rdata); return TRUE; } /* Put before less preferable gateways. */ - if (_preference_to_priority(item->preference) < _preference_to_priority(new->preference) + if (_preference_to_priority(item->preference) + < _preference_to_priority(new_item->preference) && insert_idx == G_MAXUINT) insert_idx = i; i++; } - if (new->lifetime) { - g_array_insert_val(rdata->gateways, - insert_idx == G_MAXUINT ? rdata->gateways->len : insert_idx, - *new); - } + if (new_item->expiry_msec <= now_msec) + return FALSE; + + g_array_insert_val(rdata->gateways, + insert_idx == G_MAXUINT ? rdata->gateways->len : insert_idx, + *new_item); _ASSERT_data_gateways(rdata); - return !!new->lifetime; + return TRUE; } /** @@ -504,25 +486,27 @@ complete_address(NMNDisc *ndisc, NMNDiscAddress *addr) return TRUE; } - _LOGW("complete-address: can't generate a new EUI-64 address"); + _LOGW("complete-address: can't generate a new_item EUI-64 address"); return FALSE; } static gboolean -nm_ndisc_add_address(NMNDisc *ndisc, const NMNDiscAddress *new, gint32 now_s, gboolean from_ra) +nm_ndisc_add_address(NMNDisc * ndisc, + const NMNDiscAddress *new_item, + gint64 now_msec, + gboolean from_ra) { NMNDiscPrivate * priv = NM_NDISC_GET_PRIVATE(ndisc); NMNDiscDataInternal *rdata = &priv->rdata; - NMNDiscAddress new2; + 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); + nm_assert(new_item); + nm_assert(!IN6_IS_ADDR_UNSPECIFIED(&new_item->address)); + nm_assert(!IN6_IS_ADDR_LINKLOCAL(&new_item->address)); + nm_assert(new_item->expiry_preferred_msec <= new_item->expiry_msec); + nm_assert((!!from_ra) == (now_msec > 0)); for (i = 0; i < rdata->addresses->len; i++) { NMNDiscAddress *item = &g_array_index(rdata->addresses, NMNDiscAddress, i); @@ -530,12 +514,12 @@ nm_ndisc_add_address(NMNDisc *ndisc, const NMNDiscAddress *new, gint32 now_s, gb 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 implicitly length /64). */ - if (memcmp(&item->address, &new->address, 8) == 0) { + if (memcmp(&item->address, &new_item->address, 8) == 0) { existing = item; break; } } else { - if (IN6_ARE_ADDR_EQUAL(&item->address, &new->address)) { + if (IN6_ARE_ADDR_EQUAL(&item->address, &new_item->address)) { existing = item; break; } @@ -543,67 +527,60 @@ nm_ndisc_add_address(NMNDisc *ndisc, const NMNDiscAddress *new, gint32 now_s, gb } if (existing) { + gint64 new_expiry_preferred_msec; + gint64 new_expiry_msec; + 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; + if (new_item->expiry_msec == NM_NDISC_EXPIRY_INFINITY) + new_expiry_msec = NM_NDISC_EXPIRY_INFINITY; else { - gint64 new_lifetime, remaining_lifetime; + const gint64 NDISC_PREFIX_LFT_MIN_MSEC = 7200 * 1000; /* RFC4862 5.5.3.e */ + gint64 new_lifetime; + gint64 existing_lifetime; + + new_lifetime = new_item->expiry_msec - now_msec; + if (existing->expiry_msec == NM_NDISC_EXPIRY_INFINITY) + existing_lifetime = G_MAXINT64; + else + existing_lifetime = existing->expiry_msec - now_msec; /* 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) { + if (new_lifetime >= NDISC_PREFIX_LFT_MIN_MSEC + || new_lifetime >= existing_lifetime) { + /* either extend the lifetime of the new_item lifetime is longer than + * NDISC_PREFIX_LFT_MIN_MSEC. */ + new_expiry_msec = new_item->expiry_msec; + } else if (existing_lifetime <= NDISC_PREFIX_LFT_MIN_MSEC) { /* keep the current lifetime. */ + new_expiry_msec = existing->expiry_msec; } else { - existing->timestamp = now_s; - existing->lifetime = NM_NDISC_PREFIX_LFT_MIN; + /* trim the current lifetime to NDISC_PREFIX_LFT_MIN_MSEC. */ + new_expiry_msec = now_msec + NDISC_PREFIX_LFT_MIN_MSEC; } } - 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); + new_expiry_preferred_msec = + NM_MIN(new_item->expiry_preferred_msec, new_item->expiry_msec); + new_expiry_preferred_msec = NM_MIN(new_expiry_preferred_msec, new_expiry_msec); + } else { + if (new_item->expiry_msec <= now_msec) { + g_array_remove_index(rdata->addresses, i); + return TRUE; } - return old_expiry_lifetime != get_expiry(existing) - || old_expiry_preferred != get_expiry_preferred(existing); + new_expiry_msec = new_item->expiry_msec; + new_expiry_preferred_msec = + NM_MIN(new_item->expiry_preferred_msec, new_item->expiry_msec); } - 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)) + /* the dad_counter does not get modified. */ + if (new_expiry_msec == existing->expiry_msec + && new_expiry_preferred_msec == existing->expiry_preferred_msec) { return FALSE; + } - existing->timestamp = new->timestamp; - existing->lifetime = new->lifetime; - existing->preferred = new->preferred; + existing->expiry_msec = new_expiry_msec; + existing->expiry_preferred_msec = new_expiry_preferred_msec; return TRUE; } @@ -614,36 +591,42 @@ nm_ndisc_add_address(NMNDisc *ndisc, const NMNDiscAddress *new, gint32 now_s, gb if (priv->max_addresses && rdata->addresses->len >= priv->max_addresses) return FALSE; - if (new->lifetime == 0) + if (new_item->expiry_msec <= now_msec) return FALSE; + new2 = nm_g_array_append_new(rdata->addresses, NMNDiscAddress); + + *new2 = *new_item; + + new2->expiry_preferred_msec = NM_MIN(new2->expiry_preferred_msec, new2->expiry_msec); + if (from_ra) { - new2 = *new; - new2.dad_counter = 0; - if (!complete_address(ndisc, &new2)) + new2->dad_counter = 0; + if (!complete_address(ndisc, new2)) { + g_array_set_size(rdata->addresses, rdata->addresses->len - 1); return FALSE; - new = &new2; + } } - g_array_append_val(rdata->addresses, *new); return TRUE; } gboolean -nm_ndisc_complete_and_add_address(NMNDisc *ndisc, const NMNDiscAddress *new, gint32 now_s) +nm_ndisc_complete_and_add_address(NMNDisc *ndisc, const NMNDiscAddress *new_item, gint64 now_msec) { - return nm_ndisc_add_address(ndisc, new, now_s, TRUE); + return nm_ndisc_add_address(ndisc, new_item, now_msec, TRUE); } gboolean -nm_ndisc_add_route(NMNDisc *ndisc, const NMNDiscRoute *new) +nm_ndisc_add_route(NMNDisc *ndisc, const NMNDiscRoute *new_item, gint64 now_msec) { NMNDiscPrivate * priv; NMNDiscDataInternal *rdata; guint i; guint insert_idx = G_MAXUINT; + gboolean changed = FALSE; - if (new->plen == 0 || new->plen > 128) { + if (new_item->plen == 0 || new_item->plen > 128) { /* Only expect non-default routes. The router has no idea what the * local configuration or user preferences are, so sending routes * with a prefix length of 0 must be ignored by NMNDisc. @@ -660,41 +643,48 @@ nm_ndisc_add_route(NMNDisc *ndisc, const NMNDiscRoute *new) for (i = 0; i < rdata->routes->len;) { NMNDiscRoute *item = &g_array_index(rdata->routes, NMNDiscRoute, i); - if (IN6_ARE_ADDR_EQUAL(&item->network, &new->network) && item->plen == new->plen) { - if (new->lifetime == 0) { + if (IN6_ARE_ADDR_EQUAL(&item->network, &new_item->network) + && item->plen == new_item->plen) { + if (new_item->expiry_msec <= now_msec) { g_array_remove_index(rdata->routes, i); return TRUE; } - if (item->preference != new->preference) { + if (item->preference != new_item->preference) { g_array_remove_index(rdata->routes, i); + changed = TRUE; continue; } - if (get_expiry(item) == get_expiry(new) - && IN6_ARE_ADDR_EQUAL(&item->gateway, &new->gateway)) + if (item->expiry_msec == new_item->expiry_msec + && IN6_ARE_ADDR_EQUAL(&item->gateway, &new_item->gateway)) return FALSE; - *item = *new; + item->expiry_msec = new_item->expiry_msec; + item->gateway = new_item->gateway; return TRUE; } /* Put before less preferable routes. */ - if (_preference_to_priority(item->preference) < _preference_to_priority(new->preference) + if (_preference_to_priority(item->preference) + < _preference_to_priority(new_item->preference) && insert_idx == G_MAXUINT) insert_idx = i; i++; } - if (new->lifetime) { - g_array_insert_val(rdata->routes, insert_idx == G_MAXUINT ? 0u : insert_idx, *new); + if (new_item->expiry_msec <= now_msec) { + nm_assert(!changed); + return FALSE; } - return !!new->lifetime; + + g_array_insert_val(rdata->routes, insert_idx == G_MAXUINT ? 0u : insert_idx, *new_item); + return TRUE; } gboolean -nm_ndisc_add_dns_server(NMNDisc *ndisc, const NMNDiscDNSServer *new) +nm_ndisc_add_dns_server(NMNDisc *ndisc, const NMNDiscDNSServer *new_item, gint64 now_msec) { NMNDiscPrivate * priv; NMNDiscDataInternal *rdata; @@ -706,28 +696,30 @@ nm_ndisc_add_dns_server(NMNDisc *ndisc, const NMNDiscDNSServer *new) for (i = 0; i < rdata->dns_servers->len; i++) { NMNDiscDNSServer *item = &g_array_index(rdata->dns_servers, NMNDiscDNSServer, i); - if (IN6_ARE_ADDR_EQUAL(&item->address, &new->address)) { - if (new->lifetime == 0) { + if (IN6_ARE_ADDR_EQUAL(&item->address, &new_item->address)) { + if (new_item->expiry_msec <= now_msec) { g_array_remove_index(rdata->dns_servers, i); return TRUE; } - if (get_expiry(item) == get_expiry(new)) + if (item->expiry_msec == new_item->expiry_msec) return FALSE; - *item = *new; + item->expiry_msec = new_item->expiry_msec; return TRUE; } } - if (new->lifetime) - g_array_append_val(rdata->dns_servers, *new); - return !!new->lifetime; + if (new_item->expiry_msec <= now_msec) + return FALSE; + + g_array_append_val(rdata->dns_servers, *new_item); + return TRUE; } -/* Copies new->domain if 'new' is added to the dns_domains list */ +/* Copies new_item->domain if 'new_item' is added to the dns_domains list */ gboolean -nm_ndisc_add_dns_domain(NMNDisc *ndisc, const NMNDiscDNSDomain *new) +nm_ndisc_add_dns_domain(NMNDisc *ndisc, const NMNDiscDNSDomain *new_item, gint64 now_msec) { NMNDiscPrivate * priv; NMNDiscDataInternal *rdata; @@ -740,43 +732,45 @@ nm_ndisc_add_dns_domain(NMNDisc *ndisc, const NMNDiscDNSDomain *new) for (i = 0; i < rdata->dns_domains->len; i++) { item = &g_array_index(rdata->dns_domains, NMNDiscDNSDomain, i); - if (!g_strcmp0(item->domain, new->domain)) { - if (new->lifetime == 0) { + if (nm_streq(item->domain, new_item->domain)) { + if (new_item->expiry_msec <= now_msec) { g_array_remove_index(rdata->dns_domains, i); return TRUE; } - if (get_expiry(item) == get_expiry(new)) + if (item->expiry_msec == new_item->expiry_msec) return FALSE; - item->timestamp = new->timestamp; - item->lifetime = new->lifetime; + item->expiry_msec = new_item->expiry_msec; return TRUE; } } - if (new->lifetime) { - g_array_append_val(rdata->dns_domains, *new); - item = &g_array_index(rdata->dns_domains, NMNDiscDNSDomain, rdata->dns_domains->len - 1); - item->domain = g_strdup(new->domain); - } - return !!new->lifetime; + if (new_item->expiry_msec <= now_msec) + return FALSE; + + item = nm_g_array_append_new(rdata->dns_domains, NMNDiscDNSDomain); + *item = (NMNDiscDNSDomain){ + .domain = g_strdup(new_item->domain), + .expiry_msec = new_item->expiry_msec, + }; + return TRUE; } /*****************************************************************************/ -#define _MAYBE_WARN(...) \ - G_STMT_START \ - { \ - gboolean _different_message; \ - \ - _different_message = g_strcmp0(priv->last_error, error->message) != 0; \ - _NMLOG(_different_message ? LOGL_WARN : LOGL_DEBUG, __VA_ARGS__); \ - if (_different_message) { \ - nm_clear_g_free(&priv->last_error); \ - priv->last_error = g_strdup(error->message); \ - } \ - } \ +#define _MAYBE_WARN(...) \ + G_STMT_START \ + { \ + gboolean _different_message; \ + \ + _different_message = !nm_streq0(priv->last_error, error->message); \ + _NMLOG(_different_message ? LOGL_WARN : LOGL_DEBUG, __VA_ARGS__); \ + if (_different_message) { \ + nm_clear_g_free(&priv->last_error); \ + priv->last_error = g_strdup(error->message); \ + } \ + } \ G_STMT_END static gboolean @@ -937,12 +931,16 @@ nm_ndisc_set_config(NMNDisc * ndisc, } for (i = 0; i < dns_servers->len; i++) { - if (nm_ndisc_add_dns_server(ndisc, &g_array_index(dns_servers, NMNDiscDNSServer, i))) + if (nm_ndisc_add_dns_server(ndisc, + &g_array_index(dns_servers, NMNDiscDNSServer, i), + G_MININT64)) changed = TRUE; } for (i = 0; i < dns_domains->len; i++) { - if (nm_ndisc_add_dns_domain(ndisc, &g_array_index(dns_domains, NMNDiscDNSDomain, i))) + if (nm_ndisc_add_dns_domain(ndisc, + &g_array_index(dns_domains, NMNDiscDNSDomain, i), + G_MININT64)) changed = TRUE; } @@ -1098,7 +1096,7 @@ nm_ndisc_stop(NMNDisc *ndisc) nm_clear_g_source(&priv->send_rs_id); nm_clear_g_source(&priv->send_ra_id); nm_clear_g_free(&priv->last_error); - nm_clear_g_source(&priv->timeout_id); + nm_clear_g_source_inst(&priv->timeout_expire_source); priv->solicitations_left = 0; priv->announcements_left = 0; @@ -1180,15 +1178,15 @@ _config_changed_log(NMNDisc *ndisc, NMNDiscConfigMap changed) NMNDiscDataInternal *rdata; guint i; char changedstr[CONFIG_MAP_MAX_STR]; - char addrstr[INET6_ADDRSTRLEN]; + char addrstr[NM_UTILS_INET_ADDRSTRLEN]; char str_pref[35]; char str_exp[100]; - gint64 now_ns; + gint64 now_msec; if (!_LOGD_ENABLED()) return; - now_ns = nm_utils_get_monotonic_timestamp_nsec(); + now_msec = nm_utils_get_monotonic_timestamp_msec(); priv = NM_NDISC_GET_PRIVATE(ndisc); rdata = &priv->rdata; @@ -1205,199 +1203,245 @@ _config_changed_log(NMNDisc *ndisc, NMNDiscConfigMap changed) _LOGD(" retrans timer : %u", (guint) rdata->public.retrans_timer_ms); for (i = 0; i < rdata->gateways->len; i++) { - NMNDiscGateway *gateway = &g_array_index(rdata->gateways, NMNDiscGateway, i); + const NMNDiscGateway *gateway = &g_array_index(rdata->gateways, NMNDiscGateway, i); - inet_ntop(AF_INET6, &gateway->address, addrstr, sizeof(addrstr)); _LOGD(" gateway %s pref %s exp %s", - addrstr, + _nm_utils_inet6_ntop(&gateway->address, addrstr), nm_icmpv6_router_pref_to_string(gateway->preference, str_pref, sizeof(str_pref)), - get_exp(str_exp, now_ns, gateway)); + get_exp(str_exp, now_msec, gateway)); } for (i = 0; i < rdata->addresses->len; i++) { const NMNDiscAddress *address = &g_array_index(rdata->addresses, NMNDiscAddress, i); - inet_ntop(AF_INET6, &address->address, addrstr, sizeof(addrstr)); - _LOGD(" address %s exp %s", addrstr, get_exp(str_exp, now_ns, address)); + _LOGD(" address %s exp %s", + _nm_utils_inet6_ntop(&address->address, addrstr), + get_exp(str_exp, now_msec, address)); } for (i = 0; i < rdata->routes->len; i++) { - NMNDiscRoute *route = &g_array_index(rdata->routes, NMNDiscRoute, i); - char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + const NMNDiscRoute *route = &g_array_index(rdata->routes, NMNDiscRoute, i); + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; - inet_ntop(AF_INET6, &route->network, addrstr, sizeof(addrstr)); _LOGD(" route %s/%u via %s pref %s exp %s", - addrstr, + _nm_utils_inet6_ntop(&route->network, addrstr), (guint) route->plen, _nm_utils_inet6_ntop(&route->gateway, sbuf), nm_icmpv6_router_pref_to_string(route->preference, str_pref, sizeof(str_pref)), - get_exp(str_exp, now_ns, route)); + get_exp(str_exp, now_msec, route)); } for (i = 0; i < rdata->dns_servers->len; i++) { - NMNDiscDNSServer *dns_server = &g_array_index(rdata->dns_servers, NMNDiscDNSServer, i); + const NMNDiscDNSServer *dns_server = + &g_array_index(rdata->dns_servers, NMNDiscDNSServer, i); - inet_ntop(AF_INET6, &dns_server->address, addrstr, sizeof(addrstr)); - _LOGD(" dns_server %s exp %s", addrstr, get_exp(str_exp, now_ns, dns_server)); + _LOGD(" dns_server %s exp %s", + _nm_utils_inet6_ntop(&dns_server->address, addrstr), + get_exp(str_exp, now_msec, dns_server)); } for (i = 0; i < rdata->dns_domains->len; i++) { - NMNDiscDNSDomain *dns_domain = &g_array_index(rdata->dns_domains, NMNDiscDNSDomain, i); + const NMNDiscDNSDomain *dns_domain = + &g_array_index(rdata->dns_domains, NMNDiscDNSDomain, i); - _LOGD(" dns_domain %s exp %s", dns_domain->domain, get_exp(str_exp, now_ns, dns_domain)); + _LOGD(" dns_domain %s exp %s", dns_domain->domain, get_exp(str_exp, now_msec, dns_domain)); } } +/*****************************************************************************/ + static void -clean_gateways(NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent) +clean_gateways(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint64 *next_msec) { - NMNDiscDataInternal *rdata; + NMNDiscDataInternal *rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + NMNDiscGateway * arr; guint i; + guint j; - rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + if (rdata->gateways->len == 0) + return; - for (i = 0; i < rdata->gateways->len;) { - NMNDiscGateway *item = &g_array_index(rdata->gateways, NMNDiscGateway, i); + arr = &g_array_index(rdata->gateways, NMNDiscGateway, 0); - if (!expiry_next(now, get_expiry(item), nextevent)) { - g_array_remove_index(rdata->gateways, i); - *changed |= NM_NDISC_CONFIG_GATEWAYS; + for (i = 0, j = 0; i < rdata->gateways->len; i++) { + if (!expiry_next(now_msec, arr[i].expiry_msec, next_msec)) continue; - } + if (i != j) + arr[j] = arr[i]; + j++; + } - i++; + if (i != j) { + *changed |= NM_NDISC_CONFIG_GATEWAYS; + g_array_set_size(rdata->gateways, j); } _ASSERT_data_gateways(rdata); } static void -clean_addresses(NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent) +clean_addresses(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint64 *next_msec) { - NMNDiscDataInternal *rdata; + NMNDiscDataInternal *rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + NMNDiscAddress * arr; guint i; + guint j; - rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + if (rdata->addresses->len == 0) + return; - for (i = 0; i < rdata->addresses->len;) { - const NMNDiscAddress *item = &g_array_index(rdata->addresses, NMNDiscAddress, i); + arr = &g_array_index(rdata->addresses, NMNDiscAddress, 0); - if (!expiry_next(now, get_expiry(item), nextevent)) { - g_array_remove_index(rdata->addresses, i); - *changed |= NM_NDISC_CONFIG_ADDRESSES; + for (i = 0, j = 0; i < rdata->addresses->len; i++) { + if (!expiry_next(now_msec, arr[i].expiry_msec, next_msec)) continue; - } + if (i != j) + arr[j] = arr[i]; + j++; + } - i++; + if (i != j) { + *changed = NM_NDISC_CONFIG_ADDRESSES; + g_array_set_size(rdata->addresses, j); } } static void -clean_routes(NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent) +clean_routes(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint64 *next_msec) { - NMNDiscDataInternal *rdata; + NMNDiscDataInternal *rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + NMNDiscRoute * arr; guint i; + guint j; - rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + if (rdata->routes->len == 0) + return; - for (i = 0; i < rdata->routes->len;) { - NMNDiscRoute *item = &g_array_index(rdata->routes, NMNDiscRoute, i); + arr = &g_array_index(rdata->routes, NMNDiscRoute, 0); - if (!expiry_next(now, get_expiry(item), nextevent)) { - g_array_remove_index(rdata->routes, i); - *changed |= NM_NDISC_CONFIG_ROUTES; + for (i = 0, j = 0; i < rdata->routes->len; i++) { + if (!expiry_next(now_msec, arr[i].expiry_msec, next_msec)) continue; - } + if (i != j) + arr[j] = arr[i]; + j++; + } - i++; + if (i != j) { + *changed |= NM_NDISC_CONFIG_ROUTES; + g_array_set_size(rdata->routes, j); } } static void -clean_dns_servers(NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent) +clean_dns_servers(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint64 *next_msec) { - NMNDiscDataInternal *rdata; + NMNDiscDataInternal *rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + NMNDiscDNSServer * arr; guint i; + guint j; - rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + if (rdata->dns_servers->len == 0) + return; - for (i = 0; i < rdata->dns_servers->len;) { - NMNDiscDNSServer *item = &g_array_index(rdata->dns_servers, NMNDiscDNSServer, i); + arr = &g_array_index(rdata->dns_servers, NMNDiscDNSServer, 0); - if (!expiry_next(now, get_expiry(item), nextevent)) { - g_array_remove_index(rdata->dns_servers, i); - *changed |= NM_NDISC_CONFIG_DNS_SERVERS; + for (i = 0, j = 0; i < rdata->dns_servers->len; i++) { + if (!expiry_next(now_msec, arr[i].expiry_msec, next_msec)) continue; - } + if (i != j) + arr[j] = arr[i]; + j++; + } - i++; + if (i != j) { + *changed |= NM_NDISC_CONFIG_DNS_SERVERS; + g_array_set_size(rdata->dns_servers, j); } } static void -clean_dns_domains(NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent) +clean_dns_domains(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint64 *next_msec) { - NMNDiscDataInternal *rdata; + NMNDiscDataInternal *rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + NMNDiscDNSDomain * arr; guint i; + guint j; - rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; + if (rdata->dns_domains->len == 0) + return; - for (i = 0; i < rdata->dns_domains->len;) { - NMNDiscDNSDomain *item = &g_array_index(rdata->dns_domains, NMNDiscDNSDomain, i); + arr = &g_array_index(rdata->dns_domains, NMNDiscDNSDomain, 0); - if (!expiry_next(now, get_expiry(item), nextevent)) { - g_array_remove_index(rdata->dns_domains, i); - *changed |= NM_NDISC_CONFIG_DNS_DOMAINS; + for (i = 0, j = 0; i < rdata->dns_domains->len; i++) { + if (!expiry_next(now_msec, arr[i].expiry_msec, next_msec)) continue; + + if (i != j) { + g_free(arr[j].domain); + arr[j] = arr[i]; + arr[i].domain = NULL; } - i++; + j++; + } + + if (i != 0) { + *changed |= NM_NDISC_CONFIG_DNS_DOMAINS; + g_array_set_size(rdata->dns_domains, j); } } -static gboolean timeout_cb(gpointer user_data); - static void -check_timestamps(NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed) +check_timestamps(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap changed) { - NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE(ndisc); - /* Use a magic date in the distant future (~68 years) */ - gint32 nextevent = G_MAXINT32; + NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE(ndisc); + gint64 next_msec = G_MAXINT64; - nm_clear_g_source(&priv->timeout_id); + _LOGT("router-data: check for changed router advertisement data"); - clean_gateways(ndisc, now, &changed, &nextevent); - clean_addresses(ndisc, now, &changed, &nextevent); - clean_routes(ndisc, now, &changed, &nextevent); - clean_dns_servers(ndisc, now, &changed, &nextevent); - clean_dns_domains(ndisc, now, &changed, &nextevent); + clean_gateways(ndisc, now_msec, &changed, &next_msec); + clean_addresses(ndisc, now_msec, &changed, &next_msec); + clean_routes(ndisc, now_msec, &changed, &next_msec); + clean_dns_servers(ndisc, now_msec, &changed, &next_msec); + clean_dns_domains(ndisc, now_msec, &changed, &next_msec); - if (nextevent != G_MAXINT32) { - if (nextevent <= now) - g_return_if_reached(); - _LOGD("scheduling next now/lifetime check: %d seconds", (int) (nextevent - now)); - priv->timeout_id = g_timeout_add_seconds(nextevent - now, timeout_cb, ndisc); + nm_assert(next_msec > now_msec); + + nm_clear_g_source_inst(&priv->timeout_expire_source); + + if (next_msec == NM_NDISC_EXPIRY_INFINITY) + _LOGD("router-data: next lifetime expiration will happen: never"); + else { + const gint64 timeout_msec = NM_MIN(next_msec - now_msec, ((gint64) G_MAXINT32)); + const guint TIMEOUT_APPROX_THRESHOLD_SEC = 10000; + + _LOGD("router-data: next lifetime expiration will happen: in %s%.3f seconds", + (timeout_msec / 1000) >= TIMEOUT_APPROX_THRESHOLD_SEC ? " about" : "", + ((double) timeout_msec) / 1000); + + priv->timeout_expire_source = nm_g_timeout_add_source_approx(timeout_msec, + TIMEOUT_APPROX_THRESHOLD_SEC, + timeout_expire_cb, + ndisc); } - if (changed) + if (changed != NM_NDISC_CONFIG_NONE) nm_ndisc_emit_config_change(ndisc, changed); } static gboolean -timeout_cb(gpointer user_data) +timeout_expire_cb(gpointer user_data) { - NMNDisc *self = user_data; - - NM_NDISC_GET_PRIVATE(self)->timeout_id = 0; - check_timestamps(self, nm_utils_get_monotonic_timestamp_sec(), 0); - return G_SOURCE_REMOVE; + check_timestamps(user_data, nm_utils_get_monotonic_timestamp_msec(), NM_NDISC_CONFIG_NONE); + return G_SOURCE_CONTINUE; } void -nm_ndisc_ra_received(NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed) +nm_ndisc_ra_received(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap changed) { NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE(ndisc); nm_clear_g_source_inst(&priv->ra_timeout_source); nm_clear_g_source(&priv->send_rs_id); nm_clear_g_free(&priv->last_error); - check_timestamps(ndisc, now, changed); + check_timestamps(ndisc, now_msec, changed); } void @@ -1525,7 +1569,7 @@ dispose(GObject *object) nm_clear_g_source(&priv->send_ra_id); nm_clear_g_free(&priv->last_error); - nm_clear_g_source(&priv->timeout_id); + nm_clear_g_source_inst(&priv->timeout_expire_source); G_OBJECT_CLASS(nm_ndisc_parent_class)->dispose(object); } diff --git a/src/ndisc/nm-ndisc.h b/src/ndisc/nm-ndisc.h index fd0fd98681..74b48b90b9 100644 --- a/src/ndisc/nm-ndisc.h +++ b/src/ndisc/nm-ndisc.h @@ -48,48 +48,91 @@ 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 +#define NM_NDISC_INFINITY_U32 ((uint32_t) -1) -struct _NMNDiscGateway { +/* It's important that this is G_MAXINT64, so that we can meaningfully do + * MIN(e1, e2) to find the minimum expiry time (and properly handle if any + * of them is infinity). + * + * While usually you assign this to "expiry_msec", you might say the + * unit of it is milliseconds. But of course, infinity has not really a unit. */ +#define NM_NDISC_EXPIRY_INFINITY G_MAXINT64 + +/* in common cases, the expiry_msec tracks the timestamp in nm_utils_get_monotonic_timestamp_mses() + * timestamp when the item expires. + * + * When we configure an NMNDiscAddress to be announced via the router advertisement, + * then that address does not have a fixed expiry point in time, instead, the expiry + * really contains the lifetime from the moment when we send the router advertisement. + * In that case, the expiry_msec is more a "lifetime" that starts counting at timestamp + * zero. + * + * The unit is milliseconds (but of course, the timestamp is zero, so it doesn't really matter). */ +#define NM_NDISC_EXPIRY_BASE_TIMESTAMP ((gint64) 0) + +static inline gint64 +_nm_ndisc_lifetime_to_expiry(gint64 now_msec, guint32 lifetime) +{ + if (lifetime == NM_NDISC_INFINITY_U32) + return NM_NDISC_EXPIRY_INFINITY; + return now_msec + (((gint64) lifetime) * 1000); +} + +static inline gint64 +_nm_ndisc_lifetime_from_expiry(gint64 now_msec, gint64 expiry_msec, gboolean ceil) +{ + gint64 diff; + + if (expiry_msec == NM_NDISC_EXPIRY_INFINITY) + return NM_NDISC_INFINITY_U32; + + /* we don't expect nor handle integer overflow. The time stamp and expiry + * should be reasonably small so that it cannot happen. */ + + diff = expiry_msec - now_msec; + + if (diff <= 0) + return 0; + + if (ceil) { + /* we ceil() towards the next full second (instead of floor()). */ + diff += 999; + } + + return NM_MIN(diff / 1000, (gint64)(G_MAXUINT32 - 1)); +} + +/*****************************************************************************/ + +typedef struct _NMNDiscGateway { struct in6_addr address; - guint32 timestamp; - guint32 lifetime; + gint64 expiry_msec; NMIcmpv6RouterPref preference; -}; -typedef struct _NMNDiscGateway NMNDiscGateway; +} NMNDiscGateway; -struct _NMNDiscAddress { +typedef struct _NMNDiscAddress { struct in6_addr address; + gint64 expiry_msec; + gint64 expiry_preferred_msec; guint8 dad_counter; - guint32 timestamp; - guint32 lifetime; - guint32 preferred; -}; -typedef struct _NMNDiscAddress NMNDiscAddress; +} NMNDiscAddress; -struct _NMNDiscRoute { +typedef struct _NMNDiscRoute { struct in6_addr network; - guint8 plen; struct in6_addr gateway; - guint32 timestamp; - guint32 lifetime; + gint64 expiry_msec; NMIcmpv6RouterPref preference; -}; -typedef struct _NMNDiscRoute NMNDiscRoute; + guint8 plen; +} NMNDiscRoute; typedef struct { struct in6_addr address; - guint32 timestamp; - guint32 lifetime; + gint64 expiry_msec; } NMNDiscDNSServer; typedef struct { - char * domain; - guint32 timestamp; - guint32 lifetime; + char * domain; + gint64 expiry_msec; } NMNDiscDNSDomain; typedef enum { @@ -113,6 +156,7 @@ typedef enum { } NMNDiscNodeType; #define NM_NDISC_MAX_ADDRESSES_DEFAULT 16 +#define NM_NDISC_MAX_RTR_SOLICITATION_DELAY 1 /* RFC4861 MAX_RTR_SOLICITATION_DELAY */ #define NM_NDISC_ROUTER_SOLICITATIONS_DEFAULT 3 /* RFC4861 MAX_RTR_SOLICITATIONS */ #define NM_NDISC_ROUTER_SOLICITATION_INTERVAL_DEFAULT 4 /* RFC4861 RTR_SOLICITATION_INTERVAL */ #define NM_NDISC_ROUTER_ADVERTISEMENTS_DEFAULT 3 /* RFC4861 MAX_INITIAL_RTR_ADVERTISEMENTS */ diff --git a/src/ndisc/tests/test-ndisc-fake.c b/src/ndisc/tests/test-ndisc-fake.c index 7deec0ca34..408fc420e0 100644 --- a/src/ndisc/tests/test-ndisc-fake.c +++ b/src/ndisc/tests/test-ndisc-fake.c @@ -14,6 +14,8 @@ #include "nm-test-utils-core.h" +/*****************************************************************************/ + static NMFakeNDisc * ndisc_new(void) { @@ -26,19 +28,20 @@ ndisc_new(void) iid.id_u8[7] = 1; nm_ndisc_set_iid(ndisc, iid); g_assert(ndisc); + return NM_FAKE_NDISC(ndisc); } +/*****************************************************************************/ + static void match_gateway(const NMNDiscData *rdata, guint idx, const char * addr, - guint32 ts, - guint32 lt, + gint64 expiry_msec, NMIcmpv6RouterPref pref) { const NMNDiscGateway *gw; - char buf[INET6_ADDRSTRLEN]; g_assert(rdata); g_assert_cmpint(idx, <, rdata->gateways_n); @@ -46,63 +49,57 @@ match_gateway(const NMNDiscData *rdata, gw = &rdata->gateways[idx]; - g_assert_cmpstr(inet_ntop(AF_INET6, &gw->address, buf, sizeof(buf)), ==, addr); - g_assert_cmpint(gw->timestamp, ==, ts); - g_assert_cmpint(gw->lifetime, ==, lt); + nmtst_assert_ip6_address(&gw->address, addr); + g_assert_cmpint(gw->expiry_msec, ==, expiry_msec); g_assert_cmpint(gw->preference, ==, pref); } -#define match_address(rdata, idx, addr, ts, lt, pref) \ - G_STMT_START \ - { \ - const NMNDiscData * _rdata = (rdata); \ - guint _idx = (idx); \ - const NMNDiscAddress *_a; \ - guint _ts = (ts); \ - \ - g_assert(_rdata); \ - g_assert_cmpint(_idx, <, _rdata->addresses_n); \ - g_assert(_rdata->addresses); \ - \ - _a = &_rdata->addresses[_idx]; \ - \ - nmtst_assert_ip6_address(&_a->address, (addr)); \ - g_assert_cmpint(_a->timestamp, <=, _ts + 1); \ - g_assert_cmpint((int) _a->timestamp, >=, (int) _ts - 1); \ - g_assert_cmpint(_a->timestamp + _a->lifetime, ==, _ts + (lt)); \ - g_assert_cmpint(_a->timestamp + _a->preferred, ==, _ts + (pref)); \ - } \ +#define match_address(rdata, idx, addr, _expiry_msec, _expiry_preferred_msec) \ + G_STMT_START \ + { \ + const NMNDiscData * _rdata = (rdata); \ + guint _idx = (idx); \ + const NMNDiscAddress *_a; \ + \ + g_assert(_rdata); \ + g_assert_cmpint(_idx, <, _rdata->addresses_n); \ + g_assert(_rdata->addresses); \ + \ + _a = &_rdata->addresses[_idx]; \ + \ + nmtst_assert_ip6_address(&_a->address, (addr)); \ + g_assert_cmpint(_a->expiry_msec, ==, (_expiry_msec)); \ + g_assert_cmpint(_a->expiry_preferred_msec, ==, (_expiry_preferred_msec)); \ + } \ G_STMT_END -#define match_route(rdata, idx, nw, pl, gw, ts, lt, pref) \ - G_STMT_START \ - { \ - const NMNDiscData * _rdata = (rdata); \ - guint _idx = (idx); \ - const NMNDiscRoute *_r; \ - int _plen = (pl); \ - \ - g_assert(_rdata); \ - g_assert_cmpint(_idx, <, _rdata->routes_n); \ - g_assert(_rdata->routes); \ - g_assert(_plen > 0 && _plen <= 128); \ - \ - _r = &_rdata->routes[idx]; \ - \ - nmtst_assert_ip6_address(&_r->network, (nw)); \ - g_assert_cmpint((int) _r->plen, ==, _plen); \ - nmtst_assert_ip6_address(&_r->gateway, (gw)); \ - g_assert_cmpint(_r->timestamp, ==, (ts)); \ - g_assert_cmpint(_r->lifetime, ==, (lt)); \ - g_assert_cmpint(_r->preference, ==, (pref)); \ - } \ +#define match_route(rdata, idx, nw, pl, gw, _expiry_msec, pref) \ + G_STMT_START \ + { \ + const NMNDiscData * _rdata = (rdata); \ + guint _idx = (idx); \ + const NMNDiscRoute *_r; \ + int _plen = (pl); \ + \ + g_assert(_rdata); \ + g_assert_cmpint(_idx, <, _rdata->routes_n); \ + g_assert(_rdata->routes); \ + g_assert(_plen > 0 && _plen <= 128); \ + \ + _r = &_rdata->routes[idx]; \ + \ + nmtst_assert_ip6_address(&_r->network, (nw)); \ + g_assert_cmpint((int) _r->plen, ==, _plen); \ + nmtst_assert_ip6_address(&_r->gateway, (gw)); \ + g_assert_cmpint(_r->expiry_msec, ==, (_expiry_msec)); \ + g_assert_cmpint(_r->preference, ==, (pref)); \ + } \ G_STMT_END static void -match_dns_server(const NMNDiscData *rdata, guint idx, const char *addr, guint32 ts, guint32 lt) +match_dns_server(const NMNDiscData *rdata, guint idx, const char *addr, gint64 expiry_msec) { const NMNDiscDNSServer *dns; - char buf[INET6_ADDRSTRLEN]; g_assert(rdata); g_assert_cmpint(idx, <, rdata->dns_servers_n); @@ -110,13 +107,12 @@ match_dns_server(const NMNDiscData *rdata, guint idx, const char *addr, guint32 dns = &rdata->dns_servers[idx]; - g_assert_cmpstr(inet_ntop(AF_INET6, &dns->address, buf, sizeof(buf)), ==, addr); - g_assert_cmpint(dns->timestamp, ==, ts); - g_assert_cmpint(dns->lifetime, ==, lt); + nmtst_assert_ip6_address(&dns->address, addr); + g_assert_cmpint(dns->expiry_msec, ==, expiry_msec); } static void -match_dns_domain(const NMNDiscData *rdata, guint idx, const char *domain, guint32 ts, guint32 lt) +match_dns_domain(const NMNDiscData *rdata, guint idx, const char *domain, gint64 expiry_msec) { const NMNDiscDNSDomain *dns; @@ -127,67 +123,96 @@ match_dns_domain(const NMNDiscData *rdata, guint idx, const char *domain, guint3 dns = &rdata->dns_domains[idx]; g_assert_cmpstr(dns->domain, ==, domain); - g_assert_cmpint(dns->timestamp, ==, ts); - g_assert_cmpint(dns->lifetime, ==, lt); + g_assert_cmpint(dns->expiry_msec, ==, expiry_msec); } +/*****************************************************************************/ + typedef struct { GMainLoop *loop; + gint64 timestamp_msec_1; guint counter; guint rs_counter; - guint32 timestamp1; - guint32 first_solicit; + gint64 first_solicit_msec; guint32 timeout_id; } TestData; +/*****************************************************************************/ + static void test_simple_changed(NMNDisc *ndisc, const NMNDiscData *rdata, guint changed_int, TestData *data) { NMNDiscConfigMap changed = changed_int; - g_assert_cmpint(changed, - ==, - NM_NDISC_CONFIG_DHCP_LEVEL | NM_NDISC_CONFIG_GATEWAYS - | NM_NDISC_CONFIG_ADDRESSES | NM_NDISC_CONFIG_ROUTES - | NM_NDISC_CONFIG_DNS_SERVERS | NM_NDISC_CONFIG_DNS_DOMAINS - | NM_NDISC_CONFIG_HOP_LIMIT | NM_NDISC_CONFIG_MTU); - g_assert_cmpint(rdata->dhcp_level, ==, NM_NDISC_DHCP_LEVEL_OTHERCONF); - match_gateway(rdata, 0, "fe80::1", data->timestamp1, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); - match_address(rdata, 0, "2001:db8:a:a::1", data->timestamp1, 10, 10); - match_route(rdata, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1, 10, 10); - match_dns_server(rdata, 0, "2001:db8:c:c::1", data->timestamp1, 10); - match_dns_domain(rdata, 0, "foobar.com", data->timestamp1, 10); + switch (data->counter++) { + case 0: + g_assert_cmpint(changed, + ==, + NM_NDISC_CONFIG_DHCP_LEVEL | NM_NDISC_CONFIG_GATEWAYS + | NM_NDISC_CONFIG_ADDRESSES | NM_NDISC_CONFIG_ROUTES + | NM_NDISC_CONFIG_DNS_SERVERS | NM_NDISC_CONFIG_DNS_DOMAINS + | NM_NDISC_CONFIG_HOP_LIMIT | NM_NDISC_CONFIG_MTU); + g_assert_cmpint(rdata->dhcp_level, ==, NM_NDISC_DHCP_LEVEL_OTHERCONF); + match_gateway(rdata, + 0, + "fe80::1", + data->timestamp_msec_1 + 10000, + NM_ICMPV6_ROUTER_PREF_MEDIUM); + match_address(rdata, + 0, + "2001:db8:a:a::1", + data->timestamp_msec_1 + 10000, + data->timestamp_msec_1 + 10000); + match_route(rdata, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp_msec_1 + 10000, 10); + match_dns_server(rdata, 0, "2001:db8:c:c::1", data->timestamp_msec_1 + 10000); + match_dns_domain(rdata, 0, "foobar.com", data->timestamp_msec_1 + 3500); - g_assert(nm_fake_ndisc_done(NM_FAKE_NDISC(ndisc))); - data->counter++; - g_main_loop_quit(data->loop); + g_assert(nm_fake_ndisc_done(NM_FAKE_NDISC(ndisc))); + break; + case 1: + g_main_loop_quit(data->loop); + break; + default: + g_assert_not_reached(); + } } static void test_simple(void) { - NMFakeNDisc *ndisc = ndisc_new(); - guint32 now = nm_utils_get_monotonic_timestamp_sec(); - TestData data = {g_main_loop_new(NULL, FALSE), 0, 0, now}; - guint id; + nm_auto_unref_gmainloop GMainLoop *loop = g_main_loop_new(NULL, FALSE); + gs_unref_object NMFakeNDisc *ndisc = ndisc_new(); + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); + TestData data = { + .loop = loop, + .timestamp_msec_1 = now_msec, + }; + guint id; id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_OTHERCONF, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:a::", 64, "fe80::1", now, 10, 10, 10); - nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::1", now, 10); - nm_fake_ndisc_add_dns_domain(ndisc, id, "foobar.com", now, 10); + + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now_msec + 10000, NM_ICMPV6_ROUTER_PREF_MEDIUM); + nm_fake_ndisc_add_prefix(ndisc, + id, + "2001:db8:a:a::", + 64, + "fe80::1", + now_msec + 10000, + now_msec + 10000, + 10); + nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::1", now_msec + 10000); + nm_fake_ndisc_add_dns_domain(ndisc, id, "foobar.com", now_msec + 3500); g_signal_connect(ndisc, NM_NDISC_CONFIG_RECEIVED, G_CALLBACK(test_simple_changed), &data); nm_ndisc_start(NM_NDISC(ndisc)); nmtst_main_loop_run_assert(data.loop, 15000); - g_assert_cmpint(data.counter, ==, 1); - - g_object_unref(ndisc); - g_main_loop_unref(data.loop); + g_assert_cmpint(data.counter, ==, 2); } +/*****************************************************************************/ + static void test_everything_rs_sent(NMNDisc *ndisc, TestData *data) { @@ -208,11 +233,19 @@ test_everything_changed(NMNDisc *ndisc, const NMNDiscData *rdata, guint changed_ | NM_NDISC_CONFIG_ADDRESSES | NM_NDISC_CONFIG_ROUTES | NM_NDISC_CONFIG_DNS_SERVERS | NM_NDISC_CONFIG_DNS_DOMAINS | NM_NDISC_CONFIG_HOP_LIMIT | NM_NDISC_CONFIG_MTU); - match_gateway(rdata, 0, "fe80::1", data->timestamp1, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); - match_address(rdata, 0, "2001:db8:a:a::1", data->timestamp1, 10, 10); - match_route(rdata, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1, 10, 10); - match_dns_server(rdata, 0, "2001:db8:c:c::1", data->timestamp1, 10); - match_dns_domain(rdata, 0, "foobar.com", data->timestamp1, 10); + match_gateway(rdata, + 0, + "fe80::1", + data->timestamp_msec_1 + 10000, + NM_ICMPV6_ROUTER_PREF_MEDIUM); + match_address(rdata, + 0, + "2001:db8:a:a::1", + data->timestamp_msec_1 + 10000, + data->timestamp_msec_1 + 10000); + match_route(rdata, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp_msec_1 + 10000, 10); + match_dns_server(rdata, 0, "2001:db8:c:c::1", data->timestamp_msec_1 + 10000); + match_dns_domain(rdata, 0, "foobar.com", data->timestamp_msec_1 + 10000); } else if (data->counter == 1) { g_assert_cmpint(changed, ==, @@ -221,16 +254,28 @@ test_everything_changed(NMNDisc *ndisc, const NMNDiscData *rdata, guint changed_ | NM_NDISC_CONFIG_DNS_DOMAINS); g_assert_cmpint(rdata->gateways_n, ==, 1); - match_gateway(rdata, 0, "fe80::2", data->timestamp1, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); + match_gateway(rdata, + 0, + "fe80::2", + data->timestamp_msec_1 + 10000, + NM_ICMPV6_ROUTER_PREF_MEDIUM); 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); + match_address(rdata, + 0, + "2001:db8:a:a::1", + data->timestamp_msec_1 + 10000, + data->timestamp_msec_1); + match_address(rdata, + 1, + "2001:db8:a:b::1", + data->timestamp_msec_1 + 10000, + data->timestamp_msec_1 + 10000); g_assert_cmpint(rdata->routes_n, ==, 1); - match_route(rdata, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp1, 10, 10); + match_route(rdata, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp_msec_1 + 10000, 10); g_assert_cmpint(rdata->dns_servers_n, ==, 1); - match_dns_server(rdata, 0, "2001:db8:c:c::2", data->timestamp1, 10); + match_dns_server(rdata, 0, "2001:db8:c:c::2", data->timestamp_msec_1 + 10000); g_assert_cmpint(rdata->dns_domains_n, ==, 1); - match_dns_domain(rdata, 0, "foobar2.com", data->timestamp1, 10); + match_dns_domain(rdata, 0, "foobar2.com", data->timestamp_msec_1 + 10000); g_assert(nm_fake_ndisc_done(NM_FAKE_NDISC(ndisc))); g_main_loop_quit(data->loop); @@ -243,31 +288,49 @@ test_everything_changed(NMNDisc *ndisc, const NMNDiscData *rdata, guint changed_ static void test_everything(void) { - NMFakeNDisc *ndisc = ndisc_new(); - guint32 now = nm_utils_get_monotonic_timestamp_sec(); - TestData data = {g_main_loop_new(NULL, FALSE), 0, 0, now}; - guint id; + nm_auto_unref_gmainloop GMainLoop *loop = g_main_loop_new(NULL, FALSE); + gs_unref_object NMFakeNDisc *ndisc = ndisc_new(); + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); + TestData data = { + .loop = loop, + .timestamp_msec_1 = now_msec, + }; + guint id; id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:a::", 64, "fe80::1", now, 10, 10, 10); - nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::1", now, 10); - nm_fake_ndisc_add_dns_domain(ndisc, id, "foobar.com", now, 10); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now_msec + 10000, NM_ICMPV6_ROUTER_PREF_MEDIUM); + nm_fake_ndisc_add_prefix(ndisc, + id, + "2001:db8:a:a::", + 64, + "fe80::1", + now_msec + 10000, + now_msec + 10000, + 10); + nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::1", now_msec + 10000); + nm_fake_ndisc_add_dns_domain(ndisc, id, "foobar.com", now_msec + 10000); /* expire everything from the first RA in the second */ id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now, 0, NM_ICMPV6_ROUTER_PREF_MEDIUM); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:a::", 64, "fe80::1", now, 0, 0, 0); - nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::1", now, 0); - nm_fake_ndisc_add_dns_domain(ndisc, id, "foobar.com", now, 0); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now_msec, NM_ICMPV6_ROUTER_PREF_MEDIUM); + nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:a::", 64, "fe80::1", now_msec, now_msec, 0); + nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::1", now_msec); + nm_fake_ndisc_add_dns_domain(ndisc, id, "foobar.com", now_msec); /* and add some new stuff */ - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::2", now, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:b::", 64, "fe80::2", now, 10, 10, 10); - nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::2", now, 10); - nm_fake_ndisc_add_dns_domain(ndisc, id, "foobar2.com", now, 10); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::2", now_msec + 10000, NM_ICMPV6_ROUTER_PREF_MEDIUM); + nm_fake_ndisc_add_prefix(ndisc, + id, + "2001:db8:a:b::", + 64, + "fe80::2", + now_msec + 10000, + now_msec + 10000, + 10); + nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::2", now_msec + 10000); + nm_fake_ndisc_add_dns_domain(ndisc, id, "foobar2.com", now_msec + 10000); g_signal_connect(ndisc, NM_NDISC_CONFIG_RECEIVED, G_CALLBACK(test_everything_changed), &data); g_signal_connect(ndisc, NM_FAKE_NDISC_RS_SENT, G_CALLBACK(test_everything_rs_sent), &data); @@ -276,9 +339,6 @@ test_everything(void) nmtst_main_loop_run_assert(data.loop, 15000); g_assert_cmpint(data.counter, ==, 2); g_assert_cmpint(data.rs_counter, ==, 1); - - g_object_unref(ndisc); - g_main_loop_unref(data.loop); } static void @@ -296,14 +356,30 @@ test_preference_order_cb(NMNDisc * ndisc, | NM_NDISC_CONFIG_ROUTES); g_assert_cmpint(rdata->gateways_n, ==, 2); - match_gateway(rdata, 0, "fe80::1", data->timestamp1, 10, NM_ICMPV6_ROUTER_PREF_HIGH); - match_gateway(rdata, 1, "fe80::2", data->timestamp1 + 1, 10, NM_ICMPV6_ROUTER_PREF_LOW); + match_gateway(rdata, + 0, + "fe80::1", + data->timestamp_msec_1 + 10000, + NM_ICMPV6_ROUTER_PREF_HIGH); + match_gateway(rdata, + 1, + "fe80::2", + data->timestamp_msec_1 + 11000, + NM_ICMPV6_ROUTER_PREF_LOW); g_assert_cmpint(rdata->addresses_n, ==, 2); - match_address(rdata, 0, "2001:db8:a:a::1", data->timestamp1, 10, 10); - match_address(rdata, 1, "2001:db8:a:b::1", data->timestamp1 + 1, 10, 10); + match_address(rdata, + 0, + "2001:db8:a:a::1", + data->timestamp_msec_1 + 10000, + data->timestamp_msec_1 + 10000); + match_address(rdata, + 1, + "2001:db8:a:b::1", + data->timestamp_msec_1 + 11000, + data->timestamp_msec_1 + 10000); g_assert_cmpint(rdata->routes_n, ==, 2); - match_route(rdata, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp1 + 1, 10, 10); - match_route(rdata, 1, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1, 10, 5); + match_route(rdata, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp_msec_1 + 11000, 10); + match_route(rdata, 1, "2001:db8:a:a::", 64, "fe80::1", data->timestamp_msec_1 + 10000, 5); g_assert(nm_fake_ndisc_done(NM_FAKE_NDISC(ndisc))); g_main_loop_quit(data->loop); @@ -315,31 +391,46 @@ test_preference_order_cb(NMNDisc * ndisc, static void test_preference_order(void) { - NMFakeNDisc *ndisc = ndisc_new(); - guint32 now = nm_utils_get_monotonic_timestamp_sec(); - TestData data = {g_main_loop_new(NULL, FALSE), 0, 0, now}; - guint id; + nm_auto_unref_gmainloop GMainLoop *loop = g_main_loop_new(NULL, FALSE); + gs_unref_object NMFakeNDisc *ndisc = ndisc_new(); + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); + TestData data = { + .loop = loop, + .timestamp_msec_1 = now_msec, + }; + guint id; /* Test insertion order of gateways */ id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now, 10, NM_ICMPV6_ROUTER_PREF_HIGH); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:a::", 64, "fe80::1", now, 10, 10, 5); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now_msec + 10000, NM_ICMPV6_ROUTER_PREF_HIGH); + nm_fake_ndisc_add_prefix(ndisc, + id, + "2001:db8:a:a::", + 64, + "fe80::1", + now_msec + 10000, + now_msec + 10000, + 5); id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::2", ++now, 10, NM_ICMPV6_ROUTER_PREF_LOW); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:b::", 64, "fe80::2", now, 10, 10, 10); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::2", now_msec + 11000, NM_ICMPV6_ROUTER_PREF_LOW); + nm_fake_ndisc_add_prefix(ndisc, + id, + "2001:db8:a:b::", + 64, + "fe80::2", + now_msec + 11000, + now_msec + 10000, + 10); g_signal_connect(ndisc, NM_NDISC_CONFIG_RECEIVED, G_CALLBACK(test_preference_order_cb), &data); nm_ndisc_start(NM_NDISC(ndisc)); nmtst_main_loop_run_assert(data.loop, 15000); g_assert_cmpint(data.counter, ==, 2); - - g_object_unref(ndisc); - g_main_loop_unref(data.loop); } static void @@ -356,14 +447,30 @@ test_preference_changed_cb(NMNDisc * ndisc, NM_NDISC_CONFIG_GATEWAYS | NM_NDISC_CONFIG_ADDRESSES | NM_NDISC_CONFIG_ROUTES); g_assert_cmpint(rdata->gateways_n, ==, 2); - match_gateway(rdata, 0, "fe80::2", data->timestamp1 + 1, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); - match_gateway(rdata, 1, "fe80::1", data->timestamp1, 10, NM_ICMPV6_ROUTER_PREF_LOW); + match_gateway(rdata, + 0, + "fe80::2", + data->timestamp_msec_1 + 11000, + NM_ICMPV6_ROUTER_PREF_MEDIUM); + match_gateway(rdata, + 1, + "fe80::1", + data->timestamp_msec_1 + 10000, + NM_ICMPV6_ROUTER_PREF_LOW); g_assert_cmpint(rdata->addresses_n, ==, 2); - match_address(rdata, 0, "2001:db8:a:a::1", data->timestamp1, 10, 10); - match_address(rdata, 1, "2001:db8:a:b::1", data->timestamp1 + 1, 10, 10); + match_address(rdata, + 0, + "2001:db8:a:a::1", + data->timestamp_msec_1 + 10000, + data->timestamp_msec_1 + 10000); + match_address(rdata, + 1, + "2001:db8:a:b::1", + data->timestamp_msec_1 + 11000, + data->timestamp_msec_1 + 11000); g_assert_cmpint(rdata->routes_n, ==, 2); - match_route(rdata, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp1 + 1, 10, 10); - match_route(rdata, 1, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1, 10, 5); + match_route(rdata, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp_msec_1 + 11000, 10); + match_route(rdata, 1, "2001:db8:a:a::", 64, "fe80::1", data->timestamp_msec_1 + 10000, 5); } else if (data->counter == 2) { g_assert_cmpint(changed, ==, @@ -371,14 +478,30 @@ test_preference_changed_cb(NMNDisc * ndisc, | NM_NDISC_CONFIG_ROUTES); g_assert_cmpint(rdata->gateways_n, ==, 2); - 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); + match_gateway(rdata, + 0, + "fe80::1", + data->timestamp_msec_1 + 12000, + NM_ICMPV6_ROUTER_PREF_HIGH); + match_gateway(rdata, + 1, + "fe80::2", + data->timestamp_msec_1 + 11000, + NM_ICMPV6_ROUTER_PREF_MEDIUM); g_assert_cmpint(rdata->addresses_n, ==, 2); - 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); + match_address(rdata, + 0, + "2001:db8:a:a::1", + data->timestamp_msec_1 + 12000, + data->timestamp_msec_1 + 12000); + match_address(rdata, + 1, + "2001:db8:a:b::1", + data->timestamp_msec_1 + 11000, + data->timestamp_msec_1 + 11000); g_assert_cmpint(rdata->routes_n, ==, 2); - match_route(rdata, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1 + 2, 10, 15); - match_route(rdata, 1, "2001:db8:a:b::", 64, "fe80::2", data->timestamp1 + 1, 10, 10); + match_route(rdata, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp_msec_1 + 12000, 15); + match_route(rdata, 1, "2001:db8:a:b::", 64, "fe80::2", data->timestamp_msec_1 + 11000, 10); g_assert(nm_fake_ndisc_done(NM_FAKE_NDISC(ndisc))); g_main_loop_quit(data->loop); @@ -390,10 +513,14 @@ test_preference_changed_cb(NMNDisc * ndisc, static void test_preference_changed(void) { - NMFakeNDisc *ndisc = ndisc_new(); - guint32 now = nm_utils_get_monotonic_timestamp_sec(); - TestData data = {g_main_loop_new(NULL, FALSE), 0, 0, now}; - guint id; + nm_auto_unref_gmainloop GMainLoop *loop = g_main_loop_new(NULL, FALSE); + gs_unref_object NMFakeNDisc *ndisc = ndisc_new(); + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); + TestData data = { + .loop = loop, + .timestamp_msec_1 = now_msec, + }; + guint id; /* Test that when a low-preference and medium gateway send advertisements, * that if the low-preference gateway switches to high-preference, we do @@ -402,18 +529,39 @@ test_preference_changed(void) id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now, 10, NM_ICMPV6_ROUTER_PREF_LOW); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:a::", 64, "fe80::1", now, 10, 10, 5); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now_msec + 10000, NM_ICMPV6_ROUTER_PREF_LOW); + nm_fake_ndisc_add_prefix(ndisc, + id, + "2001:db8:a:a::", + 64, + "fe80::1", + now_msec + 10000, + now_msec + 10000, + 5); id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::2", ++now, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:b::", 64, "fe80::2", now, 10, 10, 10); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::2", now_msec + 11000, NM_ICMPV6_ROUTER_PREF_MEDIUM); + nm_fake_ndisc_add_prefix(ndisc, + id, + "2001:db8:a:b::", + 64, + "fe80::2", + now_msec + 11000, + now_msec + 11000, + 10); id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", ++now, 10, NM_ICMPV6_ROUTER_PREF_HIGH); - nm_fake_ndisc_add_prefix(ndisc, id, "2001:db8:a:a::", 64, "fe80::1", now, 10, 10, 15); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now_msec + 12000, NM_ICMPV6_ROUTER_PREF_HIGH); + nm_fake_ndisc_add_prefix(ndisc, + id, + "2001:db8:a:a::", + 64, + "fe80::1", + now_msec + 12000, + now_msec + 12000, + 15); g_signal_connect(ndisc, NM_NDISC_CONFIG_RECEIVED, @@ -423,16 +571,15 @@ test_preference_changed(void) nm_ndisc_start(NM_NDISC(ndisc)); nmtst_main_loop_run_assert(data.loop, 15000); g_assert_cmpint(data.counter, ==, 3); - - g_object_unref(ndisc); - g_main_loop_unref(data.loop); } +/*****************************************************************************/ + static void -test_dns_solicit_loop_changed(NMNDisc * ndisc, - const NMNDiscData *rdata, - guint changed_int, - TestData * data) +_test_dns_solicit_loop_changed(NMNDisc * ndisc, + const NMNDiscData *rdata, + guint changed_int, + TestData * data) { data->counter++; } @@ -446,14 +593,14 @@ success_timeout(TestData *data) } static void -test_dns_solicit_loop_rs_sent(NMFakeNDisc *ndisc, TestData *data) +_test_dns_solicit_loop_rs_sent(NMFakeNDisc *ndisc, TestData *data) { - guint32 now = nm_utils_get_monotonic_timestamp_sec(); - guint id; + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); + guint id; if (data->rs_counter > 0 && data->rs_counter < 6) { if (data->rs_counter == 1) { - data->first_solicit = now; + data->first_solicit_msec = now_msec; /* Kill the test after 10 seconds if it hasn't failed yet */ data->timeout_id = g_timeout_add_seconds(10, (GSourceFunc) success_timeout, data); } @@ -464,12 +611,16 @@ test_dns_solicit_loop_rs_sent(NMFakeNDisc *ndisc, TestData *data) */ id = nm_fake_ndisc_add_ra(ndisc, 0, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM); + nm_fake_ndisc_add_gateway(ndisc, + id, + "fe80::1", + now_msec + 10000, + NM_ICMPV6_ROUTER_PREF_MEDIUM); nm_fake_ndisc_emit_new_ras(ndisc); } 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_assert_cmpint(now_msec - data->first_solicit_msec, >, 4000); g_source_remove(data->timeout_id); g_main_loop_quit(data->loop); } @@ -479,10 +630,14 @@ test_dns_solicit_loop_rs_sent(NMFakeNDisc *ndisc, TestData *data) static void test_dns_solicit_loop(void) { - NMFakeNDisc *ndisc = ndisc_new(); - guint32 now = nm_utils_get_monotonic_timestamp_sec(); - TestData data = {g_main_loop_new(NULL, FALSE), 0, 0, now, 0}; - guint id; + nm_auto_unref_gmainloop GMainLoop *loop = g_main_loop_new(NULL, FALSE); + gs_unref_object NMFakeNDisc *ndisc = ndisc_new(); + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); + TestData data = { + .loop = loop, + .timestamp_msec_1 = now_msec, + }; + guint id; g_test_skip("The solicitation behavior is wrong and need fixing. This test is not working too"); return; @@ -496,26 +651,25 @@ test_dns_solicit_loop(void) id = nm_fake_ndisc_add_ra(ndisc, 1, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now, 10, NM_ICMPV6_ROUTER_PREF_LOW); - nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::1", now, 6); + nm_fake_ndisc_add_gateway(ndisc, id, "fe80::1", now_msec + 10000, NM_ICMPV6_ROUTER_PREF_LOW); + nm_fake_ndisc_add_dns_server(ndisc, id, "2001:db8:c:c::1", now_msec + 6000); g_signal_connect(ndisc, NM_NDISC_CONFIG_RECEIVED, - G_CALLBACK(test_dns_solicit_loop_changed), + G_CALLBACK(_test_dns_solicit_loop_changed), &data); g_signal_connect(ndisc, NM_FAKE_NDISC_RS_SENT, - G_CALLBACK(test_dns_solicit_loop_rs_sent), + G_CALLBACK(_test_dns_solicit_loop_rs_sent), &data); nm_ndisc_start(NM_NDISC(ndisc)); nmtst_main_loop_run_assert(data.loop, 20000); g_assert_cmpint(data.counter, ==, 3); - - g_object_unref(ndisc); - g_main_loop_unref(data.loop); } +/*****************************************************************************/ + NMTST_DEFINE(); int diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index eeb69946a4..d48c6e30c8 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -1497,6 +1497,7 @@ nm_ip6_config_reset_addresses_ndisc(NMIP6Config * self, NMIP6ConfigPrivate *priv; guint i; gboolean changed = FALSE; + gint32 base_time_sec; g_return_if_fail(NM_IS_IP6_CONFIG(self)); @@ -1504,6 +1505,16 @@ nm_ip6_config_reset_addresses_ndisc(NMIP6Config * self, g_return_if_fail(priv->ifindex > 0); + /* the base-timestamp doesn't matter it's only an anchor for the + * expiry. However, try to re-use the same base-time for a while + * by rounding it to 10000 seconds. + * + * That is because we deduplicate and NMPlatformIP6Address instances + * so using the same timestamps is preferable. */ + base_time_sec = nm_utils_get_monotonic_timestamp_sec(); + base_time_sec = (base_time_sec / 10000) * 10000; + base_time_sec = NM_MAX(1, base_time_sec); + nm_dedup_multi_index_dirty_set_idx(priv->multi_idx, &priv->idx_ip6_addresses); for (i = 0; i < addresses_n; i++) { @@ -1516,9 +1527,13 @@ nm_ip6_config_reset_addresses_ndisc(NMIP6Config * self, a->ifindex = priv->ifindex; a->address = ndisc_addr->address; a->plen = plen; - a->timestamp = ndisc_addr->timestamp; - a->lifetime = ndisc_addr->lifetime; - a->preferred = MIN(ndisc_addr->lifetime, ndisc_addr->preferred); + a->timestamp = base_time_sec, + a->lifetime = _nm_ndisc_lifetime_from_expiry(((gint64) base_time_sec) * 1000, + ndisc_addr->expiry_msec, + TRUE), + a->preferred = _nm_ndisc_lifetime_from_expiry(((gint64) base_time_sec) * 1000, + ndisc_addr->expiry_preferred_msec, + TRUE), a->addr_source = NM_IP_CONFIG_SOURCE_NDISC; a->n_ifa_flags = ifa_flags; From 6dad4a315fae9577fba7e346df25776f5e337b69 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 26 Jan 2021 08:40:31 +0100 Subject: [PATCH 08/11] ndisc/trivial: rename defines for defaults from RFC These defaults are mentioned in RFC4861. Use a name that is the same as in the RFC. --- src/ndisc/nm-fake-ndisc.c | 2 +- src/ndisc/nm-lndp-ndisc.c | 13 ++++++------- src/ndisc/nm-ndisc.c | 2 +- src/ndisc/nm-ndisc.h | 21 +++++++++++---------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/ndisc/nm-fake-ndisc.c b/src/ndisc/nm-fake-ndisc.c index e9b329385e..5bcdb0de7a 100644 --- a/src/ndisc/nm-fake-ndisc.c +++ b/src/ndisc/nm-fake-ndisc.c @@ -372,7 +372,7 @@ nm_fake_ndisc_new(int ifindex, const char *ifname) NM_NDISC_ROUTER_SOLICITATIONS, NM_NDISC_ROUTER_SOLICITATIONS_DEFAULT, NM_NDISC_ROUTER_SOLICITATION_INTERVAL, - NM_NDISC_ROUTER_SOLICITATION_INTERVAL_DEFAULT, + NM_NDISC_RFC4861_RTR_SOLICITATION_INTERVAL, NM_NDISC_RA_TIMEOUT, 30u, NULL); diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c index 0a1a5f2274..7abd197c99 100644 --- a/src/ndisc/nm-lndp-ndisc.c +++ b/src/ndisc/nm-lndp-ndisc.c @@ -713,13 +713,12 @@ nm_lndp_ndisc_get_sysctl(NMPlatform *platform, NM_SET_OUT(out_router_solicitations, router_solicitations); } if (out_router_solicitation_interval || out_default_ra_timeout) { - router_solicitation_interval = - ipv6_sysctl_get(platform, - ifname, - "router_solicitation_interval", - 1, - G_MAXINT32, - NM_NDISC_ROUTER_SOLICITATION_INTERVAL_DEFAULT); + router_solicitation_interval = ipv6_sysctl_get(platform, + ifname, + "router_solicitation_interval", + 1, + G_MAXINT32, + NM_NDISC_RFC4861_RTR_SOLICITATION_INTERVAL); NM_SET_OUT(out_router_solicitation_interval, router_solicitation_interval); } if (out_default_ra_timeout) { diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c index a86853c081..8b813010ef 100644 --- a/src/ndisc/nm-ndisc.c +++ b/src/ndisc/nm-ndisc.c @@ -1680,7 +1680,7 @@ nm_ndisc_class_init(NMNDiscClass *klass) "", 1, G_MAXINT32, - NM_NDISC_ROUTER_SOLICITATION_INTERVAL_DEFAULT, + NM_NDISC_RFC4861_RTR_SOLICITATION_INTERVAL, G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); obj_properties[PROP_NODE_TYPE] = g_param_spec_int(NM_NDISC_NODE_TYPE, diff --git a/src/ndisc/nm-ndisc.h b/src/ndisc/nm-ndisc.h index 74b48b90b9..93aee4db38 100644 --- a/src/ndisc/nm-ndisc.h +++ b/src/ndisc/nm-ndisc.h @@ -155,16 +155,17 @@ typedef enum { NM_NDISC_NODE_TYPE_ROUTER, } NMNDiscNodeType; -#define NM_NDISC_MAX_ADDRESSES_DEFAULT 16 -#define NM_NDISC_MAX_RTR_SOLICITATION_DELAY 1 /* RFC4861 MAX_RTR_SOLICITATION_DELAY */ -#define NM_NDISC_ROUTER_SOLICITATIONS_DEFAULT 3 /* RFC4861 MAX_RTR_SOLICITATIONS */ -#define NM_NDISC_ROUTER_SOLICITATION_INTERVAL_DEFAULT 4 /* RFC4861 RTR_SOLICITATION_INTERVAL */ -#define NM_NDISC_ROUTER_ADVERTISEMENTS_DEFAULT 3 /* RFC4861 MAX_INITIAL_RTR_ADVERTISEMENTS */ -#define NM_NDISC_ROUTER_ADVERT_DELAY 3 /* RFC4861 MIN_DELAY_BETWEEN_RAS */ -#define NM_NDISC_ROUTER_ADVERT_INITIAL_INTERVAL 16 /* RFC4861 MAX_INITIAL_RTR_ADVERT_INTERVAL */ -#define NM_NDISC_ROUTER_ADVERT_DELAY_MS 500 /* RFC4861 MAX_RA_DELAY_TIME */ -#define NM_NDISC_ROUTER_ADVERT_MAX_INTERVAL 600 /* RFC4861 MaxRtrAdvInterval default */ -#define NM_NDISC_ROUTER_LIFETIME 900 /* 1.5 * NM_NDISC_ROUTER_ADVERT_MAX_INTERVAL */ +#define NM_NDISC_RFC4861_RTR_SOLICITATION_INTERVAL 4 /* seconds */ +#define NM_NDISC_RFC4861_MAX_RTR_SOLICITATION_DELAY 1 /* seconds */ + +#define NM_NDISC_MAX_ADDRESSES_DEFAULT 16 +#define NM_NDISC_ROUTER_SOLICITATIONS_DEFAULT 3 /* RFC4861, MAX_RTR_SOLICITATIONS */ +#define NM_NDISC_ROUTER_ADVERTISEMENTS_DEFAULT 3 /* RFC4861, MAX_INITIAL_RTR_ADVERTISEMENTS */ +#define NM_NDISC_ROUTER_ADVERT_DELAY 3 /* RFC4861, MIN_DELAY_BETWEEN_RAS */ +#define NM_NDISC_ROUTER_ADVERT_INITIAL_INTERVAL 16 /* RFC4861, MAX_INITIAL_RTR_ADVERT_INTERVAL */ +#define NM_NDISC_ROUTER_ADVERT_DELAY_MS 500 /* RFC4861, MAX_RA_DELAY_TIME */ +#define NM_NDISC_ROUTER_ADVERT_MAX_INTERVAL 600 /* RFC4861, MaxRtrAdvInterval default */ +#define NM_NDISC_ROUTER_LIFETIME 900 /* 1.5 * NM_NDISC_ROUTER_ADVERT_MAX_INTERVAL */ struct _NMNDiscPrivate; struct _NMNDiscDataInternal; From 5ba2f81d2dc5a32148ae6e724fd51094cc408da3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 22 Jan 2021 17:53:23 +0100 Subject: [PATCH 09/11] ndisc: rework sending router solicitations to follow RFC7559 RFC4861 describes how to solicit routers, but this is later extended by RFC7559 (Packet-Loss Resiliency for Router Solicitations). Rework the scheduling of router solicitations to follow RFC7559. Differences from RFC7559: - the initial random delay before sending the first RS is only up to 250 milliseconds (instead of 1 second). - we never completely stop sending RS, even after we receive a valid RA. We will still send RS every hour. We no longer honor the sysctl variables - /proc/sys/net/ipv6/conf/$IFNAME/router_solicitations - /proc/sys/net/ipv6/conf/$IFNAME/router_solicitation_interval I don't think having the autoconf algorithm configurable is useful. At least not, if the configuration happens via sysctl variables. https://tools.ietf.org/html/rfc4861#section-6.3.7 https://tools.ietf.org/html/rfc7559#section-2.1 --- src/ndisc/nm-ndisc.c | 192 ++++++++++++++++++++---------- src/ndisc/tests/test-ndisc-fake.c | 44 +------ 2 files changed, 131 insertions(+), 105 deletions(-) diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c index 8b813010ef..c8e4c7f9c3 100644 --- a/src/ndisc/nm-ndisc.c +++ b/src/ndisc/nm-ndisc.c @@ -20,6 +20,9 @@ #define _NMLOG_PREFIX_NAME "ndisc" +#define RFC7559_IRT ((gint32) 4) /* RFC7559, Initial Retransmission Time, in seconds */ +#define RFC7559_MRT ((gint32) 3600) /* RFC7559, Maximum Retransmission Time, in seconds */ + /*****************************************************************************/ struct _NMNDiscPrivate { @@ -29,18 +32,13 @@ struct _NMNDiscPrivate { char * last_error; GSource *ra_timeout_source; - union { - gint32 solicitations_left; - gint32 announcements_left; - }; - union { - guint send_rs_id; - guint send_ra_id; - }; - union { - gint32 last_rs; - gint32 last_ra; - }; + gint32 announcements_left; + guint send_ra_id; + gint32 last_ra; + + gint32 solicit_retransmit_time_msec; + + GSource *solicit_timer_source; GSource *timeout_expire_source; @@ -773,62 +771,111 @@ nm_ndisc_add_dns_domain(NMNDisc *ndisc, const NMNDiscDNSDomain *new_item, gint64 } \ G_STMT_END -static gboolean -send_rs_timeout(NMNDisc *ndisc) +static gint32 +solicit_retransmit_time_jitter(gint32 solicit_retransmit_time_msec) { - nm_auto_pop_netns NMPNetns *netns = NULL; - NMNDiscClass * klass = NM_NDISC_GET_CLASS(ndisc); - NMNDiscPrivate * priv = NM_NDISC_GET_PRIVATE(ndisc); - GError * error = NULL; + gint32 ten_percent; - priv->send_rs_id = 0; + nm_assert(solicit_retransmit_time_msec > 0); + nm_assert(solicit_retransmit_time_msec < 3 * RFC7559_MRT * 1000); - if (!nm_ndisc_netns_push(ndisc, &netns)) - return G_SOURCE_REMOVE; + /* Add a ±10% jitter. + * + * This is the "RAND" parameter from https://tools.ietf.org/html/rfc3315#section-14 + * as requested by RFC7559. */ - if (klass->send_rs(ndisc, &error)) { - _LOGD("router solicitation sent"); - priv->solicitations_left--; + ten_percent = NM_MAX(1, solicit_retransmit_time_msec / 10); + + return solicit_retransmit_time_msec - ten_percent + + ((gint32)(g_random_int() % (2u * ((guint32) ten_percent)))); +} + +static gboolean +solicit_timer_cb(gpointer user_data) +{ + const gint32 TIMEOUT_APPROX_THRESHOLD_SEC = 10000; + nm_auto_pop_netns NMPNetns *netns = NULL; + NMNDisc * ndisc = user_data; + NMNDiscClass * klass = NM_NDISC_GET_CLASS(ndisc); + NMNDiscPrivate * priv = NM_NDISC_GET_PRIVATE(ndisc); + gs_free_error GError *error = NULL; + gint32 timeout_msec; + + if (!nm_ndisc_netns_push(ndisc, &netns)) { + nm_utils_error_set(&error, + NM_UTILS_ERROR_UNKNOWN, + "failure to switch netns for soliciting routers"); + } else + klass->send_rs(ndisc, &error); + + if (error) + _MAYBE_WARN("solicit: failure sending router solicitation: %s", error->message); + else { + _LOGT("solicit: router solicitation sent"); nm_clear_g_free(&priv->last_error); - } else { - _MAYBE_WARN("failure sending router solicitation: %s", error->message); - g_clear_error(&error); } - priv->last_rs = nm_utils_get_monotonic_timestamp_sec(); - if (priv->solicitations_left > 0) { - _LOGD("scheduling router solicitation retry in %d seconds.", - (int) priv->router_solicitation_interval); - priv->send_rs_id = g_timeout_add_seconds(priv->router_solicitation_interval, - (GSourceFunc) send_rs_timeout, - ndisc); + /* https://tools.ietf.org/html/rfc4861#section-6.3.7 describes how to send solicitations: + * + * > ... a host SHOULD transmit up to MAX_RTR_SOLICITATIONS Router Solicitation messages, + * > each separated by at least RTR_SOLICITATION_INTERVAL seconds. + * + * but this was extended by https://tools.ietf.org/html/rfc7559#section-2 to send continuously + * and with exponential backoff (detailed the algorithm in https://tools.ietf.org/html/rfc3315#section-14). + * We do RFC7559. + */ + if (priv->solicit_retransmit_time_msec == 0) { + G_STATIC_ASSERT(RFC7559_IRT == NM_NDISC_RFC4861_RTR_SOLICITATION_INTERVAL); + priv->solicit_retransmit_time_msec = solicit_retransmit_time_jitter(RFC7559_IRT * 1000); + timeout_msec = priv->solicit_retransmit_time_msec; } else { - _LOGD("did not receive a router advertisement after %d solicitations.", - (int) priv->router_solicitations); + priv->solicit_retransmit_time_msec += + solicit_retransmit_time_jitter(priv->solicit_retransmit_time_msec); + timeout_msec = priv->solicit_retransmit_time_msec; + if (priv->solicit_retransmit_time_msec > RFC7559_MRT * 1000) { + priv->solicit_retransmit_time_msec = RFC7559_MRT * 1000; + timeout_msec = solicit_retransmit_time_jitter(priv->solicit_retransmit_time_msec); + } } - return G_SOURCE_REMOVE; + _LOGD("solicit: schedule sending next solicitation in%s %.3f seconds", + timeout_msec / 1000 >= TIMEOUT_APPROX_THRESHOLD_SEC ? " about" : "", + ((double) timeout_msec) / 1000); + + nm_clear_g_source_inst(&priv->solicit_timer_source); + priv->solicit_timer_source = nm_g_timeout_add_source_approx(timeout_msec, + TIMEOUT_APPROX_THRESHOLD_SEC, + solicit_timer_cb, + ndisc); + return G_SOURCE_CONTINUE; } static void -solicit_routers(NMNDisc *ndisc) +solicit_timer_start(NMNDisc *ndisc) { NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE(ndisc); - gint32 now, next; - gint64 t; + gint32 delay_msec; - if (priv->send_rs_id) - return; + /* rfc4861, Section 6.3.7: + * + * We should randomly wait up to NM_NDISC_RFC4861_MAX_RTR_SOLICITATION_DELAY (1 second) + * before sending the first RS. RFC4861 is from 2007, I don't think 1 second is + * a suitable delay in 2021. Wait only up to 250 msec instead. */ - now = nm_utils_get_monotonic_timestamp_sec(); - priv->solicitations_left = priv->router_solicitations; + delay_msec = + g_random_int() % ((guint32)(NM_NDISC_RFC4861_MAX_RTR_SOLICITATION_DELAY * 1000 / 4)); - t = (((gint64) priv->last_rs) + priv->router_solicitation_interval) - now; - next = CLAMP(t, 0, G_MAXINT32); - _LOGD("scheduling explicit router solicitation request in %" G_GINT32_FORMAT " seconds.", next); - priv->send_rs_id = g_timeout_add_seconds((guint32) next, (GSourceFunc) send_rs_timeout, ndisc); + _LOGD("solicit: schedule sending first solicitation (of %d) in %.3f seconds", + priv->router_solicitations, + ((double) delay_msec) / 1000); + + priv->solicit_retransmit_time_msec = 0; + nm_clear_g_source_inst(&priv->solicit_timer_source); + priv->solicit_timer_source = nm_g_timeout_add_source(delay_msec, solicit_timer_cb, ndisc); } +/*****************************************************************************/ + static gboolean announce_router(NMNDisc *ndisc) { @@ -988,7 +1035,7 @@ nm_ndisc_set_iid(NMNDisc *ndisc, const NMUtilsIPv6IfaceId iid) _LOGD("IPv6 interface identifier changed, flushing addresses"); g_array_remove_range(rdata->addresses, 0, rdata->addresses->len); nm_ndisc_emit_config_change(ndisc, NM_NDISC_CONFIG_ADDRESSES); - solicit_routers(ndisc); + solicit_timer_start(ndisc); } return TRUE; } @@ -1050,7 +1097,7 @@ nm_ndisc_start(NMNDisc *ndisc) g_source_attach(priv->ra_timeout_source, NULL); } - solicit_routers(ndisc); + solicit_timer_start(ndisc); return; } @@ -1088,20 +1135,16 @@ nm_ndisc_stop(NMNDisc *ndisc) g_array_set_size(rdata->dns_domains, 0); priv->rdata.public.hop_limit = 64; - /* Start at very low number so that last_rs - router_solicitation_interval - * is much lower than nm_utils_get_monotonic_timestamp_sec() at startup. - */ - priv->last_rs = G_MININT32; nm_clear_g_source_inst(&priv->ra_timeout_source); - nm_clear_g_source(&priv->send_rs_id); nm_clear_g_source(&priv->send_ra_id); nm_clear_g_free(&priv->last_error); nm_clear_g_source_inst(&priv->timeout_expire_source); - priv->solicitations_left = 0; + priv->solicit_retransmit_time_msec = 0; + nm_clear_g_source_inst(&priv->solicit_timer_source); + priv->announcements_left = 0; - priv->last_rs = G_MININT32; priv->last_ra = G_MININT32; } @@ -1422,6 +1465,33 @@ check_timestamps(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap changed) ndisc); } + /* When we receive an RA, we don't disable solicitations entirely. Instead, + * we set the interval the maximum (RFC7559_MRT). + * + * This contradicts https://tools.ietf.org/html/rfc7559#section-2.1, which says + * that we SHOULD stop sending RS if we receive an RA -- but only on a multicast + * capable link and if the RA has a valid router lifetime. + * + * But we really want to recover from a dead router on the network, so we + * don't want to cease sending RS entirely. + * + * But we only re-schedule the timer if the current interval is not already + * "RFC7559_MRT * 1000". Otherwise, we already have a slow interval counter + * pending. */ + if (priv->solicit_retransmit_time_msec != RFC7559_MRT * 1000) { + gint32 timeout_msec; + + priv->solicit_retransmit_time_msec = RFC7559_MRT * 1000; + timeout_msec = solicit_retransmit_time_jitter(priv->solicit_retransmit_time_msec); + + _LOGD("solicit: schedule sending next (slow) solicitation in about %.3f seconds", + ((double) timeout_msec) / 1000); + + nm_clear_g_source_inst(&priv->solicit_timer_source); + priv->solicit_timer_source = + nm_g_timeout_add_source_approx(timeout_msec, 0, solicit_timer_cb, ndisc); + } + if (changed != NM_NDISC_CONFIG_NONE) nm_ndisc_emit_config_change(ndisc, changed); } @@ -1439,7 +1509,6 @@ nm_ndisc_ra_received(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap changed) NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE(ndisc); nm_clear_g_source_inst(&priv->ra_timeout_source); - nm_clear_g_source(&priv->send_rs_id); nm_clear_g_free(&priv->last_error); check_timestamps(ndisc, now_msec, changed); } @@ -1551,11 +1620,6 @@ nm_ndisc_init(NMNDisc *ndisc) rdata->dns_domains = g_array_new(FALSE, FALSE, sizeof(NMNDiscDNSDomain)); g_array_set_clear_func(rdata->dns_domains, dns_domain_free); priv->rdata.public.hop_limit = 64; - - /* Start at very low number so that last_rs - router_solicitation_interval - * is much lower than nm_utils_get_monotonic_timestamp_sec() at startup. - */ - priv->last_rs = G_MININT32; } static void @@ -1565,7 +1629,7 @@ dispose(GObject *object) NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE(ndisc); nm_clear_g_source_inst(&priv->ra_timeout_source); - nm_clear_g_source(&priv->send_rs_id); + nm_clear_g_source_inst(&priv->solicit_timer_source); nm_clear_g_source(&priv->send_ra_id); nm_clear_g_free(&priv->last_error); diff --git a/src/ndisc/tests/test-ndisc-fake.c b/src/ndisc/tests/test-ndisc-fake.c index 408fc420e0..3c4bc1f2f7 100644 --- a/src/ndisc/tests/test-ndisc-fake.c +++ b/src/ndisc/tests/test-ndisc-fake.c @@ -584,46 +584,9 @@ _test_dns_solicit_loop_changed(NMNDisc * ndisc, 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(NMFakeNDisc *ndisc, TestData *data) { - const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); - guint id; - - if (data->rs_counter > 0 && data->rs_counter < 6) { - if (data->rs_counter == 1) { - data->first_solicit_msec = now_msec; - /* 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_ndisc_add_ra(ndisc, 0, NM_NDISC_DHCP_LEVEL_NONE, 4, 1500); - g_assert(id); - nm_fake_ndisc_add_gateway(ndisc, - id, - "fe80::1", - now_msec + 10000, - NM_ICMPV6_ROUTER_PREF_MEDIUM); - - nm_fake_ndisc_emit_new_ras(ndisc); - } else if (data->rs_counter >= 6) { - /* Fail if we've sent too many solicitations in the past 4 seconds */ - g_assert_cmpint(now_msec - data->first_solicit_msec, >, 4000); - g_source_remove(data->timeout_id); - g_main_loop_quit(data->loop); - } data->rs_counter++; } @@ -639,9 +602,6 @@ test_dns_solicit_loop(void) }; guint id; - g_test_skip("The solicitation behavior is wrong and need fixing. This test is not working too"); - return; - /* 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 @@ -664,8 +624,10 @@ test_dns_solicit_loop(void) &data); nm_ndisc_start(NM_NDISC(ndisc)); - nmtst_main_loop_run_assert(data.loop, 20000); + if (nmtst_main_loop_run(data.loop, 10000)) + g_error("we expect to run the loop until timeout. What is wrong?"); g_assert_cmpint(data.counter, ==, 3); + g_assert_cmpint(data.rs_counter, ==, 1); } /*****************************************************************************/ From 3ab5f6550bb94f861ad2e03f5c758f2ff36ffc0f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 25 Jan 2021 09:10:01 +0100 Subject: [PATCH 10/11] ndisc: minor cleanup of ra_timeout --- src/ndisc/nm-ndisc.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c index c8e4c7f9c3..375efde56f 100644 --- a/src/ndisc/nm-ndisc.c +++ b/src/ndisc/nm-ndisc.c @@ -29,7 +29,8 @@ struct _NMNDiscPrivate { /* this *must* be the first field. */ NMNDiscDataInternal rdata; - char * last_error; + char *last_error; + GSource *ra_timeout_source; gint32 announcements_left; @@ -1044,13 +1045,13 @@ nm_ndisc_set_iid(NMNDisc *ndisc, const NMUtilsIPv6IfaceId iid) } static gboolean -ndisc_ra_timeout_cb(gpointer user_data) +ra_timeout_cb(gpointer user_data) { NMNDisc *ndisc = NM_NDISC(user_data); nm_clear_g_source_inst(&NM_NDISC_GET_PRIVATE(ndisc)->ra_timeout_source); g_signal_emit(ndisc, signals[RA_TIMEOUT_SIGNAL], 0); - return G_SOURCE_REMOVE; + return G_SOURCE_CONTINUE; } void @@ -1089,12 +1090,7 @@ nm_ndisc_start(NMNDisc *ndisc) timeout_msec = priv->ra_timeout * 1000u; else timeout_msec = G_MAXUINT; - priv->ra_timeout_source = nm_g_timeout_source_new(timeout_msec, - G_PRIORITY_DEFAULT, - ndisc_ra_timeout_cb, - ndisc, - NULL); - g_source_attach(priv->ra_timeout_source, NULL); + priv->ra_timeout_source = nm_g_timeout_add_source(timeout_msec, ra_timeout_cb, ndisc); } solicit_timer_start(ndisc); From c2c8c67d8c451c29af4916b760966fcef0c1a476 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 25 Jan 2021 09:31:17 +0100 Subject: [PATCH 11/11] ndisc: rate limit number of accepted RA data to track I don't think that the way we track RA data is correct. For example, all data (like DNSSL) is tracked regardless of their router. That means, if a router sends an update, we cannot prematurely expire a DNSSL list which is no longer announced by that router. Anyway, that should be reviewed and considered another time. However, we also must rate limit the number of elements we track in general. So far we would only do that for the addresses. The limit for max_addresses, is already read by sysctl. We only limit it further to at most 300 addresses. The 300 is arbitrarily chosen. The limit for DNSSL and RDNSS are taken from systemd-networkd (NDISC_DNSSL_MAX, NDISC_RDNSS_MAX), which is 64. The limit for gateways and routes are not based on anything and just made up. --- src/ndisc/nm-ndisc.c | 61 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c index 375efde56f..05da8eb7d1 100644 --- a/src/ndisc/nm-ndisc.c +++ b/src/ndisc/nm-ndisc.c @@ -23,6 +23,12 @@ #define RFC7559_IRT ((gint32) 4) /* RFC7559, Initial Retransmission Time, in seconds */ #define RFC7559_MRT ((gint32) 3600) /* RFC7559, Maximum Retransmission Time, in seconds */ +#define _SIZE_MAX_GATEWAYS 100u +#define _SIZE_MAX_ADDRESSES 100u +#define _SIZE_MAX_ROUTES 1000u +#define _SIZE_MAX_DNS_SERVERS 64u +#define _SIZE_MAX_DNS_DOMAINS 64u + /*****************************************************************************/ struct _NMNDiscPrivate { @@ -49,10 +55,10 @@ struct _NMNDiscPrivate { int ifindex; char * ifname; char * network_id; + guint max_addresses; NMSettingIP6ConfigAddrGenMode addr_gen_mode; NMUtilsStableType stable_type; guint32 ra_timeout; - gint32 max_addresses; gint32 router_solicitations; gint32 router_solicitation_interval; NMNDiscNodeType node_type; @@ -425,6 +431,9 @@ nm_ndisc_add_gateway(NMNDisc *ndisc, const NMNDiscGateway *new_item, gint64 now_ i++; } + if (rdata->gateways->len >= _SIZE_MAX_GATEWAYS) + return FALSE; + if (new_item->expiry_msec <= now_msec) return FALSE; @@ -587,7 +596,7 @@ nm_ndisc_add_address(NMNDisc * ndisc, * what the kernel does, because it considers *all* addresses (including * static and other temporary addresses). **/ - if (priv->max_addresses && rdata->addresses->len >= priv->max_addresses) + if (rdata->addresses->len >= priv->max_addresses) return FALSE; if (new_item->expiry_msec <= now_msec) @@ -673,6 +682,9 @@ nm_ndisc_add_route(NMNDisc *ndisc, const NMNDiscRoute *new_item, gint64 now_msec i++; } + if (rdata->routes->len >= _SIZE_MAX_ROUTES) + return FALSE; + if (new_item->expiry_msec <= now_msec) { nm_assert(!changed); return FALSE; @@ -709,6 +721,9 @@ nm_ndisc_add_dns_server(NMNDisc *ndisc, const NMNDiscDNSServer *new_item, gint64 } } + if (rdata->dns_servers->len >= _SIZE_MAX_DNS_SERVERS) + return FALSE; + if (new_item->expiry_msec <= now_msec) return FALSE; @@ -745,6 +760,9 @@ nm_ndisc_add_dns_domain(NMNDisc *ndisc, const NMNDiscDNSDomain *new_item, gint64 } } + if (rdata->dns_domains->len >= _SIZE_MAX_DNS_DOMAINS) + return FALSE; + if (new_item->expiry_msec <= now_msec) return FALSE; @@ -1285,6 +1303,19 @@ _config_changed_log(NMNDisc *ndisc, NMNDiscConfigMap changed) /*****************************************************************************/ +static gboolean +_array_set_size_max(GArray *array, guint size_max) +{ + nm_assert(array); + nm_assert(size_max > 0u); + + if (array->len <= size_max) + return FALSE; + + g_array_set_size(array, size_max); + return TRUE; +} + static void clean_gateways(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint64 *next_msec) { @@ -1311,12 +1342,16 @@ clean_gateways(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint6 g_array_set_size(rdata->gateways, j); } + if (_array_set_size_max(rdata->gateways, _SIZE_MAX_GATEWAYS)) + *changed |= NM_NDISC_CONFIG_GATEWAYS; + _ASSERT_data_gateways(rdata); } static void clean_addresses(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint64 *next_msec) { + NMNDiscPrivate * priv = NM_NDISC_GET_PRIVATE(ndisc); NMNDiscDataInternal *rdata = &NM_NDISC_GET_PRIVATE(ndisc)->rdata; NMNDiscAddress * arr; guint i; @@ -1339,6 +1374,9 @@ clean_addresses(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint *changed = NM_NDISC_CONFIG_ADDRESSES; g_array_set_size(rdata->addresses, j); } + + if (_array_set_size_max(rdata->gateways, priv->max_addresses)) + *changed |= NM_NDISC_CONFIG_ADDRESSES; } static void @@ -1366,6 +1404,9 @@ clean_routes(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gint64 *changed |= NM_NDISC_CONFIG_ROUTES; g_array_set_size(rdata->routes, j); } + + if (_array_set_size_max(rdata->gateways, _SIZE_MAX_ROUTES)) + *changed |= NM_NDISC_CONFIG_ROUTES; } static void @@ -1393,6 +1434,9 @@ clean_dns_servers(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gi *changed |= NM_NDISC_CONFIG_DNS_SERVERS; g_array_set_size(rdata->dns_servers, j); } + + if (_array_set_size_max(rdata->gateways, _SIZE_MAX_DNS_SERVERS)) + *changed |= NM_NDISC_CONFIG_DNS_SERVERS; } static void @@ -1425,6 +1469,9 @@ clean_dns_domains(NMNDisc *ndisc, gint64 now_msec, NMNDiscConfigMap *changed, gi *changed |= NM_NDISC_CONFIG_DNS_DOMAINS; g_array_set_size(rdata->dns_domains, j); } + + if (_array_set_size_max(rdata->gateways, _SIZE_MAX_DNS_DOMAINS)) + *changed |= NM_NDISC_CONFIG_DNS_DOMAINS; } static void @@ -1531,6 +1578,7 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps { NMNDisc * self = NM_NDISC(object); NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE(self); + int i; switch (prop_id) { case PROP_PLATFORM: @@ -1572,7 +1620,14 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps break; case PROP_MAX_ADDRESSES: /* construct-only */ - priv->max_addresses = g_value_get_int(value); + i = g_value_get_int(value); + nm_assert(i >= 0); + priv->max_addresses = i; + + if (priv->max_addresses <= 0) + priv->max_addresses = _SIZE_MAX_ADDRESSES; + else if (priv->max_addresses > 3u * _SIZE_MAX_ADDRESSES) + priv->max_addresses = 3u * _SIZE_MAX_ADDRESSES; break; case PROP_RA_TIMEOUT: /* construct-only */