NMPolicy would at various time call nm_settings_connection_autoconnect_retries_reset()
followed by nm_settings_connection_autoconnect_retries_get().
This resulted in two logging messages, first to indicate that the value
was unset, and then reset it to the value from configuration. While that
is correct, it causes a lot of verbose logging. Especially for all connections
which autoconnect retry counter didn't actually change.
The advantage of that was, that we only loaded the actual value when we
need it the first time (during get()). That means, the user could reload
the configuration, and the value would be loaded and cached at a later
pointer.
However, the duplicate logging was annoying, but we still want to see
a message about the resetting.
So, now during reset load the value setting from NetworkManager.conf
and set it right away. Skip the intermediate UNSET value. In most
cases nothing changed now, and we don't log anything for most
connections.
Note that for the
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_NO_SECRETS)
case we no longer do the
if (nm_settings_connection_autoconnect_retries_get (connection) == 0)
check. But that is fine, because we only skip schedling a reset_connections_retries()
action. But note, that that previously we also would never actually
scheudle a new timeout, because
- either nm_settings_connection_autoconnect_retries_get (connection) != 0
- or the retries count was zero, in which case we already have a
reset_connections_retries action pending (from the time when we
set it to zero.
So, there is no change in behavior at all except dropping of a redundant
logging line.
NMSettingsConnection has 3 properties that are related to autoconnect:
- autoconnect_retries
- autoconnect_blocked_until
- autoconnect_blocked_reason
autoconnect_blocked_reason is entirely independent from the other two.
A connection have have autoconnect blocked via a blocked-reason, but the
retry count is not affected by that. The retry count is an independent
mechanism, that may additionally prevent autoconnect.
However autoconnect_retries and autoconnect_retries_blocked_until are
strongly related. The latter is set if and only if autoconnect_retries is
at zero.
Rename to reflect that better.
We cannot just call g_object_set() with an integer that is out of bound.
Otherwise, glib will warn. We can use nm_g_object_set_property*() to return
an error without asserting.
Currently both bridge.mac-address and ethernet.cloned-mac-address get
written to the same MACADDR ifcfg-rh variable; the ethernet property
wins if both are present.
When one property is set and the connection is saved (and thus reread)
both properties are populated with the same value. This is wrong
because, even if the properties have the same meaning, the setting
plugin should not read something different from what was written. Also
consider that after the following steps:
$ nmcli con mod c ethernet.cloned-mac-address 00:11:22:33:44:55
$ nmcli con mod c ethernet.cloned-mac-address ""
the connection will still have the new mac address set in the
bridge.mac-address property, which is certainly unexpected.
In general, mapping multiple properties to the same variable is
harmful and must be avoided. Therefore, let's use a different variable
for bridge.mac-address. This changes behavior, but not so much:
- connections that have MACADDR set will behave as before; the only
difference will be that the MAC will be present in the wired
setting instead of the bridge one;
- initscripts compatibility is not relevant because MACADDR for
bridges was a NM extension;
- if someone creates a new connection and sets bridge.mac-address NM
will set the BRIDGE_MACADDR property instead of MACADDR. But this
shouldn't be a big concern as bridge.mac-address is documented as
deprecated and should not be used for new connections.
https://bugzilla.redhat.com/show_bug.cgi?id=1516659
Settings plugins now return the connection that was reread from file
when adding a connection, which means that any agent-owned secret is
lost. Ensure that we don't forget agent-owned secrets by caching them
and readding them to the new connection returned by plugins.
Fixes: 8a1d483ca8
Fixes: b4594af55ehttps://bugzilla.gnome.org/show_bug.cgi?id=789383
Bond options are stored in a hash table and the order in which they
are returned by the API is not guaranteed. Sort them alphabetically so
that a connection will always be written in the same way, even if the
internal implementation of the hash table or the hashing function
changes, as it did in commit a6be2f4aa9 ("all: use nm_str_hash()
instead of g_str_hash()").
We also do this for libnm, where it causes visible changes
in behavior. But if somebody would rely on the hashing implementation
for hash tables, it would be seriously flawed.
GHashTable optimizes a NULL equality function to use direct pointer
comparison. That saves the overhead of calling g_direct_equal().
This is also documented behavior for g_hash_table_new().
While at it, also don't pass g_direct_hash() but use the default
of %NULL. The behavior is the same, but consistently don't use
g_direct_hash().
Next we will use siphash24() instead of the glib version g_direct_hash() or
g_str_hash(). Hence, the "nm-utils/nm-hash-utils.h" header becomes very
fundamental and will be needed basically everywhere.
Instead of requiring the users to include them, let it be included via
"nm-default.h" header.
Comparing @secrets_keys indicates to coverity that it might be NULL.
Below, we access @secrets_keys without check, and coverity doesn't realize
that this cannot crash, because secrets_keys_n would be zero too.
Anyway, this way we safe the sorting, in case we only have
one element.
Kernel doesn't support it for IPv6.
This is especially useful, if you combine static routes
with DHCP. In that case, you might want to get the device-route
to the gateway automatically, but add a static-route for it.
Currently the ifcfg-rh plugin doesn't explicitly store the connection
type for team slaves and is only able to read back ethernet and vlan
connections.
Leave this unchanged for ethernet and vlan slaves, but store the TYPE
variable for other connection types (Wi-Fi and Infiniband) so that we
can properly determine their type when the connection is read.
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.
It has almost no callers, and it is a bit of a strange API. Let's
not cache the last accessed value inside NMConfigData. Instead, free
it right after use. It was not reused anyway, it only hangs around
as convenience for the caller.
Names like
- nm_settings_connection_get_autoconnect_retries
- nm_settings_connection_set_autoconnect_retries
- nm_settings_connection_reset_autoconnect_retries
are about the same thing, but they are cumbersome to grep
because they share not a common prefix.
Rename them from SUBJECT_VERB_OBJECT to SUBJECT_OBJECT_VERB,
which sounds odd in English, but seems preferred to me.
Now you can grep for "nm_settings_connection_autoconnect_retries_" to
get all accessors of the retry count, or "nm_settings_connection_autoconnect_"
to get all accessors related to autoconnect in general.
When the ifcfg-rh plugin writes a 802-1x setting it currently ignores
the password-raw property and so the password disappears when the
connection is saved. Add support for the property.
31. NetworkManager-1.9.2/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c:974:
uninit_use_in_call: Using uninitialized value "contents_rest" when
calling "__strtok_r_1c".
33. NetworkManager-1.9.2/src/nm-core-utils.c:1957:
uninit_use: Using uninitialized value "s".
148. NetworkManager-1.9.2/src/nm-core-utils.c:1924:
uninit_use_in_call: Using uninitialized value "s" when calling
"nm_strstrip_avoid_copy".
We want to support large number of routes. Reduce the number
of copies, by adding internal accessor functions.
Also, work around a complaint from coverity:
46. NetworkManager-1.9.2/libnm-core/nm-utils.c:1987:
dereference: Dereferencing a null pointer "names".
