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).
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.
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).
- no need to call nm_platform_process_events() after
nmtstp_wait_for_signal(). The latter processes all events
that are pending.
- with addr_n number of addresses, we still only want to wait
a maximum time, not for each addresss individually. Basically,
the for-loop must be inside NMTST_WAIT(), not the other way around.
Having an unsigned "guint timeout_ms" argument is very inconvenient, because
nmtstp_wait_for_signal (NM_PLATFORM_GET (), end_time - now_time);
might easily be negative. In such a case, the correct behavior
is to wait not at all.
The previous behavior, of treating timeout_ms as *no timeout*, makes
no sense. At least not for unit tests. If you have a really long timeout,
then set it. "0" should really mean to schedule a zero timeout.
when adding a route with RTA_PREFSRC some kernel versions will reject
the request if the specified source address is still tentative: be sure
that the just added addresses are no more tentative before adding the
routes.
nmtstp_env1_add_test_func() allows to register test functions in a
particular test environment ("env1", for lack of a better name).
It will be reused for "test-route.c"
Commits 39d0559d9a ("platform: sort links by name instead of
ifindex") and 529a0a1a7f ("manager: sort slaves to be autoconnected
by device name") changed the order of activation of slaves. Introduce
a system-wide configuration property to preserve the old behavior.
https://bugzilla.redhat.com/show_bug.cgi?id=1452585
We listen to all RTM_GETLINK messages to get updates on interfaces statuses.
Unfortunately wireless code in the kernel sends those messages with wireless information included
and all other information excluded. When we receive such message we wipe out our valid cached entry
with new object that is almost empty because netlink message didn't contain any information.
Solution to this is to check that incoming message contains MTU field: this field is always
set for complete messages about interfaces and is not set by wireless code.
Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
https://github.com/NetworkManager/NetworkManager/pull/17
Arguably, we currently only have one instance of NMPlatform,
NMRouteManager, NMDefaultRouteManager -- the one owned by the
NMNetns singleton.
Hence, all these instances we create with "log-with-ptr" set explicitly
to false.
In the future we want to support namespaces, and it will be be common to
have multiple instances. For that we have "log-with-ptr" so we are able
to disambiguiate the logging.
Change the default to TRUE because it makes more sense. It has currently
no effect as the default is never used.
(cherry picked from commit 41148caba8)
NMPlatform, NMRouteManager and NMDefaultRouteManager are singletons
instances. Users of those are for example NMDevice, which registers
to GObject signals of both NMPlatform and NMRouteManager.
Hence, as NMDevice:dispose() disconnects the signal handlers, it must
ensure that those singleton instances live longer then the NMDevice
instance. That is usually accomplished by having users of singleton
instances own a reference to those instances.
For NMDevice that effectively means that it shall own a reference to
several singletons.
NMPlatform, NMRouteManager, and NMDefaultRouteManager are all
per-namespace. In general it doesn't make sense to have more then
one instances of these per name space. Nnote that currently we don't
support multiple namespaces yet. If we will ever support multiple
namespaces, then a NMDevice would have a reference to all of these
manager instances. Hence, introduce a new class NMNetns which bundles
them together.
(cherry picked from commit 0af2f5c28b)
Routes with a non-zero host part are not allowed by kernel and
don't really exist. We didn't reject such routes in users configuration,
so various part of NM allow such routes. NM should silently strip
the host part.
Extend the cache's route ID to clear the host part too.
Note that NM's handling of routes is fundamentally flawed, as
for kernels routes don't have an "id" (or rather: all properties
of a route are part of it's ID, not only the family,ifindex,
network/plen and metric tuple (see related bug rh#1337855).
(cherry picked from commit 57b0dce083)
Platform's add/remove operations accept a "network" argument.
Kernel requires that the host part (based on plen) is all zero.
For NetworkManager we are more resilient to user configuration.
Cleanup the input argument already before calling _nl_msg_new_route().
Note that we use the same "network" argument to construct a obj_id
instance and to find the route in the cache (do_add_addrroute()).
Without cleaning the host part, the added object cannot be found
and the add-route command seemingly fails.
(cherry picked from commit 11d8c41898)
The function is supposed to set *unamanged to NM_UNMANAGED's and indicate
whether NM_UNMANAGED was present in the return value.
Fixes: e32839838e
(cherry picked from commit b7b0227935)
We should try to guarantee a stable activation order of connections
across reboots; this is required, for example, for bonds because they
get assigned the MAC address of the first device enslaved, and thus
changing the activation order of slaves means also changing the MAC
address of the bond. Since we activate connections in the order links
are discovered, having a stable sorting of links returned by platform
is enough.
The ifindex of interfaces can change between reboots as it depends on
the order in which kernel discover interfaces. Provided that the
system uses a mechanism to enforce persistent interface naming (as
udev rules or systemd-udevd predictable names), and that NM starts
after all interfaces have been announced by udev, using the interface
name instead of ifindex will guarantee a consistent order.
nm_utils_exp10() is a better name, because it reminds of the function
exp10() from <math.h> which has a similar purpose (but whose argument
is double, not gint16).
There are very few places where we actually use floating point
or #include <math.h>.
Drop that library, although we very likely still get it as indirect
dependency (e.g. on my system it is still dragged in by libsystemd.so,
libudev.so and libnl-3.so).
For nl80211, we don't care about the interface name and only use it
when formatting error messages. For wext, an up-to-date interface name
should be obtained every time to minimize the chance of race
conditions when the interface is renamed.
