FWIW, the crosh proposal SGTM.

Also, rather than debugd, I wonder if this is something that shill (or possibly powerd?) should plumb for us. We already have powerd hooks for Wifi TX power settings, though I believe I've heard complaints that this should have been in shill.

I suppose if it's really supposed to be a debug hack though, debugd isn't a terrible option. At least it makes it a little clearer that it's not a central feature (e.g., non-persistence).

Is this something that actually needs to be plumbed through to the user?

> Be able to toggle power save as a workaround until any bugs are fixed.

This makes me nervous. If we're concerned enough about bugs that we think we need an escape hatch, we probably shouldn't be enabling this yet or (much less likely) should add a user-visible setting for it. crosh isn't the place for "super advanced" settings.

For debugging, I assume that developers are typically going to have their own builds, or at least have devices that are in dev mode. As a first step, can we just make it easier to turn this off on a device with a writable filesystem by e.g. touching a file on the stateful partition?

Wifi power-save mode has always been active when on battery. The only change is that now it's *always* on, even on AC. So whereas we could previously ask naive users questions like "does it work better when plugged in to AC", we no longer have any kind of hack for this.

"If we're concerned enough about bugs that we think we need an escape hatch, we probably shouldn't be enabling this yet"

I think we're beyond being able to disable power-save mode. It sucks way too much power, and on most devices it works just fine. It's mostly on poorly-developed and/or new chipsets/drivers where this is a concern (looking at you, Marvell).

If you'd like to restore the "disable on AC" behavior...well, I guess you can ask mka@ :)

"For debugging, I assume that developers are typically going to have their own builds, or at least have devices that are in dev mode."

This isn't targeted only at developers, and even many developers are dogfooding in verified mode now (thanks corp security!). And due to the nature of Wifi (and Wifi bugs), we really need to be able to debug (or at least do a little bit of triage for) in-the-field problems, even when reported from non-developers.

"As a first step, can we just make it easier to turn this off on a device with a writable filesystem by e.g. touching a file on the stateful partition?"

What would that be addressing? If you're in dev mode, it's already easy enough to do:

iw dev {m,w}lan0 set power_save off

Thanks for the details. Putting this in crosh sounds fine to me as long as the change that it makes isn't persistent across reboots.

I would definitely prefer it not go through powerd. :-P I'd actually prefer to remove the wifi tx power stuff from powerd as well -- powerd announces changes to the tablet mode state over D-Bus, so it'd probably be fairly straightforward to move that to shill.

Can you adjust permissions in sysfs (or wherever) to avoid needing to do this as root?

It's modified through nl80211 (netlink socket), so there's no relevant file permissions. The user has to have the CAP_NET_ADMIN capability. Best handled through delegating to some daemon (debugd or shill).

debugd would be my preference for housing such things.

Adding to my new-people bug list

Over to Ed

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/024f563527558e297cd92be14a0072fd36feaf16

commit 024f563527558e297cd92be14a0072fd36feaf16
Author: Edward Hill <ecgh@chromium.org>
Date: Fri Jul 07 20:15:47 2017

crosh/debugd: Add wifi_power_save command.

wifi_power_save <on | off>
Enable or disable WiFi power save mode.
With no parameter, show current mode.
This command is not persistent across system restarts.
Uses debugd to run iw to get/set the mode.

BUG= chromium:723084 
TEST=crosh and debugd tests
TEST=turn power save on and off using crosh on pyro

Change-Id: I3a0d3d8f5ec7394991f66822132eb98112bce502
Reviewed-on: https://chromium-review.googlesource.com/556883
Commit-Ready: Edward Hill <ecgh@chromium.org>
Tested-by: Edward Hill <ecgh@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[add] https://crrev.com/024f563527558e297cd92be14a0072fd36feaf16/debugd/src/wifi_power_tool.cc
[modify] https://crrev.com/024f563527558e297cd92be14a0072fd36feaf16/debugd/src/debugd_dbus_adaptor.cc
[modify] https://crrev.com/024f563527558e297cd92be14a0072fd36feaf16/debugd/src/debugd_dbus_adaptor.h
[modify] https://crrev.com/024f563527558e297cd92be14a0072fd36feaf16/debugd/dbus_bindings/org.chromium.debugd.xml
[modify] https://crrev.com/024f563527558e297cd92be14a0072fd36feaf16/crosh/crosh
[modify] https://crrev.com/024f563527558e297cd92be14a0072fd36feaf16/debugd/debugd.gyp
[add] https://crrev.com/024f563527558e297cd92be14a0072fd36feaf16/debugd/src/wifi_power_tool.h