When a port cannot activate because the controller is not ready, it gets
blocked from autoconnect (see commit 725fed01cf ('policy: block
connection from autoconnect in case of failed dependency')).
Later, when the master activates we call activate_slave_connections()
(see commit 32efb87d4d ('core: unblock failed connections when the
master is available')), which unblocks those port profiles so they can
autoconnect.
However, imagine you add a port profile with autoconnect enabled. The
profile tries to autoconnect, finds no master and gets blocked. Then,
add the controller profile with autoconnect disabled. The controller is
not autoactivating, not calling activate_slave_connections() and the
profiles stay down.
Fix that by unblocking autoconnect of the ports when the controller
profile changes.
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
commit_option() was used in the past to set both bridge and bridge port
options using sysfs. Currently it is only used for bridge port options.
This patch removes the dead code for bridge options and unify it on
commit_port_options(). This is simplifying the work needed to support
bridge port option through netlink.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1643
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.
When managing a device after it is realized, we previously always set
the NOW_MANAGED reason, that makes the device fully-managed.
This works based on the assumption that initially an external device
has unmanaged flag EXTERNAL_DOWN set, and therefore the device stays
unmanaged during realization.
It is possible that an external device appears already with addresses
(or attached to a controller); we need to set reason
CONNECTION_ASSUMED if it's an external device, so that we don't set
sys-iface-state=managed.
Reproducer:
ip link add br1 type bridge
killall -STOP NetworkManager
ip link add dummy1 type dummy
ip link set dummy1 master br1
ip link set dummy1 up
sleep .5
killall -CONT NetworkManager
After this, dummy1 is fully managed by NM while it shouldn't.
https://bugzilla.redhat.com/show_bug.cgi?id=2149012
If there are ports that refer the controllers by a device name, and
multiple autoconnectable controller devices of that name, the
situation gets messy. In particular, the autoconnect logic can start
activating a device with a higher autoconnect priority, but then a port
can override it by bringing up another controller of possibly lower
autoconnect priority.
Let's
1.) prefer controller connections with higher autoconnect priority
and
2.) prefer connections that are already active so that we don't
disrupt existing activation.
https://bugzilla.redhat.com/show_bug.cgi?id=2121451
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.
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
When a fixed address is assigned by the P2P group owner, then the code
would set the IPv4 configuration method to DISABLED internally. However,
this causes issues, because it means that IPv4 is considered to not have
come up internally which can cause the connection to later time out even
though it was configured properly.
As such, map this method to MANUAL in this case. The AUTO mapping
becomes then:
* MANUAL: If the remote part is the GO and assigned an IP address
* DHCP: If the remote part is the GO and did not assign an address
* SHARED: If we are the GO
This fixes an issue where the connection established by GNOME Network
Displays would fail once IPv6 configuration also times out.
See-also: https://gitlab.gnome.org/GNOME/gnome-network-displays/-/issues/279https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1636
It's obviously a change in behavior. Now accept backslash for escaping
the whitespace+comma separators when setting "connection.secondaries".
Before:
$ nmcli --offline connection add type ethernet con-name x connection.secondaries 'a b'
Error: failed to modify connection.secondaries: the value 'a' is not a valid UUID.
$ nmcli --offline connection add type ethernet con-name x connection.secondaries 'a\ b'
Error: failed to modify connection.secondaries: the value 'a\' is not a valid UUID.
After:
$ nmcli --offline connection add type ethernet con-name x connection.secondaries 'a\ b'
Error: failed to modify connection.secondaries: the value 'a b' is not a valid UUID.
https://bugzilla.redhat.com/show_bug.cgi?id=2177209
Otherwise, the logging output is confusing as it's not clear what happened about
the provider detection.
Also, don't return from _provider_detect() unless all pending callbacks
returned. It is ugly to leave callbacks pending, because they will be
later dispatched when the caller iterates the GMainContext further.
Instead, cancel pending operations bug keep running until we processed
all cancellation callbacks.
In production systems, the associate an interface by their permanate MAC address.
For testing and CI, we may want to hook that. That allows for example to run
nm-cloud-setup on a veth interface, which doesn't have a permanent MAC address.
This is only for testing.
Previously, there was NMCS_ENV_VARIABLE() macro. That macro did nothing,
it merely acted as something to grep for, when searching the source for
which environment variables nm-cloud-setup honors. That is an
interesting thing to know, because nm-cloud-setup is configured via
environment variables.
Change that. Instead add a define for each environment variable. You can
now instead grep for "NMCS_ENV_" to find them all.
"gen-metadata-nm-settings-nmcli" previously printed the <description>.
But that tag is not very useful for further processing.
For the most part it itself comes from "src/libnmc-setting/settings-docs.h",
which is generated (but lost formatting information already to be
suitable for where it's used).
Some parts are original texts from "src/libnmc-setting/nm-meta-setting-desc.c",
like TEAM_DESCRIBE_MESSAGE. However those text are also not really suitable
for any other purpose.
Rename the tag, so that the tools that process "gen-metadata-nm-settings-nmcli.xml"
don't use it.
The file "gen-metadata-nm-settings-nmcli.xml" is currently only used to
generate "man/nm-settings-docs-nmcli.xml", and that file slightly
changes with this patch. However, the manual page which is generated by
"man/nm-settings-docs-nmcli.xml" does not change.
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).
This is the version shipped in Fedora 38. As Fedora 38 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. This change
in isolation will break the "check-tree" test.
In constructed(), NMDevice starts watching the D-Bus name owner or
monitoring the unix socket, and so it is always aware if teamd is
running. When it is, NMDevice connects to it and initializes
priv->tdc.
It is not useful to try to connect to teamd in update_connection()
because warnings will be generated by NM and by libteam if teamd is
not running. As explained above the connection is always initialized
when teamd is available, and so we can just check priv->tdc.
Fixes: ab586236e3 ('core: implement update_connection() for Team')
https://bugzilla.redhat.com/show_bug.cgi?id=2182029https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1631
This ABI was backported all the way to 1.42.8 and 1.40.20 and to rhel-8.9.
Move the ABI to a separate symbol version, which we have in all those
versions.
With the unit test framework, we define special methods, like setUp()
and test_*(). This is documented, but not obvious.
Previously, TestNmClient was the base class for our tests classes, and
it provided some functionality (and state). It was utterly confusing how
pieces fit together.
Instead, move the state to a new class NMTestContext(). That contains
most of the code from TestNmClient. Drop TestNmClient and let the test
classes directly descend from unittest.TestCase.
The difference is, when you now look at a certain test (test_001()), you
can easier understand which code runs when. First, the test class has a
setUp() method which runs, but that method is now trivial without extra
context. Second, there is the @nm_test attribute that wraps the
function. But that's it. It's all at one place, and we delegate instead
of inherit.
Currently if the IPv6 link-local address is removed after it passed
DAD, NetworkManager tries to generate a new link-local address. If
this fails, which is always the case for EUI64, ipv6ll is considered
as failed and the connection can go down (depending on may-fail).
This is particularly bad for virtual interfaces because if somebody
removes the link-local address, the activation can fail and destroy
the interface, breaking all services that require it. Also, it's a
change in behavior introduced in 1.36.0.
It seems that a better approach here is to re-add the address that was
removed externally.
Fixes: aa070fb821 ('core: add NML3IPv6LL helper')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1622
