Commit graph

180 commits

Author SHA1 Message Date
Thomas Haller
49d5e76204 settings: ipv4.link-local global default
Add a "default" value, which allows to set the flag via global
defaults.

The default for the new property can be "default" because old uses
who upgrade to a new version that supports ipv4.link-local won't have
the global default configured in NetworkManager.conf. So they will
always get the expected default to "auto", unless the change their
configuration.
2022-05-12 23:45:03 +02:00
Thomas Haller
adfc7f1539 settings: honor ipv4.method like before
Rename and replace "none" flag by "auto". That means, to honor
"ipv4.method", like done it the past.

The upside is, that it requires no normalization. Normalization is confusing
to implement, because to get it really right, we probably should support
normalizing link-local based on method, but also vice versa. And since the
method affects how other properties validate/normalize, it's hard to
normalize that one, so that the result makes sense.
Normalization is also often not great to the user, because it
basically means to modify the profile based on other settings.

The downside is that the auto flag becomes API and exists because
we need backward compatibility with ipv4.method.
We would never add this flag, if we would redesign "ipv4.method"
(by replacing by per-method-specific settings).
2022-05-12 23:44:59 +02:00
Adrian Freihofer
06c741b00f settings: add ipv4.link-local flag
Allows the ip4 local-link address configuration to be started
independently of the ipv4.method configuration.

This is done by adding a new link-local parameter to the ipv4 settings.
The link-local parameter partially overlaps with the existing method
parameter. The first step is to reject inconsistent settings for method
and link-local settings.
2022-05-12 23:16:16 +02:00
Beniamino Galvani
77c8b2960a device: commit l3cfg on link change only when the device is activating
On link change, the configuration should be reapplied only when the
device is activating.

Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')

https://bugzilla.redhat.com/show_bug.cgi?id=2079054
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1216
2022-05-09 14:58:57 +02:00
Beniamino Galvani
9fcbc6b37d device: make attach_port() asynchronous
For some device types the attach-port operation doesn't complete
immediately. NMDevice needs to wait that the operation completes
before proceeding (for example, before starting stage3 for the port).

Change attach_port() so that it can return TERNARY_DEFAULT to indicate
that the operation will complete asynchronously. Most of devices are
not affected by this and can continue returning TRUE/FALSE as before
without callback.
2022-05-03 13:19:24 +02:00
Beniamino Galvani
bcc958c411 device: rename {enslave,release}_slave() to {attach,detach}_port()
Rename the enslave_slave() and release_slave() device methods to
attach_port() and detach_port().
2022-05-03 13:19:23 +02:00
Beniamino Galvani
6ab5c4e578 core: save DHCP lease information in state file in /run
DHCP leases for a given interface are already exported on D-Bus
through DHCP4Config and DHCP6Config objects. It is useful to have the
same information also available on the filesystem so that it can be
easily used by scripts.

NM already saves some information about DHCP leases in /var, however
that directory can only be accessed by root, for good reasons.

Append lease options to the existing state file
/run/NetworkManager/devices/$ifindex. Contrary to /var this directory
is not persistent, but it seems more correct to expose the lease only
when it is active and not after it expired or after a reboot.

Since the file is in keyfile format, we add new [dhcp4] and [dhcp6]
sections; however, since some options have the same name for DHCPv4
and DHCPv6, we add a "dhcp4." or "dhcp6." prefix to make the parsing
by scripts (e.g. via "grep") easier.

The option name is the same we use on D-Bus. Since some DHCPv6 options
also have a "dhcp6_" prefix, the key name can contain "dhcp6" twice.

The new sections look like this:

  [dhcp4]
  dhcp4.broadcast_address=172.25.1.255
  dhcp4.dhcp_lease_time=120
  dhcp4.dhcp_server_identifier=172.25.1.4
  dhcp4.domain_name_servers=172.25.1.4
  dhcp4.domain_search=example.com
  dhcp4.expiry=1641214444
  dhcp4.ip_address=172.25.1.182
  dhcp4.next_server=172.25.1.4
  dhcp4.routers=172.25.1.4
  dhcp4.subnet_mask=255.255.255.0

  [dhcp6]
  dhcp6.dhcp6_name_servers=fd01::1
  dhcp6.dhcp6_ntp_servers=ntp.example.com
  dhcp6.ip6_address=fd01::1aa
2022-05-03 09:12:12 +02:00
Fernando Fernandez Mancera
bf5927b978 l3cfg: drop NM_L3_CFG_COMMIT_TYPE_ASSUME and assume_config_once
ASSUME is causing more troubles than benefits it provides. This patch is
dropping NM_L3_CFG_COMMIT_TYPE_ASSUME and assume_config_once. NM3LCfg
will commit as if the sys-iface-state is MANAGED.

This patch is part of the effort to remove ASSUME from NetworkManager.
After ASSUME is dropped when starting NetworkManager it will take full
control of the interface, re-configuring it. The interface will be
managed from the start instead of assumed and then managed.

This will solve the situations where an interface is half-up and then a
restart happens. When NetworkManager is back it won't add the missing
addresses (which is what assume does) so the interface will fail during
the activation and will require a full activation.

https://bugzilla.redhat.com/show_bug.cgi?id=2050216
https://bugzilla.redhat.com/show_bug.cgi?id=2077605
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1196
2022-04-28 10:48:56 +02:00
Thomas Haller
6804c2ba04
device: set MTU after attaching bond port
When attaching a bond port, kernel will reset the MTU of the port ([1],
[2]). Configuring a different MTU on the port seems not a sensible
thing for the user to do.

Still, before commit e67ddd826f ('device: commit MTU during stage2')
we would first attach the bond port before setting the MTU. That
changed, and now the MTU set by kernel wins.

Btw, this change in behavior happens because we attach the port in
stage3 (ip-config), which seems an ugly thing to do.

Anyway, fix this by setting the MTU after attaching the ports, but still
in stage3.

It is probably not sensible for the user to configure a different MTU.
Still, if the user requested it by configuration, we should apply it.
Note that NetworkManager has some logic to constrain the MTU based on
the parent/child and controller/port. In many regards however, NetworkManager
does not fully understand or enforce the correct MTU and relies on the
user to configure it correctly. After all, if the user misconfigures the
MTU, the setup will have problems anyway (and in many cases neither
kernel nor NetworkManager could know that the configuration is wrong).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/bonding/bond_main.c?h=v5.17#n3603
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/bonding/bond_main.c?h=v5.17#n4372

https://bugzilla.redhat.com/show_bug.cgi?id=2071985

Fixes: e67ddd826f ('device: commit MTU during stage2')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1199
2022-04-27 12:53:18 +02:00
Thomas Haller
80c9e2d9ec
device: prevent IP state from reaching ready while DNS update pending
The goal would be to ensure that a device cannot move to activated,
while a DNS update is still pending.

This does not really work for most cases. That is, because NMDevice does
not directly push DNS updates to NMDnsManager, instead, NMPolicy is
watching all device changes, and doing it. But when NMPolicy decides to
to that, may not be the right moment.

We really should let NMDevice (or better, NML3Cfg) directly talk to
NMDnsManager. Why not? They have all the information when new DNS
configuration is available. The only thing that NMPolicy does on top of
that, is determining which device has the best default route. NMPolicy
could continue to do that (or maybe NMDnsManager could), but the update
needs to be directly triggered by NMDevice/NML3Cfg.
2022-04-15 09:04:31 +02:00
Thomas Haller
02a8d21e4e
all: use "NM_UTILS_TO_STRING_BUFFER_SIZE" macro 2022-04-08 15:59:49 +02:00
Thomas Haller
976e2eeb93
device: cleanup code path in nm_device_get_ip_iface_identifier() 2022-04-02 13:33:00 +02:00
Fernando Fernandez Mancera
2bbe7f9204 ovs, dpdk: ignore ovs-netdev tun interface
When configuring a OVS bridge to use the netdev datapath, OpenvSwitch
will create a tun interface named ovs-netdev. This interface should be
ignored by NetworkManager.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1169
2022-03-31 11:49:34 +02:00
Lubomir Rintel
82d16789a5 settings-connection: add a "plugin" argument to Update2()
This will allow migrating a connection. If specified, the connection will
be confined to a particular settings plugin when written back. If the
plugin differs from the existing one, it will be removed from the old one.
2022-03-28 13:29:28 +02:00
Thomas Haller
b3192d2d46
device: allow reapply for changed "connection.autoconnect-slaves" property
We list the properties that can be reapplied, and reject the reapply
operation for any other changes. The idea is that usually reapply
of a property requires an explicit implementation (or may not make
sense).

"connection.autoconnect-slaves" is something that takes effect when
activating the master device. It does not matter when the device
is already active, thus there is no need to reject the reapply
operation.

https://bugzilla.redhat.com/show_bug.cgi?id=2065049

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1150
2022-03-18 15:36:43 +01:00
Thomas Haller
8aaee8c50c
core/device: avoid logging "%p" format and use obfuscated ptr 2022-03-13 12:08:59 +01:00
Thomas Haller
4629506b80
core/device: rename nm_device_master_release_one_slave()
We have nm_device_master_add_slave(). This should be mirrored by
nm_device_master_release_slave() (not release-one-slave).

Thereby, also rename nm_device_master_release_slaves() to
nm_device_master_release_slaves_all() to make it clearer.
2022-03-13 12:08:52 +01:00
Thomas Haller
17ac71cd58
core/device: add ReleaseSlaveType argument for nm_device_master_release_one_slave()
I find the two (dependent) booleans "configure" and "force" confusing.
nm_device_master_release_one_slave() has many callers, it's interesting
to be able to grep for the release-type. Add an enum to make this more
readable.
2022-03-13 12:08:08 +01:00
Thomas Haller
a566fd8cf4
core/device: use c_list_for_each_entry() for interating of slaves list
This convenience macro is just shorter, resulting in more(?) readable
code and less clutter.
2022-03-13 12:02:09 +01:00
Thomas Haller
53406e721d
core/trivial: rename parameter to link_changed_cb()
"info" is not a good name. Variables of this kind are usually
called "plink" or "pllink". Rename.
2022-03-13 12:02:08 +01:00
Thomas Haller
69376e20a5
core: rename nm_utils_host_id_get_timestamp_ns() to "nsec"
We change more and more to prefer "nsec" (and "usec", "msec", and "sec")
as abbreviations, instead of "ns" (and "us", "ms", "s"). Rename.
2022-03-11 09:43:02 +01:00
Thomas Haller
98b3056604
core: preserve external ports during checkpoint rollback
When we have a bridge interface with ports attached externally (that is,
not by NetworkManager itself), then it can make sense that during
checkpoint rollback we want to keep those ports attached.

During rollback, we may need to deactivate the bridge device and
re-activate it. Implement this, by setting a flag before deactivating,
which prevents external ports to be detached. The flag gets cleared,
when the device state changes to activated (the following activation)
or unmanaged.

This is an ugly solution, for several reasons.

For one, NMDevice tracks its ports in the "slaves" list. But what
it does is ugly. There is no clear concept to understand what it
actually tacks. For example, it tracks externally added interfaces
(nm_device_sys_iface_state_is_external()) that are attached while
not being connected. But it also tracks interfaces that we want to attach
during activation (but which are not yet actually enslaved). It also tracks
slaves that have no actual netdev device (OVS). So it's not clear what this
list contains and what it should contain at any point in time. When we skip
the change of the slaves states during nm_device_master_release_slaves_all(),
it's not really clear what the effects are. It's ugly, but probably correct
enough. What would be better, if we had a clear purpose of what the
lists (or several lists) mean. E.g. a list of all ports that are
currently, physically attached vs. a list of ports we want to attach vs.
a list of OVS slaves that have no actual netdev device.

Another problem is that we attach state on the device
("activation_state_preserve_external_ports"), which should linger there
during the deactivation and reactivation. How can we be sure that we don't
leave that flag dangling there, and that the desired following activation
is the one we cared about? If the follow-up activation fails short (e.g. an
unmanaged command comes first), will we properly disconnect the slaves?
Should we even? In practice, it might be correct enough.

Also, we only implement this for bridges. I think this is where it makes
the most sense. And after all, it's an odd thing to preserve unknown,
external things during a rollback -- unknown, because we have no knowledge
about why these ports are attached and what to do with them.

Also, the change doesn't remember the ports that were attached when the
checkpoint was created. Instead, we preserve all ports that are attached
during rollback. That seems more useful and easier to implement. So we
don't actually rollback to the configuration when the checkpoint was
created. Instead, we rollback, but keep external devices.

Also, we do this now by default and introduce a flag to get the previous
behavior.

https://bugzilla.redhat.com/show_bug.cgi?id=2035519
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/ # 909
2022-03-03 11:25:14 +01:00
Thomas Haller
32a828080c
core/trivial: rename NM_SHUTDOWN_TIMEOUT_MS to NM_SHUTDOWN_TIMEOUT_MAX_MSEC
The abbreviations "ms", "us", "ns" don't look good.
Spell out to "msec", "usec", "nsec" as done at other places.

Also, rename NM_SHUTDOWN_TIMEOUT_MS_WATCHDOG to
NM_SHUTDOWN_TIMEOUT_ADDITIONAL_MSEC.

Also, rename NM_SHUTDOWN_TIMEOUT_MS to NM_SHUTDOWN_TIMEOUT_MAX_MSEC.
There are different timeouts, and this is the maximum gracetime we
will give during shutdown to complete async operations.

Naming is hard, but I think these are better names.
2022-02-24 09:38:52 +01:00
Wen Liang
9f6114afe8 device: commit the l3cd changes via l3cfg during cleanup
After the first time committing, the routes and addresses are removed
directly by bypassing the l3cfg in `nm_device_cleanup()`, then when
committing the second time, the l3cfg think that some addresses are
still configured but they are actually already disappeared from the
kernel already.

To fix it, commit the l3cd changes through l3cfg instead of removing
the addresses/routes directly.
2022-02-23 15:47:20 -05:00
Beniamino Galvani
f15b3f15a7 device: delay IP ready state until all objects are committed
Don't progress to the IP ready state until all objects are committed
to platform. Note that l3cfg has a 20 seconds timeout after which
unavailable objects are considered "definitely unavailable" and are
removed from the list.

Fixes-test: @ipv6_routes_with_src
https://bugzilla.redhat.com/show_bug.cgi?id=2043133
2022-02-16 15:12:52 +01:00
Beniamino Galvani
9a090fdf7b core: do a commit after all addresses complete ACD/DAD
l3cfg has a "temp_not_available" list of objects that couldn't be
added to platform, but can be added once some preconditions become
true (for example, a IPv6 route with a "src" attribute requires a
non-tentative src address to be present).

Retry to commit those objects once all addresses have completed
ACD/DAD.
2022-02-16 15:12:52 +01:00
Thomas Haller
a2c8a3228b
device: fix crash for shared IPv6 method in nm_device_copy_ip6_dns_config()
nm_l3_config_data_get_nameservers() returns a pointer to "struct in6_addr". Not
a pointer to pointers.

  #0  __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:389
  #1  0x00007f8060dd9109 in memcpy (__len=<optimized out>, __src=0xfd, __dest=<optimized out>) at /usr/include/bits/string_fortified.h:29
  #2  g_array_append_vals (len=1, data=0xfd, farray=0x55dd69332130) at ../glib/garray.c:522
  #3  g_array_append_vals (farray=0x55dd69332130, data=0xfd, len=1) at ../glib/garray.c:509
  #4  0x000055dd68d2a27d in _garray_inaddr_add (p_arr=<optimized out>, addr_family=<optimized out>, addr=0xfd) at src/core/nm-l3-config-data.c:295
  #5  0x000055dd68ef6510 in nm_l3_config_data_add_nameserver (nameserver=<optimized out>, addr_family=10, self=0x55dd6949f900) at src/core/nm-l3-config-data.c:1442
  #6  nm_device_copy_ip6_dns_config (self=0x55dd693c4420, from_device=<optimized out>) at src/core/devices/nm-device.c:10468
  #7  0x00007f8060f28aba in _g_closure_invoke_va (param_types=0x0, n_params=<optimized out>, args=0x7fffed43d610, instance=0x55dd693c4420, return_value=0x0, closure=0x55dd693cdb10)
      at ../gobject/gclosure.c:893
  #8  g_signal_emit_valist (instance=0x55dd693c4420, signal_id=<optimized out>, detail=0, var_args=var_args@entry=0x7fffed43d610) at ../gobject/gsignal.c:3406
  #9  0x00007f8060f28c03 in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at ../gobject/gsignal.c:3553
  #10 0x000055dd68efd1fb in _dev_ipac6_start (self=0x55dd693c4420) at src/core/devices/nm-device.c:11348
  #11 0x000055dd68efd698 in _dev_ipac6_start_continue (self=0x55dd693c4420) at src/core/devices/nm-device.c:11373
  #12 _dev_ipll6_set_llstate (self=0x55dd693c4420, llstate=<optimized out>, lladdr=<optimized out>) at src/core/devices/nm-device.c:10576
  #13 0x000055dd68e7915e in _emit_changed_on_idle_cb (user_data=user_data@entry=0x55dd6941ca50) at src/core/nm-l3-ipv6ll.c:221
  #14 0x00007f8060e0639b in g_idle_dispatch (source=0x55dd693eea30, callback=0x55dd68e78fd0 <_emit_changed_on_idle_cb>, user_data=0x55dd6941ca50) at ../glib/gmain.c:5897
  #15 0x00007f8060e0a05f in g_main_dispatch (context=0x55dd6922c800) at ../glib/gmain.c:3381
  #16 g_main_context_dispatch (context=0x55dd6922c800) at ../glib/gmain.c:4099
  #17 0x00007f8060e5f2a8 in g_main_context_iterate.constprop.0 (context=0x55dd6922c800, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4175
  #18 0x00007f8060e09773 in g_main_loop_run (loop=0x55dd69211010) at ../glib/gmain.c:4373
  #19 0x000055dd68d09c7b in main (argc=<optimized out>, argv=<optimized out>) at src/core/main.c:509

Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
2022-02-16 10:32:51 +01:00
Thomas Haller
9e90bb0817
platform: improve way to prune dirty route-manager entries
The general idea is that when we have entries tracked by the
route-manager, that we can mark them all as dirty. Then, calling the
"track" function will reset the dirty flag. Finally, there is a method
to delete all dirty entries.

As we can lookup an entry with O(1) (using dictionaries), we can
sync the list of tracked objects with O(n). We just need to track
all the ones we care about, and then delete those that were not touched
(that is, are still dirty).

Previously, we had to explicitly mark all entries as dirty. We can do
better. Just let nmp_route_manager_untrack_all() mark the survivors as
dirty right away. This way, we can save iterating the list once.

It also makes sense because the only purpose of the dirty flag is to
aid this prune mechanism with track/untrack-all. So, untrack-all can
just help out, and leave the remaining entries dirty, so that the next
track does the right thing.
2022-02-09 19:13:05 +01:00
Thomas Haller
7c27c63bec
platform: extend NMPRouteManager to work for routes 2022-02-09 19:13:04 +01:00
Thomas Haller
3996933c57
platform: rename "nmp-route-manager.h" to "nmp-rules-manager.h" 2022-02-09 19:13:03 +01:00
Thomas Haller
ea4f6d7994
platform: rename NMPRulesManager API to NMPRouteManager
Routes of type blackhole, unreachable, prohibit don't have an
ifindex/device. They are thus in many ways similar to routing rules,
as they are global. We need a mediator to keep track which routes
to configure.

This will be very similar to what NMPRulesManager already does for
routing rules. Rename the API, so that it also can be used for routes.

Renaming the file will be done next, so that git's rename detection
doesn't get too confused.
2022-02-09 19:13:03 +01:00
Thomas Haller
454992ed85
core/rfkill: add "nm" prefix to RfKillState and RfKillType enums
Names in header files should have an "nm" prefix. We do that pretty
consistently. Fix the offenders RfKillState and RfKillType.

Also, rename the RfKillState enums to follow the type name. For example,
NM_RFKILL_STATE_SOFT_BLOCKED instead of RFKILL_SOFT_BLOCKED.

Also, when we camel-case a typedef (NMRfKillState) we would want that
the lower-case names use underscore between the words. So it should be
`nm_rf_kill_state_to_string()`. But that looks awkward. So the right solution
here is to also rename "RfKill" to "Rfkill". That make is consistent
with the spelling of the existing `NMRfkillManager` type and the
`nm-rfkill-manager.h` file.
2022-02-08 18:58:53 +01:00
Thomas Haller
165224b485
core/rfkill: move rfkill_type property to NMDeviceClass
GObject Properties are flexible and powerful. In practice, NMDevicePrivate.rfkill_type
was only set once via the construct-only property NM_DEVICE_RFKILL_TYPE.
Which in turn was always set to a well-known value, only depending on the device
type.

We don't need this flexibility. The rfkill-type only depends on the
device type and doesn't change. Replace the property by a field in
NMDeviceClass.

For one, construct properties have an overhead, that the property setter is
called whenever we construct a NMDevice. But the real reason for this
change, is that a property give a notion as this could change during the
lifetime of a NMDevice (which it in fact did not, being construct-only).
Or that the type depends on something more complex, when instead it only
depends on the device type. A non-mutated class property is simpler,
because it's clear that it does not depend on the device instance,
only on the type/class.

Also, `git grep -w rfkill_type` now nicely shows the (few) references to
this variable and its easier to understand.
2022-02-08 18:58:52 +01:00
Beniamino Galvani
02106df3be device: fix required-timeout evaluation
Once the required-timeout expires, we should evaluate whether the
*other* address family is ready.

Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
Fixes-test: @dracut_NM_NFS_root_nfs_ip_dhcp_dhcp6_slow_ip6

https://bugzilla.redhat.com/show_bug.cgi?id=2051904
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1090
2022-02-08 18:12:07 +01:00
Thomas Haller
3399e19df8
device: update metered status when getting DHCP lease
The metered status can depend on the DHCP lease, as we accept the
ANDROID_METERED vendor option. That means, on a DHCP update we need
to re-evaluate the metered flag.

This fixes a potential race, where IPv6 might succeed first and
activation completes (with GUESS_NO metered flag). A subsequent
DHCPv4 update requires to re-evaluate that decision.

Fixes-test: @connection_metered_guess_yes

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1080
2022-02-03 18:46:38 +01:00
Beniamino Galvani
d72a292005 device: fix assuming connections when platform-init arrives late
When the NM_UNMANAGED_PLATFORM_INIT flag is cleared last in
device_link_changed(), a recheck-assume is scheduled and then the
device goes immediately to UNAVAILABLE. During the state transition,
addresses and routes are removed from the interface. Then,
recheck-assume finds that the device can be assumed but it's too late
since the device was already deconfigured.

This is a problem as the whole point of assuming a device is to
activate a connection while leaving the device untouched.

In the NMCI "dracut_NM_vlan_over_bridge and dracut_NM_vlan_over_bond"
test, NM in real root tries to assume a vlan device that was activated
in initrd. When the interface gets deconfigured in UNAVAILABLE, the
connection to the NFS server breaks and the rootfs becomes
inaccessible.

The fix to this problem is to delay state transitions in
device_link_changed() to a idle handler, so that recheck-assume can
run before.

Fixes-test: @dracut_NM_vlan_over_bridge
Fixes-test: @dracut_NM_vlan_over_bond

https://bugzilla.redhat.com/show_bug.cgi?id=2047302
2022-02-03 09:17:20 +01:00
Beniamino Galvani
b2e1bd4436 device: remove unused if branch in device_link_changed()
nm_device_set_unmanaged_by_user_settings() does nothing when the
device is unmanaged by platform-init. Remove the if branch to make
this more explicit.
2022-02-03 09:16:39 +01:00
Thomas Haller
5cbf666279
core: increase l3cfg merge priority for VPN config
Now that higher priorities numbers really mean more important,
it seems that the VPN configuration should be rather important.

Bump the number, also in relation to NMDevice's L3ConfigDataType.

It might not matter too much, because usually the VPN tunnel device does
not have NDevice to add other l3cds and those from VPN might be alone.
Except, maybe with routing VPN (libreswan) that is different. Dunno.
2022-02-01 19:22:02 +01:00
Thomas Haller
5774af9cbd
device: adjust NM_L3CFG_CONFIG_PRIORITY_IPV6LL and L3_CONFIG_DATA_TYPE_LL_6
NM_L3CFG_CONFIG_PRIORITY_IPV6LL and L3_CONFIG_DATA_TYPE_LL_6 are
somewhat related. They probably should be numerically identical.

Now, L3_CONFIG_DATA_TYPE_LL_6 cannot be zero (because
L3_CONFIG_DATA_TYPE_LL_4 is zero and L3ConfigDataType values must be
distinct). Therefore, also bump NM_L3CFG_CONFIG_PRIORITY_IPV6LL to 1.
Of course, NM_L3CFG_CONFIG_PRIORITY_IPV6LL is not obviously more important
than NM_L3CFG_CONFIG_PRIORITY_IPV4LL. But to be consistent with
L3_CONFIG_DATA_TYPE_LL_6 is probably more important here.
2022-02-01 19:22:01 +01:00
Thomas Haller
24896e636b
l3cfg: fix order of NML3ConfigData priorities in NML3Cfg
There was a disagreement whether higher priority numbers mean more/less
important. NMDevice and callers of nm_l3cfg_add_config() assumed that
higher numbers are more important, while _l3_config_datas_get_sorted_cmp()
did the inverse.

There is no obvious right or wrong. We only need to agree. It seems
slightly more sensible to keep the caller's interpretation.
2022-02-01 19:22:01 +01:00
Thomas Haller
982807ec4e
Revert "nm-device: prefer manually configured addresses to automatic"
This commit does not seem correct. The enum was moved with the declared
intent to make manual IP configuration preferred. But the code comment
in L3ConfigDataType states that higher numbers are more important.
Also, all the other values are intentionally ordered so that more
important method have higher numbers. For example, LL_4 < DHCP_4 < SHARED_4 <
DEV_4 (in increasing priority). While it's often not clear whether to
prefer one method over the other, or what the actual effect of (LL_4 < DHCP_4)
is, the numbers were chosen intentionally.

Only moving MANUALIP first, counters the relative order of the other values.
If there is the problem, that the code comment is wrong (and lower
numbers mean more important), then we would have to reverse all
enum values.

The real problem is that NML3Cfg's _l3_config_datas_get_sorted_cmp()
has the wrong order/understanding. So the real fix is there and will
be done next. That is, unless we would agree that _l3_config_datas_get_sorted_cmp()
is in the right, and prefer lower numbers -- in which case all values had to be
reversed.

This reverts commit af1903fe3f.
2022-02-01 19:22:01 +01:00
Beniamino Galvani
9d60cd2813
dhcp: fix crash accepting leases without addresses
For IPv6 the lease doesn't necessarily have an address. If the address
is missing or the DHCP client doesn't implement accept(), we don't
need to wait for the address in platform.

From https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1066#note_1233210 :

  0  0x00007ffff760f88c in __pthread_kill_implementation () at /lib64/libc.so.6
  1  0x00007ffff75c26a6 in raise () at /lib64/libc.so.6
  2  0x00007ffff75ac7d3 in abort () at /lib64/libc.so.6
  3  0x00007ffff77c5d4c in g_assertion_message (domain=<optimized out>, file=<optimized out>, line=<optimized out>, func=<optimized out>, message=<optimized out>)
      at ../glib/gtestutils.c:3223
  4  0x00007ffff782645f in g_assertion_message_expr
      (domain=domain@entry=0x5555559e7c96 "nm", file=file@entry=0x5555559deac0 "src/core/dhcp/nm-dhcp-client.c", line=line@entry=609, func=func@entry=0x5555559e0090 <__func__.31> "l3_cfg_notify_cb", expr=expr@entry=0x5555559df5cf "lease_address") at ../glib/gtestutils.c:3249
  5  0x00005555558b2866 in l3_cfg_notify_cb (l3cfg=0x555555c29790, notify_data=<optimized out>, self=0x555555e9a1b0) at src/core/dhcp/nm-dhcp-client.c:609
  9  0x00007ffff791abe3 in <emit signal ??? on instance ???> (instance=instance@entry=0x555555c29790, signal_id=<optimized out>, detail=detail@entry=0) at ../gobject/gsignal.c:3553
      6  0x00007ffff78fcc7f in g_closure_invoke (closure=0x555555ca3900, return_value=0x0, n_param_values=2, param_values=0x7fffffffd420, invocation_hint=0x7fffffffd3a0)
      at ../gobject/gclosure.c:830
      7  0x00007ffff7919106 in signal_emit_unlocked_R
      (node=node@entry=0x555555bbadc0, detail=detail@entry=0, instance=instance@entry=0x555555c29790, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffd420) at ../gobject/gsignal.c:3742
      8  0x00007ffff791a9ca in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffd5f0)
      at ../gobject/gsignal.c:3497
  10 0x000055555564c8a6 in _nm_l3cfg_emit_signal_notify (self=self@entry=0x555555c29790, notify_data=notify_data@entry=0x7fffffffdf30) at src/core/nm-l3cfg.c:576
  11 0x000055555564ce77 in _nm_l3cfg_emit_signal_notify_simple (self=self@entry=0x555555c29790, notify_type=notify_type@entry=NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT)
      at src/core/nm-l3cfg.c:585
  12 0x0000555555656082 in _l3_commit (self=self@entry=0x555555c29790, commit_type=NM_L3_CFG_COMMIT_TYPE_UPDATE, commit_type@entry=NM_L3_CFG_COMMIT_TYPE_AUTO, is_idle=is_idle@entry=1)
      at src/core/nm-l3cfg.c:4201
  13 0x0000555555656189 in _l3_commit_on_idle_cb (user_data=user_data@entry=0x555555c29790) at src/core/nm-l3cfg.c:2961
  14 0x00007ffff77f847b in g_idle_dispatch (source=0x555555d65680, callback=0x55555565612c <_l3_commit_on_idle_cb>, user_data=0x555555c29790) at ../glib/gmain.c:5897
  15 0x00007ffff77fc130 in g_main_dispatch (context=0x555555aa5020) at ../glib/gmain.c:3381
  16 g_main_context_dispatch (context=0x555555aa5020) at ../glib/gmain.c:4099
  17 0x00007ffff7851208 in g_main_context_iterate.constprop.0 (context=0x555555aa5020, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4175
  18 0x00007ffff77fb853 in g_main_loop_run (loop=0x555555aa5970) at ../glib/gmain.c:4373
  19 0x0000555555593c56 in main (argc=<optimized out>, argv=<optimized out>) at src/core/main.c:509

Fixes: e1648d0665 ('core: commit l3cd asynchronously on DHCP bound event')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1074
2022-01-27 22:17:46 +01:00
Beniamino Galvani
e1648d0665 core: commit l3cd asynchronously on DHCP bound event
When a lease is obtained, currently NMDevice performs a synchronous
commit of IP configuration and then accepts the lease.

Instead, let NMDevice only schedule a async commit; when the DHCP
client notices that the new address was committed it will
automatically accept it and emit a new signal so that the device can
succeed the activation.

Sync commits should be avoided because a commit does of things which
are outside the control of the caller (see the comment in
nm_device_l3cfg_commit()). Furthermore, when there are many pending
activations, async commits seem to help in reducing the CPU usage.

While making the commit async, also move the responsibility of the
accept() to NMDhcpClient.
2022-01-26 14:54:51 +01:00
Thomas Haller
3dd854eb1b
device: initialize nm_auto variable in _ethtool_features_reset()
The compiler is often adament to warn about maybe-uninitialized.
2022-01-21 11:38:08 +01:00
Lubomir Rintel
5f0ddaa610 Revert "nm-device: avoid starting ac6 if l3cfg is not there"
This reverts commit bb0a31e6eb.

This was pushed by accident.
2022-01-11 14:57:48 +01:00
Lubomir Rintel
fccb5608f3 nm-device: clean up IP methods if we lose ifindex
If the ovs interface goes away, the ifindex gets zeroed out and l3cfg is
cleaned. We can't follow up with IP configuration. Bad things happen if
we try to:

  #0  0x00007f769734c895 in _g_log_abort (breakpoint=1) at gmessages.c:580
  #1  0x00007f769734db98 in g_logv (log_domain=0x55b2472d8840 "nm",
        log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>,
        args=args@entry=0x7fff4041b9d0) at gmessages.c:1391
  #2  0x00007f769734dd63 in g_log (log_domain=log_domain@entry=0x55b2472d8840 "nm",
        log_level=log_level@entry=G_LOG_LEVEL_CRITICAL,
        format=format@entry=0x7f769739a620 "%s: assertion '%s' failed") at gmessages.c:1432
  #3  0x00007f769734e59d in g_return_if_fail_warning
      (log_domain=log_domain@entry=0x55b2472d8840 "nm",
        pretty_function=pretty_function@entry=0x55b2472d5fe0 <__func__.39677> "nm_lndp_ndisc_new",
        expression=expression@entry=0x55b2472d5fa3 "NM_IS_L3CFG(config->l3cfg)")
        at gmessages.c:2809
  #4  0x000055b2471ce3fa in nm_lndp_ndisc_new (config=config@entry=0x7fff4041bb30)
        at src/core/ndisc/nm-lndp-ndisc.c:680
  #5  0x000055b247123b32 in _dev_ipac6_start (self=self@entry=0x55b248078360 [NMDeviceOvsInterface])
        at src/core/devices/nm-device.c:11287
  #6  0x000055b2471232f8 in _dev_ipac6_start_continue (self=0x55b248078360 [NMDeviceOvsInterface])
        at src/core/devices/nm-device.c:11338
  #7  0x000055b2471232f8 in _dev_ipll6_set_llstate (self=0x55b248078360 [NMDeviceOvsInterface],
        llstate=<optimized out>, lladdr=<optimized out>) at src/core/devices/nm-device.c:10541
  #8  0x000055b2471c9e8b in _emit_changed_on_idle_cb (user_data=user_data@entry=0x55b24807bdd0)
        at src/core/nm-l3-ipv6ll.c:221
  #9  0x00007f769734327b in g_idle_dispatch (source=0x55b248119200,
        callback=0x55b2471c9ce0 <_emit_changed_on_idle_cb>,
        user_data=0x55b24807bdd0) at gmain.c:5579
  #10 0x00007f769734695d in g_main_dispatch (context=0x55b247f56bc0) at gmain.c:3193
  #11 0x00007f769734695d in g_main_context_dispatch (context=context@entry=0x55b247f56bc0)
        at gmain.c:3873
  #12 0x00007f7697346d18 in g_main_context_iterate (context=0x55b247f56bc0,
        block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3946
  #13 0x00007f7697347042 in g_main_loop_run (loop=0x55b247f320f0) at gmain.c:4142
  #14 0x000055b246f26b64 in main (argc=<optimized out>,
        argv=<optimized out>) at src/core/main.c:511

https://bugzilla.redhat.com/show_bug.cgi?id=2012934
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1044

Fixes-test: @ovs_cloned_mac_set_on_iface
2022-01-11 14:55:38 +01:00
Lubomir Rintel
bb0a31e6eb nm-device: avoid starting ac6 if l3cfg is not there
If the ovs interface goes away, the ifindex gets zeroed out and l3cfg is
cleaned. Avoid starting ac6 in that case -- add checks similar to what
we do for ll6.

Bad things happen otherwise:

  #0  0x00007f769734c895 in _g_log_abort (breakpoint=1) at gmessages.c:580
  #1  0x00007f769734db98 in g_logv (log_domain=0x55b2472d8840 "nm",
        log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>,
        args=args@entry=0x7fff4041b9d0) at gmessages.c:1391
  #2  0x00007f769734dd63 in g_log (log_domain=log_domain@entry=0x55b2472d8840 "nm",
        log_level=log_level@entry=G_LOG_LEVEL_CRITICAL,
        format=format@entry=0x7f769739a620 "%s: assertion '%s' failed") at gmessages.c:1432
  #3  0x00007f769734e59d in g_return_if_fail_warning
      (log_domain=log_domain@entry=0x55b2472d8840 "nm",
        pretty_function=pretty_function@entry=0x55b2472d5fe0 <__func__.39677> "nm_lndp_ndisc_new",
        expression=expression@entry=0x55b2472d5fa3 "NM_IS_L3CFG(config->l3cfg)")
        at gmessages.c:2809
  #4  0x000055b2471ce3fa in nm_lndp_ndisc_new (config=config@entry=0x7fff4041bb30)
        at src/core/ndisc/nm-lndp-ndisc.c:680
  #5  0x000055b247123b32 in _dev_ipac6_start (self=self@entry=0x55b248078360 [NMDeviceOvsInterface])
        at src/core/devices/nm-device.c:11287
  #6  0x000055b2471232f8 in _dev_ipac6_start_continue (self=0x55b248078360 [NMDeviceOvsInterface])
        at src/core/devices/nm-device.c:11338
  #7  0x000055b2471232f8 in _dev_ipll6_set_llstate (self=0x55b248078360 [NMDeviceOvsInterface],
        llstate=<optimized out>, lladdr=<optimized out>) at src/core/devices/nm-device.c:10541
  #8  0x000055b2471c9e8b in _emit_changed_on_idle_cb (user_data=user_data@entry=0x55b24807bdd0)
        at src/core/nm-l3-ipv6ll.c:221
  #9  0x00007f769734327b in g_idle_dispatch (source=0x55b248119200,
        callback=0x55b2471c9ce0 <_emit_changed_on_idle_cb>,
        user_data=0x55b24807bdd0) at gmain.c:5579
  #10 0x00007f769734695d in g_main_dispatch (context=0x55b247f56bc0) at gmain.c:3193
  #11 0x00007f769734695d in g_main_context_dispatch (context=context@entry=0x55b247f56bc0)
        at gmain.c:3873
  #12 0x00007f7697346d18 in g_main_context_iterate (context=0x55b247f56bc0,
        block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3946
  #13 0x00007f7697347042 in g_main_loop_run (loop=0x55b247f320f0) at gmain.c:4142
  #14 0x000055b246f26b64 in main (argc=<optimized out>,
        argv=<optimized out>) at src/core/main.c:511
2022-01-11 14:53:26 +01:00
Beniamino Galvani
02de04287f device: fix update of the ip-iface property
Before the l3cfg rework, the ip-iface property was exported only for
interfaces with an ifindex, and only in some device states.

Restore the old behavior since it is part of the API. For example,
firewalld uses the property to tell which interfaces have a ifindex.

Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')

https://bugzilla.redhat.com/show_bug.cgi?id=2026024
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1042
2021-12-09 15:37:44 +01:00
Lubomir Rintel
af1903fe3f nm-device: prefer manually configured addresses to automatic
This bumps L3_CONFIG_DATA_TYPE_MANUALIP to be the most important address
source; which is what had been the case before NetworkManager/next and
is presumably what the user expects.

It also comes into play for iBFT-booted machines, where iBFT contains a
permanent address (no lifetime data), while DHCP might lease out the
same one. In that case, expiry of the latter could potentially disrupt
connectivity to a vital storage volume.

Fixes: 14962cb414 ('merge: branch 'next''):

https://bugzilla.redhat.com/show_bug.cgi?id=2013921
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1011
2021-12-01 15:04:16 +01:00
Thomas Haller
615221a99c format: reformat source tree with clang-format 13.0
We use clang-format for automatic formatting of our source files.
Since clang-format is actively maintained software, the actual
formatting depends on the used version of clang-format. That is
unfortunate and painful, but really unavoidable unless clang-format
would be strictly bug-compatible.

So the version that we must use is from the current Fedora release, which
is also tested by our gitlab-ci. Previously, we were using Fedora 34 with
clang-tools-extra-12.0.1-1.fc34.x86_64.

As Fedora 35 comes along, we need to update our formatting as Fedora 35
comes with version "13.0.0~rc1-1.fc35".
An alternative would be to freeze on version 12, but that has different
problems (like, it's cumbersome to rebuild clang 12 on Fedora 35 and it
would be cumbersome for our developers which are on Fedora 35 to use a
clang that they cannot easily install).

The (differently painful) solution is to reformat from time to time, as we
switch to a new Fedora (and thus clang) version.
Usually we would expect that such a reformatting brings minor changes.
But this time, the changes are huge. That is mentioned in the release
notes [1] as

  Makes PointerAligment: Right working with AlignConsecutiveDeclarations. (Fixes https://llvm.org/PR27353)

[1] https://releases.llvm.org/13.0.0/tools/clang/docs/ReleaseNotes.html#clang-format
2021-11-29 09:31:09 +00:00