This makes the code more generic, where _match_section_infos_lookup()
accepts a match_data argument. Previously, it supported two hard-coded
approaches (from-device, from-platform).
Note that _match_section_infos_lookup() still accepts either a
"match_data" or a "device" argument. It might be nicer, if
_match_section_infos_lookup() would only accept "match_data". If a
caller wants lookup by "device", they would need to call
nm_match_spec_device_data_init_from_device() first. However, it's done
this way with a separate "device" argument, because often we don't have
any configuration to search for, and the initialization of the
match-data can be saved. So for the common case where we want to lookup
by "device", we initialize the "match-data" lazy and on demand.
Struct allow named arguments, which seems easier to maintain instead of
a function with many arguments. Also, adding a new parameter does not
require changes to most of the callers.
The real advantage of this is that we encode all the search parameters
in one argument. And we can add that argument to
_match_section_infos_lookup(), alongside lookup by NMDevice or
NMPlatformLink.
All callers eventually want a boolean instead of a NMMatchSpecMatchType.
I think the NMMatchSpecMatchType enum still has value at the lower
layers, where the enum values are clearer (when reading the code). So
don't drop NMMatchSpecMatchType entirely.
However, let's add nm_match_spec_match_type_to_bool() to convert the
match-type to a boolean to avoid duplicating the code.
Arguably, a kernel link is needed for DHCP and so the interface name
univocally identifies a device (for example, the OVS interface). But
for consistency and clarity, store the device type to be used for
logging.
When logging, messages include the interface name to specify what
device they refer to. In most case the interface name is unique.
There are some devices that don't have a kernel link associated, and
their interface name is not guaranteed to be unique. This is currently
the case for OVS bridges and OVS ports. When reading a log with
duplicate interface names, it is difficult to understand what is
happening. And this is made worse by the fact that it is common
practice to assign the same name to all devices in a OVS hierarchy
(bridge, port, interface).
To make logs unambiguous, we want to print the device type together
with the name; however we don't want to *always* print the type
because in most cases it's not useful and it would consume valuable
real estate on the screen. Adopt a simple heuristic of showing the
type only for OVS devices.
This commit adds a helper function to return the device type to show
in logs, when it is needed.
This is rather bad, because if we reach the "goto again" case,
the error variable is not cleared. Subsequently passing the
error location to nm_device_reapply_finish() will trigger a glib
warning.
Fixes: 29b0420be7 ('nm-cloud-setup: set preserve-external-ip flag during reapply')
Once we start reconfiguring the system, we need to finish on all
interfaces. Otherwise, we might reconfigure some interfaces, abort
and leave the network broken. When that happens, a subsequent run
might also be unable to recover, because we are unable to reach the
HTTP meta data service.
https://bugzilla.redhat.com/show_bug.cgi?id=2207812
Fixes: 69f048bf0c ('cloud-setup: add tool for automatic IP configuration in cloud')
network-online.target should not be reached before nm-cloud-setup
completes configuring the network, which may make user service get
started before the network is fully configured.
Setting nm-cloud-setup.service as "Before=network-online.target" would
maybe have already achieved that. However, also use a pre-up dispatcher
script, so that the device activation in NetworkManager is also waiting
for nm-cloud-setup to complete.
https://bugzilla.redhat.com/show_bug.cgi?id=2151040https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1653
dnsmasq since 2.80 properly forwards all incoming queries with DO bit
set. That ensures even if the dnsmasq does not do validation, it will
always serve all DNSSEC records if the upstream server provides them.
Regardless local validation is enabled or disabled, it will always offer
all data required for validation to its clients.
But does not set AD bit on local responses unless it did the actual
validation itself.
In case users trust their connection to validating DNS server, they
would have to declare it by adding dnssec-proxy option to dnsmasq conf.d
directory. Because there is no negated no-dnssec-proxy, it cannot be
turned off. I think there is no good reason to be on for all cases and
it would be possible to enable it if still wanted. Move the decision to
the user.
That makes it conform with RFC 4035, paragraph 3.2.3.
Signed-off-by: Petr Menšík <pemensik@redhat.com>
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1639
It's not obvious, why we couldn't have a pending dever action
at that point. Maybe we cannot, but just to be explicit about it,
handle that we potentially might.
For example, we tend to schedule the timeout priv->carrier_defer_source
only from within nm_device_set_carrier() if `priv->carrier` is FALSE.
At the same time, nm_device_set_carrier() does nothing `if
(priv->carrier == carrier)`. So probably there is no problem.
However, we also set priv->carrier directly in
nm_device_set_carrier_from_platform() without clearing the timer. It's
hard to imagine whether there can be a case where we might have two
timeouts pending.
Between ppp 2.4.8 and 2.4.9, "rp-pppoe.so" was renamed to "pppoe.so" (and a
symlink created). Between 2.4.9 and 2.5.0, the symlink was dropped.
See-also: b2c36e6c0e
I guess, NetworkManager always meant to use ppp's "(rp-)pppoe.so"
plugin, and never what rp-pppoe provides.
If a user actually wants to use the plugin from rp-pppoe project, then
this is going to break. But it seams, we usually intend to use the
plugin from the ppp project.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1312
Fixes: afe80171b2 ('ppp: move ppp code to "nm-pppd-compat.c"')
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
