Dynamic added routes i.e ECMP single-hop routes, are not managed by l3cd
as the other ones. Therefore, they need to be tracked properly and
marked as dirty when they are.
This way, NetworkManager makes sure there are no leftovers from merging
ECMP multi-hop routes.
Usually, routes from the combined NML3ConfigData and are resolved early
during a commit. For example, _obj_states_update_all() creates for each
such route an obj_state_hash entry. Let's call those static, or
"non-dynamic".
Later, we can receive additional routes. We get them back from NMNetns
during nm_netns_ip_route_ecmp_commit() (_commit_collect_routes()).
Let's call them "dynamic".
For those routes, we also must track an obj-state. Because we need to
keep track about whether the route was configured by NML3Cfg (and we
must delete it, when it no longer shall be configured). For that we need
the state.
Now we have two reasons why an obj-state is tracked. The "non-dynamic"
and the "dynamic" one.
Add two flags "os_dynamic" and "os_non_dynamic", respectively and adjust
the code for that tracking.
This commit alone is insufficient. The complexity of the following
commit is necessary, to flag who created a ObjStateData ("os_dynamic"
and "os_non_dynamic"). Without it, on each commit the object state will
be removed again.
WAS:
==============
l3cfg: track obj-state for single-hop route after ECMP commit
Before ECMP commit we don't know if the single-hop route is going to be
merged as ECMP multi-hop route. Therefore if the route has a weight
bigger than zero, it is going to be tracked only after the ECMP commit.
When a single-hop route is merged with another single-hop route from a
different interface creating a new ECMP route, NetworkManager needs to
schedule a commit on the l3cfg that is managing the first single-hop
route. Otherwise, the single-hop will continue to be there until a new
commit is scheduled.
Fixes: d9d33e2acc ('netns: fix configuring onlink routes for ECMP routes')
nm_netns_ip_route_ecmp_commit() returns the ECMP routes that didn't find
a merge-partner. It expects NML3Cfg to configure such routes in
platform.
Note that these routes have a positive "weight", which was used for
tracking the ECMP information in NML3Cfg and NMNetns.
But in kernel, single-hop routes don't have a weight. All routes in
NMPlatform will have a zero weight. While it somewhat works to call
nm_platform_ip_route_add() with a single-hop route of non-zero weight,
the result will be a different(!) route. This means for example that
nm_platform_ip_route_sync() will search the NMPlatform cache for whether
the routes exist, but in the cache single-hop routes with positive weight
never exist. Previously this happened and nm_platform_ip_route_sync()
would not delete those routes when it should.
The solution is that nm_netns_ip_route_ecmp_commit() does not tell
NML3Cfg to add a route, that cannot be added. Instead, it must first
normalize the weight to zero, to have a "regular" single-hop route.
Fixes: 5b5ce42682 ('nm-netns: track ECMP routes')
It wouldn't work otherwise. The object state is used to track routes
and compare them to what we find in platform.
A "metric_any" is useful at higher layers, to track a route where the
metric is decided by somebody else. But at the point when we add such an
object to the object-state, a fixed metric must be chosen.
Assert for that.
The hash/cmp functions were wrong with respect to IPv4 routes. Fix that.
- the weight was cast to a guint8, which is too small to hold all
values.
- NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID comparison would normalize a zero
weight to one
NM_CMP_DIRECT(NM_MAX(a->weight, 1u), NM_MAX(b->weight, 1u));
That was very wrong.
Clean this up. How the weight is handled, depends on the n_nexthops
parameter.
The remarkable thing is that upper layers find it useful to track IPv4
single-hop routes with a non-zero weight. Consequently, this is honored
by NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID which treats single-hop routes
different, if their weight differs. However, adding such a route in
kernel will not work. In kernel, single-hop routes have no weight. While
the route exists as distinct according to the implemented identity, it never
exists in kernel. Well, you can call nm_platform_ip_route_add() with such
a route, but the result will have a weight of zero (being a different
route). See also nm_platform_ip_route_normalize().
This works all mostly fine. The only thing to take care is that you
cannot look into the NMPlatform cache and ever find a IPv4 single-hop
route with a non-zero weight. If you preform such a lookup, realize that
such routes don't exist in platform. You can however normalize the
weight to zero first (nm_platform_ip_route_normalize()) and look for a
similar route with a zero weight.
Fixes: 1bbdecf5e1 ('platform: manage ECMP routes')
This commit fixes the build process for the documentation that was previously
unable to build separately via meson due to a dependency issue.
Previously, trying to build the API documentation via `ninja NetworkManager-doc`
failed due to missing dependencies (for example, `nm-dbus-types.xml` was not built).
I believe this happens due to some different handling of static paths vs. custom_target
by meson in this case.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1801
Fixes: 03637ad8b5 ('build: add initial support for meson build system')
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.
