diff mbox

hostapd: Add wowlan_triggers config param

Message ID 20140903221740.78543140557@ushik.mtv.corp.google.com
State Accepted
Headers show

Commit Message

Dmitry Shmidt Sept. 3, 2014, 9:58 p.m. UTC
New kernels in wiphy_suspend() will call cfg80211_leave_all()
that will eventually end up in cfg80211_stop_ap() unless
wowlan_triggers were set.

Change-Id: I14d2191eda090cd86cabe1e5f059975fdf2f69e8
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com>
---
 hostapd/config_file.c           |  3 ++
 hostapd/main.c                  | 11 ++++++
 src/ap/ap_config.c              |  2 ++
 src/ap/ap_config.h              |  2 ++
 src/common/wpa_common.c         | 76 +++++++++++++++++++++++++++++++++++++++++
 wpa_supplicant/wpa_supplicant.c | 70 +++++--------------------------------
 6 files changed, 103 insertions(+), 61 deletions(-)

Comments

Jouni Malinen Sept. 13, 2014, 2:17 p.m. UTC | #1
On Wed, Sep 03, 2014 at 02:58:37PM -0700, Dmitry Shmidt wrote:
> New kernels in wiphy_suspend() will call cfg80211_leave_all()
> that will eventually end up in cfg80211_stop_ap() unless
> wowlan_triggers were set.

Could you please clarify why this is needed? If kernel behavior changed
at some point to do something that is not desired, wouldn't the correct
way of fixing this be by reverting parts of the kernel behavior rather
than by trying to make hostapd do something.

I don't understand what WoWLAN triggers have to do with AP mode
operations. Or would there really be a case where WoWLAN can be used
from a station device to wake up the AP?

I'm assuming this is trying to undo some parts of kernel commit
8125696991194aacb1173b6e8196d19098b44e17 ('cfg80211/mac80211: disconnect
on suspend'). Is that the case or did I miss what the main point of the
hostapd changes here would be?
Dmitry Shmidt Sept. 13, 2014, 8:40 p.m. UTC | #2
On Sat, Sep 13, 2014 at 7:17 AM, Jouni Malinen <j@w1.fi> wrote:

> On Wed, Sep 03, 2014 at 02:58:37PM -0700, Dmitry Shmidt wrote:
> > New kernels in wiphy_suspend() will call cfg80211_leave_all()
> > that will eventually end up in cfg80211_stop_ap() unless
> > wowlan_triggers were set.
>
> Could you please clarify why this is needed? If kernel behavior changed
> at some point to do something that is not desired, wouldn't the correct
> way of fixing this be by reverting parts of the kernel behavior rather
> than by trying to make hostapd do something.
>

On some stage (from kernel 3.8 I think) wireless stack calls
cfg80211_leave_all()
on suspend if no wowlan triggers were registered - I think logic here was to
disconnect if system can not wake from packet anyway.
This logic was expanded to AP mode as well (in case of AP cfg80211_stop_ap()
will be called). It can also be explained - if you can not wake main cpu on
packet,
you can not be "good" AP.
So in order to be able to work as AP we need to honor wireless stack logic.
And I think that it is not a kernel bug.


>
> I don't understand what WoWLAN triggers have to do with AP mode
> operations. Or would there really be a case where WoWLAN can be used
> from a station device to wake up the AP?
>

I suspect nobody expected AP's cpu can go to suspend mode. But on embedded
system where most of simple AP operations are done in wlan FW, it makes
sense
to enable main cpu suspend.


>
> I'm assuming this is trying to undo some parts of kernel commit
> 8125696991194aacb1173b6e8196d19098b44e17 ('cfg80211/mac80211: disconnect
> on suspend'). Is that the case or did I miss what the main point of the
> hostapd changes here would be?
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap
>
Jouni Malinen Nov. 16, 2014, 5:54 p.m. UTC | #3
On Sat, Sep 13, 2014 at 01:40:17PM -0700, Dmitry Shmidt wrote:
> On some stage (from kernel 3.8 I think) wireless stack calls
> cfg80211_leave_all()
> on suspend if no wowlan triggers were registered - I think logic here was to
> disconnect if system can not wake from packet anyway.
> This logic was expanded to AP mode as well (in case of AP cfg80211_stop_ap()
> will be called). It can also be explained - if you can not wake main cpu on
> packet,
> you can not be "good" AP.
> So in order to be able to work as AP we need to honor wireless stack logic.
> And I think that it is not a kernel bug.

This is not documented very clearly today and some of the existing
parameters do not really make much sense for AP mode (and some other
parameters could be added). Anyway, I applied this now since it did not
look like the configuration interface would be cleaned up in the near
term.

I moved the wpa_common.c changes into driver_common.c which is a more
appropriate place for those functions that do not have anything to do
with WPA and that are using driver.h defined values.
diff mbox

Patch

diff --git a/hostapd/config_file.c b/hostapd/config_file.c
index be40398..4c0e3f8 100644
--- a/hostapd/config_file.c
+++ b/hostapd/config_file.c
@@ -3156,6 +3156,9 @@  static int hostapd_config_fill(struct hostapd_config *conf,
 		conf->local_pwr_constraint = val;
 	} else if (os_strcmp(buf, "spectrum_mgmt_required") == 0) {
 		conf->spectrum_mgmt_required = atoi(pos);
+	} else if (os_strcmp(buf, "wowlan_triggers") == 0) {
+		os_free(bss->wowlan_triggers);
+		bss->wowlan_triggers = os_strdup(pos);
 	} else {
 		wpa_printf(MSG_ERROR,
 			   "Line %d: unknown configuration item '%s'",
diff --git a/hostapd/main.c b/hostapd/main.c
index a9d7da5..af4d85d 100644
--- a/hostapd/main.c
+++ b/hostapd/main.c
@@ -28,6 +28,8 @@ 
 #include "eap_register.h"
 #include "ctrl_iface.h"
 
+struct wowlan_triggers *wpa_get_wowlan_triggers(const char *wowlan_triggers,
+						struct wpa_driver_capa *capa);
 
 struct hapd_global {
 	void **drv_priv;
@@ -211,12 +213,21 @@  static int hostapd_driver_init(struct hostapd_iface *iface)
 
 	if (hapd->driver->get_capa &&
 	    hapd->driver->get_capa(hapd->drv_priv, &capa) == 0) {
+		struct wowlan_triggers *triggs;
+
 		iface->drv_flags = capa.flags;
 		iface->probe_resp_offloads = capa.probe_resp_offloads;
 		iface->extended_capa = capa.extended_capa;
 		iface->extended_capa_mask = capa.extended_capa_mask;
 		iface->extended_capa_len = capa.extended_capa_len;
 		iface->drv_max_acl_mac_addrs = capa.max_acl_mac_addrs;
+
+		triggs = wpa_get_wowlan_triggers(conf->wowlan_triggers, &capa);
+		if (triggs && hapd->driver->set_wowlan) {
+			if (hapd->driver->set_wowlan(hapd->drv_priv, triggs))
+				wpa_printf(MSG_ERROR, "set_wowlan failed");
+		}
+		os_free(triggs);
 	}
 
 	return 0;
diff --git a/src/ap/ap_config.c b/src/ap/ap_config.c
index bc9f6cf..d127550 100644
--- a/src/ap/ap_config.c
+++ b/src/ap/ap_config.c
@@ -541,6 +541,8 @@  void hostapd_config_free_bss(struct hostapd_bss_config *conf)
 
 	os_free(conf->sae_groups);
 
+	os_free(conf->wowlan_triggers);
+
 	os_free(conf->server_id);
 
 	os_free(conf);
diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
index 905aec3..2858c6e 100644
--- a/src/ap/ap_config.h
+++ b/src/ap/ap_config.h
@@ -508,6 +508,8 @@  struct hostapd_bss_config {
 	unsigned int sae_anti_clogging_threshold;
 	int *sae_groups;
 
+	char *wowlan_triggers; /* Wake-on-WLAN triggers */
+
 #ifdef CONFIG_TESTING_OPTIONS
 	u8 bss_load_test[5];
 	u8 bss_load_test_set;
diff --git a/src/common/wpa_common.c b/src/common/wpa_common.c
index 7aeb706..998a51a 100644
--- a/src/common/wpa_common.c
+++ b/src/common/wpa_common.c
@@ -14,6 +14,7 @@ 
 #include "crypto/sha256.h"
 #include "crypto/aes_wrap.h"
 #include "crypto/crypto.h"
+#include "drivers/driver.h"
 #include "ieee802_11_defs.h"
 #include "defs.h"
 #include "wpa_common.h"
@@ -1496,3 +1497,78 @@  int wpa_select_ap_group_cipher(int wpa, int wpa_pairwise, int rsn_pairwise)
 		return WPA_CIPHER_CCMP_256;
 	return WPA_CIPHER_CCMP;
 }
+
+
+static int wpa_check_wowlan_trigger(const char *start, const char *trigger,
+				    int capa_trigger, u8 *param_trigger)
+{
+	if (os_strcmp(start, trigger) != 0)
+		return 0;
+	if (!capa_trigger)
+		return 0;
+
+	*param_trigger = 1;
+	return 1;
+}
+
+
+struct wowlan_triggers *wpa_get_wowlan_triggers(const char *wowlan_triggers,
+						struct wpa_driver_capa *capa)
+{
+	struct wowlan_triggers *triggers;
+	char *start, *end, *buf;
+	int last;
+
+	if (!wowlan_triggers)
+		return NULL;
+
+	buf = os_strdup(wowlan_triggers);
+	if (buf == NULL)
+		return NULL;
+
+	triggers = os_zalloc(sizeof(*triggers));
+	if (triggers == NULL)
+		goto out;
+
+#define CHECK_TRIGGER(trigger) \
+	wpa_check_wowlan_trigger(start, #trigger,			\
+				  capa->wowlan_triggers.trigger,	\
+				  &triggers->trigger)
+
+	start = buf;
+	while (*start != '\0') {
+		while (isblank(*start))
+			start++;
+		if (*start == '\0')
+			break;
+		end = start;
+		while (!isblank(*end) && *end != '\0')
+			end++;
+		last = *end == '\0';
+		*end = '\0';
+
+		if (!CHECK_TRIGGER(any) &&
+		    !CHECK_TRIGGER(disconnect) &&
+		    !CHECK_TRIGGER(magic_pkt) &&
+		    !CHECK_TRIGGER(gtk_rekey_failure) &&
+		    !CHECK_TRIGGER(eap_identity_req) &&
+		    !CHECK_TRIGGER(four_way_handshake) &&
+		    !CHECK_TRIGGER(rfkill_release)) {
+			wpa_printf(MSG_DEBUG,
+				   "Unknown/unsupported wowlan trigger '%s'",
+				   start);
+			os_free(triggers);
+			triggers = NULL;
+			goto out;
+		}
+
+		if (last)
+			break;
+		start = end + 1;
+	}
+#undef CHECK_TRIGGER
+
+out:
+	os_free(buf);
+	return triggers;
+}
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 9414e8f..f20bc62 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -105,6 +105,9 @@  const char *wpa_supplicant_full_license5 =
 "\n";
 #endif /* CONFIG_NO_STDOUT_DEBUG */
 
+struct wowlan_triggers *wpa_get_wowlan_triggers(const char *wowlan_triggers,
+						struct wpa_driver_capa *capa);
+
 /* Configure default/group WEP keys for static WEP */
 int wpa_set_wep_keys(struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid)
 {
@@ -3149,75 +3152,20 @@  int wpas_init_ext_pw(struct wpa_supplicant *wpa_s)
 }
 
 
-static int wpas_check_wowlan_trigger(const char *start, const char *trigger,
-				     int capa_trigger, u8 *param_trigger)
-{
-	if (os_strcmp(start, trigger) != 0)
-		return 0;
-	if (!capa_trigger)
-		return 0;
-
-	*param_trigger = 1;
-	return 1;
-}
-
-
 static int wpas_set_wowlan_triggers(struct wpa_supplicant *wpa_s,
 				    struct wpa_driver_capa *capa)
 {
-	struct wowlan_triggers triggers;
-	char *start, *end, *buf;
-	int last, ret;
+	struct wowlan_triggers *triggers;
+	int ret = 0;
 
 	if (!wpa_s->conf->wowlan_triggers)
 		return 0;
 
-	buf = os_strdup(wpa_s->conf->wowlan_triggers);
-	if (buf == NULL)
-		return -1;
-
-	os_memset(&triggers, 0, sizeof(triggers));
-
-#define CHECK_TRIGGER(trigger) \
-	wpas_check_wowlan_trigger(start, #trigger,			\
-				  capa->wowlan_triggers.trigger,	\
-				  &triggers.trigger)
-
-	start = buf;
-	while (*start != '\0') {
-		while (isblank(*start))
-			start++;
-		if (*start == '\0')
-			break;
-		end = start;
-		while (!isblank(*end) && *end != '\0')
-			end++;
-		last = *end == '\0';
-		*end = '\0';
-
-		if (!CHECK_TRIGGER(any) &&
-		    !CHECK_TRIGGER(disconnect) &&
-		    !CHECK_TRIGGER(magic_pkt) &&
-		    !CHECK_TRIGGER(gtk_rekey_failure) &&
-		    !CHECK_TRIGGER(eap_identity_req) &&
-		    !CHECK_TRIGGER(four_way_handshake) &&
-		    !CHECK_TRIGGER(rfkill_release)) {
-			wpa_printf(MSG_DEBUG,
-				   "Unknown/unsupported wowlan trigger '%s'",
-				   start);
-			ret = -1;
-			goto out;
-		}
-
-		if (last)
-			break;
-		start = end + 1;
+	triggers = wpa_get_wowlan_triggers(wpa_s->conf->wowlan_triggers, capa);
+	if (triggers) {
+		ret = wpa_drv_wowlan(wpa_s, triggers);
+		os_free(triggers);
 	}
-#undef CHECK_TRIGGER
-
-	ret = wpa_drv_wowlan(wpa_s, &triggers);
-out:
-	os_free(buf);
 	return ret;
 }