wifi: fix another activation failure when changing MAC address (rh#1371478, bgo#770456, bgo#770504)
This commit is contained in:
Thomas Haller
parent
2c68942743
commit
76dec7d585
279
0003-wifi-set-mac-addr-workaround-rh1371478-v2.patch
Normal file
279
0003-wifi-set-mac-addr-workaround-rh1371478-v2.patch
Normal file
@ -0,0 +1,279 @@
|
|||||||
|
From 3c701b60dc169a24cf52c0397560125ddce3d54c Mon Sep 17 00:00:00 2001
|
||||||
|
From: Thomas Haller <thaller@redhat.com>
|
||||||
|
Date: Wed, 7 Sep 2016 23:47:14 +0200
|
||||||
|
Subject: [PATCH 1/2] device: workaround driver issue with delayed change of
|
||||||
|
MAC address
|
||||||
|
|
||||||
|
brcmfmac and possibly other drivers don't change the MAC address
|
||||||
|
right away, but instead the result is delayed. That is problematic
|
||||||
|
because we cannot continue activation before the MAC address is
|
||||||
|
settled.
|
||||||
|
|
||||||
|
Add a hack to workaround the issue by waiting until the MAC address
|
||||||
|
changed.
|
||||||
|
|
||||||
|
The previous attempt to workaround this was less intrusive: we would
|
||||||
|
just refresh the link once and check the result. But that turns out
|
||||||
|
not to be sufficent for all cases. Now, wait and poll.
|
||||||
|
|
||||||
|
https://bugzilla.gnome.org/show_bug.cgi?id=770456
|
||||||
|
https://bugzilla.redhat.com/show_bug.cgi?id=1374023
|
||||||
|
(cherry picked from commit 1a85103765d4eaa0acab6b03658a4f9cfe684a64)
|
||||||
|
(cherry picked from commit 8d575403685208aad75f918484ae7adbc1a46085)
|
||||||
|
---
|
||||||
|
src/devices/nm-device.c | 85 ++++++++++++++++++++++++++++++++++---------------
|
||||||
|
src/devices/nm-device.h | 2 +-
|
||||||
|
2 files changed, 61 insertions(+), 26 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
|
||||||
|
index 674563f..3e549c5 100644
|
||||||
|
--- a/src/devices/nm-device.c
|
||||||
|
+++ b/src/devices/nm-device.c
|
||||||
|
@@ -11549,16 +11549,17 @@ nm_device_get_hw_address (NMDevice *self)
|
||||||
|
return priv->hw_addr;
|
||||||
|
}
|
||||||
|
|
||||||
|
-void
|
||||||
|
+gboolean
|
||||||
|
nm_device_update_hw_address (NMDevice *self)
|
||||||
|
{
|
||||||
|
NMDevicePrivate *priv;
|
||||||
|
const guint8 *hwaddr;
|
||||||
|
gsize hwaddrlen = 0;
|
||||||
|
+ gboolean changed = FALSE;
|
||||||
|
|
||||||
|
priv = NM_DEVICE_GET_PRIVATE (self);
|
||||||
|
if (priv->ifindex <= 0)
|
||||||
|
- return;
|
||||||
|
+ return FALSE;
|
||||||
|
|
||||||
|
hwaddr = nm_platform_link_get_address (NM_PLATFORM_GET, priv->ifindex, &hwaddrlen);
|
||||||
|
|
||||||
|
@@ -11585,6 +11586,7 @@ nm_device_update_hw_address (NMDevice *self)
|
||||||
|
* update our inital hw-address as well. */
|
||||||
|
nm_device_update_initial_hw_address (self);
|
||||||
|
}
|
||||||
|
+ changed = TRUE;
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
/* Invalid or no hardware address */
|
||||||
|
@@ -11597,6 +11599,7 @@ nm_device_update_hw_address (NMDevice *self)
|
||||||
|
"hw-addr: failed reading current MAC address");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
+ return changed;
|
||||||
|
}
|
||||||
|
|
||||||
|
void
|
||||||
|
@@ -11756,6 +11759,15 @@ nm_device_hw_addr_is_explict (NMDevice *self)
|
||||||
|
}
|
||||||
|
|
||||||
|
static gboolean
|
||||||
|
+_hw_addr_matches (NMDevice *self, const char *addr)
|
||||||
|
+{
|
||||||
|
+ const char *cur_addr;
|
||||||
|
+
|
||||||
|
+ cur_addr = nm_device_get_hw_address (self);
|
||||||
|
+ return cur_addr && nm_utils_hwaddr_matches (cur_addr, -1, addr, -1);
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+static gboolean
|
||||||
|
_hw_addr_set (NMDevice *self,
|
||||||
|
const char *addr,
|
||||||
|
const char *operation,
|
||||||
|
@@ -11765,7 +11777,6 @@ _hw_addr_set (NMDevice *self,
|
||||||
|
gboolean success = FALSE;
|
||||||
|
gboolean needs_refresh = FALSE;
|
||||||
|
NMPlatformError plerr;
|
||||||
|
- const char *cur_addr;
|
||||||
|
guint8 addr_bytes[NM_UTILS_HWADDR_LEN_MAX];
|
||||||
|
guint hw_addr_len;
|
||||||
|
gboolean was_up;
|
||||||
|
@@ -11776,11 +11787,9 @@ _hw_addr_set (NMDevice *self,
|
||||||
|
|
||||||
|
priv = NM_DEVICE_GET_PRIVATE (self);
|
||||||
|
|
||||||
|
- cur_addr = nm_device_get_hw_address (self);
|
||||||
|
-
|
||||||
|
/* Do nothing if current MAC is same */
|
||||||
|
- if (cur_addr && nm_utils_hwaddr_matches (cur_addr, -1, addr, -1)) {
|
||||||
|
- _LOGT (LOGD_DEVICE, "set-hw-addr: no MAC address change needed (%s)", cur_addr);
|
||||||
|
+ if (_hw_addr_matches (self, addr)) {
|
||||||
|
+ _LOGT (LOGD_DEVICE, "set-hw-addr: no MAC address change needed (%s)", addr);
|
||||||
|
return TRUE;
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -11804,8 +11813,7 @@ _hw_addr_set (NMDevice *self,
|
||||||
|
if (success) {
|
||||||
|
/* MAC address succesfully changed; update the current MAC to match */
|
||||||
|
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)) {
|
||||||
|
+ if (_hw_addr_matches (self, addr)) {
|
||||||
|
_LOGI (LOGD_DEVICE, "set-hw-addr: %s MAC address to %s (%s)",
|
||||||
|
operation, addr, detail);
|
||||||
|
} else {
|
||||||
|
@@ -11827,24 +11835,51 @@ _hw_addr_set (NMDevice *self,
|
||||||
|
}
|
||||||
|
|
||||||
|
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);
|
||||||
|
+ if (_hw_addr_matches (self, addr)) {
|
||||||
|
+ /* the MAC address already changed during nm_device_bring_up() above. */
|
||||||
|
} else {
|
||||||
|
- _LOGW (LOGD_DEVICE,
|
||||||
|
- "set-hw-addr: new MAC address %s not successfully %s (%s)",
|
||||||
|
- addr, operation, detail);
|
||||||
|
- return FALSE;
|
||||||
|
+ gint64 poll_end, now;
|
||||||
|
+
|
||||||
|
+ /* The platform call indicated success, however the address is not
|
||||||
|
+ * as expected. That is either due to a driver issue (brcmfmac, bgo#770456,
|
||||||
|
+ * rh#1374023) or a race where externally the MAC address was reset.
|
||||||
|
+ * The race is rather unlikely.
|
||||||
|
+ *
|
||||||
|
+ * The alternative would be to postpone the activation in case the
|
||||||
|
+ * MAC address is not yet ready and poll without blocking. However,
|
||||||
|
+ * that is rather complicated and it is not expected that this case
|
||||||
|
+ * happens for regular drivers.
|
||||||
|
+ * Note that brcmfmac can block NetworkManager for 500 msec while
|
||||||
|
+ * taking down the device. Let's add annother 100 msec to that.
|
||||||
|
+ *
|
||||||
|
+ * wait/poll up to 100 msec until it changes. */
|
||||||
|
+
|
||||||
|
+ poll_end = nm_utils_get_monotonic_timestamp_us () + (100 * 1000);
|
||||||
|
+ for (;;) {
|
||||||
|
+ if (!nm_platform_link_refresh (NM_PLATFORM_GET, nm_device_get_ip_ifindex (self)))
|
||||||
|
+ goto handle_fail;
|
||||||
|
+ if (!nm_device_update_hw_address (self))
|
||||||
|
+ goto handle_wait;
|
||||||
|
+ if (!_hw_addr_matches (self, addr))
|
||||||
|
+ goto handle_fail;
|
||||||
|
+
|
||||||
|
+ break;
|
||||||
|
+handle_wait:
|
||||||
|
+ now = nm_utils_get_monotonic_timestamp_us ();
|
||||||
|
+ if (now < poll_end) {
|
||||||
|
+ g_usleep (NM_MIN (poll_end - now, 500));
|
||||||
|
+ continue;
|
||||||
|
+ }
|
||||||
|
+handle_fail:
|
||||||
|
+ _LOGW (LOGD_DEVICE,
|
||||||
|
+ "set-hw-addr: new MAC address %s not successfully %s (%s)",
|
||||||
|
+ addr, operation, detail);
|
||||||
|
+ return FALSE;
|
||||||
|
+ }
|
||||||
|
}
|
||||||
|
+
|
||||||
|
+ _LOGI (LOGD_DEVICE, "set-hw-addr: %s MAC address to %s (%s)",
|
||||||
|
+ operation, addr, detail);
|
||||||
|
}
|
||||||
|
|
||||||
|
return success;
|
||||||
|
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
|
||||||
|
index be12ce7..a757a37 100644
|
||||||
|
--- a/src/devices/nm-device.h
|
||||||
|
+++ b/src/devices/nm-device.h
|
||||||
|
@@ -588,7 +588,7 @@ void nm_device_reactivate_ip6_config (NMDevice *device,
|
||||||
|
NMSettingIPConfig *s_ip6_old,
|
||||||
|
NMSettingIPConfig *s_ip6_new);
|
||||||
|
|
||||||
|
-void nm_device_update_hw_address (NMDevice *self);
|
||||||
|
+gboolean nm_device_update_hw_address (NMDevice *self);
|
||||||
|
void nm_device_update_initial_hw_address (NMDevice *self);
|
||||||
|
void nm_device_update_permanent_hw_address (NMDevice *self);
|
||||||
|
void nm_device_update_dynamic_ip_setup (NMDevice *self);
|
||||||
|
--
|
||||||
|
2.7.4
|
||||||
|
|
||||||
|
|
||||||
|
From d99c3b63e81347fb861e1306eb0b603cfa15223e Mon Sep 17 00:00:00 2001
|
||||||
|
From: Thomas Haller <thaller@redhat.com>
|
||||||
|
Date: Sun, 11 Sep 2016 09:48:56 +0200
|
||||||
|
Subject: [PATCH 2/2] device: wait for MAC address change to complete before
|
||||||
|
setting interface up
|
||||||
|
|
||||||
|
Some drivers (brcmfmac) don't change the MAC address right away.
|
||||||
|
NetworkManager works around that by waiting synchronously until
|
||||||
|
the address changes (commit 1a85103765d4eaa0acab6b03658a4f9cfe684a64).
|
||||||
|
|
||||||
|
wpa_supplicant on the other hand, only re-reads the MAC address
|
||||||
|
when changing state from DISABLED to ENABLED, which happens when
|
||||||
|
the interface comes up.
|
||||||
|
|
||||||
|
That is a bug in wpa_supplicant and the driver, but we can work-around by
|
||||||
|
waiting until the MAC address actually changed before setting the interface
|
||||||
|
IFF_UP. Also note, that there is still a race in wpa_supplicant which might
|
||||||
|
miss a change to DISABLED state altogether.
|
||||||
|
|
||||||
|
https://bugzilla.gnome.org/show_bug.cgi?id=770504
|
||||||
|
https://bugzilla.redhat.com/show_bug.cgi?id=1374023
|
||||||
|
(cherry picked from commit 32f7c1d4b9aba597a99128631f07c2985149f303)
|
||||||
|
(cherry picked from commit cd8f2ecc617a896d8007e6fe825c676a626a3b8d)
|
||||||
|
---
|
||||||
|
src/devices/nm-device.c | 27 ++++++++++++++++-----------
|
||||||
|
1 file changed, 16 insertions(+), 11 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
|
||||||
|
index 3e549c5..47e858b 100644
|
||||||
|
--- a/src/devices/nm-device.c
|
||||||
|
+++ b/src/devices/nm-device.c
|
||||||
|
@@ -11829,12 +11829,8 @@ _hw_addr_set (NMDevice *self,
|
||||||
|
nm_platform_error_to_string (plerr));
|
||||||
|
}
|
||||||
|
|
||||||
|
- if (was_up) {
|
||||||
|
- if (!nm_device_bring_up (self, TRUE, NULL))
|
||||||
|
- return FALSE;
|
||||||
|
- }
|
||||||
|
-
|
||||||
|
if (needs_refresh) {
|
||||||
|
+ success = TRUE;
|
||||||
|
if (_hw_addr_matches (self, addr)) {
|
||||||
|
/* the MAC address already changed during nm_device_bring_up() above. */
|
||||||
|
} else {
|
||||||
|
@@ -11871,15 +11867,24 @@ handle_wait:
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
handle_fail:
|
||||||
|
- _LOGW (LOGD_DEVICE,
|
||||||
|
- "set-hw-addr: new MAC address %s not successfully %s (%s)",
|
||||||
|
- addr, operation, detail);
|
||||||
|
- return FALSE;
|
||||||
|
+ success = FALSE;
|
||||||
|
+ break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
- _LOGI (LOGD_DEVICE, "set-hw-addr: %s MAC address to %s (%s)",
|
||||||
|
- operation, addr, detail);
|
||||||
|
+ if (success) {
|
||||||
|
+ _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);
|
||||||
|
+ }
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ if (was_up) {
|
||||||
|
+ if (!nm_device_bring_up (self, TRUE, NULL))
|
||||||
|
+ return FALSE;
|
||||||
|
}
|
||||||
|
|
||||||
|
return success;
|
||||||
|
--
|
||||||
|
2.7.4
|
||||||
|
|
||||||
@ -12,7 +12,7 @@
|
|||||||
|
|
||||||
%global rpm_version 1.4.0
|
%global rpm_version 1.4.0
|
||||||
%global real_version 1.4.0
|
%global real_version 1.4.0
|
||||||
%global release_version 3
|
%global release_version 4
|
||||||
%global epoch_version 1
|
%global epoch_version 1
|
||||||
|
|
||||||
%global obsoletes_nmver 1:0.9.9.95-1
|
%global obsoletes_nmver 1:0.9.9.95-1
|
||||||
@ -101,6 +101,7 @@ Source3: 20-connectivity-fedora.conf
|
|||||||
#Patch1: 0001-some.patch
|
#Patch1: 0001-some.patch
|
||||||
Patch1: 0001-wifi-set-mac-addr-workaround-rh1371478
|
Patch1: 0001-wifi-set-mac-addr-workaround-rh1371478
|
||||||
Patch2: 0002-dhcp-helper-sync-notify-rh1372854.patch
|
Patch2: 0002-dhcp-helper-sync-notify-rh1372854.patch
|
||||||
|
Patch3: 0003-wifi-set-mac-addr-workaround-rh1371478-v2.patch
|
||||||
|
|
||||||
Requires(post): systemd
|
Requires(post): systemd
|
||||||
Requires(preun): systemd
|
Requires(preun): systemd
|
||||||
@ -344,6 +345,7 @@ by nm-connection-editor and nm-applet in a non-graphical environment.
|
|||||||
|
|
||||||
%patch1 -p1
|
%patch1 -p1
|
||||||
%patch2 -p1
|
%patch2 -p1
|
||||||
|
%patch3 -p1
|
||||||
|
|
||||||
%build
|
%build
|
||||||
%if %{with regen_docs}
|
%if %{with regen_docs}
|
||||||
@ -651,6 +653,9 @@ fi
|
|||||||
%endif
|
%endif
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Tue Sep 13 2016 Thomas Haller <thaller@redhat.com> - 1:1.4.0-4
|
||||||
|
- wifi: fix another activation failure when changing MAC address (rh#1371478, bgo#770456, bgo#770504)
|
||||||
|
|
||||||
* Thu Sep 8 2016 Thoams Haller <thaller@redhat.com> - 1:1.4.0-3
|
* Thu Sep 8 2016 Thoams Haller <thaller@redhat.com> - 1:1.4.0-3
|
||||||
- dhcp: fix race to miss DHCP lease event (rh#1372854)
|
- dhcp: fix race to miss DHCP lease event (rh#1372854)
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user