From c01db59a7cc5d13f2305e0d44c97e1c7af028752 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 28 Aug 2016 13:52:32 +0200 Subject: [PATCH 1/6] platform: split processing result from do_change_link() (cherry picked from commit 3dc09446771a3434ed948bdd5e6ca9f6ef9a9e76) (cherry picked from commit 471521ca84187cd32afcd20aebe5a369fe7368dc) --- src/platform/nm-linux-platform.c | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 98c4e46..eeb24ca 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -4060,18 +4060,14 @@ out: return !!nmp_cache_lookup_obj (priv->cache, obj_id); } -static NMPlatformError -do_change_link (NMPlatform *platform, - int ifindex, - struct nl_msg *nlmsg) +static WaitForNlResponseResult +do_change_link_request (NMPlatform *platform, + int ifindex, + struct nl_msg *nlmsg) { nm_auto_pop_netns NMPNetns *netns = NULL; WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN; int nle; - char s_buf[256]; - NMPlatformError result = NM_PLATFORM_ERROR_SUCCESS; - NMLogLevel log_level = LOGL_DEBUG; - const char *log_result = "failure", *log_detail = ""; if (!nm_platform_netns_push (platform, &netns)) return NM_PLATFORM_ERROR_UNSPECIFIED; @@ -4098,6 +4094,18 @@ retry: nlmsg_hdr (nlmsg)->nlmsg_type = RTM_SETLINK; goto retry; } + return seq_result; +} + +static NMPlatformError +do_change_link_result (NMPlatform *platform, + int ifindex, + WaitForNlResponseResult seq_result) +{ + char s_buf[256]; + NMPlatformError result = NM_PLATFORM_ERROR_SUCCESS; + NMLogLevel log_level = LOGL_DEBUG; + const char *log_result = "failure", *log_detail = ""; if (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK) { log_result = "success"; @@ -4123,6 +4131,17 @@ retry: return result; } +static NMPlatformError +do_change_link (NMPlatform *platform, + int ifindex, + struct nl_msg *nlmsg) +{ + WaitForNlResponseResult seq_result; + + seq_result = do_change_link_request (platform, ifindex, nlmsg); + return do_change_link_result (platform, ifindex, seq_result); +} + static gboolean link_add (NMPlatform *platform, const char *name, -- 2.7.4 From b2fcc8f414aefd1e6caf64e45a9ff1ba543f5b09 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 28 Aug 2016 14:08:42 +0200 Subject: [PATCH 2/6] platform: workaround kernel wrongly returning ENFILE when changing MAC address https://bugzilla.gnome.org/show_bug.cgi?id=770456 (cherry picked from commit 2bef71611bd9fd2e333a7522205f0262ac25680f) (cherry picked from commit 06d1679aa9867682297316e7b2cfac6fc8f67c2a) --- src/platform/nm-linux-platform.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index eeb24ca..c36e967 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -4449,6 +4449,8 @@ link_set_address (NMPlatform *platform, int ifindex, gconstpointer address, size { nm_auto_nlmsg struct nl_msg *nlmsg = NULL; gs_free char *mac = NULL; + WaitForNlResponseResult seq_result; + char s_buf[256]; if (!address || !length) g_return_val_if_reached (NM_PLATFORM_ERROR_BUG); @@ -4468,7 +4470,30 @@ link_set_address (NMPlatform *platform, int ifindex, gconstpointer address, size NLA_PUT (nlmsg, IFLA_ADDRESS, length, address); - return do_change_link (platform, ifindex, nlmsg); + seq_result = do_change_link_request (platform, ifindex, nlmsg); + + if (NM_IN_SET (-((int) seq_result), ENFILE)) { + const NMPObject *obj_cache; + + /* workaround ENFILE which may be wrongly returned (bgo #770456). + * If the MAC address is as expected, assume success? */ + + obj_cache = nmp_cache_lookup_link (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, ifindex); + if ( obj_cache + && obj_cache->link.addr.len == length + && memcmp (obj_cache->link.addr.data, address, length) == 0) { + _NMLOG (LOGL_DEBUG, + "do-change-link[%d]: %s changing link: %s%s", + ifindex, + "success", + wait_for_nl_response_to_string (seq_result, s_buf, sizeof (s_buf)), + " (assume success changing address)"); + return NM_PLATFORM_ERROR_SUCCESS; + } + } + + return do_change_link_result (platform, ifindex, seq_result); + nla_put_failure: g_return_val_if_reached (NM_PLATFORM_ERROR_UNSPECIFIED); } -- 2.7.4 From aa0bc143376d7d4a50d45909a9776e161ee60b37 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Aug 2016 17:14:04 +0200 Subject: [PATCH 3/6] device: fix spelling in logging (cherry picked from commit d51f2c2a4e99799739e2adbeaf578144b556c4b9) (cherry picked from commit b1f5d3d798498c53fe65257490b2df3e3f71e364) --- src/devices/nm-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 199acc6..305a1bb 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11820,7 +11820,7 @@ _hw_addr_set (NMDevice *self, operation, addr, detail); } else { _LOGW (LOGD_DEVICE, - "set-hw-addr: new MAC address %s not successfully set to %s (%s)", + "set-hw-addr: new MAC address %s not successfully %s (%s)", addr, operation, detail); success = FALSE; } -- 2.7.4 From 9e3e5bbd0d0f36a117115783457caec359b763a7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Aug 2016 18:28:34 +0200 Subject: [PATCH 4/6] device: add hack to wait after changing MAC address It seems some drivers return success for nm_platform_link_set_address(), but at that point the address did not yet actually change *sigh*. It changes a bit later, possibly after setting the device up. Add a workaround to retry reading the MAC address when platform indicates success but the address still differs at first. https://bugzilla.gnome.org/show_bug.cgi?id=770456 (cherry picked from commit 67b685235847ac49712d77023e23ef5c38e82a9e) (cherry picked from commit 3b51959f48f2b40a4d85e1d36fd69a46548369cb) --- src/devices/nm-device.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 305a1bb..6939332 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11774,6 +11774,7 @@ _hw_addr_set (NMDevice *self, { NMDevicePrivate *priv; gboolean success = FALSE; + gboolean needs_refresh = FALSE; NMPlatformError plerr; const char *cur_addr; guint8 addr_bytes[NM_UTILS_HWADDR_LEN_MAX]; @@ -11819,10 +11820,10 @@ _hw_addr_set (NMDevice *self, _LOGI (LOGD_DEVICE, "set-hw-addr: %s MAC address to %s (%s)", operation, addr, detail); } else { - _LOGW (LOGD_DEVICE, - "set-hw-addr: new MAC address %s not successfully %s (%s)", + _LOGD (LOGD_DEVICE, + "set-hw-addr: new MAC address %s not successfully %s (%s) (refresh link)", addr, operation, detail); - success = FALSE; + needs_refresh = TRUE; } } else { _NMLOG (plerr == NM_PLATFORM_ERROR_NOT_FOUND ? LOGL_DEBUG : LOGL_WARN, @@ -11836,6 +11837,27 @@ _hw_addr_set (NMDevice *self, return FALSE; } + if (needs_refresh) { + /* The platform call indicated success, however the address is not + * as expected. May be a kernel issue and the MAC address takes + * a moment to change (bgo#770456). + * + * Try to reload the link and check again. */ + nm_platform_link_refresh (NM_PLATFORM_GET, nm_device_get_ip_ifindex (self)); + + nm_device_update_hw_address (self); + cur_addr = nm_device_get_hw_address (self); + if (cur_addr && nm_utils_hwaddr_matches (cur_addr, -1, addr, -1)) { + _LOGI (LOGD_DEVICE, "set-hw-addr: %s MAC address to %s (%s)", + operation, addr, detail); + } else { + _LOGW (LOGD_DEVICE, + "set-hw-addr: new MAC address %s not successfully %s (%s)", + addr, operation, detail); + return FALSE; + } + } + return success; } -- 2.7.4 From fb4ca54a6dc66ca14e5c085cdb390959ebbc861e Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 30 Aug 2016 15:05:39 +0200 Subject: [PATCH 5/6] platform: fix build Fixes: 471521ca84187cd32afcd20aebe5a369fe7368dc (cherry picked from commit 02fb3eff481f79d3caa67fdde20b2ae7aa5e640b) (cherry picked from commit 4cf61eeafd62f7c0ac9cd71b8526577d1262e152) --- src/platform/nm-linux-platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index c36e967..e3ab5e6 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -4070,7 +4070,7 @@ do_change_link_request (NMPlatform *platform, int nle; if (!nm_platform_netns_push (platform, &netns)) - return NM_PLATFORM_ERROR_UNSPECIFIED; + return seq_result; retry: nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result, NULL); @@ -4078,7 +4078,7 @@ retry: _LOGE ("do-change-link[%d]: failure sending netlink request \"%s\" (%d)", ifindex, nl_geterror (nle), -nle); - return NM_PLATFORM_ERROR_UNSPECIFIED; + return seq_result; } /* always refetch the link after changing it. There seems to be issues -- 2.7.4 From 395a0fb95d59a384dbe26cb727db5fe4212bc598 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 30 Aug 2016 16:20:40 +0200 Subject: [PATCH 6/6] platform: fix return value for error case in do_change_link_request() There is a "goto retry" in do_change_link_request(), at that point, seq_result has the value -EOPNOTSUPP, instead of WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN. Fixes: 02fb3eff481f79d3caa67fdde20b2ae7aa5e640b (cherry picked from commit 145d199589d34f8844d8e9cafc3c2c2ea8ec4d53) (cherry picked from commit 89d68b1a96b382f692e61b57239baae504e62d04) --- src/platform/nm-linux-platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index e3ab5e6..070a83e 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -4070,7 +4070,7 @@ do_change_link_request (NMPlatform *platform, int nle; if (!nm_platform_netns_push (platform, &netns)) - return seq_result; + return WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN; retry: nle = _nl_send_auto_with_seq (platform, nlmsg, &seq_result, NULL); @@ -4078,7 +4078,7 @@ retry: _LOGE ("do-change-link[%d]: failure sending netlink request \"%s\" (%d)", ifindex, nl_geterror (nle), -nle); - return seq_result; + return WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN; } /* always refetch the link after changing it. There seems to be issues -- 2.7.4