From 2838b1c5e893f5e22b23f18bc44f81560a1f65fa Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 11 Nov 2021 22:16:14 +0100 Subject: [PATCH] core: track force-commit flag for l3cd and platform objects Problem: if l3cfg commits an address and routes from DHCP, when the address expires those objects are removed automatically. NM tracks the objects as missing as if the user removed them. This is to prevent l3cfg to committing them again. If the lease if renewed, l3cfg should be allowed to commit those objects again. Introduce a l3cd flag to indicate that it should be force-committed once, and propagate this flag to platform objects. In this way, l3cfg can avoid committing again objects that are removed externally, but it can commit them when the l3cd changes. Fixes-test: @bridge_down_to_l2_only --- src/core/devices/nm-device.c | 44 +++++++++++++++++++++------ src/core/nm-l3-config-data.c | 14 +++++++++ src/core/nm-l3-config-data.h | 1 + src/core/nm-l3cfg.c | 26 +++++++++++++--- src/core/nm-l3cfg.h | 4 +++ src/libnm-platform/nm-platform.c | 52 +++++++++++++++++++++----------- src/libnm-platform/nm-platform.h | 17 +++++++---- src/libnm-platform/nmp-object.h | 15 +++++++++ 8 files changed, 134 insertions(+), 39 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 598d3fcd26..4cba253c85 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -3572,7 +3572,9 @@ after_merge_flags: } static gboolean -_dev_l3_register_l3cds_add_config(NMDevice *self, L3ConfigDataType l3cd_type) +_dev_l3_register_l3cds_add_config(NMDevice * self, + L3ConfigDataType l3cd_type, + NML3CfgConfigFlags flags) { NMDevicePrivate * priv = NM_DEVICE_GET_PRIVATE(self); NML3ConfigMergeFlags merge_flags; @@ -3595,15 +3597,16 @@ _dev_l3_register_l3cds_add_config(NMDevice *self, L3ConfigDataType l3cd_type) _prop_get_ipvx_dns_priority(self, AF_INET6), acd_defend_type, acd_timeout_msec, - NM_L3CFG_CONFIG_FLAGS_NONE, + flags, merge_flags); } static gboolean -_dev_l3_register_l3cds_set_one(NMDevice * self, - L3ConfigDataType l3cd_type, - const NML3ConfigData *l3cd, - NMTernary commit_sync) +_dev_l3_register_l3cds_set_one_full(NMDevice * self, + L3ConfigDataType l3cd_type, + const NML3ConfigData *l3cd, + NML3CfgConfigFlags flags, + NMTernary commit_sync) { NMDevicePrivate * priv = NM_DEVICE_GET_PRIVATE(self); nm_auto_unref_l3cd const NML3ConfigData *l3cd_old = NULL; @@ -3626,7 +3629,7 @@ _dev_l3_register_l3cds_set_one(NMDevice * self, if (priv->l3cfg) { if (priv->l3cds[l3cd_type].d) { - if (_dev_l3_register_l3cds_add_config(self, l3cd_type)) + if (_dev_l3_register_l3cds_add_config(self, l3cd_type, flags)) changed = TRUE; } @@ -3644,6 +3647,19 @@ _dev_l3_register_l3cds_set_one(NMDevice * self, return changed; } +static gboolean +_dev_l3_register_l3cds_set_one(NMDevice * self, + L3ConfigDataType l3cd_type, + const NML3ConfigData *l3cd, + NMTernary commit_sync) +{ + return _dev_l3_register_l3cds_set_one_full(self, + l3cd_type, + l3cd, + NM_L3CFG_CONFIG_FLAGS_NONE, + commit_sync); +} + static void _dev_l3_update_l3cds_ifindex(NMDevice *self) { @@ -3696,7 +3712,7 @@ _dev_l3_register_l3cds(NMDevice *self, } if (is_external) continue; - if (_dev_l3_register_l3cds_add_config(self, i)) + if (_dev_l3_register_l3cds_add_config(self, i, NM_L3CFG_CONFIG_FLAGS_NONE)) changed = TRUE; } @@ -9870,7 +9886,11 @@ _dev_ipdhcpx_handle_accept(NMDevice *self, int addr_family, const NML3ConfigData nm_assert(NM_IS_L3_CONFIG_DATA(l3cd)); nm_dhcp_config_set_lease(priv->ipdhcp_data_x[IS_IPv4].config, l3cd); - _dev_l3_register_l3cds_set_one(self, L3_CONFIG_DATA_TYPE_DHCP_X(IS_IPv4), l3cd, TRUE); + _dev_l3_register_l3cds_set_one_full(self, + L3_CONFIG_DATA_TYPE_DHCP_X(IS_IPv4), + l3cd, + NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE, + TRUE); /* FIXME(l3cfg:dhcp): accept also should be handled by NMDhcpClient transparently. * NMDhcpClient should do ACD (if enabled), and only after that passes, it exposes @@ -11074,7 +11094,11 @@ _dev_ipac6_ndisc_config_changed(NMNDisc * ndisc, _dev_ipac6_grace_period_start(self, 0, TRUE); - _dev_l3_register_l3cds_set_one(self, L3_CONFIG_DATA_TYPE_AC_6, l3cd, FALSE); + _dev_l3_register_l3cds_set_one_full(self, + L3_CONFIG_DATA_TYPE_AC_6, + l3cd, + NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE, + FALSE); nm_clear_l3cd(&priv->ipac6_data.l3cd); diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c index bf0f976252..c2afec8b7d 100644 --- a/src/core/nm-l3-config-data.c +++ b/src/core/nm-l3-config-data.c @@ -2906,6 +2906,7 @@ nm_l3_config_data_merge(NML3ConfigData * self, NML3ConfigMergeHookResult hook_result = { .ip4acd_not_ready = NM_OPTION_BOOL_DEFAULT, .assume_config_once = NM_OPTION_BOOL_DEFAULT, + .force_commit = NM_OPTION_BOOL_DEFAULT, }; #define _ensure_a() \ @@ -2944,6 +2945,12 @@ nm_l3_config_data_merge(NML3ConfigData * self, a.ax.a_assume_config_once = (!!hook_result.assume_config_once); } + if (hook_result.force_commit != NM_OPTION_BOOL_DEFAULT + && (!!hook_result.force_commit) != a_src->a_force_commit) { + _ensure_a(); + a.ax.a_force_commit = (!!hook_result.force_commit); + } + nm_l3_config_data_add_address_full(self, addr_family, a_src == &a.ax ? NULL : obj, @@ -2964,6 +2971,7 @@ nm_l3_config_data_merge(NML3ConfigData * self, NML3ConfigMergeHookResult hook_result = { .ip4acd_not_ready = NM_OPTION_BOOL_DEFAULT, .assume_config_once = NM_OPTION_BOOL_DEFAULT, + .force_commit = NM_OPTION_BOOL_DEFAULT, }; #define _ensure_r() \ @@ -2995,6 +3003,12 @@ nm_l3_config_data_merge(NML3ConfigData * self, r.rx.r_assume_config_once = (!!hook_result.assume_config_once); } + if (hook_result.force_commit != NM_OPTION_BOOL_DEFAULT + && (!!hook_result.force_commit) != r_src->r_force_commit) { + _ensure_r(); + r.rx.r_force_commit = (!!hook_result.force_commit); + } + if (!NM_FLAGS_HAS(merge_flags, NM_L3_CONFIG_MERGE_FLAGS_CLONE)) { if (r_src->table_any) { _ensure_r(); diff --git a/src/core/nm-l3-config-data.h b/src/core/nm-l3-config-data.h index dfaf18549a..b96317a89c 100644 --- a/src/core/nm-l3-config-data.h +++ b/src/core/nm-l3-config-data.h @@ -138,6 +138,7 @@ NML3ConfigData *nm_l3_config_data_new_from_platform(NMDedupMultiIndex * mu typedef struct { NMOptionBool ip4acd_not_ready; NMOptionBool assume_config_once; + NMOptionBool force_commit; } NML3ConfigMergeHookResult; typedef gboolean (*NML3ConfigMergeHookAddObj)(const NML3ConfigData * l3cd, diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 4c1f8dc029..3c8b13ddba 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -204,6 +204,7 @@ typedef struct { guint32 acd_timeout_msec_confdata; NML3AcdDefendType acd_defend_type_confdata : 3; bool dirty_confdata : 1; + gboolean force_commit_once : 1; } L3ConfigData; /*****************************************************************************/ @@ -1014,9 +1015,11 @@ _obj_states_sync_filter(/* const NMDedupMultiObj * */ gconstpointer o, gpointer const ObjStatesSyncFilterData *sync_filter_data = user_data; NMPObjectType obj_type; ObjStateData * obj_state; + NML3Cfg * self; nm_assert(sync_filter_data); nm_assert(NM_IS_L3CFG(sync_filter_data->self)); + self = sync_filter_data->self; obj_type = NMP_OBJECT_GET_TYPE(obj); @@ -1031,15 +1034,12 @@ _obj_states_sync_filter(/* const NMDedupMultiObj * */ gconstpointer o, gpointer nm_assert(c_list_is_empty(&obj_state->os_zombie_lst)); if (!obj_state->os_nm_configured) { - NML3Cfg *self; - if (sync_filter_data->commit_type == NM_L3_CFG_COMMIT_TYPE_ASSUME && !_obj_state_data_get_assume_config_once(obj_state)) return FALSE; obj_state->os_nm_configured = TRUE; - self = sync_filter_data->self; _LOGD("obj-state: configure-first-time: %s", _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); return TRUE; @@ -1051,7 +1051,8 @@ _obj_states_sync_filter(/* const NMDedupMultiObj * */ gconstpointer o, gpointer return TRUE; } - if (!obj_state->os_plobj && sync_filter_data->commit_type != NM_L3_CFG_COMMIT_TYPE_REAPPLY) + if (!obj_state->os_plobj && sync_filter_data->commit_type != NM_L3_CFG_COMMIT_TYPE_REAPPLY + && !nmp_object_get_force_commit(obj)) return FALSE; return TRUE; @@ -3200,7 +3201,8 @@ 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, - .dirty_confdata = FALSE, + .force_commit_once = NM_FLAGS_HAS(config_flags, NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE), + .dirty_confdata = FALSE, }; changed = TRUE; } else { @@ -3345,6 +3347,7 @@ typedef struct { gconstpointer tag; bool assume_config_once; bool to_commit; + bool force_commit_once; } L3ConfigMergeHookAddObjData; static gboolean @@ -3363,8 +3366,10 @@ _l3_hook_add_obj_cb(const NML3ConfigData * l3cd, nm_assert(hook_result); nm_assert(hook_result->ip4acd_not_ready == NM_OPTION_BOOL_DEFAULT); nm_assert(hook_result->assume_config_once == NM_OPTION_BOOL_DEFAULT); + nm_assert(hook_result->force_commit == NM_OPTION_BOOL_DEFAULT); hook_result->assume_config_once = hook_data->assume_config_once; + hook_result->force_commit = hook_data->force_commit_once; switch (NMP_OBJECT_GET_TYPE(obj)) { case NMP_OBJECT_TYPE_IP4_ADDRESS: @@ -3501,6 +3506,7 @@ _l3cfg_update_combined_config(NML3Cfg * self, hook_data.tag = l3cd_data->tag_confdata; hook_data.assume_config_once = NM_FLAGS_HAS(l3cd_data->config_flags, NM_L3CFG_CONFIG_FLAGS_ASSUME_CONFIG_ONCE); + hook_data.force_commit_once = l3cd_data->force_commit_once; nm_l3_config_data_merge(l3cd, l3cd_data->l3cd, @@ -4116,6 +4122,7 @@ _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, @@ -4179,6 +4186,15 @@ _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle) _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--; diff --git a/src/core/nm-l3cfg.h b/src/core/nm-l3cfg.h index 35560142be..768794389c 100644 --- a/src/core/nm-l3cfg.h +++ b/src/core/nm-l3cfg.h @@ -53,11 +53,15 @@ typedef enum _nm_packed { * "don't change" behavior. At least once. If the address/route * is still not (no longer) configured on the subsequent * commit, it's not getting added again. + * @NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE: if set, objects in the + * NML3ConfigData are committed to platform even if they were + * removed externally. */ typedef enum _nm_packed { NM_L3CFG_CONFIG_FLAGS_NONE = 0, NM_L3CFG_CONFIG_FLAGS_ONLY_FOR_ACD = (1LL << 0), NM_L3CFG_CONFIG_FLAGS_ASSUME_CONFIG_ONCE = (1LL << 1), + NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE = (1LL << 2), } NML3CfgConfigFlags; typedef enum _nm_packed { diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index ee8748c631..e9403c6189 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -6251,6 +6251,7 @@ nm_platform_ip4_address_to_string(const NMPlatformIP4Address *address, char *buf " src %s" "%s" /* a_acd_not_ready */ "%s" /* a_assume_config_once */ + "%s" /* a_force_commit */ "", s_address, address->plen, @@ -6269,7 +6270,8 @@ nm_platform_ip4_address_to_string(const NMPlatformIP4Address *address, char *buf str_label, nmp_utils_ip_config_source_to_string(address->addr_source, s_source, sizeof(s_source)), address->a_acd_not_ready ? " ip4acd-not-ready" : "", - address->a_assume_config_once ? " assume-config-once" : ""); + address->a_assume_config_once ? " assume-config-once" : "", + address->a_force_commit ? " force-commit" : ""); g_free(str_peer); return buf; } @@ -6390,6 +6392,7 @@ nm_platform_ip6_address_to_string(const NMPlatformIP6Address *address, char *buf len, "%s/%d lft %s pref %s%s%s%s%s src %s" "%s" /* a_assume_config_once */ + "%s" /* a_force_commit */ "", s_address, address->plen, @@ -6400,7 +6403,8 @@ nm_platform_ip6_address_to_string(const NMPlatformIP6Address *address, char *buf str_dev, _to_string_ifa_flags(address->n_ifa_flags, s_flags, sizeof(s_flags)), nmp_utils_ip_config_source_to_string(address->addr_source, s_source, sizeof(s_source)), - address->a_assume_config_once ? " assume-config-once" : ""); + address->a_assume_config_once ? " assume-config-once" : "", + address->a_force_commit ? " force-commit" : ""); g_free(str_peer); return buf; } @@ -6492,6 +6496,7 @@ nm_platform_ip4_route_to_string(const NMPlatformIP4Route *route, char *buf, gsiz "%s" /* initrwnd */ "%s" /* mtu */ "%s" /* r_assume_config_once */ + "%s" /* r_force_commit */ "", nm_net_aux_rtnl_rtntype_n2a_maybe_buf(nm_platform_route_type_uncoerce(route->type_coerced), str_type), @@ -6548,7 +6553,8 @@ nm_platform_ip4_route_to_string(const NMPlatformIP4Route *route, char *buf, gsiz route->lock_mtu ? "lock " : "", route->mtu) : "", - route->r_assume_config_once ? " assume-config-once" : ""); + route->r_assume_config_once ? " assume-config-once" : "", + route->r_force_commit ? " force-commit" : ""); return buf; } @@ -6619,6 +6625,7 @@ nm_platform_ip6_route_to_string(const NMPlatformIP6Route *route, char *buf, gsiz "%s" /* mtu */ "%s" /* pref */ "%s" /* r_assume_config_once */ + "%s" /* r_force_commit */ "", nm_net_aux_rtnl_rtntype_n2a_maybe_buf(nm_platform_route_type_uncoerce(route->type_coerced), str_type), @@ -6679,7 +6686,8 @@ nm_platform_ip6_route_to_string(const NMPlatformIP6Route *route, char *buf, gsiz " pref %s", nm_icmpv6_router_pref_to_string(route->rt_pref, str_pref2, sizeof(str_pref2))) : "", - route->r_assume_config_once ? " assume-config-once" : ""); + route->r_assume_config_once ? " assume-config-once" : "", + route->r_force_commit ? " force-commit" : ""); return buf; } @@ -7815,7 +7823,8 @@ nm_platform_ip4_address_hash_update(const NMPlatformIP4Address *obj, NMHashState NM_HASH_COMBINE_BOOLS(guint8, obj->use_ip4_broadcast_address, obj->a_acd_not_ready, - obj->a_assume_config_once)); + obj->a_assume_config_once, + obj->a_force_commit)); nm_hash_update_strarr(h, obj->label); } @@ -7838,23 +7847,25 @@ nm_platform_ip4_address_cmp(const NMPlatformIP4Address *a, const NMPlatformIP4Ad NM_CMP_FIELD_STR(a, b, label); NM_CMP_FIELD_UNSAFE(a, b, a_acd_not_ready); NM_CMP_FIELD_UNSAFE(a, b, a_assume_config_once); + NM_CMP_FIELD_UNSAFE(a, b, a_force_commit); return 0; } void nm_platform_ip6_address_hash_update(const NMPlatformIP6Address *obj, NMHashState *h) { - nm_hash_update_vals(h, - obj->ifindex, - obj->addr_source, - obj->timestamp, - obj->lifetime, - obj->preferred, - obj->n_ifa_flags, - obj->plen, - obj->address, - obj->peer_address, - NM_HASH_COMBINE_BOOLS(guint8, obj->a_assume_config_once)); + nm_hash_update_vals( + h, + obj->ifindex, + obj->addr_source, + obj->timestamp, + obj->lifetime, + obj->preferred, + obj->n_ifa_flags, + obj->plen, + obj->address, + obj->peer_address, + NM_HASH_COMBINE_BOOLS(guint8, obj->a_assume_config_once, obj->a_force_commit)); } int @@ -7875,6 +7886,7 @@ nm_platform_ip6_address_cmp(const NMPlatformIP6Address *a, const NMPlatformIP6Ad NM_CMP_FIELD(a, b, preferred); NM_CMP_FIELD(a, b, n_ifa_flags); NM_CMP_FIELD_UNSAFE(a, b, a_assume_config_once); + NM_CMP_FIELD_UNSAFE(a, b, a_force_commit); return 0; } @@ -7983,7 +7995,8 @@ nm_platform_ip4_route_hash_update(const NMPlatformIP4Route *obj, obj->lock_initcwnd, obj->lock_initrwnd, obj->lock_mtu, - obj->r_assume_config_once)); + obj->r_assume_config_once, + obj->r_force_commit)); break; } } @@ -8075,6 +8088,7 @@ nm_platform_ip4_route_cmp(const NMPlatformIP4Route *a, NM_CMP_FIELD(a, b, mtu); if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL) { NM_CMP_FIELD_UNSAFE(a, b, r_assume_config_once); + NM_CMP_FIELD_UNSAFE(a, b, r_force_commit); } break; } @@ -8168,7 +8182,8 @@ nm_platform_ip6_route_hash_update(const NMPlatformIP6Route *obj, obj->lock_initcwnd, obj->lock_initrwnd, obj->lock_mtu, - obj->r_assume_config_once), + obj->r_assume_config_once, + obj->r_force_commit), obj->window, obj->cwnd, obj->initcwnd, @@ -8253,6 +8268,7 @@ nm_platform_ip6_route_cmp(const NMPlatformIP6Route *a, NM_CMP_FIELD(a, b, rt_pref); if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL) { NM_CMP_FIELD_UNSAFE(a, b, r_assume_config_once); + NM_CMP_FIELD_UNSAFE(a, b, r_force_commit); } break; } diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index 4667125f24..f3a4534ce7 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -320,10 +320,12 @@ typedef enum { \ bool use_ip4_broadcast_address : 1; \ \ - /* Whether the address is should be configured once during assume. This is a meta flag - * that is not honored by NMPlatform (netlink code). Instead, it can be used by the upper - * layers which use NMPlatformIPAddress to track addresses that should be configured. */ \ + /* Meta flags not honored by NMPlatform (netlink code). Instead, they can be + * used by the upper layers which use NMPlatformIPRoute to track addresses that + * should be configured. */ \ + /* Whether the address is should be configured once during assume. */ \ bool a_assume_config_once : 1; \ + bool a_force_commit : 1; \ \ guint8 plen; \ ; @@ -467,10 +469,13 @@ typedef union { * the "table_coerced" field is ignored (unlike for the metric). */ \ bool table_any : 1; \ \ - /* Whether the route is should be configured once during assume. This is a meta flag - * that is not honored by NMPlatform (netlink code). Instead, it can be used by the upper - * layers which use NMPlatformIPRoute to track routes that should be configured. */ \ + /* Meta flags not honored by NMPlatform (netlink code). Instead, they can be + * used by the upper layers which use NMPlatformIPRoute to track routes that + * should be configured. */ \ + /* Whether the route is should be configured once during assume. */ \ bool r_assume_config_once : 1; \ + /* Whether the route should be committed even if it was removed externally. */ \ + bool r_force_commit : 1; \ \ /* rtnh_flags * diff --git a/src/libnm-platform/nmp-object.h b/src/libnm-platform/nmp-object.h index bf140d7838..9a35014fbb 100644 --- a/src/libnm-platform/nmp-object.h +++ b/src/libnm-platform/nmp-object.h @@ -1112,6 +1112,21 @@ nmp_object_get_assume_config_once(const NMPObject *obj) } } +static inline gboolean +nmp_object_get_force_commit(const NMPObject *obj) +{ + switch (NMP_OBJECT_GET_TYPE(obj)) { + case NMP_OBJECT_TYPE_IP4_ADDRESS: + case NMP_OBJECT_TYPE_IP6_ADDRESS: + return NMP_OBJECT_CAST_IP_ADDRESS(obj)->a_force_commit; + case NMP_OBJECT_TYPE_IP4_ROUTE: + case NMP_OBJECT_TYPE_IP6_ROUTE: + return NMP_OBJECT_CAST_IP_ROUTE(obj)->r_force_commit; + default: + return nm_assert_unreachable_val(FALSE); + } +} + static inline const char * nmp_object_link_get_ifname(const NMPObject *obj) {