diff options
Diffstat (limited to 'src/core/nm-l3cfg.c')
| -rw-r--r-- | src/core/nm-l3cfg.c | 757 |
1 files changed, 498 insertions, 259 deletions
diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index a49654fe..3c2d3ec8 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -11,6 +11,7 @@ #include <linux/if_ether.h> #include <linux/rtnetlink.h> +#include "libnm-glib-aux/nm-prioq.h" #include "libnm-glib-aux/nm-time-utils.h" #include "libnm-platform/nm-platform.h" #include "libnm-platform/nmp-object.h" @@ -123,25 +124,34 @@ typedef struct { CList os_lst; - /* If we have a timeout pending, we link the instance to - * self->priv.p->obj_state_temporary_not_available_lst_head. */ - CList os_temporary_not_available_lst; - /* If a NMPObject is no longer to be configured (but was configured * during a previous commit), then we need to remember it so that the * next commit can delete the address/route in kernel. It becomes a zombie. */ CList os_zombie_lst; - /* We might want to configure "obj" in platform, but it's currently not possible. - * For example, certain IPv6 routes can only be added after the IPv6 address - * becomes non-tentative (*sigh*). In such a case, we need to remember that, and - * retry later. If this timestamp is set to a non-zero value, then it means - * we tried to configure the obj (at that timestamp) and failed, but we are - * waiting to retry. + /* Used by _handle_routes_failed() mechanism. If "os_plobj" is set, then + * this is meaningless but should be set to zero. + * + * If set to a non-zero value, this means adding the object failed. Until + * "os_failedobj_expiry_msec" we are still waiting whether we would be able to + * configure the object. Afterwards, we consider the element failed. * - * See also self->priv.p->obj_state_temporary_not_available_lst_head - * and self->priv.p->obj_state_temporary_not_available_timeout_source. */ - gint64 os_temporary_not_available_timestamp_msec; + * Depending on "os_failedobj_prioq_idx", we are currently waiting whether the + * condition can resolve itself or becomes a failure. */ + gint64 os_failedobj_expiry_msec; + + /* The index into the "priv->failedobj_prioq" queue for objects that are failed. + * - this field is meaningless in case "os_plobj" is set (but it should be + * set to NM_PRIOQ_IDX_NULL). + * - otherwise, if "os_failedobj_expiry_msec" is 0, no error was detected so + * far. The index should be set to NM_PRIOQ_IDX_NULL. + * - otherwise, if the index is NM_PRIOQ_IDX_NULL it means that the object + * is not tracked by the queue, no grace timer is pending, and the object + * is considered failed. + * - otherwise, the index is used for tracking the element in the queue. + * It means, we are currently waiting to decide whether this will be a + * failure or not. */ + guint os_failedobj_prioq_idx; /* When the obj is a zombie (that means, it was previously configured by NML3Cfg, but * now no longer), it needs to be deleted from platform. This ratelimits the time @@ -206,7 +216,6 @@ typedef struct { guint32 acd_timeout_msec_confdata; NML3AcdDefendType acd_defend_type_confdata : 3; bool dirty_confdata : 1; - gboolean force_commit_once : 1; } L3ConfigData; struct _NML3CfgBlockHandle { @@ -241,7 +250,6 @@ typedef struct _NML3CfgPrivate { CList obj_state_lst_head; CList obj_state_zombie_lst_head; - CList obj_state_temporary_not_available_lst_head; GHashTable *acd_ipv4_addresses_on_link; @@ -288,12 +296,22 @@ typedef struct _NML3CfgPrivate { guint64 pseudo_timestamp_counter; - GSource *obj_state_temporary_not_available_timeout_source; + NMPrioq failedobj_prioq; + GSource *failedobj_timeout_source; + gint64 failedobj_timeout_expiry_msec; NML3CfgCommitType commit_on_idle_type; gint8 commit_reentrant_count; + union { + struct { + gint8 commit_reentrant_count_ip_address_sync_6; + gint8 commit_reentrant_count_ip_address_sync_4; + }; + gint8 commit_reentrant_count_ip_address_sync_x[2]; + }; + /* The value that was set before we touched the sysctl (this only is * meaningful if "ip6_privacy_set" is true. At the end, we want to restore * this value. */ @@ -340,6 +358,9 @@ G_DEFINE_TYPE(NML3Cfg, nm_l3cfg, G_TYPE_OBJECT) #define _MPTCP_TAG(self, IS_IPv4) ((gconstpointer) (&(((const char *) (self))[2 + (!(IS_IPv4))]))) +#define _NETNS_WATCHER_IP_ADDR_TAG(self, addr_family) \ + ((gconstpointer) & (((char *) self)[1 + NM_IS_IPv4(addr_family)])) + /*****************************************************************************/ #define _NMLOG_DOMAIN LOGD_CORE @@ -410,8 +431,6 @@ static NM_UTILS_ENUM2STR_DEFINE( NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE, "platform-change-on-idle"), NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT, "pre-commit"), NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT, "post-commit"), - NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_ROUTES_TEMPORARY_NOT_AVAILABLE_EXPIRED, - "routes-temporary-not-available-expired"), NM_UTILS_ENUM2STR_IGNORE(_NM_L3_CONFIG_NOTIFY_TYPE_NUM), ); static NM_UTILS_ENUM2STR_DEFINE(_l3_acd_defend_type_to_string, @@ -754,51 +773,51 @@ _nm_n_acd_data_probe_new(NML3Cfg *self, in_addr_t addr, guint32 timeout_msec, gp /*****************************************************************************/ -#define nm_assert_obj_state(self, obj_state) \ - G_STMT_START \ - { \ - if (NM_MORE_ASSERTS > 0) { \ - const NML3Cfg *_self = (self); \ - const ObjStateData *_obj_state = (obj_state); \ - \ - nm_assert(_obj_state); \ - nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(_obj_state->obj), \ - NMP_OBJECT_TYPE_IP4_ADDRESS, \ - NMP_OBJECT_TYPE_IP6_ADDRESS, \ - NMP_OBJECT_TYPE_IP4_ROUTE, \ - NMP_OBJECT_TYPE_IP6_ROUTE)); \ - nm_assert(!_obj_state->os_plobj || _obj_state->os_was_in_platform); \ - nm_assert((_obj_state->os_temporary_not_available_timestamp_msec == 0) \ - == c_list_is_empty(&_obj_state->os_temporary_not_available_lst)); \ - if (_self) { \ - if (c_list_is_empty(&_obj_state->os_zombie_lst)) { \ - nm_assert(_self->priv.p->combined_l3cd_commited); \ - \ - if (NM_MORE_ASSERTS > 5) { \ - nm_assert(c_list_contains(&_self->priv.p->obj_state_lst_head, \ - &_obj_state->os_lst)); \ - nm_assert((_obj_state->os_temporary_not_available_timestamp_msec == 0) \ - || c_list_contains( \ - &_self->priv.p->obj_state_temporary_not_available_lst_head, \ - &_obj_state->os_temporary_not_available_lst)); \ - nm_assert(_obj_state->os_plobj \ - == nm_platform_lookup_obj(_self->priv.platform, \ - NMP_CACHE_ID_TYPE_OBJECT_TYPE, \ - _obj_state->obj)); \ - nm_assert( \ - c_list_is_empty(&obj_state->os_zombie_lst) \ - ? (_obj_state->obj \ - == nm_dedup_multi_entry_get_obj(nm_l3_config_data_lookup_obj( \ - _self->priv.p->combined_l3cd_commited, \ - _obj_state->obj))) \ - : (!nm_l3_config_data_lookup_obj( \ - _self->priv.p->combined_l3cd_commited, \ - _obj_state->obj))); \ - } \ - } \ - } \ - } \ - } \ +#define nm_assert_obj_state(self, obj_state) \ + G_STMT_START \ + { \ + if (NM_MORE_ASSERTS > 0) { \ + const NML3Cfg *_self = (self); \ + const ObjStateData *_obj_state = (obj_state); \ + \ + nm_assert(_obj_state); \ + nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(_obj_state->obj), \ + NMP_OBJECT_TYPE_IP4_ADDRESS, \ + NMP_OBJECT_TYPE_IP6_ADDRESS, \ + NMP_OBJECT_TYPE_IP4_ROUTE, \ + NMP_OBJECT_TYPE_IP6_ROUTE)); \ + nm_assert(!_obj_state->os_plobj || _obj_state->os_was_in_platform); \ + nm_assert(_obj_state->os_failedobj_expiry_msec != 0 \ + || _obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); \ + nm_assert(_obj_state->os_failedobj_expiry_msec == 0 || !_obj_state->os_plobj); \ + nm_assert(_obj_state->os_failedobj_expiry_msec == 0 \ + || c_list_is_empty(&_obj_state->os_zombie_lst)); \ + nm_assert(_obj_state->os_failedobj_expiry_msec == 0 || _obj_state->obj); \ + if (_self) { \ + if (c_list_is_empty(&_obj_state->os_zombie_lst)) { \ + nm_assert(_self->priv.p->combined_l3cd_commited); \ + \ + if (NM_MORE_ASSERTS > 5) { \ + nm_assert(c_list_contains(&_self->priv.p->obj_state_lst_head, \ + &_obj_state->os_lst)); \ + nm_assert(_obj_state->os_plobj \ + == nm_platform_lookup_obj(_self->priv.platform, \ + NMP_CACHE_ID_TYPE_OBJECT_TYPE, \ + _obj_state->obj)); \ + nm_assert( \ + c_list_is_empty(&obj_state->os_zombie_lst) \ + ? (_obj_state->obj \ + == nm_dedup_multi_entry_get_obj(nm_l3_config_data_lookup_obj( \ + _self->priv.p->combined_l3cd_commited, \ + _obj_state->obj))) \ + : (!nm_l3_config_data_lookup_obj( \ + _self->priv.p->combined_l3cd_commited, \ + _obj_state->obj))); \ + } \ + } \ + } \ + } \ + } \ G_STMT_END static ObjStateData * @@ -808,13 +827,14 @@ _obj_state_data_new(const NMPObject *obj, const NMPObject *plobj) obj_state = g_slice_new(ObjStateData); *obj_state = (ObjStateData){ - .obj = nmp_object_ref(obj), - .os_plobj = nmp_object_ref(plobj), - .os_was_in_platform = !!plobj, - .os_nm_configured = FALSE, - .os_dirty = FALSE, - .os_temporary_not_available_lst = C_LIST_INIT(obj_state->os_temporary_not_available_lst), - .os_zombie_lst = C_LIST_INIT(obj_state->os_zombie_lst), + .obj = nmp_object_ref(obj), + .os_plobj = nmp_object_ref(plobj), + .os_was_in_platform = !!plobj, + .os_nm_configured = FALSE, + .os_dirty = FALSE, + .os_failedobj_expiry_msec = 0, + .os_failedobj_prioq_idx = NM_PRIOQ_IDX_NULL, + .os_zombie_lst = C_LIST_INIT(obj_state->os_zombie_lst), }; return obj_state; } @@ -824,9 +844,10 @@ _obj_state_data_free(gpointer data) { ObjStateData *obj_state = data; + nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); + c_list_unlink_stale(&obj_state->os_lst); c_list_unlink_stale(&obj_state->os_zombie_lst); - c_list_unlink_stale(&obj_state->os_temporary_not_available_lst); nmp_object_unref(obj_state->obj); nmp_object_unref(obj_state->os_plobj); nm_g_slice_free(obj_state); @@ -864,15 +885,17 @@ _obj_state_data_to_string(const ObjStateData *obj_state, char *buf, gsize buf_si } else if (obj_state->os_was_in_platform) nm_strbuf_append_str(&buf, &buf_size, ", was-in-platform"); - if (obj_state->os_temporary_not_available_timestamp_msec > 0) { + if (obj_state->os_failedobj_expiry_msec > 0) { nm_utils_get_monotonic_timestamp_msec_cached(&now_msec); - nm_strbuf_append( - &buf, - &buf_size, - ", temporary-not-available-since=%" G_GINT64_FORMAT ".%03d", - (now_msec - obj_state->os_temporary_not_available_timestamp_msec) / 1000, - (int) ((now_msec - obj_state->os_temporary_not_available_timestamp_msec) % 1000)); - } + nm_strbuf_append(&buf, + &buf_size, + ", %s-since=%" G_GINT64_FORMAT ".%03d", + (obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL) ? "failed" + : "failed-wait", + (obj_state->os_failedobj_expiry_msec - now_msec) / 1000, + (int) ((obj_state->os_failedobj_expiry_msec - now_msec) % 1000)); + } else + nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); return buf0; } @@ -934,6 +957,7 @@ _obj_states_externally_removed_track(NML3Cfg *self, const NMPObject *obj, gboole if (!in_platform && !c_list_is_empty(&obj_state->os_zombie_lst)) { /* this is a zombie. We can forget about it.*/ + nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); nm_clear_nmp_object(&obj_state->os_plobj); c_list_unlink(&obj_state->os_zombie_lst); _LOGD("obj-state: zombie gone (untrack): %s", @@ -949,8 +973,23 @@ _obj_states_externally_removed_track(NML3Cfg *self, const NMPObject *obj, gboole if (in_platform) { nmp_object_ref_set(&obj_state->os_plobj, obj); obj_state->os_was_in_platform = TRUE; - _LOGD("obj-state: appeared in platform: %s", - _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + if (obj_state->os_failedobj_expiry_msec != 0) { + obj_state->os_failedobj_expiry_msec = 0; + if (obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL) { + _LOGT("obj-state: failed-obj: object now configured after failed earlier: %s", + _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + } else { + nm_prioq_remove(&self->priv.p->failedobj_prioq, + obj_state, + &obj_state->os_failedobj_prioq_idx); + _LOGT("obj-state: failed-obj: object now configured after waiting: %s", + _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + } + } else { + _LOGD("obj-state: appeared in platform: %s", + _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + } + nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); goto out; } @@ -1039,6 +1078,7 @@ _obj_states_update_all(NML3Cfg *self) continue; if (obj_state->os_plobj && obj_state->os_nm_configured) { + nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); c_list_link_tail(&self->priv.p->obj_state_zombie_lst_head, &obj_state->os_zombie_lst); obj_state->os_zombie_count = ZOMBIE_COUNT_START; @@ -1049,6 +1089,9 @@ _obj_states_update_all(NML3Cfg *self) _LOGD("obj-state: untrack: %s", _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + nm_prioq_remove(&self->priv.p->failedobj_prioq, + obj_state, + &obj_state->os_failedobj_prioq_idx); g_hash_table_iter_remove(&h_iter); } } @@ -1086,16 +1129,32 @@ _obj_states_sync_filter(NML3Cfg *self, const NMPObject *obj, NML3CfgCommitType c return TRUE; } - if (obj_state->os_temporary_not_available_timestamp_msec > 0) { - /* we currently try to configure this address (but failed earlier). - * Definitely retry. */ - return TRUE; - } - - if (!obj_state->os_plobj && commit_type != NM_L3_CFG_COMMIT_TYPE_REAPPLY - && !nmp_object_get_force_commit(obj)) - return FALSE; - + /* 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. */ return TRUE; } @@ -1129,6 +1188,7 @@ static void _commit_collect_routes(NML3Cfg *self, int addr_family, NML3CfgCommitType commit_type, + gboolean any_addrs, GPtrArray **routes, GPtrArray **routes_nodev) { @@ -1154,6 +1214,24 @@ _commit_collect_routes(NML3Cfg *self, else { nm_assert(NMP_OBJECT_CAST_IP_ROUTE(obj)->ifindex == self->priv.ifindex); + if (!any_addrs) { + /* This is a unicast route (or a similar route, which has an + * ifindex). + * + * However, during this commit we don't plan to configure any + * IP addresses. With `ipvx.method=manual` that should not be + * possible. More likely, this is because the profile has + * `ipvx.method=auto` and static routes. + * + * Don't configure any such routes before we also have at least + * one IP address. + * + * This code applies to IPv4 and IPv6, however for IPv6 we + * early on configure a link local address, so in practice the + * branch is not taken for IPv6. */ + continue; + } + if (IS_IPv4 && NMP_OBJECT_CAST_IP4_ROUTE(obj)->weight > 0) { /* This route needs to be registered as ECMP route. */ nm_netns_ip_route_ecmp_register(self->priv.netns, self, obj); @@ -1246,6 +1324,7 @@ _obj_state_zombie_lst_get_prune_lists(NML3Cfg *self, if (--obj_state->os_zombie_count == 0) { _LOGD("obj-state: prune zombie (untrack): %s", _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); g_hash_table_remove(self->priv.p->obj_state_hash, obj_state); continue; } @@ -1280,6 +1359,7 @@ _obj_state_zombie_lst_prune_all(NML3Cfg *self, int addr_family) if (--obj_state->os_zombie_count == 0) { _LOGD("obj-state: zombie pruned during reapply (untrack): %s", _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); g_hash_table_remove(self->priv.p->obj_state_hash, obj_state); continue; } @@ -3017,16 +3097,14 @@ nm_l3cfg_get_acd_addr_info(NML3Cfg *self, in_addr_t addr) /*****************************************************************************/ gboolean -nm_l3cfg_has_temp_not_available_obj(NML3Cfg *self, int addr_family) +nm_l3cfg_has_failedobj_pending(NML3Cfg *self, int addr_family) { ObjStateData *obj_state; nm_assert(NM_IS_L3CFG(self)); nm_assert_addr_family(addr_family); - c_list_for_each_entry (obj_state, - &self->priv.p->obj_state_temporary_not_available_lst_head, - os_temporary_not_available_lst) { + nm_prioq_for_each (&self->priv.p->failedobj_prioq, obj_state) { if (NMP_OBJECT_GET_ADDR_FAMILY(obj_state->obj) == addr_family) return TRUE; } @@ -3413,8 +3491,7 @@ nm_l3cfg_add_config(NML3Cfg *self, .acd_timeout_msec_confdata = acd_timeout_msec, .priority_confdata = priority, .pseudo_timestamp_confdata = ++self->priv.p->pseudo_timestamp_counter, - .force_commit_once = NM_FLAGS_HAS(config_flags, NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE), - .dirty_confdata = FALSE, + .dirty_confdata = FALSE, }; changed = TRUE; } else { @@ -3611,7 +3688,6 @@ typedef struct { NML3Cfg *self; gconstpointer tag; bool to_commit; - bool force_commit_once; } L3ConfigMergeHookAddObjData; static gboolean @@ -3629,9 +3705,6 @@ _l3_hook_add_obj_cb(const NML3ConfigData *l3cd, nm_assert(obj); nm_assert(hook_result); nm_assert(hook_result->ip4acd_not_ready == NM_OPTION_BOOL_DEFAULT); - nm_assert(hook_result->force_commit == NM_OPTION_BOOL_DEFAULT); - - hook_result->force_commit = hook_data->force_commit_once; switch (NMP_OBJECT_GET_TYPE(obj)) { case NMP_OBJECT_TYPE_IP4_ADDRESS: @@ -3787,8 +3860,7 @@ _l3cfg_update_combined_config(NML3Cfg *self, if (NM_FLAGS_HAS(l3cd_data->config_flags, NM_L3CFG_CONFIG_FLAGS_ONLY_FOR_ACD)) continue; - hook_data.tag = l3cd_data->tag_confdata; - hook_data.force_commit_once = l3cd_data->force_commit_once; + hook_data.tag = l3cd_data->tag_confdata; nm_l3_config_data_merge(l3cd, l3cd_data->l3cd, @@ -3846,7 +3918,6 @@ _l3cfg_update_combined_config(NML3Cfg *self, IS_IPv4 ? AF_INET : AF_INET6, l3cd_data->default_route_table_x[IS_IPv4], l3cd_data->default_route_metric_x[IS_IPv4], - l3cd_data->force_commit_once, l3cd_data->l3cd); } } @@ -3921,79 +3992,91 @@ out: /*****************************************************************************/ static gboolean -_routes_temporary_not_available_timeout(gpointer user_data) +_failedobj_timeout_cb(gpointer user_data) { - NML3Cfg *self = NM_L3CFG(user_data); - ObjStateData *obj_state; - gint64 now_msec; - gint64 expiry_msec; + NML3Cfg *self = NM_L3CFG(user_data); - nm_clear_g_source_inst(&self->priv.p->obj_state_temporary_not_available_timeout_source); + _LOGT("obj-state: failed-obj: handle timeout"); - obj_state = c_list_first_entry(&self->priv.p->obj_state_temporary_not_available_lst_head, - ObjStateData, - os_temporary_not_available_lst); + nm_clear_g_source_inst(&self->priv.p->failedobj_timeout_source); - if (!obj_state) - return G_SOURCE_CONTINUE; + nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO); - now_msec = nm_utils_get_monotonic_timestamp_msec(); + return G_SOURCE_CONTINUE; +} - expiry_msec = obj_state->os_temporary_not_available_timestamp_msec - + ROUTES_TEMPORARY_NOT_AVAILABLE_MAX_AGE_MSEC; +static void +_failedobj_reschedule(NML3Cfg *self, gint64 now_msec) +{ + char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; + ObjStateData *obj_state; - if (now_msec < expiry_msec) { - /* the timeout is not yet reached. Restart the timer... */ - self->priv.p->obj_state_temporary_not_available_timeout_source = - nm_g_timeout_add_source(expiry_msec - now_msec, - _routes_temporary_not_available_timeout, - self); - return G_SOURCE_CONTINUE; + nm_utils_get_monotonic_timestamp_msec_cached(&now_msec); + +again: + obj_state = nm_prioq_peek(&self->priv.p->failedobj_prioq); + + if (obj_state && obj_state->os_failedobj_expiry_msec <= now_msec) { + /* The object is already expired... */ + + /* we shouldn't have a "os_plobj", because if we had, we should have + * removed "obj_state" from the queue. */ + nm_assert(!obj_state->os_plobj); + + /* we need to have an "obj", otherwise the "obj_state" instance + * shouldn't exist (as it also has not "os_plobj"). */ + nm_assert(obj_state->obj); + + /* It seems that nm_platform_ip_route_sync() signaled success and did + * not report the route as missing. Regardless, it is still not + * configured and the timeout expired. */ + nm_prioq_remove(&self->priv.p->failedobj_prioq, + obj_state, + &obj_state->os_failedobj_prioq_idx); + _LOGW( + "missing IPv%c route: %s", + nm_utils_addr_family_to_char(NMP_OBJECT_GET_TYPE(obj_state->obj)), + nmp_object_to_string(obj_state->obj, NMP_OBJECT_TO_STRING_PUBLIC, sbuf, sizeof(sbuf))); + goto again; } - /* One (or several) routes expired. We emit a signal, but we don't schedule it again. - * We expect the callers to commit again, which will one last time try to configure - * the route. If that again fails, we detect the timeout, log a warning and don't - * track the object as not temporary-not-available anymore. */ - _nm_l3cfg_emit_signal_notify_simple( - self, - NM_L3_CONFIG_NOTIFY_TYPE_ROUTES_TEMPORARY_NOT_AVAILABLE_EXPIRED); - return G_SOURCE_CONTINUE; + if (!obj_state) { + if (nm_clear_g_source_inst(&self->priv.p->failedobj_timeout_source)) + _LOGT("obj-state: failed-obj: cancel timeout"); + return; + } + + if (nm_g_timeout_reschedule(&self->priv.p->failedobj_timeout_source, + &self->priv.p->failedobj_timeout_expiry_msec, + obj_state->os_failedobj_expiry_msec, + _failedobj_timeout_cb, + self)) { + _LOGT( + "obj-state: failed-obj: schedule timeout in %" G_GINT64_FORMAT " msec", + NM_MAX((gint64) 0, + obj_state->os_failedobj_expiry_msec - nm_utils_get_monotonic_timestamp_msec())); + } } -static gboolean -_routes_temporary_not_available_update(NML3Cfg *self, - int addr_family, - GPtrArray *routes_temporary_not_available_arr) +static void +_failedobj_handle_routes(NML3Cfg *self, int addr_family, GPtrArray *routes_failed) { - ObjStateData *obj_state; - ObjStateData *obj_state_safe; - gint64 now_msec; - gboolean prune_all = FALSE; - gboolean success = TRUE; - guint i; - const NMPClass *klass; - - klass = nmp_class_from_type(NMP_OBJECT_TYPE_IP_ROUTE(NM_IS_IPv4(addr_family))); - now_msec = nm_utils_get_monotonic_timestamp_msec(); - - if (nm_g_ptr_array_len(routes_temporary_not_available_arr) <= 0) { - prune_all = TRUE; - goto out_prune; - } + const gint64 now_msec = nm_utils_get_monotonic_timestamp_msec(); + char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; + ObjStateData *obj_state; + guint i; - c_list_for_each_entry (obj_state, - &self->priv.p->obj_state_temporary_not_available_lst_head, - os_temporary_not_available_lst) { - if (NMP_OBJECT_GET_CLASS(obj_state->obj) == klass) { - nm_assert(obj_state->os_temporary_not_available_timestamp_msec > 0); - obj_state->os_tna_dirty = TRUE; - } - } + if (!routes_failed) + return; - for (i = 0; i < routes_temporary_not_available_arr->len; i++) { - const NMPObject *o = routes_temporary_not_available_arr->pdata[i]; - char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; + for (i = 0; i < routes_failed->len; i++) { + const NMPObject *o = routes_failed->pdata[i]; + const NMPlatformIPXRoute *rt = NMP_OBJECT_CAST_IPX_ROUTE(o); + gboolean just_started_to_fail = FALSE; + gboolean just_failed = FALSE; + gboolean arm_timer = FALSE; + int grace_timeout_msec; + gint64 grace_expiry_mesc; nm_assert(NMP_OBJECT_GET_TYPE(o) == NMP_OBJECT_TYPE_IP_ROUTE(NM_IS_IPv4(addr_family))); @@ -4005,70 +4088,83 @@ _routes_temporary_not_available_update(NML3Cfg *self, continue; } - if (obj_state->os_temporary_not_available_timestamp_msec > 0) { - nm_assert(obj_state->os_temporary_not_available_timestamp_msec > 0 - && obj_state->os_temporary_not_available_timestamp_msec <= now_msec); - - if (!obj_state->os_tna_dirty) { - /* Odd, this only can happen if routes_temporary_not_available_arr contains duplicates. - * It should not. */ - nm_assert_not_reached(); - continue; - } - - if (now_msec > obj_state->os_temporary_not_available_timestamp_msec - + ROUTES_TEMPORARY_NOT_AVAILABLE_MAX_AGE_MSEC) { - /* Timeout. Could not add this address. - * - * For now, keep it obj_state->os_tna_dirty and prune it below. */ - _LOGW("failure to add IPv%c route: %s", - nm_utils_addr_family_to_char(addr_family), - nmp_object_to_string(o, NMP_OBJECT_TO_STRING_PUBLIC, sbuf, sizeof(sbuf))); - success = FALSE; - continue; - } - - obj_state->os_tna_dirty = FALSE; + if (obj_state->os_plobj) { + /* This object is apparently present in platform. Not sure what this failure report + * is about. Probably some harmless glitch. Ignore. */ continue; } - _LOGT("(temporarily) unable to add IPv%c route: %s", - nm_utils_addr_family_to_char(addr_family), - nmp_object_to_string(o, NMP_OBJECT_TO_STRING_PUBLIC, sbuf, sizeof(sbuf))); + /* This route failed, but why? That determines the grace time that we + * give before considering it bad. */ + if (!nm_ip_addr_is_null(addr_family, + nm_platform_ip_route_get_pref_src(addr_family, &rt->rx))) { + /* This route has a pref_src. A common cause for being unable to + * configure such routes, is that the referenced IP address is not + * configured/ready (yet). Give a longer timeout to this case. */ + grace_timeout_msec = 10000; + } else { + /* Other route don't have any grace time. There is no retry/wait, + * they are a failure right away. */ + grace_timeout_msec = 0; + } - obj_state->os_tna_dirty = FALSE; - obj_state->os_temporary_not_available_timestamp_msec = now_msec; - c_list_link_tail(&self->priv.p->obj_state_temporary_not_available_lst_head, - &obj_state->os_temporary_not_available_lst); - } + grace_expiry_mesc = now_msec + grace_timeout_msec; -out_prune: - c_list_for_each_entry_safe (obj_state, - obj_state_safe, - &self->priv.p->obj_state_temporary_not_available_lst_head, - os_temporary_not_available_lst) { - if (prune_all || obj_state->os_tna_dirty) { - if (NMP_OBJECT_GET_CLASS(obj_state->obj) == klass) { - obj_state->os_temporary_not_available_timestamp_msec = 0; - c_list_unlink(&obj_state->os_temporary_not_available_lst); + if (obj_state->os_failedobj_expiry_msec == 0) { + /* This is a new failure that we didn't see before... */ + obj_state->os_failedobj_expiry_msec = grace_expiry_mesc; + if (grace_timeout_msec == 0) + just_failed = TRUE; + else { + arm_timer = TRUE; + just_started_to_fail = TRUE; } + } else { + if (obj_state->os_failedobj_expiry_msec > grace_expiry_mesc) { + /* Shorten the grace timeout. We anyway rearm below... */ + obj_state->os_failedobj_expiry_msec = grace_expiry_mesc; + } + if (obj_state->os_failedobj_expiry_msec <= now_msec) { + /* The grace period is (already) expired. */ + if (obj_state->os_failedobj_prioq_idx != NM_PRIOQ_IDX_NULL) { + /* We are still tracking the element. It just is about to become failed. */ + just_failed = TRUE; + } + } else + arm_timer = TRUE; } - } - nm_clear_g_source_inst(&self->priv.p->obj_state_temporary_not_available_timeout_source); - - obj_state = c_list_first_entry(&self->priv.p->obj_state_temporary_not_available_lst_head, - ObjStateData, - os_temporary_not_available_lst); - if (obj_state) { - self->priv.p->obj_state_temporary_not_available_timeout_source = - nm_g_timeout_add_source((obj_state->os_temporary_not_available_timestamp_msec - + ROUTES_TEMPORARY_NOT_AVAILABLE_MAX_AGE_MSEC - now_msec), - _routes_temporary_not_available_timeout, - self); + nm_prioq_update(&self->priv.p->failedobj_prioq, + obj_state, + &obj_state->os_failedobj_prioq_idx, + arm_timer); + + if (just_failed) { + _LOGW("unable to configure IPv%c route: %s", + nm_utils_addr_family_to_char(addr_family), + nmp_object_to_string(o, NMP_OBJECT_TO_STRING_PUBLIC, sbuf, sizeof(sbuf))); + } else if (just_started_to_fail) { + _LOGT("obj-state: failed-obj: unable to configure %s. Wait for %d msec", + _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf)), + grace_timeout_msec); + } } +} + +static int +_failedobj_prioq_cmp(gconstpointer a, gconstpointer b) +{ + const ObjStateData *object_state_a = a; + const ObjStateData *object_state_b = b; + + nm_assert(object_state_a); + nm_assert(object_state_a->os_failedobj_expiry_msec > 0); + nm_assert(object_state_b); + nm_assert(object_state_b->os_failedobj_expiry_msec > 0); - return success; + NM_CMP_SELF(object_state_a, object_state_b); + NM_CMP_FIELD(object_state_a, object_state_b, os_failedobj_expiry_msec); + return 0; } /*****************************************************************************/ @@ -4391,6 +4487,147 @@ _rp_filter_update(NML3Cfg *self, gboolean reapply) /*****************************************************************************/ +static void +_routes_watch_ip_addrs_cb(NMNetns *netns, + NMNetnsWatcherType watcher_type, + const NMNetnsWatcherData *watcher_data, + gconstpointer tag, + const NMNetnsWatcherEventData *event_data, + gpointer user_data) +{ + const int IS_IPv4 = NM_IS_IPv4(watcher_data->ip_addr.addr.addr_family); + NML3Cfg *self = user_data; + char sbuf[NM_INET_ADDRSTRLEN]; + + if (NMP_OBJECT_CAST_IP_ADDRESS(event_data->ip_addr.obj)->ifindex == self->priv.ifindex) { + if (self->priv.p->commit_reentrant_count_ip_address_sync_x[IS_IPv4] > 0) { + /* We are currently commiting IP addresses on this very interface. + * We can ignore the event. Also, because we will sync the routes + * immediately after already. So even if somebody externally added + * the address just this very moment, we would still do the commit + * at the right time to ensure our routes are there. */ + return; + } + } + + if (event_data->ip_addr.change_type == NM_PLATFORM_SIGNAL_REMOVED) + return; + + _LOGT("watched ip-address %s changed. Schedule an idle commit", + nm_inet_ntop(watcher_data->ip_addr.addr.addr_family, + &watcher_data->ip_addr.addr.addr, + sbuf)); + nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO); +} + +static void +_routes_watch_ip_addrs(NML3Cfg *self, int addr_family, GPtrArray *addresses, GPtrArray *routes) +{ + gconstpointer const TAG = _NETNS_WATCHER_IP_ADDR_TAG(self, addr_family); + NMNetnsWatcherData watcher_data = { + .ip_addr = + { + .addr = + { + .addr_family = addr_family, + }, + }, + }; + guint i; + guint j; + + /* IP routes that have a pref_src, can only be configured in kernel if that + * address exists (and is non-tentative, in case of IPv6). That address + * might be on another interface. So we actually watch all other + * interfaces. + * + * Note that while we track failure to configure routes via "failedobj" + * mechanism, we eagerly register watchers, even if the route is already + * successfully configured or if the route is to be configure the first + * time. Maybe that could be improved, but + * - watchers should be cheap unless they notify the event. + * - upon change we do an async commit, which is maybe not entirely cheap + * but cheap enough. More importantly, committing is something that + * *always* should be permissible -- because NML3Cfg has multiple, + * independent users, that don't know about each other and which + * independently are allowed to issue a commit when they think something + * relevant changed. If there are really too many, unnecessary commits, + * then the cause needs to be understood and addressed explicitly. */ + + if (!routes) + goto out; + + for (i = 0; i < routes->len; i++) { + const NMPlatformIPRoute *rt = NMP_OBJECT_CAST_IP_ROUTE(routes->pdata[i]); + gconstpointer pref_src; + + nm_assert(NMP_OBJECT_GET_ADDR_FAMILY(routes->pdata[i]) == addr_family); + + pref_src = nm_platform_ip_route_get_pref_src(addr_family, rt); + + if (nm_ip_addr_is_null(addr_family, pref_src)) + continue; + + if (NM_IS_IPv4(addr_family)) { + if (addresses) { + /* This nested loop makes the whole operation O(n*m). We still + * do it that way, because it's probably faster to just iterate + * over the few addresses instead of building a lookup index to + * get it in O(n+m). */ + for (j = 0; j < addresses->len; j++) { + const NMPlatformIPAddress *a = NMP_OBJECT_CAST_IP_ADDRESS(addresses->pdata[j]); + + nm_assert(NMP_OBJECT_GET_ADDR_FAMILY(addresses->pdata[j]) == addr_family); + + if (nm_ip_addr_equal(addr_family, pref_src, a->address_ptr)) { + /* We optimize for the case where the required address + * is about be configured in the same commit. That is a + * common case, because our DHCP routes have prefsrc + * set, and we commonly have the respective IP address + * ready. Otherwise, the very common DHCP case would + * also require the overhead of registering a watcher + * (every time). + */ + goto next; + } + } + } + } else { + /* For IPv6, the prefsrc address must also be non-tentative (or + * IFA_F_OPTIMISTIC). So by only looking at the addresses we are + * about to configure, it's not clear whether we will be able to + * configure the route too. + * + * Maybe we could check current platform, whether the address + * exists there as non-tentative, but that seems fragile. + * + * Maybe we should only register watchers, after we encountered a + * failure to configure a route, but that seems complicated (and + * has the potential to be wrong). + * + * The overhead for always watching the IPv6 address should be + * acceptably small. So just do that. + */ + } + + nm_assert(watcher_data.ip_addr.addr.addr_family == addr_family); + nm_ip_addr_set(addr_family, &watcher_data.ip_addr.addr.addr, pref_src); + + nm_netns_watcher_add(self->priv.netns, + NM_NETNS_WATCHER_TYPE_IP_ADDR, + &watcher_data, + TAG, + _routes_watch_ip_addrs_cb, + self); +next: + (void) 0; + } + +out: + nm_netns_watcher_remove_all(self->priv.netns, TAG, FALSE); +} +/*****************************************************************************/ + static gboolean _global_tracker_mptcp_untrack(NML3Cfg *self, int addr_family) { @@ -4576,24 +4813,22 @@ _l3_commit_mptcp(NML3Cfg *self, NML3CfgCommitType commit_type) _rp_filter_update(self, reapply); } -static gboolean +static void _l3_commit_one(NML3Cfg *self, int addr_family, NML3CfgCommitType commit_type, gboolean changed_combined_l3cd, const NML3ConfigData *l3cd_old) { - const int IS_IPv4 = NM_IS_IPv4(addr_family); - gs_unref_ptrarray GPtrArray *addresses = NULL; - gs_unref_ptrarray GPtrArray *routes = NULL; - gs_unref_ptrarray GPtrArray *routes_nodev = NULL; - gs_unref_ptrarray GPtrArray *addresses_prune = NULL; - gs_unref_ptrarray GPtrArray *routes_prune = NULL; - gs_unref_ptrarray GPtrArray *routes_temporary_not_available_arr = NULL; + const int IS_IPv4 = NM_IS_IPv4(addr_family); + gs_unref_ptrarray GPtrArray *addresses = NULL; + gs_unref_ptrarray GPtrArray *routes = NULL; + gs_unref_ptrarray GPtrArray *routes_nodev = NULL; + gs_unref_ptrarray GPtrArray *addresses_prune = NULL; + gs_unref_ptrarray GPtrArray *routes_prune = NULL; + gs_unref_ptrarray GPtrArray *routes_failed = NULL; NMIPRouteTableSyncMode route_table_sync; - gboolean final_failure_for_temporary_not_available = FALSE; char sbuf_commit_type[50]; - gboolean success = TRUE; guint i; nm_assert(NM_IS_L3CFG(self)); @@ -4609,7 +4844,12 @@ _l3_commit_one(NML3Cfg *self, addresses = _commit_collect_addresses(self, addr_family, commit_type); - _commit_collect_routes(self, addr_family, commit_type, &routes, &routes_nodev); + _commit_collect_routes(self, + addr_family, + commit_type, + nm_g_ptr_array_len(addresses) > 0, + &routes, + &routes_nodev); route_table_sync = self->priv.p->combined_l3cd_commited @@ -4689,9 +4929,14 @@ _l3_commit_one(NML3Cfg *self, } } } + + _routes_watch_ip_addrs(self, addr_family, addresses, routes); + /* FIXME(l3cfg): need to honor and set nm_l3_config_data_get_ndisc_*(). */ /* FIXME(l3cfg): need to honor and set nm_l3_config_data_get_mtu(). */ + self->priv.p->commit_reentrant_count_ip_address_sync_x[IS_IPv4]++; + nm_platform_ip_address_sync(self->priv.platform, addr_family, self->priv.ifindex, @@ -4701,26 +4946,18 @@ _l3_commit_one(NML3Cfg *self, ? NMP_IP_ADDRESS_SYNC_FLAGS_NONE : NMP_IP_ADDRESS_SYNC_FLAGS_WITH_NOPREFIXROUTE); - _nodev_routes_sync(self, addr_family, commit_type, routes_nodev); - - if (!nm_platform_ip_route_sync(self->priv.platform, - addr_family, - self->priv.ifindex, - routes, - routes_prune, - &routes_temporary_not_available_arr)) - success = FALSE; + self->priv.p->commit_reentrant_count_ip_address_sync_x[IS_IPv4]--; - final_failure_for_temporary_not_available = FALSE; - if (!_routes_temporary_not_available_update(self, - addr_family, - routes_temporary_not_available_arr)) - final_failure_for_temporary_not_available = TRUE; + _nodev_routes_sync(self, addr_family, commit_type, routes_nodev); - /* FIXME(l3cfg) */ - (void) final_failure_for_temporary_not_available; + nm_platform_ip_route_sync(self->priv.platform, + addr_family, + self->priv.ifindex, + routes, + routes_prune, + &routes_failed); - return success; + _failedobj_handle_routes(self, addr_family, routes_failed); } static void @@ -4733,7 +4970,6 @@ _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle) gboolean is_sticky_update = FALSE; char sbuf_ct[30]; gboolean changed_combined_l3cd; - guint i; g_return_if_fail(NM_IS_L3CFG(self)); nm_assert(NM_IN_SET(commit_type, @@ -4794,19 +5030,12 @@ _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle) _l3_commit_one(self, AF_INET, commit_type, changed_combined_l3cd, l3cd_old); _l3_commit_one(self, AF_INET6, commit_type, changed_combined_l3cd, l3cd_old); + _failedobj_reschedule(self, 0); + _l3_commit_mptcp(self, commit_type); _l3_acd_data_process_changes(self); - if (self->priv.p->l3_config_datas) { - for (i = 0; i < self->priv.p->l3_config_datas->len; i++) { - L3ConfigData *l3_config_data = _l3_config_datas_at(self->priv.p->l3_config_datas, i); - - if (l3_config_data->force_commit_once) - l3_config_data->force_commit_once = FALSE; - } - } - nm_assert(self->priv.p->commit_reentrant_count == 1); self->priv.p->commit_reentrant_count--; @@ -5164,7 +5393,6 @@ nm_l3cfg_init(NML3Cfg *self) c_list_init(&self->priv.p->acd_event_notify_lst_head); c_list_init(&self->priv.p->commit_type_lst_head); c_list_init(&self->priv.p->obj_state_lst_head); - c_list_init(&self->priv.p->obj_state_temporary_not_available_lst_head); c_list_init(&self->priv.p->obj_state_zombie_lst_head); c_list_init(&self->priv.p->blocked_lst_head_4); c_list_init(&self->priv.p->blocked_lst_head_6); @@ -5176,6 +5404,8 @@ nm_l3cfg_init(NML3Cfg *self) nmp_object_indirect_id_equal, _obj_state_data_free, NULL); + + nm_prioq_init(&self->priv.p->failedobj_prioq, _failedobj_prioq_cmp); } static void @@ -5214,6 +5444,18 @@ finalize(GObject *object) NML3Cfg *self = NM_L3CFG(object); gboolean changed; + if (self->priv.netns) { + nm_netns_watcher_remove_all(self->priv.netns, + _NETNS_WATCHER_IP_ADDR_TAG(self, AF_INET), + TRUE); + nm_netns_watcher_remove_all(self->priv.netns, + _NETNS_WATCHER_IP_ADDR_TAG(self, AF_INET6), + TRUE); + } + + nm_prioq_destroy(&self->priv.p->failedobj_prioq); + nm_clear_g_source_inst(&self->priv.p->failedobj_timeout_source); + nm_assert(c_list_is_empty(&self->internal_netns.signal_pending_lst)); nm_assert(c_list_is_empty(&self->internal_netns.ecmp_track_ifindex_lst_head)); @@ -5241,11 +5483,8 @@ finalize(GObject *object) nm_clear_g_source_inst(&self->priv.p->nacd_instance_ensure_retry); nm_clear_g_source_inst(&self->priv.p->nacd_event_down_source); - nm_clear_g_source_inst(&self->priv.p->obj_state_temporary_not_available_timeout_source); - nm_clear_pointer(&self->priv.p->obj_state_hash, g_hash_table_destroy); nm_assert(c_list_is_empty(&self->priv.p->obj_state_lst_head)); - nm_assert(c_list_is_empty(&self->priv.p->obj_state_temporary_not_available_lst_head)); nm_assert(c_list_is_empty(&self->priv.p->obj_state_zombie_lst_head)); if (_nodev_routes_untrack(self, AF_INET)) |