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 <[email protected]>
---
.../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
Loic Poulain <[email protected]> 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
Hi Kalle,
On Mon, 22 Nov 2021 at 12:01, Kalle Valo <[email protected]> wrote:
>
> Loic Poulain <[email protected]> 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
Loic Poulain <[email protected]> writes:
> Hi Kalle,
>
> On Mon, 22 Nov 2021 at 12:01, Kalle Valo <[email protected]> wrote:
>>
>> Loic Poulain <[email protected]> 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
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 <[email protected]>
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/[email protected]
On Mon, 22 Nov 2021 at 15:23, Kalle Valo <[email protected]> wrote:
>
> Loic Poulain <[email protected]> writes:
>
> > Hi Kalle,
> >
> > On Mon, 22 Nov 2021 at 12:01, Kalle Valo <[email protected]> wrote:
> >>
> >> Loic Poulain <[email protected]> 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