diff options
author | Thomas Haller <thaller@redhat.com> | 2022-07-20 22:56:37 +0200 |
---|---|---|
committer | Beniamino Galvani <bgalvani@redhat.com> | 2022-10-17 09:19:27 +0200 |
commit | 1f3edc8c4314364155980ba3fea00f4d3c2ee43a (patch) | |
tree | 43e39b51ff771f6a4e3ba509df49c6caa3e86515 | |
parent | bd286f68947d738031c1d4bf6a7bc92626099ad0 (diff) | |
download | NetworkManager-1f3edc8c4314364155980ba3fea00f4d3c2ee43a.tar.gz |
dhcp: fix EXTENDED DHCP event to accept lease for dhclient pluginbg/dhcp6-dad-on-1-36
n-dhcp4 only supports calling ACCEPT during the GRANTED state.
Not during a EXTENDED event. So usually, we would not want
to call accept in that case.
And we didn't. During EXTENDED event, we would usually skip ACD (because
it's either not enabled or we already passed ACD for the current address).
In that case, in _nm_dhcp_client_notify() we hit the line
if (client_event_type == NM_DHCP_CLIENT_EVENT_TYPE_BOUND && priv->l3cd_curr
&& nm_l3_config_data_get_num_addresses(priv->l3cd_curr, priv->config.addr_family) > 0)
priv->l3cfg_notify.wait_dhcp_commit = TRUE;
else
priv->l3cfg_notify.wait_dhcp_commit = FALSE;
and would not set `wait_dhpc_commit`. That means, we never called _dhcp_client_accept().
For nettools, that doesn't really matter because calling ACCEPT during EXTENDED
is invalid anyway. However, for dhclient that is fatal because we wouldn't reply the
D-Bus request from nm-dhcp-helper. The helper times out after 60 seconds and dhclient
would misbehave.
We need to fix that by also calling _dhcp_client_accept() in the case when we don't
need to wait (the EXTENDED case).
However, previously _dhcp_client_accept() was rather peculiar and didn't like to be
called in an unexpected state. Relax that. Now, when calling accept in an unexpected
state, just do nothing and signal success. That frees the caller from the complexity
to understand when they must/must not call accept.
https://bugzilla.redhat.com/show_bug.cgi?id=2109285
Fixes: 156d84217ced ('dhcp/dhclient: implement accept/decline (ACD) for dhclient plugin')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1308
(cherry picked from commit 5077018ff4dbdb073fb67d41aab721a16b1ab315)
(cherry picked from commit f7ced16ccc0e2d35266b72c173ec0e57514272c9)
-rw-r--r-- | src/core/dhcp/nm-dhcp-client.c | 19 | ||||
-rw-r--r-- | src/core/dhcp/nm-dhcp-nettools.c | 6 |
2 files changed, 15 insertions, 10 deletions
diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index d885d07454..81bef8dfda 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -436,6 +436,17 @@ _nm_dhcp_client_notify(NMDhcpClient *self, } else { priv->l3cfg_notify.wait_dhcp_commit = FALSE; } + + if (!priv->l3cfg_notify.wait_dhcp_commit && priv->l3cd) { + gs_free_error GError *error = NULL; + + _LOGD("accept lease right away"); + if (!_dhcp_client_accept(self, priv->l3cd, &error)) { + _LOGD("accept failed: %s", error->message); + /* Unclear why this happened, or what to do about it. Just proceed. */ + } + } + l3_cfg_notify_check_connected(self); { @@ -501,12 +512,8 @@ _accept(NMDhcpClient *self, const NML3ConfigData *l3cd, GError **error) if (!NM_IS_IPv4(priv->config.addr_family)) return TRUE; - if (!priv->v4.bound.invocation) { - nm_utils_error_set(error, - NM_UTILS_ERROR_UNKNOWN, - "calling accept in unexpected script state"); - return FALSE; - } + if (!priv->v4.bound.invocation) + return TRUE; g_dbus_method_invocation_return_value(g_steal_pointer(&priv->v4.bound.invocation), NULL); return TRUE; diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 3f6e3ac8d4..494745bbef 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -1036,10 +1036,8 @@ _accept(NMDhcpClient *client, const NML3ConfigData *l3cd, GError **error) g_return_val_if_fail(l3cd, FALSE); - if (priv->granted.lease_l3cd != l3cd) { - nm_utils_error_set(error, NM_UTILS_ERROR_UNKNOWN, "calling accept in unexpected state"); - return FALSE; - } + if (priv->granted.lease_l3cd != l3cd) + return TRUE; nm_assert(priv->granted.lease); |