NMNDisc has two implementations: lndp and fake. Fake only exists as a
stub for unit tests, otherwise there is no purpose to it. Also, we won't
ever add another implementation beside lndp. If lndp is not suitable, it
would be replaced, but not accompanied by a second implementation.
As such, nm_lndp_ndisc_get_sysctl() has no purpose to be in
"nm-lndp-ndisc.c". This split does not exist to abstract "nm-ndisc.c"
from NMPlatform. It exists to make it easier to test.
We no longer use tc objects from the platform cache; disable caching
by default.
The only exception where the cache is needed is in tc tests, as we
look into the platform there to check that objects look as expected.
Introduce a construct-only property for platform objects to enable or
disable the caching of tc objects. When disabled, the netlink socket
doesn't receive netlink events for tc objects, and objects are never
added to the cache. This commit doesn't change behavior yet.
Stage2 can be called multiple times. Ensure that tc_commit() is only
called the first time. This is important now that tc synchronization
requires to clear all qdiscs and recreate them.
Update nm_platform_qdisc_sync() and nm_platform_tfilter_sync() to
avoid looking into the platform cache, so that we no longer require to
keep tc and qdiscs in the cache.
There is no API in kernel to retrieve tc objects only for a specific
interface, so NM had to receive all tc events, even for unmanaged
interfaces. This could cause high CPU usage in some scenarios with
many objects.
Instead, try to delete root qdiscs and filters and then add the known
ones.
Also, combine the two functions together since they are related. In
particular, removing all qdiscs also removes all attached filters.
There is always a question between convenience of allowing %NULL (and
do nothing) and strictly require the user to check the argument to not
be %NULL. In this case, it's more convenient to accept NULL, than require
the callers to check for it.
"assuming" means to gracefully take over after restart. The result
should be a working configuration with a device fully managed by
NetworkManager.
If we are assuming, and the interface is down we still want to set it
up.
I don't think this warrants a warning. It's important to keep the number
of warnings and errors in the log low, and only print such messages if
there is really something that requires attention by the user. If you
run without /etc/network/interfaces, then this is pretty much expected
and the warning isn't going to tell you anything useful.
NML3Cfg tracks the state of each object (that is addresses and routes).
Previously, it had a boolean flag "os_in_platform", that should be
true if (and only if) we have a corresponding NMPObject in the platform
cache.
But NMPObjects are immutable and ref-counted. That means, we can just as
well track the reference to the NMPObject from the cache. The advantage
is that we have an index (dictionary) to find the object state, and by
tracking the platform object, we have it easily accessible.
NML3ConfigData is supposed to be immutable. It can be initialized from a
NMConnection, and its DNS priority property might be zero.
For the DNS priority, the value can be overwritten by global defaults.
We thus need to inject the default value at the right place.
We will use these values from NML3Cfg, and it seems wrong that NML3Cfg
would include "dns/nm-dns-manager.h" for this.
Enums are very "static". They have no logic, and there is less need to
separate the code well. Meaning, it doesn't hurt to define this enum
in "libnm-base/nm-base.h" which can be included by (almost) anybody.
Starting with OVS plugin installed but OVS service stopped, would lead to
<trace> [1631531732.8896] ovsdb: connect: opening /run/openvswitch/db.sock failed ("error connecting socket (No such file or directory)"). Retry with nm-sudo
...
<trace> [1631531732.9751] ovsdb: connect: failure to get FD from nm-sudo: GDBus.Error:org.gtk.GDBus.UnmappedGError.Quark._g_2dio_2derror_2dquark.Code1: error connecting socket (No such file or directory)
If we already know that the socket file does not exist, we don't need to ask nm-sudo.
That would only make sense, if nm-sudo somehow saw a different file systemd than
NetworkManager, but that is (currently) not the case.
The memory layout of the NMPlatformIPAddress structure changed. The unit test
needs to be adjusted.
Fixes: 9ec9a92f17 ('platform: avoid bitfield at end of __NMPlatformIPAddress_COMMON macro')
This helper class is supposed to encapsulate most logic about
configuring IPv6 link local addresses and exposes a simpler API in order
to simplify NMDevice. Currently this logic is spread out in NMDevice.
Also, NML3IPv6LL directly uses NML3Cfg, thereby freeing NMDevice to care
about that too much.
For several reasons, NML3IPv6LL works different than NML3IPv4LL.
For one, with IPv6 we need to configure the address in kernel, which does
DAD for us. So, NML3IPv6LL will tell NML3Cfg to configure those
addresses that it wants to probe. For IPv4, it only tells NML3Cfg to do
ACD, without configuring anything yet. That is left to the caller.
NML3Cfg tracks state about all addresses/routes. It needs that (at
least) for the following reaons:
1) if a address/route gets added by NetworkManager and then gets
externally removed then it is presumed that the user did this. In this
case, we remember that ("externally-removed") to not re-add the
address/route, until we do a full reapply. This was previously
tracked as "externally_removed_objs_hash".
2) when NML3Cfg configures a address/route in kernel, and later the
address/route is no longer to be configured, then NML3Cfg needs to
delete it again. It thus needs to remember which addresses/routes
it configured earlier to remove them. This was previously tracked via
"last_addresses_x" and "last_routes_x".
3) kernel rejects configuring certain routes while a related IPv6
address is still tentative. That means, NML3Cfg needs to detect that,
remember it, and retry later. That is previously tracked as
"routes_temporary_not_available_hash".
4) during NM_L3_CFG_COMMIT_TYPE_ASSUME, we don't remove extraneous
and don't add missing addresses/routes. This commit mode is done
while assuming a device, that is, gracefully taking over after
a restart. However, sometimes while assuming a device we forcefully
want to configure an address/route. That happens for example if we
do IPv6 link local addressing. Then we really want to add that
address/route, even in assume mode. That is what the
NM_L3CFG_CONFIG_FLAGS_ASSUME_CONFIG_ONCE flag does, and to implement
that we need to track whether we already tried to add the
address/route previously. This is something new.
Consolidate these various states in a new "obj_state_hash" and
"ObjStateData" structure. This solves above points the following way:
1) to track externally removed objects, we have a flag in ObjStateData
that indicates whether the object was every configured and whether
it currently is configured. Based on that we make decisions to
configure (or not) an address. See "_obj_states_sync_filter()".
2) we now mark objects that NML3Cfg configured, which are still in platform
and which are no longer to be configured as "zombies".
3) this is now tracked via ObjStateData's "os_temporary_not_available_lst".
4) with the available ObjStateData we can make appropriate decisions
in "_obj_states_sync_filter()".
It's a bit tricky how this flag works. It's needed for IPv6
link local addresses, which commits changes in %NM_L3_CFG_COMMIT_TYPE_ASSUME
mode. See the code comments how it works.
This commit only adds the flags and let's the NMPlatformIP{Address,Route}
properly track it. What is still needed is to actually implement any
meaning to that during the sync.
This flag is only relevant for IPv4. That is, because the way we do
ACD/DAD is fundamentally different between IPv4 and IPv6. For IPv4, we
use libn-acd while IPv6 we configure the address in kernel and wait for
the tentative flag to go away.
The host-id gets read from /var/lib/NetworkManager/secret_key, and cached in
a global variable. Other parts of the code can get the host ID using a
singleton function.
For testing, we need to inject a different host-id. Add two push/pop
functions for that.
Unlike nm_utils_host_id_get(), these functions are not thread-safe (nor
is it possible to make them thread-safe in a reasonable manner).
The name prefix "nmtst_*" is reserved for test helpers and stub
function. Such functions should not be in the actual build artifacts,
like the NetworkManager binary.
Instead, nmtst_connection_assert_unchanging() is not a test helper. It
is a assertion function that is only enabled with NM_MORE_ASSERTS
builds. That's different.
Rename.
In other words,
$ nm src/core/NetworkManager src/libnm-client-impl/.libs/libnm.so | grep nmtst
should give no results.
The check whether the current setting are already as expected are wrong.
The reason is that nm_platform_ethtool_set_link_settings() also sets
the announced ethernet modes, but nm_platform_ethtool_get_link_settings()
does not give them.
That means, we cannot check whether the current link configuration is
the same, because the getter doesn't give that information.
Consequently, we must not skip the setting on the assumption that
there is nothing to change.
This bug has bad effects. If the device is currently activated with ethtool
option set, then re-activating the profile will result in wrongly
skipping the update.
There is no point in logging the current speed/duplex. OK, with
the "*", we could at least see whether the printed values are
to be set, or are currently configured on the interface.
But mixing these two outputs is confusing and meaningless.
Either log what we are about to do, or what the current configuration
is. Not a mix of both.
It's great to have functions that cannot fail, because it allows to
skip any error handling.
_set_stable_privacy() as it was could not fail, so the only reason why
nm_utils_ipv6_addr_set_stable_privacy() could fail is because the DAD
counter exhausted.
Also, it will be useful to have a function that does not do the counter
check, where the caller wants to handle that differently.
Rename some functions, and make the core nm_utils_ipv6_addr_set_stable_privacy()
not failable.
While NMUtilsIPv6IfaceId is only 8 bytes large, it seems unidiomatic to
pass the plain struct around.
With a "const NMUtilsIPv6IfaceId *" argument it is more clear what the
meaning of this is.
Change to use pointers.
The preference for IPv6 routes was added in kernel v4.1,
22 June 2015. It is even in latest RHEL7 kernels.
Drop trying to be compatible with such old kernels.
We use extended IFA_FLAGS for IFA_F_MANAGETEMPADDR (IPv6) and
IFA_F_NOPREFIXROUTE (IPv4 and IPv6).
These flags for IPv4 were added to kernel 3.14, 30 March, 2014.
The flag for IPv4 was added to kernel 4.4, 11 January 2016.
Even latest RHEL-7 kernels have backport for IFA_F_NOPREFIXROUTE
for IPv4 (rh#1221311).
Drop this. The backward compatibility code paths are likely broken
anyway, and add considerable complexity.
This is supported since kernel 3.17, dated 5 October, 2014. Drop the backward
compatibility for that.
It's very hard to sensibly support a mode where we set the interface up,
but prevent kernel from enabling IPv6. We would hack around that by disabling
IPv6 altogether.
But these code paths are not tested and likely make no sense. And it's hard
to implement a sensible behavior in this case anyway.
The term "user_ipv6ll" is confusing and not something somebody familiar
with kernel or `ip -d link` would understand.
Also, it maps a boolean to addr-gen-mode "none" or "eui64", although
there are 2 more address generation modes in kernel.
Don't abstract the underlying API, and name things as they are in
kernel.
NMDevice's act_stage3_ip_config_start() has an out parameter,
so that an NMIPConfig object can be returned. That is (luckily)
not used much, and it's fundamentally flawed. We want that
the start method becomes simpler and idempotent. That argument
is problematic there.
Instead, of the result is already ready, postpone the activation
and process the return on an idle handler.
Why not use nm_device_set_dev2_ip_config() to pass the configuration?
Good question, who knows? For now, just mimic the previous behavior.
Usually the IP configuration would be announced late, so we can just
do that artificially by scheduling an idle action.
The idea, that callers to nm_modem_stage3_ip4_config_start() pass
a NMDeviceClass, which then might invoke a virtual function on
the class is really bad.
nm_modem_stage3_ip4_config_start() should indicate what the caller
should do.
The main problem is, that act_stage3_ip_config_start() is not supposed
to be called by anybody except NMDevice. It's an internal hook, not for
NMModem's concern.
