Now that every call to nm_device_set_ip_iface() and nm_device_set_ip_ifindex()
is checked, and setting an interface that does not exist causes the device
state to fail, we no longer need to allow setting an ip-iface if we are
unable to retrieve the ip-ifindex.
- don't even bother to look into the platform cache, but use
if_indextoname() / if_nametoindex(). In most cases, we obtained
the ifindex/ifname not from the platform cache in the first
place. Hence, there is a race, where the interface might not
exist.
However, try to process events of the platform cache, hoping
that the cache contains an interface for the given ifindex/ifname.
- let set_ip_ifindex() and set_ip_iface() both return a boolean
value to indicate whether a ip-interface is set or not. That is,
whether we have a positive ip_ifindex. That seems more interesting
information, then to return whether anything changed.
- as before, set_ip_ifindex() can only clear an ifindex/ifname,
or error out without doing anything. That is different from
set_ip_iface(), which will also set an ifname if no ifindex
can be resolved. That is curreently ugly, because then ip-ifindex
and ip-iface don't agree. That shall be improved in the future
by:
- trying to set an interface that cannot be resolved shall
lead to a disconnect in any case.
- we shall make less use of the ip-iface and rely more on the
ifindex.
Platform invokes change events while reading netlink events. However,
platform code is not re-entrant and calling into platform again is not
allowed (aside operations that do not process the netlink socket, like
lookup of the platform cache).
That basically means, we have to always process events in an idle
handler. That is not a too strong limitation, because we anyway don't
know the call context in which the platform event is emitted and we
should avoid unguarded recursive calls into platform.
Otherwise, we get hit an assertion/crash in nm-iface-helper:
1 raise()
2 abort()
3 g_assertion_message()
4 g_assertion_message_expr()
5 do_delete_object()
6 ip6_address_delete()
>>> 7 nm_platform_ip6_address_delete()
8 nm_platform_ip6_address_sync()
9 nm_ip6_config_commit()
10 ndisc_config_changed()
11 ffi_call_unix64()
12 ffi_call()
13 g_cclosure_marshal_generic_va()
14 _g_closure_invoke_va()
15 g_signal_emit_valist()
16 g_signal_emit()
>>> 17 nm_ndisc_dad_failed()
18 ffi_call_unix64()
19 ffi_call()
20 g_cclosure_marshal_generic()
21 g_closure_invoke()
22 signal_emit_unlocked_R()
23 g_signal_emit_valist()
24 g_signal_emit()
>>> 25 nm_platform_cache_update_emit_signal()
26 event_handler_recvmsgs()
27 event_handler_read_netlink()
28 delayed_action_handle_one()
29 delayed_action_handle_all()
30 do_delete_object()
31 ip6_address_delete()
32 nm_platform_ip6_address_delete()
33 nm_platform_ip6_address_sync()
>>> 34 nm_ip6_config_commit()
35 ndisc_config_changed()
36 ffi_call_unix64()
37 ffi_call()
38 g_cclosure_marshal_generic_va()
39 _g_closure_invoke_va()
40 g_signal_emit_valist()
41 g_signal_emit()
42 check_timestamps()
43 receive_ra()
44 ndp_call_eventfd_handler()
45 ndp_callall_eventfd_handler()
46 event_ready()
47 g_main_context_dispatch()
48 g_main_context_iterate.isra.22()
49 g_main_loop_run()
>>> 50 main()
NMPlatform already has a check to assert against recursive calls
in delayed_action_handle_all():
g_return_val_if_fail (priv->delayed_action.is_handling == 0, FALSE);
priv->delayed_action.is_handling++;
...
priv->delayed_action.is_handling--;
Fixes: f85728ecffhttps://bugzilla.redhat.com/show_bug.cgi?id=1546656
Example: when dhcpv4 lease renewal fails, if ipv4.may-fail was "yes",
check also if we have a successful ipv6 conf: if not fail.
Previously we just ignored the other ip family status.
Convert the string representation of ipv4.dhcp-client-id property already in
NMDevice to a GBytes. Next, we will support more client ID modes, and we
will need the NMDevice context to generate the client id.
GByteArray is a mutable array of bytes. For every practical purpose, the hwaddr
property of NMDhcpClient is an immutable sequence of bytes. Thus, make it a
GBytes.
While the numerical values of IFA_F_SECONDARY and IFA_F_TEMPORARY
are identical, their meaning is not.
IFA_F_SECONDARY is only relevant for IPv4 addresses, while
IFA_F_TEMPORARY is only relevant for IPv6 addresses.
IFA_F_TEMPORARY is automatically set by kernel for the addresses
that it generates as part of IFA_F_MANAGETEMPADDR. It cannot be
actively set by user-space.
IFA_F_SECONDARY is automatically set by kernel depending on the order
in which the addresses for the same subnet are added.
This essentially reverts 8b4f11927 (core: avoid IFA_F_TEMPORARY alias for
IFA_F_SECONDARY).
In device_ipx_changed() we remember the addresses for which it appears
that DAD failed. Later, on an idle handler, we process them during
queued_ip6_config_change().
Note that nm_plaform_ip6_address_sync() might very well decide to remove
some or all addresses and re-add them immidiately later. It might do so,
to get the address priority/ordering right. At that point, we already
emit platform signals that the device disappeared, and track them in
dad6_failed_addrs.
Hence, later during queued_ip6_config_change() we must check again
whether the address is really not there and not still doing DAD.
Otherwise, we wrongly claim that DAD failed and remove the address,
generate a new one, and the same issue might happen again.
dad6_failed_addrs is populated with addresses from the platform cache.
Inside the cache, all addresses have addr_source NM_IP_CONFIG_SOURCE_KERNEL,
because addr_source property for addresses is only a property that is
used NetworkManager internally.
NMPObjects are never modified after being put into the cache.
Hence, it is safe and encouraged to just keep a reference to them,
instead of cloning them.
Interestingly, NMPlatform's change signals have a platform_object
pointer, which is not the pointer to the NMPObjects itself, but
down-cast to the NMPlatformObject instance. It does so, because commonly
callers want to have a pointer to the NMPlatformObject instance, instead
of the outer NMPObjects. However, NMP_OBJECT_UP_CAST() is guaranteed
to work one would expect.
The order in which we add addresses to dad6_failed_addrs does not
matter. Hence, use g_slist_prepend() which is O(1), instead
g_slist_append() with O(n).
nm_utils_lifetime_get() already has so many arguments.
Essentially, the function returned %TRUE if and only if the
lifetime was greater then zero.
Combine the return value and the output argument for the lifetime.
It also matches better the function name: to get the lifetime.
In ndisc_set_router_config(), we initialize NMNDiscAddress based on
NMPlatformIP6Address instances. Note that their handling of timestamps
is not entirely identical.
For convenience of the user, NMPlatformIP6Address allows to not specify
any timestamp. On the contrary, for convenience of implementation does
NMNDiscAddress always require fully specified timestamps.
Properly convert one representation into the other.
We commonly only allow tabs at the beginning of a line, not
afterwards. The reason for this style is so that the code
looks formated right with tabstop=4 and tabstop=8.
Setting the state of NMActiveConnection results in invoking callbacks
in NMManager. Hence, it might be far-reaching. Clear
priv->queued_act_request before invoking the callbacks.
XXX was used to either raise attention (NOTE) or to indicate
that this is ugly code that should be fixed (FIXME). The usage
was inconsistent.
Let's avoid XXX and use either NOTE or FIXME.
We already avoid committing the IP configuration for external devices
(see commit 60334a2893). However, we still start DHCP/IPv6-autoconf
and, especially, we change sysctl values of the device.
To be sure that no action is taken on the device, return early from
the IP configuration phase, as in the method=disabled/ignore case.
https://bugzilla.redhat.com/show_bug.cgi?id=1530288
If IPV6CP terminates before IPCP, pppd enters the RUNNING phase and we
start IP configuration without having an IP interface set, which
triggers assertions.
Instead, add a SetIfindex() D-Bus method that gets called by the
plugin when pppd becomes RUNNING. The method sets the IP ifindex of
the device and starts IP configuration.
https://bugzilla.redhat.com/show_bug.cgi?id=1515829
The connection.mdns setting is a per-connection setting,
so one might expect that one activated device can only have
one MDNS setting at a time.
However, with certain VPN plugins (those that don't have their
own IP interface, like libreswan), the VPN configuration is merged
into the configuration of the device. So, in this case, there
might be multiple settings for one device that must be merged.
We already have a mechanism for that. It's NMIP4Config. Let NMIP4Config
track this piece of information. Although, stricitly speaking this
is not tied to IPv4, the alternative would be to introduce a new
object to track such data, which would be a tremendous effort
and more complicated then this.
Luckily, NMDnsManager and NMDnsPlugin are already equipped to
handle multiple NMIPConfig instances per device (IPv4 vs. IPv6,
and Device vs. VPN).
Also make "connection.mdns" configurable via global defaults in
NetworkManager.conf.
Instead, intern the string and cache it in the NMDeviceClass instance.
It anyway depends entirely on the GObject type (name), hence it should
also be cached at the type.
Add a new device state reason code for unsupported IP method. It is
returned, for example, when users select manual IP configuration for
WWAN connections:
# nmcli connection mod Gsm ipv4.method manual ipv4.address 1.2.3.4/32
# nmcli connection up Gsm
Error: Connection activation failed: The selected IP method is not
supported
compared to the old:
Error: Connection activation failed: IP configuration could not be
reserved (no available address, timeout, etc.)
Note that we could instead fail the connection validation if the
method is not supported by the connection type, but adding such
limitation now could make existing connections invalid.
https://bugzilla.redhat.com/show_bug.cgi?id=1459529
NM_FLAGS_HAS() uses a static-assert that the second argument is a
single flag (power of two). With a single flag, NM_FLAGS_HAS(),
NM_FLAGS_ANY() and NM_FLAGS_ALL() are all identical.
The second argument must be a compile time constant, and if that is
not the case, one must not use NM_FLAGS_HAS().
Use NM_FLAGS_ANY() in these cases.
In the past we had NMDefaultRouteManager which would coordinate adding
the default-route with identical metrics. That especially happened, when
activating two devices of the same type, without explicitly specifying
ipv4.route-metric. For example, with ethernet devices, the routes on
both interfaces would get a metric of 100.
Coordinating routes was especially necessary, because we added
routes with NLM_F_EXCL flag, akin to `ip route replace`. We not
only had to avoid that activating two devices in NetworkManager would
result in a fight over the default-route, but more importently
to preserve externally added default-routes on unmanaged interfaces.
NMDefaultRouteManager would ensure that in case of duplicate
metrics, that the device that activated first would keep the
best default-route. It would do so by bumping the metric
of the second device to find a unused metric. The bumping itself
was not very important -- MDefaultRouteManager could also just not
configure any default-routes that show up as second, the result
would be quite similar. More important was to keep the best
default-route on the first activating device until the device
deactivates or a device activates that really has a better
default-route..
Likewise, NMRouteManager would globally manage non-default-routes.
It would not do any bumping of metrics, but it would also ensure that the routes
of the device that activates first are not overwritten by a device activating
later.
However, the `ip route replace` approach has downsides, especially
that it messes with routes on other interfaces, interfaces that are
possibly not managed by NetworkManager. Another downside is, that
binding a socket to an interface might not result in correct
routes, because the route might just not be there (in case of
NMRouteManager, which wouldn't configure duplicate routes by bumping
their metric).
Since commit 77ec302714 we would no longer
use NLM_F_EXCL, but add routes akin to `ip route append`. When
activating for example two ethernet devices with no explict route
metric configuration, there are two routes like
default via 10.16.122.254 dev eth0 proto dhcp metric 100
default via 192.168.100.1 dev eth1 proto dhcp metric 100
This does not only affect default routes. In case of a multi-homing
setup you'd get
192.168.100.0/24 dev eth0 proto kernel scope link src 192.168.100.1 metric 100
192.168.100.0/24 dev eth1 proto kernel scope link src 192.168.100.1 metric 100
but it's visible the most for default-routes.
Note that we would append the routes that are activated later, as the order
of `ip route show` confirms. One might hence expect, that kernel selects
a route based on the order in the routing tables. However, that isn't
the case, and activating the second interface will non-deterministically
re-route traffic via the new interface. That will interfere badly with
with NAT, stateful firewalls, and existing connections (like TCP).
The solution is to have NMManager keep a global index of the default route-metrics
currently in use. So, instead of determining the default-route metric based solely
on the device-type, we now in addition generate default metrics that do not
overlap. For example, if you activate eth0 first, it gets route-metric 100,
and if you then activate eth1, it gets 101. Note that if you deactivate
and re-activate eth0, then it will get route-metric 102, because the
best route should stick on eth1 (which reserves the range 100 to 101).
Note that when a connection explititly selects a particular metric, then that
choice is honored (contrary to NMDefaultRouteManager which was more concerned
with avoiding conflicts, then keeping the exact metric).
https://bugzilla.redhat.com/show_bug.cgi?id=1505893
NMManager will need to know the state of all device at once.
Hence, load it once and cache it in NMConfig.
Note that this wastes a bit of memory in the order of
O(number-of-interfaces). But each device state entry is
rather small, and we always consume memory in the order
of O(number-of-interfaces).
The dynamic IPv4 configuration from DHCP/PPP/... and WWAN is stored in
priv->{dev,wwan}_ip4_config; when the user removes externally an
address or a route, we prune it from those configurations. Therefore
such addresses and routes can't be restored on a device reapply.
Introduce an AppliedConfig structure that stores both the original and
the current (after external changes) configuration so that we can
restore the original one on reapply.
Restarting the IP configuration removes addresses and routes for a
short time breaking connectivity. The reapply process should have the
minimal impact possible.
https://bugzilla.gnome.org/show_bug.cgi?id=790061
The accessor functions just look whether a certain flag is set. As these
functions have a different name then the flags, this is more confusing
then helpful. For example, if you want to know where the NM_GENERATED
flag matters, you had to know to grep for nm_settings_connection_get_nm_generated()
in addition to NM_SETTINGS_CONNECTION_FLAGS_NM_GENERATED.
The accessor function hid that the property was implemented as
a connection flag. For example, it was not immediately obvious
that nm_settings_connection_get_nm_generated() is the same
as having the NM_SETTINGS_CONNECTION_FLAGS_NM_GENERATED flag
set.
Drop them.
