For most strv or string properties, we cannot distinguish between
NULL/unset/default and empty.
It would be difficult to enter in nmcli or grasp how it differs. There
are probably many bugs, where we accept empty strings, and fail to
handle them correctly.
Anyway. For most strv arrays, and empty array and NULL/unset/default are
treated the same. That means, g_object_get() tends to always return NULL
(never an empty strv array) and g_object_set() of an empty strv array
will internally leave the GArray at NULL.
For a few properties, there is a difference. See "ipv[46].dns-options".
See also "clear_emptyunset_fcn" hook in libnm-setting.
Add a way to handle such strv properties with the "direct" mechanism.
Unfortunately, there are several possibilities how to handle NULL and
empty arrays. Therefore we have different variants.
Clean this up, and add a way to preserve whether the array is empty
(previous variants could not distinguish that).
Functions are also renamed, so that if you backport a user of the new
API, you'll get a compiler error if this patch is missing.
Also, nm_strvarray_get_strv_notnull() no longer takes a pointer to a
"GArray*". Previously, it used that to fake an empty strv array. Now
this returns NM_STRV_EMPTY_CC().
_nm_utils_dns_option_validate() allows specifying the address family,
and filters based on that. Note that all options are valid for IPv6,
but some are not valid for IPv4.
It's not obvious, that such filtering is only performed if
"option_descs" argument is provied. Otherwise, the "ipv6" argument is
ignored.
Regardless, it's also confusing to have a boolean "ipv6". When most
callers don't want a filtering based on the address family. They
actually don't want any filtering at all, as they don't pass an
"option_descs". At the same time passing a TRUE/FALSE "ipv6" is
redundant and ignored. It should be possible, to explicitly not select
an address family (as it's ignored anyway).
Instead, make the "gboolean ipv6" argument an "int addr_family".
Selecting AF_UNSPEC means clearly to accept any address family.
When connection down is explicitly called on the controller, the port
connection should also be deactivated with the reason user-requested,
otherwise any following connection update on the controller profile
will unblock the port connection and unnessarily make the port to
autoconnet again.
Fixes: 645a1bb0ef ('core: unblock autoconnect when master profile changes')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager-ci/-/merge_requests/1568
Add a new "stable-ssid" mode that generates the MAC address based on the
Wi-Fi's SSID.
Note that this gives the same MAC address as setting
connection.stable-id="${NETWORK_SSID}"
wifi.cloned-mac-address="stable"
The difference is that changing the stable ID of a profile also affects
"ipv6.addr-gen-mode=stable-privacy" and other settings.
For Wi-Fi profiles, this will encode the SSID in the stable-id.
For other profiles, this encodes the connection UUID (but the SSID and
the UUID will always result in distinct stable IDs).
Also escape the SSID, so that the generated stable-id is always valid
UTF-8.
We have many properties, and we aim that they have a small set of
"types". The purpose is that we can treat similar properties (with the
same type) alike.
One type are "direct" strv properties. Those still require some
C functions, like get-length(), clear(), add(), get-at-index().
The implementation of those functions should also be similar, so that
strv properties behave similar.
For that, make use of helper functions, so that little duplicate logic
is there.
Use some new nm_strvarray_*() functions, and unify/cleanup some code.
All related to strv properties in NMSetting classes.
The check "sizeof(const char *const *) ==
g_array_get_element_size((GArray *) strv)" is wrong, but probably
harmless, because most likely on our supported architectures all pointer
sizes are the same size.
Also, just use `sizeof(char *)` instead of `sizeof(const char *)`. Not
that it matters, but the GArray holds pointers of `char *`.
Also, consistently place the "sizeof()" on the left side of the
comparison.
There are various properties related to EEE, that we might want to add
support for in the future (for example, "ethtool.eee-advertise").
Don't use up the base name "eee", instead make it "eee-enabled". All
properties should have different prefixes, and "ethtool.eee" would be a
prefix of "ethtool.eee-advertise".
Also, the #define is already called NM_ETHTOOL_OPTNAME_EEE_ENABLED. This
also should be consistent.
Rename.
Fixes: 3165d9a2de ('ethtool: introduce EEE support')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1792
Previously, we first sort by the device's state, then by the active
connection's state. Contrast to `nmcli connection`, which first sorts
by the active connection's state.
It means, the sort order is somewhat different. Fix that.
In most cases, that shouldn't make a difference, because the
device's state and the active-connection's state should
correspond. However, it matters as we now treat external activations
different, and that is tied to the active connection.
CMP() is a confusing pattern. Sure enough, the sort order was wrong, for
example, `nmcli connection` would show
$ nmcli -f STATE,UUID,DEVICE c
STATE UUID DEVICE
activating 3098c902-c59c-45f4-9e5a-e4cdb79cfe1b nm-bond
activated e4fc23ac-54ab-4b1a-932a-ebed12c96d9b eth1
("activating" shown before "activated").
With `nmcli device`, we sort with compare_devices(). This first sorts by
device state (with "connected" being sorted first). Only when the device
state is equal, we fallback to nmc_active_connection_cmp(). So with
`nmcli device` we usually get "connected" devices first, and we don't
really notice that there is a problem with nmc_active_connection_cmp().
On the other hand, `nmcli connection` likes to sort first via
nmc_active_connection_cmp(), which gets it wrong. Profiles in
"activating" state are sorted first. That's inconsistent with `nmcli
device`, but it's also not what is intended.
Fix that.
Note the change in the test output. Both eth1 and eth0 are connected to
to the same profile, but one "eth0" the active-connection's state is
DEACTIVATING, while on "eth1" it's ACTIVATED (but both device's states
are "CONNECTED"). That's why "eth1" is now sorted first (as desired).
Fixes: a1b25a47b0 ('cli: rework printing of `nmcli connection` for multiple active connections')
- test for "-order" option with `nmcli connection show`.
- test for order of activated devices. Optimally, the devices
should be in activating vs. activated state. I fail to do that,
the mock implementation is cumbersome to use. It still seems useful
to have this (maybe it could be improved).
NM variants:
- evaluate arguments only once
- have a static assertion that the signedness of the argument agrees.
Like MIN()/MAX(), NM_MIN()/NM_MAX() now also evaluate to a constant
expression, if the arguments are already constant. That means, the only
reason why MIN()/MAX() was preferable over NM_MIN()/NM_MAX() is no
longer relevant. Except there are a few places where NM_MIN()/NM_MAX()
cannot be used. In those places use NM_MIN_CONST()/NM_MAX_CONST().
glib's MIN()/MAX() will be replaced by NM_MIN()/NM_MAX().
There are however a few places where NM_MIN()/NM_MAX() cannot
be used.
Adjust those places to use NM_MIN_CONST()/NM_MAX_CONST() instead.
Glib's MIN()/MAX() should not be used, in favor of NM_MIN()/NM_MAX().
That's because the NM variants
- evaluate arguments only once
- have a static assertion that the signedness of the arguments matches
However, previously those macros never evaluated to a compile time
constant. Unlike the glib variants, which do so when the arguments are
compile time constants. That is sometimes important when using the
macros in a context that requires a constant.
Extend NM_MIN()/NM_MAX() to be a compile time constant, when possible.
Note that there are still a few places where NM_MIN()/NM_MAX() cannot be
used due to the expression statement. For those cases, there is
NM_MIN_CONST()/NM_MAX_CONST().
Comparing integers of different signedness gives often unexpected
results. Adjust usages of MIN()/MAX() to ensure that the arguments agree
in signedness.
nm_hash_siphash42() uses a randomized seed like nm_hash*(). In this case,
we want to always generate the same fake timestamp, based on the host-id.
In practice, it doesn't really matter, because this is only the fallback
path for something gone horribly wrong already.
c_siphash_init() requires a 16 bytes array. That is cumbersome to use.
We have NM_HASH_SEED_16() macro for helping with that. It's still
cumbersome.
Most of the time, the caller just wants to pick an arbitrarily chosen,
fixed number. Add NM_HASH_SEED_16_U64() which takes a number and gives
a 16 seed array. The argument is in host endianness, but the resulting
seed array has it encoded in big endianness, to be architecture
independent.
Calling getpwuid_r() is cumbersome, because it has a separate passwd and
string buffer, and you shall retry, when the buffer is too small.
Extract nm_getpwuid() for that. This one always allocates a suitable
buffer, that the caller can free.
This will allow callers to get the full passwd struct. It will also
allow callers to avoid the additional strdup() of nm_utils_uid_to_name(),
when we don't need a clone of the string.
Most profiles don't have "connection.permissions" set. Avoid resolving the
UID to the name via getpwuid() (in nm_auth_is_subject_in_acl()), until we
know that we require it.
I think GSource* is preferable, because it's more type-safe than the
guint numbers. Also, g_source_remove() only works with
g_main_context_default(), while g_source_detach() of a GSource pointer
works with any GMainContext (so it's more general, even if we in this
case only have sources attached to g_main_context_default()).
Handling GSource* pointers is possibly also faster, since it saves one
extra g_main_context_find_source_by_id() -- on the other hand, tracking
the pointer costs 8 bytes while tracking the guint only costs 4 bytes.
Whether it's faster is unproven, but possibly it is. In any case it's
not an argument against using pointers.
Anyway. Update another usage of source-ids to use GSource pointers.
This is also the pattern that "checkpatch.pl" suggests.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1774
