From 1172178ce659f705987c0840ddf142eab550eb5d Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 11 Dec 2014 15:36:27 -0600 Subject: core: better handle DHCP expiry/nak during initial lease acquisition (bgo #739482) When dhclient trieds to request a previous lease and the server NAKs that lease, dhclient emits the EXPIRE state. dhcpcd has also been known to emit the 'nak' state for the same reason. (systemd's DHCP client code does not push a NAK up to NetworkManager, but jumps to the REBOOT state instead, so it is unaffected by this issue.) NetworkManager saw the expire during IP configuration and treated that as full activation failure. The connection would be restarted, the same lease requested, and the same NAK delivered, over and over. Before a lease is acquired, there is (by definition) no lease to expire, so these events should be ignored. We do, however, still want to handle abnormal failures, which is why this patch splits the EXPIRE case from the FAIL case and handles them separately. https://bugzilla.gnome.org/show_bug.cgi?id=739482 --- src/devices/nm-device.c | 12 ++++++++++-- src/dhcp-manager/nm-dhcp-client.c | 12 ++++++++---- src/dhcp-manager/nm-dhcp-client.h | 9 +++++---- src/dhcp-manager/nm-dhcp-systemd.c | 2 ++ 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 060a7aa6f5..cd548d260e 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3006,9 +3006,13 @@ dhcp4_state_changed (NMDhcpClient *client, case NM_DHCP_STATE_TIMEOUT: dhcp4_fail (self, TRUE); break; + case NM_DHCP_STATE_EXPIRE: + /* Ignore expiry before we even have a lease (NAK, old lease, etc) */ + if (priv->ip4_state == IP_CONF) + break; + /* Fall through */ case NM_DHCP_STATE_DONE: case NM_DHCP_STATE_FAIL: - /* dhclient quit and can't get/renew a lease; so kill the connection */ dhcp4_fail (self, FALSE); break; default: @@ -3597,6 +3601,11 @@ dhcp6_state_changed (NMDhcpClient *client, case NM_DHCP_STATE_TIMEOUT: dhcp6_timeout (self, client); break; + case NM_DHCP_STATE_EXPIRE: + /* Ignore expiry before we even have a lease (NAK, old lease, etc) */ + if (priv->ip6_state != IP_CONF) + dhcp6_fail (self, FALSE); + break; case NM_DHCP_STATE_DONE: /* In IPv6 info-only mode, the client doesn't handle leases so it * may exit right after getting a response from the server. That's @@ -3606,7 +3615,6 @@ dhcp6_state_changed (NMDhcpClient *client, break; /* Otherwise, fall through */ case NM_DHCP_STATE_FAIL: - /* dhclient quit and can't get/renew a lease; so kill the connection */ dhcp6_fail (self, FALSE); break; default: diff --git a/src/dhcp-manager/nm-dhcp-client.c b/src/dhcp-manager/nm-dhcp-client.c index 4e56550028..47fa53171f 100644 --- a/src/dhcp-manager/nm-dhcp-client.c +++ b/src/dhcp-manager/nm-dhcp-client.c @@ -183,6 +183,7 @@ static const char *state_table[NM_DHCP_STATE_MAX + 1] = { [NM_DHCP_STATE_UNKNOWN] = "unknown", [NM_DHCP_STATE_BOUND] = "bound", [NM_DHCP_STATE_TIMEOUT] = "timeout", + [NM_DHCP_STATE_EXPIRE] = "expire", [NM_DHCP_STATE_DONE] = "done", [NM_DHCP_STATE_FAIL] = "fail", }; @@ -208,13 +209,14 @@ reason_to_state (const char *iface, const char *reason) return NM_DHCP_STATE_BOUND; else if (g_ascii_strcasecmp (reason, "timeout") == 0) return NM_DHCP_STATE_TIMEOUT; + else if (g_ascii_strcasecmp (reason, "nak") == 0 || + g_ascii_strcasecmp (reason, "expire") == 0 || + g_ascii_strcasecmp (reason, "expire6") == 0) + return NM_DHCP_STATE_EXPIRE; else if (g_ascii_strcasecmp (reason, "end") == 0) return NM_DHCP_STATE_DONE; else if (g_ascii_strcasecmp (reason, "fail") == 0 || - g_ascii_strcasecmp (reason, "abend") == 0 || - g_ascii_strcasecmp (reason, "nak") == 0 || - g_ascii_strcasecmp (reason, "expire") == 0 || - g_ascii_strcasecmp (reason, "expire6") == 0) + g_ascii_strcasecmp (reason, "abend") == 0) return NM_DHCP_STATE_FAIL; nm_log_dbg (LOGD_DHCP, "(%s): unmapped DHCP state '%s'", iface, reason); @@ -744,6 +746,8 @@ nm_dhcp_client_handle_event (gpointer unused, old_state = priv->state; new_state = reason_to_state (priv->iface, reason); + nm_log_dbg (LOGD_DHCP, "(%s): DHCP reason '%s' -> state '%s'", + iface, reason, state_to_string (new_state)); if (new_state == NM_DHCP_STATE_BOUND) { /* Copy options */ diff --git a/src/dhcp-manager/nm-dhcp-client.h b/src/dhcp-manager/nm-dhcp-client.h index 0876061f8a..971c9dc04e 100644 --- a/src/dhcp-manager/nm-dhcp-client.h +++ b/src/dhcp-manager/nm-dhcp-client.h @@ -46,10 +46,11 @@ typedef enum { NM_DHCP_STATE_UNKNOWN = 0, - NM_DHCP_STATE_BOUND, /* lease changed (state_is_bound) */ - NM_DHCP_STATE_TIMEOUT, /* TIMEOUT */ - NM_DHCP_STATE_DONE, /* END */ - NM_DHCP_STATE_FAIL, /* failed or quit unexpectedly */ + NM_DHCP_STATE_BOUND, /* new lease or lease changed */ + NM_DHCP_STATE_TIMEOUT, /* timed out contacting server */ + NM_DHCP_STATE_DONE, /* client quit or stopped */ + NM_DHCP_STATE_EXPIRE, /* lease expired or NAKed */ + NM_DHCP_STATE_FAIL, /* failed for some reason */ __NM_DHCP_STATE_MAX, NM_DHCP_STATE_MAX = __NM_DHCP_STATE_MAX - 1, } NMDhcpState; diff --git a/src/dhcp-manager/nm-dhcp-systemd.c b/src/dhcp-manager/nm-dhcp-systemd.c index 5d36282cff..12dc03cc84 100644 --- a/src/dhcp-manager/nm-dhcp-systemd.c +++ b/src/dhcp-manager/nm-dhcp-systemd.c @@ -485,6 +485,8 @@ dhcp_event_cb (sd_dhcp_client *client, int event, gpointer user_data) switch (event) { case DHCP_EVENT_EXPIRED: + nm_dhcp_client_set_state (NM_DHCP_CLIENT (user_data), NM_DHCP_STATE_EXPIRE, NULL, NULL); + break; case DHCP_EVENT_STOP: nm_dhcp_client_set_state (NM_DHCP_CLIENT (user_data), NM_DHCP_STATE_FAIL, NULL, NULL); break; -- cgit v1.2.1