diff --git a/contrib/scripts/nm-in-container.sh b/contrib/scripts/nm-in-container.sh index 95fc306a9530fd7b1f6aae2bbb30aa932f8a03a0..ed859436fc54077b68f248af4117727b94b590fc 100755 --- a/contrib/scripts/nm-in-container.sh +++ b/contrib/scripts/nm-in-container.sh @@ -9,14 +9,17 @@ set -e # - build: build a new image, named "$CONTAINER_NAME_REPOSITORY:$CONTAINER_NAME_TAG" ("nm:nm") # - run: start the container and tag it "$CONTAINER_NAME_NAME" ("nm") # - exec: run bash inside the container +# - journal|j: print the journal from inside the container # - stop: stop the container # - clean: delete the container and the image. # # Options: # --no-cleanup: don't delete the CONTAINERFILE and other artifacts # --stop: only has effect with "run". It will stop the container afterwards. -# -- [COMMAND]: with command "exec", provide a command to run in the container. -# Defaults to "bash". +# -- [EXTRA_ARGS]: +# - with command "exec", provide a command and arguments to run in the container. +# Defaults to "bash". +# - with command "journal", additional arguments that are passed to journalctl. # # It bind mounts the current working directory inside the container. # You can run `make install` and run tests. @@ -32,11 +35,13 @@ CONTAINER_NAME_REPOSITORY=${CONTAINER_NAME_REPOSITORY:-nm} CONTAINER_NAME_TAG=${CONTAINER_NAME_TAG:-nm} CONTAINER_NAME_NAME=${CONTAINER_NAME_NAME:-nm} +EXEC_ENV=() + ############################################################################### usage() { cat <&1 | tee -a /tmp/nm-log.txt systemctl stop NetworkManager; gdb -ex run --args /opt/test/sbin/NetworkManager --debug +systemctl stop NetworkManager; /opt/test/sbin/NetworkManager --debug 2>&1 | tee -a ./nm-log.txt EOF cat < /etc/NetworkManager/conf.d/95-user.conf COPY data-bash_history /root/.bash_history COPY data-gdbinit /root/.gdbinit COPY data-gdb_history /root/.gdb_history @@ -421,18 +435,29 @@ do_run() { do_exec() { do_run + local e local EXTRA_ARGS=("$@") if [ "${#EXTRA_ARGS[@]}" = 0 ]; then EXTRA_ARGS=('bash') fi - podman exec --workdir "$BASEDIR_NM" -it "$CONTAINER_NAME_NAME" "${EXTRA_ARGS[@]}" + local ENV=() + for e in "${EXEC_ENV[@]}" ; do + ENV+=(-e "$e") + done + + podman exec "${ENV[@]}" --workdir "$BASEDIR_NM" -it "$CONTAINER_NAME_NAME" "${EXTRA_ARGS[@]}" if [ "$DO_STOP" = 1 ]; then do_stop fi } +do_journal() { + EXEC_ENV+=( "SYSTEMD_COLORS=0" ) + do_exec "journalctl" --no-pager "$@" +} + do_stop() { container_is_running "$CONTAINER_NAME_NAME" || return 0 podman stop "$CONTAINER_NAME_NAME" @@ -453,7 +478,10 @@ for (( i=1 ; i<="$#" ; )) ; do --stop) DO_STOP=1 ;; - build|run|exec|stop|clean) + j) + CMD=journal + ;; + build|run|exec|stop|clean|journal) CMD=$c ;; --) @@ -465,8 +493,13 @@ for (( i=1 ; i<="$#" ; )) ; do exit 0 ;; *) - usage - die "invalid argument: $c" + if [ "$CMD" = "journal" ]; then + EXTRA_ARGS=( "${@:$((i-1))}" ) + break; + else + usage + die "invalid argument: $c" + fi ;; esac done @@ -475,7 +508,7 @@ done test "$UID" != 0 || die "cannot run as root" -if test $CMD != exec && test "${#EXTRA_ARGS[@]}" != 0 ; then +if test "$CMD" != exec -a "$CMD" != journal -a "${#EXTRA_ARGS[@]}" != 0 ; then die "Extra arguments are only allowed with exec command" fi diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 0b19d32601c516e476f60482c7eb726926a98ae4..a16603320ae0973d5f367f4a8318779a4939053d 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -10296,7 +10296,11 @@ _dev_ipdhcpx_start(NMDevice *self, int addr_family) .vendor_class_identifier = vendor_class_identifier, .use_fqdn = hostname_is_fqdn, .reject_servers = reject_servers, - .v4.request_broadcast = request_broadcast, + .v4 = + { + .request_broadcast = request_broadcast, + .acd_timeout_msec = _prop_get_ipv4_dad_timeout(self), + }, }; priv->ipdhcp_data_4.client = @@ -10311,22 +10315,25 @@ _dev_ipdhcpx_start(NMDevice *self, int addr_family) duid = _prop_get_ipv6_dhcp_duid(self, connection, hwaddr, &enforce_duid); config = (NMDhcpClientConfig){ - .addr_family = AF_INET6, - .l3cfg = nm_device_get_l3cfg(self), - .iface = nm_device_get_ip_iface(self), - .uuid = nm_connection_get_uuid(connection), - .send_hostname = nm_setting_ip_config_get_dhcp_send_hostname(s_ip), - .hostname = nm_setting_ip_config_get_dhcp_hostname(s_ip), - .hostname_flags = _prop_get_ipvx_dhcp_hostname_flags(self, AF_INET6), - .client_id = duid, - .mud_url = _prop_get_connection_mud_url(self, s_con), - .timeout = no_lease_timeout_sec, - .anycast_address = _device_get_dhcp_anycast_address(self), - .v6.enforce_duid = enforce_duid, - .v6.iaid = iaid, - .v6.iaid_explicit = iaid_explicit, - .v6.info_only = (priv->ipdhcp_data_6.v6.mode == NM_NDISC_DHCP_LEVEL_OTHERCONF), - .v6.needed_prefixes = priv->ipdhcp_data_6.v6.needed_prefixes, + .addr_family = AF_INET6, + .l3cfg = nm_device_get_l3cfg(self), + .iface = nm_device_get_ip_iface(self), + .uuid = nm_connection_get_uuid(connection), + .send_hostname = nm_setting_ip_config_get_dhcp_send_hostname(s_ip), + .hostname = nm_setting_ip_config_get_dhcp_hostname(s_ip), + .hostname_flags = _prop_get_ipvx_dhcp_hostname_flags(self, AF_INET6), + .client_id = duid, + .mud_url = _prop_get_connection_mud_url(self, s_con), + .timeout = no_lease_timeout_sec, + .anycast_address = _device_get_dhcp_anycast_address(self), + .v6 = + { + .enforce_duid = enforce_duid, + .iaid = iaid, + .iaid_explicit = iaid_explicit, + .info_only = (priv->ipdhcp_data_6.v6.mode == NM_NDISC_DHCP_LEVEL_OTHERCONF), + .needed_prefixes = priv->ipdhcp_data_6.v6.needed_prefixes, + }, }; priv->ipdhcp_data_6.client = diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index b1e1f9ab3aa37d554628b16e4d19f5e11c2f12ab..64fa0abace48097ee6a4db314a36794051f2def9 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -32,6 +32,38 @@ /*****************************************************************************/ +/* This is how long we do ACD for each entry and reject new offers for + * the same address. Note that the maximum ACD timeout is limited to 30 seconds + * (NM_ACD_TIMEOUT_MAX_MSEC). + **/ +#define ACD_REGLIST_GRACE_PERIOD_MSEC 300000u + +G_STATIC_ASSERT(ACD_REGLIST_GRACE_PERIOD_MSEC > (NM_ACD_TIMEOUT_MAX_MSEC + 1000)); + +#define ACD_REGLIST_MAX_ENTRIES 30 + +/* To do ACD for an address (new lease), we will register a NML3ConfigData + * with l3cfg. After ACD completes, we still continue having NML3Cfg + * watch that address, for ACD_REGLIST_GRACE_PERIOD_MSEC. The reasons are: + * + * - the caller is supposed to actually configure the address right after + * ACD passed. We would not want to drop the ACD state before the caller + * got a chance to do that. + * - when ACD fails, we decline the address and expect the DHCP client + * to present a new lease. We may want to outright reject the address, + * if ACD is bad. Thus, we want to keep running ACD for the address a bit + * longer, so that future requests for the same address can be rejected. + * + * This data structure is used for tracking the registered ACD address. + */ +typedef struct { + const NML3ConfigData *l3cd; + gint64 expiry_msec; + in_addr_t addr; +} AcdRegListData; + +/*****************************************************************************/ + enum { SIGNAL_NOTIFY, LAST_SIGNAL, @@ -42,20 +74,64 @@ static guint signals[LAST_SIGNAL] = {0}; NM_GOBJECT_PROPERTIES_DEFINE(NMDhcpClient, PROP_CONFIG, ); typedef struct _NMDhcpClientPrivate { - NMDhcpClientConfig config; - const NML3ConfigData *l3cd; - GSource *no_lease_timeout_source; - GSource *ipv6_lladdr_timeout_source; - GSource *watch_source; - GBytes *effective_client_id; - pid_t pid; - bool iaid_explicit : 1; - bool is_stopped : 1; + NMDhcpClientConfig config; + + /* This is the "next" data. That is, the one what was received last via + * _nm_dhcp_client_notify(), but which is currently pending on ACD. */ + const NML3ConfigData *l3cd_next; + + /* This is the currently exposed data. It passed ACD (or no ACD was performed), + * and is set from l3cd_next. */ + const NML3ConfigData *l3cd_curr; + + GSource *no_lease_timeout_source; + GSource *watch_source; + GBytes *effective_client_id; + + union { + struct { + struct { + NML3CfgCommitTypeHandle *l3cfg_commit_handle; + GSource *done_source; + + /* When we do ACD for a l3cd lease, we will keep running ACD for + * the grace period ACD_REGLIST_GRACE_PERIOD_MSEC, even if we already + * determined the state. There are two reasons for that: + * + * - after ACD completes we notify the lease to the user, who is supposed + * to configure the address in NML3Cfg. If we were already removing the + * ACD state from NML3Cfg, ACD might need to start over. Instead, when + * the caller tries to configure the address, ACD state is already good. + * + * - if we decline on ACD offer, we may want to keep running and + * select other offers. Offers for which we just failed ACD (within + * ACD_REGLIST_GRACE_PERIOD_MSEC) are rejected. See _nm_dhcp_client_accept_offer(). + * For that, we keep monitoring the ACD state for up to ACD_REGLIST_MAX_ENTRIES + * addresses, to not restart and select the same lease twice in a row. + */ + GArray *reglist; + GSource *reglist_timeout_source; + + in_addr_t addr; + NMOptionBool state; + } acd; + struct { + GDBusMethodInvocation *invocation; + } bound; + } v4; + struct { + GSource *lladdr_timeout_source; + } v6; + }; + struct { gulong id; bool wait_dhcp_commit : 1; bool wait_ll_address : 1; } l3cfg_notify; + + pid_t pid; + bool is_stopped : 1; } NMDhcpClientPrivate; G_DEFINE_ABSTRACT_TYPE(NMDhcpClient, nm_dhcp_client, G_TYPE_OBJECT) @@ -64,9 +140,22 @@ G_DEFINE_ABSTRACT_TYPE(NMDhcpClient, nm_dhcp_client, G_TYPE_OBJECT) /*****************************************************************************/ +#define L3CD_ACD_TAG(priv) (&(priv)->v4.acd.addr) + +static gboolean _dhcp_client_accept(NMDhcpClient *self, const NML3ConfigData *l3cd, GError **error); + +static gboolean _dhcp_client_decline(NMDhcpClient *self, + const NML3ConfigData *l3cd, + const char *error_message, + GError **error); + static void l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcpClient *self); +static void _acd_reglist_timeout_reschedule(NMDhcpClient *self, gint64 now_msec); + +static void _acd_reglist_data_remove(NMDhcpClient *self, guint idx, gboolean do_log); + /*****************************************************************************/ /* we use pid=-1 for invalid PIDs. Ensure that pid_t can hold negative values. */ @@ -174,12 +263,13 @@ _emit_notify(NMDhcpClient *self, const NMDhcpClientNotifyData *notify_data) /*****************************************************************************/ static void -connect_l3cfg_notify(NMDhcpClient *self) +l3_cfg_notify_check_connected(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; + do_connect = priv->l3cfg_notify.wait_dhcp_commit | priv->l3cfg_notify.wait_ll_address + | (NM_IS_IPv4(priv->config.addr_family) && priv->v4.acd.l3cfg_commit_handle); if (!do_connect) { nm_clear_g_signal_handler(priv->config.l3cfg, &priv->l3cfg_notify.id); @@ -285,6 +375,333 @@ _no_lease_timeout_schedule(NMDhcpClient *self) /*****************************************************************************/ +static void +_acd_state_reset(NMDhcpClient *self, gboolean forget_addr, gboolean forget_reglist) +{ + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + + if (!NM_IS_IPv4(priv->config.addr_family)) + return; + + if (priv->v4.acd.addr != INADDR_ANY) { + nm_l3cfg_commit_type_clear(priv->config.l3cfg, &priv->v4.acd.l3cfg_commit_handle); + l3_cfg_notify_check_connected(self); + nm_clear_g_source_inst(&priv->v4.acd.done_source); + if (forget_addr) { + priv->v4.acd.addr = INADDR_ANY; + priv->v4.acd.state = NM_OPTION_BOOL_DEFAULT; + } + } else + nm_assert(priv->v4.acd.state == NM_OPTION_BOOL_DEFAULT); + + if (forget_reglist) { + guint n; + + while ((n = nm_g_array_len(priv->v4.acd.reglist)) > 0) + _acd_reglist_data_remove(self, n - 1, TRUE); + } + + nm_assert(!priv->v4.acd.l3cfg_commit_handle); + nm_assert(!priv->v4.acd.done_source); + nm_assert(!forget_reglist + || !nm_l3cfg_remove_config_all(priv->config.l3cfg, L3CD_ACD_TAG(priv))); +} + +static gboolean +_acd_complete_on_idle_cb(gpointer user_data) +{ + NMDhcpClient *self = user_data; + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + + nm_assert(NM_IS_IPv4(priv->config.addr_family)); + nm_assert(priv->v4.acd.addr != INADDR_ANY); + nm_assert(!priv->v4.acd.l3cfg_commit_handle); + nm_assert(priv->l3cd_next); + + _acd_state_reset(self, FALSE, FALSE); + + _nm_dhcp_client_notify(self, NM_DHCP_CLIENT_EVENT_TYPE_BOUND, priv->l3cd_next); + + return G_SOURCE_CONTINUE; +} + +#define _acd_reglist_data_get(priv, idx) \ + nm_g_array_index_p((priv)->v4.acd.reglist, AcdRegListData, (idx)) + +static guint +_acd_reglist_data_find(NMDhcpClientPrivate *priv, in_addr_t addr_needle) +{ + const guint n = nm_g_array_len(priv->v4.acd.reglist); + guint i; + + nm_assert(addr_needle != INADDR_ANY); + + for (i = 0; i < n; i++) { + AcdRegListData *reglist_data = _acd_reglist_data_get(priv, i); + + if (reglist_data->addr == addr_needle) + return i; + } + return G_MAXUINT; +} + +static void +_acd_reglist_data_remove(NMDhcpClient *self, guint idx, gboolean do_log) +{ + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + AcdRegListData *reglist_data; + + nm_assert(idx < nm_g_array_len(priv->v4.acd.reglist)); + + reglist_data = _acd_reglist_data_get(priv, idx); + + if (do_log) { + char sbuf_addr[NM_UTILS_INET_ADDRSTRLEN]; + + _LOGD("acd: drop check for address %s (l3cd " NM_HASH_OBFUSCATE_PTR_FMT ")", + _nm_utils_inet4_ntop(reglist_data->addr, sbuf_addr), + NM_HASH_OBFUSCATE_PTR(reglist_data->l3cd)); + } + + if (!nm_l3cfg_remove_config(priv->config.l3cfg, L3CD_ACD_TAG(priv), reglist_data->l3cd)) + nm_assert_not_reached(); + + nm_clear_l3cd(®list_data->l3cd); + + nm_l3cfg_commit_on_idle_schedule(priv->config.l3cfg, NM_L3_CFG_COMMIT_TYPE_UPDATE); + + g_array_remove_index(priv->v4.acd.reglist, idx); + + if (priv->v4.acd.reglist->len == 0) { + nm_clear_pointer(&priv->v4.acd.reglist, g_array_unref); + nm_clear_g_source_inst(&priv->v4.acd.reglist_timeout_source); + } +} + +static gboolean +_acd_reglist_timeout_cb(gpointer user_data) +{ + NMDhcpClient *self = user_data; + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + gint64 now_msec; + + nm_clear_g_source_inst(&priv->v4.acd.reglist_timeout_source); + + now_msec = nm_utils_get_monotonic_timestamp_msec(); + + while (nm_g_array_len(priv->v4.acd.reglist) > 0) { + AcdRegListData *reglist_data = _acd_reglist_data_get(priv, 0); + + if (reglist_data->expiry_msec > now_msec) + break; + + _acd_reglist_data_remove(self, 0, TRUE); + } + + _acd_reglist_timeout_reschedule(self, now_msec); + + return G_SOURCE_CONTINUE; +} + +static void +_acd_reglist_timeout_reschedule(NMDhcpClient *self, gint64 now_msec) +{ + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + AcdRegListData *reglist_data; + + if (nm_g_array_len(priv->v4.acd.reglist) == 0) { + nm_assert(!priv->v4.acd.reglist_timeout_source); + return; + } + + if (priv->v4.acd.reglist_timeout_source) { + /* already pending. As we only add new elements with a *later* + * expiry, we don't need to ever cancel a pending timer. Worst + * case, the timer fires, and there is nothing to do and we + * reschedule. */ + return; + } + + now_msec = nm_utils_get_monotonic_timestamp_msec(); + + reglist_data = _acd_reglist_data_get(priv, 0); + + nm_assert(reglist_data->expiry_msec > now_msec); + + priv->v4.acd.reglist_timeout_source = + nm_g_timeout_add_source(reglist_data->expiry_msec - now_msec, + _acd_reglist_timeout_cb, + self); +} + +static void +_acd_check_lease(NMDhcpClient *self, NMOptionBool *out_acd_state) +{ + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + char sbuf_addr[NM_UTILS_INET_ADDRSTRLEN]; + in_addr_t addr; + gboolean addr_changed = FALSE; + guint idx; + gint64 now_msec; + + if (!NM_IS_IPv4(priv->config.addr_family)) + goto handle_no_acd; + + if (!priv->l3cd_next) + goto handle_no_acd; + + /* an IPv4 lease is always expected to have exactly one address. */ + nm_assert(nm_l3_config_data_get_num_addresses(priv->l3cd_next, AF_INET) == 1); + + if (priv->config.v4.acd_timeout_msec == 0) + goto handle_no_acd; + + addr = NMP_OBJECT_CAST_IP4_ADDRESS( + nm_l3_config_data_get_first_obj(priv->l3cd_next, NMP_OBJECT_TYPE_IP4_ADDRESS, NULL)) + ->address; + nm_assert(addr != INADDR_ANY); + + nm_clear_g_source_inst(&priv->v4.acd.done_source); + + if (priv->v4.acd.state != NM_OPTION_BOOL_DEFAULT && priv->v4.acd.addr == addr) { + /* the ACD state is already determined. Return right away. */ + nm_assert(!priv->v4.acd.l3cfg_commit_handle); + *out_acd_state = !!priv->v4.acd.state; + return; + } + + if (priv->v4.acd.addr != addr) { + addr_changed = TRUE; + priv->v4.acd.addr = addr; + } + + _LOGD("acd: %s check for address %s (timeout %u msec, l3cd " NM_HASH_OBFUSCATE_PTR_FMT ")", + addr_changed ? "add" : "update", + _nm_utils_inet4_ntop(addr, sbuf_addr), + priv->config.v4.acd_timeout_msec, + NM_HASH_OBFUSCATE_PTR(priv->l3cd_next)); + + priv->v4.acd.state = NM_OPTION_BOOL_DEFAULT; + + if (nm_l3cfg_add_config(priv->config.l3cfg, + L3CD_ACD_TAG(priv), + FALSE, + priv->l3cd_next, + NM_L3CFG_CONFIG_PRIORITY_IPV4LL, + 0, + 0, + NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP4, + NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP6, + 0, + 0, + NM_DNS_PRIORITY_DEFAULT_NORMAL, + NM_DNS_PRIORITY_DEFAULT_NORMAL, + NM_L3_ACD_DEFEND_TYPE_ONCE, + NM_MIN(priv->config.v4.acd_timeout_msec, NM_ACD_TIMEOUT_MAX_MSEC), + NM_L3CFG_CONFIG_FLAGS_ONLY_FOR_ACD, + NM_L3_CONFIG_MERGE_FLAGS_NONE)) + addr_changed = TRUE; + + if (!priv->v4.acd.reglist) + priv->v4.acd.reglist = g_array_new(FALSE, FALSE, sizeof(AcdRegListData)); + + idx = _acd_reglist_data_find(priv, addr); + + now_msec = nm_utils_get_monotonic_timestamp_msec(); + + g_array_append_val(priv->v4.acd.reglist, + ((AcdRegListData){ + .l3cd = nm_l3_config_data_ref(priv->l3cd_next), + .addr = addr, + .expiry_msec = now_msec + ACD_REGLIST_GRACE_PERIOD_MSEC, + })); + + if (idx != G_MAXUINT) { + /* we already tracked this "addr". We don't need to track it twice, + * forget about this one. This also has the effect, that we will + * always append the new entry to the list (so the list + * stays sorted by the increasing timestamp). */ + _acd_reglist_data_remove(self, idx, FALSE); + } + + if (priv->v4.acd.reglist->len > ACD_REGLIST_MAX_ENTRIES) { + /* rate limit how many addresses we track for ACD. */ + _acd_reglist_data_remove(self, 0, TRUE); + } + + _acd_reglist_timeout_reschedule(self, now_msec); + + if (!priv->v4.acd.l3cfg_commit_handle) { + priv->v4.acd.l3cfg_commit_handle = + nm_l3cfg_commit_type_register(priv->config.l3cfg, + NM_L3_CFG_COMMIT_TYPE_UPDATE, + NULL, + "dhcp4-acd"); + l3_cfg_notify_check_connected(self); + } + + if (addr_changed) + nm_l3cfg_commit_on_idle_schedule(priv->config.l3cfg, NM_L3_CFG_COMMIT_TYPE_AUTO); + + /* ACD is started/pending... */ + nm_assert(priv->v4.acd.addr != INADDR_ANY); + nm_assert(priv->v4.acd.state == NM_OPTION_BOOL_DEFAULT); + nm_assert(priv->v4.acd.l3cfg_commit_handle); + nm_assert(priv->l3cfg_notify.id); + *out_acd_state = NM_OPTION_BOOL_DEFAULT; + return; + +handle_no_acd: + /* Indicate that ACD is good (or disabled) by returning TRUE. */ + _acd_state_reset(self, TRUE, FALSE); + *out_acd_state = NM_OPTION_BOOL_TRUE; + return; +} + +/*****************************************************************************/ + +gboolean +_nm_dhcp_client_accept_offer(NMDhcpClient *self, gconstpointer p_yiaddr) +{ + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + char sbuf_addr[NM_UTILS_INET_ADDRSTRLEN]; + NMIPAddr yiaddr; + const NML3AcdAddrInfo *acd_info; + + if (!NM_IS_IPv4(priv->config.addr_family)) + return nm_assert_unreachable_val(FALSE); + + if (priv->config.v4.acd_timeout_msec == 0) { + /* ACD is disabled. Note that we might track the address for other + * reasons and have information about the ACD state below. But + * with ACD disabled, we always ignore that information. */ + return TRUE; + } + + nm_ip_addr_set(priv->config.addr_family, &yiaddr, p_yiaddr); + + /* Note that once we do ACD for a certain address, even after completing + * it, we keep the l3cd registered in NML3Cfg for ACD_REGLIST_GRACE_PERIOD_MSEC + * The idea is, that we don't yet turn off ACD for a grace period, so that + * we can avoid selecting the same lease again. + * + * Note that we even check whether we have an ACD state if priv->v4.acd.reglist + * is empty. Maybe for odd reasons, we track ACD for the address already. */ + + acd_info = nm_l3cfg_get_acd_addr_info(priv->config.l3cfg, yiaddr.addr4); + + if (!acd_info) + return TRUE; + + if (!NM_IN_SET(acd_info->state, NM_L3_ACD_ADDR_STATE_USED, NM_L3_ACD_ADDR_STATE_CONFLICT)) + return TRUE; + + _LOGD("offered lease rejected: address %s failed ACD check", + _nm_utils_inet4_ntop(yiaddr.addr4, sbuf_addr)); + + return FALSE; +} + void _nm_dhcp_client_notify(NMDhcpClient *self, NMDhcpClientEventType client_event_type, @@ -292,6 +709,8 @@ _nm_dhcp_client_notify(NMDhcpClient *self, { NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); GHashTable *options; + gboolean l3cd_changed; + NMOptionBool acd_state; const int IS_IPv4 = NM_IS_IPv4(priv->config.addr_family); nm_auto_unref_l3cd const NML3ConfigData *l3cd_merged = NULL; char sbuf1[NM_HASH_OBFUSCATE_PTR_STR_BUF_SIZE]; @@ -320,10 +739,9 @@ _nm_dhcp_client_notify(NMDhcpClient *self, _LOGT("notify: event=%s%s%s", nm_dhcp_client_event_type_to_string(client_event_type), - NM_PRINT_FMT_QUOTED2(l3cd, "", ", l3cd=", NM_HASH_OBFUSCATE_PTR_STR(l3cd, sbuf1))); + NM_PRINT_FMT_QUOTED2(l3cd, ", l3cd=", NM_HASH_OBFUSCATE_PTR_STR(l3cd, sbuf1), "")); - if (l3cd) - nm_l3_config_data_seal(l3cd); + nm_l3_config_data_seal(l3cd); if (client_event_type >= NM_DHCP_CLIENT_EVENT_TYPE_TIMEOUT) watch_cleanup(self); @@ -332,34 +750,40 @@ _nm_dhcp_client_notify(NMDhcpClient *self, /* nm_dhcp_utils_merge_new_dhcp6_lease() relies on "life_starts" option * for merging, which is only set by dhclient. Internal client never sets that, * but it supports multiple IP addresses per lease. */ - if (nm_dhcp_utils_merge_new_dhcp6_lease(priv->l3cd, l3cd, &l3cd_merged)) { + if (nm_dhcp_utils_merge_new_dhcp6_lease(priv->l3cd_next, l3cd, &l3cd_merged)) { _LOGD("lease merged with existing one"); l3cd = nm_l3_config_data_seal(l3cd_merged); } } - if (priv->l3cd == l3cd) - return; - if (l3cd) { nm_clear_g_source_inst(&priv->no_lease_timeout_source); - } else { - if (priv->l3cd) - _no_lease_timeout_schedule(self); - } + } else + _no_lease_timeout_schedule(self); - /* FIXME(l3cfg:dhcp): the API of NMDhcpClient is changing to expose a simpler API. - * The internals like the state should not be exposed (or possibly dropped in large - * parts). */ + l3cd_changed = nm_l3_config_data_reset(&priv->l3cd_next, l3cd); - nm_l3_config_data_reset(&priv->l3cd, l3cd); + _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; + + if (_LOGI_ENABLED()) { + const char *req_str = + IS_IPv4 ? nm_dhcp_option_request_string(AF_INET, NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS) + : nm_dhcp_option_request_string(AF_INET6, NM_DHCP_OPTION_DHCP6_NM_IP_ADDRESS); + const char *addr = nm_g_hash_table_lookup(options, req_str); - options = l3cd ? nm_dhcp_lease_get_options( - nm_l3_config_data_get_dhcp_lease(l3cd, priv->config.addr_family)) - : NULL; + _LOGI("state changed %s%s%s%s", + priv->l3cd_next ? "new lease" : "no lease", + NM_PRINT_FMT_QUOTED2(addr, ", address=", addr, ""), + acd_state == NM_OPTION_BOOL_DEFAULT ? ", acd pending" + : (acd_state ? "" : ", acd conflict")); + } if (_LOGD_ENABLED()) { - if (options) { + if (l3cd_changed && options) { gs_free const char **keys = NULL; guint nkeys; guint i; @@ -373,50 +797,41 @@ _nm_dhcp_client_notify(NMDhcpClient *self, } } - if (_LOGI_ENABLED()) { - const char *req_str = - IS_IPv4 ? nm_dhcp_option_request_string(AF_INET, NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS) - : nm_dhcp_option_request_string(AF_INET6, NM_DHCP_OPTION_DHCP6_NM_IP_ADDRESS); - const char *addr = nm_g_hash_table_lookup(options, req_str); + if (acd_state == NM_OPTION_BOOL_DEFAULT) { + /* ACD is in progress... */ + return; + } - _LOGI("state changed %s%s%s%s", - priv->l3cd ? "new lease" : "no lease", - NM_PRINT_FMT_QUOTED(addr, ", address=", addr, "", "")); + if (!acd_state) { + gs_free_error GError *error = NULL; + + /* We only decline. We don't actually emit to the caller that + * something is wrong (like NM_DHCP_CLIENT_NOTIFY_TYPE_IT_LOOKS_BAD). + * If we would, NMDevice might decide to tear down the device, when + * we actually should continue trying to get a better lease. There + * is already "ipv4.dhcp-timeout" which will handle the failure if + * we don't get a good lease. */ + if (!_dhcp_client_decline(self, priv->l3cd_next, "acd failed", &error)) + _LOGD("decline failed: %s", error->message); + return; } - /* FIXME(l3cfg:dhcp:acd): NMDhcpClient must also do ACD. It needs acd_timeout_msec - * as a configuration parameter (in NMDhcpClientConfig). When ACD is enabled, - * when a new lease gets announced, it must first use NML3Cfg to run ACD on the - * interface (the previous lease -- if any -- will still be used at that point). - * If ACD fails, we call nm_dhcp_client_decline() and try to get a different - * lease. - * If ACD passes, we need to notify the new lease, and the user (NMDevice) may - * then configure the address. We need to watch the configured addresses (in NML3Cfg), - * and if the address appears there, we need to accept the lease. That is complicated - * but necessary, because we can only accept the lease after we configured the - * address. - * - * As a whole, ACD is transparent for the user (NMDevice). It's entirely managed - * by NMDhcpClient. Note that we do ACD through NML3Cfg, which centralizes IP handling - * for one interface, so for example if the same address happens to be configured - * as a static address (bypassing ACD), then NML3Cfg is aware of that and signals - * immediate success. */ - - 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) { + nm_l3_config_data_reset(&priv->l3cd_curr, priv->l3cd_next); + + 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 { + else priv->l3cfg_notify.wait_dhcp_commit = FALSE; - } - connect_l3cfg_notify(self); + + l3_cfg_notify_check_connected(self); { const NMDhcpClientNotifyData notify_data = { .notify_type = NM_DHCP_CLIENT_NOTIFY_TYPE_LEASE_UPDATE, .lease_update = { - .l3cd = priv->l3cd, + .l3cd = priv->l3cd_curr, .accepted = !priv->l3cfg_notify.wait_dhcp_commit, }, }; @@ -466,56 +881,80 @@ nm_dhcp_client_stop_watch_child(NMDhcpClient *self, pid_t pid) watch_cleanup(self); } -gboolean -nm_dhcp_client_accept(NMDhcpClient *self, GError **error) +static gboolean +_accept(NMDhcpClient *self, const NML3ConfigData *l3cd, GError **error) { - NMDhcpClientPrivate *priv; - - g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE); - - priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); - g_return_val_if_fail(priv->l3cd, FALSE); + if (!NM_IS_IPv4(priv->config.addr_family)) + return TRUE; - if (NM_DHCP_CLIENT_GET_CLASS(self)->accept) { - return NM_DHCP_CLIENT_GET_CLASS(self)->accept(self, error); + if (!priv->v4.bound.invocation) { + nm_utils_error_set(error, + NM_UTILS_ERROR_UNKNOWN, + "calling accept in unexpected script state"); + return FALSE; } + g_dbus_method_invocation_return_value(g_steal_pointer(&priv->v4.bound.invocation), NULL); return TRUE; } -gboolean -nm_dhcp_client_can_accept(NMDhcpClient *self) +static gboolean +_dhcp_client_accept(NMDhcpClient *self, const NML3ConfigData *l3cd, GError **error) { - gboolean can_accept; + NMDhcpClientClass *klass; g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE); + nm_assert(l3cd); - can_accept = !!(NM_DHCP_CLIENT_GET_CLASS(self)->accept); + klass = NM_DHCP_CLIENT_GET_CLASS(self); - nm_assert(can_accept == (!!(NM_DHCP_CLIENT_GET_CLASS(self)->decline))); + g_return_val_if_fail(NM_DHCP_CLIENT_GET_PRIVATE(self)->l3cd_curr, FALSE); - return can_accept; + return klass->accept(self, l3cd, error); } -gboolean -nm_dhcp_client_decline(NMDhcpClient *self, const char *error_message, GError **error) +static gboolean +decline(NMDhcpClient *self, const NML3ConfigData *l3cd, const char *error_message, GError **error) { - NMDhcpClientPrivate *priv; - - g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE); - - priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); - g_return_val_if_fail(priv->l3cd, FALSE); + if (!NM_IS_IPv4(priv->config.addr_family)) + return TRUE; - if (NM_DHCP_CLIENT_GET_CLASS(self)->decline) { - return NM_DHCP_CLIENT_GET_CLASS(self)->decline(self, error_message, error); + if (!priv->v4.bound.invocation) { + nm_utils_error_set(error, + NM_UTILS_ERROR_UNKNOWN, + "calling decline in unexpected script state"); + return FALSE; } + g_dbus_method_invocation_return_error(g_steal_pointer(&priv->v4.bound.invocation), + NM_DEVICE_ERROR, + NM_DEVICE_ERROR_FAILED, + "acd failed"); return TRUE; } +static gboolean +_dhcp_client_decline(NMDhcpClient *self, + const NML3ConfigData *l3cd, + const char *error_message, + GError **error) +{ + NMDhcpClientClass *klass; + + g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE); + nm_assert(l3cd); + + klass = NM_DHCP_CLIENT_GET_CLASS(self); + + g_return_val_if_fail(NM_DHCP_CLIENT_GET_PRIVATE(self)->l3cd_next, FALSE); + + return klass->decline(self, l3cd, error_message, error); +} + static GBytes * get_duid(NMDhcpClient *self) { @@ -528,7 +967,7 @@ ipv6_lladdr_timeout(gpointer user_data) NMDhcpClient *self = user_data; NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); - nm_clear_g_source_inst(&priv->ipv6_lladdr_timeout_source); + nm_clear_g_source_inst(&priv->v6.lladdr_timeout_source); _emit_notify( self, @@ -548,6 +987,8 @@ ipv6_lladdr_find(NMDhcpClient *self) NMDedupMultiIter iter; const NMPObject *obj; + nm_assert(!NM_IS_IPv4(priv->config.addr_family)); + l3cfg = priv->config.l3cfg; nmp_lookup_init_object(&lookup, NMP_OBJECT_TYPE_IP6_ADDRESS, nm_l3cfg_get_ifindex(l3cfg)); @@ -568,24 +1009,21 @@ static void l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcpClient *self) { NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + char sbuf_addr[NM_UTILS_INET_ADDRSTRLEN]; nm_assert(l3cfg == priv->config.l3cfg); - switch (notify_data->notify_type) { - case NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE: - { + if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE + && priv->l3cfg_notify.wait_ll_address) { 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"); priv->l3cfg_notify.wait_ll_address = FALSE; - connect_l3cfg_notify(self); - nm_clear_g_source_inst(&priv->ipv6_lladdr_timeout_source); + l3_cfg_notify_check_connected(self); + nm_clear_g_source_inst(&priv->v6.lladdr_timeout_source); _no_lease_timeout_schedule(self); @@ -597,11 +1035,10 @@ l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcp })); } } - - break; } - case NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT: - { + + if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT + && priv->l3cfg_notify.wait_dhcp_commit) { const NML3ConfigData *committed_l3cd; NMDedupMultiIter ipconf_iter; const NMPlatformIPAddress *lease_address; @@ -612,11 +1049,8 @@ l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcp * 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->l3cd_curr, priv->config.addr_family, &lease_address) break; @@ -630,41 +1064,82 @@ l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcp address4->address, address4->plen, address4->peer_address)) - return; + goto wait_dhcp_commit_done; } else { const NMPlatformIP6Address *address6 = (const NMPlatformIP6Address *) lease_address; if (!nm_l3_config_data_lookup_address_6(committed_l3cd, &address6->address)) - return; + goto wait_dhcp_commit_done; } priv->l3cfg_notify.wait_dhcp_commit = FALSE; - connect_l3cfg_notify(self); + l3_cfg_notify_check_connected(self); - _LOGD("accept address"); + _LOGD("accept lease"); - if (!nm_dhcp_client_accept(self, &error)) { + if (!_dhcp_client_accept(self, priv->l3cd_curr, &error)) { gs_free char *reason = g_strdup_printf("error accepting lease: %s", error->message); + _LOGD("accept failed: %s", error->message); + _emit_notify(self, &((NMDhcpClientNotifyData){ .notify_type = NM_DHCP_CLIENT_NOTIFY_TYPE_IT_LOOKS_BAD, .it_looks_bad.reason = reason, })); - return; + goto wait_dhcp_commit_done; } _emit_notify( self, &((NMDhcpClientNotifyData){.notify_type = NM_DHCP_CLIENT_NOTIFY_TYPE_LEASE_UPDATE, .lease_update = { - .l3cd = priv->l3cd, + .l3cd = priv->l3cd_curr, .accepted = TRUE, }})); - break; - }; - default: - /* ignore */; + } +wait_dhcp_commit_done: + + if (notify_data->notify_type == NM_L3_CONFIG_NOTIFY_TYPE_ACD_EVENT + && priv->v4.acd.l3cfg_commit_handle) { + nm_assert(priv->v4.acd.addr != INADDR_ANY); + nm_assert(priv->v4.acd.state == NM_OPTION_BOOL_DEFAULT); + nm_assert(!priv->v4.acd.done_source); + + if (priv->v4.acd.addr == notify_data->acd_event.info.addr + && nm_l3_acd_addr_info_find_track_info(¬ify_data->acd_event.info, + L3CD_ACD_TAG(priv), + NULL, + NULL)) { + NMOptionBool acd_state; + + switch (notify_data->acd_event.info.state) { + default: + nm_assert_not_reached(); + /* fall-through */ + case NM_L3_ACD_ADDR_STATE_INIT: + case NM_L3_ACD_ADDR_STATE_PROBING: + acd_state = NM_OPTION_BOOL_DEFAULT; + break; + case NM_L3_ACD_ADDR_STATE_USED: + case NM_L3_ACD_ADDR_STATE_CONFLICT: + case NM_L3_ACD_ADDR_STATE_EXTERNAL_REMOVED: + acd_state = NM_OPTION_BOOL_FALSE; + break; + case NM_L3_ACD_ADDR_STATE_READY: + case NM_L3_ACD_ADDR_STATE_DEFENDING: + acd_state = NM_OPTION_BOOL_TRUE; + break; + } + if (acd_state != NM_OPTION_BOOL_DEFAULT) { + _LOGD("acd: acd %s for %s", + acd_state ? "ready" : "conflict", + _nm_utils_inet4_ntop(priv->v4.acd.addr, sbuf_addr)); + nm_l3cfg_commit_type_clear(priv->config.l3cfg, &priv->v4.acd.l3cfg_commit_handle); + priv->v4.acd.state = acd_state; + priv->v4.acd.done_source = nm_g_idle_add_source(_acd_complete_on_idle_cb, self); + } + } } } @@ -696,8 +1171,8 @@ nm_dhcp_client_start(NMDhcpClient *self, GError **error) if (!addr) { _LOGD("waiting for IPv6LL address"); priv->l3cfg_notify.wait_ll_address = TRUE; - connect_l3cfg_notify(self); - priv->ipv6_lladdr_timeout_source = + l3_cfg_notify_check_connected(self); + priv->v6.lladdr_timeout_source = nm_g_timeout_add_seconds_source(10, ipv6_lladdr_timeout, self); return TRUE; } @@ -787,9 +1262,18 @@ nm_dhcp_client_stop(NMDhcpClient *self, gboolean release) priv->is_stopped = TRUE; + if (NM_IS_IPv4(priv->config.addr_family) && priv->v4.bound.invocation) { + g_dbus_method_invocation_return_error(g_steal_pointer(&priv->v4.bound.invocation), + NM_DEVICE_ERROR, + NM_DEVICE_ERROR_FAILED, + "dhcp stopping"); + } + + _acd_state_reset(self, TRUE, TRUE); + priv->l3cfg_notify.wait_dhcp_commit = FALSE; priv->l3cfg_notify.wait_ll_address = FALSE; - connect_l3cfg_notify(self); + l3_cfg_notify_check_connected(self); /* Kill the DHCP client */ old_pid = priv->pid; @@ -800,6 +1284,9 @@ nm_dhcp_client_stop(NMDhcpClient *self, gboolean release) _LOGI("canceled DHCP transaction"); nm_assert(priv->pid == -1); + nm_clear_l3cd(&priv->l3cd_next); + nm_clear_l3cd(&priv->l3cd_curr); + _nm_dhcp_client_notify(self, NM_DHCP_CLIENT_EVENT_TYPE_TERMINATED, NULL); } @@ -920,12 +1407,13 @@ nm_dhcp_client_emit_ipv6_prefix_delegated(NMDhcpClient *self, const NMPlatformIP } gboolean -nm_dhcp_client_handle_event(gpointer unused, - const char *iface, - int pid, - GVariant *options, - const char *reason, - NMDhcpClient *self) +nm_dhcp_client_handle_event(gpointer unused, + const char *iface, + int pid, + GVariant *options, + const char *reason, + GDBusMethodInvocation *invocation, + NMDhcpClient *self) { NMDhcpClientPrivate *priv; nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL; @@ -933,15 +1421,19 @@ nm_dhcp_client_handle_event(gpointer unused, NMPlatformIP6Address prefix = { 0, }; + int IS_IPv4; g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE); g_return_val_if_fail(iface != NULL, FALSE); g_return_val_if_fail(pid > 0, FALSE); g_return_val_if_fail(g_variant_is_of_type(options, G_VARIANT_TYPE_VARDICT), FALSE); g_return_val_if_fail(reason != NULL, FALSE); + g_return_val_if_fail(G_IS_DBUS_METHOD_INVOCATION(invocation), FALSE); priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + g_return_val_if_fail(!priv->is_stopped, FALSE); + if (!nm_streq0(priv->config.iface, iface)) return FALSE; if (priv->pid != pid) @@ -950,7 +1442,7 @@ nm_dhcp_client_handle_event(gpointer unused, _LOGD("DHCP event (reason: '%s')", reason); if (NM_IN_STRSET_ASCII_CASE(reason, "preinit")) - return TRUE; + goto out_handled; if (NM_IN_STRSET_ASCII_CASE(reason, "bound", "bound6", "static")) client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_BOUND; @@ -1014,7 +1506,7 @@ nm_dhcp_client_handle_event(gpointer unused, * of the DHCP client instance. Instead, we just signal the prefix * to the device. */ nm_dhcp_client_emit_ipv6_prefix_delegated(self, &prefix); - return TRUE; + goto out_handled; } if (NM_IN_SET(client_event_type, @@ -1026,7 +1518,22 @@ nm_dhcp_client_handle_event(gpointer unused, client_event_type = NM_DHCP_CLIENT_EVENT_TYPE_FAIL; } + IS_IPv4 = NM_IS_IPv4(priv->config.addr_family); + + if (IS_IPv4 && priv->v4.bound.invocation) + g_dbus_method_invocation_return_value(g_steal_pointer(&priv->v4.bound.invocation), NULL); + + if (IS_IPv4 + && NM_IN_SET(client_event_type, + NM_DHCP_CLIENT_EVENT_TYPE_BOUND, + NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED)) + priv->v4.bound.invocation = g_steal_pointer(&invocation); + _nm_dhcp_client_notify(self, client_event_type, l3cd); + +out_handled: + if (invocation) + g_dbus_method_invocation_return_value(invocation, NULL); return TRUE; } @@ -1070,6 +1577,7 @@ config_init(NMDhcpClientConfig *config, const NMDhcpClientConfig *src) nm_assert(config); nm_assert(src); nm_assert(config != src); + nm_assert_addr_family(src->addr_family); *config = *src; @@ -1165,6 +1673,28 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps case PROP_CONFIG: /* construct-only */ config_init(&priv->config, g_value_get_pointer(value)); + + /* I know, this is technically not necessary. It just feels nicer to + * explicitly initialize the respective union member. */ + if (NM_IS_IPv4(priv->config.addr_family)) { + priv->v4 = (typeof(priv->v4)){ + .bound = + { + .invocation = NULL, + }, + .acd = + { + .addr = INADDR_ANY, + .state = NM_OPTION_BOOL_DEFAULT, + .l3cfg_commit_handle = NULL, + .done_source = NULL, + }, + }; + } else { + priv->v6 = (typeof(priv->v6)){ + .lladdr_timeout_source = NULL, + }; + } break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); @@ -1196,11 +1726,15 @@ dispose(GObject *object) watch_cleanup(self); nm_clear_g_source_inst(&priv->no_lease_timeout_source); - nm_clear_g_source_inst(&priv->ipv6_lladdr_timeout_source); + + if (!NM_IS_IPv4(priv->config.addr_family)) + nm_clear_g_source_inst(&priv->v6.lladdr_timeout_source); + nm_clear_pointer(&priv->effective_client_id, g_bytes_unref); nm_assert(!priv->watch_source); - nm_assert(!priv->l3cd); + nm_assert(!priv->l3cd_next); + nm_assert(!priv->l3cd_curr); nm_assert(priv->l3cfg_notify.id == 0); G_OBJECT_CLASS(nm_dhcp_client_parent_class)->dispose(object); @@ -1227,6 +1761,8 @@ nm_dhcp_client_class_init(NMDhcpClientClass *client_class) object_class->dispose = dispose; object_class->finalize = finalize; object_class->set_property = set_property; + client_class->accept = _accept; + client_class->decline = decline; client_class->stop = stop; client_class->get_duid = get_duid; diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h index 274a954b6cefbb89b0d223ca75f20d04938e58a2..72444c3fc0edd682dc62c4e44536974da929d163 100644 --- a/src/core/dhcp/nm-dhcp-client.h +++ b/src/core/dhcp/nm-dhcp-client.h @@ -84,15 +84,6 @@ typedef struct { const char *nm_dhcp_client_event_type_to_string(NMDhcpClientEventType client_event_type); -/* FIXME(l3cfg:dhcp:config): nm_dhcp_manager_start_ip[46]() has a gazillion of parameters, - * those get passed on as CONSTRUCT_ONLY properties to the NMDhcpClient. Drop - * all these parameters, and let the caller provide one NMDhcpClientConfig - * instance. There will be only one GObject property (NM_DHCP_CLIENT_CONFIG), - * which is CONSTRUCT_ONLY and takes a (mandatory) G_TYPE_POINTER for the - * configuration. - * - * Since NMDhcpClientConfig has an addr_family, we also don't need separate - * nm_dhcp_manager_start_ip[46]() methods. */ typedef struct { int addr_family; @@ -156,12 +147,17 @@ typedef struct { union { struct { + /* The address from the previous lease */ + const char *last_address; + + /* Whether to do ACD for the DHCPv4 address. With timeout zero, ACD + * is disabled. */ + guint acd_timeout_msec; + /* Set BOOTP broadcast flag in request packets, so that servers * will always broadcast replies. */ bool request_broadcast : 1; - /* The address from the previous lease */ - const char *last_address; } v4; struct { /* If set, the DUID from the connection is used; otherwise @@ -208,9 +204,12 @@ typedef struct { gboolean (*ip4_start)(NMDhcpClient *self, GError **error); - gboolean (*accept)(NMDhcpClient *self, GError **error); + gboolean (*accept)(NMDhcpClient *self, const NML3ConfigData *l3cd, GError **error); - gboolean (*decline)(NMDhcpClient *self, const char *error_message, GError **error); + gboolean (*decline)(NMDhcpClient *self, + const NML3ConfigData *l3cd, + const char *error_message, + GError **error); gboolean (*ip6_start)(NMDhcpClient *self, const struct in6_addr *ll_addr, GError **error); @@ -249,11 +248,6 @@ nm_dhcp_client_get_lease(NMDhcpClient *self) return NULL; } -gboolean nm_dhcp_client_accept(NMDhcpClient *self, GError **error); -gboolean nm_dhcp_client_can_accept(NMDhcpClient *self); - -gboolean nm_dhcp_client_decline(NMDhcpClient *self, const char *error_message, GError **error); - void nm_dhcp_client_stop(NMDhcpClient *self, gboolean release); /* Backend helpers for subclasses */ @@ -271,12 +265,15 @@ 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, - int pid, - GVariant *options, - const char *reason, - NMDhcpClient *self); +gboolean _nm_dhcp_client_accept_offer(NMDhcpClient *self, gconstpointer p_yiaddr); + +gboolean nm_dhcp_client_handle_event(gpointer unused, + const char *iface, + int pid, + GVariant *options, + const char *reason, + GDBusMethodInvocation *invocation, + NMDhcpClient *self); void nm_dhcp_client_emit_ipv6_prefix_delegated(NMDhcpClient *self, const NMPlatformIP6Address *prefix); diff --git a/src/core/dhcp/nm-dhcp-helper.c b/src/core/dhcp/nm-dhcp-helper.c index 41862f2b688eaa6a9aac3fc3702f1f95a1ada576..5a17f4e8abb2092c6cd81cbcf6e2a514dd5159ed 100644 --- a/src/core/dhcp/nm-dhcp-helper.c +++ b/src/core/dhcp/nm-dhcp-helper.c @@ -100,32 +100,20 @@ next:; return g_variant_ref_sink(g_variant_new("(a{sv})", &builder)); } -static void -kill_pid(void) -{ - const char *pid_str; - pid_t pid = 0; - - pid_str = getenv("pid"); - if (pid_str) - pid = strtol(pid_str, NULL, 10); - if (pid) { - _LOGI("a fatal error occurred, kill dhclient instance with pid %d", pid); - kill(pid, SIGTERM); - } -} - int main(int argc, char *argv[]) { - gs_unref_object GDBusConnection *connection = NULL; - gs_free_error GError *error = NULL; - gs_unref_variant GVariant *parameters = NULL; - gs_unref_variant GVariant *result = NULL; - gboolean success = FALSE; + gs_unref_object GDBusConnection *connection = NULL; + gs_free_error GError *error = NULL; + gs_free_error GError *error_flush = NULL; + gs_unref_variant GVariant *parameters = NULL; + gs_unref_variant GVariant *result = NULL; + gs_free char *s_err = NULL; + gboolean success; guint try_count; gint64 time_start; gint64 time_end; + gint64 remaining_time; /* Connecting to the unix socket can fail with EAGAIN if there are too * many pending connections and the server can't accept them in time @@ -136,6 +124,8 @@ main(int argc, char *argv[]) time_end = time_start + (5000 * 1000L); try_count = 0; + _LOGi("nm-dhcp-helper: event called"); + do_connect: try_count++; connection = @@ -146,16 +136,16 @@ do_connect: &error); if (!connection) { if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) { - gint64 time_remaining = time_end - g_get_monotonic_time(); - gint64 interval; + remaining_time = time_end - g_get_monotonic_time(); + if (remaining_time > 0) { + gint64 interval; - if (time_remaining > 0) { _LOGi("failure to connect: %s (retry %u, waited %lld ms)", error->message, try_count, - (long long) (time_end - time_remaining - time_start) / 1000); + (long long) (time_end - remaining_time - time_start) / 1000); interval = NM_CLAMP((gint64) (100L * (1L << NM_MIN(try_count, 31))), 5000, 100000); - g_usleep(NM_MIN(interval, time_remaining)); + g_usleep(NM_MIN(interval, remaining_time)); g_clear_error(&error); goto do_connect; } @@ -163,6 +153,7 @@ do_connect: g_dbus_error_strip_remote_error(error); _LOGE("could not connect to NetworkManager D-Bus socket: %s", error->message); + success = FALSE; goto out; } @@ -180,63 +171,78 @@ do_notify: parameters, NULL, G_DBUS_CALL_FLAGS_NONE, - 1000, + 60000, NULL, &error); - if (!result) { - gs_free char *s_err = NULL; + if (result) { + success = TRUE; + goto out; + } - s_err = g_dbus_error_get_remote_error(error); - if (NM_IN_STRSET(s_err, "org.freedesktop.DBus.Error.UnknownMethod")) { - gint64 remaining_time = time_end - g_get_monotonic_time(); - gint64 interval; + s_err = g_dbus_error_get_remote_error(error); - /* I am not sure that a race can actually happen, as we register the object - * on the server side during GDBusServer:new-connection signal. - * - * However, there was also a race for subscribing to an event, so let's just - * do some retry. */ - if (remaining_time > 0) { - _LOGi("failure to call notify: %s (retry %u)", error->message, try_count); - interval = NM_CLAMP((gint64) (100L * (1L << NM_MIN(try_count, 31))), 5000, 25000); - g_usleep(NM_MIN(interval, remaining_time)); - g_clear_error(&error); - goto do_notify; - } - } + if (NM_IN_STRSET(s_err, "org.freedesktop.NetworkManager.Device.Failed")) { + _LOGi("notify failed with reason: %s", error->message); + success = FALSE; + goto out; + } + + if (!NM_IN_STRSET(s_err, "org.freedesktop.DBus.Error.UnknownMethod")) { + /* Some unexpected error. We treat that as a failure. In particular, + * the daemon will fail the request if ACD fails. This causes nm-dhcp-helper + * to fail, which in turn causes dhclient to send a DECLINE. */ _LOGW("failure to call notify: %s (try signal via Event)", error->message); + success = FALSE; + goto out; + } + + /* I am not sure that a race can actually happen, as we register the object + * on the server side during GDBusServer:new-connection signal. + * + * However, there was also a race for subscribing to an event, so let's just + * do some retry. */ + remaining_time = time_end - g_get_monotonic_time(); + if (remaining_time > 0) { + gint64 interval; + + _LOGi("failure to call notify: %s (retry %u)", error->message, try_count); + interval = NM_CLAMP((gint64) (100L * (1L << NM_MIN(try_count, 31))), 5000, 25000); + g_usleep(NM_MIN(interval, remaining_time)); g_clear_error(&error); + goto do_notify; + } - /* for backward compatibility, try to emit the signal. There is no stable - * API between the dhcp-helper and NetworkManager. However, while upgrading - * the NetworkManager package, a newer helper might want to notify an - * older server, which still uses the "Event". */ - if (!g_dbus_connection_emit_signal(connection, - NULL, - "/", - NM_DHCP_CLIENT_DBUS_IFACE, - "Event", - parameters, - &error)) { - g_dbus_error_strip_remote_error(error); - _LOGE("could not send DHCP Event signal: %s", error->message); - goto out; - } + /* for backward compatibility, try to emit the signal. There is no stable + * API between the dhcp-helper and NetworkManager. However, while upgrading + * the NetworkManager package, a newer helper might want to notify an + * older server, which still uses the "Event". */ + + _LOGW("failure to call notify: %s (try signal via Event)", error->message); + g_clear_error(&error); + + if (g_dbus_connection_emit_signal(connection, + NULL, + "/", + NM_DHCP_CLIENT_DBUS_IFACE, + "Event", + parameters, + &error)) { /* We were able to send the asynchronous Event. Consider that a success. */ success = TRUE; - } else - success = TRUE; - - if (!g_dbus_connection_flush_sync(connection, NULL, &error)) { - g_dbus_error_strip_remote_error(error); - _LOGE("could not flush D-Bus connection: %s", error->message); - success = FALSE; goto out; } + g_dbus_error_strip_remote_error(error); + _LOGE("could not send DHCP Event signal: %s", error->message); + success = FALSE; + out: - if (!success) - kill_pid(); + if (!g_dbus_connection_flush_sync(connection, NULL, &error_flush)) { + _LOGE("could not flush D-Bus connection: %s", error_flush->message); + /* if we considered this a success so far, don't fail because of this. */ + } + + _LOGi("success: %s", success ? "YES" : "NO"); return success ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/src/core/dhcp/nm-dhcp-listener.c b/src/core/dhcp/nm-dhcp-listener.c index 2c567593b5f414f57ae4c74c8d55b22fd0e391b3..0854c1dcbe5012e1dc04f10d973fed546d7561ab 100644 --- a/src/core/dhcp/nm-dhcp-listener.c +++ b/src/core/dhcp/nm-dhcp-listener.c @@ -128,7 +128,7 @@ get_option(GVariant *options, const char *key) } static void -_method_call_handle(NMDhcpListener *self, GVariant *parameters) +_method_call_handle(NMDhcpListener *self, GDBusMethodInvocation *invocation, GVariant *parameters) { gs_free char *iface = NULL; gs_free char *pid_str = NULL; @@ -142,23 +142,23 @@ _method_call_handle(NMDhcpListener *self, GVariant *parameters) iface = get_option(options, "interface"); if (iface == NULL) { _LOGW("dhcp-event: didn't have associated interface."); - return; + goto out; } pid_str = get_option(options, "pid"); pid = _nm_utils_ascii_str_to_int64(pid_str, 10, 0, G_MAXINT32, -1); if (pid == -1) { _LOGW("dhcp-event: couldn't convert PID '%s' to an integer", pid_str ?: "(null)"); - return; + goto out; } reason = get_option(options, "reason"); if (reason == NULL) { _LOGW("dhcp-event: (pid %d) DHCP event didn't have a reason", pid); - return; + goto out; } - g_signal_emit(self, signals[EVENT], 0, iface, pid, options, reason, &handled); + g_signal_emit(self, signals[EVENT], 0, iface, pid, options, reason, invocation, &handled); if (!handled) { if (g_ascii_strcasecmp(reason, "RELEASE") == 0) { /* Ignore event when the dhcp client gets killed and we receive its last message */ @@ -166,6 +166,10 @@ _method_call_handle(NMDhcpListener *self, GVariant *parameters) } else _LOGW("dhcp-event: (pid %d) unhandled DHCP event for interface %s", pid, iface); } + +out: + if (!handled) + g_dbus_method_invocation_return_value(invocation, NULL); } static void @@ -190,8 +194,7 @@ _method_call(GDBusConnection *connection, return; } - _method_call_handle(self, parameters); - g_dbus_method_invocation_return_value(invocation, NULL); + _method_call_handle(self, invocation, parameters); } static GDBusInterfaceInfo *const interface_info = NM_DEFINE_GDBUS_INTERFACE_INFO( @@ -311,9 +314,10 @@ nm_dhcp_listener_class_init(NMDhcpListenerClass *listener_class) NULL, NULL, G_TYPE_BOOLEAN, /* listeners return TRUE if handled */ - 4, + 5, G_TYPE_STRING, /* iface */ G_TYPE_INT, /* pid */ G_TYPE_VARIANT, /* options */ - G_TYPE_STRING); /* reason */ + G_TYPE_STRING, /* reason */ + G_TYPE_DBUS_METHOD_INVOCATION /* invocation*/); } diff --git a/src/core/dhcp/nm-dhcp-manager.c b/src/core/dhcp/nm-dhcp-manager.c index 9fea166614c06fec6c7c5669c53b19cc9cdbdb9d..cfff23f8d38f333bfa6a78688cdf6d71ed88758f 100644 --- a/src/core/dhcp/nm-dhcp-manager.c +++ b/src/core/dhcp/nm-dhcp-manager.c @@ -42,6 +42,30 @@ G_DEFINE_TYPE(NMDhcpManager, nm_dhcp_manager, G_TYPE_OBJECT) /*****************************************************************************/ +#undef _NMLOG_ENABLED +#define _NMLOG_ENABLED(level, addr_family) nm_logging_enabled((level), _LOGD_DHCP(addr_family)) + +#define _NMLOG(level, addr_family, ...) \ + G_STMT_START \ + { \ + const int _addr_family = (addr_family); \ + const NMLogLevel _log_level = (level); \ + const NMLogDomain _log_domain = LOGD_DHCP_af(_addr_family); \ + \ + if (nm_logging_enabled(_log_level, _log_domain)) { \ + _nm_log(_log_level, \ + _log_domain, \ + 0, \ + NULL, \ + NULL, \ + "dhcp%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ + nm_utils_addr_family_to_str(_addr_family) _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ + } \ + } \ + G_STMT_END + +/*****************************************************************************/ + /* default to installed helper, but can be modified for testing */ const char *nm_dhcp_helper_path = LIBEXECDIR "/nm-dhcp-helper"; @@ -167,11 +191,10 @@ nm_dhcp_manager_start_client(NMDhcpManager *self, NMDhcpClientConfig *config, GE gtype = _client_factory_get_gtype(priv->client_factory, config->addr_family); - nm_log_trace(LOGD_DHCP, - "dhcp%c: creating IPv%c DHCP client of type %s", - nm_utils_addr_family_to_char(config->addr_family), - nm_utils_addr_family_to_char(config->addr_family), - g_type_name(gtype)); + _LOGT(config->addr_family, + "creating IPv%c DHCP client of type %s", + nm_utils_addr_family_to_char(config->addr_family), + g_type_name(gtype)); client = g_object_new(gtype, NM_DHCP_CLIENT_CONFIG, config, NULL); @@ -244,11 +267,11 @@ nm_dhcp_manager_init(NMDhcpManager *self) if (!f) continue; - nm_log_dbg(LOGD_DHCP, - "dhcp-init: enabled DHCP client '%s'%s%s", - f->name, - _client_factory_available(f) ? "" : " (not available)", - f->undocumented ? " (undocumented internal plugin)" : ""); + _LOGD(AF_UNSPEC, + "init: enabled DHCP client '%s'%s%s", + f->name, + _client_factory_available(f) ? "" : " (not available)", + f->undocumented ? " (undocumented internal plugin)" : ""); } /* Client-specific setup */ @@ -261,20 +284,20 @@ nm_dhcp_manager_init(NMDhcpManager *self) if (client) { client_factory = _client_factory_available(_client_factory_find_by_name(client)); if (!client_factory) - nm_log_warn(LOGD_DHCP, "dhcp-init: DHCP client '%s' not available", client); + _LOGW(AF_UNSPEC, "init: DHCP client '%s' not available", client); } if (!client_factory) { client_factory = _client_factory_find_by_name("" NM_CONFIG_DEFAULT_MAIN_DHCP); if (!client_factory) - nm_log_err(LOGD_DHCP, - "dhcp-init: default DHCP client '%s' is not installed", - NM_CONFIG_DEFAULT_MAIN_DHCP); + _LOGE(AF_UNSPEC, + "init: default DHCP client '%s' is not installed", + NM_CONFIG_DEFAULT_MAIN_DHCP); else { client_factory = _client_factory_available(client_factory); if (!client_factory) - nm_log_info(LOGD_DHCP, - "dhcp-init: default DHCP client '%s' is not available", - NM_CONFIG_DEFAULT_MAIN_DHCP); + _LOGI(AF_UNSPEC, + "init: default DHCP client '%s' is not available", + NM_CONFIG_DEFAULT_MAIN_DHCP); } } if (!client_factory) { @@ -287,7 +310,7 @@ nm_dhcp_manager_init(NMDhcpManager *self) g_return_if_fail(client_factory); - nm_log_info(LOGD_DHCP, "dhcp-init: Using DHCP client '%s'", client_factory->name); + _LOGI(AF_UNSPEC, "init: Using DHCP client '%s'", client_factory->name); /* NOTE: currently the DHCP plugin is chosen once at start. It's not * possible to reload that configuration. If that ever becomes possible, diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index b86b889d4797a06d85e167c5272d59e328fb56e3..a986dffe9298f25826a502eb623ac39f1d29305a 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -52,9 +52,16 @@ typedef struct _NMDhcpNettoolsClass NMDhcpNettoolsClass; typedef struct { NDhcp4Client *client; NDhcp4ClientProbe *probe; - NDhcp4ClientLease *lease; - GSource *event_source; - char *lease_file; + + struct { + NDhcp4ClientLease *lease; + const NML3ConfigData *lease_l3cd; + } granted; + + GSource *pop_all_events_on_idle_source; + + GSource *event_source; + char *lease_file; } NMDhcpNettoolsPrivate; struct _NMDhcpNettools { @@ -73,6 +80,10 @@ G_DEFINE_TYPE(NMDhcpNettools, nm_dhcp_nettools, NM_TYPE_DHCP_CLIENT) /*****************************************************************************/ +static void dhcp4_event_pop_all_events_on_idle(NMDhcpNettools *self); + +/*****************************************************************************/ + static void set_error_nettools(GError **error, int r, const char *message) { @@ -864,15 +875,19 @@ lease_save(NMDhcpNettools *self, NDhcp4ClientLease *lease, const char *lease_fil } static void -bound4_handle(NMDhcpNettools *self, NDhcp4ClientLease *lease, gboolean extended) +bound4_handle(NMDhcpNettools *self, guint event, NDhcp4ClientLease *lease) { NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); NMDhcpClient *client = NM_DHCP_CLIENT(self); const NMDhcpClientConfig *client_config; nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL; - GError *error = NULL; + gs_free_error GError *error = NULL; + + nm_assert(NM_IN_SET(event, N_DHCP4_CLIENT_EVENT_GRANTED, N_DHCP4_CLIENT_EVENT_EXTENDED)); + nm_assert(lease); + + _LOGT("lease available (%s)", (event == N_DHCP4_CLIENT_EVENT_GRANTED) ? "granted" : "extended"); - _LOGT("lease available (%s)", extended ? "extended" : "new"); client_config = nm_dhcp_client_get_config(client); l3cd = lease_to_ip4_config(nm_dhcp_client_get_multi_idx(client), client_config->iface, @@ -881,36 +896,65 @@ bound4_handle(NMDhcpNettools *self, NDhcp4ClientLease *lease, gboolean extended) &error); if (!l3cd) { _LOGW("failure to parse lease: %s", error->message); - g_clear_error(&error); + + if (event == N_DHCP4_CLIENT_EVENT_GRANTED) { + n_dhcp4_client_lease_decline(lease, "invalid lease"); + dhcp4_event_pop_all_events_on_idle(self); + } + _nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL); return; } - lease_save(self, lease, priv->lease_file); + if (event == N_DHCP4_CLIENT_EVENT_GRANTED) { + priv->granted.lease = n_dhcp4_client_lease_ref(lease); + priv->granted.lease_l3cd = nm_l3_config_data_ref(l3cd); + } else + lease_save(self, lease, priv->lease_file); _nm_dhcp_client_notify(NM_DHCP_CLIENT(self), - extended ? NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED - : NM_DHCP_CLIENT_EVENT_TYPE_BOUND, + event == N_DHCP4_CLIENT_EVENT_GRANTED + ? NM_DHCP_CLIENT_EVENT_TYPE_BOUND + : NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED, l3cd); } static void dhcp4_event_handle(NMDhcpNettools *self, NDhcp4ClientEvent *event) { - NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); - const NMDhcpClientConfig *client_config; - struct in_addr server_id; - char addr_str[INET_ADDRSTRLEN]; - int r; + NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); + struct in_addr server_id; + struct in_addr yiaddr; + char addr_str[INET_ADDRSTRLEN]; + char addr_str2[INET_ADDRSTRLEN]; + int r; - _LOGT("client event %d", event->event); - client_config = nm_dhcp_client_get_config(NM_DHCP_CLIENT(self)); + if (event->event == N_DHCP4_CLIENT_EVENT_LOG) { + _NMLOG(nm_log_level_from_syslog(event->log.level), "event: %s", event->log.message); + return; + } + + if (!NM_IN_SET(event->event, N_DHCP4_CLIENT_EVENT_LOG)) { + /* In almost all events (even those that we don't expect below), we clear + * the currently granted lease. That is, because in GRANTED state we + * expect to follow up with accept/decline, and that only works while + * we are still in the same state. Transitioning away to another state + * (on most events) will invalidate that. */ + nm_clear_pointer(&priv->granted.lease, n_dhcp4_client_lease_unref); + nm_clear_l3cd(&priv->granted.lease_l3cd); + } switch (event->event) { case N_DHCP4_CLIENT_EVENT_OFFER: r = n_dhcp4_client_lease_get_server_identifier(event->offer.lease, &server_id); if (r) { - _LOGW("selecting lease failed: %d", r); + _LOGW("selecting lease failed: could not get DHCP server identifier (%d)", r); + return; + } + + n_dhcp4_client_lease_get_yiaddr(event->offer.lease, &yiaddr); + if (yiaddr.s_addr == INADDR_ANY) { + _LOGD("selecting lease failed: no yiaddr address"); return; } @@ -920,47 +964,102 @@ dhcp4_event_handle(NMDhcpNettools *self, NDhcp4ClientEvent *event) return; } + if (!_nm_dhcp_client_accept_offer(NM_DHCP_CLIENT(self), &yiaddr.s_addr)) { + /* We don't log about this, the parent class is expected to notify about the reasons. */ + return; + } + + _LOGT("selecting offered lease from %s for %s", + _nm_utils_inet4_ntop(server_id.s_addr, addr_str), + _nm_utils_inet4_ntop(yiaddr.s_addr, addr_str2)); + r = n_dhcp4_client_lease_select(event->offer.lease); + + dhcp4_event_pop_all_events_on_idle(self); + if (r) { _LOGW("selecting lease failed: %d", r); return; } - break; + + return; case N_DHCP4_CLIENT_EVENT_RETRACTED: case N_DHCP4_CLIENT_EVENT_EXPIRED: _nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_EXPIRE, NULL); - break; + return; case N_DHCP4_CLIENT_EVENT_CANCELLED: _nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL); - break; + return; case N_DHCP4_CLIENT_EVENT_GRANTED: - priv->lease = n_dhcp4_client_lease_ref(event->granted.lease); - bound4_handle(self, event->granted.lease, FALSE); - break; + bound4_handle(self, event->event, event->granted.lease); + return; case N_DHCP4_CLIENT_EVENT_EXTENDED: - bound4_handle(self, event->extended.lease, TRUE); - break; + bound4_handle(self, event->event, event->extended.lease); + return; case N_DHCP4_CLIENT_EVENT_DOWN: /* ignore down events, they are purely informational */ - break; - case N_DHCP4_CLIENT_EVENT_LOG: - { - NMLogLevel nm_level; - - nm_level = nm_log_level_from_syslog(event->log.level); - if (nm_logging_enabled(nm_level, LOGD_DHCP4)) { - nm_log(nm_level, - LOGD_DHCP4, - NULL, - NULL, - "dhcp4 (%s): %s", - client_config->iface, - event->log.message); - } - } break; + _LOGT("event: down (ignore)"); + return; default: - _LOGW("unhandled DHCP event %d", event->event); - break; + _LOGE("unhandled DHCP event %d", event->event); + nm_assert(event->event != N_DHCP4_CLIENT_EVENT_LOG); + nm_assert_not_reached(); + return; + } +} + +static void +dhcp4_event_pop_all_events(NMDhcpNettools *self) +{ + NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); + NDhcp4ClientEvent *event; + + while (!n_dhcp4_client_pop_event(priv->client, &event) && event) + dhcp4_event_handle(self, event); + + nm_clear_g_source_inst(&priv->pop_all_events_on_idle_source); +} + +static gboolean +dhcp4_event_pop_all_events_on_idle_cb(gpointer user_data) +{ + NMDhcpNettools *self = user_data; + NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); + + nm_clear_g_source_inst(&priv->pop_all_events_on_idle_source); + dhcp4_event_pop_all_events(self); + return G_SOURCE_CONTINUE; +} + +static void +dhcp4_event_pop_all_events_on_idle(NMDhcpNettools *self) +{ + NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); + + /* For the most part, NDhcp4Client gets driven from internal, that is + * by having events ready on the socket or the timerfd. For those + * events, we will poll on the (epoll) FD, then let it be processed + * by n_dhcp4_client_dispatch(), and pop the queued events. + * + * But certain commands (n_dhcp4_client_lease_select(), n_dhcp4_client_lease_accept(), + * n_dhcp4_client_lease_decline()) are initiated by the user. And they tend + * to log events. Logging is done by queuing a message, but that won't be processed, + * unless we pop the event. + * + * To ensure that those logging events get popped, schedule an idle handler to do that. + * + * Yes, this means, that the messages only get logged later, when the idle handler + * runs. The alternative seems even more problematic, because we don't know + * the current call-state, and it seems dangerous to pop unexpected events. + * E.g. we call n_dhcp4_client_lease_select() from inside the event-handler, + * it seems wrong to call dhcp4_event_pop_all_events() in that context again. + * + * See-also: https://github.com/nettools/n-dhcp4/issues/34 + */ + + if (!priv->pop_all_events_on_idle_source) { + priv->pop_all_events_on_idle_source = + nm_g_idle_add_source(dhcp4_event_pop_all_events_on_idle_cb, self); } } @@ -969,7 +1068,6 @@ dhcp4_event_cb(int fd, GIOCondition condition, gpointer user_data) { NMDhcpNettools *self = user_data; NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); - NDhcp4ClientEvent *event; int r; r = n_dhcp4_client_dispatch(priv->client); @@ -990,8 +1088,7 @@ dhcp4_event_cb(int fd, GIOCondition condition, gpointer user_data) return G_SOURCE_REMOVE; } - while (!n_dhcp4_client_pop_event(priv->client, &event) && event) - dhcp4_event_handle(self, event); + dhcp4_event_pop_all_events(self); return G_SOURCE_CONTINUE; } @@ -1098,46 +1195,69 @@ nettools_create(NMDhcpNettools *self, GError **error) } static gboolean -_accept(NMDhcpClient *client, GError **error) +_accept(NMDhcpClient *client, const NML3ConfigData *l3cd, GError **error) { NMDhcpNettools *self = NM_DHCP_NETTOOLS(client); NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); int r; - g_return_val_if_fail(priv->lease, FALSE); - _LOGT("accept"); - r = n_dhcp4_client_lease_accept(priv->lease); + 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; + } + + nm_assert(priv->granted.lease); + + r = n_dhcp4_client_lease_accept(priv->granted.lease); + + dhcp4_event_pop_all_events_on_idle(self); + + nm_clear_pointer(&priv->granted.lease, n_dhcp4_client_lease_unref); + nm_clear_l3cd(&priv->granted.lease_l3cd); + if (r) { set_error_nettools(error, r, "failed to accept lease"); return FALSE; } - priv->lease = n_dhcp4_client_lease_unref(priv->lease); - return TRUE; } static gboolean -decline(NMDhcpClient *client, const char *error_message, GError **error) +decline(NMDhcpClient *client, const NML3ConfigData *l3cd, const char *error_message, GError **error) { NMDhcpNettools *self = NM_DHCP_NETTOOLS(client); NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); int r; + nm_auto(n_dhcp4_client_lease_unrefp) NDhcp4ClientLease *lease = NULL; + + _LOGT("decline (%s)", error_message); + + g_return_val_if_fail(l3cd, FALSE); - g_return_val_if_fail(priv->lease, FALSE); + if (priv->granted.lease_l3cd != l3cd) { + nm_utils_error_set(error, NM_UTILS_ERROR_UNKNOWN, "calling decline in unexpected state"); + return FALSE; + } + + nm_assert(priv->granted.lease); - _LOGT("dhcp4-client: decline (%s)", error_message); + lease = g_steal_pointer(&priv->granted.lease); + nm_clear_l3cd(&priv->granted.lease_l3cd); + + r = n_dhcp4_client_lease_decline(lease, error_message); + + dhcp4_event_pop_all_events_on_idle(self); - r = n_dhcp4_client_lease_decline(priv->lease, error_message); if (r) { set_error_nettools(error, r, "failed to decline lease"); return FALSE; } - priv->lease = n_dhcp4_client_lease_unref(priv->lease); - return TRUE; } @@ -1348,7 +1468,9 @@ dispose(GObject *object) nm_clear_g_free(&priv->lease_file); nm_clear_g_source_inst(&priv->event_source); - nm_clear_pointer(&priv->lease, n_dhcp4_client_lease_unref); + nm_clear_g_source_inst(&priv->pop_all_events_on_idle_source); + nm_clear_pointer(&priv->granted.lease, n_dhcp4_client_lease_unref); + nm_clear_l3cd(&priv->granted.lease_l3cd); nm_clear_pointer(&priv->probe, n_dhcp4_client_probe_free); nm_clear_pointer(&priv->client, n_dhcp4_client_unref); diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index 79310dc5cc94c44fe20fc722d23d35d1d4b1d657..71813594a0f1e2d5279f6ce3c7ba6a6c75c90d2a 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -416,6 +416,8 @@ nm_dhcp_utils_ip4_config_from_options(NMDedupMultiIndex *multi_idx, str = g_hash_table_lookup(options, "ip_address"); if (!str || !nm_utils_parse_inaddr_bin(AF_INET, str, NULL, &addr)) return NULL; + if (addr == INADDR_ANY) + return NULL; _LOG2I(LOGD_DHCP4, iface, " address %s", str); @@ -428,6 +430,7 @@ nm_dhcp_utils_ip4_config_from_options(NMDedupMultiIndex *multi_idx, plen = _nm_utils_ip4_get_default_prefix(addr); _LOG2I(LOGD_DHCP4, iface, " plen %d (default)", plen); } + nm_platform_ip4_address_set_addr(&address, addr, plen); /* Routes: if the server returns classless static routes, we MUST ignore diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 38b9d8224ebca117a9bafd4115c2058f3e1849eb..dff69592159beac411ba0ca3e74f437df5d3b3ce 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -37,7 +37,6 @@ G_STATIC_ASSERT(NM_ACD_TIMEOUT_RFC5227_MSEC == N_ACD_TIMEOUT_RFC5227); #define ACD_ENSURE_RATELIMIT_MSEC ((guint32) 4000u) #define ACD_WAIT_PROBING_EXTRA_TIME_MSEC ((guint32) (1000u + ACD_ENSURE_RATELIMIT_MSEC)) #define ACD_WAIT_PROBING_EXTRA_TIME2_MSEC ((guint32) 1000u) -#define ACD_MAX_TIMEOUT_MSEC ((guint32) 30000u) #define ACD_WAIT_TIME_PROBING_FULL_RESTART_MSEC ((guint32) 30000u) #define ACD_WAIT_TIME_CONFLICT_RESTART_MSEC ((guint32) 120000u) #define ACD_WAIT_TIME_ANNOUNCE_RESTART_MSEC ((guint32) 30000u) @@ -1866,10 +1865,10 @@ _l3_acd_data_add(NML3Cfg *self, acd_data = _l3_acd_data_find(self, addr); - if (acd_timeout_msec > ACD_MAX_TIMEOUT_MSEC) { + if (acd_timeout_msec > NM_ACD_TIMEOUT_MAX_MSEC) { /* we limit the maximum timeout. Otherwise we have to handle integer overflow * when adding timeouts. */ - acd_timeout_msec = ACD_MAX_TIMEOUT_MSEC; + acd_timeout_msec = NM_ACD_TIMEOUT_MAX_MSEC; } if (!acd_data) { @@ -3229,8 +3228,8 @@ nm_l3cfg_add_config(NML3Cfg *self, nm_assert(l3cd); nm_assert(nm_l3_config_data_get_ifindex(l3cd) == self->priv.ifindex); - if (acd_timeout_msec > ACD_MAX_TIMEOUT_MSEC) - acd_timeout_msec = ACD_MAX_TIMEOUT_MSEC; + if (acd_timeout_msec > NM_ACD_TIMEOUT_MAX_MSEC) + acd_timeout_msec = NM_ACD_TIMEOUT_MAX_MSEC; nm_assert(NM_IN_SET(acd_defend_type, NM_L3_ACD_DEFEND_TYPE_NEVER, diff --git a/src/core/nm-l3cfg.h b/src/core/nm-l3cfg.h index f6ec39ced8f70db9ebcb2e7f4b4f20ad6fb3d94d..63ccd5a141918b9b6dd2c4dce4ee61989af88547 100644 --- a/src/core/nm-l3cfg.h +++ b/src/core/nm-l3cfg.h @@ -10,6 +10,7 @@ #define NM_L3CFG_CONFIG_PRIORITY_IPV6LL 1 #define NM_L3CFG_CONFIG_PRIORITY_VPN 9 #define NM_ACD_TIMEOUT_RFC5227_MSEC 9000u +#define NM_ACD_TIMEOUT_MAX_MSEC 30000u #define NM_TYPE_L3CFG (nm_l3cfg_get_type()) #define NM_L3CFG(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), NM_TYPE_L3CFG, NML3Cfg)) diff --git a/src/libnm-glib-aux/nm-logging-fwd.h b/src/libnm-glib-aux/nm-logging-fwd.h index 0e715c5047e344a1f4e06d52b8dd3ac95171a76f..72e5723c288f614a94d8185363f7137d92e0894e 100644 --- a/src/libnm-glib-aux/nm-logging-fwd.h +++ b/src/libnm-glib-aux/nm-logging-fwd.h @@ -59,7 +59,16 @@ typedef enum { LOGD_IP = LOGD_IP4 | LOGD_IP6, #define LOGD_DHCPX(is_ipv4) ((is_ipv4) ? LOGD_DHCP4 : LOGD_DHCP6) -#define LOGD_IPX(is_ipv4) ((is_ipv4) ? LOGD_IP4 : LOGD_IP6) + +#define LOGD_DHCP_af(addr_family) \ + ({ \ + const int _addr_family_1 = (addr_family); \ + \ + (_addr_family_1 == AF_UNSPEC ? LOGD_DHCP \ + : (NM_IS_IPv4(_addr_family_1) ? LOGD_DHCP4 : LOGD_DHCP6)); \ + }) + +#define LOGD_IPX(is_ipv4) ((is_ipv4) ? LOGD_IP4 : LOGD_IP6) } NMLogDomain; diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 5552bf0567f7c8e1eaf7a08733be541db7f715de..fddcc8fe26f44fc77adf287b687824af85b3798c 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -2194,38 +2194,57 @@ nm_g_array_unref(GArray *arr) g_array_unref(arr); } -#define nm_g_array_first(arr, type) \ - ({ \ - GArray *const _arr = (arr); \ - guint _len; \ - \ - nm_assert(_arr); \ - _len = _arr->len; \ - nm_assert(_len > 0); \ - &g_array_index(arr, type, 0); \ +#define nm_g_array_first(arr, Type) \ + ({ \ + GArray *const _arr = (arr); \ + \ + nm_assert(_arr); \ + nm_assert(sizeof(Type) == g_array_get_element_size(_arr)); \ + nm_assert(_arr->len > 0); \ + \ + &g_array_index(arr, Type, 0); \ }) -#define nm_g_array_last(arr, type) \ - ({ \ - GArray *const _arr = (arr); \ - guint _len; \ - \ - nm_assert(_arr); \ - _len = _arr->len; \ - nm_assert(_len > 0); \ - &g_array_index(arr, type, _len - 1u); \ +#define nm_g_array_last(arr, Type) \ + ({ \ + GArray *const _arr = (arr); \ + \ + nm_assert(_arr); \ + nm_assert(sizeof(Type) == g_array_get_element_size(_arr)); \ + nm_assert(_arr->len > 0); \ + \ + &g_array_index(arr, Type, _arr->len - 1u); \ }) -#define nm_g_array_append_new(arr, type) \ - ({ \ - GArray *const _arr = (arr); \ - guint _len; \ - \ - nm_assert(_arr); \ - _len = _arr->len; \ - nm_assert(_len < G_MAXUINT); \ - g_array_set_size(_arr, _len + 1u); \ - &g_array_index(arr, type, _len); \ +/* Similar to g_array_index(). The differences are + * - this does nm_assert() checks that the arguments are valid. + * - returns a pointer to the element. */ +#define nm_g_array_index_p(arr, Type, idx) \ + ({ \ + GArray *const _arr = (arr); \ + const guint _idx = (idx); \ + \ + nm_assert(_arr); \ + nm_assert(sizeof(Type) == g_array_get_element_size(_arr)); \ + nm_assert(_idx < _arr->len); \ + \ + &g_array_index(_arr, Type, _idx); \ + }) + +#define nm_g_array_append_new(arr, Type) \ + ({ \ + GArray *const _arr = (arr); \ + guint _len; \ + \ + nm_assert(_arr); \ + nm_assert(sizeof(Type) == g_array_get_element_size(_arr)); \ + \ + _len = _arr->len; \ + \ + nm_assert(_len < G_MAXUINT); \ + \ + g_array_set_size(_arr, _len + 1u); \ + &g_array_index(arr, Type, _len); \ }) /*****************************************************************************/ @@ -3102,10 +3121,14 @@ gboolean nm_utils_ifname_valid(const char *name, NMUtilsIfaceType type, GError * static inline GArray * nm_strvarray_ensure(GArray **p) { + nm_assert(p); + if (!*p) { *p = g_array_new(TRUE, FALSE, sizeof(char *)); g_array_set_clear_func(*p, nm_indirect_g_free); - } + } else + nm_assert(g_array_get_element_size(*p) == sizeof(char *)); + return *p; } @@ -3114,6 +3137,9 @@ nm_strvarray_add(GArray *array, const char *str) { char *s; + nm_assert(array); + nm_assert(g_array_get_element_size(array) == sizeof(char *)); + s = g_strdup(str); g_array_append_val(array, s); } @@ -3121,15 +3147,14 @@ nm_strvarray_add(GArray *array, const char *str) static inline const char * nm_strvarray_get_idx(GArray *array, guint idx) { - nm_assert(array); - nm_assert(idx < array->len); - - return g_array_index(array, const char *, idx); + return *nm_g_array_index_p(array, const char *, idx); } static inline const char *const * nm_strvarray_get_strv_non_empty(GArray *arr, guint *length) { + nm_assert(!arr || g_array_get_element_size(arr) == sizeof(char *)); + if (!arr || arr->len == 0) { NM_SET_OUT(length, 0); return NULL; @@ -3144,6 +3169,8 @@ nm_strvarray_get_strv_non_empty_dup(GArray *arr, guint *length) { const char *const *strv; + nm_assert(!arr || g_array_get_element_size(arr) == sizeof(char *)); + if (!arr || arr->len == 0) { NM_SET_OUT(length, 0); return NULL; @@ -3162,6 +3189,8 @@ nm_strvarray_get_strv(GArray **arr, guint *length) return (const char *const *) arr; } + nm_assert(g_array_get_element_size(*arr) == sizeof(char *)); + NM_SET_OUT(length, (*arr)->len); return &g_array_index(*arr, const char *, 0); } @@ -3173,6 +3202,8 @@ nm_strvarray_set_strv(GArray **array, const char *const *strv) array_old = g_steal_pointer(array); + nm_assert(!array_old || g_array_get_element_size(array_old) == sizeof(char *)); + if (!strv || !strv[0]) return; @@ -3189,6 +3220,7 @@ nm_strvarray_find_first(GArray *strv, const char *needle) nm_assert(needle); if (strv) { + nm_assert(g_array_get_element_size(strv) == sizeof(char *)); for (i = 0; i < strv->len; i++) { if (nm_streq(needle, g_array_index(strv, const char *, i))) return i; diff --git a/src/n-dhcp4/src/n-dhcp4-c-lease.c b/src/n-dhcp4/src/n-dhcp4-c-lease.c index eaa962765223baa739bffaa8d95bb7f0d331e65f..be0d78869b1f909a2b8ff33e920664eda7bc8552 100644 --- a/src/n-dhcp4/src/n-dhcp4-c-lease.c +++ b/src/n-dhcp4/src/n-dhcp4-c-lease.c @@ -351,32 +351,15 @@ _c_public_ int n_dhcp4_client_lease_query(NDhcp4ClientLease *lease, uint8_t opti * selected none of the others can be. * * Return: 0 on success, or a negative error code on failure. + * Returns -ENOTRECOVERABLE when called in an unexpected state. */ _c_public_ int n_dhcp4_client_lease_select(NDhcp4ClientLease *lease) { - NDhcp4ClientLease *l, *t_l; - NDhcp4ClientProbe *probe; - int r; - - /* XXX error handling, this must be an OFFER */ - if (!lease->probe) return -ENOTRECOVERABLE; if (lease->probe->current_lease) return -ENOTRECOVERABLE; - r = n_dhcp4_client_probe_transition_select(lease->probe, lease->message, n_dhcp4_gettime(CLOCK_BOOTTIME)); - if (r) - return r; - - /* - * Only one of the offered leases can be selected, so flush the list. - * All offered lease, including this one are now dead. - */ - probe = lease->probe; - c_list_for_each_entry_safe(l, t_l, &probe->lease_list, probe_link) - n_dhcp4_client_lease_unlink(l); - - return 0; + return n_dhcp4_client_probe_transition_select(lease->probe, lease->message, n_dhcp4_gettime(CLOCK_BOOTTIME)); } /** @@ -390,24 +373,15 @@ _c_public_ int n_dhcp4_client_lease_select(NDhcp4ClientLease *lease) { * can be accepted. * * Return: 0 on success, or a negative error code on failure. + * Returns -ENOTRECOVERABLE when called in an unexpected state. */ _c_public_ int n_dhcp4_client_lease_accept(NDhcp4ClientLease *lease) { - int r; - - /* XXX error handling, this must be an ACK */ - if (!lease->probe) return -ENOTRECOVERABLE; if (lease->probe->current_lease != lease) return -ENOTRECOVERABLE; - r = n_dhcp4_client_probe_transition_accept(lease->probe, lease->message); - if (r) - return r; - - n_dhcp4_client_lease_unlink(lease); - - return 0; + return n_dhcp4_client_probe_transition_accept(lease->probe, lease->message); } /** @@ -421,23 +395,13 @@ _c_public_ int n_dhcp4_client_lease_accept(NDhcp4ClientLease *lease) { * decline. * * Return: 0 on success, or a negative error code on failure. + * Returns -ENOTRECOVERABLE when called in an unexpected state. */ _c_public_ int n_dhcp4_client_lease_decline(NDhcp4ClientLease *lease, const char *error) { - int r; - - /* XXX: error handling, this must be an ACK */ - if (!lease->probe) return -ENOTRECOVERABLE; if (lease->probe->current_lease != lease) return -ENOTRECOVERABLE; - r = n_dhcp4_client_probe_transition_decline(lease->probe, lease->message, error, n_dhcp4_gettime(CLOCK_BOOTTIME)); - if (r) - return r; - - lease->probe->current_lease = n_dhcp4_client_lease_unref(lease->probe->current_lease); - n_dhcp4_client_lease_unlink(lease); - - return 0; + return n_dhcp4_client_probe_transition_decline(lease->probe, lease->message, error, n_dhcp4_gettime(CLOCK_BOOTTIME)); } diff --git a/src/n-dhcp4/src/n-dhcp4-c-probe.c b/src/n-dhcp4/src/n-dhcp4-c-probe.c index 283c1693cf81503274e09141792dbd1c836171bb..284aa428e939e8587df3f4685514b537afa6c182 100644 --- a/src/n-dhcp4/src/n-dhcp4-c-probe.c +++ b/src/n-dhcp4/src/n-dhcp4-c-probe.c @@ -1024,6 +1024,7 @@ static int n_dhcp4_client_probe_transition_nak(NDhcp4ClientProbe *probe) { int n_dhcp4_client_probe_transition_select(NDhcp4ClientProbe *probe, NDhcp4Incoming *offer, uint64_t ns_now) { _c_cleanup_(n_dhcp4_outgoing_freep) NDhcp4Outgoing *request = NULL; + NDhcp4ClientLease *l, *t_l; int r; switch (probe->state) { @@ -1042,11 +1043,16 @@ int n_dhcp4_client_probe_transition_select(NDhcp4ClientProbe *probe, NDhcp4Incom else request = NULL; /* consumed */ - /* XXX: ignore other offers */ - probe->state = N_DHCP4_CLIENT_PROBE_STATE_REQUESTING; - break; + /* + * Only one of the offered leases can be selected, so flush the list. + * All offered lease, including this one are now dead. + */ + c_list_for_each_entry_safe(l, t_l, &probe->lease_list, probe_link) + n_dhcp4_client_lease_unlink(l); + + return 0; case N_DHCP4_CLIENT_PROBE_STATE_INIT: case N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT: case N_DHCP4_CLIENT_PROBE_STATE_REBOOTING: @@ -1057,11 +1063,9 @@ int n_dhcp4_client_probe_transition_select(NDhcp4ClientProbe *probe, NDhcp4Incom case N_DHCP4_CLIENT_PROBE_STATE_REBINDING: case N_DHCP4_CLIENT_PROBE_STATE_EXPIRED: default: - /* ignore */ - break; + /* user called in invalid state. Return error. */ + return -ENOTRECOVERABLE; } - - return 0; } /** @@ -1085,10 +1089,11 @@ int n_dhcp4_client_probe_transition_accept(NDhcp4ClientProbe *probe, NDhcp4Incom return r; probe->state = N_DHCP4_CLIENT_PROBE_STATE_BOUND; - + probe->ns_decline_restart_delay = 0; + n_dhcp4_client_lease_unlink(probe->current_lease); n_dhcp4_client_arm_timer(probe->client); - break; + return 0; case N_DHCP4_CLIENT_PROBE_STATE_INIT: case N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT: @@ -1100,11 +1105,9 @@ int n_dhcp4_client_probe_transition_accept(NDhcp4ClientProbe *probe, NDhcp4Incom case N_DHCP4_CLIENT_PROBE_STATE_REBINDING: case N_DHCP4_CLIENT_PROBE_STATE_EXPIRED: default: - /* ignore */ - break; + /* user called in invalid state. Return error. */ + return -ENOTRECOVERABLE; } - - return 0; } /** @@ -1126,9 +1129,22 @@ int n_dhcp4_client_probe_transition_decline(NDhcp4ClientProbe *probe, NDhcp4Inco else request = NULL; /* consumed */ - /* XXX: what state to transition to? */ + n_dhcp4_client_lease_unlink(probe->current_lease); + probe->current_lease = n_dhcp4_client_lease_unref(probe->current_lease); + + probe->state = N_DHCP4_CLIENT_PROBE_STATE_INIT; - break; + /* RFC2131, 3.1, 5.) The client SHOULD wait a minimum of ten seconds before restarting + * the configuration process to avoid excessive network traffic in case of looping. + * + * Let's go beyond that, and use an exponential backoff. */ + probe->ns_decline_restart_delay = C_CLAMP(probe->ns_decline_restart_delay * 2u, + UINT64_C(10) * UINT64_C(1000000000), + UINT64_C(300) * UINT64_C(1000000000)); + probe->ns_deferred = n_dhcp4_gettime(CLOCK_BOOTTIME) + probe->ns_decline_restart_delay; + + n_dhcp4_client_arm_timer(probe->client); + return 0; case N_DHCP4_CLIENT_PROBE_STATE_INIT: case N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT: @@ -1140,11 +1156,9 @@ int n_dhcp4_client_probe_transition_decline(NDhcp4ClientProbe *probe, NDhcp4Inco case N_DHCP4_CLIENT_PROBE_STATE_REBINDING: case N_DHCP4_CLIENT_PROBE_STATE_EXPIRED: default: - /* ignore */ - break; + /* user called in invalid state. Return error. */ + return -ENOTRECOVERABLE; } - - return 0; } /** diff --git a/src/n-dhcp4/src/n-dhcp4-private.h b/src/n-dhcp4/src/n-dhcp4-private.h index 858c3d3ab0978c74548c7391b15b1f96d798857a..6b366884bedef7ac4175f30018ced9c8a903a5fe 100644 --- a/src/n-dhcp4/src/n-dhcp4-private.h +++ b/src/n-dhcp4/src/n-dhcp4-private.h @@ -378,6 +378,7 @@ struct NDhcp4ClientProbe { uint64_t ns_deferred; /* timeout for deferred action */ uint64_t ns_reinit; uint64_t ns_nak_restart_delay; /* restart delay after a nak */ + uint64_t ns_decline_restart_delay; /* restart delay after a decline */ NDhcp4ClientLease *current_lease; /* current lease */ NDhcp4CConnection connection; /* client connection wrapper */