We want to expose the NMPLookup and NMDedupMultiHeadEntry to the users
of NMPlatform, so that they can iterate the cache directly.
That means, NMPCache becames an integral part of NMPlatform's API
and must also be implemented by NMFakePlatform.
We want to move the multi_idx from NMLinuxPlatform to NMPlatform,
so that it can be used by NMFakePlatform as well. For that, we need
to know whether NMPlatform will use udev or not. Add a constrctor
property.
NMPlatformLnkMacvtap is a typedef of NMPlatformLnkMacvlan, hence, their
plobj implementation is idential. nmp_object_equal() already correctly
compares the object type, so we should hash it too.
Maintaining an index is expensive. We can merge indexes that
are strictly distinct, because one index can just partition the
objects into distinct sets.
Rework platform object cache to use NMDedupMultiIndex.
Already previously, NMPCache used NMMultiIndex and had thus
O(1) for most operations. What is new is:
- Contrary to NMMultiIndex, NMDedupMultiIndex preserves the order of
the cached items. That is crucial to handle routes properly as kernel
will replace the first matching route based on network/plen/metric
properties. See related bug rh#1337855.
Without tracking the order of routes as they are exposed
by kernel, we cannot properly maintain the route cache.
- All NMPObject instances are now treated immutable, refcounted
and get de-duplicated via NMDedupMultiIndex. This allows
to have a global NMDedupMultiIndex that can be shared with
NMIP4Config and NMRouteManager. It also allows to share the
objects themselves.
Immutable objects are so much nicer. We can get rid of the
update pre-hook callback, which was required previously because
we would mutate the object inplace. Now, we can just update
the cache, and compare obj_old and obj_new after the fact.
- NMMultiIndex was treated as an internal of NMPCache. On the other
hand, NMDedupMultiIndex exposes NMDedupMultiHeadEntry, which is
basically an object that allows to iterate over all related
objects. That means, we can now lookup objects in the cache
and give the NMDedupMultiHeadEntry instance to the caller,
which then can iterate the list on it's own -- without need
for copying anything.
Currently, at various places we still create copies of lookup
results. That can be improved later.
The ability to share NMPObject instances should enable us to
significantly improve performance and scale with large number
of routes.
Of course there is a memory overhead of having an index for each list
entry. Each NMPObject may also require an NMDedupMultiEntry,
NMDedupMultiHeadEntry, and NMDedupMultiBox item, which are tracked
in a GHashTable. Optimally, one NMDedupMultiHeadEntry is the head
for multiple objects, and NMDedupMultiBox is able to deduplicate several
NMPObjects, so that there is a net saving.
Also, each object type has several indexes of type NMPCacheIdType.
So, worst case an NMPlatformIP4Route in the platform cache is tracked
by 8 NMPCacheIdType indexes, for each we require a NMDedupMultiEntry,
plus the shared NMDedupMultiHeadEntry. The NMDedupMultiBox instance
is shared between the 8 indexes (and possibly other).
Eventually, every NMPlatformIP4Route, NMPlatformIP6Route,
NMPlatformIP4Address and NMPlatformIP6Address should be shared
an deduplicated via the global NMDedupMultiIndex instance.
As first proof of concept, refactor NMIP4Config to track
IPv4 routes via the shared multi_idx. There is later potential
for improvement, when we pass (deduplicated) NMPObject instances
around instead of plain NMPlatformIP4Route, which needs still
a lot of comparing and cloning.
NMIP4Config, NMIP6Config, and NMPlatform shall share one
NMDedupMultiIndex instance.
For that, pass an NMDedupMultiIndex instance to NMPlatform and NMNetns.
NMNetns than passes it on to NMDevice, NMDhcpClient, NMIP4Config and NMIP6Config.
So currently NMNetns is the access point to the shared NMDedupMultiIndex
instance, and it gets it from it's NMPlatform instance.
The NMDedupMultiIndex instance is really a singleton, we don't want
multiple instances of it. However, for testing, instead of adding a
singleton instance, pass the instance explicitly around.
Add the NMDedupMultiIndex cache. It basically tracks
objects as doubly linked list. With the addition that
each object and the list head is indexed by a hash table.
Also, it supports tracking multiple distinct lists,
all indexed by the idx-type instance.
It also deduplicates the tracked objects and shares them.
- the objects that can be put into the cache must be immutable
and ref-counted. That is, the cache will deduplicate them
and share the reference. Also, as these objects are immutable
and ref-counted, it is safe that users outside the cache
own them too (as long as they keep them immutable and manage
their reference properly).
The deduplication uses obj_id_hash_func() and obj_id_equal_func().
These functions must cover *every* aspect of the objects when
comparing equality. For example nm_platform_ip4_route_cmp()
would be a function that qualifies as obj_id_equal_func().
The cache creates references to the objects as needed and
gives them back. This happens via obj_get_ref() and
obj_put_ref(). Note that obj_get_ref() is free to create
a new object, for example to convert a stack-allocated object
to a (ref-counted) heap allocated one.
The deduplication process creates NMDedupIndexBox instances
which are the ref-counted entity. In principle, the objects
themself don't need to be ref-counted as that is handled by
the boxing instance.
- The cache doesn't only do deduplication. It is a multi-index,
meaning, callers add objects using a index handle NMDedupMultiIdxType.
The NMDedupMultiIdxType instance is the access handle to lookup
the list and objects inside the cache. Note that the idx-type
instance may partition the objects in distinct lists.
For all operations there are cross-references and hash table lookups.
Hence, every operation of this data structure is O(1) and the memory
overhead for an index tracking an object is constant.
The cache preserves ordering (due to linked list) and exposes the list
as public API. This allows users to iterate the list without any
additional copying of elements.
Platform has it's own, simple implementation of object types:
NMPObject. Extract a base type and move it to "shared/nm-utils/nm-obj.h"
so it can be reused.
The base type is trival, but it allows us to implement other objects
which are compatible with NMPObjects. Currently there is no API for generic
NMObjBaseInst type, so compatible in this case only means, that they
can be used in the same context (see example below).
The only thing that you can do with a NMObjBaseInst is check it's
NMObjBaseClass.
Incidentally, NMObjBaseInst is also made compatible to GTypeInstance.
It means, an NMObjBaseInst is not necessarily a valid GTypeInstance (like NMPObject
is not), but it could be implemented as such.
For example, you could do:
if (NMP_CLASS_IS_VALID ((NMPClass *) obj->klass)) {
/* is an NMPObject */
} else if (G_TYPE_CHECK_INSTANCE_TYPE (obj, NM_TYPE_SOMETHING)) {
/* it a NMSometing GType */
} else {
/* something else? */
}
The reason why NMPObject is not implemented as proper GTypeInstance is
because it would require us to register a GType (like
g_type_register_fundamental). However, then the NMPClass struct can
no longer be const and immutable memory. But we could.
NMObjBaseInst may or may not be a GTypeInstance. In a sense, it's
a base type of GTypeInstance and all our objects should be based
on it (optionally, they we may make them valid GTypes too).
Since commit 0922a17738 ("manager: avoid that auto-activations
preempt user activations") the manager doesn't allow a internal
activation to disconnect the same connection already active on the
device. Thus, during a rollback we must ensure that the device is
deactivated before.
Fixes: 0922a17738
Revert the change to initialize the GDBusObjectManager
synchronously. I still maintain that GDBusObjectManager's
async initialization is severely broken on it's own, but this
change broke
$ systemctl restart NetworkManager
$ nmcli connection up "$NAME"
leaving nmcli hanging, as libnm fails to process some D-Bus signals.
I think that hints to another bug, but apparently the previous
version was less broken. So revert for now.
https://bugzilla.redhat.com/show_bug.cgi?id=1450075
Strangely, this breaks
systemctl restart NetworkManager
nmcli connection up "$NAME"
It seems that with this change, libnm misses some events from D-Bus.
It looks like there is something seriously broken. Before fixing it,
revert the previous state.
https://bugzilla.redhat.com/show_bug.cgi?id=1450075
This reverts commit 529d620a59.
On ppc archtecture the "nm_bt_vtable_network_server" symbol in the small
objects section instead of .bss, represencted by a "S" letter. Also
include "G" which is an equivalent thing for initialized data. We don't
seem to have such objects at the moment, but when we do it could result
in a nasty surprise.
For consistency, never return an empty array @values.
If we have an empty array, instead return NULL.
Also fixes commit afac7621a "clients: return NULL array on
auto-completion failure", which claims that readline crashes
with empty strv arrays.
Fixes: afac7621ae
$ nmcli --complete-args connection import type non-existing-<TAB>
Leads to a double-free of out_to_free, as we call g_free(v) in
nm_meta_abstract_info_complete().
Also fix a memleak when skipping over non-matching values.
Fixes: afac7621ae
Commit df0dc912cc ("8021x: don't request secrets if they are empty
and system owned") changed need_private_key_password() to return FALSE
when flags are NONE. This broke authentication using an encrypted
private key because after this the key password is never added to the
applied connection.
Don't require a password with NONE flags only for the PKCS11 scheme.
Fixes: df0dc912cc
The default value for miimon, when missing in the setting, is 0 if
arp_interval is != 0, and 100 otherwise. So, when generating a
connection, let's ignore miimon=0 (which means that miimon is
disabled) and accept any other value. Adding miimon=100 does not cause
any harm to the connection assumption.
While at it, slightly improve the code: ignore_if_zero() is not useful
for 'updelay','downdelay','arp_interval' because zero is their default
value, so introduce a new function that checks if the value is the
default (and specially handles 'miimon').
Reported-by: Taketo Kabe <rkabe@vega.pgw.jp>
https://bugzilla.redhat.com/show_bug.cgi?id=1463077
