Commit f22c72d2 authored by Thomas Haller's avatar Thomas Haller

policy: improve get_best_device() to strictly adhering the sort order of the entries

get_best_device() has two different modes depending on the @fully_activated argument.
If @fully_activated, it only considers devices that are considered as active.
Otherwise, it returns the best activating device (if that device is expected
to be better then any of the already activated devices).

Before, the check whether an activated device is considered best device
also involved looking at the device state. This redundancy was harmful
because part of NMDefaultRouteManager considered a device as fully activated,
but get_best_device() might not return them.

Split get_best_device() in two parts. The one part _ipx_get_best_activating_device()
now checks for still activating devices. When inspecting devices with
an entry, those devices are weighted according to _ipx_get_best_device().
That means that both functions now give a consistent result.
Signed-off-by: Thomas Haller's avatarThomas Haller <thaller@redhat.com>
parent eb61cdc6
......@@ -670,6 +670,34 @@ nm_default_route_manager_ip6_connection_has_default_route (NMDefaultRouteManager
/***********************************************************************************/
/** _ipx_get_best_device:
* @vtable: the virtual table
* @self: #NMDefaultRouteManager
**/
static NMDevice *
_ipx_get_best_device (const VTableIP *vtable, NMDefaultRouteManager *self)
{
NMDefaultRouteManagerPrivate *priv;
GPtrArray *entries;
guint i;
g_return_val_if_fail (NM_IS_DEFAULT_ROUTE_MANAGER (self), NULL);
priv = NM_DEFAULT_ROUTE_MANAGER_GET_PRIVATE (self);
entries = vtable->get_entries (priv);
for (i = 0; i < entries->len; i++) {
Entry *entry = g_ptr_array_index (entries, i);
if (!NM_IS_DEVICE (entry->source.pointer))
continue;
g_assert (!entry->never_default);
return entry->source.pointer;
}
return NULL;
}
/** _ipx_get_best_activating_device:
* @vtable: the virtual table
* @self: #NMDefaultRouteManager
* @devices: list of devices to be searched. Only devices from this list will be considered
......@@ -680,101 +708,81 @@ nm_default_route_manager_ip6_connection_has_default_route (NMDefaultRouteManager
* the same priority.
**/
static NMDevice *
_ipx_get_best_device (const VTableIP *vtable, NMDefaultRouteManager *self, const GSList *devices, gboolean fully_activated, NMDevice *preferred_device)
_ipx_get_best_activating_device (const VTableIP *vtable, NMDefaultRouteManager *self, const GSList *devices, NMDevice *preferred_device)
{
NMDefaultRouteManagerPrivate *priv;
const GSList *iter;
NMDevice *best_device = NULL;
guint32 best_prio = G_MAXUINT32;
guint best_idx = G_MAXUINT;
NMDevice *best_activated_device;
g_return_val_if_fail (NM_IS_DEFAULT_ROUTE_MANAGER (self), NULL);
priv = NM_DEFAULT_ROUTE_MANAGER_GET_PRIVATE (self);
best_activated_device = _ipx_get_best_device (vtable, self);
g_return_val_if_fail (!best_activated_device || g_slist_find ((GSList *) devices, best_activated_device), NULL);
for (iter = devices; iter; iter = g_slist_next (iter)) {
NMDevice *device = NM_DEVICE (iter->data);
NMDeviceState state = nm_device_get_state (device);
guint32 prio;
guint idx;
Entry *entry;
if ( state <= NM_DEVICE_STATE_DISCONNECTED
|| state >= NM_DEVICE_STATE_DEACTIVATING)
continue;
entry = _entry_find_by_source (vtable->get_entries (priv), device, NULL);
if (fully_activated && state < NM_DEVICE_STATE_SECONDARIES)
continue;
if (entry) {
/* of all the device that have an entry, we already know that best_activated_device
* is the best. entry cannot be better. */
if (entry->source.device != best_activated_device)
continue;
prio = entry->effective_metric;
} else {
NMDeviceState state = nm_device_get_state (device);
if (!_ipx_connection_has_default_route (vtable, self, nm_device_get_connection (device)))
continue;
if ( state <= NM_DEVICE_STATE_DISCONNECTED
|| state >= NM_DEVICE_STATE_DEACTIVATING)
continue;
entry = _entry_find_by_source (vtable->get_entries (priv), device, &idx);
if (fully_activated && !entry)
continue;
if (!_ipx_connection_has_default_route (vtable, self, nm_device_get_connection (device)))
continue;
if (entry)
prio = entry->effective_metric;
else
prio = nm_device_get_ip4_route_metric (device);
}
prio = vtable->route_metric_normalize (prio);
if (!best_device || prio < best_prio)
goto NEW_BEST_DEVICE;
if (prio == best_prio) {
/* the devices have the same priority... which one to prefer? */
if (fully_activated) {
/* for @fully_activated, respect the order as from the sorted entries. */
if (idx < best_idx)
goto NEW_BEST_DEVICE;
} else {
/* for !fully_activated, prefer @preferred_device. */
if (preferred_device == device)
goto NEW_BEST_DEVICE;
if (preferred_device != best_device) {
/* if both devices are not @preferred_device, prefer
* the one with an entry (or the one with lower index,
* if both have an entry).
*
* The following is correct because if @best_device has no entry, @best_idx
* is G_MAXUINT. Also, if the currenty device has no entry, @idx is G_MAXUINT. */
if (idx < best_idx)
goto NEW_BEST_DEVICE;
}
}
if ( !best_device
|| prio < best_prio
|| (prio == best_prio && preferred_device == device)) {
best_device = device;
best_prio = prio;
}
continue;
NEW_BEST_DEVICE:
best_device = device;
best_prio = prio;
best_idx = idx;
}
if (best_device && !fully_activated) {
/* There's only a best activating device if the best device
* among all activating and already-activated devices is a
* still-activating one.
*/
if (nm_device_get_state (best_device) >= NM_DEVICE_STATE_SECONDARIES)
return NULL;
}
/* There's only a best activating device if the best device
* among all activating and already-activated devices is a
* still-activating one.
*/
if (best_device && nm_device_get_state (best_device) >= NM_DEVICE_STATE_SECONDARIES)
return NULL;
return best_device;
}
NMDevice *
nm_default_route_manager_ip4_get_best_device (NMDefaultRouteManager *self, const GSList *devices, gboolean fully_activated, NMDevice *preferred_device)
{
return _ipx_get_best_device (&vtable_ip4, self, devices, fully_activated, preferred_device);
if (fully_activated)
return _ipx_get_best_device (&vtable_ip4, self);
else
return _ipx_get_best_activating_device (&vtable_ip4, self, devices, preferred_device);
}
NMDevice *
nm_default_route_manager_ip6_get_best_device (NMDefaultRouteManager *self, const GSList *devices, gboolean fully_activated, NMDevice *preferred_device)
{
return _ipx_get_best_device (&vtable_ip6, self, devices, fully_activated, preferred_device);
if (fully_activated)
return _ipx_get_best_device (&vtable_ip6, self);
else
return _ipx_get_best_activating_device (&vtable_ip6, self, devices, preferred_device);
}
/***********************************************************************************/
......@@ -864,7 +872,7 @@ _ipx_get_best_config (const VTableIP *vtable,
/* If no VPN connections, we use the best device instead */
if (!config_result) {
device = _ipx_get_best_device (vtable, self, nm_manager_get_devices (manager), TRUE, preferred_device);
device = _ipx_get_best_device (vtable, self);
if (device) {
if (VTABLE_IS_IP4)
config_result = nm_device_get_ip4_config (device);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment