linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] brcmfmac: Configure keep-alive packet on suspend
@ 2021-11-22  9:04 Loic Poulain
  2021-11-22 11:01 ` Kalle Valo
  2021-11-22 14:45 ` kernel test robot
  0 siblings, 2 replies; 6+ messages in thread
From: Loic Poulain @ 2021-11-22  9:04 UTC (permalink / raw)
  To: aspriel, franky.lin, hante.meuleman, chi-hsien.lin, wright.feng,
	chung-hsien.hsu
  Cc: linux-wireless, brcm80211-dev-list.pdl, kvalo, Loic Poulain

When system enter suspend, there is no more wireless traffic, and
if there is no incoming data, most of the AP kick-out the client
station after few minutes because of inactivity.

The usual way to prevent this is to submit a Null function frame
periodically as a keep-alive. This is supported by brcm controllers
and can be configured via the mkeep_alive IOVAR.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 .../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..afa8ceda 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;
 }
 
+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_ */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] brcmfmac: Configure keep-alive packet on suspend
  2021-11-22  9:04 [PATCH] brcmfmac: Configure keep-alive packet on suspend Loic Poulain
@ 2021-11-22 11:01 ` Kalle Valo
  2021-11-22 12:05   ` Loic Poulain
  2021-11-22 14:45 ` kernel test robot
  1 sibling, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2021-11-22 11:01 UTC (permalink / raw)
  To: Loic Poulain
  Cc: aspriel, franky.lin, hante.meuleman, chi-hsien.lin, wright.feng,
	chung-hsien.hsu, linux-wireless, brcm80211-dev-list.pdl

Loic Poulain <loic.poulain@linaro.org> writes:

> When system enter suspend, there is no more wireless traffic, and
> if there is no incoming data, most of the AP kick-out the client
> station after few minutes because of inactivity.
>
> The usual way to prevent this is to submit a Null function frame
> periodically as a keep-alive. This is supported by brcm controllers
> and can be configured via the mkeep_alive IOVAR.

This is with brcmfmac in client mode, right? Wouldn't it make more sense
to disconnect entirely during suspend? Nobody is processing the data
packets anyway during suspend.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] brcmfmac: Configure keep-alive packet on suspend
  2021-11-22 11:01 ` Kalle Valo
@ 2021-11-22 12:05   ` Loic Poulain
  2021-11-22 14:23     ` Kalle Valo
  0 siblings, 1 reply; 6+ messages in thread
From: Loic Poulain @ 2021-11-22 12:05 UTC (permalink / raw)
  To: Kalle Valo
  Cc: aspriel, franky.lin, hante.meuleman, chi-hsien.lin, wright.feng,
	chung-hsien.hsu, linux-wireless, brcm80211-dev-list.pdl

Hi Kalle,

On Mon, 22 Nov 2021 at 12:01, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Loic Poulain <loic.poulain@linaro.org> writes:
>
> > When system enter suspend, there is no more wireless traffic, and
> > if there is no incoming data, most of the AP kick-out the client
> > station after few minutes because of inactivity.
> >
> > The usual way to prevent this is to submit a Null function frame
> > periodically as a keep-alive. This is supported by brcm controllers
> > and can be configured via the mkeep_alive IOVAR.
>
> This is with brcmfmac in client mode, right?

Right, it's in client mode.

> Wouldn't it make more sense to disconnect entirely during suspend?
> Nobody is processing the data packets anyway during suspend.

Disconnect is performed automatically when wowlan is not enabled,
otherwise we may want to wake-up on events (disconnect,
4-way-handshake) or data packets (magic, unicast, etc...). Some
devices use suspend aggressively such as Android in which the network
link is expected to be maintained.

Regards,
Loic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] brcmfmac: Configure keep-alive packet on suspend
  2021-11-22 12:05   ` Loic Poulain
@ 2021-11-22 14:23     ` Kalle Valo
  2021-11-22 15:43       ` Loic Poulain
  0 siblings, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2021-11-22 14:23 UTC (permalink / raw)
  To: Loic Poulain
  Cc: aspriel, franky.lin, hante.meuleman, chi-hsien.lin, wright.feng,
	chung-hsien.hsu, linux-wireless, brcm80211-dev-list.pdl

Loic Poulain <loic.poulain@linaro.org> writes:

> Hi Kalle,
>
> On Mon, 22 Nov 2021 at 12:01, Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Loic Poulain <loic.poulain@linaro.org> writes:
>>
>> > When system enter suspend, there is no more wireless traffic, and
>> > if there is no incoming data, most of the AP kick-out the client
>> > station after few minutes because of inactivity.
>> >
>> > The usual way to prevent this is to submit a Null function frame
>> > periodically as a keep-alive. This is supported by brcm controllers
>> > and can be configured via the mkeep_alive IOVAR.
>>
>> This is with brcmfmac in client mode, right?
>
> Right, it's in client mode.
>
>> Wouldn't it make more sense to disconnect entirely during suspend?
>> Nobody is processing the data packets anyway during suspend.
>
> Disconnect is performed automatically when wowlan is not enabled,
> otherwise we may want to wake-up on events (disconnect,
> 4-way-handshake) or data packets (magic, unicast, etc...). Some
> devices use suspend aggressively such as Android in which the network
> link is expected to be maintained.

Sure, for wowlan it makes sense but you didn't mention that in the
commit log so I assumed that was disabled.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] brcmfmac: Configure keep-alive packet on suspend
  2021-11-22  9:04 [PATCH] brcmfmac: Configure keep-alive packet on suspend Loic Poulain
  2021-11-22 11:01 ` Kalle Valo
@ 2021-11-22 14:45 ` kernel test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2021-11-22 14:45 UTC (permalink / raw)
  To: Loic Poulain, aspriel, franky.lin, hante.meuleman, chi-hsien.lin,
	wright.feng, chung-hsien.hsu
  Cc: llvm, kbuild-all, linux-wireless, brcm80211-dev-list.pdl, kvalo,
	Loic Poulain

[-- Attachment #1: Type: text/plain, Size: 3099 bytes --]

Hi Loic,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kvalo-wireless-drivers-next/master]
[also build test WARNING on kvalo-wireless-drivers/master v5.16-rc2 next-20211118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Loic-Poulain/brcmfmac-Configure-keep-alive-packet-on-suspend/20211122-165520
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: riscv-randconfig-r002-20211122 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c133fb321f7ca6083ce15b6aa5bf89de6600e649)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/13a67e895024e7cdbf0faa409fe3349cb0d741fc
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Loic-Poulain/brcmfmac-Configure-keep-alive-packet-on-suspend/20211122-165520
        git checkout 13a67e895024e7cdbf0faa409fe3349cb0d741fc
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:3904:5: warning: no previous prototype for function 'brcmf_keepalive_start' [-Wmissing-prototypes]
   int brcmf_keepalive_start(struct brcmf_if *ifp, unsigned int interval)
       ^
   drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:3904:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int brcmf_keepalive_start(struct brcmf_if *ifp, unsigned int interval)
   ^
   static 
   1 warning generated.


vim +/brcmf_keepalive_start +3904 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c

  3903	
> 3904	int brcmf_keepalive_start(struct brcmf_if *ifp, unsigned int interval)
  3905	{
  3906		struct brcmf_mkeep_alive_pkt_le kalive = {0};
  3907		int ret = 0;
  3908	
  3909		/* Configure Null function/data keepalive */
  3910		kalive.version = cpu_to_le16(1);
  3911		kalive.period_msec = cpu_to_le16(interval * MSEC_PER_SEC);
  3912		kalive.len_bytes = cpu_to_le16(0);
  3913		kalive.keep_alive_id = cpu_to_le16(0);
  3914	
  3915		ret = brcmf_fil_iovar_data_set(ifp, "mkeep_alive", &kalive, sizeof(kalive));
  3916		if (ret)
  3917			brcmf_err("keep-alive packet config failed, ret=%d\n", ret);
  3918	
  3919		return ret;
  3920	}
  3921	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36248 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] brcmfmac: Configure keep-alive packet on suspend
  2021-11-22 14:23     ` Kalle Valo
@ 2021-11-22 15:43       ` Loic Poulain
  0 siblings, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2021-11-22 15:43 UTC (permalink / raw)
  To: Kalle Valo
  Cc: aspriel, franky.lin, hante.meuleman, chi-hsien.lin, wright.feng,
	chung-hsien.hsu, linux-wireless, brcm80211-dev-list.pdl

On Mon, 22 Nov 2021 at 15:23, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Loic Poulain <loic.poulain@linaro.org> writes:
>
> > Hi Kalle,
> >
> > On Mon, 22 Nov 2021 at 12:01, Kalle Valo <kvalo@codeaurora.org> wrote:
> >>
> >> Loic Poulain <loic.poulain@linaro.org> writes:
> >>
> >> > When system enter suspend, there is no more wireless traffic, and
> >> > if there is no incoming data, most of the AP kick-out the client
> >> > station after few minutes because of inactivity.
> >> >
> >> > The usual way to prevent this is to submit a Null function frame
> >> > periodically as a keep-alive. This is supported by brcm controllers
> >> > and can be configured via the mkeep_alive IOVAR.
> >>
> >> This is with brcmfmac in client mode, right?
> >
> > Right, it's in client mode.
> >
> >> Wouldn't it make more sense to disconnect entirely during suspend?
> >> Nobody is processing the data packets anyway during suspend.
> >
> > Disconnect is performed automatically when wowlan is not enabled,
> > otherwise we may want to wake-up on events (disconnect,
> > 4-way-handshake) or data packets (magic, unicast, etc...). Some
> > devices use suspend aggressively such as Android in which the network
> > link is expected to be maintained.
>
> Sure, for wowlan it makes sense but you didn't mention that in the
> commit log so I assumed that was disabled.

Ah right, I'll do that in V2.

Thanks,
Loic

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-22 15:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-22  9:04 [PATCH] brcmfmac: Configure keep-alive packet on suspend Loic Poulain
2021-11-22 11:01 ` Kalle Valo
2021-11-22 12:05   ` Loic Poulain
2021-11-22 14:23     ` Kalle Valo
2021-11-22 15:43       ` Loic Poulain
2021-11-22 14:45 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).