Commit graph

280 commits

Author SHA1 Message Date
Thomas Haller
2630ebd7b9 device: support reapplying route-table
Changing "ipv4.route-table" and "ipv6.route-table" was not allowed
during reapply.

The main difficulty for supporting that is changing the sync-mode.

With route-table 0, we don't sync all tables but only the main table.
So, when reapply changes from full-sync to no-full-sync, it's slightly
more complicated.

But it's probably not too complicated either. The change from
no-full-sync to full-sync is simple: we just start doing a full-sync.
The reverse change is slightly more complicated, because we need to
do one last full-sync, to get rid of routes that we configured on those
other tables.
2019-06-17 11:36:33 +02:00
Thomas Haller
c0e075c902 all: drop emacs file variables from source files
We no longer add these. If you use Emacs, configure it yourself.

Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.

No manual changes, just ran commands:

    F=($(git grep -l -e '-\*-'))
    sed '1 { /\/\* *-\*-  *[mM]ode.*\*\/$/d }'     -i "${F[@]}"
    sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"

Check remaining lines with:

    git grep -e '-\*-'

The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
2019-06-11 10:04:00 +02:00
Beniamino Galvani
c0d5b58332 core: don't realize unmanaged software devices
Currently, if user configuration or settings specify that a software
device is unmanaged, for example:

 [device-bond-unmanaged]
 match-device=interface-name:bond*
 managed=0

or

 [keyfile]
 unmanaged-devices=interface-name:bond*

and there is a connection for the device with autoconnect=yes, NM
creates the platform link and a realized device in unmanaged
state. Fix this, the device should not be realized if it is unmanaged.

https://bugzilla.redhat.com/show_bug.cgi?id=1679230
2019-04-12 10:34:20 +02:00
Thomas Haller
97da3149f7 device: merge stage3 and stage4 ip-config function for IPv4 and IPv6
(cherry picked from commit 5e71f01605)
2019-03-05 12:23:59 +01:00
Thomas Haller
c3751a25a1 device: add mechanism to invoke act_stage2_config() function also for external/assume case
Usually, for external/assume we skip calling act_stage2_config().

Add a flag that allows the device to indicate that it always wants
to be called. This is useful, if the device wants to do some initialization
also for external/assume cases.
2019-02-14 08:00:29 +01:00
Rafael Fontenelle
d81e10942f all: fix misspellings
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/64
2019-01-24 17:19:44 +01:00
Thomas Haller
f9077fa74d device: add nm_device_get_ip_iface_from_platform()
We have a cached nm_device_get_ip_iface() property. However, the interface
name is not an identifier for a link because it can change at any time.

Also, we already have the (ip) ifindex as proper identifier for the
platform link. We shouldn't use two redundant identifiers to refer to
a link.

Clearly, sometimes we need an ifname. For example for ethtool ioctl or
sysctl path names. For ethtool API, we resolve the actual name as late
as possible, and for sysctl API we prefer NMP_SYSCTL_PATHID_NETDIR*().
However, that is not always possible, for example for /proc/sys/net/ipv6/conf/
sysctls.

Add a function that resolves the ifname by looking into the cache. This
of course is still racy, but it minimizes the time.

Also, we should less and less rely on the ifname, and resolve it as late
as possible. This patch adds a small wrapper going into that direction.
2018-12-19 09:09:32 +01:00
Aleksander Morgado
90e9695af5 wwan: rework when settings/device are blocked for autoconnection
The reasons to block autoconnection at settings level are not the same
as the ones to block autoconnection at device level.

E.g. if the SIM-PIN is wrong, you may want to block autoconnection
both at settings level (as the PIN configured in settings is wrong)
and at device level (so that no other setting is tried automatically).

For some other reasons, you may want to block autoconnection only at
setting level (e.g. wrong APN).

And for some other reasons you may want to block autoconnection at
device level only (e.g. SIM missing), so that the autoconnection
blocking is removed when the device goes away. This is especially
important with SIM hotplug events processed by ModemManager, as a
device without SIM will be removed from MM when a new SIM is
inserted, so that a completely new object is exposed in MM with the
newly detected SIM.

https://github.com/NetworkManager/NetworkManager/pull/259
2018-12-14 14:25:36 +01:00
Thomas Haller
589063db3b core: use addr-family argument for nm_utils_get_ip_config_method()
Recently, more and more code was refactored to use an addr_family
integer to distinguish between IPv4 and IPv6.

Refactor nm_utils_get_ip_config_method() and nm_device_get_effective_ip_config_method()
to do that too. If we use different identifiers, we need to translate from one to
another and its inconsistent. Also, accessing a GType is an unnecessary function call,
instead of a plain constant.
2018-12-13 09:16:32 +01:00
Benjamin Berg
adbb9eb246 core: allow devices to modify the meaning of the AUTO IP config method
For P2P wifi we need to do DHCP if we are a peer or provide DHCP if we
are the group owner. This may only be decided while establishing the
connection, making the meaning of the AUTO method dynamic.

This adds a way for the device subclass to override the meaning of AUTO.

Patch cherry picked early from [1].

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/24
2018-12-13 09:14:27 +01:00
Thomas Haller
54ebe32f68 connectivity: consider default route for global connectivity state
When we agregate the connectivity state, only devices that
have the best default route should be considered.

Since we do connectivity checking per-device, the per-device check
does not care whether traffic to the internet is really routed via this
device.

But when talking about the global connectivity state, we care mostly
about the (best) default route. So, we should not allow a device with
worse or now default route, to contribute its connectivity state.

Fixes: 6b7e9f9b22
2018-12-11 09:23:47 +01:00
Thomas Haller
b635b4d419 core: improve and fix keeping connection active based on "connection.permissions"
By setting "connection.permissions", a profile is restricted to a
particular user.
That means for example, that another user cannot see, modify, delete,
activate or deactivate the profile. It also means, that the profile
will only autoconnect when the user is logged in (has a session).

Note that root is always able to activate the profile. Likewise, the
user is also allowed to manually activate the own profile, even if no
session currently exists (which can easily happen with `sudo`).

When the user logs out (the session goes away), we want do disconnect
the profile, however there are conflicting goals here:

1) if the profile was activate by root user, then logging out the user
   should not disconnect the profile. The patch fixes that by not
   binding the activation to the connection, if the activation is done
   by the root user.

2) if the profile was activated by the owner when it had no session,
   then it should stay alive until the user logs in (once) and logs
   out again. This is already handled by the previous commit.

   Yes, this point is odd. If you first do

      $ sudo -u $OTHER_USER nmcli connection up $PROFILE

   the profile activates despite not having a session. If you then

      $ ssh guest@localhost nmcli device

   you'll still see the profile active. However, the moment the SSH session
   ends, a session closes and the profile disconnects. It's unclear, how to
   solve that any better. I think, a user who cares about this, should not
   activate the profile without having a session in the first place.

There are quite some special cases, in particular with internal
activations. In those cases we need to decide whether to bind the
activation to the profile's visibility.

Also, expose the "bind" setting in the D-Bus API. Note, that in the future
this flag may be modified via D-Bus API. Like we may also add related API
that allows to tweak the lifetime of the activation.

Also, I think we broke handling of connection visiblity with 37e8c53eee
"core: Introduce helper class to track connection keep alive". This
should be fixed now too, with improved behavior.

Fixes: 37e8c53eee

https://bugzilla.redhat.com/show_bug.cgi?id=1530977
2018-12-09 14:47:32 +01:00
Thomas Haller
8f36019731 device: pass active-connection's state-change reason to nm_device_disconnect_active_connection()
No change in behavior, yet.
2018-12-09 14:47:31 +01:00
Thomas Haller
461bf7aa0c device: add state-change reason argument to nm_device_disconnect_active_connection()
nm_device_disconnect_active_connection() is generally useful and a prefered
form to fail an active connection. The device's state-change reason is important,
so it needs to be injected.
2018-12-09 14:47:31 +01:00
Lubomir Rintel
b385ad0159 all: say Wi-Fi instead of "wifi" or "WiFi"
Correct the spelling across the *entire* tree, including translations,
comments, etc. It's easier that way.

Even the places where it's not exposed to the user, such as tests, so
that we learn how is it spelled correctly.
2018-11-29 17:53:35 +01:00
Thomas Haller
af48af4671 device: return void pointer from nm_device_get_applied_setting()
Literally ever use of nm_device_get_applied_setting() requires a
cast. Just don't.
2018-10-23 10:47:01 +02:00
Thomas Haller
920346a5b9 device: add and use overrule-unmanaged flag for nm_device_check_connection_available()
This flag is more granular in whether to consider the connection
available or not. We probably should never check for the combined
flag NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST directly, but
always explicitly for the relevant parts.

Also, improve the error message, to indicate whether the device is
strictly unmanaged or whether it could be overruled.
2018-10-17 15:06:52 +02:00
Thomas Haller
e6523fbbbc core/trivial: add code comment for NMDeviceCheckDevAvailableFlags and NMDeviceCheckConAvailableFlags 2018-10-17 15:06:52 +02:00
Thomas Haller
9b935fad9b modem: don't use GAsyncResult pattern for disconnecting modem
We should not use GAsyncResult. At least, not for internal API.

It's more cumbersome then helpful, in my opinion. It requires
this awkward async_finish() pattern.

Instead, let the caller pass a suitable callback of the right type.
2018-10-17 13:03:50 +02:00
Lubomir Rintel
d8971fcbcd device: expose connectivity check result on a device
Separate properties for IPv4 and IPv6.
2018-09-24 15:36:19 +02:00
Lubomir Rintel
9664f284a1 connectivity: allow limiting the connectivity check to a specified AF
Nothing changes practically, as the NMDevice still starts this with
AF_UNSPEC. That is going to change in the following commit.

The ugly part:

priv->concheck_x[0] in few places. I believe we shouldn't be using union
aliasing here, and instead of indexing the v4/v6 arrays by a boolean it
should be an enum. I'm not fixing it here, but I eventually plan to if
this gets an ACK.
2018-09-24 15:17:02 +02:00
Thomas Haller
793afb7d95 core: improve logging why startup-complete is blocked
Before:

    "manager: check_if_startup_complete returns FALSE because of eth0"

Now:

    "manager: startup complete is waiting for device 'eth0' (autoactivate)"

Also, the logging line is now more a human readable sentence, but still
follows the same pattern as later

    "manager: startup complete"

Meaning: grepping for "startup complete" becomes more helpful because
one first finds the reasons why startup-complete is not yet reached,
followed by the moment when it is reached.
2018-09-19 17:51:01 +02:00
luz.paz
f985b6944a docs: misc. typos
Found via `codespell -q 3 --skip="*.po"`

https://github.com/NetworkManager/NetworkManager/pull/203
2018-09-15 09:08:03 +02:00
Thomas Haller
b8eb0e27b8 device: rename NM_UNMANAGED_LOOPBACK to NM_UNMANAGED_BY_TYPE
It is generally useful, not only for loopback. Rename.

(cherry picked from commit 045a36b33b)
2018-09-10 11:13:49 +02:00
Thomas Haller
38273a8871 settings: use delegation instead of inheritance for NMSettingsConnection and NMConnection
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).

NMSettingsConnection does a lot of things already:

  1) it "is-a" NMDBusObject and exports the API of a connection profile
     on D-Bus
  2) it interacts with NMSettings and contains functionality
     for tracking the profiles.
  3) it is the base-class of types like NMSKeyfileConnection and
     NMIfcfgConnection. These handle how the profile is persisted
     on disk.
  4) it implements NMConnection interface, to itself track the
     settings of the profile.

3) and 4) would be better implemented via delegation than inheritance.

Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.

Advantages:

  - by delegating, there is a clearer separation of what
    NMSettingsConnection does. For example, in C we often required
    casts from NMSettingsConnection to NMConnection. NMConnection
    is a very trivial object with very little logic. When we have
    a NMConnection instance at hand, it's good to know that it is
    *only* that simple instead of also being an entire
    NMSettingsConnection instance.

    The main purpose of this patch is to simplify the code by separating
    the NMConnection from the NMSettingsConnection. We should generally
    be aware whether we handle a NMSettingsConnection or a trivial
    NMConnection instance. Now, because NMSettingsConnection no longer
    "is-a" NMConnection, this distinction is apparent.

  - NMConnection is implemented as an interface and we create
    NMSimpleConnection instances whenever we need a real instance.
    In GLib, interfaces have a performance overhead, that we needlessly
    pay all the time. With this change, we no longer require
    NMConnection to be an interface. Thus, in the future we could compile
    a version of libnm-core for the daemon, where NMConnection is not an
    interface but a GObject implementation akin to NMSimpleConnection.

  - In the previous implementation, we cannot treat NMConnection immutable
    and copy-on-write.
    For example, when NMDevice needs a snapshot of the activated
    profile as applied-connection, all it can do is clone the entire
    NMSettingsConnection as a NMSimpleConnection.
    Likewise, when we get a NMConnection instance and want to keep
    a reference to it, we cannot do that, because we never know
    who also references and modifies the instance.
    By separating NMSettingsConnection we could in the future have
    NMConnection immutable and copy-on-write, to avoid all unnecessary
    clones.
2018-08-28 22:27:55 +02:00
Thomas Haller
72de0afa35 device: refactor setting parent in device's update_connection()
Add a helper function nm_device_parent_find_for_connection() to
unify implementations of setting the parent in update_connection().

There is some change in behavior, in particular for nm-device-vlan.c,
which no longer compares the link information from platform. But
update_connection() is anyway a questionable concept, only used
for external assumed connection (which itself, is questionable). Meaning,
update_connection() is a hack not science, and it's not at all clear
what the correct behavior is.

Also, note how vlan's implementation differs from all others. Why?
Should we always resort to also check the information from platform?
Either way, one of the two approaches should be used consistently and
nm_device_parent_find_for_connection() opts to not consult platform
cache.
2018-08-28 22:27:54 +02:00
Thomas Haller
33a88ca566 core: give better error reason why device is incompatible with profile
Note the special error codes  NM_UTILS_ERROR_CONNECTION_AVAILABLE_*.
This will be used to determine, whether the profile is fundamentally
incompatible with the device, or whether just some other properties
mismatch. That information will be importand during a plain `nmcli
connection up`, where NetworkManager searches all devices for a device
to activate. If no device is found (and multiple errors happened),
we want to show the error that is most likely relevant for the user.

Also note, how NMDevice's check_connection_compatible() uses the new
class field "device_class->connection_type_check_compatible" to simplify
checks for compatible profiles.

The error reason is still unused.
2018-07-24 09:39:09 +02:00
Thomas Haller
570e1fa75b core: give better error reason why device is unavailable
The error reason is still unused.
2018-07-24 09:39:09 +02:00
Thomas Haller
b9ae79c273 device/trivial: rename NMDeviceClass.connection_type to connection_type_supported
The term "connection_type" is overused. Give it a more distinct name.
2018-07-24 09:39:09 +02:00
Lubomir Rintel
8d65f636e1 devices/ovs: expose slaves on D-Bus for OVS bridges and ports 2018-07-10 13:12:02 +02:00
Beniamino Galvani
b445c59f2e device: rework mtu priority handling
If commit_mtu() is called multiple times and dev->get_configured_mtu()
returns @is_user_config=FALSE, only the first call changes the
MTU. So, for example, when the parent MTU of a VLAN changes, we apply
the new MTU only the first time.

Rework the handling of MTU in NMDevice, and store the source of the
configured MTU. When commit_mtu() is called again, we ask the subclass
a MTU to configure and apply it only if the source has higher
priority, or when the parent MTU changed.

(cherry picked from commit 2f8917237f)
2018-06-23 12:03:43 +02:00
Beniamino Galvani
ccecc6db59 device: introduce mtu source
Instead of returning a boolean @is_user_config value from
get_configured_mtu(), return an mtu-source enum with possible values
NONE,CONNECTION. This enum will be expanded later; for now there is no
change in behavior.

(cherry picked from commit 9f8b0697de)
2018-06-23 12:03:40 +02:00
Thomas Haller
9ab3d019e4 core: rework nm_device_steal_connection()
nm_device_steal_connection() was a bit misleading. It only had one caller,
and what _internal_activate_device() really wants it to deactivate all
other active-connections for the same connection. Hence, it already
performed a lookup for the active-connection that should be disconnected,
only to then lookup the device, and tell it to steal the connection.

Note, that if existing_ac happens to be neither the queued nor the currenct
active connection, then previously it would have done nothing. It's
unclear when that exactly can happen, however, we can avoid that
question entirely.

Instead of having steal-connection(), have a disconnect-active-connection().
If there is no matching device, it will just set the active-connection's
state to DISCONNECTED. Which in turn does nothing, if the state is
already DISCONNECTED.
2018-04-30 16:36:30 +02:00
Beniamino Galvani
1b5925ce88 all: remove consecutive empty lines
Normalize coding style by removing consecutive empty lines from C
sources and headers.

https://github.com/NetworkManager/NetworkManager/pull/108
2018-04-30 16:24:52 +02:00
Thomas Haller
9731b06b78 device: fix nm_device_get_type_description() for veth devices
Without this, nm_device_get_type_description() would quite likely
return "ethernet" for NMDeviceVeth types. This is wrong and was
broken recently.

Fixes: 0775602574
2018-04-24 21:00:17 +02:00
Thomas Haller
8bb058386d connectivity: add connectivity-changed signal to NMDevice
NMManager very much cares about changes to the connectivity state
of the device and was therefore listening to notify::connectivity
signals. However, property changed signals can be suppressed by
g_object_freeze_notify(). That is something we even encourage for
NMDBusObject instances, because the D-Bus glue makes use of the
property changed notifications, and encourages to combine multiple
changes by freezing the signal.

Using the property changed notifications of NMDBusObject instances is
ugly. Don't do that and instead add a special signal.
2018-04-11 11:31:39 +02:00
Thomas Haller
0a62a0e903 connectivity: schedule connectivity timers per-device and probe for short outages
It might happen, that connectivitiy is lost only for a moment and
returns soon after. Based on that assumption, when we loose connectivity
we want to have a probe interval where we check for returning
connectivity more frequently.

For that, we handle tracking of the timeouts per-device.

The intervall shall start with 1 seconds, and double the interval time until
the full interval is reached. Actually, due to the implementation, it's unlikely
that we already perform the second check 1 second later. That is because commonly
the first check returns before the one second timeout is reached and bumps the
interval to 2 seconds right away.

Also, we go through extra lengths so that manual connectivity check
delay the periodic checks. By being more smart about that, we can reduce
the number of connectivity checks, but still keeping the promise to
check at least within the requested interval.

The complexity of book keeping the timeouts is remarkable. But I think
it is worth the effort and we should try hard to

 - have a connectivity state as accurate as possible. Clearly,
   connectivity checking means that we probing, so being more intelligent
   about timeout and backoff timers can result in a better connectivity
   state. The connectivity state is important because we use it for
   the default-route penaly and the GUI indicates bad connectivity.

 - be intelligent about avoiding redundant connectivity checks. While
   we want to check often to get an accurate connectivity state, we
   also want to minimize the number of HTTP requests, in case the
   connectivity is established and suppossedly stable.

Also, perform connectivity checks in every state of the device.
Even if a device is disconnected, it still might have connectivity,
for example if the user externally adds an IP address on an unmanaged
device.

https://bugzilla.gnome.org/show_bug.cgi?id=792240
2018-04-10 15:11:23 +02:00
Thomas Haller
d8a31794c8 connectivity: rework async connectivity check requests
An asynchronous request should either be cancellable or not keep
the target object alive. Preferably both.

Otherwise, it is impossible to do a controlled shutdown when terminating
NetworkManager. Currently, when NetworkManager is about to terminate,
it just quits the mainloop and essentially leaks everything. That is a
bug. If we ever want to fix that, every asynchronous request must be
cancellable in a controlled way (or it must not prevent objects from
getting disposed, where disposing the object automatically cancels the
callback).

Rework the asynchronous request for connectivity check to

- return a handle that can be used to cancel the operation.
  Cancelling is optional. The caller may choose to ignore the handle
  because the asynchronous operation does not keep the target object
  alive. That means, it is still possible to shutdown, by everybody
  giving up their reference to the target object. In which case the
  callback will be invoked during dispose() of the target object.

- also, the callback will always be invoked exactly once, and never
  synchronously from within the asynchronous start call. But during
  cancel(), the callback is invoked synchronously from within cancel().
  Note that it's only allowed to cancel an action at most once, and
  never after the callback is invoked (also not from within the callback
  itself).

- also, NMConnectivity already supports a fake handler, in case
  connectivity check is disabled via configuration. Hence, reuse
  the same code paths also when compiling without --enable-concheck.
  That means, instead of having #if WITH_CONCHECK at various callers,
  move them into NMConnectivity. The downside is, that if you build
  without concheck, there is a small overhead compared to before. The
  upside is, we reuse the same code paths when compiling with or without
  concheck.

- also, the patch synchronizes the connecitivty states. For example,
  previously `nmcli networking connectivity check` would schedule
  requests in parallel, and return the accumulated result of the individual
  requests.
  However, the global connectivity state of the manager might have have
  been the same as the answer to the explicit connecitivity check,
  because while the answer for the manual check is waiting for all
  pending checks to complete, the global connectivity state could
  already change. That is just wrong. There are not multiple global
  connectivity states at the same time, there is just one. A manual
  connectivity check should have the meaning of ensure that the global
  state is up to date, but it still should return the global
  connectivity state -- not the answers for several connectivity checks
  issued in parallel.
  This is related to commit b799de281b
  (libnm: update property in the manager after connectivity check),
  which tries to address a similar problem client side.
  Similarly, each device has a connectivity state. While there might
  be several connectivity checks per device pending, whenever a check
  completes, it can update the per-device state (and return that device
  state as result), but the immediate answer of the individual check
  might not matter. This is especially the case, when a later request
  returns earlier and obsoletes all earlier requests. In that case,
  earlier requests return with the result of the currend devices
  connectivity state.

This patch cleans up the internal API and gives a better defined behavior
to the user (thus, the simple API which simplifies implementation for the
caller). However, the implementation of getting this API right and properly
handle cancel and destruction of the target object is more complicated and
complex. But this but is not just for the sake of a nicer API. This fixes
actual issues explained above.

Also, get rid of GAsyncResult to track information about the pending request.
Instead, allocate our own handle structure, which ends up to be nicer
because it's strongly typed and has exactly the properties that are
useful to track the request. Also, it gets rid of the awkward
_finish() API by passing the relevant arguments to the callback
directly.
2018-04-10 15:11:23 +02:00
Thomas Haller
4a705e1a0c core: track devices in manager via embedded CList
Instead of using a GSList for tracking the devices, use a CList.
I think a CList is in most cases the more suitable data structure
then GSList:

 - you can find out in O(1) whether the object is linked. That
   is nice, for example to assert in NMDevice's destructor that
   the object was unlinked, and we will use that later in
   nm_manager_get_device_by_path().
 - you can unlink the element in O(1) and you can unlink the
   element without having access to the link's head
 - Contrary to GSList, this does not require an extra slice
   allocation for the link node. It quite possibliy consumes
   slightly less memory because the CList structure is embedded
   in a struct that we already allocate. Even if slice allocation
   would be perfect to only consume 2*sizeof(gpointer) for the link
   note, it would at most be as-good as CList. Quite possibly,
   there is an overhead though.
 - CList possibly has better memory locality, because the link
   structure and the data are close to each other.

Something which could be seen as disavantage, is that with CList
one device can only be tracked in one NMManager instance at a time.
But that is fine. There exists only one NMManager instance for now,
and even if we would ever introduce multiple managers, we probably
would not associate one NMDevice instance with multiple managers.

The advantages are arguably not huge, but CList is IMHO clearly the
more suited data structure. No need to stick to a suboptimal data
structure for the job. Refactor it.
2018-03-27 09:49:43 +02:00
Thomas Haller
e17cd1d742 core: avoid clone of all-connections list for nm_utils_complete_generic()
NMSettings exposes a cached list of all connection. We don't need
to clone it. Note that this is not save against concurrent modification,
meaning, add/remove of connections in NMSettings will invalidate the
list.

However, it wasn't save against that previously either, because
altough we cloned the container (GSList), we didn't take an additional
reference to the elements.

This is purely a performance optimization, we don't need to clone the
list. Also, since the original list is of type "NMConnection *const*",
use that type insistently, instead of dependent API requiring GSList.

IMO, GSList is anyway not a very nice API for many use cases because
it requires an additional slice allocation for each element. It's
slower, and often less convenient to use.
2018-03-20 15:08:18 +01:00
Thomas Haller
297d4985ab core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API
Previously, we used the generated GDBusInterfaceSkeleton types and glued
them via the NMExportedObject base class to our NM types. We also used
GDBusObjectManagerServer.

Don't do that anymore. The resulting code was more complicated despite (or
because?) using generated classes. It was hard to understand, complex, had
ordering-issues, and had a runtime and memory overhead.

This patch refactors this entirely and uses the lower layer API GDBusConnection
directly. It replaces the generated code, GDBusInterfaceSkeleton, and
GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager
and static descriptor instances of type GDBusInterfaceInfo.

This adds a net plus of more then 1300 lines of hand written code. I claim
that this implementation is easier to understand. Note that previously we
also required extensive and complex glue code to bind our objects to the
generated skeleton objects. Instead, now glue our objects directly to
GDBusConnection. The result is more immediate and gets rid of layers of
code in between.
Now that the D-Bus glue us more under our control, we can address issus and
bottlenecks better, instead of adding code to bend the generated skeletons
to our needs.

Note that the current implementation now only supports one D-Bus connection.
That was effectively the case already, although there were places (and still are)
where the code pretends it could also support connections from a private socket.
We dropped private socket support mainly because it was unused, untested and
buggy, but also because GDBusObjectManagerServer could not export the same
objects on multiple connections. Now, it would be rather straight forward to
fix that and re-introduce ObjectManager on each private connection. But this
commit doesn't do that yet, and the new code intentionally supports only one
D-Bus connection.
Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start()
succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to
connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough
for the moment. It could be easily extended later, for example with polling whether the
system bus appears (like was done previously). Also, restart of D-Bus daemon isn't
supported either -- just like before.

Note how NMDBusManager now implements the ObjectManager D-Bus interface
directly.

Also, this fixes race issues in the server, by no longer delaying
PropertiesChanged signals. NMExportedObject would collect changed
properties and send the signal out in idle_emit_properties_changed()
on idle. This messes up the ordering of change events w.r.t. other
signals and events on the bus. Note that not only NMExportedObject
messed up the ordering. Also the generated code would hook into
notify() and process change events in and idle handle, exhibiting the
same ordering issue too.
No longer do that. PropertiesChanged signals will be sent right away
by hooking into dispatch_properties_changed(). This means, changing
a property in quick succession will no longer be combined and is
guaranteed to emit signals for each individual state. Quite possibly
we emit now more PropertiesChanged signals then before.
However, we are now able to group a set of changes by using standard
g_object_freeze_notify()/g_object_thaw_notify(). We probably should
make more use of that.

Also, now that our signals are all handled in the right order, we
might find places where we still emit them in the wrong order. But that
is then due to the order in which our GObjects emit signals, not due
to an ill behavior of the D-Bus glue. Possibly we need to identify
such ordering issues and fix them.

Numbers (for contrib/rpm --without debug on x86_64):

- the patch changes the code size of NetworkManager by
  - 2809360 bytes
  + 2537528 bytes (-9.7%)

- Runtime measurements are harder because there is a large variance
  during testing. In other words, the numbers are not reproducible.
  Currently, the implementation performs no caching of GVariants at all,
  but it would be rather simple to add it, if that turns out to be
  useful.
  Anyway, without strong claim, it seems that the new form tends to
  perform slightly better. That would be no surprise.

  $ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .;  done)
  - real    1m39.355s
  + real    1m37.432s

  $ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done)
  - real    0m26.843s
  + real    0m25.281s

- Regarding RSS size, just looking at the processes in similar
  conditions, doesn't give a large difference. On my system they
  consume about 19MB RSS. It seems that the new version has a
  slightly smaller RSS size.
  - 19356 RSS
  + 18660 RSS
2018-03-12 18:37:08 +01:00
Thomas Haller
c03a534963 core: implement setting MDNS setting for systemd
The connection.mdns setting is a per-connection setting,
so one might expect that one activated device can only have
one MDNS setting at a time.

However, with certain VPN plugins (those that don't have their
own IP interface, like libreswan), the VPN configuration is merged
into the configuration of the device. So, in this case, there
might be multiple settings for one device that must be merged.

We already have a mechanism for that. It's NMIP4Config. Let NMIP4Config
track this piece of information. Although, stricitly speaking this
is not tied to IPv4, the alternative would be to introduce a new
object to track such data, which would be a tremendous effort
and more complicated then this.

Luckily, NMDnsManager and NMDnsPlugin are already equipped to
handle multiple NMIPConfig instances per device (IPv4 vs. IPv6,
and Device vs. VPN).

Also make "connection.mdns" configurable via global defaults in
NetworkManager.conf.
2018-01-09 14:24:54 +01:00
Thomas Haller
0775602574 device: don't keep a clone of the type-description for each device instance
Instead, intern the string and cache it in the NMDeviceClass instance.
It anyway depends entirely on the GObject type (name), hence it should
also be cached at the type.
2017-12-27 09:52:40 +01:00
Thomas Haller
989b5fabaa device: expose nm_device_get_route_metric_default() 2017-12-15 11:36:07 +01:00
Beniamino Galvani
898b978e36 device: remove 'force_restart' argument from reactivate functions
It is now unused.

https://bugzilla.gnome.org/show_bug.cgi?id=790061
2017-12-06 09:53:18 +01:00
Beniamino Galvani
dcd7760eae device: make nm_device_state_reason_to_str() public 2017-11-13 20:17:13 +01:00
Thomas Haller
bdfdabea51 shared: propagate constness in _NM_GET_PRIVATE_PTR()
The _NM_GET_PRIVATE() macro already preserved and propagated
the constness of @self to the resulting private pointer.

_NM_GET_PRIVATE_PTR() didn't do that. Extend the macro,
to make that possible.
2017-11-13 11:35:44 +01:00
Thomas Haller
3c2b9485a7 device: improve tracking autoconnect-blocked flags of NMDevice
- split NM_DEVICE_AUTOCONNECT_BLOCKED_INTERN in two parts:
  "wrong-pin" and "manual-disconnect". Setting/unsetting them
  should be tracked differently, as their reason differs.

- no longer initialize/clear the autoconnect-blocked reasons
  during realize/unrealize of the device. Instead, initialize
  it once when the object gets created (nm_device_init()), and
  keep the settings beyond unrealize/realize cycles. This only
  matters for software devices, as regular devices get deleted
  after unrealizing once. But for software devices it is essential,
  because we don't want to forget the autoconnect settings of
  the device instance.

- drop verbose logging about blocking autoconnect due to failed
  pin. We already log changes to autoconnect-blocked flags with
  TRACE level. An additional message about this particular issue
  seems not necessary at INFO level.

- in NMManager's do_sleep_wake(), no longer block autoconnect
  for devices during sleep. We already unmanage the device, which
  is a far more effective measure to prevent activation. We should
  not also block autoconnect.
2017-11-08 11:45:34 +01:00
Thomas Haller
5279ab5be6 device: refactor autoconnect blocking by introducing NMDeviceAutoconnectBlockedFlags enum
The flags allow for more then two reasons. Currently the only reasons
for allowing or disallowing autoconnect are "user" and "intern".

It's a bit odd, that NMDeviceAutoconnectBlockedFlags has a negative
meaning. So
  nm_device_set_autoconnect_intern (device, FALSE);
gets replaced by
  nm_device_set_autoconnect_blocked_set (device, NM_DEVICE_AUTOCONNECT_BLOCKED_INTERN);
and so on.

However, it's chosen this way, because autoconnect shall be allowed,
unless any blocked-reason is set. That is, to check whether autoconnect
is allowed, we do
  if (!nm_device_get_autoconnect_blocked (device, NM_DEVICE_AUTOCONNECT_BLOCKED_ALL))
The alternative check would be
  if (nm_device_get_autoconnect_allowed (device, NM_DEVICE_AUTOCONNECT_ALLOWED_ALL) == NM_DEVICE_AUTOCONNECT_ALLOWED_ALL)
which seems odd too.

So, add the inverse flags to block autoconnect.

Beside refactoring and inverting the meaning of the autoconnect
settings, there is no change in behavior.
2017-11-08 11:45:34 +01:00
Thomas Haller
32acaccf2a device: move tracking auth_retry to NMDevice
It will be also used by NMDeviceWifi. It might waste a 4 bytes for device types
that don't require authentication. But it deduplicates code.
2017-11-02 11:41:01 +01:00