GNOME Bugzilla – Bug 712745
don't break wake-on-lan
Last modified: 2014-06-24 19:06:45 UTC
NM breaks wake-on-lan, both for ethernet (qv https://bugzilla.redhat.com/show_bug.cgi?id=826652) and wifi (https://mail.gnome.org/archives/networkmanager-list/2013-November/msg00021.html). We should fix that.
We probably should just not take devices down on suspend, but wait until resume to tear down anything that needs it. That way we could do optimizations like, if the resume was shorter than the DHCP lease time, and we reconnect to the same SSID on resume as we were connected to at suspend, we just do a RENEW instead of a whole reactivation cycle.
(In reply to comment #1) > We probably should just not take devices down on suspend, but wait until resume > to tear down anything that needs it. So how do we know what needs it?
pushed danw/wol, which is based on a rebased danw/wifi-platform (bug 724365), and contains fixes for both ethernet wake-on-lan (tested) and wake-on-wlan (not tested)
> wifi: move wifi-utils into platform +NM80211Mode +nm_platform_wifi_get_mode (int ifindex) +{ + reset_error (); + + g_return_val_if_fail (ifindex > 0, FALSE); + + return klass->wifi_get_mode (platform, ifindex); +} Should return a valid NM80211Mode not FALSE. The rest looks good in general; should get another review and some more testing if we can.
> wifi: move wifi-utils into platform pushed one minor fixup!, not sure you like it. Could you split the style changes (mainly in src/platform/wifi/wifi-utils-nl80211.c) to a separate branch *before* moving the file? This patch is already quite bug, so split out these trivial changes. > core: leave wake-on-LAN devices up over suspend/resume do you have more in mind with the two new boolean arguments to do_sleep_wake()? Because right now, you could combine them to one argument (the other being it's negated value). Also, the parameter net_enable_changed is unused. The rest looks good for now
I don't think we can combine the arguments, because we need to know that sleeping is changed independently of enable/disable. eg, _internal_enable() should never be able to affect the values of suspending or waking_from_suspend, because when NM is enabled/disabled we need to preserve the current behavior of shooting your network in the head for backwards compatibility. But maybe just drop net_enabled_changed for now, and put a comment in do_sleep_wake() in the "suspending" case that says that we don't take down WOL devices at all because that kills WOL functionality. Other than that, patches look good.
pushed with a modified version of Thomas's fixup, and removing the unused net_enabled arg
Created attachment 278603 [details] [review] fix to wowlan detection WoWLAN detection didn't actually work, but the code happened to fail with the same error code you'd get if you'd made the right query, if your card didn't support WoWLAN. Now tested with a card that actually does support it.
Created attachment 278622 [details] [review] 0001-wifi-consolidate-netlink-setup-code-and-ensure-crede.patch Mostly paranoia, but lets check the sender and also consolidate the netlink socket setup code while we're at it.
(In reply to comment #8) > Created an attachment (id=278603) [details] [review] > fix to wowlan detection > > WoWLAN detection didn't actually work, but the code happened to fail with the > same error code you'd get if you'd made the right query, if your card didn't > support WoWLAN. Now tested with a card that actually does support it. Patch looks good to me.
pushed my patch. I didn't look too hard at yours, but it looks ok (and the cleanup part is definitely good).