Commit graph

32356 commits

Author SHA1 Message Date
Thomas Haller 645a1bb0ef
core: unblock autoconnect when master profile changes
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.
2023-06-06 09:13:44 +02:00
Thomas Haller 481cf3594b
core: log when we unblock port profiles for controller change 2023-06-06 09:13:44 +02:00
Thomas Haller f373e1f860
core: factor out unblocking autoconnect for port profiles from activate_slave_connections() 2023-06-06 09:13:40 +02:00
Thomas Haller 7f9db1a290
man: minor rewording in "NetworkManager.conf" manual
Just a comma actually.
2023-06-05 20:59:39 +02:00
Thomas Haller 01ab5ce4a8
ifcfg: merge branch 'th/fix-ifcfg-infiniband-p-key'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1646
2023-06-05 10:49:55 +02:00
Thomas Haller 5e3e38f291
ifcfg: better handle non-full-membership PKEY_ID with new PKEY_ID_NM variable
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
2023-06-05 10:38:01 +02:00
Thomas Haller 0d0704eaa0
ifcfg-rh/tests: add test for infiniband profile with PKEY_ID in ifcfg format
https://bugzilla.redhat.com/show_bug.cgi?id=2209164
2023-06-05 09:06:51 +02:00
Fernando Fernandez Mancera 35eb9c30aa bridge: remove dead code from commit_option()
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
2023-06-01 12:00:19 +02:00
Thomas Haller 3707a3df80
gitlab-ci: print the used clang-format version in "check-tree" test
This is the authorative version that we shall use for formatting our code.
Print the version in the test.
2023-06-01 09:24:21 +02:00
Thomas Haller ebf3f70735
CONTRIBUTING: improve clang-format section 2023-06-01 08:45:08 +02:00
Aleksandr Melman 2923eeecb9
po: update Russian (ru) translation
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1647
2023-06-01 08:31:13 +02:00
Thomas Haller 70084f2485
gitlab-ci: update ci-templates to fix installation of debian:9 containers
Debian 9 (stretch) is end of life, and the repositories are archived. We
need to patch the containers so that `apt-get update` continues to work.
A new ci-templates version brings that.

Note that at the moment, there is still another issue for debian:9
containers. Unclear whether that can be fixed. In any case, bumping to
latest ci-templates is not wrong, and works around the first issue on
debian:9, making it possible to at least look at the second issue.

https://gitlab.freedesktop.org/freedesktop/ci-templates/-/merge_requests/175
2023-05-31 22:14:23 +02:00
Thomas Haller 5fa8d13d8d
release: bump version to 1.43.9 (development) 2023-05-31 16:51:13 +02:00
Thomas Haller cba7c0ded5
man: fix XSL for documentation after <description> element change
<description> is now an XML element, no longer an attribute. Fix the
style sheets.

Fixes: 89abede3df ('docs: rework generating property infos to use <description> element')
2023-05-31 11:27:09 +02:00
Sabri Ünal 61c6d24bfe
po: update Turkish (tr) translation
Update some strings and correct some of them

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1641
2023-05-30 10:26:22 +02:00
Thomas Haller d67d63bf14
nm-in-container: improve README.md 2023-05-30 10:20:29 +02:00
Thomas Haller b9f84d623d
merge branch 'th/infiniband-devname'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1640
2023-05-30 09:19:56 +02:00
Thomas Haller b25ddc2055
libnm: use nm_net_devname_infiniband() for virtual interface-name of infiniband
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.
2023-05-30 08:52:34 +02:00
Thomas Haller 8062137cd0
libnm-base: assert for valid interface name in nm_net_devname_infiniband() 2023-05-30 08:52:33 +02:00
Thomas Haller b8b74f4000
libnm-base: move nmp_utils_new_infiniband_name() to nm_net_devname_infiniband() 2023-05-30 08:52:17 +02:00
Thomas Haller 3d71eddf63
all: replace NMP_IFNAMSIZ with NM_IFNAMSIZ 2023-05-30 08:51:10 +02:00
Thomas Haller 3fd656ed37
std-aux: add NM_IFNAMSIZE 2023-05-30 08:51:10 +02:00
Thomas Haller 6b28c2867b
platform/tests: fix infiniband_partition_delete() in fake platform
Fixes: 9c323261ea ('platform: use nm_utils_new_infiniband_name()')
2023-05-30 08:51:01 +02:00
Fernando Fernandez Mancera 14a3b27cff merge: branch 'bg/rh2149012'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1642
2023-05-29 14:24:19 +02:00
Beniamino Galvani 9d18510437 manager: set the right reason when managing device after realize
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
2023-05-29 14:23:23 +02:00
Beniamino Galvani 8bdb53f7f8 device: add nm_device_get_manage_reason_external()
Move some code to determine the reason for managing devices to a new
function.
2023-05-29 14:23:23 +02:00
Lubomir Rintel 6adfd60630 manager: refine the find_master() logic
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
2023-05-29 12:50:16 +02:00
Thomas Haller d16ac4b774
infiniband: merge branch 'th/ifcfg-infiniband-p-key-rh2209164'
https://bugzilla.redhat.com/show_bug.cgi?id=2209164

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1637
2023-05-25 22:10:02 +02:00
Thomas Haller 82f5bff882
ifcfg-rh: adjust infiniband p-key for later normalization when writing to file 2023-05-25 22:06:49 +02:00
Thomas Haller 1009f1f11f
libnm: truncate too long interface name in nm_setting_infiniband_create_virtual_interface_name()
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.
2023-05-25 22:06:48 +02:00
Thomas Haller fa05d1c169
libnm: add nm_setting_infiniband_create_virtual_interface_name() helper 2023-05-25 14:55:47 +02:00
Thomas Haller 4610fd67e6
libnm: normalize interface-name for infiniband profiles
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.
2023-05-25 14:55:47 +02:00
Thomas Haller ea18e66ef6
libnm/docs: clarify behavior of infiniband.p-key property 2023-05-25 14:55:46 +02:00
Thomas Haller f8e5e07355
Revert "infiniband: avoid normalizing the p-key when reading from ifcfg"
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
2023-05-25 14:55:37 +02:00
Benjamin Berg d07383d3f3
wifi: fix IP address assignment by group owner
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/279

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1636
2023-05-23 22:15:42 +02:00
Thomas Haller e8744076fe
cli: fix signature of dcb_check_feature_enabled() 2023-05-23 09:25:51 +02:00
Thomas Haller 6069f75e00
cli: merge branch 'th/cli-secondaries-escape-delimiter'
https://bugzilla.redhat.com/show_bug.cgi?id=2177209

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1635
2023-05-23 09:05:38 +02:00
Thomas Haller 16e2c9f155
cli: support escaping delimiters for "connection.seconaries"
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
2023-05-22 19:18:20 +02:00
Thomas Haller 30a6742d96
cli: fix offline mode for parsing "connection.secondaries"
$ nmcli --offline connection add type ethernet con-name x connection.secondaries foo

  (process:48855): nm-CRITICAL **: 09:16:03.051: nm_client_get_connections: assertion 'NM_IS_CLIENT(client)' failed
  **
  nm:ERROR:src/nmcli/common.c:437:nmc_find_connection: assertion failed: (connections)
  Bail out! nm:ERROR:src/nmcli/common.c:437:nmc_find_connection: assertion failed: (connections)
  Aborted (core dumped)
2023-05-22 19:18:20 +02:00
Thomas Haller 1b26315a1f
cloud-setup: merge branch 'th/cloud-setup-testable'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1632
2023-05-22 17:28:18 +02:00
Thomas Haller a34ce19be7
cloud-setup: log about cancellation of provider detection
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.
2023-05-22 17:26:19 +02:00
Thomas Haller bc836c3991
cloud-setup: debug log when URL is mocked 2023-05-22 17:26:18 +02:00
Thomas Haller 4ef944d504
cloud-setup: move implementation of getters for base URL to macro
It's the same code, four times. And once it was messed up.
Move the definition of the function to a macro.
2023-05-22 17:26:18 +02:00
Thomas Haller 2cc54b2bb1
cloud-setup: allow mapping device for testing
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.
2023-05-22 17:26:18 +02:00
Thomas Haller eaa2188119
cloud-setup: refactor handling of environment variables
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.
2023-05-22 17:26:18 +02:00
Thomas Haller 62104477c3
cloud-setup: fix _aliyun_base() initializing base URL
Fixes: f3404435a9 ('cloud-setup: configure secondary ip in Aliyun cloud')
2023-05-22 17:26:18 +02:00
Thomas Haller e1f3acf3a6
test-cloud-meta-mock: allow configuring the provider that are mimicked
"test-cloud-meta-mock.py" needs to be mocked, so it only replies
to stuff that we tell it to. Except for the API token, that is
pushed by the client.

We need to be able to tell the mock whether it supports that API
or not. By default it does, but by setting "/.nmtest/providers"
we can limit that.

The DEFAULT_RESOURCE are now grouped by provider. If a path is not
explicitly mocked, we may fallback to the DEFAULT_RESOURCE, but only if
the respective "/.nmtest/providers" is enabled and if /.nmtest/allow-default"
indicates so (which they do by default).
2023-05-22 17:26:10 +02:00
Thomas Haller 4691f45bde
test-cloud-meta-mock: add and use helper to send response code 2023-05-22 17:24:55 +02:00
Thomas Haller 77fd4d5f0e
test-cloud-meta-mock: update default MAC address for test-cloud-meta-mock.py
Choose a MAC address that is easily recognizable. And where you can
see which is mac1 and mac2.
2023-05-22 17:24:55 +02:00
Thomas Haller c80076c501
gitlab-ci: add CentOS Stream 8+9 containers 2023-05-22 14:47:54 +02:00