Re: [RFC PATCH 1/1] supplicant: request ACS if channel==0

2016-05-31 Thread Ulrich Ölmann 
Hi Dan,

On Fri, May 27, 2016 at 12:07:09PM -0500, Dan Williams wrote:
> On Fri, 2016-05-27 at 18:12 +0200, Ulrich Ölmann wrote:
> > By analogy to wpa_supplicant.conf Automatic Channel Selection (ACS)
> > is coded as
> > "channel=0" within a connection. wpa_supplicant then needs to know if
> > it shall
> > look out for a channel in the 2.4 GHz or the 5 GHz band. This
> > information is
> > passed over via a sample frequency from the respective band.
> 
> Definitely good to do and thanks for the patch.  Given that the ACS
> support isn't yet in a released supplicant, does this patch cause
> problems with older supplicants that don't have ACS support?

it probably will - I have to look into that.

> I think this patch will change the behavior a bit for older
> supplicants, so that for AdHoc/APs that leave band/frequency undefined
> will now no longer send the NM-determined frequency to the supplicant.
>  act_stage2_config() calls ensure_hotspot_frequency() to set a fixed
> frequency, and that gets passed
> to nm_supplicant_config_add_setting_wireless().  But since channel is
> still undefined, that random fixed_freq will get overwritten either
> with FIVE_GHZ_SAMPLE_FREQ/TWO_GHZ_SAMPLE_FREQ or will remain at 0 if
> the connection specifies no band.
> 
> We could use the supplicant's global capabilities
> (see wpas_dbus_getter_global_capabilities()) to add an ACS flag when
> the supplicant has ACS compiled in, which NM could then read and use to
> enable the new ACS behavior?

Hopefully I will have time to look into this and prepare a v2 next week.

Thanks for your suggestions
Ulrich

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list

Re: [RFC PATCH 1/1] supplicant: request ACS if channel==0

2016-05-27 Thread Dan Williams 
On Fri, 2016-05-27 at 18:12 +0200, Ulrich Ölmann wrote:
> By analogy to wpa_supplicant.conf Automatic Channel Selection (ACS)
> is coded as
> "channel=0" within a connection. wpa_supplicant then needs to know if
> it shall
> look out for a channel in the 2.4 GHz or the 5 GHz band. This
> information is
> passed over via a sample frequency from the respective band.

Definitely good to do and thanks for the patch.  Given that the ACS
support isn't yet in a released supplicant, does this patch cause
problems with older supplicants that don't have ACS support?

I think this patch will change the behavior a bit for older
supplicants, so that for AdHoc/APs that leave band/frequency undefined
will now no longer send the NM-determined frequency to the supplicant.
 act_stage2_config() calls ensure_hotspot_frequency() to set a fixed
frequency, and that gets passed
to nm_supplicant_config_add_setting_wireless().  But since channel is
still undefined, that random fixed_freq will get overwritten either
with FIVE_GHZ_SAMPLE_FREQ/TWO_GHZ_SAMPLE_FREQ or will remain at 0 if
the connection specifies no band.

We could use the supplicant's global capabilities
(see wpas_dbus_getter_global_capabilities()) to add an ACS flag when
the supplicant has ACS compiled in, which NM could then read and use to
enable the new ACS behavior?

Dan

> Signed-off-by: Ulrich Ölmann 
> ---
>  src/supplicant-manager/nm-supplicant-config.c  | 37
> ++
>  .../nm-supplicant-settings-verify.c|  1 +
>  2 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/src/supplicant-manager/nm-supplicant-config.c
> b/src/supplicant-manager/nm-supplicant-config.c
> index 5ce8bb31968a..e16353a98583 100644
> --- a/src/supplicant-manager/nm-supplicant-config.c
> +++ b/src/supplicant-manager/nm-supplicant-config.c
> @@ -381,6 +381,9 @@ wifi_freqs_to_string (gboolean bg_band)
>   return str;
>  }
>  
> +#define TWO_GHZ_SAMPLE_FREQ  2412
> +#define FIVE_GHZ_SAMPLE_FREQ 5280
> +
>  gboolean
>  nm_supplicant_config_add_setting_wireless (NMSupplicantConfig *
> self,
> NMSettingWireless *
> setting,
> @@ -428,12 +431,34 @@ nm_supplicant_config_add_setting_wireless
> (NMSupplicantConfig * self,
>   return FALSE;
>   }
>  
> - if ((is_adhoc || is_ap) && fixed_freq) {
> - gs_free char *str_freq = NULL;
> + band = nm_setting_wireless_get_band (setting);
> + channel = nm_setting_wireless_get_channel (setting);
> + if (is_adhoc || is_ap) {
> + /* By analogy to wpa_supplicant.conf Automatic
> Channel Selection (ACS)
> +  * is coded as "channel=0" within the connection -
> wpa_supplicant then
> +  * needs the option "acs=1" and a sample frequency
> which points the ACS
> +  * heuristic to the desired band.
> +  */
> + if (!channel) {
> + if (!nm_supplicant_config_add_option (self,
> "acs", "1", -1, FALSE, error))
> + return FALSE;
>  
> - str_freq = g_strdup_printf ("%u", fixed_freq);
> - if (!nm_supplicant_config_add_option (self,
> "frequency", str_freq, -1, FALSE, error))
> - return FALSE;
> + fixed_freq = 0;
> + if (band) {
> + if (!strcmp (band, "a"))
> + fixed_freq =
> FIVE_GHZ_SAMPLE_FREQ;
> + else if (!strcmp (band, "bg"))
> + fixed_freq =
> TWO_GHZ_SAMPLE_FREQ;
> + }
> + }
> +
> + if (fixed_freq) {
> + gs_free char *str_freq = NULL;
> +
> + str_freq = g_strdup_printf ("%u",
> fixed_freq);
> + if (!nm_supplicant_config_add_option (self,
> "frequency", str_freq, -1, FALSE, error))
> + return FALSE;
> + }
>   }
>  
>   /* Except for Ad-Hoc and Hotspot, request that the driver
> probe for the
> @@ -453,8 +478,6 @@ nm_supplicant_config_add_setting_wireless
> (NMSupplicantConfig * self,
>   return FALSE;
>   }
>  
> - band = nm_setting_wireless_get_band (setting);
> - channel = nm_setting_wireless_get_channel (setting);
>   if (band) {
>   if (channel) {
>   guint32 freq;
> diff --git a/src/supplicant-manager/nm-supplicant-settings-verify.c
> b/src/supplicant-manager/nm-supplicant-settings-verify.c
> index bb046f9361d9..4e7ebcd1d7d0 100644
> --- a/src/supplicant-manager/nm-supplicant-settings-verify.c
> +++ b/src/supplicant-manager/nm-supplicant-settings-verify.c
> @@ -93,6 +93,7 @@ static const struct Opt opt_table[] = {
>   { "bssid",  TYPE_KEYWORD, 0, 0, FALSE,  NULL },
>   { "scan_ssid",  TYPE_INT, 0, 1, FALSE,  NULL },
>