After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 738439 - [review] lr/ap: Adding hot-spot in GNOME Control Center crashes NM
[review] lr/ap: Adding hot-spot in GNOME Control Center crashes NM
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-10-13 10:37 UTC by Lubomir Rintel
Modified: 2014-11-07 15:23 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
[PATCH 1/2] wifi: Use SSID from connection also if AP is not configured (1.94 KB, patch)
2014-10-13 10:37 UTC, Lubomir Rintel 		none Details | Review
[PATCH 2/2] cli: Add mode option for wifi connecitons (4.33 KB, text/plain)
2014-10-13 10:38 UTC, Lubomir Rintel 		  Details

Description Lubomir Rintel 2014-10-13 10:37:28 UTC 
Created attachment 288375 [details] [review]
[PATCH 1/2] wifi: Use SSID from connection also if AP is not configured

See attached patch.
Comment 1 Lubomir Rintel 2014-10-13 10:38:59 UTC 
Created attachment 288376 [details]
[PATCH 2/2] cli: Add mode option for wifi connecitons

Somewhat related: Add support for AP-mode connection creation from nmcli. Not possible by switching mode with ifcfg-rh plugin.
Comment 2 Jiri Klimes 2014-10-20 15:56:23 UTC 
I have pushed the commits to a branch - lr/bgo738439-wifi-ap-mode

The first commit looks OK. We may perform some tests with it, though. For the second commit, I have pushed a fixup commit that fixes problems and enhances the mode parameter to be properly checked, and makes TAB-completion work.

I think we can also add MODE=Ap for ifcfg-rh plugin. Anyway, we are not 100% backwards compatible with initscripts on Wi-Fi. Initscripts Wi-Fi support always was basic only and NM had to create its own extensions.
Recently initscripts moved from iwconfig to iw and made more incompatibilities (not handling KE1-KEY4, CHANNEL, etc.). I think we should talk to lnykryn to coordinate a bit.
https://git.fedorahosted.org/cgit/initscripts.git/commit/sysconfig/network-scripts/ifup-wireless?id=ddda5f6f818831b1fa37337be0ac9c0f619df1ca
Comment 3 Dan Williams 2014-10-20 22:09:09 UTC 
Review of attachment 288375 [details] [review]:

Looks like the right diagnosis, but I think if we have an 'ap' then the SSID from the AP should take precedence over the one from the setting.
Comment 4 Thomas Haller 2014-10-21 06:49:05 UTC 
>> wifi: Use SSID from connection also if AP is not configured

I think this should be a bit different. I pushed a fixup commit.


I also pushed a fixup for "cli: Add mode option for wifi connecitons" to add bash completion.
Comment 5 Lubomir Rintel 2014-10-23 17:39:01 UTC 
Jirka's change LGTM. Thomas' fixup addresses Dan's concern and LGTM as well.

Fixed up in lr/ap: http://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=lr/ap
Comment 6 Dan Williams 2014-10-31 14:21:34 UTC 
> wifi: Ensure ssid from the AP is not an empty string

I don't think this patch is necessary, as the SSID is guaranteed to either be NULL or non-zero-length:

nm_ap_set_ssid (NMAccessPoint *ap, const guint8 *ssid, gsize len)
{
        NMAccessPointPrivate *priv;

        g_return_if_fail (NM_IS_AP (ap));
        g_return_if_fail (ssid == NULL || len > 0);

        priv = NM_AP_GET_PRIVATE (ap);

If for some reason we are getting a zero-length SSID here then we should fix that...

The rest looks good!