GNOME.org
 

Re: [RFC PATCH] WIP: support ap-mode with wpa_supplicant

On Thu, 2012-05-31 at 15:04 +0200, Jan Luebbe wrote:
> A new value for NM80211Mode is introduced (NM_802_11_MODE_AP) and the
> new mode is passed to wpa_supplicant analogous to adhoc-mode.
> 
> Signed-off-by: Jan Luebbe <jlu pengutronix de>
> ---
> Hi!
> 
> Would something like the patch below be acceptable?

Thanks for working on this!

This patch is basically the approach to take, yes.  In addition to the
patches here, you'll need to take a look at the connection matching code
(like nm_access_point_connection_valid() and
nm_ap_utils_complete_connection()) and a few other places.  Basically
grep the code for NM_802_11_MODE and every place reported probably needs
an audit.

Other comments below...

> Currently the frequency is still hard-coded, which I would change if
> the general approach is fine.
> 
> Regards,
> Jan
> 
>  include/NetworkManager.h                           |    3 +-
>  libnm-util/nm-setting-wireless.c                   |    6 ++--
>  libnm-util/nm-setting-wireless.h                   |    7 +++++
>  src/nm-device-wifi.c                               |    5 ++++
>  src/nm-wifi-ap.c                                   |    4 ++-
>  src/supplicant-manager/nm-supplicant-config.c      |   30 +++++++++++++++++---
>  .../nm-supplicant-settings-verify.c                |    2 +-
>  7 files changed, 47 insertions(+), 10 deletions(-)
> 
> diff --git a/include/NetworkManager.h b/include/NetworkManager.h
> index 0aa31ae..f1b484f 100644
> --- a/include/NetworkManager.h
> +++ b/include/NetworkManager.h
> @@ -233,7 +233,8 @@ typedef enum {
>  typedef enum {
>  	NM_802_11_MODE_UNKNOWN = 0,
>  	NM_802_11_MODE_ADHOC,
> -	NM_802_11_MODE_INFRA
> +	NM_802_11_MODE_INFRA,
> +	NM_802_11_MODE_AP
>  } NM80211Mode;
>  
>  /**
> diff --git a/libnm-util/nm-setting-wireless.c b/libnm-util/nm-setting-wireless.c
> index 3188251..ff8584b 100644
> --- a/libnm-util/nm-setting-wireless.c
> +++ b/libnm-util/nm-setting-wireless.c
> @@ -568,7 +568,7 @@ static gboolean
>  verify (NMSetting *setting, GSList *all_settings, GError **error)
>  {
>  	NMSettingWirelessPrivate *priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting);
> -	const char *valid_modes[] = { NM_SETTING_WIRELESS_MODE_INFRA, NM_SETTING_WIRELESS_MODE_ADHOC, NULL };
> +	const char *valid_modes[] = { NM_SETTING_WIRELESS_MODE_INFRA, NM_SETTING_WIRELESS_MODE_ADHOC, NM_SETTING_WIRELESS_MODE_AP, NULL };
>  	const char *valid_bands[] = { "a", "bg", NULL };
>  	GSList *iter;
>  
> @@ -866,8 +866,8 @@ nm_setting_wireless_class_init (NMSettingWirelessClass *setting_class)
>  		(object_class, PROP_MODE,
>  		 g_param_spec_string (NM_SETTING_WIRELESS_MODE,
>  						  "Mode",
> -						  "WiFi network mode; one of 'infrastructure' or "
> -						  "'adhoc'.  If blank, infrastructure is assumed.",
> +						  "WiFi network mode; one of 'infrastructure', "
> +						  "'adhoc' or 'ap'.  If blank, infrastructure is assumed.",
>  						  NULL,
>  						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));

Be sure to also update the gtk-doc comment right above the property
definition too.  That's what actually generates the HTML documentation,
while the GObject property text generates the settings specification.
Both need to stay in sync.
 
> diff --git a/libnm-util/nm-setting-wireless.h b/libnm-util/nm-setting-wireless.h
> index 3182f41..99cbe41 100644
> --- a/libnm-util/nm-setting-wireless.h
> +++ b/libnm-util/nm-setting-wireless.h
> @@ -85,6 +85,13 @@ GQuark nm_setting_wireless_error_quark (void);
>  #define NM_SETTING_WIRELESS_MODE_ADHOC  "adhoc"
>  
>  /**
> + * NM_SETTING_WIRELESS_MODE_ADHOC:

You probably want NM_SETTING_WIRELESS_MODE_AP here :)

> + *
> + * Indicates AP/master mode where this device is the access point.
> + */
> +#define NM_SETTING_WIRELESS_MODE_AP     "ap"
> +
> +/**
>   * NM_SETTING_WIRELESS_MODE_INFRA
>   *
>   * Indicates infrastructure mode where an access point is expected to be present
> diff --git a/src/nm-device-wifi.c b/src/nm-device-wifi.c
> index ed964a5..a52d895 100644
> --- a/src/nm-device-wifi.c
> +++ b/src/nm-device-wifi.c
> @@ -696,6 +696,11 @@ periodic_update (NMDeviceWifi *self)
>  	NMAccessPoint *new_ap;
>  	guint32 new_rate, percent;
>  
> +	/* In AP mode we currently have nothing to do. */
> +	if (priv->current_ap && (nm_ap_get_mode (priv->current_ap) == NM_802_11_MODE_AP)) {
> +		return;
> +	}

If there's only a single statement under an 'if' or 'for', you don't
need the curly braces.

> +
>  	/* In IBSS mode, most newer firmware/drivers do "BSS coalescing" where
>  	 * multiple IBSS stations using the same SSID will eventually switch to
>  	 * using the same BSSID to avoid network segmentation.  When this happens,
> diff --git a/src/nm-wifi-ap.c b/src/nm-wifi-ap.c
> index 92a99b6..009ca5c 100644
> --- a/src/nm-wifi-ap.c
> +++ b/src/nm-wifi-ap.c
> @@ -652,6 +652,8 @@ nm_ap_new_fake_from_connection (NMConnection *connection)
>  			nm_ap_set_mode (ap, NM_802_11_MODE_INFRA);
>  		else if (!strcmp (mode, "adhoc"))
>  			nm_ap_set_mode (ap, NM_802_11_MODE_ADHOC);
> +		else if (!strcmp (mode, "ap"))
> +			nm_ap_set_mode (ap, NM_802_11_MODE_AP);
>  		else
>  			goto error;
>  	} else {
> @@ -967,7 +969,7 @@ void nm_ap_set_mode (NMAccessPoint *ap, const NM80211Mode mode)
>  
>  	g_return_if_fail (NM_IS_AP (ap));
>  
> -	if (mode == NM_802_11_MODE_ADHOC || mode == NM_802_11_MODE_INFRA) {
> +	if (mode == NM_802_11_MODE_ADHOC || mode == NM_802_11_MODE_INFRA || mode == NM_802_11_MODE_AP) {
>  		priv = NM_AP_GET_PRIVATE (ap);
>  
>  		if (priv->mode != mode) {

This function should get reworked, I've pushed a small commit that moves
the check to a g_return_if_fail() which is what this should have been
using, but this code is really old and didn't get updated.

> diff --git a/src/supplicant-manager/nm-supplicant-config.c b/src/supplicant-manager/nm-supplicant-config.c
> index a8e4ab9..70f9a03 100644
> --- a/src/supplicant-manager/nm-supplicant-config.c
> +++ b/src/supplicant-manager/nm-supplicant-config.c
> @@ -367,7 +367,7 @@ nm_supplicant_config_add_setting_wireless (NMSupplicantConfig * self,
>                                             gboolean has_scan_capa_ssid)
>  {
>  	NMSupplicantConfigPrivate *priv;
> -	gboolean is_adhoc;
> +	gboolean is_adhoc, is_ap;
>  	const char *mode;
>  	const GByteArray *id;
>  
> @@ -378,7 +378,8 @@ nm_supplicant_config_add_setting_wireless (NMSupplicantConfig * self,
>  
>  	mode = nm_setting_wireless_get_mode (setting);
>  	is_adhoc = (mode && !strcmp (mode, "adhoc")) ? TRUE : FALSE;
> -	if (is_adhoc)
> +	is_ap = (mode && !strcmp (mode, "ap")) ? TRUE : FALSE;
> +	if (is_adhoc || is_ap)
>  		priv->ap_scan = 2;
>  	else if (is_broadcast == FALSE) {
>  		/* drivers that support scanning specific SSIDs should use
> @@ -395,7 +396,7 @@ nm_supplicant_config_add_setting_wireless (NMSupplicantConfig * self,
>  
>  	if (is_adhoc) {
>  		if (!nm_supplicant_config_add_option (self, "mode", "1", -1, FALSE)) {
> -			nm_log_warn (LOGD_SUPPLICANT, "Error adding mode to supplicant config.");
> +			nm_log_warn (LOGD_SUPPLICANT, "Error adding mode=1 (adhoc) to supplicant config.");
>  			return FALSE;
>  		}
>  
> @@ -412,10 +413,31 @@ nm_supplicant_config_add_setting_wireless (NMSupplicantConfig * self,
>  		}
>  	}
>  
> +	if (is_ap) {
> +		if (!nm_supplicant_config_add_option (self, "mode", "2", -1, FALSE)) {
> +			nm_log_warn (LOGD_SUPPLICANT, "Error adding mode=2 (ap) to supplicant config.");
> +			return FALSE;
> +		}
> +		
> +		adhoc_freq=2442; // channel 7

The adhoc code has some auto-frequency checking.  Basically, the 'band'
and 'channel' properties of the NMSettingWireless setting should be used
if they aren't 0, and if they are, we should pick a non-populated
channel.  I think we can split out the adhoc code and re-use that for
both.  What I'd do here is change the "adhoc_freq" parameter of this
function to "fixed_freq" and then modify build_supplicant_config() to do
the same thing for AP mode that is done for AdHoc.  Then you can take
the current "if (adhoc_freq)" block out of the adhoc code and use it for
both AP and adhoc in nm_supplicant_config_add_setting_wireless().

> +
> +		if (adhoc_freq) {
> +			char *str_freq;
> +
> +			str_freq = g_strdup_printf ("%u", adhoc_freq);
> +			if (!nm_supplicant_config_add_option (self, "frequency", str_freq, -1, FALSE)) {
> +				g_free (str_freq);
> +				nm_log_warn (LOGD_SUPPLICANT, "Error adding AP frequency to supplicant config.");
> +				return FALSE;
> +			}
> +			g_free (str_freq);
> +		}
> +	}
> +
>  	/* Except for Ad-Hoc networks, request that the driver probe for the
>  	 * specific SSID we want to associate with.
>  	 */
> -	if (!is_adhoc) {
> +	if (!(is_adhoc || is_ap)) {
>  		if (!nm_supplicant_config_add_option (self, "scan_ssid", "1", -1, FALSE))
>  			return FALSE;
>  	}
> diff --git a/src/supplicant-manager/nm-supplicant-settings-verify.c b/src/supplicant-manager/nm-supplicant-settings-verify.c
> index 76de84d..143e51a 100644
> --- a/src/supplicant-manager/nm-supplicant-settings-verify.c
> +++ b/src/supplicant-manager/nm-supplicant-settings-verify.c
> @@ -91,7 +91,7 @@ static const struct Opt opt_table[] = {
>  	{ "ssid",               TYPE_BYTES,   0, 32,FALSE,  NULL },
>  	{ "bssid",              TYPE_KEYWORD, 0, 0, FALSE,  NULL },
>  	{ "scan_ssid",          TYPE_INT,     0, 1, FALSE,  NULL },
> -	{ "mode",               TYPE_INT,     0, 1, FALSE,  NULL },
> +	{ "mode",               TYPE_INT,     0, 2, FALSE,  NULL },
>  	{ "frequency",          TYPE_INT,     2412, 5825, FALSE,  NULL },
>  	{ "auth_alg",           TYPE_KEYWORD, 0, 0, FALSE,  auth_alg_allowed },
>  	{ "psk",                TYPE_BYTES,   0, 0, FALSE,  NULL },

One other thing you'll want to do is modify
real_check_connection_compatible() in nm-device-wifi.c to reject AP-mode
connections if the NMDeviceWifi's priv->capabilities does not include
NM_WIFI_DEVICE_CAP_AP.  Otherwise we'll try to start AP mode connections
on devices that dont' support them.

That's all I can think of for now, looking forward to the next patch.

Thanks!
Dan
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]