I got a report of a scenario where multiple servers reply to a REQUEST
in SELECTING, and all servers send NAKs except the one which sent the
offer, which replies with a ACK. In that scenario, n-dhcp4 is not able
to obtain a lease because it restarts from INIT as soon as the first
NAK is received. For comparison, dhclient can get a lease because it
ignores all NAKs in SELECTING.
Arguably, the network is misconfigured there, but it would be great if
n-dhcp4 could still work in such scenario.
According to RFC 2131, ACK and NAK messages from server must contain a
server-id option. The RFC doesn't explicitly say that the client
should check the option, but I think it's a reasonable thing to do, at
least for NAKs.
This patch stores the server-id of the REQUEST in SELECTING, and
compares it with the server-id from NAKs, to discard other servers'
replies.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1144
When a NMDevice is involved in a PPPoE activation, it means that the
connection has connection.interface-name=<ethernet-interface>. In such
case, the ppp ifindex should be set as ip-ifindex of the ethernet
device.
Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
This is needed to get "/usr/share/gettext/its/polkit.its",
otherwise msgfmt will fail on Debian:
/usr/bin/msgfmt: cannot locate ITS rules for data/org.freedesktop.NetworkManager.policy.in
Clang 14 has a new warning "-Wbitwise-instead-of-logical", and it warns
about our usage with NM_IN_SET_SE()/NM_IN_STRSET_SE(). It complains that we
are using '|' with boolean operands. Which is true (and intended), as we bitwise-or
the result of the '==' comparisons.
Work around the warning by casting the operands to "int". Note that
in C, the comparison operators have already a type "int", so this cast
should not result in any changes in the compiled code.
../src/libnm-core-impl/tests/test-general.c:9415:17: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
_ASSERT(2, !NM_IN_SET_SE(-1, G(1), G(2)));
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/libnm-std-aux/nm-std-aux.h:800:30: note: expanded from macro 'NM_IN_SET_SE'
#define NM_IN_SET_SE(x, ...) _NM_IN_SET(|, typeof(x), x, __VA_ARGS__)
^
../src/libnm-std-aux/nm-std-aux.h:789:39: note: expanded from macro '_NM_IN_SET'
!!(NM_VA_ARGS_FOREACH(, , op, _NM_IN_SET_OP, __VA_ARGS__)); \
^
../src/libnm-std-aux/nm-std-aux.h:772:20: note: expanded from macro 'NM_VA_ARGS_FOREACH'
op, \
^
note: (skipping 7 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
../src/libnm-glib-aux/nm-macros-internal.h:1603:47: note: expanded from macro '_G_BOOLEAN_EXPR'
#define _G_BOOLEAN_EXPR(expr) NM_BOOLEAN_EXPR(expr)
~~~~~~~~~~~~~~~~^~~~~
../src/libnm-std-aux/nm-std-aux.h:167:62: note: expanded from macro 'NM_BOOLEAN_EXPR'
#define NM_BOOLEAN_EXPR(expr) _NM_BOOLEAN_EXPR_IMPL(NM_UNIQ, expr)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
../src/libnm-std-aux/nm-std-aux.h:161:13: note: expanded from macro '_NM_BOOLEAN_EXPR_IMPL'
if (expr) \
^~~~
../src/libnm-core-impl/tests/test-general.c:9415:17: note: cast one or both operands to int to silence this warning
../src/libnm-std-aux/nm-std-aux.h:800:30: note: expanded from macro 'NM_IN_SET_SE'
#define NM_IN_SET_SE(x, ...) _NM_IN_SET(|, typeof(x), x, __VA_ARGS__)
^
../src/libnm-std-aux/nm-std-aux.h:789:39: note: expanded from macro '_NM_IN_SET'
!!(NM_VA_ARGS_FOREACH(, , op, _NM_IN_SET_OP, __VA_ARGS__)); \
^
Frequencies with the 'disabled' flag are supported by the driver but
disabled in the current regulatory domain. Don't add them to the list
of supported frequencies since they are not usable.
This is especially needed since commit f18bf17dea ('wifi: cleanup
ensure_hotspot_frequency()'), as now NetworkManager explicitly sets a
random, stable channel for Wi-Fi hotspots. If the choosen channel is
disabled, the hotspot fails to start.
Disabled channels are displayed in the 'iw phy' output as '(disabled)':
[...]
Frequencies:
* 2412 MHz [1] (30.0 dBm)
* 2417 MHz [2] (30.0 dBm)
* 2422 MHz [3] (30.0 dBm)
* 2427 MHz [4] (30.0 dBm)
* 2432 MHz [5] (30.0 dBm)
* 2437 MHz [6] (30.0 dBm)
* 2442 MHz [7] (30.0 dBm)
* 2447 MHz [8] (30.0 dBm)
* 2452 MHz [9] (30.0 dBm)
* 2457 MHz [10] (30.0 dBm)
* 2462 MHz [11] (30.0 dBm)
* 2467 MHz [12] (disabled)
* 2472 MHz [13] (disabled)
* 2484 MHz [14] (disabled)
Note that currently NM loads the list only at startup and therefore,
in case of a change of regulatory domain, a restart of the daemon is
needed to have the list updated. This needs to be improved.
https://bugzilla.redhat.com/show_bug.cgi?id=2062785
Fixes: f18bf17dea ('wifi: cleanup ensure_hotspot_frequency()')
This makes it more obvious in client when a feature that will not work
(now or in future) is being used. The motivation is to phase out WEP nicely.
To keep this simple this does not impose any policy decisions, nor is
configurable (contrary to what i've suggested before).
The policy on whether a connection activation will succeed or not is left
to the daemon. For WEP the idea is to delegate the decision further to the
supplicant while also provide a reasonable error handling [1].
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1138
The lack of configurability is due to the fact that WEP is going away
everywhere, regardless of whether it is enabled at the moment (Fedora 36)
or not (RHEL 9).
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1139https://bugzilla.redhat.com/show_bug.cgi?id=2030997
We sometimes emit warnings after a connection is added. Currently
there's a warning when the connection ID collides with another one (and
a suggestion to use an UUID instead).
Let's move the check into a separate routine, so that we can reuse it
elsewhere, such as on connection "modify" (in a following commit).
Check if a connection uses something that is likely not to work --
either now or in future.
The ultimate decision on whether it's going to work is up to the daemon.
We just use the result to color the connection differently to provide
slight visual cue to the user.
Follow-up commits are going color Wi-Fi networks and connections that rely
on deprecated features differently, to provide a visual cue.
Add color definitions for those.
We have nm_device_master_add_slave(). This should be mirrored by
nm_device_master_release_slave() (not release-one-slave).
Thereby, also rename nm_device_master_release_slaves() to
nm_device_master_release_slaves_all() to make it clearer.
I find the two (dependent) booleans "configure" and "force" confusing.
nm_device_master_release_one_slave() has many callers, it's interesting
to be able to grep for the release-type. Add an enum to make this more
readable.
This makes the non-obvious fact clearer, that when you look up an object
by an untrusted, user-provided path, it might not be the object type you
are looking for. In basically all cases, you need to check that the
result is of the expected type. This helper makes that clearer.
We often create the source with default priority, no destroy function and
attach it to the default context (g_main_context_default()). For that
case, we have wrapper functions like nm_g_timeout_add_source()
and nm_g_idle_add_source(). Use those.
There should be no change in behavior.
g_idle_add() uses G_PRIORITY_DEFAULT_IDLE priority. Most of the time we don't
care much about the priority.
But at the places that this patch changes, I think that using
G_PRIORITY_DEFAULT_IDLE (and following g_idle_add()) is more correct. The
reason for this is not very strong, except that it's probably the better
choice. And the old choice was made because I didn't realize that
g_idle_add() uses another default priority. Hence, the old choice was not
for good reasons either.
nm_g_idle_add_source() is supposed to work like g_idle_add(). Use the correct
priority.
I think this causes little actual problems, because usually we don't
carefully tune the priorities and would be mostly fine with either.
Fixes: 6b18fc252d ('shared: add nm_g_{idle,timeout}_add_source() helpers')
NMClient is strongly tied to the GMainContext with which it was created.
Several operations must only be called from within the context. There
was an assertion for that.
However, creating (and init_async()) should be allowed to call not
from within the GMainContext. So if the current context has no owner
(is not acquired), then it's also OK.
Fix the assertion for that.
Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')
