From 0f1fca29dbe6f8d7909b8e85fb98b358bab4e27a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 11 Jan 2015 18:17:47 +0100 Subject: platform/tests: fix errors in fake platform handling route metrics --- src/platform/nm-fake-platform.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index a5eb7d4ed3..b4bad844f0 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -1076,6 +1076,8 @@ ip4_route_add (NMPlatform *platform, int ifindex, NMIPConfigSource source, continue; if (item->plen != route.plen) continue; + if (item->metric != metric) + continue; memcpy (item, &route, sizeof (route)); g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, ifindex, &route, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_INTERNAL); @@ -1097,6 +1099,8 @@ ip6_route_add (NMPlatform *platform, int ifindex, NMIPConfigSource source, NMPlatformIP6Route route; guint i; + metric = nm_utils_ip6_route_metric_normalize (metric); + memset (&route, 0, sizeof (route)); route.source = NM_IP_CONFIG_SOURCE_KERNEL; route.ifindex = ifindex; @@ -1116,6 +1120,8 @@ ip6_route_add (NMPlatform *platform, int ifindex, NMIPConfigSource source, continue; if (item->plen != route.plen) continue; + if (item->metric != metric) + continue; memcpy (item, &route, sizeof (route)); g_signal_emit_by_name (platform, NM_PLATFORM_SIGNAL_IP6_ROUTE_CHANGED, ifindex, &route, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_INTERNAL); @@ -1153,6 +1159,8 @@ ip6_route_get (NMPlatform *platform, int ifindex, struct in6_addr network, int p NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE (platform); int i; + metric = nm_utils_ip6_route_metric_normalize (metric); + for (i = 0; i < priv->ip6_routes->len; i++) { NMPlatformIP6Route *route = &g_array_index (priv->ip6_routes, NMPlatformIP6Route, i); -- cgit v1.2.1 From b421af3730cca4c0c5a3721a52cd2e4a1ebfc4b3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 11 Jan 2015 17:15:49 +0100 Subject: platform/tests: add ip4_route_exists() test function --- src/platform/tests/test-common.c | 107 +++++++++++++++++++++++++++++++++++++++ src/platform/tests/test-common.h | 5 ++ 2 files changed, 112 insertions(+) diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c index ab40f65240..3e716bf8c9 100644 --- a/src/platform/tests/test-common.c +++ b/src/platform/tests/test-common.c @@ -125,6 +125,113 @@ link_callback (NMPlatform *platform, int ifindex, NMPlatformLink *received, NMPl g_error ("Added/changed link not found in the local cache."); } +gboolean +ip4_route_exists (const char *ifname, guint32 network, int plen, guint32 metric) +{ + gs_free char *arg_network = NULL; + const char *argv[] = { + NULL, + "route", + "list", + "dev", + ifname, + "exact", + NULL, + NULL, + }; + int exit_status; + gs_free char *std_out = NULL, *std_err = NULL; + char *out; + gboolean success; + gs_free_error GError *error = NULL; + gs_free char *metric_pattern = NULL; + + g_assert (ifname && nm_utils_iface_valid_name (ifname)); + g_assert (!strstr (ifname, " metric ")); + g_assert (plen >= 0 && plen <= 32); + + if (!NM_IS_LINUX_PLATFORM (nm_platform_get ())) { + /* If we don't test against linux-platform, we don't actually configure any + * routes in the system. */ + return -1; + } + + argv[0] = nm_utils_file_search_in_paths ("ip", NULL, + (const char *[]) { "/sbin", "/usr/sbin", NULL }, + G_FILE_TEST_IS_EXECUTABLE, NULL, NULL, NULL); + argv[6] = arg_network = g_strdup_printf ("%s/%d", nm_utils_inet4_ntop (network, NULL), plen); + + if (!argv[0]) { + /* Hm. There is no 'ip' binary. Return *unknown* */ + return -1; + } + + success = g_spawn_sync (NULL, + (char **) argv, + (char *[]) { NULL }, + 0, + NULL, + NULL, + &std_out, + &std_err, + &exit_status, + &error); + g_assert_no_error (error); + g_assert (success); + g_assert_cmpstr (std_err, ==, ""); + g_assert (std_out); + + metric_pattern = g_strdup_printf (" metric %u", metric); + out = std_out; + while (out) { + char *eol = strchr (out, '\n'); + gs_free char *line = eol ? g_strndup (out, eol - out) : g_strdup (out); + const char *p; + + out = eol ? &eol[1] : NULL; + if (!line[0]) + continue; + + if (metric == 0) { + if (!strstr (line, " metric ")) + return TRUE; + } + p = strstr (line, metric_pattern); + if (p && NM_IN_SET (p[strlen (metric_pattern)], ' ', '\0')) + return TRUE; + } + return FALSE; +} + +void +_assert_ip4_route_exists (const char *file, guint line, const char *func, gboolean exists, const char *ifname, guint32 network, int plen, guint32 metric) +{ + int ifindex; + gboolean exists_checked; + + /* Check for existance of the route by spawning iproute2. Do this because platform + * code might be entirely borked, but we expect ip-route to give a correct result. + * If the ip command cannot be found, we accept this as success. */ + exists_checked = ip4_route_exists (ifname, network, plen, metric); + if (exists_checked != -1 && !exists_checked != !exists) { + g_error ("[%s:%u] %s(): We expect the ip4 route %s/%d metric %u %s, but it %s", + file, line, func, + nm_utils_inet4_ntop (network, NULL), plen, metric, + exists ? "to exist" : "not to exist", + exists ? "doesn't" : "does"); + } + + ifindex = nm_platform_link_get_ifindex (ifname); + g_assert (ifindex > 0); + if (!nm_platform_ip4_route_exists (ifindex, network, plen, metric) != !exists) { + g_error ("[%s:%u] %s(): The ip4 route %s/%d metric %u %s, but platform thinks %s", + file, line, func, + nm_utils_inet4_ntop (network, NULL), plen, metric, + exists ? "exists" : "does not exist", + exists ? "it doesn't" : "it does"); + } +} + void run_command (const char *format, ...) { diff --git a/src/platform/tests/test-common.h b/src/platform/tests/test-common.h index 8f6b391266..17c4029d8c 100644 --- a/src/platform/tests/test-common.h +++ b/src/platform/tests/test-common.h @@ -34,6 +34,11 @@ void accept_signal (SignalData *data); void wait_signal (SignalData *data); void free_signal (SignalData *data); +gboolean ip4_route_exists (const char *ifname, guint32 network, int plen, guint32 metric); + +void _assert_ip4_route_exists (const char *file, guint line, const char *func, gboolean exists, const char *ifname, guint32 network, int plen, guint32 metric); +#define assert_ip4_route_exists(exists, ifname, network, plen, metric) _assert_ip4_route_exists (__FILE__, __LINE__, G_STRFUNC, exists, ifname, network, plen, metric) + void link_callback (NMPlatform *platform, int ifindex, NMPlatformLink *received, NMPlatformSignalChangeType change_type, NMPlatformReason reason, SignalData *data); void run_command (const char *format, ...); -- cgit v1.2.1 From a6cd0e7a29cb8847c5af2523c8dd48ce1c422cf6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 11 Jan 2015 18:26:19 +0100 Subject: platform/tests: use assert_ip4_route_exists() function --- src/platform/tests/test-route.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c index 9ccca0a0f6..2b06c03993 100644 --- a/src/platform/tests/test-route.c +++ b/src/platform/tests/test-route.c @@ -75,11 +75,11 @@ test_ip4_route (void) accept_signal (route_added); /* Add route */ - g_assert (!nm_platform_ip4_route_exists (ifindex, network, plen, metric)); + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, metric); no_error (); g_assert (nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_USER, network, plen, gateway, 0, metric, mss)); no_error (); - g_assert (nm_platform_ip4_route_exists (ifindex, network, plen, metric)); + assert_ip4_route_exists (TRUE, DEVICE_NAME, network, plen, metric); no_error (); accept_signal (route_added); @@ -89,11 +89,11 @@ test_ip4_route (void) accept_signal (route_changed); /* Add default route */ - g_assert (!nm_platform_ip4_route_exists (ifindex, 0, 0, metric)); + assert_ip4_route_exists (FALSE, DEVICE_NAME, 0, 0, metric); no_error (); g_assert (nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_USER, 0, 0, gateway, 0, metric, mss)); no_error (); - g_assert (nm_platform_ip4_route_exists (ifindex, 0, 0, metric)); + assert_ip4_route_exists (TRUE, DEVICE_NAME, 0, 0, metric); no_error (); accept_signal (route_added); @@ -134,7 +134,7 @@ test_ip4_route (void) /* Remove route */ g_assert (nm_platform_ip4_route_delete (ifindex, network, plen, metric)); no_error (); - g_assert (!nm_platform_ip4_route_exists (ifindex, network, plen, metric)); + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, metric); accept_signal (route_removed); /* Remove route again */ -- cgit v1.2.1 From 4f390e7e4d05ad2f11d127cad99c40a3096ad2b7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 11 Jan 2015 17:39:45 +0100 Subject: platform/tests: add test for deleting IPv4 route with metric 0 --- src/platform/tests/test-route.c | 70 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c index 2b06c03993..69f5bc2e4f 100644 --- a/src/platform/tests/test-route.c +++ b/src/platform/tests/test-route.c @@ -50,6 +50,75 @@ ip6_route_callback (NMPlatform *platform, int ifindex, NMPlatformIP6Route *recei data->received = TRUE; } +static void +test_ip4_route_metric0 (void) +{ + int ifindex = nm_platform_link_get_ifindex (DEVICE_NAME); + SignalData *route_added = add_signal (NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, NM_PLATFORM_SIGNAL_ADDED, ip4_route_callback); + /*SignalData *route_changed = add_signal (NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, NM_PLATFORM_SIGNAL_CHANGED, ip4_route_callback);*/ + SignalData *route_removed = add_signal (NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, NM_PLATFORM_SIGNAL_REMOVED, ip4_route_callback); + in_addr_t network = nmtst_inet4_from_string ("192.0.2.5"); /* from 192.0.2.0/24 (TEST-NET-1) (rfc5737) */ + int plen = 32; + int metric = 22987; + int mss = 1000; + + /* No routes initially */ + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, 0); + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, metric); + + /* add the first route */ + g_assert (nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_USER, network, plen, INADDR_ANY, 0, metric, mss)); + no_error (); + accept_signal (route_added); + + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, 0); + assert_ip4_route_exists (TRUE, DEVICE_NAME, network, plen, metric); + + /* Deleting route with metric 0 does nothing */ + g_assert (nm_platform_ip4_route_delete (ifindex, network, plen, 0)); + no_error (); + g_assert (!route_removed->received); + + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, 0); + assert_ip4_route_exists (TRUE, DEVICE_NAME, network, plen, metric); + + /* add the second route */ + g_assert (nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_USER, network, plen, INADDR_ANY, 0, 0, mss)); + no_error (); + accept_signal (route_added); + + assert_ip4_route_exists (TRUE, DEVICE_NAME, network, plen, 0); + assert_ip4_route_exists (TRUE, DEVICE_NAME, network, plen, metric); + + /* Delete route with metric 0 */ + g_assert (nm_platform_ip4_route_delete (ifindex, network, plen, 0)); + no_error (); + accept_signal (route_removed); + + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, 0); + assert_ip4_route_exists (TRUE, DEVICE_NAME, network, plen, metric); + + /* Delete route with metric 0 again (we expect nothing to happen) */ + g_assert (nm_platform_ip4_route_delete (ifindex, network, plen, 0)); + no_error (); + g_assert (!route_removed->received); + + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, 0); + assert_ip4_route_exists (TRUE, DEVICE_NAME, network, plen, metric); + + /* Delete the other route */ + g_assert (nm_platform_ip4_route_delete (ifindex, network, plen, metric)); + no_error (); + accept_signal (route_removed); + + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, 0); + assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, metric); + + free_signal (route_added); + /*free_signal (route_changed);*/ + free_signal (route_removed); +} + static void test_ip4_route (void) { @@ -257,4 +326,5 @@ setup_tests (void) g_test_add_func ("/route/ip4", test_ip4_route); g_test_add_func ("/route/ip6", test_ip6_route); + g_test_add_func ("/route/ip4_metric0", test_ip4_route_metric0); } -- cgit v1.2.1 From 96c099de09888ea614477b021673bb5c2e38c542 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 11 Jan 2015 17:42:46 +0100 Subject: platform: suppress change event when deleting IPv4 route with metric 0 refresh_object() raised a spurious change event for the route we are about to delete. Suppress that by adding an internal reason flag. Fixes: 41e6c4fac1890e4302e4bfd1aa21be362cce51f2 --- src/platform/nm-linux-platform.c | 8 +++++++- src/platform/nm-platform.h | 5 ++++- src/platform/tests/test-route.c | 4 ++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 50dfe16c78..48b5f8e16b 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1668,6 +1668,9 @@ announce_object (NMPlatform *platform, const struct nl_object *object, NMPlatfor { NMPlatformIP4Route route; + if (reason == _NM_PLATFORM_REASON_CACHE_CHECK_INTERNAL) + return; + if (!_route_match ((struct rtnl_route *) object, AF_INET, 0, FALSE)) { nm_log_dbg (LOGD_PLATFORM, "skip announce unmatching IP4 route %s", to_string_ip4_route ((struct rtnl_route *) object)); return; @@ -1680,6 +1683,9 @@ announce_object (NMPlatform *platform, const struct nl_object *object, NMPlatfor { NMPlatformIP6Route route; + if (reason == _NM_PLATFORM_REASON_CACHE_CHECK_INTERNAL) + return; + if (!_route_match ((struct rtnl_route *) object, AF_INET6, 0, FALSE)) { nm_log_dbg (LOGD_PLATFORM, "skip announce unmatching IP6 route %s", to_string_ip6_route ((struct rtnl_route *) object)); return; @@ -3919,7 +3925,7 @@ ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, int plen * * Instead, re-fetch the route from kernel, and if that fails, there is nothing to do. * On success, there is still a race that we might end up deleting the wrong route. */ - if (!refresh_object (platform, (struct nl_object *) route, FALSE, NM_PLATFORM_REASON_INTERNAL)) { + if (!refresh_object (platform, (struct nl_object *) route, FALSE, _NM_PLATFORM_REASON_CACHE_CHECK_INTERNAL)) { rtnl_route_put ((struct rtnl_route *) route); return TRUE; } diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 848f78e428..3f37ed6048 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -69,7 +69,10 @@ typedef enum { /* Event came from the kernel. */ NM_PLATFORM_REASON_EXTERNAL, /* Event is a result of cache checking and cleanups. */ - NM_PLATFORM_REASON_CACHE_CHECK + NM_PLATFORM_REASON_CACHE_CHECK, + + /* Internal reason to suppress announcing change events */ + _NM_PLATFORM_REASON_CACHE_CHECK_INTERNAL, } NMPlatformReason; #define __NMPlatformObject_COMMON \ diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c index 69f5bc2e4f..fec1e38d26 100644 --- a/src/platform/tests/test-route.c +++ b/src/platform/tests/test-route.c @@ -55,7 +55,7 @@ test_ip4_route_metric0 (void) { int ifindex = nm_platform_link_get_ifindex (DEVICE_NAME); SignalData *route_added = add_signal (NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, NM_PLATFORM_SIGNAL_ADDED, ip4_route_callback); - /*SignalData *route_changed = add_signal (NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, NM_PLATFORM_SIGNAL_CHANGED, ip4_route_callback);*/ + SignalData *route_changed = add_signal (NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, NM_PLATFORM_SIGNAL_CHANGED, ip4_route_callback); SignalData *route_removed = add_signal (NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, NM_PLATFORM_SIGNAL_REMOVED, ip4_route_callback); in_addr_t network = nmtst_inet4_from_string ("192.0.2.5"); /* from 192.0.2.0/24 (TEST-NET-1) (rfc5737) */ int plen = 32; @@ -115,7 +115,7 @@ test_ip4_route_metric0 (void) assert_ip4_route_exists (FALSE, DEVICE_NAME, network, plen, metric); free_signal (route_added); - /*free_signal (route_changed);*/ + free_signal (route_changed); free_signal (route_removed); } -- cgit v1.2.1