From bbcaa8b9256ee57a26c48981737ff744b0fd2470 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Jun 2014 10:11:31 +0200 Subject: [PATCH 1/7] platform/test: use proper to_string() functions when printing NMPlaformIP[46]Address in dump_interface() Signed-off-by: Thomas Haller --- src/platform/tests/dump.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/platform/tests/dump.c b/src/platform/tests/dump.c index 9276d18439..e97ef138cb 100644 --- a/src/platform/tests/dump.c +++ b/src/platform/tests/dump.c @@ -13,7 +13,6 @@ dump_interface (NMPlatformLink *link) GArray *ip4_addresses; const NMPlatformIP6Address *ip6_address; const NMPlatformIP4Address *ip4_address; - char addrstr[INET6_ADDRSTRLEN]; GArray *ip6_routes; GArray *ip4_routes; const NMPlatformIP6Route *ip6_route; @@ -73,14 +72,12 @@ dump_interface (NMPlatformLink *link) for (i = 0; i < ip4_addresses->len; i++) { ip4_address = &g_array_index (ip4_addresses, NMPlatformIP4Address, i); - inet_ntop (AF_INET, &ip4_address->address, addrstr, sizeof (addrstr)); - printf (" ip4-address %s/%d lifetime %u preferred %u\n", addrstr, ip4_address->plen, ip4_address->lifetime, ip4_address->preferred); + printf (" ip4-address %s\n", nm_platform_ip4_address_to_string (ip4_address)); } for (i = 0; i < ip6_addresses->len; i++) { ip6_address = &g_array_index (ip6_addresses, NMPlatformIP6Address, i); - inet_ntop (AF_INET6, &ip6_address->address, addrstr, sizeof (addrstr)); - printf (" ip6-address %s/%d lifetime %u preferred %u\n", addrstr, ip6_address->plen, ip6_address->lifetime, ip6_address->preferred); + printf (" ip6-address %s\n", nm_platform_ip6_address_to_string (ip6_address)); } g_array_unref (ip4_addresses); From 225a199fd3265ce98b8908f5f7729e49d62d4134 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Jun 2014 09:29:14 +0200 Subject: [PATCH 2/7] dhcp: nm_dhcp_dhclient_read_lease_ip_configs() must anchor the address lifetime at *now* nm_dhcp_dhclient_read_lease_ip_configs() calculates the remaining time until the address expires. It must anchor the lifetimes by setting the @timestamp to nm_utils_get_monotonic_timestamp_s(). Signed-off-by: Thomas Haller --- src/dhcp-manager/nm-dhcp-dhclient-utils.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/dhcp-manager/nm-dhcp-dhclient-utils.c b/src/dhcp-manager/nm-dhcp-dhclient-utils.c index 3e7173aec3..8527e6c0fb 100644 --- a/src/dhcp-manager/nm-dhcp-dhclient-utils.c +++ b/src/dhcp-manager/nm-dhcp-dhclient-utils.c @@ -27,6 +27,7 @@ #include "nm-dhcp-dhclient-utils.h" #include "nm-ip4-config.h" #include "nm-utils.h" +#include "NetworkManagerUtils.h" #define CLIENTID_TAG "send dhcp-client-identifier" #define CLIENTID_FORMAT CLIENTID_TAG " \"%s\"; # added by NetworkManager" @@ -530,6 +531,7 @@ nm_dhcp_dhclient_read_lease_ip_configs (const char *iface, GSList *parsed = NULL, *iter, *leases = NULL; char **line, **split = NULL; GHashTable *hash = NULL; + gint32 now_monotonic_ts; g_return_val_if_fail (contents != NULL, NULL); @@ -570,6 +572,7 @@ nm_dhcp_dhclient_read_lease_ip_configs (const char *iface, g_date_time_ref (now); else now = g_date_time_new_now_utc (); + now_monotonic_ts = nm_utils_get_monotonic_timestamp_s (); for (iter = parsed; iter; iter = g_slist_next (iter)) { NMIP4Config *ip4; @@ -593,7 +596,7 @@ nm_dhcp_dhclient_read_lease_ip_configs (const char *iface, continue; /* scale expiry to seconds (and CLAMP into the range of guint32) */ - expiry = CLAMP (expiry / G_TIME_SPAN_SECOND, 0, G_MAXUINT32-1); + expiry = CLAMP (expiry / G_TIME_SPAN_SECOND, 0, NM_PLATFORM_LIFETIME_PERMANENT-1); if (expiry <= 0) { /* the address is already expired. Don't even add it. */ continue; @@ -624,6 +627,7 @@ nm_dhcp_dhclient_read_lease_ip_configs (const char *iface, if (!address.plen) address.plen = nm_utils_ip4_get_default_prefix (address.address); + address.timestamp = now_monotonic_ts; address.lifetime = address.preferred = expiry; address.source = NM_PLATFORM_SOURCE_DHCP; From 9980b9843ccce22234a16ea3801250b106238c62 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 7 Jun 2014 12:52:22 +0200 Subject: [PATCH 3/7] platform: fix off-by-two error converting lifetimes in _init_ip_address_lifetime() When setting the timestamp to 1, we have to subtract(!) one second from a_valid and a_preferred. Due to this error, NM saw the lifetimes of addresses from system as two seconds larger then the actual value. Signed-off-by: Thomas Haller --- src/platform/nm-linux-platform.c | 37 ++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 09733a45a8..7e8ba66e8a 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1032,6 +1032,23 @@ hack_empty_master_iff_lower_up (NMPlatform *platform, struct nl_object *object) rtnl_link_unset_flags (rtnllink, IFF_LOWER_UP); } +static guint32 +_get_remaining_time (guint32 start_timestamp, guint32 end_timestamp) +{ + /* Return the remaining time between @start_timestamp until @end_timestamp. + * + * If @end_timestamp is NM_PLATFORM_LIFETIME_PERMANENT, it returns + * NM_PLATFORM_LIFETIME_PERMANENT. If @start_timestamp already passed + * @end_timestamp it returns 0. Beware, NMPlatformIPAddress treats a @lifetime + * of 0 as permanent. + */ + if (end_timestamp == NM_PLATFORM_LIFETIME_PERMANENT) + return NM_PLATFORM_LIFETIME_PERMANENT; + if (start_timestamp >= end_timestamp) + return 0; + return end_timestamp - start_timestamp; +} + static void _init_ip_address_lifetime (NMPlatformIPAddress *address, const struct rtnl_addr *rtnladdr) { @@ -1057,6 +1074,15 @@ _init_ip_address_lifetime (NMPlatformIPAddress *address, const struct rtnl_addr a_valid > 0 && a_preferred > 0); + if (a_valid <= 1) { + /* Since we want to have positive @timestamp and @valid != 0, + * we must handle this case special. */ + address->timestamp = 1; + address->lifetime = 1; /* Extend the lifetime by one second */ + address->preferred = 0; /* no longer preferred. */ + return; + } + /* The correct timestamp value would probably be rtnl_addr_get_last_update_time() * (after converting into the proper time scale). * But this is relatively complicated to convert and we don't actually need it. @@ -1068,15 +1094,8 @@ _init_ip_address_lifetime (NMPlatformIPAddress *address, const struct rtnl_addr */ address->timestamp = 1; - /* account for the timestamp==1 by incrementing valid/preferred -- unless - * it is NM_PLATFORM_LIFETIME_PERMANENT or already NM_PLATFORM_LIFETIME_PERMANENT-1 - * (i.e. the largest non-permanent lifetime). */ - if (a_valid < NM_PLATFORM_LIFETIME_PERMANENT - 1) - a_valid += 1; - if (a_preferred < NM_PLATFORM_LIFETIME_PERMANENT - 1) - a_preferred += 1; - address->lifetime = a_valid; - address->preferred = a_preferred; + address->lifetime = _get_remaining_time (address->timestamp, a_valid); + address->preferred = _get_remaining_time (address->timestamp, a_preferred); } static gboolean From 2d37fedcd3565ad6b5168ebc8cdf592002e3f027 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 7 Jun 2014 13:52:01 +0200 Subject: [PATCH 4/7] platform: set timestamp in platform addresses to last_update_time() Previous patch 8310a039d81e3a316cf657aa9f28edabb9be125c modified platform to set the timestamp of addresses always to 1. So, when adding an address platform logging looked like: signal: address 4 added: 192.168.232.3/24 lft 2000sec pref 1000sec lifetime 12345-1[13344,14344] dev em1 src kernel This is confusing in the log file and during debugging. Instead set the timestamp to the last modification time of the address so that it will look like: signal: address 4 added: 192.168.232.3/24 lft 2000sec pref 1000sec lifetime 12345-12345[1000,2000] dev em1 src kernel Signed-off-by: Thomas Haller --- src/platform/nm-linux-platform.c | 85 ++++++++++++++++++++++++++++---- src/platform/nm-platform.h | 5 +- 2 files changed, 79 insertions(+), 11 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 7e8ba66e8a..f5da671380 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1049,6 +1049,75 @@ _get_remaining_time (guint32 start_timestamp, guint32 end_timestamp) return end_timestamp - start_timestamp; } +/* _timestamp_nl_to_ms: + * @timestamp_nl: a timestamp from ifa_cacheinfo. + * @monotonic_ms: *now* in CLOCK_MONOTONIC. Needed to estimate the current + * uptime and how often timestamp_nl wrapped. + * + * Convert the timestamp from ifa_cacheinfo to CLOCK_MONOTONIC milliseconds. + * The ifa_cacheinfo fields tstamp and cstamp contains timestamps that counts + * with in 1/100th of a second of clock_gettime(CLOCK_MONOTONIC). However, + * the uint32 counter wraps every 497 days of uptime, so we have to compensate + * for that. */ +static gint64 +_timestamp_nl_to_ms (guint32 timestamp_nl, gint64 monotonic_ms) +{ + const gint64 WRAP_INTERVAL = (((gint64) G_MAXUINT32) + 1) * (1000 / 100); + gint64 timestamp_nl_ms; + + /* convert timestamp from 1/100th of a second to msec. */ + timestamp_nl_ms = ((gint64) timestamp_nl) * (1000 / 100); + + /* timestamp wraps every 497 days. Try to compensate for that.*/ + if (timestamp_nl_ms > monotonic_ms) { + /* timestamp_nl_ms is in the future. Truncate it to *now* */ + timestamp_nl_ms = monotonic_ms; + } else if (monotonic_ms >= WRAP_INTERVAL) { + timestamp_nl_ms += (monotonic_ms / WRAP_INTERVAL) * WRAP_INTERVAL; + if (timestamp_nl_ms > monotonic_ms) + timestamp_nl_ms -= WRAP_INTERVAL; + } + + return timestamp_nl_ms; +} + +static guint32 +_rtnl_addr_last_update_time_to_nm (const struct rtnl_addr *rtnladdr) +{ + guint32 last_update_time = rtnl_addr_get_last_update_time ((struct rtnl_addr *) rtnladdr); + struct timespec tp; + gint64 now_nl, now_nm, result; + + /* timestamp is unset. Default to 1. */ + if (!last_update_time) + return 1; + + /* do all the calculations in milliseconds scale */ + + clock_gettime (CLOCK_MONOTONIC, &tp); + now_nm = nm_utils_get_monotonic_timestamp_ms (); + now_nl = (((gint64) tp.tv_sec) * ((gint64) 1000)) + + (tp.tv_nsec / (NM_UTILS_NS_PER_SECOND/1000)); + + result = now_nm - (now_nl - _timestamp_nl_to_ms (last_update_time, now_nl)); + + /* converting the last_update_time into nm_utils_get_monotonic_timestamp_ms() scale is + * a good guess but fails in the following situations: + * + * - If the address existed before start of the process, the timestamp in nm scale would + * be negative or zero. In this case we default to 1. + * - during hibernation, the CLOCK_MONOTONIC/last_update_time drifts from + * nm_utils_get_monotonic_timestamp_ms() scale. + */ + if (result <= 1000) + return 1; + + if (result > now_nm) + return now_nm / 1000; + + return result / 1000; +} + static void _init_ip_address_lifetime (NMPlatformIPAddress *address, const struct rtnl_addr *rtnladdr) { @@ -1083,17 +1152,15 @@ _init_ip_address_lifetime (NMPlatformIPAddress *address, const struct rtnl_addr return; } - /* The correct timestamp value would probably be rtnl_addr_get_last_update_time() - * (after converting into the proper time scale). - * But this is relatively complicated to convert and we don't actually need it. - * Especially, because rtnl_addr_get_last_update_time() might be negative in - * nm_utils_get_monotonic_timestamp_s() scale -- which we want to avoid. - * - * Remember: timestamp has no meaning, beyond anchoring the relative lifetimes - * at some point in time. We choose this point to be "1 second". + /* _rtnl_addr_last_update_time_to_nm() might be wrong, so don't rely on + * timestamp to have any meaning beyond anchoring the relative durations + * @lifetime and @preferred. */ - address->timestamp = 1; + address->timestamp = _rtnl_addr_last_update_time_to_nm (rtnladdr); + /* We would expect @timestamp to be less then @a_valid. Just to be sure, + * fix it up. */ + address->timestamp = MIN (address->timestamp, a_valid - 1); address->lifetime = _get_remaining_time (address->timestamp, a_valid); address->preferred = _get_remaining_time (address->timestamp, a_preferred); } diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index f7b14b7b9b..7dc0127648 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -169,8 +169,9 @@ typedef struct { /* Timestamp in seconds in the reference system of nm_utils_get_monotonic_timestamp_*(). * This value is mainly used to anchor the relative lifetime and preferred values. * For addresses originating from DHCP it might be set to nm_utils_get_monotonic_timestamp_s() - * of when the lease was received. For addresses from platform/kernel it is set to 1. - * For permanent addresses it is mostly set to 0. + * of when the lease was received. For addresses from platform/kernel it might be set to + * when the address was last modified. For permanent addresses it is mostly set to 0. + * For non-permanent addresses it should not be 0 (because 0 is not a valid timestamp). */ \ guint32 timestamp; \ guint32 lifetime; /* seconds since timestamp */ \ From 1184cadc7d19104f581837617887f01746e85cb3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Jun 2014 10:03:01 +0200 Subject: [PATCH 5/7] platform: handle unset address timestamp as *now* in to_string() When printing an address in nm_platform_ip4_address_to_string() and nm_platform_ip6_address_to_string() treat an unset @timestamp as counting from @now. This is useful, if you just have the remaining lifetime at hand and want to print an address. In general it is not a good idea to leave the timestamp not anchored to an absolute @timestamp. Signed-off-by: Thomas Haller --- src/platform/nm-platform.c | 28 +++++++++++++++++++++++----- src/platform/nm-platform.h | 21 ++++++++++++++++----- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 71c60ca5bf..17f452c547 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1436,6 +1436,7 @@ nm_platform_ip4_address_add (int ifindex, addr.address = address; addr.peer_address = peer_address; addr.plen = plen; + addr.timestamp = 0; /* set it at zero, which to_string will treat as *now* */ addr.lifetime = lifetime; addr.preferred = preferred; if (label) @@ -1470,6 +1471,7 @@ nm_platform_ip6_address_add (int ifindex, addr.address = address; addr.peer_address = peer_address; addr.plen = plen; + addr.timestamp = 0; /* set it to zero, which to_string will treat as *now* */ addr.lifetime = lifetime; addr.preferred = preferred; addr.flags = flags; @@ -1571,6 +1573,17 @@ _rebase_relative_time_on_now (guint32 timestamp, guint32 duration, guint32 now, if (duration == NM_PLATFORM_LIFETIME_PERMANENT) return NM_PLATFORM_LIFETIME_PERMANENT; + if (timestamp == 0) { + /* if the @timestamp is zero, assume it was just left unset and that the relative + * @duration starts counting from @now. This is convenient to construct an address + * and print it in nm_platform_ip4_address_to_string(). + * + * In general it does not make sense to set the @duration without anchoring at + * @timestamp because you don't know the absolute expiration time when looking + * at the address at a later moment. */ + timestamp = now; + } + /* For timestamp > now, just accept it and calculate the expected(?) result. */ t = (gint64) timestamp + (gint64) duration - (gint64) now; @@ -1597,13 +1610,18 @@ _address_get_lifetime (const NMPlatformIPAddress *address, guint32 now, guint32 if (!lifetime) return FALSE; preferred = _rebase_relative_time_on_now (address->timestamp, address->preferred, now, padding); - if (preferred > lifetime) { - g_warn_if_reached (); - preferred = lifetime; - } *out_lifetime = lifetime; - *out_preferred = preferred; + *out_preferred = MIN (preferred, lifetime); + + /* Assert that non-permanent addresses have a (positive) @timestamp. _rebase_relative_time_on_now() + * treats addresses with timestamp 0 as *now*. Addresses passed to _address_get_lifetime() always + * should have a valid @timestamp, otherwise on every re-sync, their lifetime will be extended anew. + */ + g_return_val_if_fail ( address->timestamp != 0 + || ( address->lifetime == NM_PLATFORM_LIFETIME_PERMANENT + && address->preferred == NM_PLATFORM_LIFETIME_PERMANENT), TRUE); + g_return_val_if_fail (preferred <= lifetime, TRUE); } return TRUE; } diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 7dc0127648..96b6cfabbd 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -167,11 +167,22 @@ typedef struct { NMPlatformSource source; \ \ /* Timestamp in seconds in the reference system of nm_utils_get_monotonic_timestamp_*(). - * This value is mainly used to anchor the relative lifetime and preferred values. - * For addresses originating from DHCP it might be set to nm_utils_get_monotonic_timestamp_s() - * of when the lease was received. For addresses from platform/kernel it might be set to - * when the address was last modified. For permanent addresses it is mostly set to 0. - * For non-permanent addresses it should not be 0 (because 0 is not a valid timestamp). + * + * The rules are: + * 1 @lifetime==0: @timestamp and @preferred is irrelevant (but mostly set to 0 too). Such addresses + * are permanent. This rule is so that unset addresses (calloc) are permanent by default. + * 2 @lifetime==@preferred==NM_PLATFORM_LIFETIME_PERMANENT: @timestamp is irrelevant (but mostly + * set to 0). Such addresses are permanent. + * 3 Non permanent addreses should (almost) always have @timestamp > 0. 0 is not a valid timestamp + * and never returned by nm_utils_get_monotonic_timestamp_s(). In this case @valid/@preferred + * is anchored at @timestamp. + * 4 Non permanent addresses with @timestamp == 0 are implicitely anchored at *now*, thus the time + * moves as time goes by. This is usually not useful, except e.g. nm_platform_ip[46]_address_add(). + * + * Non permanent addresses from DHCP/RA might have the @timestamp set to the moment of when the + * lease was received. Addresses from kernel might have the @timestamp based on the last modification + * time of the addresses. But don't rely on this behaviour, the @timestamp is only defined for anchoring + * @lifetime and @preferred. */ \ guint32 timestamp; \ guint32 lifetime; /* seconds since timestamp */ \ From 60477f3f1cf897f966a99c97de52a7b4904ad971 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Jun 2014 10:27:48 +0200 Subject: [PATCH 6/7] platform: modify address to_string() to show raw lifetime values The "lifetime" part when printing an address in nm_platform_ip[46]_address_to_string() is supposed to show the raw, internal values of the address. We already have the "lft" and "pref" output that presents the expiries based on now. These fields are already crafted to show what the user probably wants to see when looking at debugging log. "lifetime" should not do any special casing and just print the raw values. Signed-off-by: Thomas Haller --- src/platform/nm-platform.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 17f452c547..e15de984f7 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2107,12 +2107,6 @@ _lifetime_to_string (guint32 timestamp, guint32 lifetime, gint32 now, char *buf, static const char * _lifetime_summary_to_string (gint32 now, guint32 timestamp, guint32 preferred, guint32 lifetime, char *buf, size_t buf_size) { - if (timestamp == 0) { - if (preferred == NM_PLATFORM_LIFETIME_PERMANENT && lifetime == NM_PLATFORM_LIFETIME_PERMANENT) - return ""; - if (preferred == 0 && lifetime == 0) - return " lifetime unset"; - } g_snprintf (buf, buf_size, " lifetime %d-%u[%u,%u]", (signed) now, (unsigned) timestamp, (unsigned) preferred, (unsigned) lifetime); return buf; From 590c0d27e687aa6c3d3c831716126ca85997a90b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Jun 2014 10:28:55 +0200 Subject: [PATCH 7/7] platform: extend nm_platform_ip_address_cmp_expiry() to handle addresses without timestamp If the timestamp is set to zero, the to_string() functions treat the lifetime as based on *now*. For nm_platform_ip_address_cmp_expiry() this makes no sense, because there is no absolute exiry to compare. Instead compare them as expire earlier then the other address. Signed-off-by: Thomas Haller --- src/platform/nm-platform.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index e15de984f7..542b0faab2 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2506,31 +2506,32 @@ nm_platform_ip6_route_cmp (const NMPlatformIP6Route *a, const NMPlatformIP6Route int nm_platform_ip_address_cmp_expiry (const NMPlatformIPAddress *a, const NMPlatformIPAddress *b) { - gint64 ta, tb; + gint64 ta = 0, tb = 0; _CMP_POINTER (a, b); if (a->lifetime == NM_PLATFORM_LIFETIME_PERMANENT || a->lifetime == 0) ta = G_MAXINT64; - else + else if (a->timestamp) ta = ((gint64) a->timestamp) + a->lifetime; if (b->lifetime == NM_PLATFORM_LIFETIME_PERMANENT || b->lifetime == 0) tb = G_MAXINT64; - else + else if (b->timestamp) tb = ((gint64) b->timestamp) + b->lifetime; if (ta == tb) { /* if the lifetime is equal, compare the preferred time. */ + ta = tb = 0; if (a->preferred == NM_PLATFORM_LIFETIME_PERMANENT || a->lifetime == 0 /* liftime==0 means permanent! */) ta = G_MAXINT64; - else + else if (a->timestamp) ta = ((gint64) a->timestamp) + a->preferred; if (b->preferred == NM_PLATFORM_LIFETIME_PERMANENT|| b->lifetime == 0) tb = G_MAXINT64; - else + else if (b->timestamp) tb = ((gint64) b->timestamp) + b->preferred; if (ta == tb)