Skip to content

Conversation

@thom311
Copy link
Collaborator

@thom311 thom311 commented Nov 15, 2017

except in libnm-util and libnm-glib and a few parts of test-code.

@thom311 thom311 force-pushed the th/nm-hash-all branch 2 times, most recently from eac9ab1 to db3240b Compare November 15, 2017 15:53
@bengal
Copy link
Contributor

bengal commented Nov 16, 2017

LGTM

When using siphash24(), the hash value depends on the hashed input
and the key from _get_hash_key(). If the input is static, so is also
the result of siphash24(), albeit the bits are scrabbled more.

Add a nm_hash_static() to get such a static key, but without actually
doing siphash24(). The static key is also xored with a static_seed.

For that, also mangle the first byte of the hash key using siphash24()
itself. That is, because nm_hash_static() only uses the first guint of the
random key. Hence, we want that this first guint has all the entropy
of the entire key. We use siphash24() itself, to mangle all bits
of the 16 byte key into the first guint.
siphash24() mixes the bits much better then our naive xor.
Don't bypass siphash24(). We supposedly use it for the
better hashing properties, so use it also for pointers.
Next we will use siphash24() instead of the glib version g_direct_hash() or
g_str_hash(). Hence, the "nm-utils/nm-hash-utils.h" header becomes very
fundamental and will be needed basically everywhere.

Instead of requiring the users to include them, let it be included via
"nm-default.h" header.
GHashTable optimizes a NULL equality function to use direct pointer
comparison. That saves the overhead of calling g_direct_equal().
This is also documented behavior for g_hash_table_new().

While at it, also don't pass g_direct_hash() but use the default
of %NULL. The behavior is the same, but consistently don't use
g_direct_hash().
We also do this for libnm, where it causes visible changes
in behavior. But if somebody would rely on the hashing implementation
for hash tables, it would be seriously flawed.
We also do this for libnm and libnm-core, where it causes visible changes
in behavior. But if somebody would rely on the hashing implementation
for hash tables, it would be seriously flawed.
This allows the compiler to inline the siphash24*() functions
for nm_hash_ptr() and nm_hash_str() (even without LTO).

This of course only applies to nm_hash_ptr() and nm_hash_str(),
which are implemented in "nm-hash-utils.c" itself. All other
nm_hash_*() functions are inline functions in "nm-hash-utils.h",
and thus these functions can be inlined instead. That is, in
other cases, the nm_hash_*() function instead can be inlined.
For nm_hash_ptr() and nm_hash_str() instead we want to inline the
siphash24*() functions.

So, no longer compile "siphash24.c" directly. Instead, only
build "nm-hash-utils.c" which internally #include "siphash24.c".
@lkundrak lkundrak closed this Nov 16, 2017
@lkundrak lkundrak deleted the th/nm-hash-all branch November 16, 2017 11:00
lkundrak pushed a commit that referenced this pull request Feb 2, 2021
When NM is restarted and a new object-manager is created, ensure that
signal handlers are disconnected from the old one.

Fixes the following:
  assertion failed: (object_manager == priv->object_manager)

 #0  __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 #1  __GI_abort () at abort.c:90
 #2  g_assertion_message (domain=domain@entry=0x7fcac0b845ff "libnm", file=file@entry=0x7fcac0b84c95 "libnm/nm-client.c", line=line@entry=2506, func=func@entry=0x7fcac0b863a0 <__func__.34881> "name_owner_changed", message=message@entry=0x7fcac20b05f0 "assertion failed: (object_manager == priv->object_manager)") at gtestutils.c:2429
 #3  g_assertion_message_expr (domain=domain@entry=0x7fcac0b845ff "libnm", file=file@entry=0x7fcac0b84c95 "libnm/nm-client.c", line=line@entry=2506, func=func@entry=0x7fcac0b863a0 <__func__.34881> "name_owner_changed", expr=expr@entry=0x7fcac0b856a0 "object_manager == priv->object_manager") at gtestutils.c:2444
 #4  name_owner_changed (object=<optimized out>, pspec=<optimized out>, user_data=0x7fcac204e480) at libnm/nm-client.c:2506
 #8  <emit signal notify:name-owner on instance 0x7fcac2053c60 [GDBusObjectManagerClient]> (instance=instance@entry=0x7fcac2053c60, signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3439
     #5  g_closure_invoke (closure=0x7fcac20af390, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7ffde58d9ec0, invocation_hint=invocation_hint@entry=0x7ffde58d9e60) at gclosure.c:801
     #6  signal_emit_unlocked_R (node=node@entry=0x7fcac2052090, detail=detail@entry=185, instance=instance@entry=0x7fcac2053c60, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7ffde58d9ec0) at gsignal.c:3627
     #7  g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7ffde58da050) at gsignal.c:3383
 #9  g_object_dispatch_properties_changed (object=0x7fcac2053c60 [GDBusObjectManagerClient], n_pspecs=<optimized out>, pspecs=<optimized out>) at gobject.c:1061
 #10 g_object_notify (pspec=<optimized out>, object=0x7fcac2053c60 [GDBusObjectManagerClient]) at gobject.c:1155
 #11 g_object_notify (object=object@entry=0x7fcac2053c60 [GDBusObjectManagerClient], property_name=property_name@entry=0x7fcabe9d2b29 "name-owner") at gobject.c:1202
 #12 on_notify_g_name_owner (object=<optimized out>, pspec=<optimized out>, user_data=0x7fcac2053c60) at gdbusobjectmanagerclient.c:1262
 #16 <emit signal notify:g-name-owner on instance 0x7fcaa8004440 [GDBusProxy]> (instance=instance@entry=0x7fcaa8004440, signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3439
     #13 g_closure_invoke (closure=0x7fcaa80194f0, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7ffde58da370, invocation_hint=invocation_hint@entry=0x7ffde58da310) at gclosure.c:801
     #14 signal_emit_unlocked_R (node=node@entry=0x7fcac2052090, detail=detail@entry=299, instance=instance@entry=0x7fcaa8004440, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7ffde58da370) at gsignal.c:3627
     #15 g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7ffde58da500) at gsignal.c:3383
 #17 g_object_dispatch_properties_changed (object=0x7fcaa8004440 [GDBusProxy], n_pspecs=<optimized out>, pspecs=<optimized out>) at gobject.c:1061
 #18 g_object_notify (pspec=<optimized out>, object=0x7fcaa8004440 [GDBusProxy]) at gobject.c:1155
 #19 g_object_notify (object=object@entry=0x7fcaa8004440 [GDBusProxy], property_name=property_name@entry=0x7fcabe9d2b27 "g-name-owner") at gobject.c:1202
 #20 on_name_owner_changed (connection=<optimized out>, sender_name=<optimized out>, object_path=<optimized out>, interface_name=<optimized out>, signal_name=<optimized out>, parameters=<optimized out>, user_data=0x7fcaa8015b50) at gdbusproxy.c:1353
 #21 emit_signal_instance_in_idle_cb (data=0x7fcaa40ff400) at gdbusconnection.c:3701
 #22 g_main_context_dispatch (context=0x7fcac204eea0) at gmain.c:3152
 #23 g_main_context_dispatch (context=context@entry=0x7fcac204eea0) at gmain.c:3767
 #24 g_main_context_iterate (context=context@entry=0x7fcac204eea0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3838
 #25 g_main_context_iteration (context=0x7fcac204eea0, context@entry=0x0, may_block=may_block@entry=1) at gmain.c:3899
 #26 nmc_readline_helper (prompt=prompt@entry=0x7fcac2087f60 "The connection is not saved. Do you really want to quit? (yes/no) [no] ") at clients/cli/common.c:986
 #27 nmc_readline (prompt_fmt=<optimized out>) at clients/cli/common.c:1055
 #28 confirm_quit () at clients/cli/connections.c:6459
 #29 do_connection_edit (connection_type=<optimized out>, connection=0x7fcac208eca0, nmc=0x7fcac12a3a60 <nm_cli>) at clients/cli/connections.c:7611
 #30 do_connection_edit (nmc=0x7fcac12a3a60 <nm_cli>, argc=1, argv=0x7ffde58db0b0) at clients/cli/connections.c:7948
 #31 call_cmd (nmc=0x7fcac12a3a60 <nm_cli>, simple=0x7fcac2052490 [GSimpleAsyncResult], cmd=0x7fcac1291ae0 <connection_cmds+128>, argc=2, argv=0x7ffde58db0a8) at clients/cli/common.c:1315
 #32 got_client (source_object=<optimized out>, res=<optimized out>, user_data=0x7fcac2051830) at clients/cli/common.c:1297
 #33 g_simple_async_result_complete (simple=0x7fcac2052500 [GSimpleAsyncResult]) at gsimpleasyncresult.c:801
 #34 client_inited (source=0x7fcac204e480 [NMClient], result=0x7fcac20565f0, user_data=0x7fcac2052500) at libnm/nm-client.c:1839
 #35 g_simple_async_result_complete (simple=0x7fcac20565f0 [GSimpleAsyncResult]) at gsimpleasyncresult.c:801
 #36 init_async_complete (init_data=init_data@entry=0x7fcac2046820) at libnm/nm-client.c:2339
 #37 async_inited_obj_nm (init_data=0x7fcac2046820) at libnm/nm-client.c:2337
 #38 async_inited_obj_nm (object=0x7fcac2073080 [NMRemoteConnection], result=0x7fcac2089ed0, user_data=0x7fcac2046820) at libnm/nm-client.c:2357
 #39 g_simple_async_result_complete (simple=0x7fcac2089ed0 [GSimpleAsyncResult]) at gsimpleasyncresult.c:801
 #40 init_async_parent_inited (error=0x0, init_data=0x7fcaa408a0f0) at libnm/nm-remote-connection.c:677
 #41 init_async_parent_inited (source=<optimized out>, result=<optimized out>, user_data=0x7fcaa408a0f0) at libnm/nm-remote-connection.c:689
 #42 g_simple_async_result_complete (simple=0x7fcac2089c30 [GSimpleAsyncResult]) at gsimpleasyncresult.c:801
 #43 complete_in_idle_cb (data=<optimized out>) at gsimpleasyncresult.c:813
 #44 g_main_context_dispatch (context=0x7fcac204eea0) at gmain.c:3152
 #45 g_main_context_dispatch (context=context@entry=0x7fcac204eea0) at gmain.c:3767
 #46 g_main_context_iterate (context=0x7fcac204eea0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3838
 #47 g_main_loop_run (loop=0x7fcac2045ab0) at gmain.c:4032
 #48 main (argc=<optimized out>, argv=<optimized out>) at clients/cli/nmcli.c:642

https://bugzilla.redhat.com/show_bug.cgi?id=1471245
(cherry picked from commit 7758071)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants