Move the warning about the presence of ifcfg-rh profiles from the
plugin to NMSettings. In this way, it will be easier to implement the
migration option in the next commit.
The default behavior is not to rename profiles. I guess, that makes
sense, as renaming a file when changing the "connection.id" could break
users who rely on the name.
My use case is the following. When I connect a Wi-Fi hotspot I use
`nmcli device wifi connect $SSID`, which -- as expected -- persists the
profile to "/etc/NetworkManager/system-connections/$SSID.nmconnection".
Later, I always update the profile's name to "w_$SSID" so I can see on
the name that this is wireless profile. I also want the filename to
reflect that change of name.
Add a configuration option for that. All the infrastructure
("force_rename" parameter) already exists.
There was already a force_rename argument to nms_keyfile_writer_connection(), which
-- if TRUE -- means to always rename the file, if it exists.
What we also want, is to follow the change of a connection.id. So we don't want
to force a rename, if we already use the preferred name, but we also want to rename
otherwise.
Extend the boolean "force_rename" argument to a NMTernary, where NM_TERNARY_DEFAULT
now means to follow the preferred name.
"force_rename" parameter was not used previously, but it also was broken.
Fix it. We need to create a new NMSettingsStorage instance when the filename
changes, as the storage's filename is immutable.
No bad effects so far, it was unused.
But as it wasn't used, also no longer set the update_reason flag
NM_SETTINGS_CONNECTION_UPDATE_REASON_FORCE_RENAME. We didn't have the
force-rename behavior so far. This makes the flag totally unused, and
maybe should be dropped. It's kept for now, if only to show what could
be done.
We have some interal code that is only used to expose functionality for
the tests. Those functions should be easily distinguishable from code
that is used by the "real" code. Give a "nmtst" prefix. Rename
nms_keyfile_writer_test_connection() to nmtst_keyfile_writer_test_connection().
Fail to save a connection with a 'link' setting instead of just
ignoring it. Now:
$ nmcli connection add type ethernet ifname foobar
Connection 'ethernet-foobar' (c3f6f067-e1d5-4bb1-8d67-e09109253a79) successfully added.
$ nmcli connection modify ethernet-foobar link.tx-queue-length 1234
Error: Failed to modify connection 'ethernet-foobar': failed to update connection: The ifcfg-rh plugin doesn't support setting 'link'. If you are modifying an existing connection profile saved in ifcfg-rh format, please migrate the connection to keyfile using 'nmcli connection migrate c3f6f067-e1d5-4bb1-8d67-e09109253a79' or via the Update2() D-Bus API and try again.
$ nmcli connection migrate c3f6f067-e1d5-4bb1-8d67-e09109253a79
Connection 'ethernet-foobar' (c3f6f067-e1d5-4bb1-8d67-e09109253a79) successfully migrated.
$ nmcli connection modify ethernet-foobar link.tx-queue-length 1234
$
Fixes: 39bfcf7aab ('all: add "link" setting')
The ifcfg-rh plugin is now deprecated and in bugfixes-only mode. When
users try to set a property that is not supported by the plugin, we
need to report an error.
Add an helper function to set such error. Also, introduce a new error
code so that the situation can be detected and dealt with
programmatically.
We want to guard against concurrent modifications of profiles. We cannot
lock profiles, so what we instead do is expose (and bump) a version ID.
The user can check the version ID, plan ahead what to do, and tell
NetworkManager to only make the modification if no concurrent
modification was done. The conflict can be detected via the version ID.
The Update2() D-Bus call gets a parameter to only allow the request if
the version ID still matches.
nmcli should use this, but it is quite some effort to retry upon
concurrent modification. This is still to do.
Note that the user might make a decision that is based on multiple
profiles. As the new version-id is only per-profile, we cannot guard
against such inter-profile modifications. What would be needed, is a
UpdateMany() call, where we could modify multiple profiles at once, and
the action only takes effect if all version IDs show no concurrent
modification. That's not done yet, and maybe never will be.
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.
Historically, initscripts' ifup-ib would set the highest bit of
PKEY_ID=. That changed and needs to be restored.
Note that it probably makes little sense to ever configure p-keys
without the highest bit set, because that flag indicates full membership
and kernel will automatically add it. At least, kernel will add the flag
for the p-key, but not for the automatically chosen interface name.
Meaning, writing 0x00f0 to create_child sysctl, results in an interface
"$parent.00f0", but `ip -d link` shows pkey 0x80f0.
As NetworkManager otherwise supports p-keys without the highest bit set,
and since that high bit is honored for the interface name, we cannot
just always add the high bit. NetworkManager always assuming the highest
bit is set, would change the interface names of existing configuration.
With this revert, when a user configures a small p-key and the profile
is stored in ifcfg-rh format, the settings backend will automatically
mangle the profile and set 0x8000. That is different from when the
profile is stored in keyfile format. Since using small p-keys is
probably an odd case, we don't try to workaround that any other way
(like that ifcfg format could represent the orignal value of the profile
and not doing such mangling, or to add the high bit throughout
NetworkManager to the p-key). It's an inconsistency, but given the
existing behaviors it seems best to stick (revert) to it.
This reverts commit a4fe16a426.
Affected versions were 1.42.2+ and 1.40.2+.
See-also: 05333c3602/f/rdma.ifup-ib (_75)https://bugzilla.redhat.com/show_bug.cgi?id=2209164
For efficiently and conveniently lookup an NMSetting from the
NMConnection inside the NMSettingsConnection.
Note that this uses the NMMetaSettingType as lookup key. That is a novel
approach, compared to lookup by name (nm_connection_get_setting_by_name())
or GType (nm_connection_get_setting()).
Using the NMMetaSettingType enum is however faster, because it does not
require resolving the name/GType first. This is perfecly fine internal API,
we should use it.
Cleanup logging to always print a "block-autoconnect:" prefix to related
lines. Also, make sure that everywhere where the state changes, a line
gets logged. Also, for devconf data print both the interface and the
profile.
We only have a few blocked reasons. Some of them can be only set on the
devcon data, and some only on the settings connection. Assert that we
don't mix that up.
Add per port priority support for bond active port re-selection during
failover. A higher number means a higher priority in selection. The
primary port still has the highest priority. This option is only
compatible with active-backup, balance-tlb and balance-alb modes.
"wifi.seen-bssids" looks like a regular property, but it is not. Unlike
almost all other properties, it does not contain user configuration,
rather it gets filled by the daemon.
The values are thus stored in "/var/lib/NetworkManager/seen-bssids"
file, and the daemon maintains the values separately from the profile.
Only before exporting the profile on D-Bus, the value gets merged (see
NM_SETTINGS_CONNECTION_GET_PRIVATE(self)->>getsettings_cached and
nm_connection_to_dbus_full().
Hence, looking at nm_setting_wireless_get_num_seen_bssids() is not
working. Fix that.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1253
Fixes: 0f3203338c ('wifi: roam aggressively if we on a multi-AP network')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1577
We are passing to the plugin only 'sett_flags', which is the bitmask
of flags to change and works together with 'sett_mask'; however,
plugins interpret that value as the new flags value. The result is
that if there is no change needed (0/0), the existing flags are lost.
Simple reproducer:
ip link add dummy1 type dummy
ip link set dummy1 up
ip addr add dev dummy1 fd01::12/64
sleep 1
# now, a external connection is created by NM
echo "BEFORE:"
cat /run/NetworkManager/system-connections/dummy1.nmconnection | grep "nm-generated\|volatile\|external"
# just add a new address to the interface to make it lose
# the external flag
ip addr add dev dummy1 172.25.42.1/24
sleep 1
echo "AFTER:"
cat /run/NetworkManager/system-connections/dummy1.nmconnection | grep "nm-generated\|volatile\|external"
Output:
BEFORE:
nm-generated=true
volatile=true
external=true
AFTER:
Fixes: d35d3c468a ('settings: rework tracking settings connections and settings plugins')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1548
Autoconnect retries are not being tracked by connection anymore. Now it
is tracked per Device x Connection. In addition, autoconnect might be
blocked for the connection due to no secrets or user requested.
All the properties tracking the retries and blocked time were move to
DevConData and the functions to manipulate them aswell. In NMPolicy the
logic didn't change very much. Instead of looking into the connection
when the device failed activation it looks for DevConData.
This setting allows the user to remove the local route rule that is
autogenerated for both IPv4 and IPv6. By default, NetworkManager won't
touch the local route rule.
In kernel, the valid range for the weight is 1-256 (on netlink this is
expressed as u8 in rtnh_hops, ranging 0-255).
We need an additional value, to represent
- unset weight, for non-ECMP routes in kernel.
- in libnm API, to express routes that should not be merged as ECMP
routes (the default).
Extend the type in NMPlatformIP4Route.weight to u16, and fix the code
for the special handling of the numeric range.
Also the libnm API needs to change. Modify the type of the attribute on
D-Bus from "b" to "u", to use a 32 bit integer. We use 32 bit, because
we already have common code to handle 32 bit unsigned integers, despite
only requiring 257 values. It seems better to stick to a few data types
(u32) instead of introducing more, only because the range is limited.
Co-Authored-By: Fernando Fernandez Mancera <ffmancera@riseup.net>
Fixes: 1bbdecf5e1 ('platform: manage ECMP routes')
G_TYPE_CHECK_INSTANCE_CAST() can trigger a "-Wcast-align":
src/core/devices/nm-device-macvlan.c: In function 'parent_changed_notify':
/usr/include/glib-2.0/gobject/gtype.h:2421:42: error: cast increases required alignment of target type [-Werror=cast-align]
2421 | # define _G_TYPE_CIC(ip, gt, ct) ((ct*) ip)
| ^
/usr/include/glib-2.0/gobject/gtype.h:501:66: note: in expansion of macro '_G_TYPE_CIC'
501 | #define G_TYPE_CHECK_INSTANCE_CAST(instance, g_type, c_type) (_G_TYPE_CIC ((instance), (g_type), c_type))
| ^~~~~~~~~~~
src/core/devices/nm-device-macvlan.h:13:6: note: in expansion of macro 'G_TYPE_CHECK_INSTANCE_CAST'
13 | (G_TYPE_CHECK_INSTANCE_CAST((obj), NM_TYPE_DEVICE_MACVLAN, NMDeviceMacvlan))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
Avoid that by using _NM_G_TYPE_CHECK_INSTANCE_CAST().
This can only be done for our internal usages. The public headers
of libnm are not changed.
This is the version shipped in Fedora 37. As Fedora 37 is now out, the
core developers switch to it. Our gitlab-ci will also use that as base
image for the check-{patch.tree} tests and to generate the pages. There
is a need that everybody agrees on which clang-format version to use,
and that version should be the one of the currently used Fedora release.
Also update the used Fedora image in "contrib/scripts/nm-code-format-container.sh"
script.
The gitlab-ci still needs update in the following commit. The change
in isolation will break the "check-tree" test.
If there were any pause options and any non-pause options,
the created setting was invalid.
I don't think it's reasonably possible to parse the broken settings.
So there is no workaround trying to read the existing broken settings
from disk. Luckily, the broken setting was just silently ignored by
the parser, so you simply could not persist certain settings.
https://bugzilla.redhat.com/show_bug.cgi?id=2134569
Fixes: 652ddca04c ('ethtool: Introducing PAUSE support')
