Reapply() is supposed to make sure that the system (the interface)
is configured as indicated by the applied-connection. That means,
it will remove/add configuration to make the system match the requested
configuration.
Add a flag "preserve-external-ip" which relaxes this. During reapply,
IP addresses/routes that exist on the interface and which are not known
(or added) by NetworkManager will be left alone.
This will be used by nm-cloud-setup, so that it can reconfigure the
interface in a less destructive way, which does not conflict with
external `ip addr/route` calls.
Note that the previous commit just adds "VersionInfo" and the
possibility to expose capabilities (patch-level). This is not used
for the new reapply flag, because, while we might backport the
reapply flag, we won't backport the "VersionInfo" property. Exposing
new capabilities via the "VersionInfo" property will only become useful
in the future, where we can backport a capability to older NM versions
(but those that have "VersionInfo" too).
Changing an error code is an API change. But, so far no flags existed,
so it's unlikely that somebody would send invalid flags or care about
the return code.
This exposes NM_VERSION as number (contrary to the "Version", which is a
string). That is in particular useful, because the number can be
compared with <> due to the encoding of the version.
While at it, don't make it a single number. Expose an array of numbers,
where the following numbers are a bitfield of capabilities.
Note that before commit 3c67a1ec5e ('cli: remove version check against
NM'), we used to parse the "Version" string to detect the version. As
such, the information that "VersionInfo" exposes now, was already
(somewhat) available, you just had to parse the string. The main benefit of
"VersionInfo" is that it can expose capabilities (patched behavior) in
in a lightweight bitfield. To include the numerical version there is
just useful on top.
Currently no additional capabilities are exposed. The idea is of course
to have a place in the future, where we can expose additional
capabilities. Adding a capability flag is most useful for behavior that we
backport to older branches. Otherwise, we could just check the daemon version
alone. But since we only add "VersionInfo" property only now, we cannot backport
any capability further than this, because the "VersionInfo" property itself
won't be backported. As such, this will only be useful in the future by having
a place where we can add (and backport) capabilities.
Note that there is some overlap with the existing "Capability" property
and NMCapability enum. The difference is that adding a capability via "VersionInfo"
is only one bit, and thus cheaper. Most importantly, having it cheaper means
the downsides of adding a capability flag is significantly removed. In
practice, we could live without capabilities for a long time, so they
must be very cheap for them to be worth to add. Another difference might be,
that we will want that the VersionInfo is about compile time defaults (e.g.
a certain patch/behavior that is in or not), while NM_CAPABILITY_TEAM depends on
whether the team plugin is loaded at runtime.
The warning "-Wcast-align=strict" seems useful and will be enabled
next. Fix places that currently cause the warning by using the
new macro NM_CAST_ALIGN(). This macro also nm_assert()s that the alignment
is correct.
GArray.data is a char pointer. Most of the time we track other data in
a GArray. Casting that pointer can trigger "-Wcast-align=strict"
warnings.
Avoid them. Most of the time, instead use the nm_g_array*() helpers,
which also assert that the expected element size is correct.
We put all these structs inside the tagged union NMPObject.
Also, in a sense NMPlatformObject is the base "type" of all
these structs, meaning, it should be able to up and downcast.
Ensure the alignment matches.
This helps to avoid "-Wcast-align" warnings when trying to cast
a (NMPlatformObject*) to another (NMPlatformXXX *) type. Something
we commonly do.
nm_utils_get_ipv6_interface_identifier() has non-obvious requirements on
the hardware address. If the caller passes a wrong length, it will
trigger an assertion or even cause out of bound read. This would mean
that the caller needs to carefully check the length. Such requirements
on the caller are wrong.
Also, in practice the hardware length comes from platform/kernel. We
don't want to trust that what kernel tells us always has the required
address length, so the caller would always have to double check before
calling the function.
Instead, handle unexpected address lengths.
Fixes: e2270040c0 ('core: use Interface Identifiers for IPv6 SLAAC addresses')
Fixes: 1d396e9972 ('core-utils: use 64-bit WPAN address for a 6LoWPAN IID')
For link type NM_LINK_TYPE_6LOWPAN, nm_utils_get_ipv6_interface_identifier()
expects 8 bytes hardware address. It even just accesses the buffer
without checking (that needs to be fixed too).
For 6lowpan devices, the caller might construct a fake ethernet MAC
address, which is only 6 bytes long. So wrong.
Fixes: 49844ea55f ('device: generate pseudo 48-bit address from the WPAN short one')
The fields "l3cfg" and "l3cfg_" are union aliases. One of them is const,
the other is not. The idea is that all places that modify the field need
to use the special name "l3cfg_", and grepping for that will lead you to
all the relevant places.
This mistake happened, because g_clear_object() casts constness away.
Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
"connection" variable might be NULL, which fails an assertion in
g_dbus_connection_flush_sync(). Consequently, "error_flush" is also
NULL which leads to a crash of "nm-dhcp-helper".
Reported-by: Jules Maselbas <jmaselbas@zdiv.net>
Fixes: 240ec7f891 ('dhcp: implement ACD (address collision detection) for DHCPv4')
In some scenarios, autoconnect should not be blocked if the device is
activated on the external connection (e.g. autoconnect on the loopback
device).
Adding the `allow_autoconnect_on_external` flag to support such
behavior.
Support managing the loopback interface through NM as the users want to
set the proper mtu for loopback interface when forwarding the packets.
Additionally, the IP addresses, DNS, route and routing rules are also
allowed to configure for the loopback connection profiles.
https://bugzilla.redhat.com/show_bug.cgi?id=2060905
We soon will handle loopback, so -- if no loopback profile is activated
in NetworkManager -- we will have an externally managed profile on
loopback. This messes up the result.
In general, external connections don't make much sense for
build_device_hostname_infos(). Ignore them.
any_devices_active() exists to avoid hostname update when no devices are
active. See [1] and commit b07f6712e9 ('policy: check for active
devices before triggering dns update on hostname change').
Soon, we will add support for loopback device, so "lo" will
almost always be activated (either externally or actively managed by
NetworkManager).
In any case, external devices should not count here, even if they appear
activating/activated.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1344303
This is the version shipped in Fedora 37. As Fedora 37 is now out, the
core developers switch to it. Our gitlab-ci will also use that as base
image for the check-{patch.tree} tests and to generate the pages. There
is a need that everybody agrees on which clang-format version to use,
and that version should be the one of the currently used Fedora release.
Also update the used Fedora image in "contrib/scripts/nm-code-format-container.sh"
script.
The gitlab-ci still needs update in the following commit. The change
in isolation will break the "check-tree" test.
We sometimes have functions foo() and foo_full(), in which case
foo() has fewer arguments and just calls foo_full(). The "full"
function here is the more powerful one, and foo() is implemented
in terms of the former.
nm_platform_ip4_route_cmp_full() and m_platform_ip4_route_cmp() inverted
that pattern. The "_full" there stands for the full comparison, to not
allowing to select the comparison type.
That inconsistency is ugly. Also, these wrappers were used at only few
places. Let's drop them.
While at it, also drop nm_platform_qdisc_cmp() and rename
nm_platform_qdisc_cmp_full(). Here cmp()/cmp_full() followed the common
pattern foo()/foo_full(), but it's still hardly used and unnecessary.
When adding a new route we need to consider it contains extra nexthops
i.e it is a ECMP route. As we cannot modify the NMPObject once created,
we need to pass the extra nexthops as an argument.
We cannot use the original NMPObject because normalization is happening
during when adding the route.
When reading from netlink an ECMP IPv4 route, we need to parse the
multiple nexthops. In order to do that, we are introducing
NMPlatformIP4RtNextHop struct.
The first nexthop information will be kept at the original
NMPlatformIP4Route and the new property n_nexthops will indicate how
many nexthops we need to consider.
This test is inherently fragile, as it depends on starting processes,
wait for something and kill the process. There are timings involved
that are out of control of the test. Try to adjust the timing.
# NetworkManager-DEBUG: <debug> [1668755976.9741] kill child process test-s-4 (111487): sending SIGKILL...
# NetworkManager-DEBUG: <debug> [1668755976.9753] kill child process test-s-4 (111487): waiting for process to terminate after sending SIGTERM (15) and SIGKILL...
# NetworkManager-DEBUG: <debug> [1668755976.9758] kill child process test-s-4 (111487): after sending SIGTERM (15) and SIGKILL, process 111487 exited by signal 9 (5759 usec elapsed)
Bail out! GLib:ERROR:../src/core/tests/test-core-with-expect.c:154:test_nm_utils_kill_child_sync_do: Did not see expected message NetworkManager-DEBUG: *<debug> [*] kill child process test-s-4 (*): waiting up to 1 milliseconds for process to terminate normally after sending SIGTERM (15)...
Bail out! nm:ERROR:../src/core/tests/test-core-with-expect.c:457:test_nm_utils_kill_child: assertion failed (exit_status == 0): (6 == 0)
--- stderr ---
**
GLib:ERROR:../src/core/tests/test-core-with-expect.c:154:test_nm_utils_kill_child_sync_do: Did not see expected message NetworkManager-DEBUG: *<debug> [*] kill child process test-s-4 (*): waiting up to 1 milliseconds for process to terminate normally after sending SIGTERM (15)...
**
nm:ERROR:../src/core/tests/test-core-with-expect.c:457:test_nm_utils_kill_child: assertion failed (exit_status == 0): (6 == 0)
/builds/NetworkManager/NetworkManager/tools/run-nm-test.sh: line 337: 110662 Aborted "${NMTST_DBUS_RUN_SESSION[@]}" "${NMTST_LIBTOOL[@]}" "$NMTST_VALGRIND" --quiet --error-exitcode=$VALGRIND_ERROR --leak-check=full --gen-suppressions=all "${NMTST_SUPPRESSIONS[@]}" --num-callers=100 --log-file="$LOGFILE" "$TEST" "${TEST_ARGV[@]}"
See wpa_supplicant commit [1]:
macsec: Make pre-shared CKN variable length
IEEE Std 802.1X-2010, 9.3.1 defines following restrictions for
CKN:
"MKA places no restriction on the format of the CKN, save that it
comprise an integral number of octets, between 1 and 32
(inclusive), and that all potential members of the CA use the same
CKN. No further constraints are placed on the CKNs used with PSKs,
..."
Hence do not require a 32 octet long CKN but instead allow a
shorter CKN to be configured.
This fixes interoperability with some Aruba switches, that do not
accept a 32 octet long CKN (only support shorter ones).
[1] https://w1.fi/cgit/hostap/commit/?id=b678ed1efc50e8da4638d962f8eac13312a4048f
When called with update_carrier=TRUE, nm_device_bring_up_full() checks
for carrier changes and it may queue a transition to DISCONNECTED
through the following call chain:
-> nm_device_bring_up_full()
-> nm_device_set_carrier_from_platform()
-> nm_device_set_carrier()
-> carrier_changed()
-> nm_device_queue_state()
In _set_state_full(state=UNAVAILABLE) after bringing the interface up
we also call nm_device_cleanup() which clears the enqueued state
change to DISCONNECTED. When this happens, the device remains in
UNAVAILABLE and never gets activated even if it was ready.
This was observed with macsec interfaces, but in theory can happen
with all those interfaces that get carrier immediately after being
brought up.
Avoid this issue by not checking the carrier synchronously from
_set_state_full(). The carrier change event will be processed in the
next asynchronous invocation of device_link_changed().
https://bugzilla.redhat.com/show_bug.cgi?id=2122564
In the next commit nm_device_bring_up() will be extended with a new
argument. Most callers just want to bring up the device synchronously
and don't care about the "no_firmware" argument. Introduce a
nm_device_bring_up_full() for callers that need special behavior.
Currently VPNs always apply the configuration sent by the server for
both address families. So, even if users set e.g. ipv6.method=disabled,
they might end up with IPv6 configured.
Change that and apply the automatic configuration only when the method
is "auto".
This is a change in behavior and as such it might be disruptive for
users that had a method different from "auto" and expected to have the
interface configured. However, that scenario seems unlikely and can be
easily fixed by setting the right method.
When creating NMPlatformRoutingRule from NMIPRouteRule object, the
protocol is being set to RTPROT_UNSPEC. According to linux kernel
documentation FRA_PROTOCOL indicates the originator of the rule.
In this case the route rule is coming from a connection and therefore
the originator of the rule is the user. The correct value is
RTPROT_STATIC which means the rule is installed by the administrator.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1457
Fixes: 3f9347745b ('core: add handling of IP routing rules to NMDevice')
These are helpers for GDBusProxy. I think we should avoid GDBusProxy where possible,
and these functions too. Give the function a more specific name to show that this
is only for the proxy.
These are just general purpose D-Bus utils, based on glib and GDBus.
They fit perfectly to libnm-glib-aux. Move the code.
Also, there is already the file "src/core/nm-dbus-utils.c", having two
files with the same name on our source tree is just confusing.
Most users included this by accident, by including nm-connection.h. That
is not too great, becuase stuff it contains is by no means specific to
NMConnection.
Anyways, it's not like it would matter too that. I mainly care about it
being included in NetworkManager.h, so that there's one less special
case in a test that makes sure useful stuff from NetworkManager.h ends up
in gtk-doc (a separate commit).
NMSettingWired does not reject invalid flags. Filter them out in wake_on_lan_enable().
In practice, it makes no difference, the unknown flags were ignored anyway.
If there were any pause options and any non-pause options,
the created setting was invalid.
I don't think it's reasonably possible to parse the broken settings.
So there is no workaround trying to read the existing broken settings
from disk. Luckily, the broken setting was just silently ignored by
the parser, so you simply could not persist certain settings.
https://bugzilla.redhat.com/show_bug.cgi?id=2134569
Fixes: 652ddca04c ('ethtool: Introducing PAUSE support')
It's possible that the modem is enabled outside of NM. If not
re-check, device could stay disabled throughout until something else
about the modem changes again.
