We already protected route-metrics that are configured as default-routes
in platform. For most cases, that list is identical to our internal list
of non-synced routes.
But if for some reason that is not the case, we must also protect the
metric of routs that we currently track as "non-synced".
Also accept a NULL connection in
nm_default_route_manager_ip4_connection_has_default_route() and
nm_default_route_manager_ip6_connection_has_default_route().
Most nm_platform_*() functions operate on the platform
singleton nm_platform_get(). That made sense because the
NMPlatform instance was mainly to hook fake platform for
testing.
While the implicit argument saved some typing, I think explicit is
better. Especially, because NMPlatform could become a more usable
object then just a hook for testing.
With this change, NMPlatform instances can be used individually, not
only as a singleton instance.
Before this change, the constructor of NMLinuxPlatform could not
call any nm_platform_*() functions because the singleton was not
yet initialized. We could only instantiate an incomplete instance,
register it via nm_platform_setup(), and then complete initialization
via singleton->setup().
With this change, we can create and fully initialize NMPlatform instances
before/without setting them up them as singleton.
Also, currently there is no clear distinction between functions
that operate on the NMPlatform instance, and functions that can
be used stand-alone (e.g. nm_platform_ip4_address_to_string()).
The latter can not be mocked for testing. With this change, the
distinction becomes obvious. That is also useful because it becomes
clearer which functions make use of the platform cache and which not.
Inside nm-linux-platform.c, continue the pattern that the
self instance is named @platform. That makes sense because
its type is NMPlatform, and not NMLinuxPlatform what we
would expect from a paramter named @self.
This is a major diff that causes some pain when rebasing. Try
to rebase to the parent commit of this commit as a first step.
Then rebase on top of this commit using merge-strategy "ours".
During dipose(), NMDefaultRouteManager unrefed all the source pointers
in its list -- thereby having dangling pointers in the list of entries.
The unrefing can cause the final destruction of the device (during
shutdown), which would again call into NMDefaultRouteManager.
Fix this by ensuring that after disposing starts, all external calls
into NMDefaultRouteManager return early.
#0 0x00007ffff4a2cc60 in g_logv (log_domain=0x535b51 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd530) at gmessages.c:1046
#1 0x00007ffff4a2ce9f in g_log (log_domain=log_domain@entry=0x535b51 "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x528f68 "file %s: line %d (%s): should not be reached") at gmessages.c:1079
#2 0x000000000049b83b in _ipx_update_default_route (vtable=vtable@entry=0x7a49c0 <vtable_ip6>, self=0x7d1350 [NMDefaultRouteManager], source=source@entry=0x8b64e0) at nm-default-route-manager.c:659
#3 0x000000000049c652 in nm_default_route_manager_ip6_update_default_route (self=<optimized out>, source=source@entry=0x8b64e0) at nm-default-route-manager.c:819
#4 0x00000000004526a8 in _cleanup_generic_post (self=self@entry=0x8b64e0, deconfigure=deconfigure@entry=0) at devices/nm-device.c:7235
#5 0x0000000000452bc0 in dispose (object=0x8b64e0) at devices/nm-device.c:8324
#6 0x00007ffff4d29cbc in g_object_unref (_object=0x8b64e0) at gobject.c:3133
#7 0x0000000000499091 in _entry_free (entry=0x8bf140) at nm-default-route-manager.c:187
#8 0x00007ffff49fa82b in g_ptr_array_foreach (array=0x81d220, func=0x499080 <_entry_free>, user_data=0x0) at garray.c:1502
#9 0x00007ffff49fa8c0 in ptr_array_free (array=0x81d220, flags=FREE_SEGMENT) at garray.c:1088
#10 0x00007ffff49fa939 in g_ptr_array_free (array=<optimized out>, free_segment=free_segment@entry=1) at garray.c:1075
#11 0x000000000049abcf in dispose (object=0x7d1350 [NMDefaultRouteManager]) at nm-default-route-manager.c:1357
#12 0x00007ffff4d29cbc in g_object_unref (_object=0x7d1350) at gobject.c:3133
#13 0x00007ffff7deb507 in _dl_fini () at dl-fini.c:252
#14 0x00007ffff4658382 in __run_exit_handlers (status=status@entry=0, listp=0x7ffff49d66a0 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true) at exit.c:82
#15 0x00007ffff46583d5 in __GI_exit (status=status@entry=0) at exit.c:104
#16 0x0000000000432fd6 in main (argc=1, argv=0x7fffffffdeb8) at main.c:473
Since 0c136c1e2, we also track the default route for devices
without active-connection (unmanaged). They must not be returned
as best_config() or as best_device(), because the caller don't
expect unmanaged devices here.
This also gets closer to the original behavior of get_best_device()
before merging default-route-manager in d4417e3460
where we also would ignore devices depending on the state.
This fixes an assertion when having an interface unmanaged
and activating it externally:
#0 0x00007ffff6806187 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55
#1 0x00007ffff6807dea in __GI_abort () at abort.c:89
#2 0x00007ffff6c0aec5 in g_assertion_message (domain=domain@entry=0x5332d5 "NetworkManager", file=file@entry=0x539460 "nm-default-route-manager.c", line=line@entry=1056, func=func@entry=0x539920 <__FUNCTION__.28934> "_ipx_get_best_config", message=message@entry=0x8c9cb0 "assertion failed: (req)") at gtestutils.c:2356
#3 0x00007ffff6c0af5a in g_assertion_message_expr (domain=domain@entry=0x5332d5 "NetworkManager", file=file@entry=0x539460 "nm-default-route-manager.c", line=line@entry=1056, func=func@entry=0x539920 <__FUNCTION__.28934> "_ipx_get_best_config", expr=expr@entry=0x53132f "req") at gtestutils.c:2371
#4 0x000000000049a196 in _ipx_get_best_config (self=<optimized out>, ignore_never_default=ignore_never_default@entry=1, out_ip_iface=out_ip_iface@entry=0x7fffffffd310, out_ac=out_ac@entry=0x0, out_device=0x0, out_vpn=0x7fffffffd318, vtable=0x7a0a00 <vtable_ip4>, vtable=0x7a0a00 <vtable_ip4>) at nm-default-route-manager.c:1056
#5 0x000000000049a3f6 in nm_default_route_manager_ip4_get_best_config (self=<optimized out>, ignore_never_default=ignore_never_default@entry=1, out_ip_iface=out_ip_iface@entry=0x7fffffffd310, out_ac=out_ac@entry=0x0, out_device=out_device@entry=0x0, out_vpn=out_vpn@entry=0x7fffffffd318) at nm-default-route-manager.c:1079
#6 0x00000000004b7518 in update_ip4_dns (self=0x7ecac0 [NMPolicy], out_vpn=0x7fffffffd318, out_device=0x0, out_ac=0x0, out_ip_iface=0x7fffffffd310, ignore_never_default=1) at nm-policy.c:390
#7 0x00000000004b7518 in update_ip4_dns (dns_mgr=0x8623a0 [NMDnsManager], policy=0x7ecac0 [NMPolicy]) at nm-policy.c:406
#8 0x00000000004b99d5 in device_ip4_config_changed (device=0x8844b0 [NMDeviceEthernet], new_config=0x908aa0 [NMIP4Config], old_config=0x908aa0 [NMIP4Config], user_data=0x7ecac0) at nm-policy.c:1260
#9 0x000000368f405d60 in ffi_call_unix64 () at /lib64/libffi.so.6
#10 0x000000368f4057d1 in ffi_call () at /lib64/libffi.so.6
#11 0x00007ffff6ee5b8c in g_cclosure_marshal_generic_va (closure=0x886a40, return_value=0x0, instance=0x8844b0, args_list=<optimized out>, marshal_data=0x0, n_params=2, param_types=0x87d3a0) at gclosure.c:1541
#12 0x00007ffff6ee5144 in _g_closure_invoke_va (closure=closure@entry=0x886a40, return_value=return_value@entry=0x0, instance=instance@entry=0x8844b0, args=args@entry=0x7fffffffd810, n_params=<optimized out>, param_types=0x87d3a0) at gclosure.c:831
#13 0x00007ffff6eff900 in g_signal_emit_valist (instance=0x8844b0, signal_id=<optimized out>, detail=0, var_args=var_args@entry=0x7fffffffd810) at gsignal.c:3201
#14 0x00007ffff6f0014f in g_signal_emit (instance=instance@entry=0x8844b0, signal_id=<optimized out>, detail=detail@entry=0) at gsignal.c:3348
#15 0x000000000044a1f4 in nm_device_set_ip4_config (self=self@entry=0x8844b0 [NMDeviceEthernet], new_config=new_config@entry=0x9089a0 [NMIP4Config], default_route_metric=default_route_metric@entry=100, commit=commit@entry=0, reason=reason@entry=0x0) at devices/nm-device.c:5866
#16 0x000000000044a5af in ip4_config_merge_and_apply (self=self@entry=0x8844b0 [NMDeviceEthernet], config=config@entry=0x0, commit=commit@entry=0, out_reason=out_reason@entry=0x0) at devices/nm-device.c:3037
#17 0x000000000044b4fd in update_ip_config (self=self@entry=0x8844b0 [NMDeviceEthernet], initial=initial@entry=0) at devices/nm-device.c:6617
#18 0x000000000044bc39 in queued_ip_config_change (user_data=<optimized out>) at devices/nm-device.c:6688
#19 0x00007ffff6be4e1b in g_main_context_dispatch (context=0x7ba3a0) at gmain.c:3122
#20 0x00007ffff6be4e1b in g_main_context_dispatch (context=context@entry=0x7ba3a0) at gmain.c:3737
#21 0x00007ffff6be51b0 in g_main_context_iterate (context=0x7ba3a0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3808
#22 0x00007ffff6be54d2 in g_main_loop_run (loop=0x7ba460) at gmain.c:4002
#23 0x000000000043291d in main (argc=1, argv=0x7fffffffdef8) at main.c:442
Fixes: da708059da
The previous syntax (s/S for synced, n/N for never-default) is confusing.
Indicate 'never-default' by '0', vs. '1'.
Indicated synced/non-synced as '+sync' and '-sync'.
Before, we would only track a device in NMDefaultRouteManager
if it had a default route. Otherwise the entry for the device
was removed.
That was wrong, because having no entry meant that the interface
is assumed and hence we would not touch the interface. Instead we must
esplicitly track devices without default route to know when an interface
has no default route.
Signed-off-by: Thomas Haller <thaller@redhat.com>
It's better to add the more important routes first. Otherwise there
might be a short time when a lower priority route has precedence.
Signed-off-by: Thomas Haller <thaller@redhat.com>
The previous commit made NM enforce the default route on interfaces for
which NM manages a default route.
For interfaces that are configured never-default, NM will now pick up
any externally configured default route, as if it was managed by NM.
This is important, because NMDefaultRouteManager needs a notion of which
is the best device. Without this change, it was agnostic to default routes
on managed, never-default interfaces.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Monitor default routes from platform, and resync the default routes
on changes.
For one, this fixes the following use-case: have an assumed device em1
with two routes of metric 20 and 21. Activate em2, which will get effective
metric 22.
When externally removing route em1/20, em2 would resync the effective metric to
20. This is correct and already worked before. However, when deleting em1/21,
nothing happened. With this change, em2 would resync to metric 21 to fill the gap.
However this commit has much bigger effects: whenever the user externally adds
a default route to an interface for which NM manages an default route, NM will
delete it.
Also, when deleting the default route (managed by NM), NM would readd
it. Effectivly, the user can no longer mess with the default route on
interfaces for which it manages the default route.
If the connection is configured never-default, the user still can add
default routes and NM will not touch them.
Obviously, this has no effect for assumed devices either and the user
can externally add and remove default routes as he wishes.
https://bugzilla.gnome.org/show_bug.cgi?id=735512
Signed-off-by: Thomas Haller <thaller@redhat.com>
Don't only consider the best route of assumed devices when syncing the route
metrics. This fixes the following scenario:
Have em1 assumed, with two default routes (metric 20 and 21).
When activating em2, NMDefaultRouteManager would have determined
21 as the effective metric, thus replacing the assumed route of em1.
Since we don't want to touch assumed interfaces, it is wrong to
replace their default routes.
Instead, keep track of all the assumed default routes and consider their
metrics when choosing effective_metric.
Signed-off-by: Thomas Haller <thaller@redhat.com>
When calling update_default_route(), NMDefaultRouteManager will look at the
source, and determine whether it has a default route or not. For example
for device sources, this means calling nm_device_get_ip4_default_route().
If the source indicates that it has no default route, the effect of
calling update_default_route() is the same as calling
remove_default_route() (hence, remove() can be replaced by update()).
If the source however still indicates a default route, the behavior
would be different. This case would be an undesired inconsistancy,
because source and NMDefaultRouteManager would disagree of whether
the source has a default route.
Source must always properly indicate whether it has a default route
or not, hence this situation does not arise.
Hence it is always better to call update().
Signed-off-by: Thomas Haller <thaller@redhat.com>
The case of having a metric MAXUINT32 is special, because in face of
multiple default routes with the same metric, NMDefaultRouteManager
cannot reduce the effective metric (because there is no lower priority
value).
This case works already correct, just when adding such a default route,
ensure that we add it to the *first* entry.
Signed-off-by: Thomas Haller <thaller@redhat.com>
config.h should be included from every .c file, and it should be
included before any other include. Fix that.
(As a side effect of how I did this, this also changes us to
consistently use "config.h" rather than <config.h>. To the extent that
it matters [which is not much], quotes are more correct anyway, since
we're talking about a file in our own build tree, not a system
include.)
This fixes an assertion during shutdown. NMManager:dispose()
calls remove_device(), which eventually hit the assertion
in nm_default_route_manager_ip4_get_best_device().
Remove the assertion, but also make sure that the function only
returns devices from the provided list. It is counter intuitive,
that the function might return devices that are not in the provided
list.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Now that both VPN and devices are managed (and ordered) by
NMDefaultRouteManager, refactor get_best_config() to use the
priority accordingly.
Before, we would first iterate over all VPN connections and
returning the best one. Only if no suitable VPN connection
was found, a best device would be returned.
Modify get_best_config() to treat VPN and device the same and
return the best one based on the route metric.
With this change, get_best_config() gives consistent results
together with get_best_device(). Also, you can configure
that a device gets a higher priority then a VPN.
Signed-off-by: Thomas Haller <thaller@redhat.com>
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 <thaller@redhat.com>
Extend NMDefaultRouteManager to track NMVpnConnection beside
NMDevice. That way, all default routes are managed by
NMDefaultRouteManager.
For VPN connections the manager also tracks connections that are
set never_default. That is useful because NMPolicy still uses VPNs
without default route to setup DNS. Hence, NMDefaultRouteManager
trackes those connections to have the relative priority of the
devices.
Interestingly, that means that for VPNs that are ipv4.never-default,
ipv4.route-metric still has an effect in determining relative priorities
for DNS configuration.
This commit only adds the parts to track the default route. NMPolicy
still sets the route as before.
Signed-off-by: Thomas Haller <thaller@redhat.com>
NMDefaultRouteManager has a sorted list of routes. Change get_best_device()
to better consider that list when choosing a best device.
Always prefer the priority as reported from entry->effective_metric
if the device is registered to have a default route. Before for
!fully_activated, we choose the metric based on
nm_device_get_ipx_route_metric().
Add more checks in case of equal priority. For @fully_activated,
always prefer the device that is sorted by NMDefaultRouteManager.
For non @fully_activated, prefer the device with an entry.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Up to now, NMPolicy would iterate over all devices to find the "best"
device and assign the default route to that device.
A better approach is to add a default route to *all* devices that
are never-default=no. The relative priority is choosen according to
the route metrics.
If two devices receive the same metric, we want to prefer the device
that activates first. That way, the default route sticks to the same
device until a better device activates or the device deactivates.
Hence, the order of activation is imporant in this case (as it is
already now).
Also, if several devices have identical metrics, increment their
metrics so that every metric is unique.
This makes the routing deterministic according to what we choose as best
device.
A special case is assumed devices. In this case we cannot adjust the metric
in face of equal metrics.
Add a new singleton class NMDefaultRouteManager that has a list of all
devices and their default routes. The manager will order the devices by
their priority and configure the routes using platform.
Also update the metric for VPN connections. Later we will track VPN
routes also via NMDefaultRouteManager. For now, fix the VPN metric because
otherwise VPNs would always get metric 1024 (which is usually much larger then the
device metrics).
https://bugzilla.gnome.org/show_bug.cgi?id=735512
Signed-off-by: Thomas Haller <thaller@redhat.com>
