Since commit 528a63d9cc ('platform: avoid unnecessary configuration of
IP address in nm_platform_ip_address_sync()'), we no longer configure the
IP address if it is in the platform cache. But the cache might not be
up to date. Process any pending netlink events.
https://bugzilla.redhat.com/show_bug.cgi?id=2073926
Fixes: 528a63d9cc ('platform: avoid unnecessary configuration of IP address in nm_platform_ip_address_sync()')
(cherry picked from commit 7f427ac4e6)
(cherry picked from commit e92639d89c)
Try to do one change at a time when reconfiguring addresses, to not
remove several/all addresses at once.
For IP addresses, kernel cares about the order in which they were added.
This mostly affects source address selection, and the "secondary" flag
for IPv4 addresses. The order is thus related to the priority of an
address.
There is no direct kernel API to change the order. Instead, we have to
add them in the correct order. During a sync, if an address already
exists in the wrong order, we need to remove it, and re-add it.
Btw, with IPv4 addresses added first via netlink are the primary
address, while with IPv6 it's reverse.
Previously, we would first iterate over all addresses and remove those
that had a conflicting order. This means, that we would potentially
remove all addresses for a short while, before readding them. That seems
problematic.
Instead, first track all addresses that are in the wrong order. And in
the step when we add/update the address, remove it. We now only remove
and address shortly before re-adding it. This way the time for which the
address on the interface is missing is shorter. More importantly, we will
never remove all addresses at the same time.
(cherry picked from commit a6fd641634)
(cherry picked from commit a1835c2c05)
The order of addresses matters. For "ipv4.addresses", the list
contains the primary address first. For "ipv6.addresses", the
order was reverted. This was also documented behavior.
The previous patch just changed behavior with respect to relative order
of static IPv6 addresses and autoconf6/DHCPv6. As we seem in the mood
for changing behavior, here is another one.
Now the addresses are interpreted in an order consistent with IPv4 and
how one might expect: preferred addresses first.
(cherry picked from commit 3d6b6aa317)
(cherry picked from commit 257221d198)
The order of addresses can matter for source address selection.
This is described in RFC 6724 section 5, but if the rules don't
determine a clear winner, the order matters.
Change the relative order of IPv6 addresses. Previously, we would prefer
autoconf6, over DHCPv6, over manual addresses. Now that got reverted
to make more sense and be consistent with IPv4.
Also, if we had multiple autoconf6 addresses (received at different
moments in time), then previously a newly received address would be
added with highest priority. Now, the older address will be preferred
and that order will be enforced (this can be a problem, see (*) below).
For IPv4, it's all simple and sensible. When we add addresses in kernel
via netlink, the first address (of a subnet) becomes the primary.
Note that we only control the order of addresses of the same subnet.
The addresses in ipv4.addresses" are sorted with primary address first.
In the same way is the order for addresses in NML3ConfigData and for
@known_addresses in nm_platform_ip_address_sync(), all primary-first.
Also, manual addresses are sorted with higher priority compared to DHCPv4
addresses (at least since NetworkManager 1.36). That means the way how we
merge NML3ConfigData makes sense (nm_l3_config_data_merge()) because we first
merge the static configuration, then the DHCPv4 configuration, where we just
append the lower priority DHCPv4 addresses.
For IPv6, the address priority is messed up. On netlink/kernel, the last added
address becomes the preferred one (we thus need to add them in the order of
lowest priority first). Consequently and historically, the IPv6 addresses in
@known_addresses parameter to nm_platform_ip_address_sync() were
lowest priority first. And so they were tracked in NML3ConfigData
and in the profile ("ipv6.addresses"). That is confusing.
Also, we usually want to merge NML3ConfigData with different priorities
(e.g. static configuration from the profile before autoconf6/DHCPv6),
as we do with IPv4. However, since internally IPv6 addresses are tracked in
reverse order, it means later NML3ConfigData would be appended and get effectively
a higher priority. That means, autoconf6 addresses were preferred over DHCPv6 and
over manual "ipv6.addresses", respectively. That seems undesirable and inconsistent
with IPv4. Change that. This is a change in behavior.
Note that changing the order of addresses means to remove and re-add
them in the right (inverse) order, with lease important first. This
means, when we add a new address with lower priority, we need to remove
all higher priority addresses temporarily, before readding them. That
is a problem(*).
Note that in the profile, "ipv6.addresses" is still tracked in reverse
order. This did not change, but might change later.
(cherry picked from commit 4a548423b9)
(cherry picked from commit 171d70bbf7)
Next we are going to assert that the prefix length is valid.
The test needs to have valid prefix lengths too. Adjust.
(cherry picked from commit a850e438a7)
(cherry picked from commit fee1d627e9)
Of course, the prefix length cannot be larger than 32 or 128.
But as C does implicit conversions, a buggy prefix length can
lead to a (wrongly) valid prefix length.
Make the type uint32, to prevent that (at least for common cases,
unless you pass a huge 64 bit integer).
(cherry picked from commit 0cf9db42d4)
(cherry picked from commit fb6e912810)
Some were duplicated. Drop those.
Some function were in an order where they required forward declarations.
Reorder.
(cherry picked from commit d7990b359b)
(cherry picked from commit 04d982e278)
We have this util function, presumably because it's good to have it.
Use it.
(cherry picked from commit 3a545fd041)
(cherry picked from commit 09832c5639)
For convenience, most to-string methods call nm_utils_to_string_buffer_init().
This allows to omit the string buffer and use a global (thread-local)
buffer.
That "convenience" seems error prone. Start drop it.
Start by adding a g_return_if_reached() assertion to catch the cases
that use it.
(cherry picked from commit 27752bfd5b)
(cherry picked from commit 3b56f33aa2)
These string functions allow to omit the string buffer. This is for
convenience, to use a global (thread-local) buffer. I think that is
error prone and we should drop that "convenience" feature.
At various places, pass a stack allocated buffer.
(cherry picked from commit b87afac8e8)
(cherry picked from commit 14b920d3cf)
I want to get rid of "_nm_utils_to_string_buffer" (or at least, limit
and control its use). Currently it's used all over the place only
to get the size of it. Add a define instead.
(cherry picked from commit 36e709c021)
(cherry picked from commit e13c2426c8)
We call sync many times. Often there is nothing to update. Check the
cache first, before (re) adding it.
Note that many addresses have a limited lifetime, that is, a lifetime
that keeps counting down with seconds granularity. For those (common)
cases we will only avoid the call to kernel if there are two syncs
within less than a second.
(cherry picked from commit 528a63d9cc)
(cherry picked from commit 429540a6b7)
Revert this change again. We are going to backport all the relevant
fixes from nm-1-38 about the address order.
This reverts commit da721a3f320b1e7e4628f7661bf3f78b7f12ed24.
There is only one caller of _addr_array_clean_expired(), and it always
provides the "idx" pointer.
(cherry picked from commit de9f174d51)
(cherry picked from commit 1f05866821)
Before 1.34, DHCPv6 addresses were preferred over SLAAC addresses
and all was good. Well, actually, we didn't have any CI tests, so
whether it really worked is only an assumption. But it probably was.
With 1.36 this broke for two reasons:
1) 1.36 would now prefer SLAAC over DHCPv6 over manual addresses.
2) 1.36 would also not adjust the order of already existing addresses.
This means, we first would get the SLAAC address and the DHCPv6
address later. Adding the address later would mean that it becomes
more important. This would go against 1), but due to 2) effectively
DHCPv6 was still preferred over SLAAC.
Commit [1] would fix 2), but now the address order changed.
We will need to fix also 1), but in the meantime, disable parts of
commit [1] so that we still get the old behavior.
[1] cd4601802d ('platform: fix address order in nm_platform_ip_address_sync()')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/ ## 1021
It is rather unlikely, that we call this function with no existing
routes/addresses. Hence, usually this does not safe an allocation
of the GPtrArray.
However, it's slightly less code and makes more sense this way
(instead of checking afterwards, whether the array is empty and
destroy it).
(cherry picked from commit 6bc9b73c55)
(cherry picked from commit e9d3ba66df)
The code is disabled at compile time. It's only useful for printf
debugging to modify the source to get more logging.
(cherry picked from commit fcb4033a81)
(cherry picked from commit d361bfc945)
pppd also tries to configure addresses by itself through some
ioctls. If we remove between those calls an address that was added,
pppd fails and quits.
To avoid this race condition, don't remove addresses while IPCP and
IPV6CP are running. Once pppd sends an IP configuration, it has
finished configuring the interface and we can proceed normally.
https://bugzilla.redhat.com/show_bug.cgi?id=2085382
(cherry picked from commit b41b11d613)
(cherry picked from commit e95b44bacb)
Add a function prevent the removal of addresses and routes from the
interface for a given address family.
(cherry picked from commit e8275d7139)
(cherry picked from commit 59ef1b4c78)
Currently we call nm_device_update_dynamic_ip_setup() in
carrier_changed() every time the carrier goes up again and the device
is activating, to kick a restart of DHCP.
Since we process link events in a idle handler, it can happen that the
handler is called only once for different events; in particular
device_link_changed() might be called once for a link-down/link-up
sequence.
carrier_changed() is "level-triggered" - it cares only about the
current carrier state. nm_device_update_dynamic_ip_setup() should
instead be "edge-triggered" - invoked every time the link goes from
down to up. We have a mechanism for that in device_link_changed(), use
it.
Fixes-test: @ipv4_spurious_leftover_route
https://bugzilla.redhat.com/show_bug.cgi?id=2079406https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1250
(cherry picked from commit d6429d3ddb)
(cherry picked from commit 1c158a5f37)
impl_ppp_manager_set_ip4_config always has been setting interface mtu
based on ppp configuration: do the same for ip6 in case it matters.
(cherry picked from commit 4d7b494eb3)
(cherry picked from commit 423e5e5011)
ipv6 DNS received on ppp interface were being ignored because their
priority was not set.
Fix this by using default priority in impl_ppp_manager_set_ip6_config(),
as was done for ip4_config in b2e559fab2 ("core: initialize l3cd
dns-priority for ppp and wwan")
Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1022
(cherry picked from commit 6991333bc0)
(cherry picked from commit d04eba0c40)
l3cd instances must be removed from the old l3cfg before calling
_cleanup_ip_pre(). Otherwise, _cleanup_ip_pre() unregisters them from
the device, and later _dev_l3_register_l3cds(self, l3cfg_old, FALSE,
FALSE) does nothing because the device doesn't have any l3cd.
Previously the l3cds would linger in the l3cfg, keeping a reference to
it and causing a memory leak; the leak was not detected by valgrind
because the l3cfg was still referenced by the NMNetns.
Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
Fixes-test: @stable_mem_consumption2
https://bugzilla.redhat.com/show_bug.cgi?id=2083453https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1252
(cherry picked from commit f69a1cc874)
(cherry picked from commit 83ee0f0779)
We want to assert that our alignment-guarantees do not exceed the
guarantees of the system-linker or system-allocator on the target
platform. Hence, we check against max_align_t. This is a lower bound,
but not the exact check we actually want. And as it turns out, on m64k
it is too low. Add a static check against 4-byte alignment for m64k as
a workaround.
Reported-by: Michael Biebl
Signed-off-by: David Rheinsberg <david.rheinsberg@gmail.com>
https://github.com/c-util/c-rbtree/issues/9eb778d3969
(cherry picked from commit 78831d127f)
(cherry picked from commit a83c884fb6)
Supplicant does not allow setting certain properties to empty values.
It also does not make sense.
Also, ifcfg-rh writer uses svSetValueStr() for these properties, so
the ifcfg plugin would always loose having hte values set to "".
Also, you couldn't enter these strings in nmcli.
It's fair to assume that it makes no sense to have these values set to
an empty value. Since we cannot just tighten up verification to reject
them, normalize them.
It also seems that some GUI now starts setting domain_suffix_match to an
empty string. Or maybe it was always doing it, and ifcfg plugin just hid
the problem? Anyway, we have users out there who set these properties to
"".
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/973
(cherry picked from commit 915e923928)
