colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

l3cfg: simplify signals

Browse Source 
During a commit of layer-3 configuration, multiple signals are
emitted:

 - if the combined l3cd configuration changes, we first emit a
   L3CD_CHANGED signal, with flag `commited` FALSE;
 - if the previously committed configuration is different from the one
   we want to commit, we emit again the same signal with `commited`
   TRUE;
 - a PRE_COMMIT signal
 - a POST_COMMIT signal

The usefulness of the first and third signals is questionable: there
is no need to signal that the configuration changes if we are not
going to commit it. Also, PRE_COMMIT is redundant as we just emitted
L3CD_CHANGED. Nobody is using those 2 signals.

Simplify this by leaving only PRE_COMMIT and POST_COMMIT, which are
always emitted during a commit and provide information on the l3cd
changes.

This commit doesn't change behavior.
This commit is contained in:
Beniamino Galvani
2024-10-03 21:33:37 +02:00
parent 3e93134c04
commit 3eb45c1d40
5 changed files with 50 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/core/devices/nm-device.c
View File

@@ -4654,13 +4654,13 @@ _dev_l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, N
nm_assert(l3cfg == priv->l3cfg); nm_assert(l3cfg == priv->l3cfg);
switch (notify_data->notify_type) { switch (notify_data->notify_type) {
case NM_L3_CONFIG_NOTIFY_TYPE_L3CD_CHANGED: case NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT:
if (notify_data->l3cd_changed.commited) { if (notify_data->commit.l3cd_changed) {
g_signal_emit(self, g_signal_emit(self,
signals[L3CD_CHANGED], signals[L3CD_CHANGED],
0, 0,
notify_data->l3cd_changed.l3cd_old, notify_data->commit.l3cd_old,
notify_data->l3cd_changed.l3cd_new); notify_data->commit.l3cd_new);
} }
return; return;
case NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT: case NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT:

6
src/core/nm-ip-config.c
View File

@@ -142,9 +142,9 @@ static void
_l3cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMIPConfig *self) _l3cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMIPConfig *self)
{ {
switch (notify_data->notify_type) { switch (notify_data->notify_type) {
case NM_L3_CONFIG_NOTIFY_TYPE_L3CD_CHANGED: case NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT:
if (notify_data->l3cd_changed.commited) if (notify_data->commit.l3cd_changed)
_handle_l3cd_changed(self, notify_data->l3cd_changed.l3cd_new); _handle_l3cd_changed(self, notify_data->commit.l3cd_new);
break; break;
case NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE: case NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE:
_notify_platform(self, notify_data->platform_change_on_idle.obj_type_flags); _notify_platform(self, notify_data->platform_change_on_idle.obj_type_flags);

66
src/core/nm-l3cfg.c
View File

@@ -436,7 +436,6 @@ static NM_UTILS_ENUM2STR_DEFINE(
NML3ConfigNotifyType, NML3ConfigNotifyType,
NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT, "acd-event"), NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT, "acd-event"),
NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_IPV4LL_EVENT, "ipv4ll-event"), NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_IPV4LL_EVENT, "ipv4ll-event"),
NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_L3CD_CHANGED, "l3cd-changed"),
NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE, "platform-change"), NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE, "platform-change"),
NM_UTILS_ENUM2STR(NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE, "platform-change-on-idle"), 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_PRE_COMMIT, "pre-commit"),
@@ -591,16 +590,17 @@ _l3_config_notify_data_to_string(const NML3ConfigNotifyData *notify_data,
nm_strbuf_seek_end(&s, &l); nm_strbuf_seek_end(&s, &l);
switch (notify_data->notify_type) { switch (notify_data->notify_type) {
case NM_L3_CONFIG_NOTIFY_TYPE_L3CD_CHANGED: case NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT:
case NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT:
nm_strbuf_append(&s, nm_strbuf_append(&s,
&l, &l,
", l3cd-old=%s", ", l3cd-old=%s",
NM_HASH_OBFUSCATE_PTR_STR(notify_data->l3cd_changed.l3cd_old, sbufobf)); NM_HASH_OBFUSCATE_PTR_STR(notify_data->commit.l3cd_old, sbufobf));
nm_strbuf_append(&s, nm_strbuf_append(&s,
&l, &l,
", l3cd-new=%s", ", l3cd-new=%s",
NM_HASH_OBFUSCATE_PTR_STR(notify_data->l3cd_changed.l3cd_new, sbufobf)); NM_HASH_OBFUSCATE_PTR_STR(notify_data->commit.l3cd_new, sbufobf));
nm_strbuf_append(&s, &l, ", commited=%d", notify_data->l3cd_changed.commited); nm_strbuf_append(&s, &l, ", l3cd-changed=%d", notify_data->commit.l3cd_changed);
break; break;
case NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT: case NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT:
nm_strbuf_append(&s, nm_strbuf_append(&s,
@@ -659,27 +659,22 @@ _nm_l3cfg_emit_signal_notify(NML3Cfg *self, const NML3ConfigNotifyData *notify_d
} }
static void static void
_nm_l3cfg_emit_signal_notify_simple(NML3Cfg *self, NML3ConfigNotifyType notify_type) _nm_l3cfg_emit_signal_notify_commit(NML3Cfg *self,
NML3ConfigNotifyType type,
const NML3ConfigData *l3cd_old,
const NML3ConfigData *l3cd_new,
gboolean l3cd_changed)
{ {
NML3ConfigNotifyData notify_data; NML3ConfigNotifyData notify_data;
notify_data.notify_type = notify_type; nm_assert(
_nm_l3cfg_emit_signal_notify(self, &notify_data); NM_IN_SET(type, NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT, NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT));
}
static void notify_data.notify_type = type;
_nm_l3cfg_emit_signal_notify_l3cd_changed(NML3Cfg *self, notify_data.commit = (typeof(notify_data.commit)){
const NML3ConfigData *l3cd_old, .l3cd_old = l3cd_old,
const NML3ConfigData *l3cd_new, .l3cd_new = l3cd_new,
gboolean commited) .l3cd_changed = l3cd_changed,
{
NML3ConfigNotifyData notify_data;
notify_data.notify_type = NM_L3_CONFIG_NOTIFY_TYPE_L3CD_CHANGED;
notify_data.l3cd_changed = (typeof(notify_data.l3cd_changed)) {
.l3cd_old = l3cd_old,
.l3cd_new = l3cd_new,
.commited = commited,
}; };
_nm_l3cfg_emit_signal_notify(self, &notify_data); _nm_l3cfg_emit_signal_notify(self, &notify_data);
} }
@@ -4068,11 +4063,6 @@ _l3cfg_update_combined_config(NML3Cfg *self,
self->priv.p->combined_l3cd_merged = nm_l3_config_data_seal(g_steal_pointer(&l3cd)); self->priv.p->combined_l3cd_merged = nm_l3_config_data_seal(g_steal_pointer(&l3cd));
merged_changed = TRUE; merged_changed = TRUE;
_nm_l3cfg_emit_signal_notify_l3cd_changed(self,
l3cd_old,
self->priv.p->combined_l3cd_merged,
FALSE);
if (!to_commit) { if (!to_commit) {
NM_SET_OUT(out_old, g_steal_pointer(&l3cd_old)); NM_SET_OUT(out_old, g_steal_pointer(&l3cd_old));
NM_SET_OUT(out_changed_combined_l3cd, TRUE); NM_SET_OUT(out_changed_combined_l3cd, TRUE);
@@ -4087,11 +4077,6 @@ out:
_obj_states_update_all(self); _obj_states_update_all(self);
_nm_l3cfg_emit_signal_notify_l3cd_changed(self,
l3cd_commited_old,
self->priv.p->combined_l3cd_commited,
TRUE);
NM_SET_OUT(out_old, g_steal_pointer(&l3cd_commited_old)); NM_SET_OUT(out_old, g_steal_pointer(&l3cd_commited_old));
NM_SET_OUT(out_changed_combined_l3cd, TRUE); NM_SET_OUT(out_changed_combined_l3cd, TRUE);
} }
@@ -4948,7 +4933,6 @@ static void
_l3_commit_one(NML3Cfg *self, _l3_commit_one(NML3Cfg *self,
int addr_family, int addr_family,
NML3CfgCommitType commit_type, NML3CfgCommitType commit_type,
gboolean changed_combined_l3cd,
const NML3ConfigData *l3cd_old) const NML3ConfigData *l3cd_old)
{ {
const int IS_IPv4 = NM_IS_IPv4(addr_family); const int IS_IPv4 = NM_IS_IPv4(addr_family);
@@ -5163,10 +5147,14 @@ _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle)
&l3cd_old, &l3cd_old,
&changed_combined_l3cd); &changed_combined_l3cd);
_nm_l3cfg_emit_signal_notify_simple(self, NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT); _nm_l3cfg_emit_signal_notify_commit(self,
NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT,
l3cd_old,
self->priv.p->combined_l3cd_commited,
changed_combined_l3cd);
_l3_commit_one(self, AF_INET, commit_type, changed_combined_l3cd, l3cd_old); _l3_commit_one(self, AF_INET, commit_type, l3cd_old);
_l3_commit_one(self, AF_INET6, commit_type, changed_combined_l3cd, l3cd_old); _l3_commit_one(self, AF_INET6, commit_type, l3cd_old);
_failedobj_reschedule(self, 0); _failedobj_reschedule(self, 0);
@@ -5177,7 +5165,11 @@ _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle)
nm_assert(self->priv.p->commit_reentrant_count == 1); nm_assert(self->priv.p->commit_reentrant_count == 1);
self->priv.p->commit_reentrant_count--; self->priv.p->commit_reentrant_count--;
_nm_l3cfg_emit_signal_notify_simple(self, NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT); _nm_l3cfg_emit_signal_notify_commit(self,
NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT,
l3cd_old,
self->priv.p->combined_l3cd_commited,
changed_combined_l3cd);
} }
NML3CfgBlockHandle * NML3CfgBlockHandle *

19
src/core/nm-l3cfg.h
View File

@@ -123,22 +123,17 @@ nm_l3_acd_addr_info_find_track_info(const NML3AcdAddrInfo *addr_info,
} }
typedef enum { typedef enum {
/* emitted when the merged/commited NML3ConfigData instance changes. NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT,
/* Emitted before the merged l3cd is committed to platform.
* Note that this gets emitted "under unsafe circumstances". That means, * Note that this gets emitted "under unsafe circumstances". That means,
* you should not perform complex operations inside this callback, * you should not perform complex operations inside this callback,
* and neither should you call into NML3Cfg again (reentrancy). */ * and neither should you call into NML3Cfg again (reentrancy). */
NM_L3_CONFIG_NOTIFY_TYPE_L3CD_CHANGED,
NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT,
/* emitted before the merged l3cd is committed to platform.
*
* This event also gets emitted "under unsafe circumstances".
* See NM_L3_CONFIG_NOTIFY_TYPE_L3CD_CHANGED. */
NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT, NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT,
/* emitted at the end of nm_l3cfg_platform_commit(). This signals also that /* emitted at the end of nm_l3cfg_platform_commit(). This signals also that
* nm_l3cfg_is_ready() might have switched to TRUE. */ * nm_l3cfg_is_ready() might have switched to TRUE. Also emitted
* "under unsafe circumstances". */
NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT, NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT,
/* NML3Cfg hooks to the NMPlatform signals for link, addresses and routes. /* NML3Cfg hooks to the NMPlatform signals for link, addresses and routes.
@@ -168,8 +163,8 @@ typedef struct {
struct { struct {
const NML3ConfigData *l3cd_old; const NML3ConfigData *l3cd_old;
const NML3ConfigData *l3cd_new; const NML3ConfigData *l3cd_new;
bool commited; bool l3cd_changed;
} l3cd_changed; } commit;
struct { struct {
NML3AcdAddrInfo info; NML3AcdAddrInfo info;

12
src/core/tests/test-l3cfg.c
View File

@@ -192,11 +192,13 @@ _test_l3cfg_signal_notify(NML3Cfg *l3cfg,
nm_assert(NM_IS_L3_CONFIG_DATA(ti->l3cd)); nm_assert(NM_IS_L3_CONFIG_DATA(ti->l3cd));
nm_assert(ti->tag); nm_assert(ti->tag);
} }
} else if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_L3CD_CHANGED) { } else if (NM_IN_SET(notify_data->notify_type,
g_assert(!notify_data->l3cd_changed.l3cd_old NM_L3_CONFIG_NOTIFY_TYPE_PRE_COMMIT,
|| NM_IS_L3_CONFIG_DATA(notify_data->l3cd_changed.l3cd_old)); NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT)) {
g_assert(!notify_data->l3cd_changed.l3cd_new g_assert(!notify_data->commit.l3cd_old
|| NM_IS_L3_CONFIG_DATA(notify_data->l3cd_changed.l3cd_new)); || NM_IS_L3_CONFIG_DATA(notify_data->commit.l3cd_old));
g_assert(!notify_data->commit.l3cd_new
|| NM_IS_L3_CONFIG_DATA(notify_data->commit.l3cd_new));
return; return;
} }