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.
This is the same what kernel does, when the parent name is so long
that it would result in a too long overall name.
We need that the result is still a valid interface name.
NetworkManager does not support changing the interface name for
infiniband interfaces. Consequently, we verify that
"connection.interface-name" is either unset or set to the expected
"$parent.$p_key". Anything else wouldn't work anyway and is rejected as
invalid configuration. That brings problems however.
Rejecting invalid configuration seems fine at first:
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name xxx
Error: Error writing connection: connection.interface-name: interface name of software infiniband device must be 'ib0.8010' or unset (instead it is 'xxx')
However, when we modify the p-key, we also get an error message:
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name ib0.8010 |
nmcli --offline connection modify infiniband.p-key 5
Error: Error writing connection: connection.interface-name: interface name of software infiniband device must be 'ib0.0005' or unset (instead it is 'ib0.8010')
It's worse, because ifcfg-rh reader will mangle the PKEY_ID with |=0x8000 to set
the full membership flag. That means, if you add a profile like
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x0010 connection.interface-name ib0.0010
it gets written to ifcfg-rh file. Then upon reload it's invalid (as the
interface name mismatches).
There are multiple solutions for this. For example, ifcfg-rh reader could also
mangle the connection.interface-name, so that the overall result is valid. Or
we could just not validate at all, and accept any bogus interface-name.
With this patch instead we will just normalize the invalid configuration to
make it right.
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name ib0.8010 |
nmcli --offline connection modify infiniband.p-key 5
...
The downside is that this happens silently, so a user doesn't
notice that configuration is ignored:
$ nmcli --offline connection add type infiniband infiniband.parent ib0 infiniband.p-key 0x8010 connection.interface-name foo
...
interface-name=ib0.8010
This approach still seems preferable, because setting
"connection.interface-name" for infiniband profiles makes little sense,
so what we care here is to avoid problems.
If we have an override with "description-docbook:", we soon will require that
there is also an accompanying "description:", for plain uses.
The text is copied from what otherwise gets merged (it comes from the gir file).
We have several variants to get the NMSetting from an NMConnection. Some
of them are public API (nm_connection_get_setting(), nm_connection_get_setting_by_name()).
The most efficient way is lookup by NMMetaSettingType. Expose that as
internal API, so it can be used. The NMMetaSettingType is internal, but
it exists because it's a very useful enum. Allow others to make use of
it.
Also, add a static assert which prevents various wrong uses at compile
time, for example
_nm_connection_get_setting_by_metatype(connection, NM_TYPE_SETTING_CONNECTION)
Refactor and cleanup the functions to get a setting from a connection.
As the NMConnection tracks the settings in an array indexed by
NMMetaSettingType, the most direct and efficient way is to look up via
that enum.
Previously, nm_connection_get_setting_by_name() would first look up the GType
(which already involved looking up the NMMetaSettingInfo), then based on the
GType it would look up the NMMetaSettingInfo again to get the meta_type. That
is unnecessary. Directly look up the NMMetaSettingInfo, which directly
gives the meta_type.
