diff mbox series

[2/3] wcn36xx: Enable firmware link monitoring

Message ID 20201031022311.1677337-3-bryan.odonoghue@linaro.org (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series wcn36xx: Firmware link monitor/keepalive offload | expand

Commit Message

Bryan O'Donoghue Oct. 31, 2020, 2:23 a.m. UTC
This patch switches on CONNECTION_MONITOR. Once done it is up to the
firmware to send keep alive and to monitor the link state.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Loic Poulain Oct. 31, 2020, 9:57 a.m. UTC | #1
On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> This patch switches on CONNECTION_MONITOR. Once done it is up to the
> firmware to send keep alive and to monitor the link state.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/main.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index 706728fba72d..e924cc4acde0 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -1246,6 +1246,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>         ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
>         ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
>         ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
> +       ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);

The problem could be that when connection monitor is enabled, mac80211
stop sending regular null/probe packet to the AP (as expected), but
also stop monitoring beacon miss:
https://elixir.bootlin.com/linux/v5.10-rc1/source/net/mac80211/mlme.c#L115

That's not a big problem, but that would mean that in active mode
(power_save disabled, non PS), the mac80211 will not detect if the AP
has left immediately, and in worst case, only after 30 seconds. Note
that in PS mode, beacon monitoring is well done by the firmware.

>
>         wcn->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
>                 BIT(NL80211_IFTYPE_AP) |
> --
> 2.28.0
>
Bryan O'Donoghue Oct. 31, 2020, 1:02 p.m. UTC | #2
On 31/10/2020 09:57, Loic Poulain wrote:
> On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> This patch switches on CONNECTION_MONITOR. Once done it is up to the
>> firmware to send keep alive and to monitor the link state.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   drivers/net/wireless/ath/wcn36xx/main.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
>> index 706728fba72d..e924cc4acde0 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> @@ -1246,6 +1246,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>>          ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
>>          ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
>>          ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
>> +       ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);
> 
> The problem could be that when connection monitor is enabled, mac80211
> stop sending regular null/probe packet to the AP (as expected), but
> also stop monitoring beacon miss:
> https://elixir.bootlin.com/linux/v5.10-rc1/source/net/mac80211/mlme.c#L115
> 
> That's not a big problem, but that would mean that in active mode
> (power_save disabled, non PS), the mac80211 will not detect if the AP
> has left immediately, and in worst case, only after 30 seconds. Note
> that in PS mode, beacon monitoring is well done by the firmware.
> 

If you pull the plug out of the AP it can take up to 30 seconds to see 
it agreed.

On the flip side, the amount of NULL data packets produced drops off 
significantly once we delegate this completely to the firmware.

IMO you gain more by reducing the regular runtime noise than you loose 
with the timing out of an gone away AP.
Loic Poulain Nov. 2, 2020, 7:15 p.m. UTC | #3
On Sat, 31 Oct 2020 at 14:01, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 31/10/2020 09:57, Loic Poulain wrote:
> > On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
> > <bryan.odonoghue@linaro.org> wrote:
> >>
> >> This patch switches on CONNECTION_MONITOR. Once done it is up to the
> >> firmware to send keep alive and to monitor the link state.
> >>
> >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> >> ---
> >>   drivers/net/wireless/ath/wcn36xx/main.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> >> index 706728fba72d..e924cc4acde0 100644
> >> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> >> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> >> @@ -1246,6 +1246,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
> >>          ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
> >>          ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
> >>          ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
> >> +       ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);
> >
> > The problem could be that when connection monitor is enabled, mac80211
> > stop sending regular null/probe packet to the AP (as expected), but
> > also stop monitoring beacon miss:
> > https://elixir.bootlin.com/linux/v5.10-rc1/source/net/mac80211/mlme.c#L115
> >
> > That's not a big problem, but that would mean that in active mode
> > (power_save disabled, non PS), the mac80211 will not detect if the AP
> > has left immediately, and in worst case, only after 30 seconds. Note
> > that in PS mode, beacon monitoring is well done by the firmware.
> >
>
> If you pull the plug out of the AP it can take up to 30 seconds to see
> it agreed.
>
> On the flip side, the amount of NULL data packets produced drops off
> significantly once we delegate this completely to the firmware.
>
> IMO you gain more by reducing the regular runtime noise than you loose
> with the timing out of an gone away AP.

OK, fair enough!


Loic
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 706728fba72d..e924cc4acde0 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1246,6 +1246,7 @@  static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
 	ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
 	ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
 	ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
+	ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);
 
 	wcn->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
 		BIT(NL80211_IFTYPE_AP) |