- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
This enum was unused and meaningless because the platform signals
are emitted as a consequence of netlink messages. It is not clear
whether a netlink message was received due to an external event
or an internal action.
Arguably, it is more convenient to use the static buffer as
it saves typing.
But having such a low-level function use a static buffer also
limits the way how to use it. As it was, you could not avoid
using the static buffer.
E.g. you cannot do:
char buf[100];
_LOGD ("nmp-object: %s; platform-link: %s",
nmp_object_to_string (nmpobj, buf, sizeof(buf)),
nm_platform_link_to_string (link));
This will fail for non-obvious reasons because both
to-string functions end up using the same static buffer.
Also change the to-string implementations to accept NULL
as valid and return it as "(null)".
https://bugzilla.gnome.org/show_bug.cgi?id=756427
Clone the connection upon activation. This makes it safe for the user
to modify the original connection while it is activated.
This involves several changes:
- NMActiveConnection gets @settings_connection and @applied_connection.
To support add-and-activate, we constructing a NMActiveConnection with
no connection set. Previously, we would set the "connection" field to
a temporary NMConnection. Now NMManager piggybacks this temporary
connection as object-data (TAG_ACTIVE_CONNETION_ADD_AND_ACTIVATE).
- get rid of the functions nm_active_connection_get_connection_type()
and nm_active_connection_get_connection_uuid(). From their names
it is unclear whether this returns the settings or applied connection.
The (few) callers should figure that out themselves.
- rename nm_active_connection_get_id() to
nm_active_connection_get_settings_connection_id(). This function
is only used internally for logging.
- dispatcher calls now get two connections as well. The
applied-connection is used for the connection data, while
the settings-connection is used for the connection path.
- needs special handling for properties that apply immediately
when changed (nm_device_reapply_settings_immediately()).
Co-Authored-By: Thomas Haller <thaller@redhat.com>
https://bugzilla.gnome.org/show_bug.cgi?id=724041
The logging macros _LOGD(), etc. are specific to each
file as they format the message according to their context.
Still, they were cumbersome to define and their implementation
was repeated over and over (slightly different at times).
Move the declaration of these macros to "nm-logging.h".
The source file now only needs to define _NMLOG(), and either
_NMLOG_ENABLED() or _NMLOG_DOMAIN.
This reduces code duplication and encourages a common implementation
and usage of these macros.
It is wrong to only consider internal_gateway of the VPN connection.
Instead, we must first set the gateway of NMIP4Config and then overwrite
it with the connection settings.
When a VPN has no default route, it is wrong to enforce the absence
of a default route on that device. Instead, if there is no default
route, NMDefaultRouteManager should just forget about the route.
This is especially important, because for VPN types like openswan
there is no distinct tunnel interface. Instead, it shares the ifindex
with the parent-device.
Note that devices usually only enforce their default-route for a short
time and afterwards switch to non-synced. If that happens and there
is a VPN that enforces the absense of the default route on that device,
we end up deleting the default route.
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>
