If we can't generate a connection and maybe_later is TRUE, it means
that the device can generate/assume connections but it failed for the
moment due to missing master/slaves/addresses. In this case, just
assume the connection from state file.
https://bugzilla.redhat.com/show_bug.cgi?id=1551958
Do not have multiple ways of expressing a certain thing. There is
a way how to express that the parser shouldn't check for keys, and
that is via the parse-information. No extra hacks.
Rework this to have a value "parser_no_check_key" so that:
- the default value for this is FALSE, so that we don't need to
explicitly set it in @parse_infos to only get the default.
Contrary to check_for_key.
- check_for_key only had meaning when also "parser" was set.
That means, the value was really "pip->parser && pip->check_for_key".
That came from the fact, that orginally this was tracked as
key_parsers array, which had "parser" always set.
That is confusing, don't do that. The field "parser_no_check_key"
has it's meaning, regardless of whether "parser" is set.
For one, these functions are not often needed. No need to define them in the
"nm-macros-internal.h" header, which is included everywhere. Move them to
"nm-shared-utils.h", which must be explicitly included.
Also, these functions are usually not called directly, but by passing their
function pointer to a sort function or similar. There is no point in having
defined in the header file.
Splitting keyfile handling in two "reader.c" and "writer.c" files
is not helpful. What is most interesting, is to see how property XYZ
is serialized to keyfile, and to verify that the parser does the
inverse. For that, it's easier if both the write_xzy() and parse_xyz()
function are beside each other, and not split accross files.
The more important reason is, that both reader and writer have their
separate handler arrays, for special handling of certain properties:
@key_parsers and @key_writers. These two should not be separate but will
be merged. Since they reference static functions, these functions must
all be in the same source file (unless, we put them into headers, which
would be unnecessary complex).
No code was changed, only moved.
The key_writers array is searched by matching the @key during
write_setting_value(). Note how write_setting_value() is called
by nm_connection_for_each_setting_value(), thus, @key is the name
of a GObject property for NMSettingIP4Config. But NMSettingIP4Config
has no property names "address-labels". Hence, this was unused
since introducing libnm-core (which never had this internal property).
With this, parsing the properties address/route (for both IPv4/IPv6)
has a runtime complexity of O(n*ln(n)).
Previously, parsing these properties was O(1), but the constant factor
was very high because for each address/route x ipv4/ipv6 combination we would
search about 2*1001 times whether there is a matching value.
Now the runtime complexity is O(n*ln(n)) for each of these 4 properties
where n is the number of entries in the keyfile.
Also note, that we only have 4 properties for which the parsing has
this complexity. Hence, parsing the entire keyfile is still O(n) + 4*O(n*ln(n))
which reduces to O(n*ln(n)). So, parsing the entire keyfile is still benign
and the logarithmic factor comes merely from sorting (which is fast).
Now, the number of supported addresses/routes is no longer limited
to 1000 (as before). Now we would accept all keys up from 0 up to
G_MAXINT32.
Like before, indexes will be automatically adjusted and gaps in the
numbering are accepted. That is convenient, if the user edits the
keyfile manually and deletes some lines. And we anyway must not change
behavior.
$ multitime -n 200 -s 0 -q ./src/settings/plugins/keyfile/tests/test-keyfile
# build with -O2 --without-more-asserts
# before:
Mean Std.Dev. Min Median Max
real 0.290+/-0.0000 0.013 0.275 0.289 0.418
user 0.284+/-0.0000 0.010 0.267 0.284 0.331
# after:
Mean Std.Dev. Min Median Max
real 0.101+/-0.0000 0.002 0.099 0.100 0.118
user 0.096+/-0.0000 0.003 0.091 0.096 0.113
sys 0.004+/-0.0000 0.002 0.001 0.004 0.009
When the D-Bus name is already taken, NM crashes in the following
way. That's because disposed object are not unexported when quitting
and so they linger in the bus-manager's list of exported objects,
causing an invalid access when a neighboring item is accessed. Instead
of just clearing the path, fully unexport the object.
The behavior of not forcefully exporting objects on quit was added in
f9ee20a7b2 ("core: explicitly unexport objects when we're done with
them"), but such behavior doesn't seem to be needed by the stated
goal.
<error> [1524062008.1886] bus-manager: fatal failure to acquire D-Bus service "org.freedesktop.NetworkManager" (3). Service already taken
<trace> [1524062008.2327] config: state: success writing state file "/var/lib/NetworkManager/NetworkManager.state"
<trace> [1524062008.2338] dns-mgr: stopping...
<info> [1524062008.2344] exiting (error)
<debug> [1524062008.2628] disposing NMManager singleton (0xce587e0)
<trace> [1524062008.2640] dns-mgr: disposing
<debug> [1524062008.2651] disposing NMDnsManager singleton (0xceb8b50)
<debug> [1524062008.2666] disposing NMFirewallManager singleton (0xceb62b0)
<debug> [1524062008.2709] disposing NMHostnameManager singleton (0xce7b370)
<trace> [1524062008.2722] dbus-object[0xce70f40]: unexport: "/org/freedesktop/NetworkManager/AgentManager"
==16381== Invalid write of size 8
==16381== at 0x42F511: c_list_unlink_stale (c-list.h:158)
==16381== by 0x42F511: c_list_unlink (c-list.h:171)
==16381== by 0x42F511: _nm_dbus_manager_obj_unexport (nm-dbus-manager.c:1135)
==16381== by 0x4C5E35: nm_dbus_object_unexport (nm-dbus-object.c:165)
==16381== by 0x5C01E9: dispose (nm-agent-manager.c:1634)
==16381== by 0x6636F37: g_object_unref (gobject.c:3303)
==16381== by 0x4BDC89: _nm_singleton_instance_destroy (nm-core-utils.c:138)
==16381== by 0x400FA85: _dl_fini (in /usr/lib64/ld-2.27.so)
==16381== by 0x7F806AB: __run_exit_handlers (in /usr/lib64/libc-2.27.so)
==16381== by 0x7F807DB: exit (in /usr/lib64/libc-2.27.so)
==16381== by 0x41DA34: main (main.c:463)
==16381== Address 0xce706a0 is 48 bytes inside a block of size 176 free'd
==16381== at 0x4C2EDAC: free (vg_replace_malloc.c:530)
==16381== by 0x6ACA3E1: g_free (gmem.c:194)
==16381== by 0x6AE2572: g_slice_free1 (gslice.c:1136)
==16381== by 0x66550AE: g_type_free_instance (gtype.c:1943)
==16381== by 0x4505F8: dispose (nm-manager.c:6867)
==16381== by 0x6636F37: g_object_unref (gobject.c:3303)
==16381== by 0x4BDC89: _nm_singleton_instance_destroy (nm-core-utils.c:138)
==16381== by 0x400FA85: _dl_fini (in /usr/lib64/ld-2.27.so)
==16381== by 0x7F806AB: __run_exit_handlers (in /usr/lib64/libc-2.27.so)
==16381== by 0x7F807DB: exit (in /usr/lib64/libc-2.27.so)
==16381== by 0x41DA34: main (main.c:463)
==16381== Block was alloc'd at
==16381== at 0x4C2DBAB: malloc (vg_replace_malloc.c:299)
==16381== by 0x6ACA2D5: g_malloc (gmem.c:99)
==16381== by 0x6AE1E36: g_slice_alloc (gslice.c:1025)
==16381== by 0x6AE247C: g_slice_alloc0 (gslice.c:1051)
==16381== by 0x6654E09: g_type_create_instance (gtype.c:1848)
==16381== by 0x66376C7: g_object_new_internal (gobject.c:1799)
==16381== by 0x6638E14: g_object_new_with_properties (gobject.c:1967)
==16381== by 0x66399D0: g_object_new (gobject.c:1639)
==16381== by 0x5D6F18: nm_settings_new (nm-settings.c:1897)
==16381== by 0x4514B4: constructed (nm-manager.c:6489)
==16381== by 0x66378FA: g_object_new_internal (gobject.c:1839)
==16381== by 0x6638E14: g_object_new_with_properties (gobject.c:1967)
https://github.com/NetworkManager/NetworkManager/pull/96
Introduce a new ifcfg-rh variable ACD_TIMEOUT that stores the exact
value of ipv4.dad-timeout without rounding. We still write the
initscripts-compatible ARPING_WAIT variable, and read it when
ACD_TIMEOUT is missing.
Print a warning whenever we find a IP conflict on the network. In the
future we may export a flag on the device or send a signal so that
clients can notify the user of the conflict.
Don't return an error from nm_arping_manager_start_probe() since it is
currently useless and the arping-manager already prints the failure
reason. Also, drop a log print from add_address().
NMArpingManager previously spawned an arping process for each
probed/announced address and watched it. This has the disadvantage of
being inefficient and also that for small timeouts we can't be sure
that arping actually started the probe.
Switch to an implementation that doesn't need to spawn external
processes, by using the n-acd code [1] currently imported in our
source tree. The long term plan is that n-acd will become a shared
library we can link against.
The file is still called nm-arping-manager for lazyness, even if a
better name would be nm-acd-manager.
[1] https://github.com/nettools/n-acd/https://bugzilla.redhat.com/show_bug.cgi?id=1507864
If the main loop is quit before the timeout expires, we leave the
timeout source running on the main loop context. Since we usually
create the main loop using the default context, the source will fire
on the next main loop we create during the test.
Therefore, destroy the timeout source if it is still active.
Fixes: 766f31507b
The README states that a kernel >= 3.0 is enough, however
CLOCK_BOOTTIME is only available since kernel 3.15.
Fall back to CLOCK_MONOTONIC when CLOCK_BOOTTIME is not available.
See: https://github.com/nettools/n-acd/pull/3
Signed-off-by: Beniamino Galvani <bgalvani@redhat.com>
During _new_active_connection() we just create the NMActiveConnection
instance to proceed with authorization. The caller might not even
authorize, so we must not touch the device yet.
Do that only later.
Often, functions perform a series of steps, and when they fail,
they bail out. It's simpler if the code is structured that way,
so you can read it from top to bottom and whenever something is
wrong, either return directly (or goto a cleanup label at the
bottom).
From the D-Bus layer, no specific-object is represented by "/". We
should early on normalize such values to NULL, and not expect or
handle them later (like during _new_active_connection()).
Merge _new_vpn_active_connection() into _new_active_connection(). It was the
only caller, and it is simpler to have all the code visible at one place.
That also shows, that the device argument is ignored and not handled.
Ensure that no device is specified for VPN type activations.
