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.)
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>
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>
get_best_ip4_config() and get_best_ip6_config() checked both for
never-default of the setting. This check was redundant, because
the never-default value was already merged into NMIPXConfig.
Signed-off-by: Thomas Haller <thaller@redhat.com>
In get_best_ip4_device() and get_best_ip6_device(), move
conditions to check for suitable connection first.
Makes the following patch more coherent.
Signed-off-by: Thomas Haller <thaller@redhat.com>
When adding a default route fails, the most common
reason is that we don't have a direct route to the gateway.
In that case, NMPolicy tries to add a direct route to
the gateway and then retries adding the default route.
For VPN however, previously NMPolicy would not added a direct
route to the gateway via the VPN device. Instead it would add a
direct route to the external gateway via the parent interface.
That is wrong.
Indeed the external gateway must be reachable directly not via the
VPN interface itself. But for that the vpn connection already sets
a route via nm_device_set_vpn4_config().
Signed-off-by: Thomas Haller <thaller@redhat.com>
Split a base NMSettingIPConfig class out of NMSettingIP4Config and
NMSettingIP6Config, and update things accordingly.
Further simplifications of now-redundant IPv4-vs-IPv6 code are
possible, and should happen in the future.
Previously the only thing preventing default-unmanaged devices from
being auto-activated was luck and the fact that they didn't have any
available connections when in the UNMANAGED state. That's no longer
true, so we must be more explicit about their behavior.
Furthermore it makes no sense to allow default-unmanaged devices
to set priv->autoconnect=TRUE since that is never supposed to
happen, so enforce that both in NM itself and if the change
request comes in over the D-Bus interface.
Lastly, internal priv->autoconnect=TRUE changes never emitted a
property change notification, meaning the NMPolicy would never
schedule an autoconnect check if the device's priv->autoconnect
was set to TRUE as a result of re-activating or waking from sleep.
Next we want to sort the array, g_slist_sort() is not guaranteed to be
stable, while g_ptr_array_sort() is. Also, sorting a GSList has
worse performance.
Signed-off-by: Thomas Haller <thaller@redhat.com>
nm_device_get_best_auto_connection() was only used at one place.
It was a very simple function, just iterated over a list finding
the first can_auto_connect() connection. At the very least, the name
was misleading, because it did not return the 'best', but the 'first'
connection.
Get rid of the function altogether.
Signed-off-by: Thomas Haller <thaller@redhat.com>
GLib/Gtk have mostly settled on the convention that two-letter
acronyms in type names remain all-caps (eg, "IO"), but longer acronyms
become initial-caps-only (eg, "Tcp").
NM was inconsistent, with most long acronyms using initial caps only
(Adsl, Cdma, Dcb, Gsm, Olpc, Vlan), but others using all caps (DHCP,
PPP, PPPOE, VPN). Fix libnm and src/ to use initial-caps only for all
three-or-more-letter-long acronyms (and update nmcli and nmtui for the
libnm changes).
Clean up some of the cross-includes between headers (which made it so
that, eg, if you included NetworkManagerUtils.h in a test program, you
would need to build the test with -I$(top_srcdir)/src/platform, and if
you included nm-device.h you'd need $(POLKIT_CFLAGS)) by moving all
GObject struct definitions for src/ and src/settings/ into nm-types.h
(which already existed to solve the NMDevice/NMActRequest circular
references).
Update various .c files to explicitly include the headers they used to
get implicitly, and remove some now-unnecessary -I options from
Makefiles.
Even if administrator-configured hostname (/etc/hostname) takes precedence
over other hostname configurations, we don't take "localhost", "localhost6",
"localhost.localdomain", "localhost6.localdomain6" as such. These values might
be set by some tools (like installer). But that's not right and we compensate
for that. It doesn't make much sense that an admimistrator would set these
values manually (intentionally), because leaving /etc/hostname empty will
result in "localhost" hostname anyway (set by systemd).
https://bugzilla.redhat.com/show_bug.cgi?id=1110436
No need to keep references of the singleton and take an additional ref
when accessing nm_firewall_manager_get().
Especially, since the firewall manager instance was nowhere passed in from
externally, it doesn't even sense for some vague testing purporse. Not to
mention, that there are no tests that actually inject a firewall manager stub.
Signed-off-by: Thomas Haller <thaller@redhat.com>
An activating device may have an IP config that is unrelated to
the current activation (for example if it comes from capturing
the existing config when NM is started), and that config might
not have a gateway, which would have NM ignore that the device
is activating until after DHCP.
https://bugzilla.gnome.org/show_bug.cgi?id=726400
[thaller@redhat.com: move variables inside if-block]
Signed-off-by: Thomas Haller <thaller@redhat.com>
Remove all remaining GParamSpec name and blurb strings (and fix
indentation while we're there), and add G_PARAM_STATIC_STRINGS to all
paramspecs that were lacking it.
On shutdown we can't defer the response to a callback, so we need to
use synchronous D-Bus calls. Second, sometimes we want to block on
the dispatcher response, like for pre-down.
NMIP[46]Route had a "source" field, but it was always set to KERNEL
for routes read from the kernel (even if they were originally added by
NM).
Fix things a bit by translating between our "source" field and the
kernel's "protocol" field.
https://bugzilla.gnome.org/show_bug.cgi?id=729203
Firewalld call addInterface() fails with ZONE_CONFLICT if the interface
is already part of another zone. This complicates the code in NM,
because we would have to keep better track of the zone in which the
interface currently is. Which might be quite difficult because
the zone might be changed from an external program (so we would have
to monitor the firewall configuration and work around potential races).
A better and simpler fix is to simply always use the changeZone() call.
This will do the right thing, regardless if the interface is already part
of a zone or not.
https://bugzilla.redhat.com/show_bug.cgi?id=1103782
Signed-off-by: Thomas Haller <thaller@redhat.com>
When starting firewall, NMPolicy would fail the following assertion:
NetworkManager[1462]: <debug> [1401708294.250829] [firewall-manager/nm-firewall-manager.c:218] name_owner_changed(): firewall started
(NetworkManager:1462): libnm-util-CRITICAL **: nm_connection_get_setting_connection: assertion 'NM_IS_CONNECTION (connection)' failed
#0 0x0000003370c504e9 in g_logv () from /lib64/libglib-2.0.so.0
#1 0x0000003370c5063f in g_log () from /lib64/libglib-2.0.so.0
#2 0x00007f306f960e11 in nm_connection_get_setting_connection (connection=0x0) at nm-connection.c:1441
#3 0x0000000000482319 in firewall_started (manager=<optimized out>, user_data=<optimized out>) at nm-policy.c:1881
#4 0x0000003371c104c7 in _g_closure_invoke_va () from /lib64/libgobject-2.0.so.0
#5 0x0000003371c29749 in g_signal_emit_valist () from /lib64/libgobject-2.0.so.0
#6 0x0000003371c2a3af in g_signal_emit () from /lib64/libgobject-2.0.so.0
#7 0x0000000000445d39 in name_owner_changed (dbus_mgr=<optimized out>, name=<optimized out>, old_owner=0x1452660 "", new_owner=0x1536720 ":1.175", user_data=<optimized out>) at firewall-manager/nm-firewall-manager.c:220
...
Signed-off-by: Thomas Haller <thaller@redhat.com>
Make Wi-Fi support a plugin using the new device factory interface.
Provides a 7% size reduction in the core NM binary.
Before After
NM: 1154104 1071992 (-7%)
Wi-Fi: 0 110464
(all results from stripped files)
Add a parameter to nm_device_add_pending_action() to silently
accept adding duplicate actions.
Same for nm_device_remove_pending_action(), to silently ignore
removing non-pending actions.
Signed-off-by: Thomas Haller <thaller@redhat.com>
We have to check the previous base device state in process_secondaries() when
making a state change. The device might got disconnected in the meantime and
thus the transition from DISCONNECTED to ACTIVATED or FAILED would have been
incorrect.
Logs showing the problem:
NetworkManager[2655]: <info> (eth0): disconnecting for new activation request.
NetworkManager[2655]: <info> (eth0): device state change: secondaries -> deactivating (reason 'none') [90 110 0]
NetworkManager[2655]: <info> (eth0): device state change: deactivating -> disconnected (reason 'none') [110 30 0]
NetworkManager[2655]: <info> (eth0): deactivating device (reason 'none') [0]
NetworkManager[2655]: <info> (eth0): canceled DHCP transaction, DHCP client pid 11409
NetworkManager[2655]: <info> NetworkManager state is now DISCONNECTED
NetworkManager[2655]: (devices/nm-device.c:6591):nm_device_state_changed: runtime check failed: (priv->in_state_changed == FALSE)
NetworkManager[2655]: <info> (eth0): device state change: disconnected -> failed (reason 'secondary-connection-failed') [30 120 54]
NetworkManager[2655]: <warn> Activation (eth0) failed for connection '<unknown>'
NetworkManager[2655]: <warn> (eth0): add_pending_action (4): 'queued state change to disconnected' already added
NetworkManager[2655]: file devices/nm-device.c: line 7682 (nm_device_add_pending_action): should not be reached
NetworkManager[2655]: <info> Activation (eth0) starting connection 'ethernet-12'
NetworkManager[2655]: <info> Activation (eth0) Stage 1 of 5 (Device Prepare) scheduled...
NetworkManager[2655]: <info> (eth0): device state change: failed -> disconnected (reason 'none') [120 30 0]
NetworkManager[2655]: <info> (eth0): deactivating device (reason 'none') [0]
NetworkManager[2655]: <warn> (eth0): remove_pending_action (2): 'queued state change to disconnected' never added
NetworkManager[2655]: file devices/nm-device.c: line 7733 (nm_device_remove_pending_action): should not be reached
NetworkManager[2655]: <info> VPN service 'openvpn' disappeared
https://bugzilla.redhat.com/show_bug.cgi?id=1055099https://bugzilla.redhat.com/show_bug.cgi?id=1055101
activate_data_free() deletes the data from priv->pending_activation_checks,
thus iterating over the list with g_slist_free_full() causes a double
free or invalid memory access.
This bug does not hit easily, because the policy only get's disposed
when NM shuts down and then there are likely no pending activations
queued.
Fixes regression introduced by commit 4f0c70e945.
Signed-off-by: Thomas Haller <thaller@redhat.com>
When activating a master, it reenables the auto activation of slave
connections for this master. Do not only match the device name, but also
check the connection UUID.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Add a generic signal that devices can use to indicate that something
material in the network situation changed, and that auto-activation
may now be possible. This reduces specific knowledge of device types
in the policy.
get_best_ip4_device() and get_best_ip6_device() iterate over
the list of devices to find the device with the default route.
The order of iteration is arbitrarly choosen.
Before, if two devices had the same priority, it would choose
the first one. Change it so that the device which currently has
the default route keeps it -- until it gets deactivated or a higher
priorty device gets connected.
Signed-off-by: Thomas Haller <thaller@redhat.com>
NMPolicy's auto_activate_device() was immediately removing the device
from priv->pending_activation_checks, which meant that if
nm_manager_activate_connection() had some side effect that would cause
schedule_activation_check() to be called again, another
auto-activation check could be queued while the first was still in
progress (causing a warning). Fix this by not removing the device from
the list until the activation attempt is complete.
This requires some additional minor changes to correctly handle the
possibility of remove_device() being triggered as a side effect of
nm_manager_activate_connection().
Also merge activate_data_new() into schedule_activation_check() so
that all the "start an auto-activation" code is in one place.
This change removed the "autoactivate" pending action too soon,
creating a window where the device had no pending actions, allowing
the manager to declare startup complete while devices were still being
activated.
This reverts commit a16b7a8253.
The previous version is not severely wrong, it is just be better
to treat connections whose retry block expires *now* as ready to
reconnect.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Two issues:
1) routes added by external programs or by users with /sbin/ip should not
be modified, but NetworkManager was always changing those routes' metrics
to match the device priority. This caused the nm_platform_ipX_route_sync()
functions to remove the original, external route (due to mismatched metric)
and re-add the route with the NetworkManager specified metric. Fix that
by not touching routes which came from the kernel.
2) Static routes (from persistent connections) that specified a metric were
getting their metric overwritten with the NetworkManager device priority.
Stop doing that.
Since the platform no longer defaults the metric to 1024, callers of
nm_platform_ip4_route_add() (like NMPolicy's default route handling)
must do that themselves, if they desire this behavior.
