Commit graph

406 commits

Author SHA1 Message Date
Thomas Haller
aa7acb0ae3 build: avoid header conflict for <linux/if.h> and <net/if.h> with "nm-platform.h"
In the past, the headers "linux/if.h" and "net/if.h" were incompatible.
That means, we can either include one or the other, but not both.
This is fixed in the meantime, however the issue still exists when
building against older kernel/glibc.

That means, including one of these headers from a header file
is problematic. In particular if it's a header like "nm-platform.h",
which itself is dragged in by many other headers.

Avoid that by not including these headers from "platform.h", but instead
from the source files where needed (or possibly from less popular header
files).

Currently there is no problem. However, this allows an unknowing user to
include <net/if.h> at the same time with "nm-platform.h", which is easy
to get wrong.

(cherry picked from commit 37e47fbdab)
2018-11-14 14:39:10 +01:00
Thomas Haller
f70e762a4f all: add "${MAC}" substituion for "connection.stable-id"
We already had "${DEVICE}" which uses the interface name.
In times of predictable interface naming, that works well.
It allows the user to generate IDs per device which don't
change when the hardware is replaced.

"${MAC}" is similar, except that is uses the permanent MAC
address of the device. The substitution results in the empty
word, if the device has no permanent MAC address (like software
devices).

The per-device substitutions "${DEVICE}" and "${MAC}" are especially
interesting with "connection.multi-connect=multiple".

(cherry picked from commit 7ffbf71276)
2018-11-14 14:18:06 +01:00
Thomas Haller
3b8d882658 dhcp: reimplement node-specific DHCP client-id generation from systemd
Our internal DHCP client (from systemd) defaults to a particular client ID.
It is currently exposed as nm_sd_utils_generate_default_dhcp_client_id()
and is based on the systemd implementation.

One problem with that is, that it internally looks up the interface name
with if_indextoname() and reads /etc/machine-id. Both makes it harder
for testing.

Another problem is, that this way of generating the client-id is
currently limited to internal client. Why? If you use dhclient plugin,
you may still want to use the same algorithm. Also, there is no explict
"ipv4.dhcp-client-id" mode to select this client-id (so that it could
be used in combination with "dhclient" plugin).
As such, this code will be useful also aside systemd DHCP plugin.
Hence, the function should not be obviously tied to systemd code.

The implementation is simple enough, and since we already have a
unit-test, refactor the code to our own implementation.

(cherry picked from commit a55795772a)
2018-11-14 14:18:06 +01:00
Thomas Haller
13bf09fbd9 dhcp: test systemd's default DHCP client identifier generation
Internal DHCP client generates a default client ID. For one,
we should ensure that this algorithm does not change without
us noticing, for example, when upgrading systemd code. Add
a test, that the generation algorithm works as we expect.

Also note, that the generation algorithm uses siphash24().
That means, siphash24() implementation also must not change
in the future, to ensure the client ID doesn't change. As we
patch systemd sources to use shared/c-siphash, this is not
obviously the case. Luckily c-siphash and systemd's siphash24 do
agree, so all is good. The test is here to ensure that.

Also, previously the generation algorithm is not exposed as a
function, sd_dhcp_client will just generate a client-id when
it needs it. However, later we want to know (and set) the client
id before starting DHCP and not leave it unspecified to an
implementation detail.

This patch only adds a unit-test for the existing DHCP client
ID generation to have something for comparison. In the next
commit this will change further.

(cherry picked from commit 187d356198)
2018-11-14 14:18:06 +01:00
Thomas Haller
0c1ee8c68e core: don't persist secret-key for tests
Tests might access the secret-key.

For CI builds we may very well build NM as root and also run
unit tests. In such a situation it's bad to persist the secret
key. For example, the SELinux label may be wrong, and subsequently
starting NetworkManager may cause errors. Avoid persisting the secret
key for tests.

(cherry picked from commit 581e1c3269)
2018-11-14 14:18:05 +01:00
Thomas Haller
36ca7dd2c0 core: refactor loading machine-id and cache it
Previously, whenever we needed /etc/machine-id we would re-load it
from file. The are 3 downsides of that:

 - the smallest downside is the runtime overhead of repeatedly
   reading the file and parse it.

 - as we read it multiple times, it may change anytime. Most
   code in NetworkManager does not expect or handle a change of
   the machine-id.
   Generally, the admin should make sure that the machine-id is properly
   initialized before NetworkManager starts, and not change it. As such,
   a change of the machine-id should never happen in practice.
   But if it would change, we would get odd behaviors. Note for example
   how generate_duid_from_machine_id() already cached the generated DUID
   and only read it once.
   It's better to pick the machine-id once, and rely to use the same
   one for the remainder of the program.
   If the admin wants to change the machine-id, NetworkManager must be
   restarted as well (in case the admin cares).
   Also, as we now only load it once, it makes sense to log an error
   (once) when we fail to read the machine-id.

 - previously, loading the machine-id could fail each time. And we
   have to somehow handle that error. It seems, the best thing what we
   anyway can do, is to log an error once and continue with a fake
   machine-id. Here we add a fake machine-id based on the secret-key
   or the boot-id. Now obtaining a machine-id can no longer fail
   and error handling is no longer necessary.

Also, ensure that a machine-id of all zeros is not valid.

Technically, a machine-id is not an RFC 4122 UUID. But it's
the same size, so we also use NMUuid data structure for it.

While at it, also refactor caching of the boot-id and the secret
key. In particular, fix the thread-safety of the double-checked
locking implementations.

(cherry picked from commit 8308311264)
2018-11-14 14:18:02 +01:00
Thomas Haller
7494145649 core: add "nm-sd-utils.h" to access system internal helper
We have a fork of a lot of useful systemd helper code.
However, until now we shyed away from using it aside from
the bits that we really need.

That means, although we have some really nice implementations
in our source-tree, we didn't use them. Either we were missing
them, or we had to re-implement them.

Add "nm-sd-utils.h" header to very carefully make internal
systemd API accessible to the rest of core.

This is not intended as a vehicle to access all of internal
API. Instead, this must be used with care, and only a hand picked
selection of functions must be exposed. Use with caution, but where it
makes sense.

(cherry picked from commit eece5aff09)
2018-11-14 14:17:34 +01:00
Thomas Haller
9ac4bdb501 device: add "dhcp-plugin" match spec for device
The need for this is the following:

"ipv4.dhcp-client-id" can be specified via global connection defaults.
In absence of any configuration in NetworkManager, the default depends
on the DHCP client plugin. In case of "dhclient", the default further
depends on /etc/dhcp.

For "internal" plugin, we may very well want to change the default
client-id to "mac" by universally installing a configuration
snippet

    [connection-use-mac-client-id]
    ipv4.dhcp-client-id=mac

However, if we the user happens to enable "dhclient" plugin, this also
forces the client-id and overrules configuration from /etc/dhcp. The real
problem is, that dhclient can be configured via means outside of NetworkManager,
so our defaults shall not overwrite defaults from /etc/dhcp.

With the new device spec, we can avoid this issue:

    [connection-dhcp-client-id]
    match-device=except:dhcp-plugin:dhclient
    ipv4.dhcp-client-id=mac

This will be part of the solution for rh#1640494. Note that merely
dropping a configuration snippet is not yet enough. More fixes for
DHCP will follow. Also, bug rh#1640494 may have alternative solutions
as well. The nice part of this new feature is that it is generally
useful for configuring connection defaults and not specifically for
the client-id issue.

Note that this match spec is per-device, although the plugin is selected
globally. That makes some sense, because in the future we may or may not
configure the DHCP plugin per-device or per address family.

https://bugzilla.redhat.com/show_bug.cgi?id=1640494
(cherry picked from commit b9eb264efe)
2018-11-14 12:38:05 +01:00
Thomas Haller
dfce87b2b7 dns: avoid truncation of searches list due to 256 char limit in glibc
Before glibc 2.26, glibc's resolver would only honor 6 search entries
and a character limit of 256. This was lifted recently ([1], [2], [3]).

We also lift this limitation in NetworkManager ([4], [5]).

However, older glibc versions would just truncate the string at 255
characters. In particular, it would not only tuncate the list to 6
entries, but the entry which crosses the 256th character boundary would
be mangled. Avoid that, by adding spaces.

[1] https://sourceware.org/ml/libc-alpha/2017-08/msg00010.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=19569
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=21475
[4] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/47
[5] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/80

(cherry picked from commit 49c11a44e4)
2018-11-14 10:38:57 +01:00
Thomas Haller
b78a0ebcb1 dns/tests: add test for writing resolv.conf
(cherry picked from commit 60cd93612f)
2018-11-14 10:38:54 +01:00
Thomas Haller
c3808550eb shared: change nm_utils_strbuf_seek_end() handling truncated strings
Ok, I changed my mind.

The new behavior seems to make more sense to me. Not that it matters,
because we always use nm_utils_strbuf*() API with buffers that we expect
to be large enough to contain the result. And when truncation occurs,
we usually don't care much about it. That is, there is no code that
uses nm_utils_strbuf*() API and handles string truncation in particular.
2018-09-07 18:13:10 +02:00
Thomas Haller
0a8248af10 shared: add nm_utils_strbuf_seek_end() helper 2018-09-07 11:24:17 +02:00
Beniamino Galvani
5ef8284a01 core: add test for nm_wildcard_match_check()
https://github.com/NetworkManager/NetworkManager/pull/181
2018-09-05 15:12:39 +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
a75ab799e4 build: create "config-extra.h" header instead of passing directory variables via CFLAGS
1) the command line gets shorter. I frequently run `make V=1` to see
   the command line arguments for the compiler, and there is a lot
   of noise.

2) define each of these variables at one place. This makes it easy
   to verify that for all compilation units, a particular
   define has the same value. Previously that was not obvious or
   even not the case (see commit e5d1a71396
   and commit d63cf1ef2f).
   The point is to avoid redundancy.

3) not all compilation units need all defines. In fact, most modules
   would only need a few of these defines. We aimed to pass the necessary
   minium of defines to each compilation unit, but that was non-obvious
   to get right and often we set a define that wasn't used. See for example
   "src_settings_plugins_ibft_cppflags" which needlessly had "-DSYSCONFDIR".
   This question is now entirely avoided by just defining all variables in
   a header. We don't care to find the minimum, because every component
   gets anyway all defines from the header.

4) this also avoids the situation, where a module that previously did
   not use a particular define gets modified to require it. Previously,
   that would have required to identify the missing define, and add
   it to the CFLAGS of the complation unit. Since every compilation
   now includes "config-extra.h", all defines are available everywhere.

5) the fact that each define is now available in all compilation units
   could be perceived as a downside. But it isn't, because these defines
   should have a unique name and one specific value. Defining the same
   name with different values, or refer to the same value by different
   names is a bug, not a desirable feature. Since these defines should
   be unique accross the entire tree, there is no problem in providing
   them to every compilation unit.

6) the reason why we generate "config-extra.h" this way, instead of using
   AC_DEFINE() in configure.ac, is due to the particular handling of
   autoconf for directory variables. See [1].
   With meson, it would be trivial to put them into "config.h.meson".
   While that is not easy with autoconf, the "config-extra.h" workaround
   seems still preferable to me.

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Installation-Directory-Variables.html
2018-07-17 17:46:39 +02:00
Thomas Haller
e1c7a2b5d0 all: don't use gchar/gshort/gint/glong but C types
We commonly don't use the glib typedefs for char/short/int/long,
but their C types directly.

    $ git grep '\<g\(char\|short\|int\|long\|float\|double\)\>' | wc -l
    587
    $ git grep '\<\(char\|short\|int\|long\|float\|double\)\>' | wc -l
    21114

One could argue that using the glib typedefs is preferable in
public API (of our glib based libnm library) or where it clearly
is related to glib, like during

  g_object_set (obj, PROPERTY, (gint) value, NULL);

However, that argument does not seem strong, because in practice we don't
follow that argument today, and seldomly use the glib typedefs.
Also, the style guide for this would be hard to formalize, because
"using them where clearly related to a glib" is a very loose suggestion.

Also note that glib typedefs will always just be typedefs of the
underlying C types. There is no danger of glib changing the meaning
of these typedefs (because that would be a major API break of glib).

A simple style guide is instead: don't use these typedefs.

No manual actions, I only ran the bash script:

  FILES=($(git ls-files '*.[hc]'))
  sed -i \
      -e 's/\<g\(char\|short\|int\|long\|float\|double\)\>\( [^ ]\)/\1\2/g' \
      -e 's/\<g\(char\|short\|int\|long\|float\|double\)\>  /\1   /g' \
      -e 's/\<g\(char\|short\|int\|long\|float\|double\)\>/\1/g' \
      "${FILES[@]}"
2018-07-11 12:02:06 +02:00
Lubomir Rintel
e27b15c00d all: remove CLOCK_BOOTTIME defintions
It's useless and redundant noise.

The original motivation seems to have been compatibility with ancient
versions uClibc (2011), but given CLOCK_BOOTTIME definition is shipped with
kernel headers, the libc version shall not matter anyway.

Even if it was the case, uClibc has shipped the definition for over
7 years now and been superseded by uClibc-ng that always had the
definition.
2018-06-18 17:21:32 +02:00
Thomas Haller
b8b6100c78 all: replace systemd's siphash24 with c-siphash
Originally, we used "nm-utils/siphash24.c", which was copied
from systemd's source tree. It was both used by our own NetworkManager
code, and by our internal systemd fork.

Then, we added "shared/c-siphash" as a dependency for n-acd.

Now, drop systemd's implementation and use c-siphash also
for our internal purpose. Also, let systemd code use c-siphash,
by patching "src/systemd/src/basic/siphash24.h".
2018-05-31 15:59:38 +02:00
Thomas Haller
b7426e91db build: use default NM_BUILD_* defines for tests
Use two common defines NM_BUILD_SRCDIR and NM_BUILD_BUILDDIR
for specifying the location of srcdir and builddir.

Note that this is only relevant for tests, as they expect
a certain layout of the directories, to find files that concern
them.
2018-05-31 15:59:38 +02:00
Thomas Haller
eb821ead15 all: add stable-id specifier "${DEVICE}"
Add new stable-id specifier "${DEVICE}" to explicitly declare that the
connection's identity differs per-device.

Note that for settings like "ipv6.addr-gen-mode=stable" we already hash
the interface's name. So, in combination with addr-gen-mode, using this
specifier has no real use. But for example, we don't do that for
"ipv4.dhcp-client-id=stable".
Point being, in various context we possibly already include a per-device
token into the generation algorithm. But that is not the case for all
contexts and uses.

Especially the DHCPv4 client identifier is supposed to differ between interfaces
(according to RFC). We don't do that by default with "ipv4.dhcp-client-id=stable",
but with "${DEVICE}" can can now be configured by the user.
Note that the fact that the client-id is the same accross interfaces, is not a
common problem, because profiles are usually restricted to one device via
connection.interface-name.
2018-05-28 14:59:08 +02:00
Beniamino Galvani
82ebfa7351 core: reject invalid domains from ip configurations
Reject domains containing ".." or starting with "."
2018-05-14 15:22:50 +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
dcbb5c07e1 core: drop unused NMConnectionProvider typedef
We dopped NMConnectionProvider a while ago. Forgot something.

Fixes: 5337003c4c
2018-04-13 09:09:46 +02:00
Thomas Haller
b0bf9b2b9b core: explicitly pass D-Bus path to nm_utils_log_connection_diff()
No longer rely on nm_connection_get_path() being meaningful in server.
It also was wrong. During update, nm_settings_connection_update()
would call
  nm_utils_log_connection_diff (replace_connection, NM_CONNECTION (self), ...
where replace_connection has no path set, and nothing was logged.

Fix it, by explicitly passing the D-Bus path. Also, because
nm-core-utils.c should be independent of nm-dbus-object.h.
2018-04-13 09:09:46 +02:00
Beniamino Galvani
0136915211 build: meson: add prefix to test names
There are multiple tests with the same in different directories; add a
unique prefix to test names so that it is clear from the output which
one is running.
2018-04-12 09:21:10 +02:00
Beniamino Galvani
a2479b95c0 build: meson: use run-nm-test.sh to run tests
Like autotools, use the wrapper script 'run-nm-test.sh' that starts a
separate D-Bus session when needed.
2018-04-12 09:21:10 +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
d18d292b69 Revert "core: merge branch 'bg/restart-assume-rh1551958'"
This reverts commit cc1920d714, reversing
changes made to eb8257dea5.

This breaks restart, at least for Wi-Fi devices:

    #0  0x00007ffff5ee8771 in _g_log_abort (breakpoint=breakpoint@entry=1) at gmessages.c:554
    #1  0x00007ffff5ee9a5b in g_logv (log_domain=0x7ffff671a738 "GLib-GIO", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd720) at gmessages.c:1362
    #2  0x00007ffff5ee9baf in g_log (log_domain=log_domain@entry=0x7ffff671a738 "GLib-GIO", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7ffff5f347ea "%s: assertion '%s' failed") at gmessages.c:1403
    #3  0x00007ffff5eea0f9 in g_return_if_fail_warning (log_domain=log_domain@entry=0x7ffff671a738 "GLib-GIO", pretty_function=pretty_function@entry=0x7ffff673fc10 <__func__.25628> "g_dbus_proxy_call_internal", expression=expression@entry=0x7ffff673fb1c "G_IS_DBUS_PROXY (proxy)") at gmessages.c:2702
    #4  0x00007ffff66cdc5f in g_dbus_proxy_call_internal (proxy=0x0, method_name=method_name@entry=0x555555810510 "Scan", parameters=0x555555c7a530, flags=flags@entry=G_DBUS_CALL_FLAGS_NONE, timeout_msec=timeout_msec@entry=-1, fd_list=fd_list@entry=0x0, cancellable=0x0, callback=0x55555574cb96 <scan_request_cb>, user_data=0x555555ac2220) at gdbusproxy.c:2664
    #5  0x00007ffff66cf686 in g_dbus_proxy_call (proxy=<optimized out>, method_name=method_name@entry=0x555555810510 "Scan", parameters=<optimized out>, flags=flags@entry=G_DBUS_CALL_FLAGS_NONE, timeout_msec=timeout_msec@entry=-1, cancellable=cancellable@entry=0x0, callback=0x55555574cb96 <scan_request_cb>, user_data=0x555555ac2220) at gdbusproxy.c:2970
    #6  0x000055555574e026 in nm_supplicant_interface_request_scan (self=0x555555ac2220 [NMSupplicantInterface], ssids=ssids@entry=0x0) at src/supplicant/nm-supplicant-interface.c:1821
    #7  0x00007fffe1038276 in request_wireless_scan (self=self@entry=0x555555c6ee60 [NMDeviceWifi], periodic=periodic@entry=0, force_if_scanning=force_if_scanning@entry=0, ssids=<optimized out>, ssids@entry=0x0) at src/devices/wifi/nm-device-wifi.c:1347
    #8  0x00007fffe1039011 in device_state_changed (device=0x555555c6ee60 [NMDeviceWifi], new_state=NM_DEVICE_STATE_DISCONNECTED, old_state=<optimized out>, reason=<optimized out>)
        at src/devices/wifi/nm-device-wifi.c:2998
    #9  0x00007ffff432ed1e in ffi_call_unix64 () at ../src/x86/unix64.S:76
    #10 0x00007ffff432e68f in ffi_call (cif=cif@entry=0x7fffffffdc70, fn=fn@entry=0x7fffe1038e1e <device_state_changed>, rvalue=<optimized out>, avalue=avalue@entry=0x7fffffffdb60)
        at ../src/x86/ffi64.c:525
    #15 0x00007ffff63db66f in <emit signal ??? on instance 0x555555c6ee60 [NMDeviceWifi]> (instance=instance@entry=0x555555c6ee60, signal_id=<optimized out>, detail=detail@entry=0)
        at gsignal.c:3447
        #11 0x00007ffff63bff39 in g_cclosure_marshal_generic (closure=0x555555c22ea0, return_gvalue=0x0, n_param_values=<optimized out>, param_values=<optimized out>, invocation_hint=<optimized out>, marshal_data=<optimized out>) at gclosure.c:1490
        #12 0x00007ffff63bf73d in g_closure_invoke (closure=0x555555c22ea0, return_value=0x0, n_param_values=4, param_values=0x7fffffffdea0, invocation_hint=0x7fffffffde20) at gclosure.c:804
        #13 0x00007ffff63d1f30 in signal_emit_unlocked_R (node=node@entry=0x555555c22750, detail=detail@entry=0, instance=instance@entry=0x555555c6ee60, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffdea0) at gsignal.c:3673
        #14 0x00007ffff63dad05 in g_signal_emit_valist (instance=0x555555c6ee60, signal_id=<optimized out>, detail=0, var_args=var_args@entry=0x7fffffffe0b0) at gsignal.c:3391
    #16 0x00005555556f0f18 in _set_state_full (self=self@entry=0x555555c6ee60 [NMDeviceWifi], state=state@entry=NM_DEVICE_STATE_DISCONNECTED, reason=reason@entry=NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED, quitting=quitting@entry=0) at src/devices/nm-device.c:13268
    #17 0x00005555556f1774 in nm_device_state_changed (self=self@entry=0x555555c6ee60 [NMDeviceWifi], state=state@entry=NM_DEVICE_STATE_DISCONNECTED, reason=reason@entry=NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) at src/devices/nm-device.c:13435
    #18 0x00005555555bcf95 in recheck_assume_connection (self=self@entry=0x555555b09140 [NMManager], device=device@entry=0x555555c6ee60 [NMDeviceWifi]) at src/nm-manager.c:2297
    #19 0x00005555555bd53e in _device_realize_finish (self=self@entry=0x555555b09140 [NMManager], device=device@entry=0x555555c6ee60 [NMDeviceWifi], plink=plink@entry=0x555555ae43d8)
        at src/nm-manager.c:2473
    #20 0x00005555555c01d0 in platform_link_added (self=self@entry=0x555555b09140 [NMManager], ifindex=<optimized out>, plink=plink@entry=0x555555ae43d8, guess_assume=<optimized out>, dev_state=<optimized out>) at src/nm-manager.c:2789
    #21 0x00005555555c0cec in platform_query_devices (self=self@entry=0x555555b09140 [NMManager]) at src/nm-manager.c:2901
    #22 0x00005555555c439e in nm_manager_start (self=0x555555b09140 [NMManager], error=<optimized out>) at src/nm-manager.c:5632
    #23 0x000055555558498e in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:413
2018-04-04 14:49:04 +02:00
Beniamino Galvani
346064189a core: remove @indicated argument of nm_utils_match_connection()
It is not needed anymore.
2018-04-04 13:34:38 +02:00
Thomas Haller
cd48bc74b6 config: cleanup fields in NMGlobalDnsConfig
- consistently set options, searches, domains fields to %NULL,
  if there are no values.

- in nm_global_dns_config_update_checksum(), ensure that we uniquely
  hash values. E.g. a config with "searches[a], options=[b]" should
  hash differently from "searches=[ab], options=[]".

- in nm_global_dns_config_to_dbus(), reuse the sorted domain list.
  We already have it, and it guarantees a consistent ordering of
  fields.

- in global_dns_domain_from_dbus(), fix memleaks if D-Bus strdict
  contains duplicate entries.
2018-03-27 09:58:00 +02:00
Thomas Haller
868c3cedfd dhcp: remove unused nm_utils_resolve_conf_parse() function 2018-03-20 21:03:20 +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
a1f37964f0 core: rename "nm-bus-manager.h" to "nm-dbus-manager.h"
The next commit will completely rework NMBusManager and replace
NMExportedObject by a new type NMDBusObject.

Originally, NMDBusObject was added along NMExportedObject to ease
the rework and have compilable, intermediate stages of refactoring. Now,
I think the new name is better, because NMDBusObject is very strongly related
to the bus manager and the old name NMExportedObject didn't make that
clear.

I also slighly prefer the name NMDBusObject over NMBusObject, hence
for consistancy, also rename NMBusManager to NMDBusManager.

This commit only renames the file for a nicer diff in the next commit.
It does not actually update the type name in sources. That will be done
later.
2018-03-12 18:03:07 +01:00
Thomas Haller
3e9e51f1dd core: distinguish between IFA_F_SECONDARY and IFA_F_TEMPORARY
While the numerical values of IFA_F_SECONDARY and IFA_F_TEMPORARY
are identical, their meaning is not.

IFA_F_SECONDARY is only relevant for IPv4 addresses, while
IFA_F_TEMPORARY is only relevant for IPv6 addresses.

IFA_F_TEMPORARY is automatically set by kernel for the addresses
that it generates as part of IFA_F_MANAGETEMPADDR. It cannot be
actively set by user-space.

IFA_F_SECONDARY is automatically set by kernel depending on the order
in which the addresses for the same subnet are added.

This essentially reverts 8b4f11927 (core: avoid IFA_F_TEMPORARY alias for
IFA_F_SECONDARY).
2018-02-09 21:07:57 +01:00
Iñigo Martínez
5e16bcf268 meson: Improve dependency system
Some targets are missing dependencies on some generated sources in
the meson port. These makes the build to fail due to missing source
files on a highly parallelized build.

These dependencies have been resolved by taking advantage of meson's
internal dependencies which can be used to pass source files,
include directories, libraries and compiler flags.

One of such internal dependencies called `core_dep` was already in
use. However, in order to avoid any confusion with another new
internal dependency called `nm_core_dep`, which is used to include
directories and source files from the `libnm-core` directory, the
`core_dep` dependency has been renamed to `nm_dep`.

These changes have allowed minimizing the build details which are
inherited by using those dependencies. The parallelized build has
also been improved.
2018-01-10 12:20:17 +01:00
Thomas Haller
25ade39752 tests: use NMTST_EXPECT*() macros
Tests are commonly created via copy&paste. Hence, it's
better to express a certain concept explicitly via a function
or macro. This way, the implementation of the concept can be
adjusted at one place, without requiring to change all the callers.

Also, the macro is shorter, and brevity is better for tests
so it's easier to understand what the test does. Without being
bothered by noise from the redundant information.

Also, the macro knows better which message to expect. For example,
messages inside "src" are prepended by nm-logging.c with a level
and a timestamp. The expect macro is aware of that and tests for it

  #define NMTST_EXPECT_NM_ERROR(msg)      NMTST_EXPECT_NM (G_LOG_LEVEL_MESSAGE, "*<error> [*] "msg)

This again allows the caller to ignore this prefix, but still assert
more strictly.
2018-01-08 12:38:54 +01:00
Thomas Haller
22ef6a507a build: refine the NETWORKMANAGER_COMPILATION define
Note that:

 - we compile some source files multiple times. Most notably those
   under "shared/".

 - we include a default header "shared/nm-default.h" in every source
   file. This header is supposed to setup a common environment by defining
   and including parts that are commonly used. As we always include the
   same header, the header must behave differently depending
   one whether the compilation is for libnm-core, NetworkManager or
   libnm-glib. E.g. it must include <glib/gi18n.h> or <glib/gi18n-lib.h>
   depending on whether we compile a library or an application.

For that, the source files need the NETWORKMANAGER_COMPILATION #define
to behave accordingly.

Extend the define to be composed of flags. These flags are all named
NM_NETWORKMANAGER_COMPILATION_WITH_*, they indicate which part of the
build are available. E.g. when building libnm-core.la itself, then
WITH_LIBNM_CORE, WITH_LIBNM_CORE_INTERNAL, and WITH_LIBNM_CORE_PRIVATE
are available. When building NetworkManager, WITH_LIBNM_CORE_PRIVATE
is not available but the internal parts are still accessible. When
building nmcli, only WITH_LIBNM_CORE (the public part) is available.
This granularily controls the build.
2018-01-08 12:38:53 +01:00
Thomas Haller
31b6abd4b5 config: adjust logging message for duplicate config prefix
The logging macros already prepend a "config: " prefix. Don't
repeat that in the message, otherwise we get

    config: config: signal SIGHUP (no changes from disk)

Now:

    config: signal: SIGHUP (no changes from disk)
2018-01-08 12:18:52 +01:00
Lubomir Rintel
6672c5e92e all: get rid of a handful of unused-but-set variables 2017-12-18 13:29:32 +01:00
Iñigo Martínez
0735b35dd0 build: use template files for enum types' sources generation
Source files for enum types are generated by passing segments of the
source code of the files to the `glib-mkenums` command.

This patch removes those parameters where source code is used from
meson build files by moving those segmeents to template files.

https://mail.gnome.org/archives/networkmanager-list/2017-December/msg00057.html
2017-12-18 11:25:06 +01:00
Thomas Haller
bb7fcdf21e src/tests: fix test_nm_utils_kill_child() under meson
meson spawns the tests in a way that the test process is
a session leader. Since the test wants to create a new
process group to kill a group of processes that it starts,
it failed.

  $ meson test -C build test-general-with-expect

The test would have succeed when wrapping the test for example
by strace:

  $ meson test -C build --wrap='strace' test-general-with-expect

Fix that, by forking once more.
2017-12-15 09:30:20 +01:00
Thomas Haller
c04f4febfe src/tests: split test code in test_nm_utils_kill_child()
Move the actual tests to a separate function. The remaingin parts currently
setup and kill a process group, but that needs adjustment.
2017-12-15 09:11:22 +01:00
Iñigo Martínez
d849366230 build: rename unit tests with the test- pattern
There are some tests located in different directories which are
using the same name. To avoid any confussion a prefix was used to
name the test and the target.

This patch uses the prefix just for the target, to avoid any
collision that may happen, and uses the `test-` pattern as the
name.

https://mail.gnome.org/archives/networkmanager-list/2017-December/msg00051.html
2017-12-14 20:07:38 +01:00
Iñigo Martínez
03637ad8b5 build: add initial support for meson build system
meson is a build system focused on speed an ease of use, which
helps speeding up the software development. This patch adds meson
support along autotools.

[thaller@redhat.com: rebased patch and adjusted for iwd support]

https://mail.gnome.org/archives/networkmanager-list/2017-December/msg00022.html
2017-12-13 15:48:50 +01:00
Beniamino Galvani
4cbf594779 core: add nm_ipX_config_clone() 2017-12-06 09:53:18 +01:00
Thomas Haller
cc74cffe12 device: add "indicated" argument to nm_utils_match_connection()
The matching works fuzzy and is not reliable. That is why we store
which connection should be assumed after restart in the state file
of NetworkManager.

In that case, we don't need to do a full check (with the possibility
of a false-reject). Just check for the minimum required properties:
the type and slave-type.

Yes, if the user modifies the connection while restarting NM, then
we might wrongly assume a connection that no longer would match.
But NM should not read minds, it should do as indicated.
2017-11-30 14:47:49 +01:00
Thomas Haller
93adadbdcb all: use nm_direct_hash() instead of g_direct_hash()
We also do this for libnm, where it causes visible changes
in behavior. But if somebody would rely on the hashing implementation
for hash tables, it would be seriously flawed.
2017-11-16 11:49:52 +01:00
Thomas Haller
5b29c2e5b9 all: use nm_close() instead of close() 2017-11-14 15:10:42 +01:00
Thomas Haller
d5c9c95e96 core: use NM_CONSTCAST() for NM_IP_CONFIG_CAST() 2017-11-13 11:35:44 +01:00
Beniamino Galvani
a33baf8bf7 core: fix build without connectivity check
Fixes: 4dd30b784c

https://bugzilla.gnome.org/show_bug.cgi?id=790222
2017-11-12 10:33:32 +01:00