In many cases we want to treat IPv4 and IPv6 generically. That looks nicer
if we distingish by an @addr_family integer, instead of a boolean.
Replace the @is_ipv6 boolean with an @addr_family paramter. The @is_ipv6
boolean is inconsistent with other places where we use @is_ipv4 to
indicate the opposite. Eventually, we should use @addr_family
everywhere.
Also, at the call site it's not immediately clear what TRUE/FALSE means,
here AF_INET/AF_INET6 is better.
- cleanup data type and use guint32 consistently. We might want to
introduce a new "infinity" value. But since libnm's
NM_SETTING_IP_CONFIG_DHCP_TIMEOUT asserts against the range
0 - G_MAXINT32, we cannot express it as -1 anyway. So, infinity
will have the numerical value G_MAXINT32, hence guint32 is just
fine.
- make use of existing ipv6.dhcp-timeout setting and add global
default configuration in NetworkManager.conf
- instead of having subclasses call nm_device_set_dhcp_timeout(),
add a virtual function get_dhcp_timeout().
A typo in the new dhcp-timeout option caused the dhclient daemon to exit
with error when the dhcp-timeout option was specified.
This prevents dhcp connection to be upped.
Fixes: 82ef497cc9
Previously when the interface created by pppd was already the one we
expected, we would rename it to itself and remove the device from the
manager. Don't do it.
Fixes: 6c3195931e
Remove NMDefaultRouteManager. Instead, add the default-route to the
NMIP4Config/NMIP6Config instance.
This basically reverts commit e8824f6a52.
We added NMDefaultRouteManager because we used the corresponding to `ip
route replace` when configuring routes. That would replace default-routes
on other interfaces so we needed a central manager to coordinate routes.
Now, we use the corresponding of `ip route append` to configure routes,
and each interface can configure routes indepdentently.
In NMDevice, when creating the default-route, ignore @auto_method for
external devices. We shall not touch these devices.
Especially the code in NMPolicy regarding selection of the best-device
seems wrong. It probably needs further adjustments in the future.
Especially get_best_ip_config() should be replaced, because this
distinction VPN vs. devices seems wrong to me.
Thereby, remove the @ignore_never_default argument. It was added by
commit bb75026004, I don't think it's
needed anymore.
This brings another change. Now that we track default-routes in
NMIP4Config/NMIP6Config, they are also exposed on D-Bus like regular
routes. I think that makes sense, but it is a change in behavior, as
previously such routes were not exposed there.
Default-routes are for the most part like regular routes. Add support to
track them like regular routes in NMIP4Config/NMIP6Config.
One thing is, sometimes we need to figure out whether an ip-config
instance has a default-route. For that, keep track of the best
default-route (there might be multiple) and expose it. That is
the most complicated part of this patch, because there are so many
places where the list of routes gets modified (replace, intersect,
subtract, merge, add), and they all need to take care of updating
the best default-route.
In a next patch, NMDefaultRouteManager will be dropped and default-routes
will be tracked by NMIP4Config/NMIP6Config.
Previously, we would first delete routes that are not to be added,
before adding the new ones.
This has the advantage, that even if delete removes the wrong route,
add would restore the expected state. This tries to workaround the fact
that RTM_DELROUTE allows for wild-card fields, and might delete the
wrong route.
However, for example when bumping the route metric after connectivty
check (removing the default-route with metric 20100 and adding the one
with metric 100), there is a short moment when there is no
default-route.
To avoid that, don't do delete-then-add, but add-then-delete.
Avoid calling nm_dedup_multi_index_add() directly, like we do for all other places.
Instead, call the wrapper _nm_ip_config_add_obj() which does some pre-precessing.
In practice, the result is exactly the same (at the moment). But there should by
only one way to add the route.
Don't rely on manager keeping them alive long enough. E.g.
get-best-device() is used when resetting the best device,
however, it accesses the current device (hence, it relies
on manager removing the device from the list, but keeping
it alive long enough).
We already track the best device as priv->default_device4 / priv->default_device6.
Don't try to look it up again. If the cached values from @priv are invalid/outdated,
that should be fixed instead.
This was already introduced by commit 773c006a4c.
But I don't think it should be done.
Until recently, we would only consier the IP config of the parent device
to determine the route to the external VPN gateway. We changed that, to
additionally improve the guess by letting kernel resolve the route.
Now, drop checking the parent's config entirely. The only thing that
matters is the here and now runtime configuraion on the parent device.
And for that we ask kernel to resolve the route.
Previously, we would try to resolve the route in general (unrestricted
to a certain ifindex), and reject it the result wasn't on the parent
device.
Now, use the oif argument, and resolve the route only on the parent device.
The problem is that kernel would pretend that the destination is onlink, if
there is no route to it. Hence, hack around that by only accepting an onlink
route, if the VPN gateway itself is site-local. Yes, there are scenarios where
this will still lead to a wrong guess. See related bug rh#1489343 for kernel.
In practice, it shouldn't matter much, because NM may frequently
reapply the IP config. Hence, it anyway must cope with the fact that
IP config from a previous iteration is already applied on the VPN device,
before applying it to the parent device.
Anyway, it makes a bit more sense to apply it first the the parent device.
When creating the NMIP4Config/NMIP6Config instance, we must always use the right
ifindex. That is the ifindex, on which we want to apply the config. It also means,
that for device-based VPNs (those with priv->ip_ifindex set, like OpenVPN), the
parent's config must have the ip-ifindex of the parent device. Not of the
VPN's device.
One effect of this bug is that in add_ip4_vpn_gateway_route() we resolve
the route to the external gateway and only accept it if it's on the
parent device. But since the ifindex of the config was wrong, we would accept
route on the wrong interface.
https://bugzilla.gnome.org/show_bug.cgi?id=787370
After commit 5a69b27a64 ("platform: let platform operations only
consider kernel response") the platform only relies on kernel messages
and doesn't check if a deleted object is gone from the cache. For IPv6
addresses it can happen that the RTM_DELADDR comes after the ack, and
this causes random failures in test /address/ipv6/general-2:
[10.8009] platform: address: deleting IPv6 address 2001:db8:a🅱️1:2:3:4/64, ifindex 12 dev nm-test-device
[10.8009] platform-linux: delayed-action: schedule wait-for-nl-response (seq 55, timeout in 0.199999680, response-type 0)
[10.8009] platform-linux: delayed-action: handle wait-for-nl-response (any)
[10.8009] platform-linux: netlink: recvmsg: new message (2), flags 0x0100, seq 55
[10.8009] platform-linux: delayed-action: complete wait-for-nl-response (seq 55, timeout in 0.199980533, response-type 0, success)
[10.8009] platform-linux: do-delete-ip6-address[12: 2001:db8:a🅱️1:2:3:4]: success
**
NetworkManager:ERROR:src/platform/tests/test-common.c:1127:_ip_address_del: assertion failed: (external_command)
Use the same workaround in place for the addition of IPv6 addresses,
i.e. refetch the object if the address is still present after the ack.
Due to a bug, NetworkManager used to write device routes with "via (null)".
That was fixed in commit af8aac9b54 and
bug rh#1452648.
Add a unit test to ensure we keep accepting such (invalid) routes that
NetworkManager once wrote.
It seems the assert there is too strict. I don't really understand why
it fails, but I also don't see why the assert is supposed to hold.
Just return in case the device is unmanagable at this point.
The activation shall fail later.
Traceback from a test build of commit a7aca2ab08:
#0 0x00007fdb28ffb643 in g_logv (log_domain=0x7fdb2b584cc9 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fff10630200) at gmessages.c:1086
#1 0x00007fdb28ffb7bf in g_log (log_domain=log_domain@entry=0x7fdb2b584cc9 "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7fdb29069190 "%s: assertion '%s' failed") at gmessages.c:1119
#2 0x00007fdb28ffb7f9 in g_return_if_fail_warning (log_domain=log_domain@entry=0x7fdb2b584cc9 "NetworkManager", pretty_function=pretty_function@entry=0x7fdb2b54fee0 <__func__.38922> "unmanaged_to_disconnected", expression=expression@entry=0x7fdb2b54d450 "nm_device_get_managed (device, FALSE)") at gmessages.c:1128
#3 0x00007fdb2b36e05b in unmanaged_to_disconnected (device=device@entry=0x7fdb2d2384f0 [NMDeviceVlan]) at src/nm-manager.c:3201
#4 0x00007fdb2b37eb3a in _internal_activate_generic (error=0x7fff106303d0, active=0x7fdb2d1d4550 [NMActRequest], self=0x0) at src/nm-manager.c:3430
#5 0x00007fdb2b37eb3a in _internal_activate_generic (self=self@entry=0x7fdb2d02b090 [NMManager], active=active@entry=0x7fdb2d1d4550 [NMActRequest], error=error@entry=0x7fff10630450) at src/nm-manager.c:3458
#6 0x00007fdb2b37fe90 in _activation_auth_done (active=0x7fdb2d1d4550 [NMActRequest], success=1, error_desc=0x0, user_data1=0x7fdb2d02b090, user_data2=0x7fdb0800bec0) at src/nm-manager.c:3866
#7 0x00007fdb2b4cc9d7 in auth_done (chain=0x7fdb2d17de30, error=0x0, unused=<optimized out>, user_data=<optimized out>) at src/nm-active-connection.c:929
#8 0x00007fdb2b4d6884 in auth_chain_finish (user_data=0x7fdb2d17de30) at src/nm-auth-utils.c:92
#9 0x00007fdb28ff4d7a in g_main_context_dispatch (context=0x7fdb2cff2e00) at gmain.c:3152
#10 0x00007fdb28ff4d7a in g_main_context_dispatch (context=context@entry=0x7fdb2cff2e00) at gmain.c:3767
#11 0x00007fdb28ff50b8 in g_main_context_iterate (context=0x7fdb2cff2e00, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3838
#12 0x00007fdb28ff538a in g_main_loop_run (loop=0x7fdb2cff2ec0) at gmain.c:4032
#13 0x00007fdb2b349ed7 in main (argc=1, argv=0x7fff106307a8) at src/main.c:438
https://bugzilla.redhat.com/show_bug.cgi?id=1478911
If the slave is 'external' we should never touch it, in particular we
should not release the link from its master; we only have to remove it
from master's list.
https://bugzilla.redhat.com/show_bug.cgi?id=1442361
Previously, if a master device had internal state 'managed', we would
promote the slave to 'managed' as well. However,
- if the slave is 'external', it should stay as is because we don't
want to start managing it
- if the slave is 'assumed', it will become managed when the
activation succeeds, so it's not necessary to do it here
Fixes: 850c977953
When we generate the connection in nm_device_generate_connection(), we
add all routes that have rt_source (roundtrip(NM_IP_CONFIG_SOURCE_USER)).
Especially since commit e470e13922, this
includes automatically added host-routes to the gateway, added by
ip4_config_merge_and_apply().
Later, during nm_utils_match_connection() this route most not prevent
matching. Either nm_device_generate_connection() should not add it, or
nm_utils_match_connection() should ignore it.
I think adjusting the matching is better, because ip-configs are used
for several things, including exposing routes on D-Bus. We don't want
to hide this route on D-Bus.
Fixes: e470e13922https://bugzilla.redhat.com/show_bug.cgi?id=1487384
teamd uses a PID file to guarantee a single instance is running for
each device. If we spawn a new teamd process without waiting the
termination of the existing one, the new process can fail:
<debug> [1486191713.2530] kill child process 'teamd' (2676): wait for process to terminate after sending SIGTERM (15) (send SIGKILL in 2000 milliseconds)...
...
<debug> [1486191713.2539] device[0x7f737f5d7c40] (team1): running: /usr/bin/teamd -o -n -U -D -N -t team1 -c {"runner": {"name": "activebackup"}} -gg
Using team device "team1".
Using PID file "/var/run/teamd/team1.pid"
This program is not intended to be run as root.
Daemon already running on PID 2676.
Failed: File exists
To avoid this, keep track that a kill is in progress and postpone the
start of teamd.
https://bugzilla.redhat.com/show_bug.cgi?id=1415641
When unenslaving an interface from a bridge, kernel sends a RTM_DELLINK
message with ifi_family AF_BRIDGE. We only care about regular
RTM_DELLINK/RTM_NEWLINK messages, ignore all but ifi_family AF_UNSPEC.
There is also test_nl_bugs_spuroius_dellink(), added in commit
8a87a91813 for related bug rh#1302037.
The workaround was masking a bug in NetworkManager (to not ignore AF_BRIDGE
messages) and can now be removed as well.
