GNOME Bugzilla – Bug 686732
[PATCH] network: enable real AP mode
Last modified: 2013-01-30 18:43:01 UTC
Created attachment 227099 [details] [review] Support new AP-mode hotspots Recent NetworkManager can create AP-mode hotspots instead of Ad-Hoc mode ones, which are less compatible with mobile devices. Do that if NetworkManager and the device support it.
Review of attachment 227099 [details] [review]: ::: panels/network/net-device-wifi.c @@ +438,3 @@ } + else if (strcmp (key_mgmt, "wpa-none") == 0 || + strcmp (key_mgmt, "wpa-psk") == 0) { I don't understand this change. @@ +1416,3 @@ NMClient *client; NMRemoteSettings *remote_settings; + const char *mode = NM_SETTING_WIRELESS_MODE_ADHOC; Can you initialise it before the #ifdef NM_SETTING_WIRELESS_MODE_AP block?
(In reply to comment #1) > Review of attachment 227099 [details] [review]: > > ::: panels/network/net-device-wifi.c > @@ +438,3 @@ > } > + else if (strcmp (key_mgmt, "wpa-none") == 0 || > + strcmp (key_mgmt, "wpa-psk") == 0) { > > I don't understand this change. WPA AdHoc mode requires the "wpa-none" key management setting. WPA AP mode requires "wpa-psk". Thus when creating a real AP-mode WPA protected network as opposed to an AdHoc mode one, we need to use "wpa-psk" not "wpa-none". I expect "wpa-none" to actually die a long-deserved death, since when we merge IBSS RSN support (ie, WPA2 AdHoc) which also uses "wpa-psk", then we'll kill wpa-none completely. Long story short, "wpa-none" is only used for WPAv1 AdHoc, which is completely broken in most kernel drivers, and never will be fixed since IBSS RSN (ie, WPAv2 AdHoc) exists and works. We removed WPAv1 AdHoc support from NM in 0.9.4 specifically due to the kernel breakage, because the kernel may silently create your supposedly WPA-protected adhoc networks as open. > @@ +1416,3 @@ > NMClient *client; > NMRemoteSettings *remote_settings; > + const char *mode = NM_SETTING_WIRELESS_MODE_ADHOC; > > Can you initialise it before the #ifdef NM_SETTING_WIRELESS_MODE_AP block? Sure.
Created attachment 228181 [details] [review] Support new AP-mode hotspots
Can this get another review and get merged ?
Comment on attachment 228181 [details] [review] Support new AP-mode hotspots >+#ifdef NM_SETTING_WIRELESS_MODE_AP >+ /* Use real AP mode if the device supports it */ >+ if (nm_device_wifi_get_capabilities (NM_DEVICE_WIFI (device)) & NM_WIFI_DEVICE_CAP_AP) >+ mode = NM_SETTING_WIRELESS_MODE_AP; >+#endif I'd add a comment there explaining that the #ifdef can go away once g-c-c depends on NM 0.9.8 or later.
Have you tested the patch, Dan ?
(In reply to comment #4) > Can this get another review and get merged ? I was waiting for the panel rework to land (or have this land as part of the panel rework). If it doesn't clash, the patch looks fine to commit with danw's comment fixed.
(In reply to comment #7) > I was waiting for the panel rework to land (or have this land as part of the > panel rework). If it doesn't clash, the patch looks fine to commit with danw's > comment fixed. we actually depend on new-enough NM now, so I just removed the #ifdef altogether.