For a long time we have a function like nm_uuid_generate_from_strings().
This was recently reworked and renamed, but it preserved behavior. Preserving
behavior is important for this function, because for the existing users,
we need to keep generating the same UUIDs.
Originally, this function was a variadic function with NULL sentinel.
That means, when you write
nm_uuid_generate_from_strings(uuid_type, type_arg, v1, v2, v3, NULL);
and v2 happens to be NULL, then v3 is ignored. That is most likely not
what the user intended. Maybe they had a bug and v2 should not be NULL.
But nm_uuid_generate_from_strings() should not require that all
arguments are non-NULL and it should not ignore arguments after the
first NULL.
For example, one user works around this via
uuid = nm_uuid_generate_from_strings_old("ibft",
s_hwaddr,
s_vlanid ? "V" : "v",
s_vlanid ? s_vlanid : "",
s_ipaddr ? "A" : "DHCP",
s_ipaddr ? s_ipaddr : "");
which is cumbersome and ugly.
That will be fixed next, by adding a function that doesn't suffer
from this problem. But "this problem" is part of the API of the
function, we cannot just change it. Instead, rename it and all
users, so they can keep doing the same.
New users of course should no longer use the "old" function.
The file "nm-meta-setting-base-impl.c" is shared by "libnm-core-impl" and
"libnmc-setting". For "libnm-core-impl" it uses a efficient lookup from the
gtype. For "libnmc-setting", that class information is not available, so
it did a linear search. Instead, do a binary search.
Tested:
diff --git a/src/libnm-core-impl/nm-meta-setting-base-impl.c b/src/libnm-core-impl/nm-meta-setting-base-impl.c
index 3434c858391f..62c366d2ca42 100644
--- a/src/libnm-core-impl/nm-meta-setting-base-impl.c
+++ b/src/libnm-core-impl/nm-meta-setting-base-impl.c
@@ -821,6 +821,11 @@ nm_meta_setting_infos_by_gtype(GType gtype)
{
const NMMetaSettingInfo *setting_info;
+#if _NM_META_SETTING_BASE_IMPL_LIBNM
+ return _infos_by_gtype_binary_search(gtype);
+#endif
+ nm_assert_not_reached();
+
#if _NM_META_SETTING_BASE_IMPL_LIBNM
setting_info = _infos_by_gtype_from_class(gtype);
#else
diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c
index 85d549eb766c..65fcafd076c9 100644
--- a/src/libnm-core-impl/tests/test-setting.c
+++ b/src/libnm-core-impl/tests/test-setting.c
@@ -5134,6 +5134,29 @@ main(int argc, char **argv)
{
nmtst_init(&argc, &argv, TRUE);
+ {
+ gs_unref_object NMConnection *con = NULL;
+ guint8 ctr = 0;
+ guint i;
+
+ con = nmtst_create_minimal_connection("test", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL);
+
+ nm_connection_add_setting(con, nm_setting_wired_new());
+
+ nmtst_connection_normalize(con);
+ nmtst_assert_connection_verifies_without_normalization(con);
+
+ for (i = 0; i < 10000000; i++) {
+ ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_WIRED));
+ ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_CONNECTION));
+ ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_PROXY));
+ ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_WIREGUARD));
+ ctr += GPOINTER_TO_UINT(nm_connection_get_setting(con, NM_TYPE_SETTING_IP4_CONFIG));
+ }
+
+ return !!ctr;
+ }
+
g_test_add_func("/libnm/test_connection_uuid", test_connection_uuid);
g_test_add_func("/libnm/settings/test_nm_meta_setting_types_by_priority",
Results of `make src/libnm-core-impl/tests/test-setting && libtool --mode=execute perf stat -r 5 -B src/libnm-core-impl/tests/test-setting`:
1) previous linear search: 3,182.81 msec
2) bsearch not inlined: 1,611.19 msec
3) bsearch open-coded: 1,214.45 msec
4) bsearch inlined: 1,167.70 msec
5) lookup from class: 1,147.64 msec
1) previous implementation
2) using nm_array_find_bsearch()
3) manually implementing binary search
4) using nm_array_find_bsearch_inline()
5) only available to libnm-core as it uses internal meta data
Note that for "libnm-core-impl" the implementation was already fast (5), and
it didn't change. It only changed to binary search for "libnmc-setting",
which arguably is a less strong use-case.
The file "nm-meta-setting-base-impl.c" is present twice in our source
tree and compiled twice. Once with internal headers of libnm-core and
once with only public headers.
Consequently, there are two implementations for
nm_meta_setting_infos_by_gtype(), one that can benefit from internal
access to the data structure, and the one for libnmc, which cannot.
Refactor the implementations, in the hope to have it clearer.
The G_TYPE_INSTANCE_GET_CLASS() macro is just one pointer dereference
(self)->g_class, plus additional assertions with debug builds.
As such, it is as fast as it gets. Embed the address family there, and
implement NM_SETTING_IP_CONFIG_GET_ADDR_FAMILY() that way.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1395
NMConnection is an interface, implemented by NMSimpleConnection and
NMRemoteConnection.
For the most part, an NMConnection is only the content of the profile
(the settings). The "path" of the connection refers to the D-Bus path,
and wouldn't really make sense of the NMConnection interface or the
NMSimpleConnection type.
As such, the daemon (which only uses NMConnection and
NMSimpleConnection) never sets the path. Only libnm does.
NMClient uses NMRefString extensively for the D-Bus interface and the
path is already internalized. Take advantage of that. It is very likely,
that we are able to share the path instance in libnm at which point it
makes sense to use NMRefString.
Also, during nm_simple_connection_new_clone(), we can just take another
reference instead of cloning the string.
For new uses of nm_uuid_generate_from_strings() we should generate version5
UUIDs and we should use unique namespace UUID arguments.
The namespace UUID was so far replaced by always passing a special prefix
as first string. It seems nicer to use a namespace instead.
Version3 UUIDs should not be used for new applications.
Hence, nm_uuid_generate_from_strings_v3() is no longer a desirable way to
generate UUIDs, so drop the wrapper.
nm_uuid_generate_from_strings() accepts a uuid_type and type_arg
parameter, so that we can use it to generate version 5 UUIDs.
This is a more flexible variant of nm_uuid_generate_from_strings_v3(),
which will be used to replace it. With the right parameters, the new
function behaves the same as nm_uuid_generate_from_strings_v3().
nm_uuid_generate_from_strings() uses variant3 UUIDs based on MD5.
We shouldn't use that in the future.
We will add a replacement, so rename this function so that the "good"
name is free again. Of course, code that uses this function currently
relies on that the behavior doesn't change. We cannot just drop it
entirely, but will replace it by something that gives the same result.
Rename.
We already redefine those checks to optimize for NMSimpleConnection.
Which, in particular when libnm-core is used by the daemon, is the only
implementation of the NMConnection interface.
Move those to the private header file. No need to keep it private to
"nm-connection.c".
NMConnection is an interface, and as such has no data itself.
In practice, there are only two implementations of this interface,
NMSimpleConnection and NMRemoteConnection. The latter only exists
in libnm, not the daemon.
Thus, lookup of the private data is already optimized for
NMSimpleConnection instances via _nm_simple_connection_private_offset.
Use the same mechanism also for NMSimpleConnection itself.
Have "len" before "elem_size". That is consistent with g_qsort_with_data()
and bsearch(), and is also what I would expect.
Note that the previous commit just renamed the function. If a user
of the new, changed API gets backported to an older branch, we will
get a compilation error and note that the arguments need to be adjusted.
The "nm_utils_" prefix is just too verbose. Drop it.
Also, Posix has a bsearch function. As this function
is similar, rename it.
Note that currently the arguments are provided in differnt
order from bsearch(). That will be partly addressed next.
That is the main reason for the rename. The next commit
will swap the arguments, so do a rename first to get a compilation
error when backporting a patch that uses the changed API.
It is allowed to have a connection with empty connection.slave-type
and a NMSettingBondPort; the property will be set automatically during
normalization if a master is set, otherwise the setting will be removed.
With this change, it becomes possible to remove a port from a bond
from nmcli, turning it into a non-slave connection. Before, this used
to fail with:
$ nmcli connection add type ethernet ifname test con-name test+ connection.master bond0 connection.slave-type bond
$ nmcli connection modify test+ connection.master '' connection.slave-type ''
Error: Failed to modify connection 'test+': connection.slave-type: A connection with a 'bond-port' setting must have the slave-type set to 'bond'
https://bugzilla.redhat.com/show_bug.cgi?id=2126262https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1382
Fixes: 9958510f28 ('bond: add support of queue_id of bond port')
Bond option values are just strings, however, some of them get
validated to be numbers, etc.
We also have effectively boolean values, like "use-carrier". Internally,
this is not validates as a boolean (_nm_utils_ascii_str_to_bool()) but
instead is an integer of either "0" or "1".
Add a helper function_nm_setting_bond_opt_value_as_intbool() to access
and parse such values.
The bond setting does some minimal validation of the options.
At least for those number typed values, it validates that the
string can be interpreted as a number and is within a certain range.
Add nm_assert() checks to our opt_value_u$SIZE() functions, that the
requested option is validated to be in a range which is sufficiently
narrow to be converted to the requested type. If that were not the case,
we would need some special handling (or question whether the option should
be retrieved as this type).
These variants provide additional nm_assert() checks, and are thus
preferable.
Note that we cannot just blindly replace &g_array_index() with
&nm_g_array_index(), because the latter would not allow getting a
pointer at index [arr->len]. That might be a valid (though uncommon)
usecase. The correct replacement of &g_array_index() is thus
nm_g_array_index_p().
I checked the code manually and replaced uses of nm_g_array_index_p()
with &nm_g_array_index(), if that was a safe thing to do. The latter
seems preferable, because it is familar to &g_array_index().
When serializing setting properties to GVariant/D-Bus, we usually
omit values that are set to the default. That is done by libnm(-core),
so it happens both on the daemon and client side. That might be
useful to avoid a large number of properties on D-Bus.
Before changing the default value for "ipv6.addr-gen-mode" ([1]), we
would not serialize the previous default value ("stable-privacy").
Now we would serialize the new default value ("default). This change
causes problems.
Scenario 1: have a profile in the daemon with "ipv6.addr-gen-mode=stable-privacy",
have an older daemon version before [1] and a newer client after [1]. Result:
The daemon exposes the profile on D-Bus without the addr-gen-mode
field (because it's the default). To the client, that is interpreted
differently, as "ipv6.addr-gen-mode=default". This is already somewhat
a problem.
More severe is when modifying the profile, the client would now serialize
the value "default" on D-Bus, which the older daemon rejects as invalid.
That means, you could not modify the profile, unless also resetting
addr-gen-mode to "stable-privacy" or "eui64".
You can imagine other scenarios where either the daemon or the client is
before/after change [1] and the addr-gen-mode is set to either "default"
or "stable-privacy". Depending on what scenario you look, that can either be
good or bad.
Scenario 1 is pretty bad, because it means `dnf upgrade NetworkManager
&& nmcli connection modify ...` will fail (if the daemon was not
restated). So try to fix Scenario 1, by also not serializing the new
default value on D-Bus. Of course, some of the scenarios will get
different problems, by exacerbating one side misunderstanding the actually
set value and interpreting a missing value on D-Bus wrongly. But those
problems are likely less severe.
In case both client and daemon are older/newer than [1], it doesn't
matter either way. The problem happens with different version and is
caused by a change of the default value.
[1] e6a33c04ebhttps://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1372
Add deprecation tags to "subject-match" and "phase2-subject-match"
properties and adjust the documentation slightly.
They've been deprecated since commit 64b76ba906 ('libnm-core: add
domain-suffix-match properties to NMSetting8021x').
The parser is reworked, and this line could be wrongly parsed
because it starts with " * value:" which could be misinterpreted
as a tag. It actually won't be parsed wrongly and is not parsed
wrongly now. Still, avoid this potential ambiguity by breaking
the line differently.
Add option to set ofport_request when configuring ovs interface. When
connection with ofport_request configured is activated ovsdb will first
try to activated on the port set by ofport_request.
It is useful to modify the UUID in offline mode. Otherwise, it's
cumbersome to clone a profile, because the cloned profile will
have the same UUID (and NetworkManager cannot load them both
at the same time).
umask 077
nmcli --offline connection modify \
connection.id profile2 \
connection.uuid new \
< /etc/NetworkManager/system-connections/profile1.nmconnection \
> /etc/NetworkManager/system-connections/profile2.nmconnection \
The doctext doesn't actually work for `man nm-settings-nmcli`. The
generation of our docs is still an incomprehensible mess that needs
fixing.
nm_utils_enum_to_str() can print flags, that is, combinations of
powers of two integers.
It also supports nicks, for certain flags.
When we have a nick for value zero, then that requires special
handling. Otherwise, that zero nick will always show up in the
string representation, although, it should only be used if the
enum value is exactly zero.
1) The "enabled-on-global-iface" flag was odd. Instead, have only
and "enabled" flag and skip (by default) endpoints on interface
that have no default route. With the new flag "also-without-default-route",
this can be overruled. So previous "enabled-on-global-default" now is
the same as "enabled", and "enabled" from before behaves now like
"enabled,also-without-default-route".
2) What was also odd, as that the fallback default value for the flags
depends on "/proc/sys/net/mptcp/enabled". There was not one fixed
fallback default, instead the used fallback value was either
"enabled-on-global-iface,subflow" or "disabled".
Usually that is not a problem (e.g. the default value for
"ipv6.ip6-privacy" also depends on use_tempaddr sysctl). In this case
it is a problem, because the mptcp-flags (for better or worse) encode
different things at the same time.
Consider that the mptcp-flags can also have their default configured in
"NetworkManager.conf", a user who wants to switch the address flags
could previously do:
[connection.mptcp]
connection.mptcp-flags=0x32 # enabled-on-global-iface,signal,subflow
but then the global toggle "/proc/sys/net/mptcp/enabled" was no longer
honored. That means, MPTCP handling was always on, even if the sysctl was
disabled. Now, "enabled" means that it's only enabled if the sysctl
is enabled too. Now the user could write to "NetworkManager.conf"
[connection.mptcp]
connection.mptcp-flags=0x32 # enabled,signal,subflow
and MPTCP handling would still be disabled unless the sysctl
is enabled.
There is now also a new flag "also-without-sysctl", so if you want
to really enable MPTCP handling regardless of the sysctl, you can.
The point of that might be, that we still can configure endpoints,
even if kernel won't do anything with them. Then you could just flip
the sysctl, and it would start working (as NetworkManager configured
the endpoints already).
Fixes: eb083eece5 ('all: add NMMptcpFlags and connection.mptcp-flags property')
- name things related to `in_addr_t`, `struct in6_addr`, `NMIPAddr` as
`nm_ip4_addr_*()`, `nm_ip6_addr_*()`, `nm_ip_addr_*()`, respectively.
- we have a wrapper `nm_inet_ntop()` for `inet_ntop()`. This name
of our wrapper is chosen to be familiar with the libc underlying
function. With this, also name functions that are about string
representations of addresses `nm_inet_*()`, `nm_inet4_*()`,
`nm_inet6_*()`. For example, `nm_inet_parse_str()`,
`nm_inet_is_normalized()`.
<<<<
R() {
git grep -l "$1" | xargs sed -i "s/\<$1\>/$2/g"
}
R NM_CMP_DIRECT_IN4ADDR_SAME_PREFIX NM_CMP_DIRECT_IP4_ADDR_SAME_PREFIX
R NM_CMP_DIRECT_IN6ADDR_SAME_PREFIX NM_CMP_DIRECT_IP6_ADDR_SAME_PREFIX
R NM_UTILS_INET_ADDRSTRLEN NM_INET_ADDRSTRLEN
R _nm_utils_inet4_ntop nm_inet4_ntop
R _nm_utils_inet6_ntop nm_inet6_ntop
R _nm_utils_ip4_get_default_prefix nm_ip4_addr_get_default_prefix
R _nm_utils_ip4_get_default_prefix0 nm_ip4_addr_get_default_prefix0
R _nm_utils_ip4_netmask_to_prefix nm_ip4_addr_netmask_to_prefix
R _nm_utils_ip4_prefix_to_netmask nm_ip4_addr_netmask_from_prefix
R nm_utils_inet4_ntop_dup nm_inet4_ntop_dup
R nm_utils_inet6_ntop_dup nm_inet6_ntop_dup
R nm_utils_inet_ntop nm_inet_ntop
R nm_utils_inet_ntop_dup nm_inet_ntop_dup
R nm_utils_ip4_address_clear_host_address nm_ip4_addr_clear_host_address
R nm_utils_ip4_address_is_link_local nm_ip4_addr_is_link_local
R nm_utils_ip4_address_is_loopback nm_ip4_addr_is_loopback
R nm_utils_ip4_address_is_zeronet nm_ip4_addr_is_zeronet
R nm_utils_ip4_address_same_prefix nm_ip4_addr_same_prefix
R nm_utils_ip4_address_same_prefix_cmp nm_ip4_addr_same_prefix_cmp
R nm_utils_ip6_address_clear_host_address nm_ip6_addr_clear_host_address
R nm_utils_ip6_address_same_prefix nm_ip6_addr_same_prefix
R nm_utils_ip6_address_same_prefix_cmp nm_ip6_addr_same_prefix_cmp
R nm_utils_ip6_is_ula nm_ip6_addr_is_ula
R nm_utils_ip_address_same_prefix nm_ip_addr_same_prefix
R nm_utils_ip_address_same_prefix_cmp nm_ip_addr_same_prefix_cmp
R nm_utils_ip_is_site_local nm_ip_addr_is_site_local
R nm_utils_ipaddr_is_normalized nm_inet_is_normalized
R nm_utils_ipaddr_is_valid nm_inet_is_valid
R nm_utils_ipx_address_clear_host_address nm_ip_addr_clear_host_address
R nm_utils_parse_inaddr nm_inet_parse_str
R nm_utils_parse_inaddr_bin nm_inet_parse_bin
R nm_utils_parse_inaddr_bin_full nm_inet_parse_bin_full
R nm_utils_parse_inaddr_prefix nm_inet_parse_with_prefix_str
R nm_utils_parse_inaddr_prefix_bin nm_inet_parse_with_prefix_bin
R test_nm_utils_ip6_address_same_prefix test_nm_ip_addr_same_prefix
./contrib/scripts/nm-code-format.sh -F
The strength of CList is of course to use it as a stack of queue,
and only append/remove from the front/tail.
However, since this is an intrusive list, it can also be useful to
just use it to track elements, and -- when necessary -- sort them
via c_list_sort().
If we have a sorted list, we might want to insert a new element
honoring the sort order. This function achieves that.
It can be useful to choose a different "ipv6.addr-gen-mode". And it can be
useful to override the default for a set of profiles.
For example, in cloud or in a data center, stable-privacy might not be
the best choice. Add a mechanism to override the default via global defaults
in NetworkManager.conf:
# /etc/NetworkManager/conf.d/90-ipv6-addr-gen-mode-override.conf
[connection-90-ipv6-addr-gen-mode-override]
match-device=type:ethernet
ipv6.addr-gen-mode=0
"ipv6.addr-gen-mode" is a special property, because its default depends on
the component that configures the profile.
- when read from disk (keyfile and ifcfg-rh), a missing addr-gen-mode
key means to default to "eui64".
- when configured via D-Bus, a missing addr-gen-mode property means to
default to "stable-privacy".
- libnm's ip6-config::addr-gen-mode property defaults to
"stable-privacy".
- when some tool creates a profile, they either can explicitly
set the mode, or they get the default of the underlying mechanisms
above.
- nm-initrd-generator explicitly sets "eui64" for profiles it creates.
- nmcli doesn' explicitly set it, but inherits the default form
libnm's ip6-config::addr-gen-mode.
- when NM creates a auto-default-connection for ethernet ("Wired connection 1"),
it inherits the default from libnm's ip6-config::addr-gen-mode.
Global connection defaults only take effect when the per-profile
value is set to a special default/unset value. To account for the
different cases above, we add two such special values: "default" and
"default-or-eui64". That's something we didn't do before, but it seams
useful and easy to understand.
Also, this neatly expresses the current behaviors we already have. E.g.
if you don't specify the "addr-gen-mode" in a keyfile, "default-or-eui64"
is a pretty clear thing.
Note that usually we cannot change default values, in particular not for
libnm's properties. That is because we don't serialize the default
values to D-Bus/keyfile, so if we change the default, we change
behavior. Here we change from "stable-privacy" to "default" and
from "eui64" to "default-or-eui64". That means, the user only experiences
a change in behavior, if they have a ".conf" file that overrides the default.
https://bugzilla.redhat.com/show_bug.cgi?id=1743161https://bugzilla.redhat.com/show_bug.cgi?id=2082682
See-also: https://github.com/coreos/fedora-coreos-tracker/issues/907https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1213
We have two variants of the function: nm_utils_ip4_netmask_to_prefix()
and _nm_utils_ip4_netmask_to_prefix(). The former only exists because it
is public API in libnm. Internally, only use the latter.
