The writer should write all properties of the sriov setting when the
setting exists without additional logic. Likewise, the reader should
instantiate a sriov setting when any sriov key is present and blindly
set properties from keys.
The old code did not always preserve the presence of a sriov setting
after a write/read cycle.
Fixes: c02d1c488f
Don't configure the static number of VFs when the device is realized
because the device could still be unmanaged. Instead, do it when the
device becomes managed.
Describe how to specify multiple VFs and which attributes are
supported, so that this information is available in the nm-settings
manual page.
Also, clarify that SR-IOV parameters are managed only when the setting
is present.
https://bugzilla.redhat.com/show_bug.cgi?id=1651979
Report an error when the user tries to add an unknown attribute
instead of silently accepting (and ignoring) it.
Note that this commit also changes the behavior of public API
nm_utils_sriov_vf_from_str() to return an error when an unknown
attribute is found. I think the previous behavior was buggy as wrong
attributes were simply ignored without any way for the user to know.
Fixes: a9b4532fa7
With the default 128KiB buffer size it is easy to lose events. For
example when 64 interfaces appear at the same time, we lose events for
the last 16. Increase the buffer size to 4MiB.
https://bugzilla.redhat.com/show_bug.cgi?id=1651578
nm_team_link_watcher_new_arp_ping2() is the more powerful variant of
nm_team_link_watcher_new_arp_ping(). It can do everything the new
variant can, and more.
Hence, v1 should be implemented based on v2. This way, validating and
constructing the watcher is all done in one place, not split in two places.
The timestamp of the host-id is the timestamp of the secret_key file.
Under normal circumstances, reading the timestamp should never fail,
and reading it multiple times should always yield the same result.
If we unexpectedly fail to read the timestamp from the file we want:
- log a warning, so that the user can find out what's wrong. But
do so only once.
- we don't want to handle errors or fail operation due to a missing
timestamp. Remember, it's not supposed to ever fail, and if it does,
just log a warning and proceed with a fake timestamp instead. In
that case something is wrong, but using a non-stable, fake timestamp
is the least of the problems here.
We already have a stable identifier (the host-id) which we can use to
generate a fake timestamp. Use it.
In case the user would replace the secret_key file, we also don't want
that accessing nm_utils_host_id_get_timestamp*() yields different
results. It's not implemented (nor necessary) to support reloading a
different timestamp. Hence, nm_utils_host_id_get_timestamp() should
memoize the value and ensure that it never changes.
Now that the secret-key is hashed with the machine-id, the name is
no longer best.
Sure, part of the key are persisted in /var/lib/NetworkManager/secret_key
file, which the user is well advised to keep secret.
But what nm_utils_secret_key_get() returns is first and foremost a binary
key that is per-host and used for hashing a per-host component. It's
really the "host-id". Compare that to what we also have, the
"machine-id" and the "boot-id".
Rename.
CSiphash is a first class citizen, it's fine to use everwhere where we
need it.
NMHash wraps CSiphash and provides three things:
1) Convenience macros that make hashing nicer to use.
2) it uses a randomly generated, per-run hash seed, that can be combined
with a guint static seed.
3) it's a general API for hashing data. It nowhere promises that it
actually uses siphash24, although currently it does everywhere.
NMHash is not (officially) siphash24.
Add API nm_hash_siphash42_init() and nm_hash_siphash42() to "nm-hash-utils.h",
that exposes (2) for use with regular CSiphash. You of course no longer
get the convenice macros (1) but you get plain siphash24 (which
NMHash does not give (3)).
While at it, also add a nm_hash_complete_u64(). Usually, for hasing we
want guint types. But we don't need to hide the fact, that the
underlying value is first uint64. Expose it.
NetworkManager loads (and generates) a secret key as
"/var/lib/NetworkManager/secret_key".
The secret key is used for seeding a per-host component when generating
hashed, stable data. For example, it contributes to "ipv4.dhcp-client-id=duid"
"ipv6.addr-gen-mode=stable-privacy", "ethernet.cloned-mac-address=stable", etc.
As such, it corresponds to the identity of the host.
Also "/etc/machine-id" is the host's identity. When cloning a virtual machine,
it may be a good idea to generate a new "/etc/machine-id", at least in those
cases where the VM's identity shall be different. Systemd provides various
mechanisms for doing that, like accepting a new machine-id via kernel command line.
For the same reason, the user should also regenerate a new NetworkManager's
secrey key when the host's identity shall change. However, that is less obvious,
less understood and less documented.
Support and use a new variant of secret key. This secret key is combined
with "/etc/machine-id" by sha256 hashing it together. That means, when the
user generates a new machine-id, NetworkManager's per-host key also changes.
Since we don't want to change behavior for existing installations, we
only do this when generating a new secret key file. For that, we encode
a version tag inside the "/var/lib/NetworkManager/secret_key" file.
Note that this is all abstracted by nm_utils_secret_key_get(). For
version 2 secret-keys, it internally combines the secret_key file with
machine-id (via sha256). The advantage is that callers don't care that
the secret-key now also contains the machine-id. Also, since we want to
stick to the previous behavior if we have an old secret-key, this is
nicely abstracted. Otherwise, the caller would not only need to handle
two per-host parts, but it would also need to check the version to
determine whether the machine-id should be explicitly included.
At this point, nm_utils_secret_key_get() should be renamed to
nm_utils_host_key_get().
g_file_get_contents() fails with G_FILE_ERROR, G_FILE_ERROR_NOENT when the
file does not exist.
That wasn't obvious to me, nm_utils_error_is_notfound() to the rescue.
Fixes: dbcb1d6d97
For older NetworkManager versions, a match spec that only contained except:
specifiers could never yield a positive match. That is not very useful and
got fixed by commit 242de347adbf653a709607979d36a0da1ca3ff0b (core: fix
device spec matching for a list of "except:").
Still, adjust the configuration snippet so that it also works with
configurations that predate the fix.
If the spec specifies only negative matches (and none of them matches),
then the result shall be positive.
Meaning:
[connection*] match-device=except:dhcp-plugin:dhclient
[connection*] match-device=except:interface-name:eth0
[.config] enabled=except:nm-version:1.14
should be the same as:
[connection*] match-device=*,except:dhcp-plugin:dhclient
[connection*] match-device=*,except:interface-name:eth0
[.config] enabled=*,except:nm-version:1.14
and match by default. Previously, such specs would never yield a
positive match, which seems wrong.
Note that "except:" already has a special meaning. It is not merely
"not:". That is because we don't support "and:" nor grouping, but all
matches are combined by an implicit "or:". With such a meaning, having
a "not:" would be unclear to define. Instead it is defined that any
"except:" match always wins and makes the entire condition to explicitly
not match. As such, it makes sense to treat a match that only consists
of "except:" matches special.
This is a change in behavior, but the alternative meaning makes
little sense.
When we agregate the connectivity state, only devices that
have the best default route should be considered.
Since we do connectivity checking per-device, the per-device check
does not care whether traffic to the internet is really routed via this
device.
But when talking about the global connectivity state, we care mostly
about the (best) default route. So, we should not allow a device with
worse or now default route, to contribute its connectivity state.
Fixes: 6b7e9f9b22
If the URI does not specify a port, we always assumed "80". That is
wrong for https. Arguably, https is discouraged for connectivity checking,
but we still shouldn't break it.
Fixes: 9664f284a1
The settings of NMConnectivity can change any time, by reloading the
configuration.
When reloading the configration, we don't want to interrupt or cancel
the pending reuqests, they should just complete with the old settings with
which they started. Note, that NMDevice is smart enough, that when a
newer request completes earlier, it invalidates all older, still pending
requests.
Anyway, that means, we cannot rely on the value to stay alive. Fix that,
by adding adding a new ref-counted struct for these parameters.
Fixes: 2cec94bacc
During a config change notification, we determine a "changed" value,
to know whether things significantly changed.
Also, we want to log a warning about invalid configuration,
only when the config actually changed. Previously, when the URI was
invalid, on every reload (SIGHUP) we would log an error message,
even if the configuration did not change. There is no need to
warn multiple times about the same thing.
Keep track of the original URI in priv->uri. Whenever that changed,
we know the user reconfigured something. But also, now the URI might
be set to an invalid value. That means, we need to remember whether
the URI is valid.
Also, log a warning if we fail to parse the host and port. Already
before, such an URI was considered invalid and we would effectively
not to connectivity checking.
- use cleanup attribute except explicit free/unref.
- check that the result has the expected address family.
Arguably, it should always, unless there is a bug in systemd-resolved.
By that reasoning, we also wouldn't have to check the address length
either.
- don't use strndup() for values that are later freed by g_free().
We should always agree whether to malloc/free or g_malloc/g_free.
- don't use strcasecmp(). Always use the locale independent g_ascii_strcasecmp()
instead.
- use nm_utils_inet_ntop() instead of inet_ntop(). It's our preferred
wrapper, which as a stricter semantic (for example, it cannot fail
and it's input arguments are stricter defined).
- use nm_clear_g_free() instead of g_clear_pointer().
If the user disabled systemd-resolved, two things seem apparent:
- the user does not want us to use systemd-resolved
- NetworkManager is not pushing the DNS configuration to
systemd-resoved.
It seems to me, we should not consult systemd-resolved in that case.
Each time when enabling/disabling "systemd-resolved" in combination with another
plugin (which is unchanged), another pair of signal handlers was
connected. That's wrong.
Fixes: d4eb4cb45f
In most cases, we don't want the translated string (only marked
for translation, and then programatically the caller deciedes
whether to translate or not).
The few places that always call gettext() can do it explicitly.
Now, that our functions are all "no_l10n" by default, rename them.
While at it, replace "AF_INET" with "IPv". The connectivity check
logging line is already much too long. Save a few characters. Also,
I think the meaning of "AF_INET" is less clear (to a novice user) than
"IPv4".
The NMDBusManager owns a reference to the system bus. Expose it, so we
can use it. Note that g_bus_get() -- as alternative to get the systembus
singleton -- is asynchronous, and g_bus_get_sync() has an API that makes
one wonder what it does. Since we already have a reference to the connection
object we want to use, expose it.
Since we determine the connectivity state of each device individually,
the global connectivity state is an aggregate of all these states.
I am not sure about considering here devices that don't have the (best)
default route for their respective address family. But anyway.
When we aggregate the best connectivity, we chose the numerical largest
value. That is wrong, because PORTAL is numerically smaller than
LIMITED.
That means, if you have two devices, one with connectivity LIMITED and
one with connectivity PORTAL, then LIMITED wrongly wins.
Fixes: 6b7e9f9b22https://bugzilla.redhat.com/show_bug.cgi?id=1619873
