When we accept/decline a lease, then that only works if we are in state
GRANTED. n-dhcp4 API also requires us, to provide the exact lease, that
we were announced earlier.
As such, we need to make sure that we don't accept/decline in the wrong
state. That means, to keep track of what we are doing more carefully.
The functions _dhcp_client_accept()/_dhcp_client_decline() now take
a l3cd argument, the one that we announced earlier. And we check that it
still matches.
(cherry picked from commit 52a0fe584c)
(cherry picked from commit 1f7bede222)
They are no longer used from outside, NMDhcpClient fully handles this.
Make them static and internal.
Also, decline is currently unused. It will be used soon, with ACD
support.
(cherry picked from commit 4a256092ee)
(cherry picked from commit 27eb23ea44)
The function subscribes a callback l3_cfg_notify_cb(). Rename so that
related functions have a clearly related name.
(cherry picked from commit 9abcf3a53c)
(cherry picked from commit 344c0b3dfc)
The l3_cfg_notify_cb() handler is used for different purposes, and
different events will be considered.
Usually a switch statement is very nice for enums, especially if all
enum values should be handled (because the compiler can warn about
unhandled cases). In this case, not all events are supposed to be
handled. At this point, it seems nicer to just use an if block. It
better composes.
The compiler should be able to optimize both variants to the same
result. In any case, checking some integers for equality is in any case
going to be efficient.
(cherry picked from commit 7db07faa5e)
(cherry picked from commit 9acb6f9082)
As almost always, there is a point in keeping IPv4 and IPv6 implementations
similar. Behave different where there is an actual difference, at the bottom
of the stack.
(cherry picked from commit 7f943f5fa6)
(cherry picked from commit 0edfa4456a)
Technically, g_warn_if_reached() may not be an assertion, according to
glib. However, there is G_DEBUG=fatal-warnings and we want to run with
that.
So this is an assertion to us. Also, logging to stderr/stdout is not a
useful thing to the daemon. Don't do this. Especially, since it depends
on user provided (untrusted) input.
(cherry picked from commit 892cde1436)
(cherry picked from commit 38b8fdb75c)
Optimally we want stateless, pure code. Obviously, NMDhcpClient needs to
keep state to know what it's doing. However, we should well encapsulate
the state inside NMDhcpClient, and only accept events/notifications that
mutate the internal state according to certain rules.
Having a function public set_state(self, new_state) means that other
components (subclasses of NMDhcpClient) can directly mangle the state.
That means, you no longer need to only reason about the internal state
of NMDhcpClient (and the events/notifications/state-changes that it
implements). You also need to reason that other components take part of
maintaining that internal state.
Rename nm_dhcp_client_set_state() to nm_dhcp_client_notify(). Also, add
a new enum NMDhcpClientEventType with notification/event types.
In practice, this is only renaming. But naming is important, because it
suggests the reader how to think about the code.
(cherry picked from commit 97e65e4b50)
(cherry picked from commit 2dba874c5a)
The "noop" state is almost unused, however, nm_dhcp_set_state()
has a check "if (new_state >= NM_DHCP_STATE_TIMEOUT)", so the order
of the NOOP state matters.
Fix that by reordering.
Also, just return right away from NOOP.
(cherry picked from commit 9761e38f7e)
(cherry picked from commit 62ae5c0d0d)
NMDhcpState is very tied to events from dhclient. But most of these
states we don't care about, and NMDhcpClient definitely should abstract
and hide them.
We should repurpose NMDhcpState to simpler state. For that, first drop
the state from nm_dhcp_client_handle_event().
This is only the first step (which arguably makes the code more
complicated, because reason_to_state() gets spread out and the logic
happens more than once). That will be addressed next.
(cherry picked from commit f102051a29)
(cherry picked from commit 2c7f74ad94)
g_bytes_ref() does not accept NULL. But doing so can be convenient,
add a helper for that.
Note that g_bytes_unref() does accept NULL, so there is no corresponding
helper.
(cherry picked from commit 222f404928)
(cherry picked from commit 01bcde8ab0)
- use NM_IN_STRSET_ASCII_CASE().
- don't use else block after we return.
- don't accept the "iface" argument just for logging. The caller
can do the logging, if they wish.
(cherry picked from commit 9b9c07530c)
(cherry picked from commit ea7ad68ed2)
The "take" parameter of _set_effective_client_id() was always "FALSE". Drop it.
Also, drop _set_effective_client_id() and just call nm_dhcp_client_set_effective_client_id()
directly.
(cherry picked from commit 6b191d6ea7)
In nm_dns_manager_set_ip_config() we try to avoid calling update_dns()
unless something changes, because updating DNS is expensive and can
trigger other actions such as a new hostname resolution.
When we add a new ip_data, even if the new element is equivalent to
the old one that was removed, we need to sort the list again.
Fixes: ce0a36d20f ('dns: better track l3cd changes')
https://bugzilla.redhat.com/show_bug.cgi?id=2098574
(cherry picked from commit 3cc7801779)
(cherry picked from commit db4c55c8d3)
(cherry picked from commit 6ac62a746f)
Fixes: eb883e34a5 ('core: Add option to AddAndActivateConnection2 to bind the lifetime')
(cherry picked from commit 88f5e7518a)
(cherry picked from commit afe53b902f)
(cherry picked from commit 659ee74d8d)
There is no reason to disallow resetting the state.
(cherry picked from commit 607a9544cb)
(cherry picked from commit 6af0233a21)
(cherry picked from commit aebfb3461e)
Sync/blocking methods are ugly. Their name should highlight this.
Also, we may have an async variant, so we will need the "good" name
for apply() and apply_finish().
(cherry picked from commit dc66fb7d04)
(cherry picked from commit 558bcd5aae)
(cherry picked from commit 5235dce259)
Blocking calls are ugly. Rename those to have a "_sync()" suffix.
Also, split from _fw_nft_set_shared() the part that constructs the
stdin for nft.
(cherry picked from commit 7362ad6266)
(cherry picked from commit bbf3d01e82)
(cherry picked from commit 61ed013e7b)
In practice there is little difference.
Previously, "strbuf" would own the string until the end of the function,
when the "nm_auto_str_buf" cleanup attribute destroys it. In the
meantime, we would pass it on to _fw_nft_call_sync(), which in fact
won't access the string after returning.
Instead, we can just transfer ownership to the GBytes instance. That seems
more logical and safer than aliasing the buffer owned by NMStrBuf with
a g_bytes_new_static(). That way, we don't add a non-obvious restriction
on the lifetime of the string. The lifetime is now guarded by the GBytes
instance, which, could be referenced and kept alive longer.
There is also no runtime/memory overhead in doing this.
(cherry picked from commit 6a04bcc59d)
(cherry picked from commit c598f0ff0f)
NMStrBuf can also contains NUL characters. We thus cannot use g_strndup(),
which uses strncpy() and truncates at the first NUL.
Fixes: 13d25f9d0b ('glib-aux: add support for starting with stack-allocated buffer in NMStrBuf')
(cherry picked from commit 520411623d)
(cherry picked from commit 7a3de841b8)
(cherry picked from commit 51b9f0ad4c)
It's wrong, and it breaks certain uses.
Fixes: 13d25f9d0b ('glib-aux: add support for starting with stack-allocated buffer in NMStrBuf')
(cherry picked from commit c5ec4ebd77)
(cherry picked from commit 7b487e6951)
(cherry picked from commit fed6e78d05)
NM_MORE_ASSERT is a compile time constant. The compiler can optimize
it away just fine.
(cherry picked from commit 560feecb4c)
(cherry picked from commit de6da97e9d)
NM_STR_BUF_INIT() and nm_str_buf_init() were pretty much redundant. Drop one of
them.
Usually our pattern is that we don't have functions that return structs.
But NM_STR_BUF_INIT() returns a struct, because it's convenient to use
with
nm_auto_str_buf NMStrBuf strbuf = NM_STR_BUF_INIT(...);
So use that variant instead.
(cherry picked from commit 532f3e34a8)
(cherry picked from commit 90255a8aa8)
Allow to initialize NMStrBuf with an externally allocated array.
Usually a stack buffer. If the NMStrBuf grows beyond the size of
that initial buffer, then it would switch using malloc.
The idea is to support the common case where the result is small enough
to fit on the stack.
I always wanted to do such optimization because the main purpose of
NMStrBuf is to put it on the stack and ad-hoc construct a string.
I just figured, it would complicate the implementation and add
a runtime overhead. But turns out, it doesn't really.
The biggest question is how NMStrBuf should behave with a pre-allocated
buffer? Turns out, most choices can be made in a rather obvious way.
The only non-obvious thing is that nm_str_buf_finalize() would malloc()
a buffer, but that too seems consistent and what a user would probably
expect. As such, this doesn't seem to add unexpected semantics to the API.
(cherry picked from commit 13d25f9d0b)
(cherry picked from commit 51393413b4)
