mirror of
https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git
synced 2026-05-17 02:58:07 +02:00
Revert "platform: always reconfigure IP routes even if removed externally"
The change in behavior introduced by the patch departs from the policy that NM had for long time of trying not to interfere with user modifications. This seems a fundamental aspect of how NM works and indeed we got already one report: https://bugzilla.redhat.com/show_bug.cgi?id=2218866 of a user that was affected by the change. The specific case is about routes from DHCP, but also static routes are affected. When a user removes the route added by NM, NM now can add it back at any time. Changing behavior is bad, it causes pain for users and for people who need to support them. However, if the new behavior provides clear advantages to users, that might be ok. This doesn't seem the case in my opinion. Commit7ca95ceedescribes a couple of scenarios: > - kernel can automatically remove routes. For example, deleting an > IPv4 address that is the prefsrc of a route, will cause kernel to > delete that route. Sure, we may be unable to re-configure the > route at this moment, but we shouldn't remember indefinitely that > the route is supposed to be absent. Rather, we should re-add it > when possible > - kernel is a pain with validating consistencies of routes. For > example, when a route has a nexthop gateway, then the gateway must > be onlink (directly reachable), or kernel refuses to add it with > "Nexthop has invalid gateway". Of course, when removing the onlink > route kernel is fine leaving the gateway route behind, which it > would otherwise refuse to add. > Anyway. Such interdependencies for when kernel rejects adding a > route with "Nexthop has invalid gateway" are non-trivial. We try > to work around that by always adding the necessary onlink > routes. See nm_l3_config_data_add_dependent_onlink_routes(). But > if the user externally removed the dependent onlink route, and > NetworkManager remembers to not re-adding it, then the efforts > from nm_l3_config_data_add_dependent_onlink_routes() are > ignored. This causes ripple effects and NetworkManager will also > be unable to add the nexthop route. Kernel usually removes addresses as consequence of user actions. If not (e.g. DHCP lease expiring) we have solutions in place for that to re-add the route. If the route removal is the consequence of a user action, then the user must do something to undo it. For example, if the user removes an address on the same interface, a route using the address as prefsrc will be deleted. If the user wants it back, it must be re-added manually together with the address; I don't see any problem with this. The prefsrc address could be on another interface; in such case by simply deactivating the connection providing the address, a dependent route could be removed on another interface and never readded. This doesn't look as a setup that anybody would use; in case we need to support it, it is better to find alternative solutions. So, my opinion is that the change in behavior potentially breaks many users and doesn't bring clear advantages. Therefore, restore the old behavior. This reverts commit7ca95cee15. Revert conflicts: - the following code was removed from _obj_states_sync_filter() in nm-l3cfg.c because the mechanism to set temporarily-unavailable routes was changed in1feaf427d2('platform: rework handling of failed routes during nm_platform_ip_route_sync()'), and so `os_temporary_not_available_timestamp_msec` no longer exists: if (obj_state->os_temporary_not_available_timestamp_msec > 0) { /* we currently try to configure this address (but failed earlier). * Definitely retry. */ return TRUE; }
This commit is contained in:
parent
bdd87cd5f4
commit
0df304b790
1 changed files with 4 additions and 26 deletions
|
|
@ -1129,32 +1129,10 @@ _obj_states_sync_filter(NML3Cfg *self, const NMPObject *obj, NML3CfgCommitType c
|
|||
return TRUE;
|
||||
}
|
||||
|
||||
/* One goal would be that we don't forcefully re-add routes which were
|
||||
* externally removed (e.g. by the user via `ip route del`).
|
||||
*
|
||||
* However,
|
||||
*
|
||||
* - some routes get automatically deleted by kernel (for example,
|
||||
* when we have an IPv4 route with RTA_PREFSRC set and the referenced
|
||||
* IPv4 address gets removed). The absence of such a route does not
|
||||
* mean that the user doesn't want the route there. It means, kernel
|
||||
* removed it because of some consistency check, but we want it back.
|
||||
* - a route with a non-zero gateway requires that the gateway is
|
||||
* directly reachable via an onlink route. The rules for this are
|
||||
* complex, but kernel will reject adding a route which has such a
|
||||
* gateway. If the user manually removed the needed onlink route, the
|
||||
* gateway route cannot be added in kernel ("Nexthop has invalid
|
||||
* gateway"). To handle that is a nightmare, so we always ensure that
|
||||
* the onlink route is there.
|
||||
* - a route with RTA_PREFSRC requires that such an address is
|
||||
* configured otherwise kernel rejects adding the route with "Invalid
|
||||
* prefsrc address"/"Invalid source address". Removing an address can
|
||||
* thus prevent adding the route, which is a problem for us.
|
||||
*
|
||||
* So the goal is not tenable and causes problems. NetworkManager will
|
||||
* try hard to re-add routes and address that it thinks should be
|
||||
* present. If you externally remove them, then you are starting a
|
||||
* fight where NetworkManager tries to re-add them on every commit. */
|
||||
if (!obj_state->os_plobj && commit_type != NM_L3_CFG_COMMIT_TYPE_REAPPLY
|
||||
&& !nmp_object_get_force_commit(obj))
|
||||
return FALSE;
|
||||
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue