Kernel does not allow to add an IPv4 route with rt_scope RT_SCOPE_NOWHERE
(255). It would fail with EINVAL.
While adding a route, we coerce/normalize the scope in
nm_platform_ip_route_normalize(). However, that should only be
done, if the scope is not explicitly set already. Otherwise,
leave it unchanged.
nm_platform_ip_route_normalize() is related to the compare functions.
Several compare modes do a fuzzy comparison, and they should compare
equal as if they would be normalized. Hence, we must do the same
normalization there.
One pecularity in NetworkManager is that we track scope as it's
inverse. The reason is to have a default value of zero meaning
RT_SCOPE_NOWHERE. Hence "scope_inv".
Rename to nm_platform_ip_address_flush(), it's more consistent with naming
for other platform functions.
Also, pass an address family argument. Sometimes I feel an option makes it clearer
what the function does. Otherwise, from the name it's not clear which address
families are affected. As an API, it feels more correct to me.
We soon also get a nm_platform_ip_route_flush() function, which will
look similar.
_nm_ip_config_add_obj() does some additional checking, like setting the ifindex.
We shall not bypass this also during bulk-update (replace).
Add options @merge and @append_force to make _nm_ip_config_add_obj() suitable
in those cases too, and use it.
In several cases, does the route compare function a fuzzy match, to get
the result as what would happen if you add that route to kernel.
The rt_source enum contains some NetworkManager specific values which
are mapped to a certain rtm_protocol value. Especially, when adding
a route to kernel, the resulting value will be coerced (and end up being
different).
We must take this coercion into account.
For completeness of the API. remove_obj() is basically a shortcut
of nm_dedup_multi_index_lookup_obj() combined with
nm_dedup_multi_index_remove_entry(). As such, it is useful to return
the actually deleted object. Note that the lookup needle @obj is not
necessarily the same instance as the one that will be removed, it's
only an instance that compares equal according to the index's equality
operator.
The return value shall indicate whether the add-call changed anything.
Reordering shall count as a change too.
On the other hand, clearing the dirty flag of the entry does not count
as a change.
No reason to, the other types are no less likely. Quite the contrary, if
the user specifies a GSM APN we're sure to use a DUN profile.
$ ./clients/cli/nmcli c add type bluetooth ifname '*' bluetooth.bdaddr 1C:E2:CC:56:6C:45 apn internet
$ nmcli c show bluetooth-1 |grep bluetooth.type
bluetooth.type: panu
^^^^ not cool
When the user sets a GSM or CDMA setting along with a Bluetooth setting
we know we're dealing with a DUN profile. No need to ask.
[thaller@redhat.com: verify() and normalize() must strongly agree whether a
connection is normalizable, and now to do it. That is, after verify()
determines the connection is normalizable, normalize() must fix it as
anticipated.
The reason is, we only want to modify the connection, if we are able
to create a valid result. Hence, after normalize() it *must* verify().
Try to simplify that by moving the logic of fixing the bt-type to a
common place _nm_connection_detect_bluetooth_type().]
Co-Authored-By: Thomas Haller <thaller@redhat.com>
Unbreaks Bluetooth DUN. Probably broken with the nm-meta-setting-desc
refactor, hence the Fixes tag. I didn't actually check.
$ nmcli c add type bluetooth ifname '*' bluetooth.bdaddr 1C:E2:CC:56:6C:45 connection.id bt bt-type dun-gsm
Error: 'apn' argument is required.
$ nmcli c add type bluetooth ifname '*' bluetooth.bdaddr 1C:E2:CC:56:6C:45 connection.id bt bt-type dun-gsm apn internet
Error: invalid <setting>.<property> 'apn'.
$
This is where it starts to get sad ^
$ nmcli c add type bluetooth ifname '*' bluetooth.bdaddr 1C:E2:CC:56:6C:45 connection.id bt bt-type dun-gsm gsm.apn internet
Error: invalid or not allowed setting 'gsm': 'gsm' not among [connection, bluetooth, bridge, ipv4, ipv6, proxy].
$
This is where it gets obvious what went wrong ^
Fixes: b5c8622ad3
do a check on parent ifindex before calling "nm_device_supports_vlans"
otherwise if the parent device is a software device and its ifindex
member has not been updated yet we will trigger the g_return_if_fail
statement in "nmp_cache_lookup_entry_link".
This has been osserved in NetworkManager CI test suite, on NetworkManager
boot, during the creation of a vlan on top of a bond interface.
CI test: vlan_update_mac_from_bond
[...]
<info> [1503323670.0229] manager: (bond0): new Bond device (/org/freedesktop/NetworkManager/Devices/23)
<debug> [1503323670.0231] device[0x555555c3e320] (vlan10): constructed (NMDeviceVlan)
<debug> [1503323670.0231] manager: (vlan-vlan10) create virtual device vlan10
<debug> [1503323670.0231] device[0x555555c3e320] (vlan10): unmanaged: flags set to [platform-init,!sleeping=0x10/0x11/unmanaged/unrealized], set-managed [sleeping=0x1])
<trace> [1503323670.0235] exported-object[0x555555c3e320]: export: "/org/freedesktop/NetworkManager/Devices/24"
<trace> [1503323670.0235] properties-changed[0x555555c3e320]: ignoring notification for prop g-object-path on type NMDeviceVlan
<trace> [1503323670.0236] properties-changed[0x555555c3e320]: ignoring notification for prop path on type NMDeviceVlan
<info> [1503323670.0237] manager: (vlan10): new VLAN device (/org/freedesktop/NetworkManager/Devices/24)
<debug> [1503323670.0239] device[0x555555c3e320] (vlan10): create (is nm-owned)
Program received signal SIGTRAP, Trace/breakpoint trap.
g_logv (log_domain=0x5555557c39a9 "NetworkManager", log_level=
G_LOG_LEVEL_CRITICAL, format=<optimized out>,
args=args@entry=0x7fffffffdef0) at gmessages.c:1086
1086 g_private_set (&g_log_depth, GUINT_TO_POINTER (depth));
(gdb) bt
#0 0x00007ffff5ce3643 in g_logv (log_domain=0x5555557c39a9 "NetworkManager", log_level=
G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffdef0) at gmessages.c:1086
#1 0x00007ffff5ce37bf in g_log (log_domain=log_domain@entry=0x5555557c39a9 "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7ffff5d51190 "%s: assertion '%s' failed") at gmessages.c:1119
#2 0x00007ffff5ce37f9 in g_return_if_fail_warning (log_domain=log_domain@entry=0x5555557c39a9 "NetworkManager", pretty_function=pretty_function@entry=0x5555557b2a20 <__func__.32407> "nmp_cache_lookup_entry_link", expression=expression@entry=0x5555557b1037 "ifindex > 0") at gmessages.c:1128
#3 0x000055555566688a in nmp_cache_lookup_entry_link (cache=0x555555a780f0, ifindex=<optimized out>) at src/platform/nmp-object.c:1449
#4 0x00005555556668f9 in nmp_cache_lookup_link (cache=<optimized out>, ifindex=ifindex@entry=0) at src/platform/nmp-object.c:1464
#5 0x00005555556515e9 in nm_platform_link_get_obj (self=self@entry=0x555555a88880 [NMLinuxPlatform], ifindex=ifindex@entry=0, visible_only=visible_only@entry=1) at src/platform/nm-platform.c:618
#6 0x0000555555633e91 in link_supports_vlans (platform=0x555555a88880 [NMLinuxPlatform], ifindex=0) at src/platform/nm-linux-platform.c:4482
#7 0x00005555556d6d41 in create_and_realize (device=0x555555c3e320 [NMDeviceVlan], connection=0x7fffdc007890, parent=0x555555c33560 [NMDeviceBond], out_plink=0x7fffffffe1f8, error=0x7fffffffe358) at src/devices/nm-device-vlan.c:239
#8 0x00005555556b934c in nm_device_create_and_realize (self=self@entry=0x555555c3e320 [NMDeviceVlan], connection=connection@entry=0x7fffdc007890, parent=0x555555c33560 [NMDeviceBond], error=error@entry=0x7fffffffe358)
at src/devices/nm-device.c:2946
#9 0x00005555555b84c7 in connection_changed (connection=0x7fffdc007890, self=0x555555ab1070 [NMManager]) at src/nm-manager.c:1381
#10 0x00005555555b84c7 in connection_changed (self=0x555555ab1070 [NMManager], connection=0x7fffdc007890) at src/nm-manager.c:1431
#11 0x00005555555b9130 in retry_connections_for_parent_device (self=self@entry=0x555555ab1070 [NMManager], device=device@entry=0x555555c33560 [NMDeviceBond])
at src/nm-manager.c:1416
#12 0x00005555555b95c7 in add_device (self=self@entry=0x555555ab1070 [NMManager], device=device@entry=0x555555c33560 [NMDeviceBond], error=error@entry=0x7fffffffe598) at src/nm-manager.c:2238
#13 0x00005555555b83e1 in connection_changed (connection=0x7fffdc007b30, self=0x555555ab1070 [NMManager]) at src/nm-manager.c:1352
#14 0x00005555555b83e1 in connection_changed (self=0x555555ab1070 [NMManager], connection=0x7fffdc007b30) at src/nm-manager.c:1431
#15 0x00005555555be25b in nm_manager_start (self=0x555555ab1070 [NMManager], error=error@entry=0x7fffffffe720) at src/nm-manager.c:5202
#16 0x0000555555586b13 in main (argc=1, argv=0x7fffffffe888) at src/main.c:413
The internal state file is supposed to overwrite the files from /etc.
Hence, we must also explicitly enable connectivity checking, when the
user wishes to do so. Otherwise, if /etc contains connectivity=false,
the setting cannot be overruled via D-Bus.
_idxcache_update() may remove @entry_old or it may update @entry_old
in-place.
We must assign @out_obj_old before and not touch @entry_old after
_idxcache_update().
Fixes: cdd8c65799
Fix the platform cache for routes, to be able to track routes
like they can be added to kernel. Basically, the previous notion
of the identifier for routes (ifindex,network/plen,metric) was
not what kernel does.
NMRouteManager and other code outside of platform is still oblivious
to this change. That has to be addressed in future commits.
https://github.com/NetworkManager/NetworkManager/pull/24
nmp_lookup_init_route_visible() was originally named this way, to only return routes
that are nmp_object_is_visible(). However, all routes are visible (as long as they are
nmp_object_is_alive()). Hence, this is a historic misnomer.
Also, passing @only_default FALSE is identical to the
nmp_lookup_init_addrroute() lookup.
So, rename the function to indicate it is a lookup for default routes
only. Also, get rid of the unsupported ifindex argument for which there
is no index.
Deleting an IPv4 route with metric zero will either delete the intended route,
or if no such route exists, it will delete another existing route with different
metric (but otherwise matching parameters).
I think this is a shortcoming of the kernel API. It allows omitting
the metric during delete. However, it gives not way to express to
explicitly delete an IPv4 route with metric zero, but no other.
Since we only delete routes that we obtain from the platform cache
in the first place, we don't need the workaround. Of course, there
is still a race that platform cache might be out of date at the
moment we attempt to delete the route. Or the cache might be
inconsistent, both cases leading to deletion of the wrong route.
But such cases should be very rare, and only present when the user
changes the routing table outside of NM.
Until now, NetworkManager's platform cache for routes used the quadruple
network/plen,metric,ifindex for equaliy. That is not kernel's
understanding of how routes behave. For example, with `ip route append`
you can add two IPv4 routes that only differ by their gateway. To
the previous form of platform cache, these two routes would wrongly
look identical, as the cache could not contain both routes. This also
easily leads to cache-inconsistencies.
Now that we have NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID, fix the route's
compare operator to match kernel's.
Well, not entirely. Kernel understands more properties for routes then
NetworkManager. Some of these properties may also be part of the ID according
to kernel. To NetworkManager such routes would still look identical as
they only differ in a property that is not understood. This can still
cause cache-inconsistencies. The only fix here is to add support for
all these properties in NetworkManager as well. However, it's less serious,
because with this commit we support several of the more important properties.
See also the related bug rh#1337855 for kernel.
Another difficulty is that `ip route replace` and `ip route change`
changes an existing route. The replaced route has the same
NM_PLATFORM_IP_ROUTE_CMP_TYPE_WEAK_ID, but differ in the actual
NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID:
# ip -d -4 route show dev v
# ip monitor route &
# ip route add 192.168.5.0/24 dev v
192.168.5.0/24 dev v scope link
# ip route change 192.168.5.0/24 dev v scope 10
192.168.5.0/24 dev v scope 10
# ip -d -4 route show dev v
unicast 192.168.5.0/24 proto boot scope 10
Note that we only got one RTM_NEWROUTE message, although from NMPCache's
point of view, a new route (with a particular ID) was added and another
route (with a different ID) was deleted. The cumbersome workaround is,
to keep an ordered list of the routes, and figure out which route was
replaced in response to an RTM_NEWROUTE. In absence of bugs, this should
work fine. However, as we only rely on events, we might wrongly
introduce a cache-inconsistancy as well. See the related bug rh#1337860.
Also drop nm_platform_ip4_route_get() and the like. The ID of routes
is complex, so it makes little sense to look up a route directly.
NMPCache can preserve the order of the objects. Until now, the order
was however arbitrary. Soon we will require to preserve the order of
routes.
During a dump, force appending new objects at the end. That ensures,
correct ordering during the dump.
Note that we track objects in several distrinct indexes. Those partition the
set of all objects. Outside a dump when receiving events about new objects (e.g.
RTM_NEWROUTE), it is very unclear at which place the new object should be sorted.
It is especially unclear, as an object might move from one partition (of
an index) to another.
In general, a deterministic order will only be useful in one particular
instance: the NMP_CACHE_ID_TYPE_ROUTES_BY_DESTINATION index for routes.
In this case, we will ensure a particular order of the routes.
Default g_log() logs to stdout for INFO level and higher, but logs to stderr
for DEBUG/TRACE. That is annoying, because especially when redirecting the streams,
the messages get mixed up. Install a log handler and just print to stdout for
the tests.
We already had a nmp_object_id_equal() function. Generally, an equal() function
is more useful then a cmp() function.
However, implementing a cmp() function is about the same effort then implementing
an equal() function. Also, an equal function can be trivially implemented based on
a compare function, but not the other way around.
That means, it is little extra effort to have both an equal() function
and a cmp() function. Add nmp_object_id_cmp(). If only to be
consistent with other code, which also provides both.
Only the D-Bus bits use it, and we wouldn't pass a GVariant array around
in internal code anyway. Also validate the scan options earlier rather
than waiting for the supplicant to tell us they are invalid.
Just because the user requested a scan doesn't mean the supplicant should
use the result of that scan to jump to an AP that's slightly better than
the current one. Let the supplicant handle when it's supposed to roam
based on it's own logic, not random scans from users or NM clients.
Enable background scanning for most WiFi connections except for
shared/AP and BSSID-locked ones. Make the non-WPA-Enterprise
interval very, very long to effectively disable periodic scanning
while connected.
Related: https://bugzilla.gnome.org/show_bug.cgi?id=766482
Change it to return TRUE when scanning is prohibited so that we
don't have to use use g_signal_emitv() and its special handling of
return values. Make the return value only change when we don't
want the default behavior (which would be to allow the scan).
Also add a parameter to the signal indicating whether the scan is
user/dbus-requested or whether it's an internal periodic scan.
While rebasing systemd from upstream the "sd-adapt/process-util.h" file
was renamed and few other header files were added in the sources.
Update Makefile.am.
Fixes: e0cdaf9880
