From 329791ad55ee66e2d12c3cedcc2daf8c0865f7a6 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 20 Oct 2014 20:36:57 -0400 Subject: [PATCH] all: stop pretending to support multiple "gateway"s NMSettingIP[46]Config let you associate a gateway with each address, and the writable settings backends record that information. But it never actually gets used: NMIP4Config and NMIP6Config only ever use the first gateway, and completely ignore any others. (And in the common usage of the term, an interface can only have one gateway anyway.) So, stop pretending that multiple gateways are meaningful; don't serialize or deserialize gateways other than the first in the 'addresses' properties, and don't read or write multiple gateway values either. --- libnm-core/nm-utils.c | 21 ++++++++++++------- src/nm-ip4-config.c | 18 +++++++--------- src/nm-ip6-config.c | 11 +++++----- src/settings/plugins/ifcfg-rh/reader.c | 3 +++ .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 7 ++----- src/settings/plugins/ifcfg-rh/writer.c | 4 ++-- .../plugins/ifnet/connection_parser.c | 9 ++++---- src/settings/plugins/ifupdown/parser.c | 8 +++++-- src/settings/plugins/keyfile/reader.c | 11 ++++++---- .../plugins/keyfile/tests/test-keyfile.c | 11 +++++----- src/settings/plugins/keyfile/writer.c | 5 ++++- 11 files changed, 59 insertions(+), 49 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 0f67da373..110142fbf 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1140,7 +1140,7 @@ nm_utils_ip4_dns_from_variant (GVariant *value) * Utility function to convert a #GPtrArray of #NMIPAddress objects representing * IPv4 addresses into a #GVariant of type 'aau' representing an array of * NetworkManager IPv4 addresses (which are tuples of address, prefix, and - * gateway). + * gateway, although only the first "gateway" field is preserved) * * Returns: (transfer none): a new floating #GVariant representing @addresses. **/ @@ -1162,7 +1162,7 @@ nm_utils_ip4_addresses_to_variant (GPtrArray *addresses) nm_ip_address_get_address_binary (addr, &array[0]); array[1] = nm_ip_address_get_prefix (addr); - if (nm_ip_address_get_gateway (addr)) + if (i == 0 && nm_ip_address_get_gateway (addr)) inet_pton (AF_INET, nm_ip_address_get_gateway (addr), &array[2]); else array[2] = 0; @@ -1182,7 +1182,8 @@ nm_utils_ip4_addresses_to_variant (GPtrArray *addresses) * * Utility function to convert a #GVariant of type 'aau' representing a list of * NetworkManager IPv4 addresses (which are tuples of address, prefix, and - * gateway) into a #GPtrArray of #NMIPAddress objects. + * gateway, although only the first "gateway" field is preserved) into a + * #GPtrArray of #NMIPAddress objects. * * Returns: (transfer full) (element-type NMIPAddress): a newly allocated * #GPtrArray of #NMIPAddress objects @@ -1213,7 +1214,8 @@ nm_utils_ip4_addresses_from_variant (GVariant *value) } addr = nm_ip_address_new_binary (AF_INET, - &addr_array[0], addr_array[1], &addr_array[2], + &addr_array[0], addr_array[1], + addresses->len == 0 ? &addr_array[2] : NULL, &error); if (addr) g_ptr_array_add (addresses, addr); @@ -1471,7 +1473,7 @@ nm_utils_ip6_dns_from_variant (GVariant *value) * Utility function to convert a #GPtrArray of #NMIPAddress objects representing * IPv6 addresses into a #GVariant of type 'a(ayuay)' representing an array of * NetworkManager IPv6 addresses (which are tuples of address, prefix, and - * gateway). + * gateway, although only the first "gateway" field is preserved). * * Returns: (transfer none): a new floating #GVariant representing @addresses. **/ @@ -1498,7 +1500,7 @@ nm_utils_ip6_addresses_to_variant (GPtrArray *addresses) prefix = nm_ip_address_get_prefix (addr); - if (nm_ip_address_get_gateway (addr)) + if (i == 0 && nm_ip_address_get_gateway (addr)) inet_pton (AF_INET6, nm_ip_address_get_gateway (addr), &gateway_bytes); else memset (&gateway_bytes, 0, sizeof (gateway_bytes)); @@ -1517,7 +1519,8 @@ nm_utils_ip6_addresses_to_variant (GPtrArray *addresses) * * Utility function to convert a #GVariant of type 'a(ayuay)' representing a * list of NetworkManager IPv6 addresses (which are tuples of address, prefix, - * and gateway) into a #GPtrArray of #NMIPAddress objects. + * and gateway, although only the first "gateway" field is preserved) into a + * #GPtrArray of #NMIPAddress objects. * * Returns: (transfer full) (element-type NMIPAddress): a newly allocated * #GPtrArray of #NMIPAddress objects @@ -1560,7 +1563,9 @@ nm_utils_ip6_addresses_from_variant (GVariant *value) goto next; } - addr = nm_ip_address_new_binary (AF_INET6, addr_bytes, prefix, gateway_bytes, &error); + addr = nm_ip_address_new_binary (AF_INET6, addr_bytes, prefix, + addresses->len == 0 ? gateway_bytes : NULL, + &error); if (addr) g_ptr_array_add (addresses, addr); else { diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index c51b44992..af284f0bc 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -319,14 +319,13 @@ nm_ip4_config_merge_setting (NMIP4Config *config, NMSettingIPConfig *setting, in nm_ip4_config_set_never_default (config, TRUE); else if (nm_setting_ip_config_get_ignore_auto_routes (setting)) nm_ip4_config_set_never_default (config, FALSE); - for (i = 0; i < naddresses; i++) { - const char *gateway_str = nm_ip_address_get_gateway (nm_setting_ip_config_get_address (setting, i)); + if (naddresses) { + const char *gateway_str = nm_ip_address_get_gateway (nm_setting_ip_config_get_address (setting, 0)); guint32 gateway; if (gateway_str) { inet_pton (AF_INET, gateway_str, &gateway); nm_ip4_config_set_gateway (config, gateway); - break; } } @@ -426,13 +425,9 @@ nm_ip4_config_create_setting (const NMIP4Config *config) if (!method) method = NM_SETTING_IP4_CONFIG_METHOD_MANUAL; - s_addr = nm_ip_address_new_binary (AF_INET, &address->address, address->plen, NULL, NULL); - /* For backwards compatibility, attach the gateway to an address if it's - * in the same subnet. - */ - if (same_prefix (address->address, gateway, address->plen)) - nm_ip_address_set_gateway (s_addr, nm_utils_inet4_ntop (gateway, NULL)); - + s_addr = nm_ip_address_new_binary (AF_INET, &address->address, address->plen, + i == 0 ? &gateway : NULL, + NULL); if (*address->label) nm_ip_address_set_attribute (s_addr, "label", g_variant_new_string (address->label)); @@ -1723,10 +1718,11 @@ get_property (GObject *object, guint prop_id, for (i = 0; i < naddr; i++) { const NMPlatformIP4Address *address = nm_ip4_config_get_address (config, i); GArray *array = g_array_sized_new (FALSE, TRUE, sizeof (guint32), 3); + guint32 gateway = i == 0 ? priv->gateway : 0; g_array_append_val (array, address->address); g_array_append_val (array, address->plen); - g_array_append_val (array, priv->gateway); + g_array_append_val (array, gateway); g_ptr_array_add (addresses, array); } diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index e022e7178..fd21e5a62 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -423,14 +423,13 @@ nm_ip6_config_merge_setting (NMIP6Config *config, NMSettingIPConfig *setting, in nm_ip6_config_set_never_default (config, TRUE); else if (nm_setting_ip_config_get_ignore_auto_routes (setting)) nm_ip6_config_set_never_default (config, FALSE); - for (i = 0; i < naddresses; i++) { - const char *gateway_str = nm_ip_address_get_gateway (nm_setting_ip_config_get_address (setting, i)); + if (naddresses) { + const char *gateway_str = nm_ip_address_get_gateway (nm_setting_ip_config_get_address (setting, 0)); struct in6_addr gateway; if (gateway_str) { inet_pton (AF_INET6, gateway_str, &gateway); nm_ip6_config_set_gateway (config, &gateway); - break; } } @@ -532,7 +531,9 @@ nm_ip6_config_create_setting (const NMIP6Config *config) if (!method || strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL) == 0) method = NM_SETTING_IP6_CONFIG_METHOD_MANUAL; - s_addr = nm_ip_address_new_binary (AF_INET6, &address->address, address->plen, gateway, NULL); + s_addr = nm_ip_address_new_binary (AF_INET6, &address->address, address->plen, + i == 0 ? gateway : NULL, + NULL); nm_setting_ip_config_add_address (s_ip6, s_addr); nm_ip_address_unref (s_addr); } @@ -1610,7 +1611,7 @@ get_property (GObject *object, guint prop_id, /* Gateway */ g_value_init (&element, DBUS_TYPE_G_UCHAR_ARRAY); ba = g_byte_array_new (); - g_byte_array_append (ba, (guint8 *) (gateway ? gateway : &in6addr_any), sizeof (*gateway)); + g_byte_array_append (ba, (guint8 *) (i == 0 && gateway ? gateway : &in6addr_any), sizeof (*gateway)); g_value_take_boxed (&element, ba); g_value_array_append (array, &element); g_value_unset (&element); diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c index b16302c0b..7b193589d 100644 --- a/src/settings/plugins/ifcfg-rh/reader.c +++ b/src/settings/plugins/ifcfg-rh/reader.c @@ -1042,6 +1042,9 @@ make_ip4_setting (shvarFile *ifcfg, continue; } + if (nm_setting_ip_config_get_num_addresses (s_ip4)) + nm_ip_address_set_gateway (addr, NULL); + if (!nm_setting_ip_config_add_address (s_ip4, addr)) PARSE_WARNING ("duplicate IP4 address"); nm_ip_address_unref (addr); 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 3c43a2f5d..e9ce7ca43 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -7831,14 +7831,11 @@ test_write_gateway (void) g_assert_cmpstr (val, ==, "1.1.1.254"); g_free (val); - val = svGetValue (f, "GATEWAY1", FALSE); - g_assert (val); - g_assert_cmpstr (val, ==, "2.2.2.254"); - g_free (val); - val = svGetValue (f, "GATEWAY0", FALSE); g_assert (val == NULL); + val = svGetValue (f, "GATEWAY1", FALSE); + g_assert (val == NULL); svCloseFile (f); diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c index d12ff03cf..694ed4315 100644 --- a/src/settings/plugins/ifcfg-rh/writer.c +++ b/src/settings/plugins/ifcfg-rh/writer.c @@ -1916,7 +1916,7 @@ write_ip4_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) svSetValue (ifcfg, netmask_key, NULL, FALSE); - svSetValue (ifcfg, gw_key, nm_ip_address_get_gateway (addr), FALSE); + svSetValue (ifcfg, gw_key, n == 0 ? nm_ip_address_get_gateway (addr) : NULL, FALSE); g_free (addr_key); g_free (prefix_key); @@ -2172,7 +2172,7 @@ write_ip4_aliases (NMConnection *connection, char *base_ifcfg_path) svSetValue (ifcfg, "PREFIX", tmp, FALSE); g_free (tmp); - svSetValue (ifcfg, "GATEWAY", nm_ip_address_get_gateway (addr), FALSE); + svSetValue (ifcfg, "GATEWAY", i == 0 ? nm_ip_address_get_gateway (addr) : NULL, FALSE); svWriteFile (ifcfg, 0644, NULL); svCloseFile (ifcfg); diff --git a/src/settings/plugins/ifnet/connection_parser.c b/src/settings/plugins/ifnet/connection_parser.c index 4233c75f2..6deee55f0 100644 --- a/src/settings/plugins/ifnet/connection_parser.c +++ b/src/settings/plugins/ifnet/connection_parser.c @@ -614,8 +614,9 @@ make_ip4_setting (NMConnection *connection, NMIPAddress *ip4_addr; GError *local = NULL; - /* currently all the IPs has the same gateway */ - ip4_addr = nm_ip_address_new (AF_INET, iblock->ip, iblock->prefix, iblock->next_hop, &local); + ip4_addr = nm_ip_address_new (AF_INET, iblock->ip, iblock->prefix, + nm_setting_ip_config_get_num_addresses (ip4_setting) == 0 ? iblock->next_hop : NULL, + &local); if (iblock->next_hop) g_object_set (ip4_setting, NM_SETTING_IP_CONFIG_IGNORE_AUTO_ROUTES, @@ -2379,7 +2380,6 @@ write_ip4_setting (NMConnection *connection, const char *conn_name, GError **err GString *ips; GString *routes; GString *dns; - gboolean has_def_route = FALSE; gboolean success = FALSE; s_ip4 = nm_connection_get_setting_ip4_config (connection); @@ -2408,11 +2408,10 @@ write_ip4_setting (NMConnection *connection, const char *conn_name, GError **err nm_ip_address_get_prefix (addr)); /* only the first gateway will be written */ - if (!has_def_route && nm_ip_address_get_gateway (addr)) { + if (i == 0 && nm_ip_address_get_gateway (addr)) { g_string_append_printf (routes, "\"default via %s\" ", nm_ip_address_get_gateway (addr)); - has_def_route = TRUE; } } ifnet_set_data (conn_name, "config", ips->str); diff --git a/src/settings/plugins/ifupdown/parser.c b/src/settings/plugins/ifupdown/parser.c index f916fa0e7..8dc37dcd7 100644 --- a/src/settings/plugins/ifupdown/parser.c +++ b/src/settings/plugins/ifupdown/parser.c @@ -486,7 +486,9 @@ update_ip4_setting_from_if_block(NMConnection *connection, gateway_v = address_v; /* dcbw: whaaa?? */ /* Add the new address to the setting */ - addr = nm_ip_address_new (AF_INET, address_v, netmask_int, gateway_v, error); + addr = nm_ip_address_new (AF_INET, address_v, netmask_int, + nm_setting_ip_config_get_num_addresses (s_ip4) == 0 ? gateway_v : NULL, + error); if (!addr) goto error; @@ -599,7 +601,9 @@ update_ip6_setting_from_if_block(NMConnection *connection, gateway_v = address_v; /* dcbw: whaaa?? */ /* Add the new address to the setting */ - addr = nm_ip_address_new (AF_INET6, address_v, prefix_int, gateway_v, error); + addr = nm_ip_address_new (AF_INET6, address_v, prefix_int, + nm_setting_ip_config_get_num_addresses (s_ip6) == 0 ? gateway_v : NULL, + error); if (!addr) goto error; diff --git a/src/settings/plugins/keyfile/reader.c b/src/settings/plugins/keyfile/reader.c index 526831a32..46c80552e 100644 --- a/src/settings/plugins/keyfile/reader.c +++ b/src/settings/plugins/keyfile/reader.c @@ -360,11 +360,14 @@ ip_address_or_route_parser (NMSetting *setting, const char *key, GKeyFile *keyfi key_name = g_strdup (*key_basename); item = read_one_ip_address_or_route (keyfile, setting_name, key_name, ipv6, routes); - - if (item) - g_ptr_array_add (list, item); - g_free (key_name); + + if (!item) + continue; + + if (!routes && list->len > 0) + nm_ip_address_set_gateway (item, NULL); + g_ptr_array_add (list, item); } } diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 89504b38a..9cc98615f 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -247,10 +247,10 @@ test_read_valid_wired_connection (void) /* IPv4 addresses */ g_assert (nm_setting_ip_config_get_num_addresses (s_ip4) == 6); check_ip_address (s_ip4, 0, "2.3.4.5", 24, "2.3.4.6"); - check_ip_address (s_ip4, 1, "192.168.0.5", 24, "192.168.0.1"); - check_ip_address (s_ip4, 2, "1.2.3.4", 16, "1.2.1.1"); + check_ip_address (s_ip4, 1, "192.168.0.5", 24, NULL); + check_ip_address (s_ip4, 2, "1.2.3.4", 16, NULL); check_ip_address (s_ip4, 3, "3.4.5.6", 16, NULL); - check_ip_address (s_ip4, 4, "4.5.6.7", 24, "1.2.3.4"); + check_ip_address (s_ip4, 4, "4.5.6.7", 24, NULL); check_ip_address (s_ip4, 5, "5.6.7.8", 24, NULL); /* IPv4 routes */ @@ -337,7 +337,7 @@ test_read_valid_wired_connection (void) check_ip_address (s_ip6, 6, "3:4:5:6:7:8:9:16", 66, NULL); check_ip_address (s_ip6, 7, "3:4:5:6:7:8:9:17", 67, NULL); check_ip_address (s_ip6, 8, "3:4:5:6:7:8:9:18", 68, NULL); - check_ip_address (s_ip6, 9, "3:4:5:6:7:8:9:19", 69, "1::9"); + check_ip_address (s_ip6, 9, "3:4:5:6:7:8:9:19", 69, NULL); /* Route #1 */ g_assert (nm_setting_ip_config_get_num_routes (s_ip6) == 7); @@ -406,7 +406,6 @@ test_write_wired_connection (void) const char *address1 = "192.168.0.5"; const char *address1_gw = "192.168.0.1"; const char *address2 = "1.2.3.4"; - const char *address2_gw = "1.2.1.1"; const char *route1 = "10.10.10.2"; const char *route1_nh = "10.10.10.1"; const char *route2 = "1.1.1.1"; @@ -467,7 +466,7 @@ test_write_wired_connection (void) /* Addresses */ add_one_ip_address (s_ip4, address1, 24, address1_gw); - add_one_ip_address (s_ip4, address2, 8, address2_gw); + add_one_ip_address (s_ip4, address2, 8, NULL); /* Routes */ add_one_ip_route (s_ip4, route1, route1_nh, 24, 3); diff --git a/src/settings/plugins/keyfile/writer.c b/src/settings/plugins/keyfile/writer.c index fdf4823a5..e6515398e 100644 --- a/src/settings/plugins/keyfile/writer.c +++ b/src/settings/plugins/keyfile/writer.c @@ -147,7 +147,10 @@ write_ip_values (GKeyFile *file, addr = nm_ip_address_get_address (address); plen = nm_ip_address_get_prefix (address); - gw = nm_ip_address_get_gateway (address); + if (i == 0) + gw = nm_ip_address_get_gateway (address); + else + gw = NULL; metric = 0; }