When determining the hostname, it is preferable to evaluate devices in
a predictable order to avoid that the hostname changes between
different boots.
The current order is based first on hostname priority, then on the
presence of a best default route, and then on activation order.
The activation order is not a very strong condition, as it is
basically useless for devices that are autoactivated at boot.
As we already prefer IPv4 over IPv6 within the same connection, also
prefer it when 2 connections have the same priority and the same
default route status, to achieve better predictability.
https://bugzilla.redhat.com/show_bug.cgi?id=1970335https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/895
We now have a cached list of NMSettingsConnection instances,
sorted by their autoconnect priority.
However, the sort order nm_settings_connection_cmp_autoconnect_priority()
depends on various properties of the connection:
- "connection.autoconnect" and "connection.autoconnect-priority"
- the timestamp
- "connection.uuid"
These properties almost never change, so it's a waste that every call
to nm_settings_get_connections_sorted_by_autoconnect_priority() needs
to check whether the sort order is still satisfied.
We can do better by tracking when the sort order might have been
destroyed and only check in those (much fewer) cases.
Note that we end up calling nm_settings_get_connections_sorted_by_autoconnect_priority()
a lot, so this makes a difference.
The GetSettings() call is not the only place where we convert a
NMConnection to D-Bus. However it is one of the most prominent ones
with a measurable performance overhead.
The connection seldom changes, so it makes sense to cache it.
Note that GetSettings() is the only caller that specifies an option,
thus it's the only caller that converts a NMConnection to variant
in this particular way. That means, other callers don't benefit from
this caching and we could not cache the variant in the NMConnection
instance itself, because those callers use different parameters.
Not very useful, but it seems nicer to read. They anyway can be
inlined. After all, naming and structure is important and the places
where we emit signals are important. By having well-named helper
functions, these places are easier to find and reason about.
Turns out, we call nm_settings_get_connection_clone() *a lot* with sort order
nm_settings_connection_cmp_autoconnect_priority_p_with_data().
As we cache the (differently sorted) list of connections, also cache
the presorted list. The only complication is that every time we still
need to check whether the list is still sorted, because it would be
more complicated to invalidate the list when an entry changes which
affects the sort order. Still, such a check is usually successful
and requires "only" N-1 comparisons.
NMConnection is a glib interface, implemented only by NMSimpleConnection
and NMRemoteConnection.
Inside the daemon, every NMConnection instance is always a NMSimpleConnection.
Using glib interfaces has an overhead, for example NM_IS_CONNECTION() needs
to search the implemented types for the pointer. And NM_CONNECTION_GET_PRIVATE()
is implemented by attaching user data to the GObject instance. Both have measurable
overhead.
Special case them for NMSimpleConnection.
This optimizes primarily the call to nm_connection_get_setting_connection(),
which easily gets called millions of times. This is easily measurable.
The NM_TYPE_SETTING_* macros are really function calls (to a GType/gsize which is
guarded by an atomic operation for thread safe initialization). Also, finding
the setting_info based on the GType requires additional lookups.
It's no longer necessary. We can directly find the setting using the
well known index.
A NMConnection tracks a list of NMSetting instances. For
each setting type, it only can track one instance, as is
clear by the API nm_connection_get_setting().
The number of different setting types is known at compile time,
currently it is 52. Also, we have an NMMetaSettingType enum,
which assigns each type a number.
Previously, we were tracking the settings in a GHashTable.
Rework that, to instead use a fixed size array.
Now every NMConnection instance consumes 52 * sizeof(pointer)
for the settings array. Previously, the GHashTable required to malloc
the "struct _GHashTable" (on 64bit that is about the size of 12
pointers) and for N settings it allocated two buffers (for
the key and the values) plus one buffer for the hash values. So,
it may or may not consume a bit more memory now, but also can lookup
settings directly without hashing.
When looking at all settings, we iterate the entire array. Most
entries will be NULL, so it's a question whether this could be done
better. But as the array is of a fixed, small size, naive iteration
is probably still faster and simpler than anything else.
---
Test: compiled with -O2, x86_64:
$ T=src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh; \
make -j 8 "$T" && \
"$T" 1>/dev/null && \
perf stat -r 200 -B "$T" 1>/dev/null
Before:
Performance counter stats for 'src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh' (200 runs):
338.39 msec task-clock:u # 0.962 CPUs utilized ( +- 0.68% )
0 context-switches:u # 0.000 K/sec
0 cpu-migrations:u # 0.000 K/sec
1,121 page-faults:u # 0.003 M/sec ( +- 0.03% )
1,060,001,815 cycles:u # 3.132 GHz ( +- 0.50% )
1,877,905,122 instructions:u # 1.77 insn per cycle ( +- 0.01% )
374,065,113 branches:u # 1105.429 M/sec ( +- 0.01% )
6,862,991 branch-misses:u # 1.83% of all branches ( +- 0.36% )
0.35185 +- 0.00247 seconds time elapsed ( +- 0.70% )
After:
Performance counter stats for 'src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh' (200 runs):
328.07 msec task-clock:u # 0.959 CPUs utilized ( +- 0.39% )
0 context-switches:u # 0.000 K/sec
0 cpu-migrations:u # 0.000 K/sec
1,130 page-faults:u # 0.003 M/sec ( +- 0.03% )
1,034,858,368 cycles:u # 3.154 GHz ( +- 0.33% )
1,846,714,951 instructions:u # 1.78 insn per cycle ( +- 0.00% )
369,754,267 branches:u # 1127.052 M/sec ( +- 0.01% )
6,594,396 branch-misses:u # 1.78% of all branches ( +- 0.23% )
0.34193 +- 0.00145 seconds time elapsed ( +- 0.42% )
nm_meta_setting_infos is a list of all NMMetaSettingInfo, sorted by name.
Add nm_meta_setting_types_by_priority which provides a mapping with a
different sort order (first by priority). We need that sometimes.
NSS is not available in musl libc:
./src/nm-daemon-helper/nm-daemon-helper.c:9:10: fatal error: nss.h: No such file or directory
9 | #include <nss.h>
| ^~~~~~~
This fixes commit 21c8a6b20e ('libnm-core, all: merge IPv4 and IPv6
address/route types'), which introduced this API but didn't export it
in the library. In practice this API is thus only usable since 1.32.0.
During timeout we cancel the (internal) GCancellable. But the overall
error reason is not cancellation by the user, it's timeout. Fix
the error reason.
Fixes: 9ebdb967de ('firewall: implement masquerading for shared mode with nftables')
It seems that the nftables backend works well. Let's change the default.
This will also be backported to nm-1-32 branch, for 1.32.0 release.
This reverts commit 0609f1f31c.
Coverity wrongly think there is a use after free in the test:
Error: USE_AFTER_FREE (CWE-416): [#def559] [important]
NetworkManager-1.31.90/src/libnm-glib-aux/tests/test-shared-general.c:1305: alias: Assigning: "s1" = "_s". Now both point to the same storage.
NetworkManager-1.31.90/src/libnm-glib-aux/tests/test-shared-general.c:1324: freed_arg: "g_source_unref" frees "s1".
NetworkManager-1.31.90/src/libnm-glib-aux/tests/test-shared-general.c:1330: deref_after_free: Dereferencing freed pointer "s1".
# 1328| s2 = nm_g_source_sentinel_get(0);
# 1329| g_assert(s2 == s1);
# 1330|-> g_assert_cmpint(g_atomic_int_get(&s1->ref_count), >=, 1);
# 1331| }
# 1332| }
Rework the code in the hope to avoid the false warning.
The warning is wrong, because we already assert for the string length a few
lines earlier.
Error: STRING_OVERFLOW (CWE-120): [#def595]
NetworkManager-1.31.90/src/libnm-platform/nm-platform-utils.c:1896: fixed_size_dest: You might overrun the 16-character fixed-size string "ifname_buf_last_try" by copying "ifname" without checking the length.
# 1894| if (nm_streq(ifname, ifname_buf_last_try))
# 1895| return -1;
# 1896|-> strcpy(ifname_buf_last_try, ifname);
# 1897|
# 1898| fd_dir = open(sysdir, O_DIRECTORY | O_CLOEXEC);
When subclassing a GObject type, the class and object structs
must be available and defined in the header.
For libnm, and in particular for NMSetting classes, we don't want
users to subclass NMSetting. It also doesn't work, because libnm
has internal code that is necessary to hook up the NMSetting class.
You cannot define your own type and make it work together with
libnm.
Having the structs in public headers limits what we can do with them.
For example, we could embed the private data directly in the structures
and avoid the additional indirection.
This is an API break, but for something that most likely nobody cares
about. Or better, nobody should care about. API is not what is
accidentally defined in a header, API was the library provides to
meaningfully use. Subclassing these types is not meaningful and was
only accidentally possible so far.
Only hide the structs for now. More cleanup is possible later. We shall
however aim to keep the padding and struct layout to not also break ABI.
The new function resolve an address via DNS, first by using
systemd-resolved (disabling synthesized results) and then by spawning
the daemon helper.
Trying systemd-resolved via D-Bus before spawning the helper is
important to get a correct result. Suppose that resolv.conf points to
the local stub listener at 127.0.0.53; if NM only spawns the helper,
the helper will query the local systemd-resolved which could return a
synthesized result.
Therefore, we first query systemd-resolved with NO_SYNTHESIZE and
then, in case of error, we spawn the helper.
Implement a new operation to resolve the hostname from an
address. Note the call to __nss_configure_lookup("hosts", "dns"): this
configures the glibc Name Service Switch (NSS) to only use DNS,
bypassing other modules that might be configured in
/etc/nsswitch.conf. Other modules like 'myhostname' or 'resolve' can
return synthesized names, while we are only interested in real DNS
results.
