All users are supposed to include files from nm-utils by fully specifying
the path. -I.*shared/nm-utils is wrong.
Only, systemd code likes to include "siphash24.h" directly. Instead of
adding "-Ishared/nm-utils" to the search path, add an intermediary
header to sd-adapt. Note, that in the meantime we anyway should rework
siphash24 to use shared/c-siphash instead.
This also fixes build for meson, which was broken recently.
Use the path instead. This drop an useless use of the "name" property,
which is, coincidentally also wrong. (We use "ibft" in the plugin path
whereas the property is set to "iBFT".)
It's actually annoying, useless and wraps over even on wide displays.
Let's make it consistent with the log line we use for device plugins.
Also, this drops the last use of the "info" property and one useless use
of the "name" property.
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
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
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.
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
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.
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
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.
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.
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
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.
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.
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.
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.
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.
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
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.
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.