New upstream version 1.47.90 upstream/1.47.90

4 files changed, 149 insertions, 22 deletions

diff --git a/src/core/dhcp/README.next.md b/src/core/dhcp/README.next.md
new file mode 100644
index 00000000..88fa6683
--- /dev/null
+++ b/src/core/dhcp/README.next.md
@@ -0,0 +1,103 @@
+`NMDhcpClient`
+==============
+
+Using `NMDhcpClient` still requires a lot of logic in `NMDevice`. The main goal
+is to simplify `NMDevice`, so `NMDhcpClient` must become more complicated to
+provide a simpler (but robust) API.
+
+NMDevice has basically two timeouts (talking about IPv4, but it applies
+similarly to IPv6): `ipv4.dhcp-timeout` and `ipv4.required-timeout`. They
+control how long NMDevice is willing to try, before failing the activation
+altogether. Note that with `ipv4.may-fail=yes`, we may very well never want to
+fail the activation entirely, regardless how DHCP is doing. In that case we
+want to stay up, but also constantly retrying whether we cannot get a lease and
+recover.
+
+Currently, if `NMDhcpClient` signals a failure, then it's basically up to
+`NMDevice` to schedule and retry. That is complicated, and we should move the
+complexity out of `NMDevice`.
+
+`NMDhcpClient` should have a simpler API:
+
+- `nm_dhcp_manager_start_ip[46]()`: creates (and starts) a `NMDhcpClient`
+  instance. The difference is, this function tries really hard not to fail
+  to create an `NMDhcpClient`. There is no explicit `start()`, but note that the
+  instance must not emit any signals before the next maincontext iteration. That is,
+  it only will call back the user after a timeout/idle or some other IO event, which
+  happens during a future iteration of the maincontext.
+
+- `nm_dhcp_client_stop()`: when `NMDevice` is done with the `NMDhcpClient`
+  instance, it will stop it and throw it away. This method exists because
+  `NMDhcpClient` is a `GObject` and ref-counted. Thus, we don't want to rely on
+  the last unref to stop the instance, but have an explicit stop. After stop, the
+  instance is defunct and won't emit any signals anymore. The class does not need
+  to support restarting a stopped instance. If `NMDevice` wants to restart DHCP, it
+  should create a new one. `NMDevice` would only want to do that, if the parameters
+  change, hence a new instance is in order (and no need for the complexity of
+  restart in `NMDhcpClient`).
+
+- as already now, `NMDhcpClient` is not very configurable. You provide most
+  (all) parameters during `nm_dhcp_manager_start_ip[46]()`, and then it keeps
+  running until stop.
+
+- `NMDhcpClient` exposes a simple state to the user:
+
+   1. "no lease, but good". When starting, there is no lease, but we are
+      optimistic to get one. This is the inital state, but we can also get back to
+      this state after we had a lease (which might expire).
+
+   1. "has a lease". Here there is no need to distinguish whether the current
+      lease was the first we received, or whether this was an update. In this state,
+      the instance has a lease and we are good.
+
+   1. "no lease, but bad". `NMDhcpClient` tries really hard, and "bad" does not
+      mean that it gave up. It will keep retrying, it's just that there is little
+      hope of getting a new lease. This happens, when you try to run DHCP on a Layer3
+      link (WireGuard). There is little hope to succeed, but `NMDhcpClient`
+      (theoretically) will retry and may recover from this. Another example is when
+      we fail to start dhclient because it's not installed. In that case, we are not
+      optimistic to recover, however `NMDhcpDhclient` will retry (with backoff
+      timeout) and might still recover from this. For most cases, `NMDevice` will
+      treat the no-lease cases the same, but in case of "bad" it might give up
+      earlier.
+
+When a lease expires, that does not necessarily mean that we are now in a bad
+state. It might mean that the DHCP server is temporarily down, but we might
+recover from that easily. "bad" really means, something is wrong on our side
+which prevents us from getting a lease. Also, imagine `dhclient` dies (we would
+try to restart, but assume that fails too), but we still have a valid lease,
+then possibly `NMDhcpClient` should still pretend all is good and we still have
+a lease until it expires. It may be we can recover before that happens. The
+point of all of this, is to hide errors as much as possibly and automatically
+recover. `NMDevice` will decide to tear down, if we didn't get a lease after
+`ipv4.dhcp-timeout`. That's the main criteria, and it might not even
+distinguish between "no lease, but good" and "no lease, but bad".
+
+- `NMDhcpClient` will also take care of the `ipv4.dhcp-timeout` grace period.
+  That timeout is provided during start, and starts ticking whenever there is
+  no lease. When it expires, a timeout signal gets emitted. That's it. This is
+  independent from the 3 states above, and only saves `NMDevice` from scheduling
+  this timer themselves.
+  This is NM_DHCP_CLIENT_NOTIFY_TYPE_NO_LEASE_TIMEOUT notification.
+
+- for nettools, `nm_dhcp_client_can_accept()` indicates that when we receive a
+  lease, we need to accept/decline it first. In that case, `NMDevice`
+optionally does ACD first, then configures the IP address first and calls
+`nm_dhcp_client_accept()`. In case of ACD conflict, it will call
+`nm_dhcp_client_decline()` (which optimally causes `NMDhcpClient` to get a
+different lease). With this, the above state "has a lease" has actually three
+flavors: "has a lease but not yet ACD probed" and "has a lease but
+accepted/declined" (but `NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED` gets only emitted
+when we get the lease, not when we accept/decline it). With `dhclient`, when we
+receive a lease, it means  "has a lease but accepted" right away.
+
+- for IPv6 prefix delegation, there is also `needed_prefixes` and
+  `NM_DHCP_CLIENT_NOTIFY_TYPE_PREFIX_DELEGATED`. Currently `needed_prefixes` needs
+  to be specified during start (which simplifies things). Maybe `needed_prefixes`
+  should be changable at runtime. Otherwise, whether we have prefixes is similar
+  to whether we have a lease, and the simple 3 states apply.
+
+When NetworkManager quits, it may want to leave the interface up. In that case,
+we still always want to stop the DHCP client, but possibly not deconfiguring
+the interface. I don't think that this concerns `NMDhcpClient`, because `NMDhcpClient`
+only provides the lease information and `NMDevice` is responsible to configure it.
diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index 8770656b..4ebc1754 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -824,9 +824,10 @@ _nm_dhcp_client_notify(NMDhcpClient         *self,
 
     _acd_check_lease(self, &acd_state);
 
-    options = priv->l3cd_next ? nm_dhcp_lease_get_options(
-                  nm_l3_config_data_get_dhcp_lease(priv->l3cd_next, priv->config.addr_family))
-                              : NULL;
+    options = priv->l3cd_next
+                  ? nm_dhcp_lease_get_options(
+                        nm_l3_config_data_get_dhcp_lease(priv->l3cd_next, priv->config.addr_family))
+                  : NULL;
 
     if (_LOGI_ENABLED()) {
         const char *req_str =
diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c
index ce1e9a45..b81ce77a 100644
--- a/src/core/dhcp/nm-dhcp-nettools.c
+++ b/src/core/dhcp/nm-dhcp-nettools.c
@@ -1483,6 +1483,11 @@ stop(NMDhcpClient *client, gboolean release)
     NMDhcpNettools        *self = NM_DHCP_NETTOOLS(client);
     NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self);
 
+    if (release) {
+        if (n_dhcp4_client_probe_release(priv->probe))
+            _LOGT("dhcp-client4: failed to send request with RELEASE message");
+    }
+
     NM_DHCP_CLIENT_CLASS(nm_dhcp_nettools_parent_class)->stop(client, release);
 
     _LOGT("dhcp-client4: stop " NM_HASH_OBFUSCATE_PTR_FMT, NM_HASH_OBFUSCATE_PTR(priv->client));
diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c
index 0fc5f928..5ede0df9 100644
--- a/src/core/dhcp/nm-dhcp-systemd.c
+++ b/src/core/dhcp/nm-dhcp-systemd.c
@@ -67,6 +67,15 @@ G_DEFINE_TYPE(NMDhcpSystemd, nm_dhcp_systemd, NM_TYPE_DHCP_CLIENT)
 
 /*****************************************************************************/
 
+static guint32
+lifetime_to_uint32(guint64 lft)
+{
+    if (lft == G_MAXUINT64)
+        return G_MAXUINT32;
+
+    return lft / 1000000;
+}
+
 static NML3ConfigData *
 lease_to_ip6_config(NMDhcpSystemd *self, sd_dhcp6_lease *lease, gint32 ts, GError **error)
 {
@@ -100,18 +109,19 @@ lease_to_ip6_config(NMDhcpSystemd *self, sd_dhcp6_lease *lease, gint32 ts, GErro
 
     if (!config->v6.info_only) {
         gboolean has_any_addresses = FALSE;
-        uint32_t lft_pref;
-        uint32_t lft_valid;
+        uint64_t lft_pref;
+        uint64_t lft_valid;
 
-        sd_dhcp6_lease_reset_address_iter(lease);
+        sd_dhcp6_lease_address_iterator_reset(lease);
         nm_gstring_prepare(&str);
-        while (sd_dhcp6_lease_get_address(lease, &tmp_addr, &lft_pref, &lft_valid) >= 0) {
-            const NMPlatformIP6Address address = {
+        while (sd_dhcp6_lease_get_address(lease, &tmp_addr) >= 0
+               && sd_dhcp6_lease_get_address_lifetime(lease, &lft_pref, &lft_valid) >= 0) {
+            NMPlatformIP6Address address = {
                 .plen        = 128,
                 .address     = tmp_addr,
                 .timestamp   = ts,
-                .lifetime    = lft_valid,
-                .preferred   = lft_pref,
+                .lifetime    = lifetime_to_uint32(lft_valid),
+                .preferred   = lifetime_to_uint32(lft_pref),
                 .addr_source = NM_IP_CONFIG_SOURCE_DHCP,
             };
 
@@ -121,6 +131,7 @@ lease_to_ip6_config(NMDhcpSystemd *self, sd_dhcp6_lease *lease, gint32 ts, GErro
             g_string_append(nm_gstring_add_space_delimiter(str), addr_str);
 
             has_any_addresses = TRUE;
+            sd_dhcp6_lease_address_iterator_next(lease);
         }
 
         if (str->len) {
@@ -160,11 +171,12 @@ lease_to_ip6_config(NMDhcpSystemd *self, sd_dhcp6_lease *lease, gint32 ts, GErro
         uint8_t         prefix_len;
 
         nm_gstring_prepare(&str);
-        sd_dhcp6_lease_reset_pd_prefix_iter(lease);
-        while (!sd_dhcp6_lease_get_pd(lease, &prefix, &prefix_len, NULL, NULL)) {
+        sd_dhcp6_lease_pd_iterator_reset(lease);
+        while (!sd_dhcp6_lease_get_pd_prefix(lease, &prefix, &prefix_len)) {
             nm_gstring_add_space_delimiter(str);
             nm_inet6_ntop(&prefix, addr_str);
             g_string_append_printf(str, "%s/%u", addr_str, prefix_len);
+            sd_dhcp6_lease_pd_iterator_next(lease);
         }
         if (str->len > 0) {
             nm_dhcp_option_add_option(options,
@@ -235,6 +247,8 @@ bound6_handle(NMDhcpSystemd *self)
     gs_free_error GError                   *error  = NULL;
     NMPlatformIP6Address                    prefix = {0};
     sd_dhcp6_lease                         *lease  = NULL;
+    guint64                                 lft_valid;
+    guint64                                 lft_pref;
 
     if (sd_dhcp6_client_get_lease(priv->client6, &lease) < 0 || !lease) {
         _LOGW(" no lease!");
@@ -254,14 +268,14 @@ bound6_handle(NMDhcpSystemd *self)
 
     _nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_BOUND, l3cd);
 
-    sd_dhcp6_lease_reset_pd_prefix_iter(lease);
-    while (!sd_dhcp6_lease_get_pd(lease,
-                                  &prefix.address,
-                                  &prefix.plen,
-                                  &prefix.preferred,
-                                  &prefix.lifetime)) {
+    sd_dhcp6_lease_pd_iterator_reset(lease);
+    while (!sd_dhcp6_lease_get_pd_prefix(lease, &prefix.address, &prefix.plen)
+           && !sd_dhcp6_lease_get_pd_lifetime(lease, &lft_pref, &lft_valid)) {
+        prefix.preferred = lifetime_to_uint32(lft_pref);
+        prefix.lifetime  = lifetime_to_uint32(lft_valid);
         prefix.timestamp = ts;
         nm_dhcp_client_emit_ipv6_prefix_delegated(NM_DHCP_CLIENT(self), &prefix);
+        sd_dhcp6_lease_pd_iterator_next(lease);
     }
 }
 
@@ -339,10 +353,10 @@ ip6_start(NMDhcpClient *client, const struct in6_addr *ll_addr, GError **error)
         return FALSE;
     }
 
-    r = sd_dhcp6_client_set_duid(sd_client,
-                                 unaligned_read_be16(&duid_arr[0]),
-                                 &duid_arr[2],
-                                 duid_len - 2);
+    r = sd_dhcp6_client_set_duid_raw(sd_client,
+                                     unaligned_read_be16(&duid_arr[0]),
+                                     &duid_arr[2],
+                                     duid_len - 2);
     if (r < 0) {
         nm_utils_error_set_errno(error, r, "failed to set DUID: %s");
         return FALSE;
@@ -450,6 +464,10 @@ stop(NMDhcpClient *client, gboolean release)
     if (!priv->client6)
         return;
 
+    r = sd_dhcp6_client_set_send_release(priv->client6, release);
+    if (r)
+        _LOGT("dhcp-client6: failed setting send-release");
+
     sd_dhcp6_client_set_callback(priv->client6, NULL, NULL);
     r = sd_dhcp6_client_stop(priv->client6);
     if (r)