netdev: move power save disabling until after interface is up

Very rarely on ath10k (potentially other ath cards), disabling
power save while the interface is down causes a timeout when
bringing the interface back up. This seems to be a race in the
driver or firmware but it causes IWD to never start up properly
since there is no retry logic on that path.

Retrying is an option, but a more straight forward approach is
to just reorder the logic to set power save off after the
interface is already up. If the power save setting fails we can
just log it, ignore the failure, and continue. From a users point
of view there is no real difference in doing it this way as
PS still gets disabled prior to IWD connecting/sending data.

Changing behavior based on a buggy driver isn't something we
should be doing, but in this instance the change shouldn't have
any downside and actually isn't any different than how it has
been done prior to the driver quirks change (i.e. use network
manager, iw, or iwconfig to set power save after IWD starts).

For reference, this problem is quite rare and difficult to say
exactly how often but certainly <1% of the time:

iwd[1286641]: src/netdev.c:netdev_disable_ps_cb() Disabled power save for ifindex 54
kernel: ath10k_pci 0000:02:00.0: wmi service ready event not received
iwd[1286641]: Error bringing interface 54 up: Connection timed out
kernel: ath10k_pci 0000:02:00.0: Could not init core: -110

After this IWD just sits idle as it has no interface to start using.

This is even reproducable outside of IWD if you loop and run:

ip link set <wlan> down
iw dev <wlan> set power_save off
ip link set <wlan> up

Eventually the 'up' command will fail with a timeout.

I've brought this to the linux-wireless/ath10k mailing list but
even if its fixed in future kernels we'd still need to support
older kernels, so a workaround/change in IWD is still required.

1 files changed, 44 insertions, 42 deletions

diff --git a/src/netdev.c b/src/netdev.c
index e31a51617..f23ca59ea 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -5935,6 +5935,44 @@ static void netdev_dellink_notify(const struct ifinfomsg *ifi, int bytes)
 	netdev_free(netdev);
 }
 
+static void netdev_disable_ps_cb(struct l_genl_msg *msg, void *user_data)
+{
+	struct netdev *netdev = user_data;
+	int err = l_genl_msg_get_error(msg);
+
+	netdev->power_save_cmd_id = 0;
+
+	/* Can't do anything about it but inform the user */
+	if (err < 0)
+		l_error("Failed to disable power save for ifindex %u (%s: %d)",
+				netdev->index, strerror(-err), err);
+	else
+		l_debug("Disabled power save for ifindex %u", netdev->index);
+
+	WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t,
+				netdev, NETDEV_WATCH_EVENT_NEW);
+	netdev->events_ready = true;
+}
+
+static bool netdev_disable_power_save(struct netdev *netdev)
+{
+	struct l_genl_msg *msg = l_genl_msg_new(NL80211_CMD_SET_POWER_SAVE);
+	uint32_t disabled = NL80211_PS_DISABLED;
+
+	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
+	l_genl_msg_append_attr(msg, NL80211_ATTR_PS_STATE, 4, &disabled);
+
+	netdev->power_save_cmd_id = l_genl_family_send(nl80211, msg,
+							netdev_disable_ps_cb,
+							netdev, NULL);
+	if (!netdev->power_save_cmd_id) {
+		l_error("Failed to send SET_POWER_SAVE (-EIO)");
+		return false;
+	}
+
+	return true;
+}
+
 static void netdev_initial_up_cb(int error, uint16_t type, const void *data,
 					uint32_t len, void *user_data)
 {
@@ -5967,6 +6005,12 @@ static void netdev_initial_up_cb(int error, uint16_t type, const void *data,
 
 	scan_wdev_add(netdev->wdev_id);
 
+	if (wiphy_power_save_disabled(netdev->wiphy)) {
+		/* Wait to issue EVENT_NEW until power save is disabled */
+		if (netdev_disable_power_save(netdev))
+			return;
+	}
+
 	WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t,
 				netdev, NETDEV_WATCH_EVENT_NEW);
 	netdev->events_ready = true;
@@ -6130,42 +6174,6 @@ static void netdev_get_link(struct netdev *netdev)
 	l_free(rtmmsg);
 }
 
-static void netdev_disable_ps_cb(struct l_genl_msg *msg, void *user_data)
-{
-	struct netdev *netdev = user_data;
-	int err = l_genl_msg_get_error(msg);
-
-	netdev->power_save_cmd_id = 0;
-
-	/* Can't do anything about it but inform the user */
-	if (err < 0)
-		l_error("Failed to disable power save for ifindex %u (%s: %d)",
-				netdev->index, strerror(-err), err);
-	else
-		l_debug("Disabled power save for ifindex %u", netdev->index);
-
-	netdev_get_link(netdev);
-}
-
-static bool netdev_disable_power_save(struct netdev *netdev)
-{
-	struct l_genl_msg *msg = l_genl_msg_new(NL80211_CMD_SET_POWER_SAVE);
-	uint32_t disabled = NL80211_PS_DISABLED;
-
-	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_PS_STATE, 4, &disabled);
-
-	netdev->power_save_cmd_id = l_genl_family_send(nl80211, msg,
-							netdev_disable_ps_cb,
-							netdev, NULL);
-	if (!netdev->power_save_cmd_id) {
-		l_error("Failed to send SET_POWER_SAVE (-EIO)");
-		return false;
-	}
-
-	return true;
-}
-
 struct netdev *netdev_create_from_genl(struct l_genl_msg *msg,
 					const uint8_t *set_mac)
 {
@@ -6237,12 +6245,6 @@ struct netdev *netdev_create_from_genl(struct l_genl_msg *msg,
 
 	netdev_setup_interface(netdev);
 
-	if (wiphy_power_save_disabled(wiphy)) {
-		/* Wait to issue GET_LINK until PS is disabled */
-		if (netdev_disable_power_save(netdev))
-			return netdev;
-	}
-
 	netdev_get_link(netdev);
 
 	return netdev;