core: fix route metrics for subnet routes

For IPv4 addresses, the kernel automatically adds a route when
configuring an IP address. Unfortunately, there is no way to control
this behavior or to set the route metric.

Fix this, by adding our own route and removing the kernel provided
one.

Note that this adds a major change in that we no longer call
nm_ip4_config_commit() for assumed devices.

https://bugzilla.gnome.org/show_bug.cgi?id=723178

Signed-off-by: Thomas Haller <thaller@redhat.com>
This commit is contained in:
Thomas Haller 2014-11-11 14:55:07 +01:00
parent 0b50940f43
commit 34124296c0
7 changed files with 54 additions and 13 deletions

View file

@ -314,6 +314,7 @@ typedef struct {
static gboolean nm_device_set_ip4_config (NMDevice *self,
NMIP4Config *config,
guint32 default_route_metric,
gboolean commit,
NMDeviceStateReason *reason);
static gboolean ip4_config_merge_and_apply (NMDevice *self,
@ -2782,6 +2783,7 @@ ip4_config_merge_and_apply (NMDevice *self,
NMConnection *connection;
gboolean success;
NMIP4Config *composite;
const guint32 default_route_metric = nm_device_get_ip4_route_metric (self);
/* Merge all the configs into the composite config */
if (config) {
@ -2817,7 +2819,7 @@ ip4_config_merge_and_apply (NMDevice *self,
if (!nm_settings_connection_get_nm_generated_assumed (NM_SETTINGS_CONNECTION (connection))) {
nm_ip4_config_merge_setting (composite,
nm_connection_get_setting_ip4_config (connection),
nm_device_get_ip4_route_metric (self));
default_route_metric);
}
/* Add the default route.
@ -2848,7 +2850,7 @@ ip4_config_merge_and_apply (NMDevice *self,
memset (route, 0, sizeof (*route));
route->source = NM_IP_CONFIG_SOURCE_USER;
route->gateway = gateway;
route->metric = nm_device_get_ip4_route_metric (self);
route->metric = default_route_metric;
route->mss = nm_ip4_config_get_mss (composite);
priv->default_route.v4_has = TRUE;
@ -2878,7 +2880,7 @@ ip4_config_merge_and_apply (NMDevice *self,
NM_DEVICE_GET_CLASS (self)->ip4_config_pre_commit (self, composite);
}
success = nm_device_set_ip4_config (self, composite, commit, out_reason);
success = nm_device_set_ip4_config (self, composite, default_route_metric, commit, out_reason);
g_object_unref (composite);
return success;
}
@ -5602,6 +5604,7 @@ nm_device_get_ip4_config (NMDevice *self)
static gboolean
nm_device_set_ip4_config (NMDevice *self,
NMIP4Config *new_config,
guint32 default_route_metric,
gboolean commit,
NMDeviceStateReason *reason)
{
@ -5623,7 +5626,12 @@ nm_device_set_ip4_config (NMDevice *self,
/* Always commit to nm-platform to update lifetimes */
if (commit && new_config) {
success = nm_ip4_config_commit (new_config, ip_ifindex);
gboolean assumed = nm_device_uses_assumed_connection (self);
/* for assumed devices we set the device_route_metric to the default which will
* stop nm_platform_ip4_address_sync() to replace the device routes. */
success = nm_ip4_config_commit (new_config, ip_ifindex,
assumed ? NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE : default_route_metric);
if (!success)
reason_local = NM_DEVICE_STATE_REASON_CONFIG_FAILED;
}
@ -6994,7 +7002,7 @@ _cleanup_generic_post (NMDevice *self, gboolean deconfigure)
/* Clean up IP configs; this does not actually deconfigure the
* interface; the caller must flush routes and addresses explicitly.
*/
nm_device_set_ip4_config (self, NULL, TRUE, &ignored);
nm_device_set_ip4_config (self, NULL, 0, TRUE, &ignored);
nm_device_set_ip6_config (self, NULL, TRUE, &ignored);
g_clear_object (&priv->dev_ip4_config);
g_clear_object (&priv->ext_ip4_config);

View file

@ -80,7 +80,7 @@ dhcp4_state_changed (NMDhcpClient *client,
nm_ip4_config_subtract (existing, last_config);
nm_ip4_config_merge (existing, ip4_config);
if (!nm_ip4_config_commit (existing, ifindex))
if (!nm_ip4_config_commit (existing, ifindex, priority))
nm_log_warn (LOGD_DHCP4, "(%s): failed to apply DHCPv4 config", ifname);
if (last_config) {

View file

@ -251,7 +251,7 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf)
}
gboolean
nm_ip4_config_commit (const NMIP4Config *config, int ifindex)
nm_ip4_config_commit (const NMIP4Config *config, int ifindex, guint32 default_route_metric)
{
NMIP4ConfigPrivate *priv = NM_IP4_CONFIG_GET_PRIVATE (config);
guint32 mtu = nm_ip4_config_get_mtu (config);
@ -261,7 +261,7 @@ nm_ip4_config_commit (const NMIP4Config *config, int ifindex)
g_return_val_if_fail (config != NULL, FALSE);
/* Addresses */
nm_platform_ip4_address_sync (ifindex, priv->addresses);
nm_platform_ip4_address_sync (ifindex, priv->addresses, default_route_metric);
/* Routes */
{

View file

@ -64,7 +64,7 @@ const char * nm_ip4_config_get_dbus_path (const NMIP4Config *config);
/* Integration with nm-platform and nm-setting */
NMIP4Config *nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf);
gboolean nm_ip4_config_commit (const NMIP4Config *config, int ifindex);
gboolean nm_ip4_config_commit (const NMIP4Config *config, int ifindex, guint32 default_route_metric);
void nm_ip4_config_merge_setting (NMIP4Config *config, NMSettingIPConfig *setting, guint32 default_route_metric);
NMSetting *nm_ip4_config_create_setting (const NMIP4Config *config);

View file

@ -1736,6 +1736,8 @@ _address_get_lifetime (const NMPlatformIPAddress *address, guint32 now, guint32
* nm_platform_ip4_address_sync:
* @ifindex: Interface index
* @known_addresses: List of addresses
* @device_route_metric: the route metric for adding subnet routes (replaces
* the kernel added routes).
*
* A convenience function to synchronize addresses for a specific interface
* with the least possible disturbance. It simply removes addresses that are
@ -1744,7 +1746,7 @@ _address_get_lifetime (const NMPlatformIPAddress *address, guint32 now, guint32
* Returns: %TRUE on success.
*/
gboolean
nm_platform_ip4_address_sync (int ifindex, const GArray *known_addresses)
nm_platform_ip4_address_sync (int ifindex, const GArray *known_addresses, guint32 device_route_metric)
{
GArray *addresses;
NMPlatformIP4Address *address;
@ -1768,6 +1770,7 @@ nm_platform_ip4_address_sync (int ifindex, const GArray *known_addresses)
for (i = 0; i < known_addresses->len; i++) {
const NMPlatformIP4Address *known_address = &g_array_index (known_addresses, NMPlatformIP4Address, i);
guint32 lifetime, preferred;
guint32 network;
/* add a padding of 5 seconds to avoid potential races. */
if (!_address_get_lifetime ((NMPlatformIPAddress *) known_address, now, 5, &lifetime, &preferred))
@ -1775,6 +1778,31 @@ nm_platform_ip4_address_sync (int ifindex, const GArray *known_addresses)
if (!nm_platform_ip4_address_add (ifindex, known_address->address, known_address->peer_address, known_address->plen, lifetime, preferred, known_address->label))
return FALSE;
if (known_address->plen == 0 || known_address->plen == 32)
continue;
if (device_route_metric == NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE) {
/* Kernel already adds routes for us with this metric. */
continue;
}
/* Kernel automatically adds a device route for us with metric 0. That is not what we want.
* Remove it, and re-add it.
*
* In face of having the same subnets on two different interfaces with the same metric,
* this is a problem. Surprisingly, kernel is able to add two routes for the same subnet/prefix,metric
* to different interfaces. We cannot. Adding one, will replace the other. Indeed we will
* toggle the routes between the interfaces.
*
* Indeed we have that problem for all our routes, that if another interface wants the same route
* we don't coordinate them. See bug 740064.
*
* The workaround is to configure different device priorities via ipv4.route-metric. */
network = nm_utils_ip4_address_clear_host_address (known_address->address, known_address->plen);
nm_platform_ip4_route_add (ifindex, NM_IP_CONFIG_SOURCE_KERNEL, network, known_address->plen, 0, known_address->address, device_route_metric, 0);
nm_platform_ip4_route_delete (ifindex, network, known_address->plen, NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE);
}
return TRUE;
@ -1837,7 +1865,7 @@ nm_platform_ip6_address_sync (int ifindex, const GArray *known_addresses)
gboolean
nm_platform_address_flush (int ifindex)
{
return nm_platform_ip4_address_sync (ifindex, NULL)
return nm_platform_ip4_address_sync (ifindex, NULL, 0)
&& nm_platform_ip6_address_sync (ifindex, NULL);
}

View file

@ -188,6 +188,10 @@ typedef union {
* Thus, the value is not choosen arbitraily, but matches kernel IPv6 default. */
#define NM_PLATFORM_ROUTE_METRIC_DEFAULT 1024
/* For IPv4, kernel adds a device route (subnet routes) with metric 0 when user
* configures addresses. */
#define NM_PLATFORM_ROUTE_METRIC_IP4_DEVICE_ROUTE 0
#define __NMPlatformIPRoute_COMMON \
__NMPlatformObject_COMMON; \
NMIPConfigSource source; \
@ -556,7 +560,7 @@ gboolean nm_platform_ip4_address_delete (int ifindex, in_addr_t address, int ple
gboolean nm_platform_ip6_address_delete (int ifindex, struct in6_addr address, int plen);
gboolean nm_platform_ip4_address_exists (int ifindex, in_addr_t address, int plen);
gboolean nm_platform_ip6_address_exists (int ifindex, struct in6_addr address, int plen);
gboolean nm_platform_ip4_address_sync (int ifindex, const GArray *known_addresses);
gboolean nm_platform_ip4_address_sync (int ifindex, const GArray *known_addresses, guint32 device_route_metric);
gboolean nm_platform_ip6_address_sync (int ifindex, const GArray *known_addresses);
gboolean nm_platform_address_flush (int ifindex);

View file

@ -936,7 +936,8 @@ nm_vpn_connection_apply_config (NMVpnConnection *connection)
nm_platform_link_set_up (priv->ip_ifindex);
if (priv->ip4_config) {
if (!nm_ip4_config_commit (priv->ip4_config, priv->ip_ifindex))
if (!nm_ip4_config_commit (priv->ip4_config, priv->ip_ifindex,
nm_vpn_connection_get_ip4_route_metric (connection)))
return FALSE;
}