Missing fix for 1299975 claimed to be in pm-utils (1.4.1-13ubuntu0.1)

[Dirk F]

The patch debian/patches/17-fix-wireless-hook.patch for bug 1299975 is supposed to be applied in pm-utils (1.4.1-13ubuntu0.1) but is not installed in that version from trusty-updates.

The file /usr/lib/pm-utils/power.d/wireless dated 2014-07-15 16:15 (attached) supposedly installed by the updated package still contains line 23
   [ "$(cat /sys/class/net/$1/device/enabled)" = "1" ] || return 1
which should be
   [ "$(cat /sys/class/net/$1/device/enable)" = "1" ] || return 1

ProblemType: Bug
DistroRelease: Ubuntu 14.04
Package: pm-utils 1.4.1-13ubuntu0.1 [modified: usr/lib/pm-utils/pm-functions]
ProcVersionSignature: Ubuntu 3.13.0-78.122-generic 3.13.11-ckt33
Uname: Linux 3.13.0-78-generic i686
ApportVersion: 2.14.1-0ubuntu3.19
Architecture: i386
CurrentDesktop: LXDE
Date: Thu Feb 11 15:10:05 2016
InstallationDate: Installed on 2014-07-15 (576 days ago)
InstallationMedia: Lubuntu 14.04 LTS "Trusty Tahr" - Release i386 (20140416.2)
PackageArchitecture: all
SourcePackage: pm-utils
UpgradeStatus: No upgrade log present (probably fresh install)

[Dirk F (fieldhouse)]

NB The mod identified in the line from the automated package report
    Package: pm-utils 1.4.1-13ubuntu0.1 [modified: usr/lib/pm-utils/pm-functions]
arises from my manually applied fix for https://bugs.freedesktop.org/show_bug.cgi?id=52572.

It would be good to get that into the repos too.

[Brian Murray (brian-murray)]

The patch, as one can see athttps://launchpadlibrarian.net/179682498/pm-utils_1.4.1-13_1.4.1-13ubuntu0.1.diff.gz, actually makes the following change:

+- [ "$(cat /sys/class/net/$1/device/enable)" = "1" ] || return 1
++ [ "$(cat /sys/class/net/$1/device/enabled)" = "1" ] || return 1

So the patch is applied, but it seems to be wrong as there is no "enabled" sys file now.

bdmurray@bizarro:~$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=14.04
DISTRIB_CODENAME=trusty
DISTRIB_DESCRIPTION="Ubuntu 14.04.3 LTS"
bdmurray@bizarro:~$ ls /sys/class/net/eth0/device/
broken_parity_status consistent_dma_mask_bits dma_mask_bits irq modalias net remove resource resource4 subsystem uevent
class d3cold_allowed driver local_cpulist msi_bus numa_node rescan resource0 resource4_wc subsystem_device vendor
config device enable local_cpus msi_irqs power reset resource2 rom subsystem_vendor vpd

[Dirk F (fieldhouse)]

Actually, this is more tricky than I thought. The validity of the bug 1299975 patch depends on which wireless driver is in use, apparently reflecting some crufty aspects of sysfs and its driver implementations.

In bug 1299975, on rereading, the reporter (savage) says that with an Intel wireless device the virtual file presented in /sys/class/net/$device/device was named "enabled" whereas the script /usr/lib/pm-utils/power.d/wireless referenced "enable".

Whereas I observe that, if present, the virtual file presented in /sys/class/net/$device/device is named "enable", just as in the directory listing in comment #3.

In a Toshiba Tecra 8200 with wlan1 identified as Qualcomm Atheros ARS212/5213/2414 (driver "ath5k"), it's present as "enable".

In a Dell Inspiron 1721 with wlan0 identified as Broadcom Corporation BCM4321 (driver "b43-pci-bridge"), it's not present at all in /sys/class/net/$device/device, but there is a corresponding /sys/bus/pci/drivers/b43-pci-bridge/0000:0b:00.0/enable (and writing 0 to this file disables the driver). However the test in the wireless script assumes that the device is disabled if the /sys/class/net/$device/device/enable* file doesn't exist, so the script fails in this case.

Presumably

a) the presence of the /sys/class/net/$device/device/enable* file depends on whether the ability to disable the device via /sys/class/net/$device/device is supported by the device+driver combination,

b) if the file is present its exact name depends on some details of the driver's sysfs interface and installation procedure, but it is either "enable" or "enabled" (and both are never present together), and

c) if the file is not present the wireless script should assume the device is enabled.

If these assumptions are valid, the following might be a better bet for lines 22-23 of /usr/lib/pm-utils/power.d/wireless:

    # Assume device is disabled if enable or enabled file exists containing 0; then skip
    [ -f /sys/class/net/$1/device/enable* ] && [ "$(cat /sys/class/net/$1/device/enable*)" = "0" ] && return 1

Obviously there are multiline options that could be more precise.

[Martin Pitt (pitti)]

Thanks Dirk! I committed http://anonscm.debian.org/cgit/collab-maint/pm-utils.git/commit/?id=1b7600eca which is more liberal about the spelling now.

[Martin Pitt (pitti)]

I uploaded the adjusted patch to the trusty-proposed SRU queue.

[Launchpad Janitor (janitor)]

This bug was fixed in the package pm-utils - 1.4.1-16

---------------
pm-utils (1.4.1-16) unstable; urgency=medium

  * 17-fix-wireless-hook.patch: Some drivers actually do have an "enable"
    attribute, so accept both "enable" and "enabled". Also invert the logic so
    that if neither attribute exists the device is considered enabled.
    (Closes: #773647, LP: #1544612)
  * Use dh_install instead of debian/rules code for installing Debian hook
    files.
  * Use debian/clean instead of debian/rules code.
  * Move from cdbs to dh.
  * Bump Standards-Version to 3.9.7.
  * Drop Tim Dijkstra as Maintainer, he hasn't uploaded since 2007. Move to
    "Utopia Maintenance Team".

 -- Martin Pitt <email address hidden> Mon, 15 Feb 2016 10:26:45 +0100

[Dirk F (fieldhouse)]

Thanks, Martin.

But, the absence of the enable[d] file is apparently a legitimate case, and your patch as it stands causes an undesirable error message in the PM log in that case, such as:

cat: /sys/class/net/wlan0/device/enable*: No such file or directory

Hence my original proposed test for the file's existence.

As an alternative you could have $(cat .... 2>/dev/null) but that would risk masking a genuine error.

Incidentally, with a fix similar to the one you committed in place, I've observed that the b43 driver, which gets the default power-save command (lines 41-2 of wireless script) currently fails to respect the command, eg:

# iwconfig wlan0 power on
Error for wireless request "Set Power Management" (8B2C) :
    SET failed on device wlan0 ; Operation not supported.

Although the script has special cases for certain Intel drivers and you could add eg new line 41 before the default case

    b43) return1;;

I wouldn't recommend doing so both to avoid complexity (don't want it to end up like pmutils video quirks) and more importantly in case a future version of the driver fixes the lack of power-saving support.

[Martin Pitt (pitti)]

> Hence my original proposed test for the file's existence.

This isn't very robust, as it blows up if the glob expands to more than one file. That also Should Not Happen™, but I prefer to just quiesce the error. I fixed that in http://anonscm.debian.org/cgit/collab-maint/pm-utils.git/commit/?id=aa33266.

I'll reupload the SRU with that too, thanks for pointing out!

[Brian Murray (brian-murray)]

Hello Dirk, or anyone else affected,

Accepted pm-utils into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/pm-utils/1.4.1-13ubuntu0.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Dirk F (fieldhouse)]

Confirmed fix with 1.4.1-13ubuntu0.2, other details per original report.

[Launchpad Janitor (janitor)]

This bug was fixed in the package pm-utils - 1.4.1-13ubuntu0.2

---------------
pm-utils (1.4.1-13ubuntu0.2) trusty-proposed; urgency=medium

  * 17-fix-wireless-hook.patch: Some drivers actually do have an "enable"
      attribute, so accept both "enable" and "enabled". Also invert the logic
      so that if neither attribute exists the device is considered enabled.
      (Closes: #773647, LP: #1544612)

 -- Martin Pitt <email address hidden> Mon, 15 Feb 2016 10:30:26 +0100

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for pm-utils has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.