diff mbox series

[v2] brcmfmac: Configure keep-alive packet on suspend

Message ID 1637596046-21651-1-git-send-email-loic.poulain@linaro.org (mailing list archive)
State Accepted
Commit 7a6cfe28ae3ef6a78774fd1e21e3b76c90937112
Delegated to: Kalle Valo
Headers show
Series [v2] brcmfmac: Configure keep-alive packet on suspend | expand

Commit Message

Loic Poulain Nov. 22, 2021, 3:47 p.m. UTC
When entering suspend as a client station with wowlan enabled,
the Wi-Fi link is supposed to be maintained. In that state, no
more data is generated from client side, and the link stays idle
as long the station is suspended and as long the AP as no data to
transmit.

However, most of the APs kick-off such 'inactive' stations after
few minutes, causing unexpected disconnect (reconnect, etc...).

The usual way to prevent this is to submit a Null function frame
periodically as a keep-alive. This is something that can be host
/software generated (e.g. wpa_supplicant), but that needs to be
offloaded to the Wi-Fi controller in case of suspended host.

This change enables firmware generated keep-alive frames when
entering wowlan suspend, using the 'mkeep_alive' IOVAR.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 v2: - commit reword metioning wowlan
     - brcmf_keepalive_start as static function

 .../wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 21 +++++++++++++++++++++
 .../broadcom/brcm80211/brcmfmac/fwil_types.h        | 19 +++++++++++++++++++
 2 files changed, 40 insertions(+)

Comments

Alvin Šipraga Nov. 23, 2021, 2:03 p.m. UTC | #1
Hi Loic,

This seems like a good change. I left some comments below.

On 11/22/21 16:47, Loic Poulain wrote:
> When entering suspend as a client station with wowlan enabled,
> the Wi-Fi link is supposed to be maintained. In that state, no
> more data is generated from client side, and the link stays idle
> as long the station is suspended and as long the AP as no data to
> transmit.
> 
> However, most of the APs kick-off such 'inactive' stations after
> few minutes, causing unexpected disconnect (reconnect, etc...).
> 
> The usual way to prevent this is to submit a Null function frame
> periodically as a keep-alive. This is something that can be host
> /software generated (e.g. wpa_supplicant), but that needs to be
> offloaded to the Wi-Fi controller in case of suspended host.
> 
> This change enables firmware generated keep-alive frames when
> entering wowlan suspend, using the 'mkeep_alive' IOVAR.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>   v2: - commit reword metioning wowlan
>       - brcmf_keepalive_start as static function
> 
>   .../wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 21 +++++++++++++++++++++
>   .../broadcom/brcm80211/brcmfmac/fwil_types.h        | 19 +++++++++++++++++++
>   2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index fb72777..1679361 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -3901,6 +3901,24 @@ static void brcmf_configure_wowl(struct brcmf_cfg80211_info *cfg,
>   	cfg->wowl.active = true;
>   }
>   
> +static int brcmf_keepalive_start(struct brcmf_if *ifp, unsigned int interval)
> +{
> +	struct brcmf_mkeep_alive_pkt_le kalive = {0};
> +	int ret = 0;
> +
> +	/* Configure Null function/data keepalive */
> +	kalive.version = cpu_to_le16(1);
> +	kalive.period_msec = cpu_to_le16(interval * MSEC_PER_SEC);
> +	kalive.len_bytes = cpu_to_le16(0);
> +	kalive.keep_alive_id = cpu_to_le16(0);
> +
> +	ret = brcmf_fil_iovar_data_set(ifp, "mkeep_alive", &kalive, sizeof(kalive));
> +	if (ret)
> +		brcmf_err("keep-alive packet config failed, ret=%d\n", ret);
> +
> +	return ret;
> +}
> +
>   static s32 brcmf_cfg80211_suspend(struct wiphy *wiphy,
>   				  struct cfg80211_wowlan *wowl)
>   {
> @@ -3947,6 +3965,9 @@ static s32 brcmf_cfg80211_suspend(struct wiphy *wiphy,
>   	} else {
>   		/* Configure WOWL paramaters */
>   		brcmf_configure_wowl(cfg, ifp, wowl);
> +
> +		/* Prevent disassociation due to inactivity with keep-alive */
> +		brcmf_keepalive_start(ifp, 30);

Do you also want to disable it on resume?

There is also the wowl_keepalive IOVAR which might be more suitable:

wowl_keepalive
         Send specified keep alive packet periodically in w mode.
         Usage: wl wowl_keepalive <index0-1> <period> <packet>
                 index: 0 - 1.
                 period: Re-transmission period in milli-seconds. 0 to 
disable packet transmits.
                 packet: Hex packet contents to transmit. The packet 
contents should include the entire ethernet packet (ethernet header, IP 
header, UDP header, and UDP payload) specified in network byte order.

         e.g. Send keep alive packet every 30 seconds using id-1:
         wl wowl_keepalive 1 30000 
0x0014a54b164f000f66f45b7e08004500001e000040004011c52a0a8830700a88302513c413c4000a00000a0d

In that case you probably don't have to unset it either, or worry about 
other users of mkeep_alive.

Kind regards,

	Alvin
Kalle Valo Nov. 29, 2021, 10:25 a.m. UTC | #2
Loic Poulain <loic.poulain@linaro.org> wrote:

> When entering suspend as a client station with wowlan enabled,
> the Wi-Fi link is supposed to be maintained. In that state, no
> more data is generated from client side, and the link stays idle
> as long the station is suspended and as long the AP as no data to
> transmit.
> 
> However, most of the APs kick-off such 'inactive' stations after
> few minutes, causing unexpected disconnect (reconnect, etc...).
> 
> The usual way to prevent this is to submit a Null function frame
> periodically as a keep-alive. This is something that can be host
> /software generated (e.g. wpa_supplicant), but that needs to be
> offloaded to the Wi-Fi controller in case of suspended host.
> 
> This change enables firmware generated keep-alive frames when
> entering wowlan suspend, using the 'mkeep_alive' IOVAR.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

Patch applied to wireless-drivers-next.git, thanks.

7a6cfe28ae3e brcmfmac: Configure keep-alive packet on suspend
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index fb72777..1679361 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3901,6 +3901,24 @@  static void brcmf_configure_wowl(struct brcmf_cfg80211_info *cfg,
 	cfg->wowl.active = true;
 }
 
+static int brcmf_keepalive_start(struct brcmf_if *ifp, unsigned int interval)
+{
+	struct brcmf_mkeep_alive_pkt_le kalive = {0};
+	int ret = 0;
+
+	/* Configure Null function/data keepalive */
+	kalive.version = cpu_to_le16(1);
+	kalive.period_msec = cpu_to_le16(interval * MSEC_PER_SEC);
+	kalive.len_bytes = cpu_to_le16(0);
+	kalive.keep_alive_id = cpu_to_le16(0);
+
+	ret = brcmf_fil_iovar_data_set(ifp, "mkeep_alive", &kalive, sizeof(kalive));
+	if (ret)
+		brcmf_err("keep-alive packet config failed, ret=%d\n", ret);
+
+	return ret;
+}
+
 static s32 brcmf_cfg80211_suspend(struct wiphy *wiphy,
 				  struct cfg80211_wowlan *wowl)
 {
@@ -3947,6 +3965,9 @@  static s32 brcmf_cfg80211_suspend(struct wiphy *wiphy,
 	} else {
 		/* Configure WOWL paramaters */
 		brcmf_configure_wowl(cfg, ifp, wowl);
+
+		/* Prevent disassociation due to inactivity with keep-alive */
+		brcmf_keepalive_start(ifp, 30);
 	}
 
 exit:
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index ff2ef55..e69d1e5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -1052,4 +1052,23 @@  struct brcmf_gscan_config {
 	struct brcmf_gscan_bucket_config bucket[1];
 };
 
+/**
+ * struct brcmf_mkeep_alive_pkt_le - configuration data for keep-alive frame.
+ *
+ * @version: version for mkeep_alive
+ * @length: length of fixed parameters in the structure.
+ * @period_msec: keep-alive period in milliseconds.
+ * @len_bytes: size of the data.
+ * @keep_alive_id: ID  (0 - 3).
+ * @data: keep-alive frame data.
+ */
+struct brcmf_mkeep_alive_pkt_le {
+	__le16  version;
+	__le16  length;
+	__le32  period_msec;
+	__le16  len_bytes;
+	u8   keep_alive_id;
+	u8   data[0];
+} __packed;
+
 #endif /* FWIL_TYPES_H_ */