Commit graph

20257 commits

Author SHA1 Message Date
Beniamino Galvani
851d89dc6a platform: sort known IPv6 addresses by scope before a sync
The order we want to enforce is only among addresses with the same
scope, as the kernel always keeps addresses sorted by
scope. Therefore, apply the same sorting to known addresses, so that
we don't try to unnecessary change the order of addresses with
different scopes.

https://bugzilla.redhat.com/show_bug.cgi?id=1578668
2018-05-30 17:59:57 +02:00
Thomas Haller
54356ac8c7 shared: don't use nm_str_hash() in "nm-enum-utils.c"
This was only used for some extra assertions. It' is not essential.
If this would be for real usage, we should add a dependancy so that
nm-utils/nm-enum-utils.c requires nm-hash-utils.h. But as it is,
this is not necessary.

This fixes build for meson, which wrongly tries to build nm-enum-utils.c
for libnm-util, but then fails to include nm-hash-utils.c. That should
be fixed independently.

Fixes: 84a6eff106
2018-05-30 11:57:10 +02:00
Thomas Haller
84a6eff106 shared: don't allow aliases re-numbering in _nm_utils_enum_from_str_full()
For _nm_utils_enum_to_str_full(), we always first look whether we have
an alias/nick for the numeric value, and preferably use that. That makes a
lot of sense, as it allows the caller to provide better names (aliases),
which are preferred over the name from the GLib type. It renames the
numeric value.

For the reverse conversion, this makes less sense. A name should have a
unique numeric value. That is, we should not use one name that maps to
a different numeric value based on value_infos and GLib type. IOW, we
should not re-number names.

Add an assertion that we don't provide such a value_infos parameter,
that conflicts with names from GLib type.

Also, although the case where GLib type and value_infos disagree is now
forbidden by an assert, reorder the statements in _nm_utils_enum_from_str_full()
too. There is no difference in practice, but it mirros what we do in the
to-str case.
2018-05-29 16:28:09 +02:00
Thomas Haller
3021a8cf6b shared/trivial: rename local variable class
There is no real problem with using "class" in C. However, it would
be a keyword in C++. Just avoid it, and use "klass" instead.
2018-05-29 15:18:54 +02:00
Thomas Haller
7bde4bd492 libnm/tests: fix crash in tests
Fixes: daf4ba43da
2018-05-29 14:33:52 +02:00
Thomas Haller
3c6bd6769b shared: fix parsing aliases for flags in _nm_utils_enum_from_str_full()
Otherwise, the last alias overwrites previous values.

Fixes: b9fa0e0a19
2018-05-29 13:14:01 +02:00
Thomas Haller
daf4ba43da shared/tests: extend tests for nm_utils_enum_from_str() 2018-05-29 13:14:01 +02:00
Beniamino Galvani
33e87bd6ec n-acd: merge branch 'bg/n-acd-fixes-rh1578675'
https://bugzilla.redhat.com/show_bug.cgi?id=1578675
2018-05-29 11:21:53 +02:00
Beniamino Galvani
39b5691847 device: vlan: restart ARP announcement after we change MAC
After the parent MAC address changes and we update the VLAN MAC, also
restart ARP announcement to notify neighbors of the new address mapping.
2018-05-29 11:18:30 +02:00
Beniamino Galvani
b6b158e310 core: vlan: avoid unneeded casts 2018-05-29 11:18:30 +02:00
Beniamino Galvani
7f6a19b1ad n-acd: slightly improve logging
If timeout is 0 we don't really do a probe. Also, log the timeout.
2018-05-29 11:18:30 +02:00
Beniamino Galvani
d082af6b5c n-acd: better handle interfaces going temporarily down
NM sometimes brings an interface temporarily down (for example to
change a VLAN MAC to align it to the parent interface's one). When
this happens, any recv() or send() in n-acd fails, the n-acd instance
is reset to the initial state and a DOWN event is reported to the
manager, which currently does not handle it. The result is an
inconsistent state.

There is no simple way of dealing with the DOWN event in the
manager. What we can do instead is to:

 - ignore errors during recv() because there is really nothing we can
   do, except for waiting timeouts to expire;

 - during probe, ignore errors during send() so that we don't exceed
   the probe timeout;

 - during announcement, retry after a send() error to ensure we send
   all 3 announcements.

https://bugzilla.redhat.com/show_bug.cgi?id=1578675
2018-05-29 11:18:30 +02:00
Beniamino Galvani
2f4b3392d5 n-acd: use RFC 5227 timeout for announcements
When doing announcements, use the the timeout specified by RFC
5227. Note that timeout_multiplier might be 0.

This aligns behavior to upstream version of n-acd.
2018-05-29 11:18:30 +02:00
Aleksander Morgado
d97eab6c5a policy: don't block connection if device is gone
If the active connection is deactivated because the device is gone,
don't block autoconnection. Otherwise, whenever the device comes
back (e.g. maybe it was reset in the middle of a connection attempt),
the autoconnection logic won't be triggered, as the settings are still
blocked.

I'm able to reproduce this by performing a WWAN modem reset in the
middle of a connection attempt.

https://github.com/NetworkManager/NetworkManager/pull/121
2018-05-28 17:04:01 +02:00
Thomas Haller
baaab52266 clients/tests: run nmcli commands in parallel
Most nmcli calls from clients/tests don't change the server's state.
Hence, they can easily run in parallel.

Run tests in parallel. No longer handle one nmcli invocation after the other.
Instead, spawn groups of processes in parallel, and track the pending jobs.

Only at certain synchronization points we call self.async_wait() to
wait for all previous jobs to complete.

This reduces the test time on my machine from 7 to 3 seconds. Arguably,
that matters less during a full `make check -j 8`, because the entire
set of tests anyway takes longer than 7 seconds. So when running the
entire test suite, the machine is kept busy anyway. It matters however
for manual invocations.
2018-05-28 16:24:22 +02:00
Thomas Haller
f0cdd28db3 all: merge branch 'th/per-host-stable-id'
https://github.com/NetworkManager/NetworkManager/pull/117
2018-05-28 15:15:19 +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
Thomas Haller
d1a94a85b1 device: hash a per-host key for ipv4.dhcp-client-id=stable
Otherwise, the generated client-id depends purely on the profile's
stable-id. It means, the same profile (that is, either the same UUID
or same stable-id) on different hosts will result in identical client-ids.

That is clearly not desired. Hash a per-host secret-key as well.

Note, that we don't hash the interface name. So, activating the
profile on different interfaces, will still yield the same client-id.
But also note, that commonly a profile is restricted to one device,
via "connection.interface-name".

Note that this is a change in behavior. However, "ipv4.dhcp-client-id=stable"
was only added recently and not yet released.

Fixes: 62a7863979
2018-05-28 14:58:24 +02:00
Thomas Haller
dbcb1d6d97 core: let nm_utils_secret_key_read() handle failures internally
and add nm_utils_secret_key_get() to cache the secret-key, to only
obtain it once.

nm_utils_secret_key_read() is not expected to fail. However, in case
of an unexpected error, don't propagate the error to the caller,
but instead handle it internally.

That means, in case of error:
  - log a warning within nm_utils_secret_key_read() itself.
  - always return a generated secret-key. In case of error, the
    key won't be persisted (obviously). But the caller can ignore
    the error and just proceed with an in-memory key.

Hence, also add nm_utils_secret_key_get() to cache the key. This way,
we only try to actually generate/read the secret-key once.  Since that
might fail and return an in-memory key, we must for future invocations
return the same key, without generating a new one.
2018-05-28 14:58:24 +02:00
Thomas Haller
d0f1dc654e core: ensure NUL terminated secret_key buffer
The secret_key is binary random data, so one shouldn't ever use it as a
NUL terminated string directly.

Still, just ensure that the entire buffer is always NUL terminated.
2018-05-28 14:58:24 +02:00
Thomas Haller
5f5f75ce0e libnm: expose _bin2str() helper function as internal API 2018-05-28 14:58:24 +02:00
Thomas Haller
4e463df100 libnm: allow nm_utils_hwaddr_valid() of length zero
nm_utils_hwaddr_valid() is used for validating strings. It should
not assert against calling it with an empty string "". That is just
an invalid hwaddr.
2018-05-28 14:58:24 +02:00
Lubomir Rintel
c28b334fdd rpm/build.sh: suggest dnf builddep after BUILDTYPE=SRPM
Convenient for copy & paste.
2018-05-28 11:44:02 +02:00
Lubomir Rintel
de326a4e8a rpm: prefer python3
From Fedora 28 on we can build without Python 2. That is good,
because it's eventually going to be removed.

Based on a change in Fedora dist-git by Iryna Shcherbina.
2018-05-28 11:44:02 +02:00
Beniamino Galvani
1f7780cba9 cli: fix property matching
@ret was not initialized when there was only one partial match.

Also, refactor the code to return all matching values.

Fixes: 3fd9bf9d7d

https://github.com/NetworkManager/NetworkManager/pull/123
2018-05-28 11:33:40 +02:00
Beniamino Galvani
af94687161 tests: implement Update2() in NetworkManager stub 2018-05-28 11:21:07 +02:00
Thomas Haller
78e877ee3c clients/tests: refactor by moving replace-text helper function
Have less logic in TestNmcli.
2018-05-28 08:57:29 +02:00
Thomas Haller
ee85151a4a clients/tests: generate Makefile.am for expected files
The developer can re-generate .expected files with

 $ NM_TEST_REGENERATE=1 ./clients/tests/test-client.py

Note that these files are also dist-ed, so that the tests also work
from a source-tarball. For that, we need to add them to EXTRA_DIST.

Previously, this was done manually in the base Makefile.am file. This
was cumbersome, because when adding a new test, the developer would need
to manually add the files.

Now, let the test (with NM_TEST_REGENERATE=1) also generate a makefile
part.
2018-05-27 22:25:44 +02:00
Thomas Haller
c9b0d3a425 core/dhcp: merge branch 'th/dhcp-cleanup'
https://github.com/NetworkManager/NetworkManager/pull/122
2018-05-26 20:13:30 +02:00
Thomas Haller
6d41864c8b dhcp: refactor dhclient_start() to use cleanup attribute 2018-05-26 20:11:04 +02:00
Thomas Haller
5bd3f7bd67 dhcp: cache errno in nm_dhcp_client_stop_existing() before using it 2018-05-26 20:11:04 +02:00
Thomas Haller
181cf5c709 dhcp: use NMIPConfig type for argument of nm_dhcp_client_set_state() 2018-05-26 20:11:04 +02:00
Thomas Haller
ef8782127f dhcp: use cleanup attribute in nm_dhcp_client_handle_event()
and use NMIPConfig type.
2018-05-26 20:11:04 +02:00
Thomas Haller
97dc1ec1e4 dhcp: minor cleanup initializing name of leasefile for NMDhcpDhclint 2018-05-26 20:11:04 +02:00
Thomas Haller
1c5e790277 core: let NM_IS_IP_CONFIG() check for expected addr-family
Still unused...
2018-05-26 20:11:04 +02:00
Thomas Haller
f11bb3d93d shared: minor cleanup of nm_utils_get_start_time_for_pid() 2018-05-26 20:11:04 +02:00
Thomas Haller
fd9f1b7cdd dhcp: fix leaking error in dhclient_start()
Fixes: 1b1b4bd91c
2018-05-26 20:11:04 +02:00
Thomas Haller
e1bbc2e155 cli: avoid bogus line seprator in pretty-output
This bug resulted in spurious lines with "--pretty --mode tabular",
whenever nmc_print() was called with multiple rows.

Currently, the only case where this was visible was with:

  $ nmcli --pretty general permissions

(note that "--mode tabular" is the default).

Fixes: 16299e5ac0
2018-05-25 17:36:47 +02:00
Thomas Haller
5090c1f255 cli/tests: add test for output of nmcli general permissions 2018-05-25 17:24:57 +02:00
Thomas Haller
dbedce8674 ppp: merge branch 'th/ppp-stop'
https://github.com/NetworkManager/NetworkManager/pull/113
2018-05-25 12:41:21 +02:00
Thomas Haller
eaf36db68b core: add and use NM_SHUTDOWN_TIMEOUT_MS as duration that we plan for shutdown
nm_ppp_manager_stop() wants to ensure that the pppd process is really
gone. For that it uses nm_utils_kill_child_async() to first send
SIGTERM, and sending SIGKILL after a timeout.

Later, we want to fix shutdown of NetworkManager to iterate the mainloop
during shutdown, so that such operations are still handled. However, we
can only delay shutdown for a certain time. After a timeout (NM_SHUTDOWN_TIMEOUT_MS
plus NM_SHUTDOWN_TIMEOUT_MS_GRACE) we really have to give up and
terminate.

That means, the right amount of time between sending SIGTERM and SIGKILL
is exactly NM_SHUTDOWN_TIMEOUT_MS. Hopefully that is of course
sufficient in the first place. If not, send SIGKILL afterwards, and give
a bit more time (NM_SHUTDOWN_TIMEOUT_MS_GRACE) to reap the child.
And if all this time is still not enough, something is really odd and we
abort waiting, with a warning in the logfile.

Since we don't properly handle shutdown yet, the description above is
not really true. But with this patch, we fix it from point of view of
NMPPPManager.
2018-05-25 12:35:49 +02:00
Thomas Haller
43f67b4210 ppp-manager: rework stopping NMPPPManager by merging async/sync methods
Previously, there were two functions nm_ppp_manager_stop_sync() and
nm_ppp_manager_stop_async().

However, stop-sync() would still kill the process asynchronously (with a
2 seconds timeout before sending SIGKILL).

On the other hand, stop-async() did pretty much the same thing as
sync-code, except also using the GAsyncResult.

Merge the two functions. Stopping the instance for the most part can be
done entirely synchrnous. The only thing that is asynchronous, is
to wait for the process to terminate. For that, add a new callback
argument to nm_ppp_manager_stop(). This replaces the GAsyncResult
pattern.

Also, always ensure that NetworkManager runs the mainloop at least as
long until the process really terminated. Currently we don't get that
right, and during shutdown we just stop iterating the mainloop. However,
fix this from point of view of NMPPPManager and register a wait-object,
that later will correctly delay shutdown.

Also, NMDeviceWwan cared to wait (asynchronously) until pppd really
terminated. Keep that functionality. nm_ppp_manager_stop() returns
a handle that can be used to cancel the asynchrounous request and invoke
the callback right away. However note, that even when cancelling the
request, the wait-object that prevents shutdown of NetworkManager is
kept around, so that we can be sure to properly clean up.
2018-05-25 12:35:49 +02:00
Thomas Haller
53d04a1dfa ppp-manager/trivial: rename variables holding self pointers
We usually structure our code in a (pseudo) object oriented way.
It makes sense to call the variable for the target object "self",
it is more familiar and usually done.
2018-05-25 12:35:49 +02:00
Thomas Haller
515663519f ppp-manager: refactor killing pppd process by using _ppp_kill() function
- add callback arguments to _ppp_kill(). Although most callers don't
  care, it makes it more obvious that this kills the process
  asynchronously.

- the call to nm_utils_kill_child_async() is complicated, with many
  arguments. Only call it from one place, and re-use the simpler wrapper
  function _ppp_kill() everywhere.
2018-05-25 12:35:49 +02:00
Thomas Haller
f09e7797d4 core: add nm_shutdown_register_watchdog() for marking object to wait for shutdown
Eventually we should do a coordinated shutdown when NetworkManager exits.
That is a large work, ensuring that all asynchronous actions are cancellable
(in time), and that we wait for all pending operations to complete.

Add a function nm_shutdown_register_watchdog(), so that objects can register
themselves, to block shutdown until they are destroyed. It's not yet used,
because actually iterating the mainloop during shutdown can only be done,
once the code is prepared to be ready for that. But we already need the
function, so that we can refactor individual parts of the code, in preparation
of using it in the future.
2018-05-25 12:35:49 +02:00
Lubomir Rintel
3fd9bf9d7d clients: fix an error message in case of property ambiguity
Before:

  $ nmcli c modify Dukkha ipv4.ignore no
  Error: invalid property 'ignore': 'ignore' is ambiguous (ignore-auto-routes x (null)).

After:

  $ nmcli c modify Dukkha ipv4.ignore no
  Error: invalid property 'ignore': 'ignore' is ambiguous (ignore-auto-routes x ignore-auto-dns).
2018-05-24 18:39:11 +02:00
Thomas Haller
bb87c67f10 tests: merge branch 'th/cli-tests' 2018-05-24 16:40:27 +02:00
Thomas Haller
41dbf2b9d3 clients/tests: drop duplicate tests for German translation
call_nmcli_l() would test for 3 languages: 'C', 'de', and 'pl'. There
is no fundamental difference between 'de' and 'pl', so there is no need
to test for two languages.
2018-05-24 16:40:17 +02:00
Thomas Haller
2cae37ca40 clients/tests: test nmcli output for active-connection state DEACTIVATING 2018-05-24 16:40:17 +02:00
Thomas Haller
6f82c880a6 clients/tests: load libnm via GObject introspection
We will use it, and it must be available anyway, because the stub server
also requires it.
2018-05-24 16:40:17 +02:00