l3cfg: mediate commit_type with nm_l3cfg_commit_on_idle_schedule()

We have nm_l3cfg_commit(), however that is synchronous and triggers an
avalanche of side effects. So it should be avoided if a component is
not aware of the current circumstances in which it gets called (most of them).

The alternative is nm_l3cfg_commit_on_idle_schedule(), but previously
that only supported the auto type.

Two changes:

- add a commit_type parameter to nm_l3cfg_commit_on_idle_schedule().
  This allows to explicitly select a type for the next commit.
  Previously, if the caller wanted for example to trigger a reapply
  once, they had to register a handle, trigger the commit and unregister
  the handle again. This basically allows to specify an ad-hoc commit
  type that is only used once.

- if an explicit commit type is requested, then still always combine
  it with auto. That means, we always use the "maximum" of what is
  requested and what is registered.
This commit is contained in:
Thomas Haller 2021-09-24 19:32:03 +02:00
parent ddfd1e8ddf
commit a446e490f9
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
5 changed files with 74 additions and 49 deletions

View file

@ -602,7 +602,7 @@ _l3cd_config_add(NML3IPv4LL *self)
self->l3cfg_commit_handle = nm_l3cfg_commit_type_register(self->l3cfg,
NM_L3_CFG_COMMIT_TYPE_ASSUME,
self->l3cfg_commit_handle);
nm_l3cfg_commit_on_idle_schedule(self->l3cfg);
nm_l3cfg_commit_on_idle_schedule(self->l3cfg, NM_L3_CFG_COMMIT_TYPE_AUTO);
}
static gboolean
@ -624,7 +624,7 @@ _l3cd_config_remove(NML3IPv4LL *self)
nm_assert_not_reached();
nm_l3cfg_commit_type_unregister(self->l3cfg, g_steal_pointer(&self->l3cfg_commit_handle));
nm_l3cfg_commit_on_idle_schedule(self->l3cfg);
nm_l3cfg_commit_on_idle_schedule(self->l3cfg, NM_L3_CFG_COMMIT_TYPE_AUTO);
return TRUE;
}
@ -1028,7 +1028,7 @@ nm_l3_ipv4ll_unref(NML3IPv4LL *self)
nm_assert_not_reached();
nm_l3cfg_commit_type_unregister(self->l3cfg, g_steal_pointer(&self->l3cfg_commit_handle));
nm_l3cfg_commit_on_idle_schedule(self->l3cfg);
nm_l3cfg_commit_on_idle_schedule(self->l3cfg, NM_L3_CFG_COMMIT_TYPE_AUTO);
} else
nm_assert(!self->l3cfg_commit_handle);

View file

@ -433,7 +433,7 @@ _lladdr_handle_changed(NML3IPv6LL *self)
self->l3cfg_commit_handle);
if (changed)
nm_l3cfg_commit_on_idle_schedule(self->l3cfg);
nm_l3cfg_commit_on_idle_schedule(self->l3cfg, NM_L3_CFG_COMMIT_TYPE_AUTO);
if (!self->emit_changed_idle_source) {
_LOGT("schedule changed signal on idle");

View file

@ -256,6 +256,8 @@ typedef struct _NML3CfgPrivate {
GSource *obj_state_temporary_not_available_timeout_source;
NML3CfgCommitType commit_on_idle_type;
gint8 commit_reentrant_count;
bool commit_type_update_sticky : 1;
@ -1616,7 +1618,7 @@ _l3_acd_nacd_instance_reset(NML3Cfg *self, NMTernary start_timer, gboolean acd_d
switch (start_timer) {
case NM_TERNARY_FALSE:
_l3_changed_configs_set_dirty(self);
nm_l3cfg_commit_on_idle_schedule(self);
nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO);
break;
case NM_TERNARY_TRUE:
self->priv.p->nacd_instance_ensure_retry =
@ -2124,7 +2126,7 @@ _nm_printf(5, 6) static void _l3_acd_data_state_set_full(NML3Cfg * self,
/* The availability of an address just changed (and we are instructed to
* trigger a new commit). Do it. */
_l3_changed_configs_set_dirty(self);
nm_l3cfg_commit_on_idle_schedule(self);
nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO);
}
}
@ -2970,27 +2972,46 @@ nm_l3cfg_check_ready(NML3Cfg *self, const NML3ConfigData *l3cd, NML3CfgCheckRead
static gboolean
_l3_commit_on_idle_cb(gpointer user_data)
{
NML3CfgCommitType commit_type;
NML3Cfg *self = user_data;
nm_clear_g_source_inst(&self->priv.p->commit_on_idle_source);
commit_type = self->priv.p->commit_on_idle_type;
_LOGT("commit on idle");
_l3_commit(self, NM_L3_CFG_COMMIT_TYPE_AUTO, TRUE);
nm_clear_g_source_inst(&self->priv.p->commit_on_idle_source);
self->priv.p->commit_on_idle_type = NM_L3_CFG_COMMIT_TYPE_AUTO;
_l3_commit(self, commit_type, TRUE);
return G_SOURCE_REMOVE;
}
gboolean
nm_l3cfg_commit_on_idle_schedule(NML3Cfg *self)
nm_l3cfg_commit_on_idle_schedule(NML3Cfg *self, NML3CfgCommitType commit_type)
{
char sbuf_commit_type[50];
nm_assert(NM_IS_L3CFG(self));
nm_assert(NM_IN_SET(commit_type,
NM_L3_CFG_COMMIT_TYPE_AUTO,
NM_L3_CFG_COMMIT_TYPE_ASSUME,
NM_L3_CFG_COMMIT_TYPE_UPDATE,
NM_L3_CFG_COMMIT_TYPE_REAPPLY));
if (self->priv.p->commit_on_idle_source)
if (self->priv.p->commit_on_idle_source) {
if (self->priv.p->commit_on_idle_type < commit_type) {
_LOGT("commit on idle (scheduled) (update to %s)",
_l3_cfg_commit_type_to_string(commit_type,
sbuf_commit_type,
sizeof(sbuf_commit_type)));
self->priv.p->commit_on_idle_type = commit_type;
}
return FALSE;
}
_LOGT("commit on idle (scheduled)");
self->priv.p->commit_on_idle_source =
nm_g_idle_source_new(G_PRIORITY_DEFAULT, _l3_commit_on_idle_cb, self, NULL);
g_source_attach(self->priv.p->commit_on_idle_source, NULL);
_LOGT("commit on idle (scheduled) (%s)",
_l3_cfg_commit_type_to_string(commit_type, sbuf_commit_type, sizeof(sbuf_commit_type)));
self->priv.p->commit_on_idle_source = nm_g_idle_add_source(_l3_commit_on_idle_cb, self);
self->priv.p->commit_on_idle_type = commit_type;
return TRUE;
}
@ -3797,8 +3818,10 @@ _l3_commit_one(NML3Cfg * self,
static void
_l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle)
{
nm_auto_unref_l3cd const NML3ConfigData *l3cd_old = NULL;
gboolean commit_type_detected = FALSE;
nm_auto_unref_l3cd const NML3ConfigData *l3cd_old = NULL;
NML3CfgCommitType commit_type_auto;
gboolean commit_type_from_auto = FALSE;
gboolean is_sticky_update = FALSE;
char sbuf_ct[30];
gboolean changed_combined_l3cd;
@ -3811,50 +3834,50 @@ _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle)
NM_L3_CFG_COMMIT_TYPE_REAPPLY));
nm_assert(self->priv.p->commit_reentrant_count == 0);
switch (commit_type) {
case NM_L3_CFG_COMMIT_TYPE_AUTO:
/* if in "AUTO" mode we currently have commit-type "UPDATE", that
* causes also the following update to still be "UPDATE". Either
* the same commit */
commit_type_detected = TRUE;
commit_type = nm_l3cfg_commit_type_get(self);
if (commit_type == NM_L3_CFG_COMMIT_TYPE_UPDATE)
self->priv.p->commit_type_update_sticky = TRUE;
else if (self->priv.p->commit_type_update_sticky) {
self->priv.p->commit_type_update_sticky = FALSE;
commit_type = NM_L3_CFG_COMMIT_TYPE_UPDATE;
}
break;
case NM_L3_CFG_COMMIT_TYPE_ASSUME:
break;
case NM_L3_CFG_COMMIT_TYPE_REAPPLY:
case NM_L3_CFG_COMMIT_TYPE_UPDATE:
self->priv.p->commit_type_update_sticky = FALSE;
break;
case NM_L3_CFG_COMMIT_TYPE_NONE:
break;
/* The actual commit type is always the maximum of what is requested
* and what is registered via nm_l3cfg_commit_type_register(). */
commit_type_auto = nm_l3cfg_commit_type_get(self);
if (commit_type == NM_L3_CFG_COMMIT_TYPE_AUTO || commit_type_auto > commit_type) {
commit_type_from_auto = TRUE;
commit_type = commit_type_auto;
}
_LOGT("commit %s%s%s",
/* UPDATE and higher are sticky. That means, when do perform such a commit
* type, then the next one will at least be of level "UPDATE". The idea is
* that if the current commit adds an address, then the following needs
* to do at least "UPDATE" level to remove it again. Even if in the meantime
* the "UPDATE" is unregistered (nm_l3cfg_commit_type_unregister()). */
if (commit_type < NM_L3_CFG_COMMIT_TYPE_UPDATE) {
if (self->priv.p->commit_type_update_sticky) {
self->priv.p->commit_type_update_sticky = FALSE;
commit_type = NM_L3_CFG_COMMIT_TYPE_UPDATE;
is_sticky_update = TRUE;
}
} else
self->priv.p->commit_type_update_sticky = TRUE;
_LOGT("commit %s%s%s%s",
_l3_cfg_commit_type_to_string(commit_type, sbuf_ct, sizeof(sbuf_ct)),
commit_type_detected ? " (auto)" : "",
commit_type_from_auto ? " (auto)" : "",
is_sticky_update ? " (sticky-update)" : "",
is_idle ? " (idle handler)" : "");
if (commit_type == NM_L3_CFG_COMMIT_TYPE_NONE)
nm_assert(commit_type > NM_L3_CFG_COMMIT_TYPE_AUTO);
nm_clear_g_source_inst(&self->priv.p->commit_on_idle_source);
self->priv.p->commit_on_idle_type = NM_L3_CFG_COMMIT_TYPE_AUTO;
if (commit_type <= NM_L3_CFG_COMMIT_TYPE_NONE)
return;
self->priv.p->commit_reentrant_count++;
nm_clear_g_source_inst(&self->priv.p->commit_on_idle_source);
_l3cfg_update_combined_config(self,
TRUE,
commit_type == NM_L3_CFG_COMMIT_TYPE_REAPPLY,
&l3cd_old,
&changed_combined_l3cd);
/* FIXME(l3cfg): handle items currently not configurable in kernel. */
_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);

View file

@ -367,7 +367,7 @@ typedef enum _nm_packed {
void nm_l3cfg_commit(NML3Cfg *self, NML3CfgCommitType commit_type);
gboolean nm_l3cfg_commit_on_idle_schedule(NML3Cfg *self);
gboolean nm_l3cfg_commit_on_idle_schedule(NML3Cfg *self, NML3CfgCommitType commit_type);
gboolean nm_l3cfg_commit_on_idle_is_scheduled(NML3Cfg *self);

View file

@ -606,7 +606,8 @@ _test_l3_ipv4ll_signal_notify(NML3Cfg * l3cfg,
NM_L3CFG_CONFIG_FLAGS_NONE,
NM_L3_CONFIG_MERGE_FLAGS_NONE))
g_assert_not_reached();
nm_l3cfg_commit_on_idle_schedule(nm_l3_ipv4ll_get_l3cfg(tdata->l3ipv4ll));
nm_l3cfg_commit_on_idle_schedule(nm_l3_ipv4ll_get_l3cfg(tdata->l3ipv4ll),
NM_L3_CFG_COMMIT_TYPE_AUTO);
tdata->l3cfg_commit_type_1 =
nm_l3cfg_commit_type_register(nm_l3_ipv4ll_get_l3cfg(tdata->l3ipv4ll),
@ -626,7 +627,8 @@ _test_l3_ipv4ll_signal_notify(NML3Cfg * l3cfg,
if (!nm_l3cfg_remove_config_all(nm_l3_ipv4ll_get_l3cfg(tdata->l3ipv4ll),
TEST_L3_IPV4LL_TAG(tdata, 1)))
g_assert_not_reached();
nm_l3cfg_commit_on_idle_schedule(nm_l3_ipv4ll_get_l3cfg(tdata->l3ipv4ll));
nm_l3cfg_commit_on_idle_schedule(nm_l3_ipv4ll_get_l3cfg(tdata->l3ipv4ll),
NM_L3_CFG_COMMIT_TYPE_AUTO);
nm_l3cfg_commit_type_unregister(nm_l3_ipv4ll_get_l3cfg(tdata->l3ipv4ll),
g_steal_pointer(&tdata->l3cfg_commit_type_1));
}