diff options
author | Thomas Haller <thaller@redhat.com> | 2020-09-28 16:19:31 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2020-09-30 09:49:24 +0200 |
commit | e83a6f5898eec620a3d2e49c267cac618b9deee9 (patch) | |
tree | 4af64255ddf9244d2f94945f04f2b97b94ec917b | |
parent | ef198d0202341569f848f3bbf76a6751a1f621b8 (diff) | |
download | NetworkManager-e83a6f5898eec620a3d2e49c267cac618b9deee9.tar.gz |
l3cfg: combine notify_type and payload data in NM_L3Cfg_SIGNAL_NOTIFY signalth/l3cfg-11
Emitting signals is relatively expensive, because the arguments have to be packed
into a GValue. Avoid some overhad by only passing one signal argument: the notify-data
which also contains the type. Also with this we can use g_cclosure_marshal_VOID__POINTER.
Also, it's nice to have the type field part of the notify-data. Because clearly
the notify-data union is unusable without knowing the type. That means, if a user
passes the notify-data to a function, they anyway would also need to pass along
the type.
-rw-r--r-- | src/nm-l3cfg.c | 112 | ||||
-rw-r--r-- | src/nm-l3cfg.h | 7 | ||||
-rw-r--r-- | src/tests/test-l3cfg.c | 43 |
3 files changed, 74 insertions, 88 deletions
diff --git a/src/nm-l3cfg.c b/src/nm-l3cfg.c index 949d53e1a8..36aaf0fdc2 100644 --- a/src/nm-l3cfg.c +++ b/src/nm-l3cfg.c @@ -293,10 +293,9 @@ static NM_UTILS_ENUM2STR_DEFINE( /*****************************************************************************/ static const char * -_l3_config_notify_type_and_payload_to_string(NML3ConfigNotifyType notify_type, - const NML3ConfigNotifyPayload *payload, - char * sbuf, - gsize sbuf_size) +_l3_config_notify_data_to_string(const NML3ConfigNotifyData *notify_data, + char * sbuf, + gsize sbuf_size) { char sbuf_addr[NM_UTILS_INET_ADDRSTRLEN]; char *s = sbuf; @@ -305,31 +304,31 @@ _l3_config_notify_type_and_payload_to_string(NML3ConfigNotifyType noti nm_assert(sbuf); nm_assert(sbuf_size > 0); - _l3_config_notify_type_to_string(notify_type, s, l); + _l3_config_notify_type_to_string(notify_data->notify_type, s, l); nm_utils_strbuf_seek_end(&s, &l); - switch (notify_type) { + switch (notify_data->notify_type) { case NM_L3_CONFIG_NOTIFY_TYPE_ACD_COMPLETED: nm_utils_strbuf_append(&s, &l, ", addr=%s, probe-result=%d", - _nm_utils_inet4_ntop(payload->acd_completed.addr, sbuf_addr), - (int) payload->acd_completed.probe_result); + _nm_utils_inet4_ntop(notify_data->acd_completed.addr, sbuf_addr), + (int) notify_data->acd_completed.probe_result); break; case NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE: nm_utils_strbuf_append( &s, &l, ", obj-type=%s, change=%s, obj=", - NMP_OBJECT_GET_CLASS(payload->platform_change.obj)->obj_type_name, - nm_platform_signal_change_type_to_string(payload->platform_change.change_type)); - nmp_object_to_string(payload->platform_change.obj, NMP_OBJECT_TO_STRING_PUBLIC, s, l); + NMP_OBJECT_GET_CLASS(notify_data->platform_change.obj)->obj_type_name, + nm_platform_signal_change_type_to_string(notify_data->platform_change.change_type)); + nmp_object_to_string(notify_data->platform_change.obj, NMP_OBJECT_TO_STRING_PUBLIC, s, l); break; case NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE: nm_utils_strbuf_append(&s, &l, ", obj-type-flags=0x%x", - payload->platform_change_on_idle.obj_type_flags); + notify_data->platform_change_on_idle.obj_type_flags); break; default: break; @@ -339,24 +338,26 @@ _l3_config_notify_type_and_payload_to_string(NML3ConfigNotifyType noti } void -_nm_l3cfg_emit_signal_notify(NML3Cfg * self, - NML3ConfigNotifyType notify_type, - const NML3ConfigNotifyPayload *payload) +_nm_l3cfg_emit_signal_notify(NML3Cfg *self, const NML3ConfigNotifyData *notify_data) { char sbuf[sizeof(_nm_utils_to_string_buffer)]; - nm_assert(_NM_INT_NOT_NEGATIVE(notify_type)); - nm_assert(notify_type < _NM_L3_CONFIG_NOTIFY_TYPE_NUM); - nm_assert((!!payload) - == NM_IN_SET(notify_type, - NM_L3_CONFIG_NOTIFY_TYPE_ACD_COMPLETED, - NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE, - NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE)); + nm_assert(notify_data); + nm_assert(_NM_INT_NOT_NEGATIVE(notify_data->notify_type)); + nm_assert(notify_data->notify_type < _NM_L3_CONFIG_NOTIFY_TYPE_NUM); - _LOGT("emit signal (%s)", - _l3_config_notify_type_and_payload_to_string(notify_type, payload, sbuf, sizeof(sbuf))); + _LOGT("emit signal (%s)", _l3_config_notify_data_to_string(notify_data, sbuf, sizeof(sbuf))); - g_signal_emit(self, signals[SIGNAL_NOTIFY], 0, (int) notify_type, payload); + g_signal_emit(self, signals[SIGNAL_NOTIFY], 0, notify_data); +} + +static void +_nm_l3cfg_emit_signal_notify_simple(NML3Cfg *self, NML3ConfigNotifyType notify_type) +{ + NML3ConfigNotifyData notify_data; + + notify_data.notify_type = notify_type; + _nm_l3cfg_emit_signal_notify(self, ¬ify_data); } /*****************************************************************************/ @@ -675,18 +676,16 @@ _load_link(NML3Cfg *self, gboolean initial) void _nm_l3cfg_notify_platform_change_on_idle(NML3Cfg *self, guint32 obj_type_flags) { - NML3ConfigNotifyPayload payload; + NML3ConfigNotifyData notify_data; if (self->priv.plobj_next != self->priv.plobj) _load_link(self, FALSE); - payload = (NML3ConfigNotifyPayload){ - .platform_change_on_idle = - { - .obj_type_flags = obj_type_flags, - }, + notify_data.notify_type = NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE; + notify_data.platform_change_on_idle = (typeof(notify_data.platform_change_on_idle)){ + .obj_type_flags = obj_type_flags, }; - _nm_l3cfg_emit_signal_notify(self, NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE, &payload); + _nm_l3cfg_emit_signal_notify(self, ¬ify_data); _l3_acd_data_notify_acd_completed_all(self); @@ -701,8 +700,8 @@ _nm_l3cfg_notify_platform_change(NML3Cfg * self, NMPlatformSignalChangeType change_type, const NMPObject * obj) { - NML3ConfigNotifyPayload payload; - NMPObjectType obj_type; + NML3ConfigNotifyData notify_data; + NMPObjectType obj_type; nm_assert(NMP_OBJECT_IS_VALID(obj)); @@ -731,14 +730,12 @@ _nm_l3cfg_notify_platform_change(NML3Cfg * self, break; } - payload = (NML3ConfigNotifyPayload){ - .platform_change = - { - .obj = obj, - .change_type = change_type, - }, + notify_data.notify_type = NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE; + notify_data.platform_change = (typeof(notify_data.platform_change)){ + .obj = obj, + .change_type = change_type, }; - _nm_l3cfg_emit_signal_notify(self, NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE, &payload); + _nm_l3cfg_emit_signal_notify(self, ¬ify_data); nm_assert(NMP_OBJECT_IS_VALID(obj)); } @@ -1530,7 +1527,7 @@ _l3_acd_data_notify_acd_completed(NML3Cfg *self, AcdData *acd_data, gboolean for { gs_free NML3ConfigNotifyPayloadAcdFailedSource *sources_free = NULL; NML3ConfigNotifyPayloadAcdFailedSource * sources = NULL; - NML3ConfigNotifyPayload payload; + NML3ConfigNotifyData notify_data; AcdTrackData * acd_track; guint i, n; NMTernary acd_failed_notified_selector; @@ -1575,17 +1572,14 @@ _l3_acd_data_notify_acd_completed(NML3Cfg *self, AcdData *acd_data, gboolean for } nm_assert(i == n); - payload = (NML3ConfigNotifyPayload){ - .acd_completed = - { - .addr = acd_data->addr, - .probe_result = acd_data->probe_result, - .sources_len = n, - .sources = sources, - }, + notify_data.notify_type = NM_L3_CONFIG_NOTIFY_TYPE_ACD_COMPLETED; + notify_data.acd_completed = (typeof(notify_data.acd_completed)){ + .addr = acd_data->addr, + .probe_result = acd_data->probe_result, + .sources_len = n, + .sources = sources, }; - - _nm_l3cfg_emit_signal_notify(self, NM_L3_CONFIG_NOTIFY_TYPE_ACD_COMPLETED, &payload); + _nm_l3cfg_emit_signal_notify(self, ¬ify_data); for (i = 0; i < n; i++) { nmp_object_unref(sources[i].obj); @@ -2756,10 +2750,9 @@ _routes_temporary_not_available_timeout(gpointer user_data) if (any_expired) { /* a route expired. We emit a signal, but we don't schedule it again. That will * only happen if the user calls nm_l3cfg_commit() again. */ - _nm_l3cfg_emit_signal_notify( + _nm_l3cfg_emit_signal_notify_simple( self, - NM_L3_CONFIG_NOTIFY_TYPE_ROUTES_TEMPORARY_NOT_AVAILABLE_EXPIRED, - NULL); + NM_L3_CONFIG_NOTIFY_TYPE_ROUTES_TEMPORARY_NOT_AVAILABLE_EXPIRED); return G_SOURCE_REMOVE; } @@ -3070,7 +3063,7 @@ _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle) _l3_acd_data_process_changes(self); - _nm_l3cfg_emit_signal_notify(self, NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT, NULL); + _nm_l3cfg_emit_signal_notify_simple(self, NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT); } void @@ -3390,9 +3383,8 @@ nm_l3cfg_class_init(NML3CfgClass *klass) 0, NULL, NULL, - NULL, + g_cclosure_marshal_VOID__POINTER, G_TYPE_NONE, - 2, - G_TYPE_INT /* NML3ConfigNotifyType */, - G_TYPE_POINTER /* (const NML3ConfigNotifyPayload *) */); + 1, + G_TYPE_POINTER /* (const NML3ConfigNotifyData *) */); } diff --git a/src/nm-l3cfg.h b/src/nm-l3cfg.h index e122947004..9f957ae10c 100644 --- a/src/nm-l3cfg.h +++ b/src/nm-l3cfg.h @@ -48,6 +48,7 @@ typedef struct { } NML3ConfigNotifyPayloadAcdFailedSource; typedef struct { + NML3ConfigNotifyType notify_type; union { struct { in_addr_t addr; @@ -65,7 +66,7 @@ typedef struct { guint32 obj_type_flags; } platform_change_on_idle; }; -} NML3ConfigNotifyPayload; +} NML3ConfigNotifyData; struct _NML3CfgPrivate; @@ -170,9 +171,7 @@ gboolean nm_l3cfg_get_acd_is_pending(NML3Cfg *self); /*****************************************************************************/ -void _nm_l3cfg_emit_signal_notify(NML3Cfg * self, - NML3ConfigNotifyType notify_type, - const NML3ConfigNotifyPayload *pay_load); +void _nm_l3cfg_emit_signal_notify(NML3Cfg *self, const NML3ConfigNotifyData *notify_data); /*****************************************************************************/ diff --git a/src/tests/test-l3cfg.c b/src/tests/test-l3cfg.c index 486eb6261b..d55dcfbcaa 100644 --- a/src/tests/test-l3cfg.c +++ b/src/tests/test-l3cfg.c @@ -116,26 +116,21 @@ _test_l3cfg_data_set_notify_type(TestL3cfgData *tdata, TestL3cfgNotifyType notif } static void -_test_l3cfg_signal_notify(NML3Cfg * l3cfg, - int notify_type_i, - const NML3ConfigNotifyPayload *payload, - TestL3cfgData * tdata) +_test_l3cfg_signal_notify(NML3Cfg * l3cfg, + const NML3ConfigNotifyData *notify_data, + TestL3cfgData * tdata) { - NML3ConfigNotifyType l3_notify_type = notify_type_i; - g_assert(NM_IS_L3CFG(l3cfg)); g_assert(tdata); - g_assert((!!payload) - == NM_IN_SET(l3_notify_type, - NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE, - NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE, - NM_L3_CONFIG_NOTIFY_TYPE_ACD_COMPLETED)); - - if (l3_notify_type == NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE) - g_assert(payload->platform_change_on_idle.obj_type_flags != 0u); - else if (l3_notify_type == NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE) { - g_assert(NMP_OBJECT_IS_VALID(payload->platform_change.obj)); - g_assert(payload->platform_change.change_type != 0); + g_assert(notify_data); + g_assert(_NM_INT_NOT_NEGATIVE(notify_data->notify_type)); + g_assert(notify_data->notify_type < _NM_L3_CONFIG_NOTIFY_TYPE_NUM); + + if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE) + g_assert(notify_data->platform_change_on_idle.obj_type_flags != 0u); + else if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE) { + g_assert(NMP_OBJECT_IS_VALID(notify_data->platform_change.obj)); + g_assert(notify_data->platform_change.change_type != 0); } switch (tdata->notify_type) { @@ -143,7 +138,7 @@ _test_l3cfg_signal_notify(NML3Cfg * l3cfg, g_assert_not_reached(); break; case TEST_L3CFG_NOTIFY_TYPE_IDLE_ASSERT_NO_SIGNAL: - if (NM_IN_SET(l3_notify_type, + if (NM_IN_SET(notify_data->notify_type, NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE, NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE)) return; @@ -151,15 +146,15 @@ _test_l3cfg_signal_notify(NML3Cfg * l3cfg, return; case TEST_L3CFG_NOTIFY_TYPE_COMMIT_1: g_assert_cmpint(tdata->post_commit_event_count, ==, 0); - switch (l3_notify_type) { + switch (notify_data->notify_type) { case NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT: tdata->post_commit_event_count++; return; case NM_L3_CONFIG_NOTIFY_TYPE_ACD_COMPLETED: switch (tdata->f->test_idx) { case 2: - nmtst_assert_ip4_address(payload->acd_completed.addr, "192.167.133.45"); - g_assert(payload->acd_completed.probe_result); + nmtst_assert_ip4_address(notify_data->acd_completed.addr, "192.167.133.45"); + g_assert(notify_data->acd_completed.probe_result); g_assert(tdata->general_event_count == 0); tdata->general_event_count++; return; @@ -174,16 +169,16 @@ _test_l3cfg_signal_notify(NML3Cfg * l3cfg, return; } case TEST_L3CFG_NOTIFY_TYPE_WAIT_FOR_ACD_READY_1: - if (NM_IN_SET(l3_notify_type, + if (NM_IN_SET(notify_data->notify_type, NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE, NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE)) return; - if (l3_notify_type == NM_L3_CONFIG_NOTIFY_TYPE_ACD_COMPLETED) { + if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_ACD_COMPLETED) { g_assert(tdata->notify_data.wait_for_acd_ready_1.cb_count == 0); tdata->notify_data.wait_for_acd_ready_1.cb_count++; return; } - if (l3_notify_type == NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT) { + if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT) { g_assert(tdata->notify_data.wait_for_acd_ready_1.cb_count == 1); tdata->notify_data.wait_for_acd_ready_1.cb_count++; nmtstp_platform_ip_addresses_assert(tdata->f->platform, |