Commit graph

991 commits

Author SHA1 Message Date
Thomas Haller
c2f8400e66 libnm: fix another leak when cleaning up NMClient
We now move the deletion of the context-busy-watcher to and idle handler
on the D-Bus GMainContext.

Note that the idle source does not take an additional reference on the
context. Hence, in certain cases it might happen that the context will
be completely unrefed before the idle handler runs. In that case, we
would leak the object.

Avoid that, by taking an additional reference to the GMainContext.

Note that the alternative would be to unref the context-busy-watcher
via the GSource's GDestroyNotify. That is not done, because then the
busy watcher might be unrefed in a different thread. Instead, we want
that to happen for the right context. The only minor downside of this
is that the user now always pays the price and must iterate the context
to fully clean up. But note that the user anyway must be prepared to
iterate the context after NMClient is gone. And that depends on some
unpredictable events that the user cannot control. That means, either
the user handles this correctly already, or the problem anyway exists
(randomly).

Of course all of the discussed "problems" are very specific. In practice, the
users uses the g_main_context_default() instance and anyway will either
keep iterating it or quit the process after the NMClient instance goes
away.
2020-01-16 14:43:29 +01:00
Thomas Haller
b572c0542a libnm: keep context-busy-watcher of NMClient alive for one more idle round
The context-busy-watch has two purposes:

1) it allows the user to watch whether the NMClient still has pending
  GSource'es attached to the GMainContext.
2) thereby, it also keeps the inner GMainContext integrated into the
  caller's (in case of synchronous initialization of NMClient).

Especially for 2), we must not get this wrong. Otherwise, we might
un-integrate the inner GMainContext too early and it will be leaked
indefinitely (because the user has no means to access or iterate it).

To be extra careful, extend the lifetime of the context-busy-watcher
for one more idle invocation. Theoretically, this should not be necessary,
but it's not clear whether something else is still pending.

The downside of that extra safety is that it is probably unnecessary in
practice. And in case where it is necessary, it hides an actual
issue, making it harder to notice and fix it.
2020-01-16 12:44:56 +01:00
Thomas Haller
a2fd2ab55d shared: remove nm_dbus_connection_signal_subscribe_object_manager() helper
It seems to complicate things more than helping. Drop it. What we still have
is a wrapper around plain g_dbus_connection_signal_subscribe(). That one is
trivial and helpful. The previous wrapper seems to add more complexity.
2020-01-16 12:42:41 +01:00
Thomas Haller
e280124757 libnm: avoid leaking GMainContext for sync initialization after context-busy-watcher quits
When passing a destroy notify to g_dbus_connection_signal_subscribe(),
that callback gets invoked as an idle handler of the associated
GMainContext. That caused to have yet another source attached to the
context after the NMClient gets destroyed.

Especially with synchronous initialization of NMClient that is bad,
because we may destroy the context-busy-watcher too early. That results
in removing the integration of the inner GMainContext into the caller's
context, and thus we leak the inner context indefinitely.

Avoid that leak by not passing a cleanup function to
g_dbus_connection_signal_subscribe().

Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')
2020-01-16 12:42:41 +01:00
Thomas Haller
05f66697e4 libnm: ignore "Groups" property of WifiP2PPeer
Groups currently are not exposed on D-Bus as separate objects.
Also, we might want to expose the property as "ao" instead of "as".
This API needs more thought.

There are likely no users that rely on this property. So, we will
drop it from server side, until it will be requested and newly designed.

Regardless, NMClient needs to gracefully ignore the property.
Despite we will remove it from 1.24 API, libnm should ignore the
property on previous versions. Mark it accordingly.
2020-01-15 13:53:57 +01:00
Thomas Haller
e978eca644 libnm: fix exposing nm_wifi_p2p_peer_get_name() from D-Bus property
Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')
2020-01-15 13:37:20 +01:00
Thomas Haller
51b39ceb33 libnm: fix wrong assertion in nm_client_add_and_activate_connection2_finish()
Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')
2020-01-15 12:32:02 +01:00
Benjamin Berg
e8cc69c37e libnm: fix interface and object path for P2P StartFind/StopFind calls
The change to use a GDBusConnection (7871105ff9) introduced a bug by
using the wrong hardcoded path and the wrong interface. Correct this
mistake again.

Fixes: 7871105ff9 ('libnm: implement nm_device_wifi_p2p_start_find()/nm_device_wifi_p2p_stop_find() by using GDBusConnection directly')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/399
2020-01-15 11:55:57 +01:00
Beniamino Galvani
667568d1b2 core,libnm: add VRF support
Add VRF support to the daemon. When the device we are activating is a
VRF or a VRF's slave, put routes in the table specified by the VRF
connection.

Also, introduce a VRF device type in libnm.
2020-01-14 09:51:56 +01:00
Beniamino Galvani
f4ced16791 libnm-core,cli: add VRF setting
Add new VRF setting and connection types to libnm-core and support
them in nmcli.
2020-01-14 09:49:01 +01:00
Thomas Haller
900af25263 client: add nm_client_get_object_by_path() and nm_object_get_client() API
When iterating the GMainContext of the NMClient instance, D-Bus events
get processed. That means, every time you iterate the context (or "return to
the main loop"), the content of the cache might change completely.

It makes sense to keep a reference to an NMObject instance, do something,
and afterwards check whether the instance can still be found in the cache.

Add an API for that. nm_object_get_client() allows to know whether the
object is still cached.

Likewise, while NMClient abstracts D-Bus, it should still provide a way
to look up an NMObject by D-Bus path. Add nm_client_get_object_by_path()
for that.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/384
2020-01-08 18:33:10 +01:00
Thomas Haller
72e025c33e libnm: move nm_client_get_capabilities() to separate linker version
nm_client_get_capabilities() was backported to 1.22.2. Add to to the
appropriate linker version.

Officially (and according to docs) nm_client_get_capabilities() still
appears first in libnm 1.24.0. However, as it got backported to 1.22.2,
it needs to be part of a different symbol version on 1.22. Instead
of adding the symbol twice (once for libnm_1_24_0 and libnm_1_22_2),
move it only to the libnm_1_22_2 symbol version, also on master.
2019-12-24 14:47:27 +01:00
Thomas Haller
21b008d0ff libnm: add nm_client_get_capabilities() to expose server Capabilities
I hesitated to add this to libnm, because it's hardly used.

However, we already fetch the property during GetManagedObjects(),
we we should make it accessible, instead of requiring the user to
make another D-Bus call.
2019-12-21 12:25:12 +01:00
Thomas Haller
453e56c6e9 libnm: handle boxed GType properties in nm_utils_g_param_spec_is_default() 2019-12-21 12:25:12 +01:00
Thomas Haller
3ae97cc543 libnm: emit property changed signal when setting NM_CLIENT_DBUS_CONNECTION 2019-12-18 17:13:27 +01:00
Thomas Haller
5f9bcc91c7 libnm/secret-agent: remove unused variable
Fixes: f0d3243f2b ('libnm/secret-agent: fix race registering secret agent')
2019-12-17 08:57:04 +01:00
Thomas Haller
1f9dabcb73 libnm/secret-agent: fix reseting timeout when registration completes
Fixes: f0d3243f2b ('libnm/secret-agent: fix race registering secret agent')
2019-12-16 18:53:23 +01:00
Thomas Haller
f0d3243f2b libnm/secret-agent: fix race registering secret agent
When NetworkManager starts, NMSecretAgentOld gets a name-owner changed
signal and registers right away.

Especially since commit ce0e898fb4 ('libnm: refactor caching of D-Bus
objects in NMClient') this hits a race where NetworkManager does not yet
export the org.freedesktop.NetworkManager.AgentManager interface and
the registration fails:

   GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such interface “org.freedesktop.NetworkManager.AgentManager” on object at path /org/freedesktop/NetworkManager/AgentManager

Previously, when NMClient recevied a name-owner changed, that would
block the main loop long enough to avoid the race. Note that NMClient
has nothing to do with NMSecretAgentOld, however in practice all
applications that use NMSecretAgentOld also use NMClient.

While we should fix the race server-side, we also need to work around it
in the client. Retry.

Also, make the async request actually cancellable and actually honor the passed
GCancellable.

Check output:

    $ LIBNM_CLIENT_DEBUG=trace ./clients/cli/nmcli agent secret |& grep secret-agent
    libnm-dbus: <trace> [21399.04862] secret-agent[2f2af4ee102d7570]: create new instance
    libnm-dbus: <trace> [21399.04863] secret-agent[2f2af4ee102d7570]: init-sync
    libnm-dbus: <trace> [21404.08147] secret-agent[2f2af4ee102d7570]: name owner changed: (null)
    libnm-dbus: <trace> [21404.09085] secret-agent[2f2af4ee102d7570]: name owner changed: ":1.2504"
    libnm-dbus: <trace> [21404.09085] secret-agent[2f2af4ee102d7570]: register: starting asynchronous registration...
    libnm-dbus: <trace> [21404.09178] secret-agent[2f2af4ee102d7570]: register: registration failed with error "GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such interface “org.freedesktop.NetworkManager.AgentManager” on object at path /org/freedesktop/NetworkManager/AgentManager". Retry in 0 msec...
    libnm-dbus: <trace> [21404.09178] secret-agent[2f2af4ee102d7570]: register: retry registration...
    libnm-dbus: <trace> [21404.09195] secret-agent[2f2af4ee102d7570]: register: registration failed with error "GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such interface “org.freedesktop.NetworkManager.AgentManager” on object at path /org/freedesktop/NetworkManager/AgentManager". Retry in 4 msec...
    libnm-dbus: <trace> [21404.09236] secret-agent[2f2af4ee102d7570]: register: retry registration...
    [...]
    libnm-dbus: <trace> [21405.01782] secret-agent[2f2af4ee102d7570]: register: registration failed with error "GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such interface “org.freedesktop.NetworkManager.AgentManager” on object at path /org/freedesktop/NetworkManager/AgentManager". Retry in 128 msec...
    libnm-dbus: <trace> [21405.03063] secret-agent[2f2af4ee102d7570]: register: retry registration...
    libnm-dbus: <trace> [21405.03068] secret-agent[2f2af4ee102d7570]: register: registration failed with error "GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such interface “org.freedesktop.NetworkManager.AgentManager” on object at path /org/freedesktop/NetworkManager/AgentManager". Retry in 128 msec...
    libnm-dbus: <trace> [21405.04354] secret-agent[2f2af4ee102d7570]: register: retry registration...
    libnm-dbus: <trace> [21406.01097] secret-agent[2f2af4ee102d7570]: register: registration succeeded
2019-12-16 18:38:49 +01:00
Thomas Haller
cff4e937ac libnm/secret-agent: use GTask for nm_secret_agent_old_register*()
This change is of course right and read nicer. Also, the GTask captures the
current g_main_context_get_thread_default(). We will need that next.
2019-12-16 18:38:49 +01:00
Thomas Haller
392befb5fd libnm/secret-agent: support debug logging from secret-agent 2019-12-16 18:38:49 +01:00
Thomas Haller
263aa63caa libnm/secret-agent: drop fallback to Register() method for secret-agent
RegisterWithCapabilities() is supported since NetworkManager 0.9.9.1. Of course,
we don't support such old server anymore (also, because we require the standard
D-Bus interfaces like ObjectManager).
2019-12-16 18:38:49 +01:00
Thomas Haller
1b0f0f8c47 libnm: allow using _LOGx() macros in libnm 2019-12-16 18:38:49 +01:00
Thomas Haller
bd9b253540 all: rename time related function to spell out nsec/usec/msec/sec
The abbreviations "ns" and "ms" seem not very clear to me. Spell them
out to nsec/msec. Also, in parts we already used the longer abbreviations,
so it wasn't consistent.
2019-12-13 16:54:40 +01:00
Thomas Haller
3843e0c87d shared: add "shared/nm-libnm-aux" static library
We have "shared/nm-libnm-core-aux", which is shared code that can be used
by anybody (including libnm-core, src, libnm and clients).

We have "clients/common", which are helper function for clients. But
that implies that the code is inside "clients". I think it would be
useful to have auxiliary code that extends libnm, but is not only
usable by code in "clients". In other words, "shared/nm-libnm-aux"
is a better place than "clients/common", and I think most of the
functionality form "clients/common" should move there.
2019-12-10 09:17:17 +01:00
Thomas Haller
a33ed5ad82 libnm: allow to enable/disable fetching of permissions in NMClient
Currently, NMClient by default always fetches the permissions
("GetPermissions()") and refreshes them on "CheckPermissions" signal.

Fetching permissions is relatively expensive, while they are not used
most of the time. Allow the user to opt out of this.

For that, have a NMClientInstanceFlags to enable/disable automatic
fetching. Also add a "permissions-state" property that allows the user
to understand whether the cached permissions are up to date or not.

This is a bit an awkward API for handling this. E.g. you cannot
explicitly request permissions, you can just enable/disable fetching
permissions. And then you can watch the permission-state to know whether
you are ready. It's done this way because it fits the previous model
and extends the API with a (relative) small amount of new functions and
properties.
2019-12-10 09:17:17 +01:00
Thomas Haller
f7aeda0390 libnm: add NMClient:instance-flags property
Add a flags property to control behavior of NMClient.
Possible future use cases:

 - currently it would always automatically fetch permissions. Often that
   is not used and the user could opt out of it.

 - currently, using sync init creates an internal GMainContext. This
   has an overhead and may be undesirable. We could implement another
   "sync" initialization that would merely iterate the callers mainloop
   until the initialization completes. A flag would allow to opt in.

 - currently, NMClient always fetches all connection settings
   automatically. Via a flag the user could opt out of that.
   Instead NMClient could provide an API so the user can request
   settings as they are needed.
2019-12-10 07:53:25 +01:00
Thomas Haller
51bc2c0224 libnm: track permissions in NMClient as an array of well known permissions
On D-Bus, the permission names are just the PolicyKit action names, like
"org.freedesktop.NetworkManager.wifi.scan". But NMClient already
ignores all strings that it doesn't know at compile time and only
keeps track of well known permission.

And neither does the API nm_client_get_permissions_result() allow to
expose permissions unknown to libnm.

Maybe the API of NMClient should be more generic and allow exposing
any permissions announced by NetworkManager. As it is however, it's
not necessary to track the permissions in a hash table. An array with
fixed indices is sufficient.
2019-12-10 07:53:25 +01:00
Thomas Haller
b7462b1910 libnm,shared: move nm_permission_result_to_client() to shared's nm_client_permission_result_from_string() 2019-12-10 07:53:25 +01:00
Thomas Haller
bfdd352a61 libnm,cli: cleanup mapping between NMClientPermission and strings 2019-12-10 07:53:25 +01:00
Thomas Haller
09e17888f7 libnm: add mapping functions between string and NMClientPermission enum 2019-12-10 07:53:25 +01:00
Thomas Haller
5ad095374f libnm: move NMClientPermission/NMClientPermissionResult enums from libnm to libnm-core
We will also use the enum from core.
2019-12-10 07:53:25 +01:00
Thomas Haller
53db3a2da9 libnm: don't emit property changed "notify" signal while destructing NMClient
It seems to trip up gnome-control-center (rh #1778668). Just don't emit
anymore signals once NMClient goes down.
2019-12-03 14:50:18 +01:00
Thomas Haller
61381b8ee4 libnm: add nm_ip_address_cmp_full() function
Not being able to compare two NMIPAddress instances is a major
limitation. Add nm_ip_address_cmp_full(). The choice here for adding
a "cmp()" function instead of a "equals()" function is that cmp is
more useful. We only want to add one of the two, so choose the
more powerful one. Yes, usually its also not the variant we want
or the variant that is convenient to use, such is life.

Compare this to:

  - nm_ip_route_equal_full(), which is an equal() method and not
    a cmp().

  - nm_ip_route_equal_full() which has a guint flags argument,
    instead of a typedef for an enum, with a proper generated
    GType.
2019-11-28 19:20:33 +01:00
Beniamino Galvani
1bde86396b libnm: add ipvx.dhcp-hostname-flags properties
When using the dhclient DHCP backend users can tweak the behavior in
the dhclient configuration file. One of the options that was reported
as useful in the past was the FQDN flags [1] [2].

Add native support for FQDN flags to NM by introducing new
ipv{4,6}.dhcp-hostname-flags properties.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1684595
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1255507
2019-11-28 17:56:35 +01:00
Thomas Haller
61807e9b6b libnm: add assertion for object returned by nm_device_get_active_connection()
I have a coredump that seems to indicate that nm_device_get_active_connection()
did not return a valid object. Let's add an assertion, trying to identify the
issue earlier. Aside from that, this change isn't useful, but an nm_assert()
shouldn't hurt anyway.
2019-11-28 12:47:07 +01:00
Thomas Haller
81bd50874b libnm: add nm_client_get_main_context() function
The NMClient is associated with a certain context. Add a getter
function to give the context.

The context is really not internal API of NMClient, that is because
the user must iterate this context and be aware of it.
2019-11-26 13:37:38 +01:00
Thomas Haller
812ad586dd libnm: fix assertion for cleaning up nml_dbus_property_o_notify()
Usually, the nmobj never gets reused for one dbobj. That means,
we really don't expect a nml_dbus_property_o_notify() for a property
that was already cleared.

However, that is for example not the case with NMClient itself. As NetworkManager
gets restarted, the name owner gets lost, the property cleared but afterwards
it might get notified again.

That means, nml_dbus_property_o_notify() and nml_dbus_property_o_clear() must
work well together, otherwise a sequence of

   nml_dbus_property_o_notify()
   nml_dbus_property_o_clear()
   nml_dbus_property_o_notify()

leads to an assertion failure "nm_assert (!pr_o->is_ready)".

Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')
2019-11-26 12:40:13 +01:00
Thomas Haller
2078acfddc libnm: fix leaking internal GMainContext for synchronously initialized NMClient
NMClient makes asynchronous D-Bus calls via g_dbus_connection_call().
This references the current GMainContext to later invoke the
asynchronous callback. Even when we cancel the asynchronous call,
the callback will still be invoked (later) to complete the request.

In particular this means when we destroy (unref) an NMClient, there
are quite possibly pending requests in the GMainContext. Although they
are cancelled, they keep the GMainContext alive.

With synchronous initialization, we have an internal GMainContext.
When we destroy the NMClient, we cannot just unhook the integrated
source, instead, we need to keep it integrated in the caller's main
context, as long as there are pending requests.

Add a mechanism to track those pending requests and fix the leak for the
internal GMainContext. Also expose the same mechanism to the user via a new
API called nm_client_get_context_busy_watcher(). This allows the user
to know when it can stop iterating the main context and when all
resources are reclaimed.

For example the following will lead to a crash:

    for i in range(1,2000):
        nmc = NM.Client.new(None)

This creates a number of NMClient instances and destroys them again.
Note that here the GMainContext is never iterated, because
synchronous initialization does not iterate the caller's context. So,
while we correctly unref and dispose the created NMClient instances,
there are pending requests left in the inner GMainContext. These pile
up and soon the program will crash because it runs out of file descriptors.

We can have a similar problem with asynchronous initialization, when
we create a new GMainContext per client, and don't iterate it after
we are done with the client.

Note that this patch does not avoid the problem in general. The problem
cannot be avoided, the user must iterate the main contex at some point.
Otherwise resources (memory and file descriptors) will be leaked.

Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/347
2019-11-26 10:02:58 +01:00
Thomas Haller
ce0e898fb4 libnm: refactor caching of D-Bus objects in NMClient
No longer use GDBusObjectMangaerClient and gdbus-codegen generated classes
for the NMClient cache. Instead, use GDBusConnection directly and a
custom implementation (NMLDBusObject) for caching D-Bus' ObjectManager
data.

CHANGES
-------

- This is a complete rework. I think the previous implementation was
difficult to understand. There were unfixed bugs and nobody understood
the code well enough to fix them. Maybe somebody out there understood the
code, but I certainly did not. At least nobody provided patches to fix those
issues. I do believe that this implementation is more straightforward and
easier to understand. It removes a lot of layers of code. Whether this claim
of simplicity is true, each reader must decide for himself/herself. Note
that it is still fairly complex.

- There was a lingering performance issue with large number of D-Bus
objects. The patch tries hard that the implementation scales well. Of
course, when we cache N objects that have N-to-M references to other,
we still are fundamentally O(N*M) for runtime and memory consumption (with
M being the number of references between objects). But each part should behave
efficiently and well.

- Play well with GMainContext. libnm code (NMClient) is generally not
thread safe. However, it should work to use multiple instances in
parallel, as long as each access to a NMClient is through the caller's
GMainContext. This follows glib's style and effectively allows to use NMClient
in a multi threaded scenario. This implies to stick to a main context
upon construction and ensure that callbacks are only invoked when
iterating that context. Also, NMClient itself shall never iterate the
caller's context. This also means, libnm must never use g_idle_add() or
g_timeout_add(), as those enqueue sources in the g_main_context_default()
context.

- Get ordering of messages right. All events are consistently enqueued
in a GMainContext and processed strictly in order. For example,
previously "nm-object.c" tried to combine signals and emit them on an
idle handler. That is wrong, signals must be emitted in the right order
and when they happen. Note that when using GInitable's synchronous initialization
to initialize the NMClient instance, NMClient internally still operates fully
asynchronously. In that case NMClient has an internal main context.

- NMClient takes over most of the functionality. When using D-Bus'
ObjectManager interface, one needs to handle basically the entire state
of the D-Bus interface. That cannot be separated well into distinct
parts, and even if you try, you just end up having closely related code
in different source files. Spreading related code does not make it
easier to understand, on the contrary. That means, NMClient is
inherently complex as it contains most of the logic. I think that is
not avoidable, but it's not as bad as it sounds.

- NMClient processes D-Bus messages and state changes in separate steps.
First NMClient unpacks the message (e.g. _dbus_handle_properties_changed()) and
keeps track of the changed data. Then we update the GObject instances
(_dbus_handle_obj_changed_dbus()) without emitting any signals yet. Finally,
we emit all signals and notifications that were collected
(_dbus_handle_changes_commit()). Note that for example during the initial
GetManagedObjects() reply, NMClient receive a large amount of state at once.
But we first apply all the changes to our GObject instances before
emitting any signals. The result is that signals are always emitted in a moment
when the cache is consistent. The unavoidable downside is that when you receive
a property changed signal, possibly many other properties changed
already and more signals are about to be emitted.

- NMDeviceWifi no longer modifies the content of the cache from client side
during poke_wireless_devices_with_rf_status(). The content of the cache
should be determined by D-Bus alone and follow what NetworkManager
service exposes. Local modifications should be avoided.

- This aims to bring no API/ABI change, though it does of course bring
various subtle changes in behavior. Those should be all for the better, but the
goal is not to break any existing clients. This does change internal
(albeit externally visible) API, like dropping NM_OBJECT_DBUS_OBJECT_MANAGER
property and NMObject no longer implementing GInitableIface and GAsyncInitableIface.

- Some uses of gdbus-codegen classes remain in NMVpnPluginOld, NMVpnServicePlugin
and NMSecretAgentOld. These are independent of NMClient/NMObject and
should be reworked separately.

- While we no longer use generated classes from gdbus-codegen, we don't
need more glue code than before. Also before we constructed NMPropertiesInfo and
a had large amount of code to propagate properties from NMDBus* to NMObject.
That got completely reworked, but did not fundamentally change. You still need
about the same effort to create the NMLDBusMetaIface. Not using
generated bindings did not make anything worse (which tells about the
usefulness of generated code, at least in the way it was used).

- NMLDBusMetaIface and other meta data is static and immutable. This
avoids copying them around. Also, macros like NML_DBUS_META_PROPERTY_INIT_U()
have compile time checks to ensure the property types matches. It's pretty hard
to misuse them because it won't compile.

- The meta data now explicitly encodes the expected D-Bus types and
makes sure never to accept wrong data. That would only matter when the
server (accidentally or intentionally) exposes unexpected types on
D-Bus. I don't think that was previously ensured in all cases.
For example, demarshal_generic() only cared about the GObject property
type, it didn't know the expected D-Bus type.

- Previously GDBusObjectManager would sometimes emit warnings (g_log()). Those
probably indicated real bugs. In any case, it prevented us from running CI
with G_DEBUG=fatal-warnings, because there would be just too many
unrelated crashes. Now we log debug messages that can be enabled with
"LIBNM_CLIENT_DEBUG=trace". Some of these messages can also be turned
into g_warning()/g_critical() by setting LIBNM_CLIENT_DEBUG=warning,error.
Together with G_DEBUG=fatal-warnings, this turns them into assertions.
Note that such "assertion failures" might also happen because of a server
bug (or change). Thus these are not common assertions that indicate a bug
in libnm and are thus not armed unless explicitly requested. In our CI we
should now always run with LIBNM_CLIENT_DEBUG=warning,error and
G_DEBUG=fatal-warnings and to catch bugs. Note that currently
NetworkManager has bugs in this regard, so enabling this will result in
assertion failures. That should be fixed first.

- Note that this changes the order in which we emit "notify:devices" and
"device-added" signals. I think it makes the most sense to emit first
"device-removed", then "notify:devices", and finally "device-added"
signals.
This changes behavior for commit 52ae28f6e5 ('libnm: queue
added/removed signals and suppress uninitialized notifications'),
but I don't think that users should actually rely on the order. Still,
the new order makes the most sense to me.

- In NetworkManager, profiles can be invisible to the user by setting
"connection.permissions". Such profiles would be hidden by NMClient's
nm_client_get_connections() and their "connection-added"/"connection-removed"
signals.
Note that NMActiveConnection's nm_active_connection_get_connection()
and NMDevice's nm_device_get_available_connections() still exposes such
hidden NMRemoteConnection instances. This behavior was preserved.

NUMBERS
-------

I compared 3 versions of libnm.

  [1] 962297f908, current tip of nm-1-20 branch
  [2] 4fad8c7c64, current master, immediate parent of this patch
  [3] this patch

All tests were done on Fedora 31, x86_64, gcc 9.2.1-1.fc31.
The libraries were build with

  $ ./contrib/fedora/rpm/build_clean.sh -g -w test -W debug

Note that RPM build already stripped the library.

---

N1) File size of libnm.so.0.1.0 in bytes. There currently seems to be a issue
  on Fedora 31 generating wrong ELF notes. Usually, libnm is smaller but
  in these tests it had large (and bogus) ELF notes. Anyway, the point
  is to show the relative sizes, so it doesn't matter).

  [1] 4075552 (102.7%)
  [2] 3969624 (100.0%)
  [3] 3705208 ( 93.3%)

---

N2) `size /usr/lib64/libnm.so.0.1.0`:

          text             data              bss                dec               hex   filename
  [1]  1314569 (102.0%)   69980 ( 94.8%)   10632 ( 80.4%)   1395181 (101.4%)   1549ed   /usr/lib64/libnm.so.0.1.0
  [2]  1288410 (100.0%)   73796 (100.0%)   13224 (100.0%)   1375430 (100.0%)   14fcc6   /usr/lib64/libnm.so.0.1.0
  [3]  1229066 ( 95.4%)   65248 ( 88.4%)   13400 (101.3%)   1307714 ( 95.1%)   13f442   /usr/lib64/libnm.so.0.1.0

---

N3) Performance test with test-client.py. With checkout of [2], run

```
prepare_checkout() {
    rm -rf /tmp/nm-test && \
    git checkout -B test 4fad8c7c64 && \
    git clean -fdx && \
    ./autogen.sh --prefix=/tmp/nm-test && \
    make -j 5 install && \
    make -j 5 check-local-clients-tests-test-client
}
prepare_test() {
    NM_TEST_REGENERATE=1 NM_TEST_CLIENT_BUILDDIR="/data/src/NetworkManager" NM_TEST_CLIENT_NMCLI_PATH=/usr/bin/nmcli python3 ./clients/tests/test-client.py -v
}
do_test() {
  for i in {1..10}; do
      NM_TEST_CLIENT_BUILDDIR="/data/src/NetworkManager" NM_TEST_CLIENT_NMCLI_PATH=/usr/bin/nmcli python3 ./clients/tests/test-client.py -v || return -1
  done
  echo "done!"
}
prepare_checkout
prepare_test
time do_test
```

  [1]  real 2m14.497s (101.3%)     user 5m26.651s (100.3%)     sys  1m40.453s (101.4%)
  [2]  real 2m12.800s (100.0%)     user 5m25.619s (100.0%)     sys  1m39.065s (100.0%)
  [3]  real 1m54.915s ( 86.5%)     user 4m18.585s ( 79.4%)     sys  1m32.066s ( 92.9%)

---

N4) Performance. Run NetworkManager from build [2] and setup a large number
of profiles (551 profiles and 515 devices, mostly unrealized). This
setup is already at the edge of what NetworkManager currently can
handle. Of course, that is a different issue. Here we just check how
long plain `nmcli` takes on the system.

```
do_cleanup() {
    for UUID in $(nmcli -g NAME,UUID connection show | sed -n 's/^xx-c-.*:\([^:]\+\)$/\1/p'); do
        nmcli connection delete uuid "$UUID"
    done
    for DEVICE in $(nmcli -g DEVICE device status | grep '^xx-i-'); do
        nmcli device delete "$DEVICE"
    done
}

do_setup() {
    do_cleanup
    for i in {1..30}; do
        nmcli connection add type bond autoconnect no con-name xx-c-bond-$i ifname xx-i-bond-$i ipv4.method disabled ipv6.method ignore
        for j in $(seq $i 30); do
            nmcli connection add type vlan autoconnect no con-name xx-c-vlan-$i-$j vlan.id $j ifname xx-i-vlan-$i-$j vlan.parent xx-i-bond-$i  ipv4.method disabled ipv6.method ignore
        done
    done
    systemctl restart NetworkManager.service
    sleep 5
}

do_test() {
    perf stat -r 50 -B nmcli 1>/dev/null
}

do_test
```

  [1]

   Performance counter stats for 'nmcli' (50 runs):

              456.33 msec task-clock:u              #    1.093 CPUs utilized            ( +-  0.44% )
                   0      context-switches:u        #    0.000 K/sec
                   0      cpu-migrations:u          #    0.000 K/sec
               5,900      page-faults:u             #    0.013 M/sec                    ( +-  0.02% )
       1,408,675,453      cycles:u                  #    3.087 GHz                      ( +-  0.48% )
       1,594,741,060      instructions:u            #    1.13  insn per cycle           ( +-  0.02% )
         368,744,018      branches:u                #  808.061 M/sec                    ( +-  0.02% )
           4,566,058      branch-misses:u           #    1.24% of all branches          ( +-  0.76% )

             0.41761 +- 0.00282 seconds time elapsed  ( +-  0.68% )

  [2]

   Performance counter stats for 'nmcli' (50 runs):

              477.99 msec task-clock:u              #    1.088 CPUs utilized            ( +-  0.36% )
                   0      context-switches:u        #    0.000 K/sec
                   0      cpu-migrations:u          #    0.000 K/sec
               5,948      page-faults:u             #    0.012 M/sec                    ( +-  0.03% )
       1,471,133,482      cycles:u                  #    3.078 GHz                      ( +-  0.36% )
       1,655,275,369      instructions:u            #    1.13  insn per cycle           ( +-  0.02% )
         382,595,152      branches:u                #  800.433 M/sec                    ( +-  0.02% )
           4,746,070      branch-misses:u           #    1.24% of all branches          ( +-  0.49% )

             0.43923 +- 0.00242 seconds time elapsed  ( +-  0.55% )

  [3]

   Performance counter stats for 'nmcli' (50 runs):

              352.36 msec task-clock:u              #    1.027 CPUs utilized            ( +-  0.32% )
                   0      context-switches:u        #    0.000 K/sec
                   0      cpu-migrations:u          #    0.000 K/sec
               4,790      page-faults:u             #    0.014 M/sec                    ( +-  0.26% )
       1,092,341,186      cycles:u                  #    3.100 GHz                      ( +-  0.26% )
       1,209,045,283      instructions:u            #    1.11  insn per cycle           ( +-  0.02% )
         281,708,462      branches:u                #  799.499 M/sec                    ( +-  0.01% )
           3,101,031      branch-misses:u           #    1.10% of all branches          ( +-  0.61% )

             0.34296 +- 0.00120 seconds time elapsed  ( +-  0.35% )

---

N5) same setup as N4), but run `PAGER= /bin/time -v nmcli`:

  [1]

        Command being timed: "nmcli"
        User time (seconds): 0.42
        System time (seconds): 0.04
        Percent of CPU this job got: 107%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.43
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 34456
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 6128
        Voluntary context switches: 1298
        Involuntary context switches: 1106
        Swaps: 0
        File system inputs: 0
        File system outputs: 0
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0

  [2]
        Command being timed: "nmcli"
        User time (seconds): 0.44
        System time (seconds): 0.04
        Percent of CPU this job got: 108%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.44
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 34452
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 6169
        Voluntary context switches: 1849
        Involuntary context switches: 142
        Swaps: 0
        File system inputs: 0
        File system outputs: 0
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0

  [3]

        Command being timed: "nmcli"
        User time (seconds): 0.32
        System time (seconds): 0.02
        Percent of CPU this job got: 102%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.34
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 29196
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 5059
        Voluntary context switches: 919
        Involuntary context switches: 685
        Swaps: 0
        File system inputs: 0
        File system outputs: 0
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0

---

N6) same setup as N4), but run `nmcli monitor` and look at `ps aux` for
  the RSS size.

      USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
  [1] me     1492900 21.0  0.2 461348 33248 pts/10   Sl+  15:02   0:00 nmcli monitor
  [2] me     1490721  5.0  0.2 461496 33548 pts/10   Sl+  15:00   0:00 nmcli monitor
  [3] me     1495801 16.5  0.1 459476 28692 pts/10   Sl+  15:04   0:00 nmcli monitor
2019-11-25 15:08:00 +01:00
Beniamino Galvani
e397582cca libnm: export interface flags
Add libnm support for the new InterfaceFlags property of NMDevice.
2019-11-22 10:18:26 +01:00
Beniamino Galvani
1b90ad41bb core: export interface flags of devices
Add a new read-only "InterfaceFlags" property to the Device interface
to export via D-Bus kernel flags and possibly other NM specific
flags. At the moment IFF_UP and IFF_LOWERUP are implemented.
2019-11-22 10:18:26 +01:00
Thomas Haller
6bf206eb81 libnm/tests: drop test_activate_failed() test
With this test the stub service simulates a failure to add-and-activate
the connection.

However the implementation of the stub service was not simulating the
real behavior of NetworkManager service. libnm will add the possibility
to assert against invalid server behavior by setting LIBNM_CLIENT_DEBUG=error.
With that change, libnm will complain that the stub service behaves
invalid, and rightly so.

Instead of fixing the test, just drop it.
2019-11-07 11:34:36 +01:00
Thomas Haller
feea4222ef libnm/tests: unsubscribe signal handler during test_activate_virtual()
libnm is gonna change, where it would still emit signals when the
instance gets destructed. In particular, when the device gets removed
from the NMClient cache, the references to other objects would be
cleared (and consequently property changed signals emitted).

This will cause a test failure, because the signal was not unsubscribed:

    test:ERROR:libnm/tests/test-nm-client.c:694:device_ac_changed_cb: assertion failed: (nm_device_get_active_connection (NM_DEVICE (device)) != NULL)
2019-11-07 11:34:36 +01:00
Thomas Haller
f21b8781ed tests: use nmtstc_client_new() to create NMClient instance and cleanup tests
The advantage of nmtstc_client_new() is that it randomly either uses the
synchronous or asynchronous constructor. Of course, both should behave
pretty much the same. Hence, this increases our test coverage.
2019-11-07 11:34:36 +01:00
Thomas Haller
abb68d90fc libnm: retire nm_client_wimax_*() functions
The server doesn's support WiMAX anymore. Hence there is no point in keeping
this functionality. While we cannot drop the functions, let them not do anything.

The code in NMManager is still there. But since we will soon drop
NMManager entirely, it doesn't matter.
2019-11-07 11:34:36 +01:00
Thomas Haller
2e91add7e4 libnm: move nm_permission_to_client()/nm_permission_result_to_client() to nm-libnm-utils.c
It's nicely trivial and independent. Move it to a separate place,
to avoid cluttering the more complicated code and to make it testable.

Also, use binary search to find the value by string.
2019-11-07 11:34:36 +01:00
Thomas Haller
83d7599acc libnm: deprecate nm_device_set_managed() and nm_device_set_autoconnect() API
These setters not only invoke a synchronous D-Bus call (ignoring the
return value). They also modify the content of the cache client-side,
bypassing the information that we receive via notifications from the
server.

Also, they don't emit property changed signals, but in any case they
are broken beyond repair.

Fully mark them as deprecated. Note that they were already marked as
_NM_DEPRECATED_SYNC_METHOD. However, that does not actually mark
the API as deprecated, because fully deprecating all synchronous
methods is premature at this point.
2019-11-07 11:34:36 +01:00
Thomas Haller
9c01d6ca67 libnm: print timestamp in LIBNM_CLIENT_DEBUG debug logging
It's useful, because it's easy to get overwhelemed by the logging output.
The timestamp makes it easier to keep track. Also, it allows to see how long
things take.
2019-11-07 11:34:36 +01:00
Thomas Haller
1463450393 all: use nm_dbus_path_not_empty() 2019-11-07 11:34:36 +01:00
Thomas Haller
e2dac63de6 libnm: fix handling "q" (uint16) property types in libnm
NMDeviceVxlan has some "q" type properties. They were not handled:

    $ G_MESSAGES_DEBUG=all PAGER= LIBNM_GLIB_DEBUG=properties-changed nmcli 2>&1 | grep "couldn't be set from D-Bus type"
    libnm-Message: 10:44:04.538: demarshal_generic: NMDeviceVxlan:dst-port (type guint) couldn't be set from D-Bus type q.
    libnm-Message: 10:44:04.538: demarshal_generic: NMDeviceVxlan:src-port-max (type guint) couldn't be set from D-Bus type q.
    libnm-Message: 10:44:04.538: demarshal_generic: NMDeviceVxlan:src-port-min (type guint) couldn't be set from D-Bus type q.
    libnm-Message: 10:44:04.539: demarshal_generic: NMDeviceWireGuard:listen-port (type guint) couldn't be set from D-Bus type q.
2019-11-06 17:30:25 +01:00