- split NM_DEVICE_AUTOCONNECT_BLOCKED_INTERN in two parts:
"wrong-pin" and "manual-disconnect". Setting/unsetting them
should be tracked differently, as their reason differs.
- no longer initialize/clear the autoconnect-blocked reasons
during realize/unrealize of the device. Instead, initialize
it once when the object gets created (nm_device_init()), and
keep the settings beyond unrealize/realize cycles. This only
matters for software devices, as regular devices get deleted
after unrealizing once. But for software devices it is essential,
because we don't want to forget the autoconnect settings of
the device instance.
- drop verbose logging about blocking autoconnect due to failed
pin. We already log changes to autoconnect-blocked flags with
TRACE level. An additional message about this particular issue
seems not necessary at INFO level.
- in NMManager's do_sleep_wake(), no longer block autoconnect
for devices during sleep. We already unmanage the device, which
is a far more effective measure to prevent activation. We should
not also block autoconnect.
The flags allow for more then two reasons. Currently the only reasons
for allowing or disallowing autoconnect are "user" and "intern".
It's a bit odd, that NMDeviceAutoconnectBlockedFlags has a negative
meaning. So
nm_device_set_autoconnect_intern (device, FALSE);
gets replaced by
nm_device_set_autoconnect_blocked_set (device, NM_DEVICE_AUTOCONNECT_BLOCKED_INTERN);
and so on.
However, it's chosen this way, because autoconnect shall be allowed,
unless any blocked-reason is set. That is, to check whether autoconnect
is allowed, we do
if (!nm_device_get_autoconnect_blocked (device, NM_DEVICE_AUTOCONNECT_BLOCKED_ALL))
The alternative check would be
if (nm_device_get_autoconnect_allowed (device, NM_DEVICE_AUTOCONNECT_ALLOWED_ALL) == NM_DEVICE_AUTOCONNECT_ALLOWED_ALL)
which seems odd too.
So, add the inverse flags to block autoconnect.
Beside refactoring and inverting the meaning of the autoconnect
settings, there is no change in behavior.
nm_device_can_auto_connect() only has one caller, auto_activate_device()
in NMPolicy.
That caller already checks whether the connection has autoconnect
enabled, so drop the duplicate check.
This saves some duplication, but it also makes some sense:
NMSettingsConnection has a complex blocking of autoconnect,
so just looking at connection.autoconnect is not enough in
any case to determine whether the connection should autoconnect.
We move thus more handling of autoconnect to NMPolicy, where
it belongs.
The number of authentication retires is useful also for passwords aside
802-1x settings. For example, src/devices/wifi/nm-device-wifi.c also has
a retry counter and uses a hard-coded value of 3.
Move the setting, so that it can be used in general. Although it is still
not implemented for other settings.
This is an API and ABI break.
Since commit 4a6fd0e83e (device: honor the
connection.autoconnect-retries for 802.1X) and the related bug bgo#723084,
we reuse the autoconnect-retries setting to control the retry count
for requesting passwords.
I think that is wrong. These are two different settings, we should not
reuse the autoconnect retry counter while the device is still active.
For example, the user might wish to set autoconnect-retries to infinity
(zero). In that case, we would retry indefinitly to request a password.
That could be problematic, if there is a different issue with the
connection, that makes it appear tha the password is wrong.
A full re-activation might succeed, but we would never stop retrying
to authenticate. Instead, we should have two different settings for
retrying to authenticate and to autoconnect.
This is a change in behavior compared to 1.8.
Since 32b3eb1181 [core: merge IPv4 and IPv6 implementation of
nm_utils_ip4_property_path()], nm_utils_sysctl_ip_conf_path() introduced
in cd271d5cb1 [core: add nm_utils_sysctl_ip_conf_is_path() util] is used to
cunstruct sysctl paths and it is way less tolerant towards using something
that is not an interface name in the path.
It's always been incorrect to assume the ifname is a linux link name and
it resulted it ugly, if benign, sysctl access attempts such as
"/sys/class/net/28:B2:BD:5D:23:AB/phys_port_id" etc.
Now it triggers an assertion failure. Let's guard all such accesses.
Fixes: 32b3eb1181
Fixes: cd271d5cb1
For a while now, all NMPObject instances are not modified after
being cached. They are immutable, and can be passed around by keeping
a reference to them.
No longer copy the NMPlatformLink data to a @info variable. Instead,
take a reference (which ensures that the instance stays alive). It
won't change, as it's immutable.
The advantage is, that whenever you see a NMPlatformLink pointer,
for exmple in device_recheck_slave_status(), you can be sure that
it's actually a NMPObect, and NMP_OBJECT_UP_CAST() will work.
We now can be enslaved and have L3 configuration at the same time.
This also reduces some unnecessary complexity, because the decision to
progress to IP_CHECK or SECONDARIES now happens in a single place, in
the check_ip_state() routine.
The OpenVSwitch interfaces come into existence by their enslavement to a port.
They can also bear an IP4 or IP6 configuration -- waiting on a carrier would
deadlock the acitvation.
That one is special. All interfaces that are attached to OpenVSwitch
ports appear as slaves to that one even for our purposes we like to
pretend they're slaves to the actual OpenVSwitch bridges.
For some software devices, the platform link appears only after they've been
realized. Update their properties and let them know that the link has changed
so they can eventually proceed with activation.
Also, reset the properties (udi, iface, driver) that are set from the platform
link when the link goes away. At that point they don't reflect reality anymore.
Removes some code duplication too.
Coverity doesn't like this. Refactor a bit, hoping that it fares better.
1. Defect type: ASSERT_SIDE_EFFECT
1. NetworkManager-1.9.2/src/devices/nm-device.c:10226: assignment_where_comparison_intended: Assignment "ip_ifindex = nm_device_get_ip_ifindex(self)" has a side effect. This code will work differently in a non-debug build.
2. NetworkManager-1.9.2/src/devices/nm-device.c:10226: remediation: Did you intend to use a comparison ("==") instead?
Add a proxy setting to generated connections because after commit
6f94b16507 ("libnm: fix nm_connection_diff() for settings without
properties") the matching between a connection having the setting and
another connection without it fails. Before the commit, since no proxy
property is marked as inferrable, such comparison succeeded.
Since commit 5c299454b4 we can configure
multiple default-routes.
That is especially useful with IPv6 to configure multiple routers.
It will also be useful, once we allow configuring manual default-routes,
like regular static routes.
However the problem is, that the default-route for the manual gateway
and the gateway from DHCP both get the same metric. So it's undefined
which route is used. To avoid that problem, and to restore previous
behavior, don't accept any default-routes if a gateway is set.
Fixes: 5c299454b4
Setting the MTU might fail when the underlying device's MTU
is not set.
Detect that case, and log a better warning message.
Unfortunately, it's tricky to detect whether this is a complete
failure, or whether we will later try again to change the MTU.
So, we log a failure, altough later we might fix it. It would
be better not to warn about non-errors.
and nm_utils_ip6_property_path(). The API with static buffers
looks a bit nicer. But I think they are dangerous, because
we tend to pass the buffer down several layers of the stack, and
it's not immediately clear, that we don't overwrite the static
buffer again (which we probably did not, but it's hard to verify
that there is no bug there).
Kernel does not allow setting the MTU of a VLAN larger
then the MTU of the underlying device. Hence, we might
initially fail to set a large MTU of the VLAN, but we
have to retry when the MTU of the parent changes.
https://bugzilla.redhat.com/show_bug.cgi?id=1414901
By using a macro, we don't cast all the types to guint. Instead,
we use their native types directly. Hence, we don't need
nm_hash_update_uint64() nor nm_hash_update_ptr().
Also, for types smaller then guint like char, we save hashing
the all zero bytes.
Replace the usage of g_str_hash() with our own nm_str_hash().
GLib's g_str_hash() uses djb2 hashing function, just like we
do at the moment. The only difference is, that we use a diffrent
seed value.
Note, that we initialize the hash seed with random data (by calling
getrandom() or reading /dev/urandom). That is a change compared to
before.
This change of the hashing function and accessing the random pool
might be undesired for libnm/libnm-core. Hence, the change is not
done there as it possibly changes behavior for public API. Maybe
we should do that later though.
At this point, there isn't much of a change. This patch becomes
interesting, if we decide to use a different hashing algorithm.
The privious NM_HASH_* macros directly operated on a guint value
and were thus close to the actual implementation.
Replace them by adding a NMHashState struct and accessors to
update the hash state. This hides the implementation better
and would allow us to carry more state. For example, we could
switch to siphash24() transparently.
For now, we still do a form basically djb2 hashing, albeit with
differing start seed.
Also add nm_hash_str() and nm_str_hash():
- nm_hash_str() is our own string hashing implementation
- nm_str_hash() is our own string implementation, but with a
GHashFunc signature, suitable to pass it to g_hash_table_new().
Also, it has this name in order to remind you of g_str_hash(),
which it is replacing.
Introduce a NM_HASH_INIT() function. It makes the places
where we initialize a hash with a certain seed visually clear.
Also, move them from "shared/nm-utils/nm-shared-utils.h" to
"shared/nm-utils/nm-macros-internal.h". We might want to
have NM_HASH_INIT() non-inline (hence, define it in the
source file).
For routes and the default-route from NDisc, set the router preference
RTA_PREF.
Also, previously, we would only configure one IPv6 default-route. That by itself
was not really a problem, as long as NetworkManager would always make sure that
it configured the route to the ~best~ router.
Actually, NM should have done that already. It keeps the list of gateways
sorted, and prefers them according to their preference. But maybe
it didn't, so we have bug rh#1445417 (??).
Change that by configuring a default-route for all gateways, with
appropriate router prefrence. In case, kernel doesn't support RTA_PREF
yet, only configure all routes that share the same maxiumum preference.
https://bugzilla.redhat.com/show_bug.cgi?id=1445417
Instead of having 3 properties @gateway, @never_default and @has_gateway
on NMIP4Config/NMIP6Config that determine the default-route, track the
default-route as a regular route.
The gateway setting is the configuration knob for the default-route.
Since an NMIP4Config/NMIP6Config instance only has one gateway property,
it cannot track more then one default-routes (see related bug rh#1445417).
Especially with policy routing, it might be interesting to configure a
default-route in multiple tables.
Also, later it might be interesting to allow adding default-routes as
regular static routes in a connection, so that the user can configure additional
route parameters for the default-route or add default-routes in multiple tables.
With this patch, default-routes now have a rt_source property according to their
origin.
Also, the previous commits of this branch broke handling of the
default-route :) . That should be working now again.
The MSS is only set for VPN connections (by merging it in the respective
NMIP4Config/NMIP6Config).
It is also only used when setting the MSS of the default route.
Don't track that property in NMIP4Config/NMIP6Config, instead, set the
mss of the route directly in nm_vpn_connection_ip4_config_get() and
nm_vpn_connection_ip6_config_get().
There is a potential change in behavior here: NMDevice also consisdered
the MSS for the default route, but that would only be set if the MSS
gets merged from an vpn-ip-config. Which at most is the case for
iterface-less VPN types (libreswan). But even in that case, it doesn't
seem right to me to use the VPN's MSS for the device's default-route.
We added "ipv4.route-table-sync" and "ipv6.route-table-sync" to not change
behavior for users that configured policy routing outside of NetworkManager,
for example, via a dispatcher script. Users had to explicitly opt-in
for NetworkManager to fully manage all routing tables.
These settings were awkward. Replace them with new settings "ipv4.route-table"
and "ipv6.route-table". Note that this commit breaks API/ABI on the unstable
development branch by removing recently added API.
As before, a connection will have no route-table set by default. This
has the meaning that policy-routing is not enabled and only the main table
will be fully synced. Once the user sets a table, we recognize that and
NetworkManager manages all routing tables.
The new route-table setting has other important uses: analog to
"ipv4.route-metric", it is the default that applies to all routes.
Currently it only works for static routes, not DHCP, SLAAC,
default-route, etc. That will be implemented later.
For static routes, each route still can explicitly set a table, and
overwrite the per-connection setting in "ipv4.route-table" and
"ipv6.route-table".
The line
device (wlan0): state change: ip-config -> ip-check (reason none, internal state managed)
prints the sys-iface-state, which is at other places logged with
device[0x55914506ed70] (wlan0): sys-iface-state: external -> managed
For consistency, name the same parameter the same.
The name nm_device_get_priority() is misleading. Nowadays it's only used
for the default route metric, and nothing else.
Rename it, and make it static.
