colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

core: commit l3cd asynchronously on DHCP bound event

Browse Source 
When a lease is obtained, currently NMDevice performs a synchronous
commit of IP configuration and then accepts the lease.

Instead, let NMDevice only schedule a async commit; when the DHCP
client notices that the new address was committed it will
automatically accept it and emit a new signal so that the device can
succeed the activation.

Sync commits should be avoided because a commit does of things which
are outside the control of the caller (see the comment in
nm_device_l3cfg_commit()). Furthermore, when there are many pending
activations, async commits seem to help in reducing the CPU usage.

While making the commit async, also move the responsibility of the
accept() to NMDhcpClient.
This commit is contained in:
Beniamino Galvani
2022-01-18 13:28:58 +01:00
parent 62b2aa85e8
commit e1648d0665
3 changed files with 150 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

95
src/core/devices/nm-device.c
View File

@@ -815,8 +815,6 @@ static void _dev_ipdhcpx_set_state(NMDevice *self, int addr_family, NMDeviceIPSt
static void _dev_ipdhcpx_restart(NMDevice *self, int addr_family, gboolean release);
static void _dev_ipdhcpx_handle_accept(NMDevice *self, int addr_family, const NML3ConfigData *l3cd);
static gboolean
_dev_ipac6_grace_period_start(NMDevice *self, guint32 timeout_sec, gboolean force_restart);
@@ -9915,63 +9913,6 @@ _dev_ipdhcpx_handle_fail(NMDevice *self, int addr_family, const char *reason)
_dev_ip_state_check_async(self, addr_family);
}
static void
_dev_ipdhcpx_handle_accept(NMDevice *self, int addr_family, const NML3ConfigData *l3cd)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self);
const int IS_IPv4 = NM_IS_IPv4(addr_family);
gs_free_error GError *error = NULL;
nm_assert(NM_IS_L3_CONFIG_DATA(l3cd));
nm_dhcp_config_set_lease(priv->ipdhcp_data_x[IS_IPv4].config, l3cd);
_dev_l3_register_l3cds_set_one_full(self,
L3_CONFIG_DATA_TYPE_DHCP_X(IS_IPv4),
l3cd,
NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE,
TRUE);
/* FIXME(l3cfg:dhcp): accept also should be handled by NMDhcpClient transparently.
* NMDhcpClient should do ACD (if enabled), and only after that passes, it exposes
* the lease to the user (NMDevice). NMDevice then may choose to configure the address.
* NMDhcpClient then checks in NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT whether the requested
* address is to be configured by NML3Cfg (here, the intent is what matters, not
* whether the address is actually visible in NMPlatform -- that is because while NML3Cfg
* configures the address (in platform), the user might delete it right away. Also,
* depending on the commit type, NML3Cfg may even choose not to configure it right now
* (which arguably will be odd). Anyway, what matters is whether the user configured
* the address in NML3Cfg (by checking the ObjStateData).
*
* If yes, then NMDhcpClient needs to accept automatically.
*
* Doing it here is wrong for two reasons:
*
* - NMDevice already has enough to do, it should not be concerned with telling
* NMDhcpClient to accept the lease, it should only configure the address.
*
* - we should only accept the lease *after* configuring the address (see n_dhcp4_client_lease_accept().
* Currently this only works by passing commit_sync=TRUE to _dev_l3_register_l3cds_set_one(), but
* doing that is wrong (see FIXME why commit_sync needs to go away). */
if (priv->ipdhcp_data_x[IS_IPv4].state != NM_DEVICE_IP_STATE_READY
&& !nm_dhcp_client_accept(priv->ipdhcp_data_x[IS_IPv4].client, &error)) {
_LOGW(LOGD_DHCPX(IS_IPv4), "error accepting lease: %s", error->message);
_dev_ipdhcpx_set_state(self, addr_family, NM_DEVICE_IP_STATE_FAILED);
_dev_ip_state_check_async(self, addr_family);
return;
}
_dev_ipdhcpx_set_state(self, addr_family, NM_DEVICE_IP_STATE_READY);
nm_dispatcher_call_device(NM_DISPATCHER_ACTION_DHCP_CHANGE_X(IS_IPv4),
self,
NULL,
NULL,
NULL,
NULL);
_dev_ip_state_check_async(self, addr_family);
}
static void
_dev_ipdhcpx_notify(NMDhcpClient *client, const NMDhcpClientNotifyData *notify_data, NMDevice *self)
{
@@ -10015,8 +9956,32 @@ _dev_ipdhcpx_notify(NMDhcpClient *client, const NMDhcpClientNotifyData *notify_d
return;
}
if (notify_data->lease_update.accepted) {
_LOGT_ipdhcp(addr_family, "lease accepted");
if (priv->ipdhcp_data_x[IS_IPv4].state != NM_DEVICE_IP_STATE_READY) {
_dev_ipdhcpx_set_state(self, addr_family, NM_DEVICE_IP_STATE_READY);
nm_dispatcher_call_device(NM_DISPATCHER_ACTION_DHCP_CHANGE_X(IS_IPv4),
self,
NULL,
NULL,
NULL,
NULL);
_dev_ip_state_check_async(self, addr_family);
}
return;
}
/* Schedule a commit of the configuration. The DHCP client
* will accept the lease once the address is committed, and
* will send a LEASE_UPDATE notification with accepted=1. */
_LOGT_ipdhcp(addr_family, "lease update");
_dev_ipdhcpx_handle_accept(self, addr_family, notify_data->lease_update.l3cd);
nm_dhcp_config_set_lease(priv->ipdhcp_data_x[IS_IPv4].config,
notify_data->lease_update.l3cd);
_dev_l3_register_l3cds_set_one_full(self,
L3_CONFIG_DATA_TYPE_DHCP_X(IS_IPv4),
notify_data->lease_update.l3cd,
NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE,
FALSE);
return;
}
@@ -10213,8 +10178,14 @@ _dev_ipdhcpx_start(NMDevice *self, int addr_family)
priv->ipdhcp_data_x[IS_IPv4].config = nm_dhcp_config_new(addr_family, previous_lease);
_notify(self, PROP_DHCPX_CONFIG(IS_IPv4));
}
if (previous_lease)
_dev_ipdhcpx_handle_accept(self, addr_family, previous_lease);
if (previous_lease) {
nm_dhcp_config_set_lease(priv->ipdhcp_data_x[IS_IPv4].config, previous_lease);
_dev_l3_register_l3cds_set_one_full(self,
L3_CONFIG_DATA_TYPE_DHCP_X(IS_IPv4),
previous_lease,
NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE,
FALSE);
}
return;

124
src/core/dhcp/nm-dhcp-client.c
View File

@@ -43,12 +43,17 @@ typedef struct _NMDhcpClientPrivate {
const NML3ConfigData *l3cd;
GSource *no_lease_timeout_source;
GSource *ipv6_lladdr_timeout_source;
GBytes *effective_client_id;
pid_t pid;
guint watch_id;
NMDhcpState state;
bool iaid_explicit : 1;
bool is_stopped : 1;
GBytes *effective_client_id;
struct {
gulong id;
bool wait_dhcp_commit : 1;
bool wait_ll_address : 1;
} l3cfg_notify;
} NMDhcpClientPrivate;
G_DEFINE_ABSTRACT_TYPE(NMDhcpClient, nm_dhcp_client, G_TYPE_OBJECT)
@@ -57,6 +62,11 @@ G_DEFINE_ABSTRACT_TYPE(NMDhcpClient, nm_dhcp_client, G_TYPE_OBJECT)
/*****************************************************************************/
static void
l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcpClient *self);
/*****************************************************************************/
/* we use pid=-1 for invalid PIDs. Ensure that pid_t can hold negative values. */
G_STATIC_ASSERT(!(((pid_t) -1) > 0));
@@ -70,6 +80,27 @@ _emit_notify(NMDhcpClient *self, const NMDhcpClientNotifyData *notify_data)
/*****************************************************************************/
static void
connect_l3cfg_notify(NMDhcpClient *self)
{
NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self);
gboolean do_connect;
do_connect = priv->l3cfg_notify.wait_dhcp_commit | priv->l3cfg_notify.wait_ll_address;
if (!do_connect) {
nm_clear_g_signal_handler(priv->config.l3cfg, &priv->l3cfg_notify.id);
return;
}
if (priv->l3cfg_notify.id == 0) {
priv->l3cfg_notify.id = g_signal_connect(priv->config.l3cfg,
NM_L3CFG_SIGNAL_NOTIFY,
G_CALLBACK(l3_cfg_notify_cb),
self);
}
}
pid_t
nm_dhcp_client_get_pid(NMDhcpClient *self)
{
@@ -357,6 +388,9 @@ nm_dhcp_client_set_state(NMDhcpClient *self, NMDhcpState new_state, const NML3Co
},
};
priv->l3cfg_notify.wait_dhcp_commit = (new_state == NM_DHCP_STATE_BOUND);
connect_l3cfg_notify(self);
_emit_notify(self, &notify_data);
}
}
@@ -523,14 +557,20 @@ l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcp
nm_assert(l3cfg == priv->config.l3cfg);
if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE) {
switch (notify_data->notify_type) {
case NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE:
{
const NMPlatformIP6Address *addr;
gs_free_error GError *error = NULL;
if (!priv->l3cfg_notify.wait_ll_address)
return;
addr = ipv6_lladdr_find(self);
if (addr) {
_LOGD("got IPv6LL address, starting transaction");
g_signal_handlers_disconnect_by_func(l3cfg, l3_cfg_notify_cb, self);
priv->l3cfg_notify.wait_ll_address = FALSE;
connect_l3cfg_notify(self);
nm_clear_g_source_inst(&priv->ipv6_lladdr_timeout_source);
schedule_no_lease_timeout(self);
@@ -543,6 +583,74 @@ l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcp
}));
}
}
break;
}
case NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT:
{
const NML3ConfigData *committed_l3cd;
NMDedupMultiIter ipconf_iter;
const NMPlatformIPAddress *lease_address;
gs_free_error GError *error = NULL;
/* A new configuration was committed to the interface. If we previously
* got a lease, check whether we are waiting for the address to be
* configured. If the address was added, we can proceed accepting the
* lease and notifying NMDevice. */
if (!priv->l3cfg_notify.wait_dhcp_commit)
return;
nm_l3_config_data_iter_ip_address_for_each (&ipconf_iter,
priv->l3cd,
priv->config.addr_family,
&lease_address)
break;
nm_assert(lease_address);
committed_l3cd = nm_l3cfg_get_combined_l3cd(l3cfg, TRUE);
if (priv->config.addr_family == AF_INET) {
const NMPlatformIP4Address *address4 = (const NMPlatformIP4Address *) lease_address;
if (!nm_l3_config_data_lookup_address_4(committed_l3cd,
address4->address,
address4->plen,
address4->peer_address))
return;
} else {
const NMPlatformIP6Address *address6 = (const NMPlatformIP6Address *) lease_address;
if (!nm_l3_config_data_lookup_address_6(committed_l3cd, &address6->address))
return;
}
priv->l3cfg_notify.wait_dhcp_commit = FALSE;
connect_l3cfg_notify(self);
_LOGD("accept address");
if (!nm_dhcp_client_accept(self, &error)) {
gs_free char *reason = g_strdup_printf("error accepting lease: %s", error->message);
_emit_notify(self,
&((NMDhcpClientNotifyData){
.notify_type = NM_DHCP_CLIENT_NOTIFY_TYPE_IT_LOOKS_BAD,
.it_looks_bad.reason = reason,
}));
return;
}
_emit_notify(
self,
&((NMDhcpClientNotifyData){.notify_type = NM_DHCP_CLIENT_NOTIFY_TYPE_LEASE_UPDATE,
.lease_update = {
.l3cd = priv->l3cd,
.accepted = TRUE,
}}));
break;
};
default:
/* ignore */;
}
}
@@ -569,10 +677,8 @@ nm_dhcp_client_start_ip6(NMDhcpClient *self, GError **error)
addr = ipv6_lladdr_find(self);
if (!addr) {
_LOGD("waiting for IPv6LL address");
g_signal_connect(priv->config.l3cfg,
NM_L3CFG_SIGNAL_NOTIFY,
G_CALLBACK(l3_cfg_notify_cb),
self);
priv->l3cfg_notify.wait_ll_address = TRUE;
connect_l3cfg_notify(self);
priv->ipv6_lladdr_timeout_source =
nm_g_timeout_add_seconds_source(10, ipv6_lladdr_timeout, self);
return TRUE;
@@ -657,7 +763,9 @@ nm_dhcp_client_stop(NMDhcpClient *self, gboolean release)
priv->is_stopped = TRUE;
g_signal_handlers_disconnect_by_func(priv->config.l3cfg, l3_cfg_notify_cb, self);
priv->l3cfg_notify.wait_dhcp_commit = FALSE;
priv->l3cfg_notify.wait_ll_address = FALSE;
connect_l3cfg_notify(self);
/* Kill the DHCP client */
old_pid = priv->pid;

1
src/core/dhcp/nm-dhcp-client.h
View File

@@ -71,6 +71,7 @@ typedef struct {
* or NULL (if a previous lease timed out). It can also be the
* previous lease, that was injected. */
const NML3ConfigData *l3cd;
bool accepted;
} lease_update;
struct {
const NMPlatformIP6Address *prefix;