NetworkManager

Merge lp:~phablet-team/network-manager/wifi-fix-cancel-scan into lp:~phablet-team/network-manager/vivid-phone-overlay

  1. wifi-fix-cancel-scan
  2. Merge into vivid-phone-overlay
Proposed by Tony Espy
Status: Merged
Approved by: Alfonso Sanchez-Beato
Approved revision: 975
Merged at revision: 972
Proposed branch: lp:~phablet-team/network-manager/wifi-fix-cancel-scan
Merge into: lp:~phablet-team/network-manager/vivid-phone-overlay
Diff against target: 246 lines (+154/-13)
5 files modified
debian/changelog (+20/-0)
debian/patches/ignore_rfkill_if_urfkill_is_present.patch (+5/-5)
debian/patches/lp1099983_ignore-p2p-wifi-devices.patch (+9/-8)
debian/patches/series (+1/-0)
debian/patches/wifi-fix-cancel-scan.patch (+119/-0)
To merge this branch: bzr merge lp:~phablet-team/network-manager/wifi-fix-cancel-scan
Reviewer Review Type Date Requested Status
Simon Fels Approve
Alfonso Sanchez-Beato Approve
Mathieu Trudel-Lapierre Pending
Review via email: mp+288440@code.launchpad.net

Description of the change

This patch fixes a race condition in NMDeviceWiFi's scanning logic.
It's possible for cancel_pending_scan() to be called when an actual
supplicant scan is pending, which can cause scanning to stall,
effectively preventing the device from re-connecting to WiFi.

To post a comment you must log in.
lp:~phablet-team/network-manager/wifi-fix-cancel-scan updated
973. By Tony Espy

* d/p/lp1099983_ignore-p2p-wifi-devices.patch: updated to ignore
  all p2p devices ( not just p2p0 ), as some drivers dynamically
  create differently named p2p devices which are managed externally
  in the case of WiFi Direct usage.
* d/p/lp1099983_ignore-p2p-wifi-devices.patch: refreshed

974. By Tony Espy

d/p/wifi-fix-cancel-scan.patch: add logic to supplicant_iface_state_cb
to schedule a scan if supplicant state becomes inactive, and
old_state was scanning.

975. By Tony Espy

d/p/wifi-fix-cancel-scan.patch: cancel_pending_scan, and use
request_wireless_scan instead of sched_scan to re-trigger scanning
logic when supplicant_state becomes inactive.

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

LGTM

review: Approve
Revision history for this message
Simon Fels (morphis) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2015-10-02 14:56:14 +0000
+++ debian/changelog 2016-03-15 23:32:40 +0000
@@ -1,3 +1,23 @@
1network-manager (0.9.10.0-4ubuntu15.1.10) UNRELEASED; urgency=medium
2
3 [ Vicamo Yang ]
4 * d/p/wifi-fix-cancel-scan.patch: This patch fixes a race condition
5 in NMDeviceWiFi's scanning logic which can cause scanning to
6 stall, leading to a situation where the device never re-connects
7 to WiFi.
8
9 [ Tony Espy ]
10 * d/p/lp1099983_ignore-p2p-wifi-devices.patch: updated to ignore
11 all p2p devices ( not just p2p0 ), as some drivers dynamically
12 create differently named p2p devices which are managed externally
13 in the case of WiFi Direct usage.
14 * d/p/lp1099983_ignore-p2p-wifi-devices.patch: refreshed
15 * d/p/wifi-fix-cancel-scan.patch: add logic to supplicant_iface_state_cb
16 to schedule a scan if supplicant state becomes inactive, and
17 old_state was scanning.
18
19 -- Tony Espy <espy@canonical.com> Sat, 12 Mar 2016 15:37:31 -0500
20
1network-manager (0.9.10.0-4ubuntu15.1.8) vivid; urgency=medium21network-manager (0.9.10.0-4ubuntu15.1.8) vivid; urgency=medium
222
3 * d/p/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch:23 * d/p/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch:
424
=== modified file 'debian/patches/ignore_rfkill_if_urfkill_is_present.patch'
--- debian/patches/ignore_rfkill_if_urfkill_is_present.patch 2014-11-18 22:46:35 +0000
+++ debian/patches/ignore_rfkill_if_urfkill_is_present.patch 2016-03-15 23:32:40 +0000
@@ -34,7 +34,7 @@
34 34
35 /* List of NMDeviceFactoryFunc pointers sorted in priority order */35 /* List of NMDeviceFactoryFunc pointers sorted in priority order */
36 GSList *factories;36 GSList *factories;
37@@ -217,6 +221,7 @@ typedef struct {37@@ -219,6 +223,7 @@ typedef struct {
38 gboolean ifstate_force_online;38 gboolean ifstate_force_online;
39 39
40 guint timestamp_update_id;40 guint timestamp_update_id;
@@ -42,7 +42,7 @@
42 42
43 gboolean startup;43 gboolean startup;
44 } NMManagerPrivate;44 } NMManagerPrivate;
45@@ -670,7 +675,20 @@ find_unmanaged_state (NMManager *manager45@@ -673,7 +678,20 @@ find_unmanaged_state (NMManager *manager
46 if (state == NM_DEVICE_STATE_UNMANAGED) {46 if (state == NM_DEVICE_STATE_UNMANAGED) {
47 const char *iface = nm_device_get_ip_iface (dev);47 const char *iface = nm_device_get_ip_iface (dev);
48 if (priv->ifstate_force_online) {48 if (priv->ifstate_force_online) {
@@ -63,7 +63,7 @@
63 nm_log_dbg (LOGD_CORE, "Unmanaged device found: %s; state CONNECTED forced.", iface);63 nm_log_dbg (LOGD_CORE, "Unmanaged device found: %s; state CONNECTED forced.", iface);
64 }64 }
65 }65 }
66@@ -4669,6 +4687,49 @@ dbus_connection_changed_cb (NMDBusManage66@@ -4784,6 +4802,49 @@ dbus_connection_changed_cb (NMDBusManage
67 67
68 /**********************************************************************/68 /**********************************************************************/
69 69
@@ -113,7 +113,7 @@
113 static NMManager *singleton = NULL;113 static NMManager *singleton = NULL;
114 114
115 NMManager *115 NMManager *
116@@ -4686,6 +4747,43 @@ nm_connection_provider_get (void)116@@ -4801,6 +4862,43 @@ nm_connection_provider_get (void)
117 return NM_CONNECTION_PROVIDER (NM_MANAGER_GET_PRIVATE (singleton)->settings);117 return NM_CONNECTION_PROVIDER (NM_MANAGER_GET_PRIVATE (singleton)->settings);
118 }118 }
119 119
@@ -157,7 +157,7 @@
157 NMManager *157 NMManager *
158 nm_manager_new (NMSettings *settings,158 nm_manager_new (NMSettings *settings,
159 const char *state_file,159 const char *state_file,
160@@ -4777,13 +4875,35 @@ nm_manager_new (NMSettings *settings,160@@ -4892,13 +4990,35 @@ nm_manager_new (NMSettings *settings,
161 G_CALLBACK (rfkill_manager_rfkill_changed_cb),161 G_CALLBACK (rfkill_manager_rfkill_changed_cb),
162 singleton);162 singleton);
163 163
164164
=== modified file 'debian/patches/lp1099983_ignore-p2p-wifi-devices.patch'
--- debian/patches/lp1099983_ignore-p2p-wifi-devices.patch 2015-02-27 00:29:09 +0000
+++ debian/patches/lp1099983_ignore-p2p-wifi-devices.patch 2016-03-15 23:32:40 +0000
@@ -2,23 +2,24 @@
2Subject: Ignore p2p0 wifi devices from android.2Subject: Ignore p2p0 wifi devices from android.
33
4---4---
5 src/nm-manager.c | 10 ++++++++++5 src/nm-manager.c | 11 +++++++++++
6 1 file changed, 10 insertions(+)6 1 file changed, 11 insertions(+)
77
8Index: network-manager-0.9.10.0/src/nm-manager.c8Index: b/src/nm-manager.c
9===================================================================9===================================================================
10--- network-manager-0.9.10.0.orig/src/nm-manager.c10--- a/src/nm-manager.c
11+++ network-manager-0.9.10.0/src/nm-manager.c11+++ b/src/nm-manager.c
12@@ -2060,6 +2060,16 @@ platform_link_added (NMManager *self,12@@ -2151,6 +2151,17 @@ platform_link_added (NMManager *self,
13 !strncmp (plink->name, "ccmni", STRLEN ("ccmni")))13 !strncmp (plink->name, "ccmni", STRLEN ("ccmni")))
14 return;14 return;
15 15
16+ /*16+ /*
17+ * Ubuntu: Explicitly unmanage p2p Wi-Fi devices exposed by Android JB Wi-Fi drivers.17+ * Ubuntu: Explicitly unmanage all p2p Wi-Fi devices which are
18+ * managed externally in the case of Wi-Fi Direct.
18+ */19+ */
19+ NMDeviceType devtype = nm_device_get_device_type (device);20+ NMDeviceType devtype = nm_device_get_device_type (device);
20+ if ((plink->type == NM_LINK_TYPE_WIFI)21+ if ((plink->type == NM_LINK_TYPE_WIFI)
21+ && !strncmp (plink->name, "p2p0", STRLEN ("p2p0"))) {22+ && !strncmp (plink->name, "p2p", STRLEN ("p2p"))) {
22+ nm_log_info (LOGD_HW, "(%s): ignoring P2P wireless iface", plink->name);23+ nm_log_info (LOGD_HW, "(%s): ignoring P2P wireless iface", plink->name);
23+ return;24+ return;
24+ }25+ }
2526
=== modified file 'debian/patches/series'
--- debian/patches/series 2015-10-01 21:39:53 +0000
+++ debian/patches/series 2016-03-15 23:32:40 +0000
@@ -76,3 +76,4 @@
76lp1444162-add-ip6-config-to-nm-ofono-connections.patch76lp1444162-add-ip6-config-to-nm-ofono-connections.patch
77fix-ofono-plugin-leaks.patch77fix-ofono-plugin-leaks.patch
78rm-scofono-plugin-dbus.patch78rm-scofono-plugin-dbus.patch
79wifi-fix-cancel-scan.patch
7980
=== added file 'debian/patches/wifi-fix-cancel-scan.patch'
--- debian/patches/wifi-fix-cancel-scan.patch 1970-01-01 00:00:00 +0000
+++ debian/patches/wifi-fix-cancel-scan.patch 2016-03-15 23:32:40 +0000
@@ -0,0 +1,119 @@
1From: Vicamo Yang <vicamo.yang@canonical.com>
2Date: Tue, 11 Jan 2016 18:40:30 -0500
3Subject: [PATCH] wwan: add support for using oFono as a modem manager
4
5This patch fixes a race condition in NMDeviceWiFi's scanning logic.
6It's possible for cancel_pending_scan() to be called when an actual
7supplicant scan is pending, which can cause scanning to stall,
8effectively preventing the device from re-connecting to WiFi.
9
10---
11 src/devices/wifi/nm-device-wifi.c | 44 +++++++++++++++++++++++++++++++-------
12 src/nm-policy.c | 4 ++-
13 2 files changed, 39 insertions(+), 9 deletions(-)
14
15Index: b/src/devices/wifi/nm-device-wifi.c
16===================================================================
17--- a/src/devices/wifi/nm-device-wifi.c
18+++ b/src/devices/wifi/nm-device-wifi.c
19@@ -774,8 +774,10 @@ deactivate (NMDevice *dev)
20
21 /* Ensure we trigger a scan after deactivating a Hotspot */
22 if (old_mode == NM_802_11_MODE_AP) {
23- cancel_pending_scan (self);
24- request_wireless_scan (self);
25+ if (!priv->requested_scan) {
26+ cancel_pending_scan (self);
27+ request_wireless_scan (self);
28+ }
29 }
30 }
31
32@@ -1236,6 +1238,7 @@ request_scan_cb (NMDevice *device,
33 gpointer user_data)
34 {
35 NMDeviceWifi *self = NM_DEVICE_WIFI (device);
36+ NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
37 GError *local = NULL;
38
39 if (error) {
40@@ -1252,8 +1255,10 @@ request_scan_cb (NMDevice *device,
41 return;
42 }
43
44- cancel_pending_scan (self);
45- request_wireless_scan (self);
46+ if (!priv->requested_scan) {
47+ cancel_pending_scan (self);
48+ request_wireless_scan (self);
49+ }
50 dbus_g_method_return (context);
51 }
52
53@@ -2139,8 +2144,13 @@ supplicant_iface_state_cb (NMSupplicantI
54 nm_device_get_iface (device));
55
56 /* Request a scan to get latest results */
57- cancel_pending_scan (self);
58- request_wireless_scan (self);
59+ if (!priv->requested_scan) {
60+ /* Scan request should have been issued in nm_device_state_changed if
61+ * the device was previously unavailable.
62+ */
63+ cancel_pending_scan (self);
64+ request_wireless_scan (self);
65+ }
66
67 if (old_state < NM_SUPPLICANT_INTERFACE_STATE_READY)
68 nm_device_remove_pending_action (device, "waiting for supplicant", TRUE);
69@@ -2219,6 +2229,22 @@ supplicant_iface_state_cb (NMSupplicantI
70 NM_DEVICE_STATE_UNAVAILABLE,
71 NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED);
72 break;
73+ case NM_SUPPLICANT_INTERFACE_STATE_INACTIVE:
74+ if (old_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING) {
75+ nm_log_warn (LOGD_HW | LOGD_WIFI, "(%s): supplicant state: SCANNING -> INACTIVE",
76+ nm_device_get_iface (NM_DEVICE (self)));
77+
78+ if (priv->requested_scan) {
79+ nm_log_dbg (LOGD_WIFI_SCAN, "(%s): clearing requested_scan",
80+ nm_device_get_iface (NM_DEVICE (self)));
81+
82+ priv->requested_scan = FALSE;
83+ }
84+
85+ cancel_pending_scan (self);
86+ request_wireless_scan (self);
87+ }
88+
89 default:
90 break;
91 }
92@@ -3109,8 +3135,10 @@ device_state_changed (NMDevice *device,
93 case NM_DEVICE_STATE_DISCONNECTED:
94 /* Kick off a scan to get latest results */
95 priv->scan_interval = SCAN_INTERVAL_MIN;
96- cancel_pending_scan (self);
97- request_wireless_scan (self);
98+ if (!priv->requested_scan) {
99+ cancel_pending_scan (self);
100+ request_wireless_scan (self);
101+ }
102 break;
103 default:
104 break;
105Index: b/src/nm-policy.c
106===================================================================
107--- a/src/nm-policy.c
108+++ b/src/nm-policy.c
109@@ -1034,7 +1034,9 @@ auto_activate_device (gpointer user_data
110 g_error_free (error);
111 }
112 g_object_unref (subject);
113- }
114+ } else
115+ nm_log_dbg (LOGD_DEVICE, "auto_activate_connection: no best_connection found for '%s'!",
116+ nm_device_get_iface (data->device));
117
118 g_slist_free (connections);
119

Subscribers

People subscribed via source and target branches

to all changes: