Watch for NMSettingConnection changes and creation signals and convert
them to IWD format and write them to the configured IWD profile storage
directory. The logic is off by default and gets enabled when the new
iwd-config-path setting in nm.conf's [main] group is set to a path to
an existing directory.
The idea here is that when a user edits an NM connection profile, the
change is immediately mirrored in IWD since IWD watches its
configuration directory using inotify. This way NM clients can be used
to edit 802.1x settings, the PSK passphrase or the SSID -- changes that
would previously not take effect with the IWD backend.
Some precautions are taken to not make connections owned by a user
available to other users, such connections are not converted at all.
In all other cases where a connection cannot be converted sufficiently
well to the IWD format, for various reasons, we also give up and not
mirror these connections.
Due to IWD limitations and design differences with NM this logic has
many problems where it may not do its task properly. It's meant to work
on a best-effort and "better than nothing" basis, but it should be safe
in that it shouldn't delete users data or reveal secrets, etc. The most
obvious limitation is that there can be multiple NM connections
referring to the same SSID+Security tuple and only one IWD profile can
exist because the filename is based on only the SSID+Security type. We
already had one NM connection selected for each IWD KnownNetwork and
referenced by a pointer, so we ignore changes in NM connections other
than that selected one.
Add code that can take an NMConnection and convert it to the IWD
network config file format so as to be able to mirror NM connection
profiles to IWD connection profiles and make basic editing IWD
profile possible from nm-connection-editor. The focus here is on 802.1x
settings.
Imaging you track a list of NMRefString instances. You could
directly expose them as strv array, but then you need a way
from the string back to the NMRefString instance.
That's easy to do. Add NM_REF_STRING_UPCAST() for that.
Previously, NMRefString was the public part of the struct, while
there was an internal RefString struct with private fields.
That might make sense if we would need to preserve some stable ABI, but
we don't because this is all internal (unstable) API. It also might
make sense to hide fields, but in practice that is not necessary
because the leading underscore is indicator enough that these are
private fields that are not supposed to be touched (unless you really
know what you do). So, drop RefString and move all fields in the public
NMRefString. The advantage is that we can later inline certain trivial
functions, that we otherwise couldn't.
Also, drop the "str" pointer and only use the "str" array field. The
pointer existed so that during nm_ref_string_new_len() we could create
a lookup needle with external str pointer. That is now solved
differently by using "len == G_MAXSIZE" as indicator that this is
a special lookup instance. The advantage is that we save one pointer
field per NMRefString, that we reduce the redundancy of the data, and
that we don't need the additional indirection.
NMRefString has only const fields itself, and all operations (except
ref/unref) don't mutate the instance. As such, the type is already
immutable, and using "const" is redundant and unnecessary.
Drop "const" from all API of NMRefString.
The script runs with "set -e", as such `cmd && r=ok` seems wrong.
It worked apparently, but I don't understand why. Anyway, change
it.
Fixes: e643703418 ('tests/client: run "test-client.py" also for meson')
See also commit 00e3fc036a ('clients/tests: ensure that we run nmcli
before client tests for LTO').
With the latest rework that code was dropped and tests (with LTO) are
broken as they hit a timeout (aside taking much longer).
Fixes: e643703418 ('tests/client: run "test-client.py" also for meson')
Originally, we would define G_LOG_DOMAIN via CFLAGS arguments.
Since commit 341b6e0704 ('all: change G_LOG_DOMAIN to "nm"') we would
instead set it in source and uniformly define it as "nm".
The reasons are that most parts of our source should not use g_log() directly,
and there is an aim to avoid special CFLAGS to simplify the build setup.
However, dispatcher indeed uses g_log() for logging, so the value there
is important.
Fix that, but this time by setting the define in source not via
CFLAGS.
Fixes: 341b6e0704 ('all: change G_LOG_DOMAIN to "nm"')
When using IWD-side autoconnect mode (current default), in .deactivate()
and .deactivate_async() refrain from commanding IWD to actually
disconnect until the device is managed. Likely the device is already
disconnected but in any case it's up to IWD to decide in this mode.
Calling IWD device's .Disconnect() D-Bus method has the side effect of
disabling autoconnect and doing this while NM is still in platform-init
was unexpectedly leaving the device without autoconnect after
platform-init was done, according to user reports.
Fixes: dc0e31fb70 ('iwd: Add the wifi.iwd.autoconnect setting')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/786
Use g_cancellable_connect(). That approach better handles the case where the
cancellable is already cancelled. Theoretically, we could extend that
approach further and make it thread-safe, but in the current form is
nm_utils_invoke_on_idle() not thread-safe.
While at it, also cleanup duplicate code during completion.
We now get unit test failures hitting this timeout. That is
likely a new bug introduced somewhere, but to rule out that
the timeout is simply too short, increase it.
Ignore a rd.znet argument without subchannels. When using net.ifnames
(the default), subchannels are used to build the interface name, which
is required to match the right connection.
With net.ifnames=0 the interface name is build using a prefix and a
global counter and therefore in theory it is possible to omit
subchannels. However, without subchannels there won't be a udev rule
that renames the interface and so it can't work.
https://bugzilla.redhat.com/show_bug.cgi?id=1931284https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/783
The code did:
key = g_strndup(tmp, val - tmp);
val[0] = '\0';
That is pointless. If we strndup the key, we don't need to truncate
the string at the '='. It might be nicer not to mutate the input string,
however, the entire code with "argument" parsing is about mutating the
input string, so that is something we apparently are fine with.
As such, don't clone the string anymore.
nm_utils_ptrarray_find_binary_search() had two additional output
arguments: the first and last index -- in case the sorted list contains
duplicates.
That's nice, and was used in the past. But now, those output arguments
are no longer used.
So drop them from nm_utils_ptrarray_find_binary_search().
Actually, we could now also drop the previous variant
nm_utils_ptrarray_find_binary_search_range(), as it's only used by unit
tests. However, although not rocket science, getting this right is not
entirely trivial, so lets keep the code in case we need it again.
Asserting against user input is not nice, because it always requires the
caller to check the value first. Don't do that.
Also, don't even check. You can set NM_SETTING_WIRED_S390_OPTIONS
property to any values (except duplicated keys). The C add function
should not be more limited than that. This is also right because
we have verify() which checks for valid settings. And it does so beyond
only checking the keys.
So you could set NM_SETTING_WIRED_S390_OPTIONS properties to invalid
keys. And you could use nm_setting_wired_add_s390_option() to set
invalid values. No need to let nm_setting_wired_add_s390_option() check
for valid keys.
- integers are unsigned. Mark the constants as such.
- assert that we don't overflow G_MAXUINT32. Note that
nm_setting_wired_get_s390_option()'s index argument
is of type guint32. So with that API you cannot track
more than G_MAXUINT32 elements.
- use nm_utils_strdup_reset(). It's less code, but it's
also self-assignment safe (contrary to the previous code).
A const global variable is stored in immutable memory.
You thus get a crash trying to modify it, which is desirable.
The user is really not supposed to modify this buffer,
even if nm_setting_wired_get_valid_s390_options() wrongly
returns a non-const pointer.
Note that "test-client.py" currently only tests nmcli. But what it does
is to spawn test-networkmanager-service.py and run nmcli against it.
As such, it could really be used to test any NetworkManager client
against the stub service. Hence this test is not under
"src/nmcli/tests", but under "src/tests/client/" where it is more
general.
