In the past, nmp_lookup_init_object() could both lookup all object for a
certain ifindex, and lookup all objects of a type. That fallback path
already leads to an assertion failure fora while now, so nobody should
be using this function to lookup all objects of a certain type (for
what, we have nmp_lookup_init_obj_type()).
Now, remove the fallback path, and rename the function to what it really
does.
ASSUME is causing more troubles than benefits it provides. This patch is
dropping NM_L3_CFG_COMMIT_TYPE_ASSUME and assume_config_once. NM3LCfg
will commit as if the sys-iface-state is MANAGED.
This patch is part of the effort to remove ASSUME from NetworkManager.
After ASSUME is dropped when starting NetworkManager it will take full
control of the interface, re-configuring it. The interface will be
managed from the start instead of assumed and then managed.
This will solve the situations where an interface is half-up and then a
restart happens. When NetworkManager is back it won't add the missing
addresses (which is what assume does) so the interface will fail during
the activation and will require a full activation.
https://bugzilla.redhat.com/show_bug.cgi?id=2050216https://bugzilla.redhat.com/show_bug.cgi?id=2077605https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1196
On glibc, HOST_NAME_MAX is defined as 64. Also, Linux'
sethostname() enforces that limit (__NEW_UTS_LEN). Also,
`man gethostname` comments that HOST_NAME_MAX on Linux is
64.
However, when building against musl, HOST_NAME_MAX is defined as 255.
That seems wrong. We use this limit to validate the hostname, and that
should not depend on the libc or on the compilation.
Hardcode the value to 64.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1197
This was only for unit testing, to check whether our reader
for "/etc/machine-id" agrees with systemd's.
That unit test was anyway flawed, because it actually accesses
the machine-id on the test system.
Anyway. Drop this. Most likely our parser is good enough, and
if we get a bug report with a defect, we can unit test against
that.
This is long replaced by nettools' n-dhcp4 client.
Drop it.
We still require NMDhcpSystemd for the DHCPv6 client.
Note that "[main].dhcp=systemd" now falls back to the internal client.
But this option was undocumented and internal anyway.
test_machine_id_read() is a flawed unit test, as it reads the machine-id
of the machine where it's running. That means the test depends on the
test machine, which is obviously a problem.
If you had no /etc/machine-id but a /var/lib/dbus/machine-id, then previously
the test would fail. If the file exists, assume we are able to read a
valid machine-id.
On test systems that have a bogus /etc/machine-id or /var/lib/dbus/machine-id,
the test would still fail. Just don't do that.
We use clang-format for automatic formatting of our source files.
Since clang-format is actively maintained software, the actual
formatting depends on the used version of clang-format. That is
unfortunate and painful, but really unavoidable unless clang-format
would be strictly bug-compatible.
So the version that we must use is from the current Fedora release, which
is also tested by our gitlab-ci. Previously, we were using Fedora 34 with
clang-tools-extra-12.0.1-1.fc34.x86_64.
As Fedora 35 comes along, we need to update our formatting as Fedora 35
comes with version "13.0.0~rc1-1.fc35".
An alternative would be to freeze on version 12, but that has different
problems (like, it's cumbersome to rebuild clang 12 on Fedora 35 and it
would be cumbersome for our developers which are on Fedora 35 to use a
clang that they cannot easily install).
The (differently painful) solution is to reformat from time to time, as we
switch to a new Fedora (and thus clang) version.
Usually we would expect that such a reformatting brings minor changes.
But this time, the changes are huge. That is mentioned in the release
notes [1] as
Makes PointerAligment: Right working with AlignConsecutiveDeclarations. (Fixes https://llvm.org/PR27353)
[1] https://releases.llvm.org/13.0.0/tools/clang/docs/ReleaseNotes.html#clang-format
Completely rework IP configuration in the daemon. Use NML3Cfg as layer 3
manager for the IP configuration of an interface. Use NML3ConfigData as
pieces of configuration that the various components collect and
configure. NMDevice is managing most of the IP configuration at a higher
level, that is, it starts DHCP and other IP methods. Rework the state
handling there.
This is a huge rework of how NetworkManager daemon handles IP
configuration. Some fallout is to be expected.
It appears the patch deletes many lines of code. That is not accurate, because
you also have to count the files `src/core/nm-l3*`, which were unused previously.
Co-authored-by: Beniamino Galvani <bgalvani@redhat.com>
- add "pre-commit" signal.
- fix assertion in nm_l3_config_data_get_ip6_privacy().
- set IPv6 privacy in _init_from_connection_ip() from profile.
- fix leaking "os_zombie_lst" in _obj_state_data_free().
- remove wrong assertion about VRF.
- fix _routes_temporary_not_available_update() to honor only the
requested object type. Otherwise, we always prune unrelated objects
too.
We have nm_l3cfg_commit(), however that is synchronous and triggers an
avalanche of side effects. So it should be avoided if a component is
not aware of the current circumstances in which it gets called (most of them).
The alternative is nm_l3cfg_commit_on_idle_schedule(), but previously
that only supported the auto type.
Two changes:
- add a commit_type parameter to nm_l3cfg_commit_on_idle_schedule().
This allows to explicitly select a type for the next commit.
Previously, if the caller wanted for example to trigger a reapply
once, they had to register a handle, trigger the commit and unregister
the handle again. This basically allows to specify an ad-hoc commit
type that is only used once.
- if an explicit commit type is requested, then still always combine
it with auto. That means, we always use the "maximum" of what is
requested and what is registered.
NML3ConfigData is supposed to be immutable. It can be initialized from a
NMConnection, and its DNS priority property might be zero.
For the DNS priority, the value can be overwritten by global defaults.
We thus need to inject the default value at the right place.
This helper class is supposed to encapsulate most logic about
configuring IPv6 link local addresses and exposes a simpler API in order
to simplify NMDevice. Currently this logic is spread out in NMDevice.
Also, NML3IPv6LL directly uses NML3Cfg, thereby freeing NMDevice to care
about that too much.
For several reasons, NML3IPv6LL works different than NML3IPv4LL.
For one, with IPv6 we need to configure the address in kernel, which does
DAD for us. So, NML3IPv6LL will tell NML3Cfg to configure those
addresses that it wants to probe. For IPv4, it only tells NML3Cfg to do
ACD, without configuring anything yet. That is left to the caller.
It's great to have functions that cannot fail, because it allows to
skip any error handling.
_set_stable_privacy() as it was could not fail, so the only reason why
nm_utils_ipv6_addr_set_stable_privacy() could fail is because the DAD
counter exhausted.
Also, it will be useful to have a function that does not do the counter
check, where the caller wants to handle that differently.
Rename some functions, and make the core nm_utils_ipv6_addr_set_stable_privacy()
not failable.
The "only_dirty" parameter to a remove-all() function is odd.
For one, the function is called remove-all, but depending on a parameter
it does not remove all.
Also, setting remove-all(only_dirty=TRUE) means it will remove not
everything, so passing TRUE will remove only parts. That logic seems
confusing.
Avoid that, by removing the parameter from nm_l3cfg_remove_config_all()
and add nm_l3cfg_remove_config_all_dirty().
Each NML3ConfigData should have a source set, and in fact most callers
would call nm_l3_config_data_set_source() right after creating the
instance.
Move the source parameter to the new() constructor function. Also remove
the setter, making the source of an instance immutable.
As every l3cfg instance generally has a clear purpose, the source should
always be known from the start and doesn't need to change.
The "utils" part does not seem useful in the name.
Note that we also have NMStrBuf, which is named nm_str_buf_*().
There is an unfortunate similarity between the two, but it's still
distinct enough (in particular, because one takes an NMStrBuf and
the other not).
I missed that we already have a gettid() wrapper. Drop the duplicated
again and use nm_utils_gettid().
Fixes: e874c5bf6b ('random: Provide missing gettid() declaration')
NMConfigData is immutable and with the previous commit are the strings
already cached internally. There is no need to clone it.
Of course, the callers must not assume that the string stays alive after
a config reload (SIGHUP), where the NMConfigData might change. So they
are not always alive, but long enough for all callers to avoid cloning
the string.
Coverity says:
Error: ALLOC_FREE_MISMATCH (CWE-762):
NetworkManager-1.31.3/src/core/tests/test-systemd.c:261: alloc: Allocation of memory which must be freed using "free".
NetworkManager-1.31.3/src/core/tests/test-systemd.c:274: free: Calling "_nm_auto_g_free" frees "exp2_arr" using "g_free" but it should have been freed using "free".
# 272| g_assert_cmpmem(expected_arr, expected_len, exp3_arr, exp3_len);
# 273| }
# 274|-> }
# 275|
# 276| #define _test_unbase64mem(base64, expected_str) \
Error: ALLOC_FREE_MISMATCH (CWE-762):
NetworkManager-1.31.3/src/core/tests/test-systemd.c:270: alloc: Allocation of memory which must be freed using "free".
NetworkManager-1.31.3/src/core/tests/test-systemd.c:274: free: Calling "_nm_auto_g_free" frees "exp3_arr" using "g_free" but it should have been freed using "free".
# 272| g_assert_cmpmem(expected_arr, expected_len, exp3_arr, exp3_len);
# 273| }
# 274|-> }
# 275|
# 276| #define _test_unbase64mem(base64, expected_str) \
Fixes: 0298d54078 ('systemd: expose unbase64mem() as nm_sd_utils_unbase64mem()')
Having two functions like link_set_x() and link_set_nox() it is not a
good idea. This patch is introducing nm_platform_link_change_flags().
This allow flag modification directly, so the developer does not need to
define the virtual functions all the time everywhere.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
glib requires G_LOG_DOMAIN defined so that log messages are labeled
to belong to NetworkManager or libnm.
However, we don't actually want to use glib logging. Our library libnm
MUST not log anything, because it spams the user's stdout/stderr.
Instead, a library must report notable events via its API. Note that
there is also LIBNM_CLIENT_DEBUG to explicitly enable debug logging,
but that doesn't use glib logging either.
Also, the daemon does not use glib logging instead it logs to syslog.
When run with `--debug`.
Hence, it's not useful for us to define different G_LOG_DOMAIN per
library/application, because none of our libraries/applications should
use glib logging.
It also gets slightly confusing, because we have the static library like
`src/libnm-core-impl`, which is both linked into `libnm` (the library)
and `NetworkManager` (the daemon). Which logging domain should they use?
Set the G_LOG_DOMAIN to "nm" everywhere. But no longer do it via `-D`
arguments to the compiler.
See-also: https://developer.gnome.org/glib/stable/glib-Message-Logging.html#G-LOG-DOMAIN:CAPS
"libnm-core/" is rather complicated. It provides a static library that
is linked into libnm.so and NetworkManager. It also contains public
headers (like "nm-setting.h") which are part of public libnm API.
Then we have helper libraries ("libnm-core/nm-libnm-core-*/") which
only rely on public API of libnm-core, but are themself static
libraries that can be used by anybody who uses libnm-core. And
"libnm-core/nm-libnm-core-intern" is used by libnm-core itself.
Move "libnm-core/" to "src/". But also split it in different
directories so that they have a clearer purpose.
The goal is to have a flat directory hierarchy. The "src/libnm-core*/"
directories correspond to the different modules (static libraries and set
of headers that we have). We have different kinds of such modules because
of how we combine various code together. The directory layout now reflects
this.
Currently "src/" mostly contains the source code of the daemon.
I say mostly, because that is not true, there are also the device,
settings, wwan, ppp plugins, the initrd generator, the pppd and dhcp
helper, and probably more.
Also we have source code under libnm-core/, libnm/, clients/, and
shared/ directories. That is all confusing.
We should have one "src" directory, that contains subdirectories. Those
subdirectories should contain individual parts (libraries or
applications), that possibly have dependencies on other subdirectories.
There should be a flat hierarchy of directories under src/, which
contains individual modules.
As the name "src/" is already taken, that prevents any sensible
restructuring of the code.
As a first step, move "src/" to "src/core/". This gives space to
reorganize the code better by moving individual components into "src/".
For inspiration, look at systemd's "src/" directory.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/743
