Unfortunately, there are several possibilities how to handle NULL and
empty arrays. Therefore we have different variants.
Clean this up, and add a way to preserve whether the array is empty
(previous variants could not distinguish that).
Functions are also renamed, so that if you backport a user of the new
API, you'll get a compiler error if this patch is missing.
Also, nm_strvarray_get_strv_notnull() no longer takes a pointer to a
"GArray*". Previously, it used that to fake an empty strv array. Now
this returns NM_STRV_EMPTY_CC().
_nm_utils_dns_option_validate() allows specifying the address family,
and filters based on that. Note that all options are valid for IPv6,
but some are not valid for IPv4.
It's not obvious, that such filtering is only performed if
"option_descs" argument is provied. Otherwise, the "ipv6" argument is
ignored.
Regardless, it's also confusing to have a boolean "ipv6". When most
callers don't want a filtering based on the address family. They
actually don't want any filtering at all, as they don't pass an
"option_descs". At the same time passing a TRUE/FALSE "ipv6" is
redundant and ignored. It should be possible, to explicitly not select
an address family (as it's ignored anyway).
Instead, make the "gboolean ipv6" argument an "int addr_family".
Selecting AF_UNSPEC means clearly to accept any address family.
Add a new "stable-ssid" mode that generates the MAC address based on the
Wi-Fi's SSID.
Note that this gives the same MAC address as setting
connection.stable-id="${NETWORK_SSID}"
wifi.cloned-mac-address="stable"
The difference is that changing the stable ID of a profile also affects
"ipv6.addr-gen-mode=stable-privacy" and other settings.
For Wi-Fi profiles, this will encode the SSID in the stable-id.
For other profiles, this encodes the connection UUID (but the SSID and
the UUID will always result in distinct stable IDs).
Also escape the SSID, so that the generated stable-id is always valid
UTF-8.
We have many properties, and we aim that they have a small set of
"types". The purpose is that we can treat similar properties (with the
same type) alike.
One type are "direct" strv properties. Those still require some
C functions, like get-length(), clear(), add(), get-at-index().
The implementation of those functions should also be similar, so that
strv properties behave similar.
For that, make use of helper functions, so that little duplicate logic
is there.
Use some new nm_strvarray_*() functions, and unify/cleanup some code.
All related to strv properties in NMSetting classes.
There are various properties related to EEE, that we might want to add
support for in the future (for example, "ethtool.eee-advertise").
Don't use up the base name "eee", instead make it "eee-enabled". All
properties should have different prefixes, and "ethtool.eee" would be a
prefix of "ethtool.eee-advertise".
Also, the #define is already called NM_ETHTOOL_OPTNAME_EEE_ENABLED. This
also should be consistent.
Rename.
Fixes: 3165d9a2de ('ethtool: introduce EEE support')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1792
glib's MIN()/MAX() will be replaced by NM_MIN()/NM_MAX().
There are however a few places where NM_MIN()/NM_MAX() cannot
be used.
Adjust those places to use NM_MIN_CONST()/NM_MAX_CONST() instead.
c_siphash_init() requires a 16 bytes array. That is cumbersome to use.
We have NM_HASH_SEED_16() macro for helping with that. It's still
cumbersome.
Most of the time, the caller just wants to pick an arbitrarily chosen,
fixed number. Add NM_HASH_SEED_16_U64() which takes a number and gives
a 16 seed array. The argument is in host endianness, but the resulting
seed array has it encoded in big endianness, to be architecture
independent.
Most profiles don't have "connection.permissions" set. Avoid resolving the
UID to the name via getpwuid() (in nm_auth_is_subject_in_acl()), until we
know that we require it.
CC src/libnm-core-impl/libnm_core_impl_la-nm-setting.lo
src/libnm-core-impl/nm-setting.c: In function '_nm_setting_class_commit':
src/libnm-core-impl/nm-setting.c:339:41: error: unused variable 'i' [-Werror=unused-variable]
339 | guint i;
| ^
src/libnm-core-impl/nm-setting.c: At top level:
src/libnm-core-impl/nm-setting.c:168:1: error: '_nm_sett_info_property_find_in_array' defined but not used [-Werror=unused-function]
168 | _nm_sett_info_property_find_in_array(const NMSettInfoProperty *properties,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Fixes: 2dd5b065a8 ('libnm: drop special casing G_TYPE_STRV from _nm_setting_class_commit()')
What we sort is very static, the names of properties in
NMSettingConnection. We know that the list contains no two identical
names. There were already assertions for that, just rework them a bit to
make the code clearer.
Previously, _nm_setting_class_commit() required that the "name" of a
NMSettInfoProperty is unset, when the property also has a "param_spec".
_nm_setting_class_commit() would then as first iterate over all
properties, and set the name.
In practice, all callers only initialize NMSettInfoProperty via
_nm_properties_override_gobj(). Now, let _nm_properties_override_gobj()
set the "name" right away.
Now _nm_setting_class_commit() will instead assert that the name is always
set, and that the caller takes care of that. That means, we have less to
do in _nm_setting_class_commit() (assertions aside).
Direct properties are automatically cleaned up by the base class
(_finalize_direct()). No need to duplicate that. The point of
the direct property implementation is to free us from this repeated
cumbersome steps (and forgetting this step without a direct property
would not be only unnecessary, but erroneous).
There are no such properties left. They now all use
_nm_setting_property_define_gprop_strv_oldstyle() and the
properties_override array already contains the properties.
This simplifies _nm_setting_class_commit() and moves logic away.
Note that most of the code in _nm_setting_class_commit() is only asserts
for consistency.
Since "properties_override" now contains all properties, it doesn't
really "override" any default and the name is bad. Anyway.
Use _nm_setting_property_define_gprop_strv_oldstyle() for all existing
(remaining) G_TYPE_STRV properties.
The benefit is that the properties_override array already lists the
property, and we don't need special hacks in _nm_setting_class_commit()
to initialize those properties.
Also, this style is discouraged. We can now easier find all properties
that should be reworked.
This will be used for adding G_TYPE_STRV properties. This is a legacy
approach, new properties should use _nm_setting_property_define_direct_strv(),
which is more efficient and where the meta-data knows more about the
strv property.
Will be used next.
For settings with many properties, pre-allocate a larger buffer via
_nm_sett_info_property_override_create_array_sized().
The buffer is larger than needed, so when we add more properties it
still works. In any case, GArray will grow automatically, so getting
this wrong is not fatal (just suboptimal).
The goal is to have the properties_overrides already pre-populated with
all properties. Then we will be able to drop special cases from
_nm_setting_class_commit().
Some Applications require to explicitly enable or disable EEE.
Therefore introduce EEE (Energy Efficient Ethernet) support with:
* ethtool.eee on/off
Unit test case included.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
Historically, the NMSetting types were in public headers. Theoretically,
that allowed users to subtype our classes. However in practice that was
impossible because they lacked required access to internal functions to
fully create an NMSetting class outside of libnm. And it also was not
useful, because you simply cannot extend libnm by subtyping a libnm
class. And supporting such a use case would be hard and limit what we can
do in libnm.
Having GObject structs in public headers also require that we don't
change it's layout. The ABI of those structs must not change, if anybody
out there was actually subclassing our GObjects.
In libnm 1.34 (commit e46d484fae ('libnm: hide NMSetting types from
public headers')) we moved the structs from headers to internal.
This would have caused a compiler error if anybody was using those
struct definitions. However, we still didn't change the ABI/layout so
that we didn't break users who relied on it (for whatever reason).
It doesn't seem there were any affected user. We waited long enough.
Change internal ABI.
No longer use g_type_class_add_private(). Instead, embed the private
structs directly (_NM_GET_PRIVATE()) or indirectly
(_NM_GET_PRIVATE_PTR()) in the object.
The main benefit is for debugging in the debugger, where we can now
easily find the private data. Previously that was so cumbersome to be
effectively impossible.
It's also the fastest possible way, since NM_SETTING_*_GET_PRIVATE()
literally resolves to "&self->_priv" (plus static asserts and
nm_assert() for type checking).
_NM_GET_PRIVATE() also propagates constness and requires that the
argument is a compatible pointer type (at compile time).
Note that g_type_class_add_private() is also deprecated in glib 2.58 and
replaced by G_ADD_PRIVATE(). For one, we still don't rely on 2.58. Also,
G_ADD_PRIVATE() is a worse solution as it supports a usecase that we
don't care for (public structs in headers). _NM_GET_PRIVATE() is still
faster, works with older glib and most importantly: is better for
debugging as you can find the private data from an object pointer.
For NMSettingIPConfig this is rather awkward, because all direct
properties require a common "klass->private_offset". This was however
the case before this change. Nothing new here. And if you ever touch
this and do something wrong, many unit tests will fail. It's almost
impossible to get wrong, albeit it can be confusing to understand.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1773
nm_strv_find_first() is useful (and used) to find the first index (if
any). I can thus also used to check for membership.
However, we also have nm_strv_contains(), which seems better for
readability, when we check for membership. Use it.
ethtool "channels" parameters can be used to configure multiple queues
for a NIC, which helps to improve performances. Until now, users had
to use dispatcher scripts to change those parameters. Introduce native
support in NetworkManager by adding the following properties:
- ethtool.channels-rx
- ethtool.channels-tx
- ethtool.channels-other
- ethtool.channels-combined
Sending a client-id is not mandatory according to RFC2131. It is
mandatory according to RFC4361 that superseedes it.
Some weird DHCP servers conforming RFC2131 can get confused and break
existing DHCP leases if they start receiving a client-id when it was not
being previously received. Users that were using other DHCP client like
dhclient, but want to use NetworkManager's internal DHCP client, can
suffer this problem.
Add "none" as accepted value in ipv4.dhcp-client-id to specify that
client-id must not be sent. Note that this is generally not recommended
unless it's explicitly needed for some reason like the explained above.
Client-id is mandatory in DHCPv6.
This commit allow to set the "none" value and properly parse it in the
NMDhcpClientConfig struct. Next commits will modify the different DHCP
plugins to honor it.
Adds a new WiFi 6GHz capability flag, NM_WIFI_DEVICE_CAP_FREQ_6GHZ,
along side the existing NM_WIFI_DEVICE_CAP_FREQ_2GHZ &
NM_WIFI_DEVICE_CAP_FREQ_5GHZ flags.
Gnome settings utilizes the 2 existing flags to present supported
bands in gnome-settings. I will be using this additional flag in
modifications there.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1739
Show all valid properties for ip-tunnel.mode, not only 2 examples.
Show constants as values suitable for user input in nmcli. That means
showing, for example, "ipip (1)" instead of "IP_TUNNEL_MODE_IPIP (1)".
Infiniband profiles can have a p-key set. Both in kernel API
("create_child" sysctl) and in NetworkManager API, that key can range
from 0x0001 to 0xFFFF (0x8000 excluded). NetworkManager does not support
renaming the interface, so kernel always assigns the interface name
"$PHYSDEV.$PKEY_ID" (with $PKEY_ID as 4 character hex digits).
Note that the highest bit in the p-key (0x8000) is the full-membership
flag. Internally, kernel only supports full-membership so when we create
for example "ib0.00c1" and "ib0.80c1" interfaces, their actually used
p-key is in both cases 0x80c1 and you can see it with `ip -d link`.
Nonetheless, kernel and NetworkManager allow to configure the p-key
without the highest bit set, and the result differs in the interface
name.
Note that initscripts' ifup-ib0 would always internally coerce the
PKEY_ID variable to have the high bit set ([1]). It also would require
that the `DEVICE=` variable is specified and matches the expected
interface name. So both these configurations are identical and valid:
DEVICE=ib0.80c1
PHYSDEV=ib0
PKEY_ID=0x80c1
and
DEVICE=ib0.80c1
PHYSDEV=ib0
PKEY_ID=0x00c1
Historically, NetworkManager would also implement the same restrictions
([2], [3], [4]). That meant, not all valid NetworkManager infiniband
profiles could be expressed as ifcfg file. For example, NetworkManager
allows to have "connection.interface-name" (`DEVICE=`) unset (which
ifup-ib and ifcfg reader did not allow). Also, NetworkManager would
allow configuring a "infiniband.p-key" without full membership flag, and
the reader would mangle that.
This caused various problems to the point that when you configure an
infiniband.p-key with a non-full-membership key, the ifcfg-rh written by
NetworkManager was invalid. Either, you could leave
"connection.interface-name" unset, but then the reader would complain
about missing `DEVICE=`. Or, we could write `DEVICE=ib0.00c1;
PKEY_ID=0x00c1`, which was invalid as we expected `DEVICE=ib0.80c1`.
This was addressed by rhbz 2122703 ([5]). The fix was to
- not require a `DEVICE=` ([6]).
- don't mangle the `PKEY_ID=` in the reader ([7]).
which happened in 1.41.2 and 1.40.2 (rhel-8.8).
With this change, we could persist any valid infiniband profile to ifcfg
format. We also could read back any valid ifcfg file that NetworkManager
would have written in the past (note that it could not write valid ifcfg
files previously, if the p-key didn't have the full-membership key set).
The problem is, that users were used to edit ifcfg files by hand, and
users would have files with:
DEVICE=ib0.80c1
PHYSDEV=ib0
PKEY_ID=0x00c1
This files had worked before, but now failed to verify as we would
expect `DEVICE=ib0.00c1`. Also, there was a change in behavior that
PKEY_ID is now interpreted without the high bit set. This is reported as
rhbz 2209164 ([8]).
We will do several things to fix that:
1) we now normalize the "connection.interface-name" to be valid. It was
not useful to set it anyway, as it was redundant. Complaining about a
redundant setting, which makes little sense to configure, is not useful.
This is done by [9].
2) we now again treat PKEY_ID= as if it had 0x8000 flag set. This was done by
[10].
With step 1) and 2), we are able to read any existing ifcfg files out
there in the way we did before 1.41.2.
There is however one piece missing. When we now create a profile using
nmcli/libnm/D-Bus, which has a non-full-membership p-key, then the
profile gets mangled in the process.
If the user uses NetworkManager API to configure an interface and
chooses a non-full-membership p-key, then this should work the same as
with keyfile plugin (or on rhel-9, where keyfile is the default). Note
that before 1.41.2 it didn't work at all, when the user used ifcfg-rh
backend. Likely(?) there are no users who rely on creating such a profile
with nmcli/libnm/D-Bus and expect to automatically have the p-key
normalized. That didn't work before 1.41.2 and didn't behave that way
between 1.41.2 and now.
This patch fixes that by introducing a new key PKEY_ID_NM= for holding
the real p-key. Now ifcfg backend is consistent with handling infiniband
profiles, and old, hand-written ifcfg files still work as before.
There is of course change in behavior, that ifcfg files between 1.41.2
and now were interpreted differently. But that is bug 2209164 ([8]) and
what we fix here.
For now strong reasons, we keep writing the PKEY_ID to file too. It's
redundant, but that is what a human might expect there.
[1] 05333c3602/f/rdma.ifup-ib (_75)
[2] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/1.40.0/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c#L5386
[3] cb5606cf1c (a7a78fccb2c8c945fd09038656ae734c1b0349ab_3493_3532)
[4] cb5606cf1c (a7a78fccb2c8c945fd09038656ae734c1b0349ab_3493_3506)
[5] https://bugzilla.redhat.com/show_bug.cgi?id=2122703
[6] 4c32dd9d25
[7] a4fe16a426
[8] https://bugzilla.redhat.com/show_bug.cgi?id=2209164
[9] 4610fd67e6
[10] f8e5e07355
In nm_setting_infiniband_get_virtual_interface_name(), no longer try to
detect whether the cached value is still up to date. Instead, as we now
have a fix sized buffer for the name, just always generate the name on
every call. It's simpler.
