From 5cf58e6ba5a94615b0e318d25fac1fd405119aab Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Dec 2014 15:10:22 +0100 Subject: [PATCH 1/5] ifcfg-rh: support ipvx.route-metric property as IPVX_ROUTE_METRIC Write ipv4.route-metric and ipv6.route-metric property of NMSettingConnection as IPV4_ROUTE_METRIC and IPV6_ROUTE_METRIC, respectively. (cherry picked from commit 3e33a5a6c553ba9f8d8c888b127e86ad3b462e7d) Conflicts: src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c --- libnm-core/nm-setting-ip4-config.c | 9 +++++++++ libnm-core/nm-setting-ip6-config.c | 9 +++++++++ src/settings/plugins/ifcfg-rh/reader.c | 4 ++++ .../tests/network-scripts/ifcfg-test-wifi-open | 2 ++ .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 13 ++++++++++++- src/settings/plugins/ifcfg-rh/writer.c | 14 ++++++++++++++ 6 files changed, 50 insertions(+), 1 deletion(-) diff --git a/libnm-core/nm-setting-ip4-config.c b/libnm-core/nm-setting-ip4-config.c index 686192678a..81cc1f8889 100644 --- a/libnm-core/nm-setting-ip4-config.c +++ b/libnm-core/nm-setting-ip4-config.c @@ -571,6 +571,15 @@ nm_setting_ip4_config_class_init (NMSettingIP4ConfigClass *ip4_class) * ---end--- */ + /* ---ifcfg-rh--- + * property: route-metric + * variable: IPV4_ROUTE_METRIC(+) + * default: -1 + * description: IPV4_ROUTE_METRIC is the default IPv4 metric for routes on this connection. + * If set to -1, a default metric based on the device type is used. + * ---end--- + */ + /** * NMSettingIP4Config:dhcp-client-id: * diff --git a/libnm-core/nm-setting-ip6-config.c b/libnm-core/nm-setting-ip6-config.c index f5d903340c..29ca551525 100644 --- a/libnm-core/nm-setting-ip6-config.c +++ b/libnm-core/nm-setting-ip6-config.c @@ -488,6 +488,15 @@ nm_setting_ip6_config_class_init (NMSettingIP6ConfigClass *ip6_class) * ---end--- */ + /* ---ifcfg-rh--- + * property: route-metric + * variable: IPV6_ROUTE_METRIC(+) + * default: -1 + * description: IPV6_ROUTE_METRIC is the default IPv6 metric for routes on this connection. + * If set to -1, a default metric based on the device type is used. + * ---end--- + */ + /** * NMSettingIP6Config:ip6-privacy: * diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c index 145a0a3915..0fba321e48 100644 --- a/src/settings/plugins/ifcfg-rh/reader.c +++ b/src/settings/plugins/ifcfg-rh/reader.c @@ -966,6 +966,8 @@ make_ip4_setting (shvarFile *ifcfg, NM_SETTING_IP_CONFIG_IGNORE_AUTO_ROUTES, !svTrueValue (ifcfg, "PEERROUTES", TRUE), NM_SETTING_IP_CONFIG_NEVER_DEFAULT, never_default, NM_SETTING_IP_CONFIG_MAY_FAIL, !svTrueValue (ifcfg, "IPV4_FAILURE_FATAL", FALSE), + NM_SETTING_IP_CONFIG_ROUTE_METRIC, svGetValueInt64 (ifcfg, "IPV4_ROUTE_METRIC", 10, + -1, G_MAXUINT32, -1), NULL); if (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) == 0) @@ -1362,6 +1364,8 @@ make_ip6_setting (shvarFile *ifcfg, NM_SETTING_IP_CONFIG_IGNORE_AUTO_ROUTES, !svTrueValue (ifcfg, "IPV6_PEERROUTES", TRUE), NM_SETTING_IP_CONFIG_NEVER_DEFAULT, never_default, NM_SETTING_IP_CONFIG_MAY_FAIL, !svTrueValue (ifcfg, "IPV6_FAILURE_FATAL", FALSE), + NM_SETTING_IP_CONFIG_ROUTE_METRIC, svGetValueInt64 (ifcfg, "IPV6_ROUTE_METRIC", 10, + -1, G_MAXUINT32, -1), NM_SETTING_IP6_CONFIG_IP6_PRIVACY, ip6_privacy_val, NULL); diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-open b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-open index 48db45b25d..b089103dae 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-open +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-open @@ -12,3 +12,5 @@ ONBOOT=yes USERCTL=yes PEERDNS=yes IPV6INIT=no +IPV4_ROUTE_METRIC=104 +IPV6_ROUTE_METRIC=106 diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 40650cc022..b65537882c 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -2610,7 +2610,7 @@ test_read_wifi_open (void) NMSettingConnection *s_con; NMSettingWireless *s_wireless; NMSettingWirelessSecurity *s_wsec; - NMSettingIPConfig *s_ip4; + NMSettingIPConfig *s_ip4, *s_ip6; char *unmanaged = NULL; char *keyfile = NULL; char *routefile = NULL; @@ -2764,6 +2764,8 @@ test_read_wifi_open (void) TEST_IFCFG_WIFI_OPEN, NM_SETTING_IP4_CONFIG_SETTING_NAME); + g_assert_cmpint (nm_setting_ip_config_get_route_metric (s_ip4), ==, 104); + /* Method */ tmp = nm_setting_ip_config_get_method (s_ip4); ASSERT (strcmp (tmp, NM_SETTING_IP4_CONFIG_METHOD_AUTO) == 0, @@ -2772,6 +2774,10 @@ test_read_wifi_open (void) NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP_CONFIG_METHOD); + s_ip6 = nm_connection_get_setting_ip6_config (connection); + g_assert( s_ip6); + g_assert_cmpint (nm_setting_ip_config_get_route_metric (s_ip6), ==, 106); + g_free (unmanaged); g_free (keyfile); g_free (routefile); @@ -6294,6 +6300,7 @@ test_write_wired_static (void) NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_MANUAL, NM_SETTING_IP_CONFIG_MAY_FAIL, TRUE, NM_SETTING_IP_CONFIG_GATEWAY, "1.1.1.1", + NM_SETTING_IP_CONFIG_ROUTE_METRIC, (gint64) 204, NULL); addr = nm_ip_address_new (AF_INET, "1.1.1.3", 24, &error); @@ -6319,6 +6326,7 @@ test_write_wired_static (void) g_object_set (s_ip6, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_MANUAL, NM_SETTING_IP_CONFIG_MAY_FAIL, TRUE, + NM_SETTING_IP_CONFIG_ROUTE_METRIC, (gint64) 206, NULL); /* Add addresses */ @@ -6406,6 +6414,9 @@ test_write_wired_static (void) nm_setting_ip_config_remove_dns_search (reread_s_ip4, 3); nm_setting_ip_config_remove_dns_search (reread_s_ip4, 2); + g_assert_cmpint (nm_setting_ip_config_get_route_metric (reread_s_ip4), ==, 204); + g_assert_cmpint (nm_setting_ip_config_get_route_metric (reread_s_ip6), ==, 206); + ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE, "wired-static-write", "written and re-read connection weren't the same."); diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c index f4ffffd6a8..300b0c1665 100644 --- a/src/settings/plugins/ifcfg-rh/writer.c +++ b/src/settings/plugins/ifcfg-rh/writer.c @@ -1806,6 +1806,7 @@ write_ip4_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) char *route_path = NULL; gint32 j; guint32 i, n, num; + gint64 route_metric; GString *searches; gboolean success = FALSE; gboolean fake_ip4 = FALSE; @@ -2015,6 +2016,11 @@ write_ip4_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) nm_setting_ip_config_get_may_fail (s_ip4) ? "no" : "yes", FALSE); + route_metric = nm_setting_ip_config_get_route_metric (s_ip4); + tmp = route_metric != -1 ? g_strdup_printf ("%"G_GINT64_FORMAT, route_metric) : NULL; + svSetValue (ifcfg, "IPV4_ROUTE_METRIC", tmp, FALSE); + g_free (tmp); + /* Static routes - route- file */ route_path = utils_get_route_path (ifcfg->fileName); if (!route_path) { @@ -2249,10 +2255,12 @@ write_ip6_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) NMSettingIPConfig *s_ip4; const char *value; char *addr_key; + char *tmp; guint32 i, num, num4; GString *searches; NMIPAddress *addr; const char *dns; + gint64 route_metric; GString *ip_str1, *ip_str2, *ip_ptr; char *route6_path; @@ -2266,6 +2274,7 @@ write_ip6_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) svSetValue (ifcfg, "IPV6_PEERDNS", "yes", FALSE); svSetValue (ifcfg, "IPV6_PEERROUTES", "yes", FALSE); svSetValue (ifcfg, "IPV6_FAILURE_FATAL", "no", FALSE); + svSetValue (ifcfg, "IPV6_ROUTE_METRIC", NULL, FALSE); return TRUE; } @@ -2380,6 +2389,11 @@ write_ip6_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) nm_setting_ip_config_get_may_fail (s_ip6) ? "no" : "yes", FALSE); + route_metric = nm_setting_ip_config_get_route_metric (s_ip6); + tmp = route_metric != -1 ? g_strdup_printf ("%"G_GINT64_FORMAT, route_metric) : NULL; + svSetValue (ifcfg, "IPV6_ROUTE_METRIC", tmp, FALSE); + g_free (tmp); + /* IPv6 Privacy Extensions */ svSetValue (ifcfg, "IPV6_PRIVACY", NULL, FALSE); svSetValue (ifcfg, "IPV6_PRIVACY_PREFER_PUBLIC_IP", NULL, FALSE); From 5103cd944f7acc944e9845c6d120ef897d1012bb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Dec 2014 18:05:17 +0100 Subject: [PATCH 2/5] platform: fix data type for metric argument of refresh_route() (cherry picked from commit 4ba8df425f749adb606979b64ab03af9ff890ae3) --- src/platform/nm-linux-platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 5d1522ce98..6fe1d466d2 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3884,7 +3884,7 @@ route_search_cache (struct nl_cache *cache, int family, int ifindex, const void } static gboolean -refresh_route (NMPlatform *platform, int family, int ifindex, const void *network, int plen, int metric) +refresh_route (NMPlatform *platform, int family, int ifindex, const void *network, int plen, guint32 metric) { struct nl_cache *cache; auto_nl_object struct rtnl_route *cached_object = NULL; From 818150bf6eea487c256a6cc40bb8b65272d2f46c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Dec 2014 17:06:13 +0100 Subject: [PATCH 3/5] platform: fix IPv6 route methods for metric 0 Handling a route with metric 0 effectively means a metric of 1024 (user default). Adjust the add(), delete() and exist() functions to consider routes with metric 0 as 1024. (cherry picked from commit 06e4eee0ce782460274ffab3c83bb8369f73d834) --- src/platform/nm-linux-platform.c | 6 ++++++ src/platform/tests/test-route.c | 7 ++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 6fe1d466d2..0cf743b453 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3845,6 +3845,8 @@ ip6_route_add (NMPlatform *platform, int ifindex, NMIPConfigSource source, struct in6_addr network, int plen, struct in6_addr gateway, guint32 metric, guint32 mss) { + metric = nm_utils_ip6_route_metric_normalize (metric); + return add_object (platform, build_rtnl_route (AF_INET6, ifindex, source, &network, plen, &gateway, NULL, metric, mss)); } @@ -3962,6 +3964,8 @@ ip6_route_delete (NMPlatform *platform, int ifindex, struct in6_addr network, in { struct in6_addr gateway = IN6ADDR_ANY_INIT; + metric = nm_utils_ip6_route_metric_normalize (metric); + return delete_object (platform, build_rtnl_route (AF_INET6, ifindex, NM_IP_CONFIG_SOURCE_UNKNOWN ,&network, plen, &gateway, NULL, metric, 0), FALSE) && refresh_route (platform, AF_INET6, ifindex, &network, plen, metric); } @@ -3989,6 +3993,8 @@ ip4_route_exists (NMPlatform *platform, int ifindex, in_addr_t network, int plen static gboolean ip6_route_exists (NMPlatform *platform, int ifindex, struct in6_addr network, int plen, guint32 metric) { + metric = nm_utils_ip6_route_metric_normalize (metric); + return ip_route_exists (platform, AF_INET6, ifindex, &network, plen, metric); } diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c index 2c70b412bb..9ccca0a0f6 100644 --- a/src/platform/tests/test-route.c +++ b/src/platform/tests/test-route.c @@ -2,6 +2,7 @@ #include "test-common.h" #include "nm-test-utils.h" +#include "NetworkManagerUtils.h" #define DEVICE_NAME "nm-test-device" @@ -205,21 +206,21 @@ test_ip6_route (void) rts[0].plen = 128; rts[0].ifindex = ifindex; rts[0].gateway = in6addr_any; - rts[0].metric = metric; + rts[0].metric = nm_utils_ip6_route_metric_normalize (metric); rts[0].mss = mss; rts[1].source = NM_IP_CONFIG_SOURCE_USER; rts[1].network = network; rts[1].plen = plen; rts[1].ifindex = ifindex; rts[1].gateway = gateway; - rts[1].metric = metric; + rts[1].metric = nm_utils_ip6_route_metric_normalize (metric); rts[1].mss = mss; rts[2].source = NM_IP_CONFIG_SOURCE_USER; rts[2].network = in6addr_any; rts[2].plen = 0; rts[2].ifindex = ifindex; rts[2].gateway = gateway; - rts[2].metric = metric; + rts[2].metric = nm_utils_ip6_route_metric_normalize (metric); rts[2].mss = mss; g_assert_cmpint (routes->len, ==, 3); g_assert (!memcmp (routes->data, rts, sizeof (rts))); From dd418598d5ab274fd0488fd20241ea1241202317 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Dec 2014 18:01:12 +0100 Subject: [PATCH 4/5] platform: route_search_cache() should not treat a metric 0 as "any" route This match-any behavior ignoring metric is nowhere used. And even if we would need such a behavior, using 0 is wrong because IPv4 routes can have a metric of zero. (cherry picked from commit 2cb3c7e8a0164219dcbdc06a4fe073527bb23b7b) --- src/platform/nm-linux-platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 0cf743b453..d59b55bc1f 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3865,7 +3865,7 @@ route_search_cache (struct nl_cache *cache, int family, int ifindex, const void if (!_route_match (rtnlroute, family, ifindex, FALSE)) continue; - if (metric && metric != rtnl_route_get_priority (rtnlroute)) + if (metric != rtnl_route_get_priority (rtnlroute)) continue; dst = rtnl_route_get_dst (rtnlroute); From 59352bd4b03b41a286a347e5f09671e1e0531376 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Dec 2014 17:59:16 +0100 Subject: [PATCH 5/5] platform: workaround deletion of IPv4 route with metric 0 Deleting routes with metric 0 might end up deleting other routes with a different metric. Workaround this in platform to only delete a route with metric 0 if such a route can be found prior to deletion. Don't only look into the cache (which might be out of date). Instead refetch the route we are about to delete to be sure. There is still a race that we might end up deleting the wrong route. https://bugzilla.gnome.org/show_bug.cgi?id=741871 https://bugzilla.redhat.com/show_bug.cgi?id=1172780 (cherry picked from commit 41e6c4fac1890e4302e4bfd1aa21be362cce51f2) --- src/platform/nm-linux-platform.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index d59b55bc1f..0c35376cdb 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3912,6 +3912,19 @@ ip4_route_delete (NMPlatform *platform, int ifindex, in_addr_t network, int plen cache = choose_cache_by_type (platform, OBJECT_TYPE_IP4_ROUTE); + if (metric == 0) { + /* Deleting an IPv4 route with metric 0 does not only delete an exectly matching route. + * If no route with metric 0 exists, it might delete another route to the same destination. + * For nm_platform_ip4_route_delete() we don't want this semantic. + * + * 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)) { + rtnl_route_put ((struct rtnl_route *) route); + return TRUE; + } + } + /* when deleting an IPv4 route, several fields of the provided route must match. * Lookup in the cache so that we hopefully get the right values. */ cached_object = (struct rtnl_route *) nl_cache_search (cache, route);