fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-03-03 12:30:46 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

dhcp: rename/refactor nm_dhcp_client_set_state() to be notifications

Browse source 
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)
This commit is contained in:
Thomas Haller 2022-05-06 08:03:39 +02:00 • committed by Beniamino Galvani
parent 62ae5c0d0d
commit 2dba874c5a
4 changed files with 94 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

70
src/core/dhcp/nm-dhcp-client.c
View file

@ -244,23 +244,41 @@ _no_lease_timeout_schedule(NMDhcpClient *self)
/*****************************************************************************/
void
nm_dhcp_client_set_state(NMDhcpClient *self, NMDhcpState new_state, const NML3ConfigData *l3cd)
_nm_dhcp_client_notify(NMDhcpClient *self,
NMDhcpClientEventType client_event_type,
const NML3ConfigData *l3cd)
{
NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self);
GHashTable *options;
const int IS_IPv4 = NM_IS_IPv4(priv->config.addr_family);
nm_auto_unref_l3cd const NML3ConfigData *l3cd_merged = NULL;
if (NM_IN_SET(new_state, NM_DHCP_STATE_BOUND, NM_DHCP_STATE_EXTENDED)) {
nm_assert(NM_IS_L3_CONFIG_DATA(l3cd));
nm_assert(nm_l3_config_data_get_dhcp_lease(l3cd, priv->config.addr_family));
} else
nm_assert(!l3cd);
nm_assert(NM_IN_SET(client_event_type,
NM_DHCP_CLIENT_EVENT_TYPE_UNSPECIFIED,
NM_DHCP_CLIENT_EVENT_TYPE_BOUND,
NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED,
NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT,
NM_DHCP_CLIENT_EVENT_TYPE_EXPIRE,
NM_DHCP_CLIENT_EVENT_TYPE_FAIL,
NM_DHCP_CLIENT_EVENT_TYPE_TERMINATED));
nm_assert((client_event_type >= NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT)
== NM_IN_SET(client_event_type,
NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT,
NM_DHCP_CLIENT_EVENT_TYPE_EXPIRE,
NM_DHCP_CLIENT_EVENT_TYPE_FAIL,
NM_DHCP_CLIENT_EVENT_TYPE_TERMINATED));
nm_assert((!!l3cd)
== NM_IN_SET(client_event_type,
NM_DHCP_CLIENT_EVENT_TYPE_BOUND,
NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED));
nm_assert(!l3cd || NM_IS_L3_CONFIG_DATA(l3cd));
nm_assert(!l3cd || nm_l3_config_data_get_dhcp_lease(l3cd, priv->config.addr_family));
if (l3cd)
nm_l3_config_data_seal(l3cd);
if (new_state >= NM_DHCP_STATE_TIMEOUT)
if (client_event_type >= NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT)
watch_cleanup(self);
if (!IS_IPv4 && l3cd) {
@ -283,7 +301,7 @@ nm_dhcp_client_set_state(NMDhcpClient *self, NMDhcpState new_state, const NML3Co
}
/* FIXME(l3cfg:dhcp): the API of NMDhcpClient is changing to expose a simpler API.
* The internals like NMDhcpState should not be exposed (or possibly dropped in large
* The internals like the state should not be exposed (or possibly dropped in large
* parts). */
nm_l3_config_data_reset(&priv->l3cd, l3cd);
@ -344,7 +362,8 @@ nm_dhcp_client_set_state(NMDhcpClient *self, NMDhcpState new_state, const NML3Co
* as a static address (bypassing ACD), then NML3Cfg is aware of that and signals
* immediate success. */
if (nm_dhcp_client_can_accept(self) && new_state == NM_DHCP_STATE_BOUND && priv->l3cd
if (nm_dhcp_client_can_accept(self) && client_event_type == NM_DHCP_CLIENT_EVENT_TYPE_BOUND
&& priv->l3cd
&& nm_l3_config_data_get_num_addresses(priv->l3cd, priv->config.addr_family) > 0) {
priv->l3cfg_notify.wait_dhcp_commit = TRUE;
} else {
@ -381,7 +400,7 @@ daemon_watch_cb(GPid pid, int status, gpointer user_data)
priv->pid = -1;
nm_dhcp_client_set_state(self, NM_DHCP_STATE_TERMINATED, NULL);
_nm_dhcp_client_notify(self, NM_DHCP_CLIENT_EVENT_TYPE_TERMINATED, NULL);
}
void
@ -748,7 +767,7 @@ nm_dhcp_client_stop(NMDhcpClient *self, gboolean release)
_LOGI("canceled DHCP transaction");
nm_assert(priv->pid == -1);
nm_dhcp_client_set_state(self, NM_DHCP_STATE_TERMINATED, NULL);
_nm_dhcp_client_notify(self, NM_DHCP_CLIENT_EVENT_TYPE_TERMINATED, NULL);
}
/*****************************************************************************/
@ -882,6 +901,7 @@ nm_dhcp_client_handle_event(gpointer unused,
{
NMDhcpClientPrivate *priv;
nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL;
NMDhcpClientEventType client_event_type;
NMDhcpState new_state;
NMPlatformIP6Address prefix = {
0,
@ -988,7 +1008,33 @@ nm_dhcp_client_handle_event(gpointer unused,
new_state = NM_DHCP_STATE_UNKNOWN;
}
nm_dhcp_client_set_state(self, new_state, l3cd);
switch (new_state) {
case NM_DHCP_STATE_UNKNOWN:
client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_UNSPECIFIED;
break;
case NM_DHCP_STATE_BOUND:
client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_BOUND;
break;
case NM_DHCP_STATE_EXTENDED:
client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED;
break;
case NM_DHCP_STATE_TIMEOUT:
client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT;
break;
case NM_DHCP_STATE_EXPIRE:
client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_EXPIRE;
break;
case NM_DHCP_STATE_FAIL:
client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_FAIL;
break;
case NM_DHCP_STATE_TERMINATED:
case NM_DHCP_STATE_DONE:
case NM_DHCP_STATE_NOOP:
client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_TERMINATED;
break;
}
_nm_dhcp_client_notify(self, client_event_type, l3cd);
return TRUE;
}

17
src/core/dhcp/nm-dhcp-client.h
View file

@ -38,6 +38,18 @@ typedef enum {
NM_DHCP_STATE_TERMINATED, /* client is no longer running */
} NMDhcpState;
typedef enum {
NM_DHCP_CLIENT_EVENT_TYPE_UNSPECIFIED,
NM_DHCP_CLIENT_EVENT_TYPE_BOUND,
NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED,
NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT,
NM_DHCP_CLIENT_EVENT_TYPE_EXPIRE,
NM_DHCP_CLIENT_EVENT_TYPE_FAIL,
NM_DHCP_CLIENT_EVENT_TYPE_TERMINATED,
} NMDhcpClientEventType;
typedef enum _nm_packed {
NM_DHCP_CLIENT_NOTIFY_TYPE_LEASE_UPDATE,
@ -268,8 +280,9 @@ void nm_dhcp_client_watch_child(NMDhcpClient *self, pid_t pid);
void nm_dhcp_client_stop_watch_child(NMDhcpClient *self, pid_t pid);
void
nm_dhcp_client_set_state(NMDhcpClient *self, NMDhcpState new_state, const NML3ConfigData *l3cd);
void _nm_dhcp_client_notify(NMDhcpClient *self,
NMDhcpClientEventType client_event_type,
const NML3ConfigData *l3cd);
gboolean nm_dhcp_client_handle_event(gpointer unused,
const char *iface,

15
src/core/dhcp/nm-dhcp-nettools.c
View file

@ -796,15 +796,16 @@ bound4_handle(NMDhcpNettools *self, NDhcp4ClientLease *lease, gboolean extended)
if (!l3cd) {
_LOGW("failure to parse lease: %s", error->message);
g_clear_error(&error);
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
return;
}
lease_save(self, lease, priv->lease_file);
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self),
extended ? NM_DHCP_STATE_EXTENDED : NM_DHCP_STATE_BOUND,
l3cd);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self),
extended ? NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED
: NM_DHCP_CLIENT_EVENT_TYPE_BOUND,
l3cd);
}
static void
@ -841,10 +842,10 @@ dhcp4_event_handle(NMDhcpNettools *self, NDhcp4ClientEvent *event)
break;
case N_DHCP4_CLIENT_EVENT_RETRACTED:
case N_DHCP4_CLIENT_EVENT_EXPIRED:
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_EXPIRE, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_EXPIRE, NULL);
break;
case N_DHCP4_CLIENT_EVENT_CANCELLED:
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
break;
case N_DHCP4_CLIENT_EVENT_GRANTED:
priv->lease = n_dhcp4_client_lease_ref(event->granted.lease);
@ -896,7 +897,7 @@ dhcp4_event_cb(int fd, GIOCondition condition, gpointer user_data)
*/
_LOGE("error %d dispatching events", r);
nm_clear_g_source_inst(&priv->event_source);
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
return G_SOURCE_REMOVE;
}

25
src/core/dhcp/nm-dhcp-systemd.c
View file

@ -459,7 +459,7 @@ bound4_handle(NMDhcpSystemd *self, gboolean extended)
if (sd_dhcp_client_get_lease(priv->client4, &lease) < 0 || !lease) {
_LOGW("no lease!");
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
return;
}
@ -473,15 +473,16 @@ bound4_handle(NMDhcpSystemd *self, gboolean extended)
if (!l3cd) {
_LOGW("%s", error->message);
g_clear_error(&error);
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
return;
}
dhcp_lease_save(lease, priv->lease_file);
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self),
extended ? NM_DHCP_STATE_EXTENDED : NM_DHCP_STATE_BOUND,
l3cd);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self),
extended ? NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED
: NM_DHCP_CLIENT_EVENT_TYPE_BOUND,
l3cd);
}
static int
@ -500,10 +501,10 @@ dhcp_event_cb(sd_dhcp_client *client, int event, gpointer user_data)
switch (event) {
case SD_DHCP_CLIENT_EVENT_EXPIRED:
nm_dhcp_client_set_state(NM_DHCP_CLIENT(user_data), NM_DHCP_STATE_EXPIRE, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(user_data), NM_DHCP_CLIENT_EVENT_TYPE_EXPIRE, NULL);
break;
case SD_DHCP_CLIENT_EVENT_STOP:
nm_dhcp_client_set_state(NM_DHCP_CLIENT(user_data), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(user_data), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
break;
case SD_DHCP_CLIENT_EVENT_RENEW:
case SD_DHCP_CLIENT_EVENT_IP_CHANGE:
@ -876,7 +877,7 @@ bound6_handle(NMDhcpSystemd *self)
if (sd_dhcp6_client_get_lease(priv->client6, &lease) < 0 || !lease) {
_LOGW(" no lease!");
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
return;
}
@ -892,11 +893,11 @@ bound6_handle(NMDhcpSystemd *self)
if (!l3cd) {
_LOGW("%s", error->message);
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
return;
}
nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), NM_DHCP_STATE_BOUND, l3cd);
_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,
@ -921,11 +922,11 @@ dhcp6_event_cb(sd_dhcp6_client *client, int event, gpointer user_data)
switch (event) {
case SD_DHCP6_CLIENT_EVENT_RETRANS_MAX:
nm_dhcp_client_set_state(NM_DHCP_CLIENT(user_data), NM_DHCP_STATE_TIMEOUT, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(user_data), NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT, NULL);
break;
case SD_DHCP6_CLIENT_EVENT_RESEND_EXPIRE:
case SD_DHCP6_CLIENT_EVENT_STOP:
nm_dhcp_client_set_state(NM_DHCP_CLIENT(user_data), NM_DHCP_STATE_FAIL, NULL);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(user_data), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
break;
case SD_DHCP6_CLIENT_EVENT_IP_ACQUIRE:
case SD_DHCP6_CLIENT_EVENT_INFORMATION_REQUEST: