Add very simple periodic scanning because IWD itself only does periodic
scanning when it is in charge of autoconnecting (by policy). Since we
keep IWD out of the autoconnect state in order to use NM's autoconnect
logic, we need to request the scanning. The policy in this patch is to
use a simple 10s period between the end of one scan the requesting of
another while not connected, and 20s when connected. This is so that
users can expect similar results from both wifi backends but without
duplicating the more elaborate code in the wpa_supplicant backend which
can potentially be moved to a common superclass.
Although IFA_F_TEMPORARY is numerically equal to IFA_F_SECONDARY,
their meaning is different. One applies to IPv6 temporary addresses,
and the other to IPv4 secondary addresses.
During _addr_array_clean_expired() we want to ignore and clear
IPv6 temporary addresses, but not IPv4 secondary addresses.
Fixes: f2c4720bca
While the numerical values of IFA_F_SECONDARY and IFA_F_TEMPORARY
are identical, their meaning is not.
IFA_F_SECONDARY is only relevant for IPv4 addresses, while
IFA_F_TEMPORARY is only relevant for IPv6 addresses.
IFA_F_TEMPORARY is automatically set by kernel for the addresses
that it generates as part of IFA_F_MANAGETEMPADDR. It cannot be
actively set by user-space.
IFA_F_SECONDARY is automatically set by kernel depending on the order
in which the addresses for the same subnet are added.
This essentially reverts 8b4f11927 (core: avoid IFA_F_TEMPORARY alias for
IFA_F_SECONDARY).
In device_ipx_changed() we remember the addresses for which it appears
that DAD failed. Later, on an idle handler, we process them during
queued_ip6_config_change().
Note that nm_plaform_ip6_address_sync() might very well decide to remove
some or all addresses and re-add them immidiately later. It might do so,
to get the address priority/ordering right. At that point, we already
emit platform signals that the device disappeared, and track them in
dad6_failed_addrs.
Hence, later during queued_ip6_config_change() we must check again
whether the address is really not there and not still doing DAD.
Otherwise, we wrongly claim that DAD failed and remove the address,
generate a new one, and the same issue might happen again.
dad6_failed_addrs is populated with addresses from the platform cache.
Inside the cache, all addresses have addr_source NM_IP_CONFIG_SOURCE_KERNEL,
because addr_source property for addresses is only a property that is
used NetworkManager internally.
NMPObjects are never modified after being put into the cache.
Hence, it is safe and encouraged to just keep a reference to them,
instead of cloning them.
Interestingly, NMPlatform's change signals have a platform_object
pointer, which is not the pointer to the NMPObjects itself, but
down-cast to the NMPlatformObject instance. It does so, because commonly
callers want to have a pointer to the NMPlatformObject instance, instead
of the outer NMPObjects. However, NMP_OBJECT_UP_CAST() is guaranteed
to work one would expect.
The order in which we add addresses to dad6_failed_addrs does not
matter. Hence, use g_slist_prepend() which is O(1), instead
g_slist_append() with O(n).
We want to add addresses in a particular order so that source address
selection works.
Note that @known_addresses contains the desired addresses in order of
least-important first, while @plat_addresses contains them in opposite
order. Previously, this inverted order was not considered, and we
essentially ended up removing and re-adding all addresses every time.
Fix that. While at it, get rid of the O(n^2) runtime complexity, and
make it O(n) by iterating both lists simultaneously.
Temporary addresses (RFC4941) are not handled by NetworkManager directly, but by
kernel. If they are in the @known_addresses list, clear them out early.
They shall be ignored.
Often, we want in API that an input argument is read-only and not modified
by the function call. Not modifying input arguments is a good
convention.
However, in this case there are only two callers, and both clearly do
not care whether the @known_addresses array will be modified.
Clear out addresses that are already expired and enforce that there are
no duplicate addresses. Basically, use @known_addresses for bookkeeping
which addresses are to be ignored.
We do a pre-run that constructs an index of all addresses and drops
addresses that are already expired.
Move this code to a separate function, it will be reused for IPv6.
Also, note that nm_platform_ip4_address_sync() has only 2 callers. Both
callers make sure to not pass duplicate known addresses, because the
addresses also come from a cache. Make that a requirement and assert
against unique addresses. If we would allow duplicate addresses, we would
have to handle them in a defined way (like, dropping the ones with lower
priority). That would be more complicated, and since no caller is
supposed to provide duplicate addresses, don't bother but assert.
nm_utils_lifetime_get() already has so many arguments.
Essentially, the function returned %TRUE if and only if the
lifetime was greater then zero.
Combine the return value and the output argument for the lifetime.
It also matches better the function name: to get the lifetime.
Originally, the platform cache did not preserve any stable order.
We added that during the large cache rework. However, we still
would only care about a particular ordering for route's BY_WEAK_ID
index. For all other indexes, it was sufficient to have the
object in some arbitrary order, not necessarily the one as indicated by
kernel.
However, for addresses we actually care about the order (at least,
regarding the the OBJECT_BY_IFINDEX index, which is considered by
platform's address sync).
During a dump we get all objects in the right order. That means,
as we (re) insert the objects into the cache, we must forcefully move
them to the end of their list.
If the object didn't actually change, previously we would not have
updated their position in the cache. Fix that now.
Add a function that allows to re-request all objects of a certain type.
Usually, the cache is supposed to keep itself in a consistent state and
this function is not useful.
It is however useful during testing and debugging to explicitly reload
an object type.
If you ever think to need this function in non-testing code, then
something else is probably wrong with the cache implementation.
In ndisc_set_router_config(), we initialize NMNDiscAddress based on
NMPlatformIP6Address instances. Note that their handling of timestamps
is not entirely identical.
For convenience of the user, NMPlatformIP6Address allows to not specify
any timestamp. On the contrary, for convenience of implementation does
NMNDiscAddress always require fully specified timestamps.
Properly convert one representation into the other.
Will be used by CI trigger to name packages that are build during testing
of a github pull request with the corresponding pull request ID.
"build_clean.sh" now supports a command line option -s|--snapshot. But the
same paramter can also be set via $NM_BUILD_SNAPSHOT environment
variable. Using the environment variable is useful to support older versions
and new versions of "build_clean.sh", so that the script can just ignore the
snapshot setting if it doesn't understand it yet.
Previously we always updated resolv.conf on quit. When we are using
systemd-resolved the update is not necessary because the resolver on
127.0.0.53 would still be reachable after NM quits. Also, when NM
manages resolv.conf directly there is no need to update the file
again. Let's rewrite resolv.conf only when using dnsmasq.
https://bugzilla.redhat.com/show_bug.cgi?id=1541031
The code was passing the gpointer alias of the GValue, rather than the
GValue* itself. This doesn’t matter normally, but broke an experimental
patch in GLib to remove a cast from G_VALUE_TYPE.
We’ve reverted the patch in GLib (see
https://bugzilla.gnome.org/show_bug.cgi?id=793186), but this should be
fixed in NetworkManager anyway.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://bugzilla.gnome.org/show_bug.cgi?id=793302
GCC 8.0's -Wcast-function-type objects casting function pointers to ones
with incompatible prototypes. Sometimes we do that on purpose though.
Notably, the g_source_set_callback()'s func argument can point to functions
of various prototypes. Also, libnm-glib/nm-remote-connection is perhaps
just not worth reworking, that would just be a waste of time.
A cast to void(*)(void) avoids the GCC warning, let's use it.
This makes its prototype compatible with GDestroyNotify so that GCC 8.0
won't warn.
The return value is not used anywhere and the unref() functions typically
don't return any.
This makes its prototype compatible with GDestroyNotify so that GCC 8.0
won't warn.
The return value is not used anywhere and the unref() functions typically
don't return any.
Fixes the following error when building with gcc 4.8.5 and address
sanitizer:
src/dns/nm-dns-dnsmasq.c: In function 'update':
src/dns/nm-dns-dnsmasq.c:506:44: error: 'first_prio' may be used uninitialized in this function [-Werror=maybe-uninitialized]
} else if (first_prio < 0 && first_prio != prio)
^
Print the "tentative" flags as last. Most other flags, have more the character of
a user configured attribute, while "tentative" reflects the current state of the address.
Previously, we would log
secondary,tentative
and
tentative,mngtmpaddr,noprefixroute
Print the "tenative" flag last. This way, the flag that commonly
will flip by kernel's decision, is consistently printed last.
Previously, we would directly log get_expiry(), which is the absolute timestamp
inn nm_utils_get_monotonic_timestamp_s() scale. This time scale starts counting
somewhere around the time when the NetworkManager process starts, hence it is not
very intuitive to look at.
Instead, print the remaining time that is left counting from now. Since
we anyway only track timeouts with a granularity of whole seconds,
printing up to 4 decimal places is sufficiently precise.
We commonly only allow tabs at the beginning of a line, not
afterwards. The reason for this style is so that the code
looks formated right with tabstop=4 and tabstop=8.
I think we should avoid non-trailing tabs in source code.
Allowing unescaped tab characters in string literals, adds
noise when searching the code for non-trailing tabs.
Also, depending on the editor configuration, it might be
non-obvious that tabs are used. And while I dislike tabs in general,
I think they are especially bad, when they have actual meaning
in code.
Setting the state of NMActiveConnection results in invoking callbacks
in NMManager. Hence, it might be far-reaching. Clear
priv->queued_act_request before invoking the callbacks.
There is a small change in behavior:
Previously, the DEACTIVATING/DEACTIVATED states were set if and only if
the previous state was less or equal then ACTIVATED. For example,
if the state was already DEACTIVATING, it would have done nothing.
Now, nm_active_connection_set_state_fail() transitions the states
depending on the previous state. E.g. it would only set DEACTIVATING
state, if the previous state was ACTIVATING/ACTIVATED. On the other hand,
it would always progress the state to DEACTIVATED.
The new behavior makes more sense to me, although I doubt that there is
a visible difference.
