diff mbox series

mt76: Enable NL80211_EXT_FEATURE_CQM_RSSI_LIST

Message ID 20180812145207.11395-1-kristian.evensen@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series mt76: Enable NL80211_EXT_FEATURE_CQM_RSSI_LIST | expand

Commit Message

Kristian Evensen Aug. 12, 2018, 2:52 p.m. UTC
Enable the use of CQM with mt76-devices.

Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
---
 drivers/net/wireless/mediatek/mt76/mac80211.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Kalle Valo Aug. 12, 2018, 6:14 p.m. UTC | #1
Kristian Evensen <kristian.evensen@gmail.com> writes:

> Enable the use of CQM with mt76-devices.
>
> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mac80211.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
> index 029d54bc..3eb328ff 100644
> --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
> +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
> @@ -305,6 +305,8 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
>  
>  	wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR;
>  
> +	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);

So have you tested this and with what devices? For example, does it work
with recently added USB devices?
Arend van Spriel Aug. 12, 2018, 6:44 p.m. UTC | #2
On 8/12/2018 8:14 PM, Kalle Valo wrote:
> Kristian Evensen <kristian.evensen@gmail.com> writes:
>
>> Enable the use of CQM with mt76-devices.
>>
>> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
>> ---
>>   drivers/net/wireless/mediatek/mt76/mac80211.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
>> index 029d54bc..3eb328ff 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
>> @@ -305,6 +305,8 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
>>
>>   	wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR;
>>
>> +	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>
> So have you tested this and with what devices? For example, does it work
> with recently added USB devices?

I was looking into this as it looks suspicious to me. From reading the 
description of this ext_feature flag it seems this is an extention of CQM:

"""
  * @NL80211_EXT_FEATURE_CQM_RSSI_LIST: With this driver the
  *	%NL80211_ATTR_CQM_RSSI_THOLD attribute accepts a list of zero or more
  *	RSSI threshold values to monitor rather than exactly one threshold.
"""

Also looking at mt76x2_bss_info_changed() it does not handle 
BSS_CHANGED_CQM so I doubt it has support for it (yet). The driver does 
not use IEEE80211_VIF_SUPPORTS_CQM_RSSI which is a requirement for it.

Regards,
Arend


https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/mediatek/mt76/mt76x2_main.c#L223
Kristian Evensen Aug. 13, 2018, 4:58 a.m. UTC | #3
Hi Kalle & Arnd,
On Sun, Aug 12, 2018 at 8:44 PM Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> > So have you tested this and with what devices? For example, does it work
> > with recently added USB devices?
>
> I was looking into this as it looks suspicious to me. From reading the
> description of this ext_feature flag it seems this is an extention of CQM:

Thank you very much for your feedback. My commit message should have
been more detailed, sorry about that. I have checked that the flag
works as intended with mt7602-, mt7603- and mt7612-based wifi cards. I
have not had the opportunity to test with any of the recently added
USB devices, as I don't have access to any of those.

In order to test the flag, I wrote a small program which subscribes to
the CQM-multicast group, passes an RSSI threshold-list to the kernel
and logs the received CQM-events. I then disconnected and connected
the wifi-antennas of the different cards. My threshold list was {-70,
-60, -50, -40} and while unscrewing the antenna I received multiple
below-events. When I attached the antenna again, I received multiple
above-events. As an example, here is the log when I tested with mt7612
(singal level when starting was ~-48 dBm):

Requested nl80211 generic netlink id
nl80211 has generic netlink id: 23
mlme ID is 5
Added socket to mlme group
Sent NL80211_CMD_SET_CQM
No error
Wifi (idx 18) went below threshold. RSSI -52
Wifi (idx 18) went above threshold. RSSI -49
Wifi (idx 18) went below threshold. RSSI -52
Wifi (idx 18) went below threshold. RSSI -62
Wifi (idx 18) went above threshold. RSSI -59
Wifi (idx 18) went above threshold. RSSI -49

Based on how I interpret the output and my understanding of how CQM +
RSSI_LIST works, this output shows that mt76 works fine with
NL80211_EXT_FEATURE_CQM_RSSI_LIST (at least for my cards). The list
was interpreted and handled correctly, as I received events when the
RSSI passed different thresholds in my list (-50, -60).

BR,
Kristian
Lorenzo Bianconi Aug. 13, 2018, 10:55 a.m. UTC | #4
> On 8/12/2018 8:14 PM, Kalle Valo wrote:
> > Kristian Evensen <kristian.evensen@gmail.com> writes:
> >
> >> Enable the use of CQM with mt76-devices.
> >>
> >> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
> >> ---
> >>   drivers/net/wireless/mediatek/mt76/mac80211.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
> >> index 029d54bc..3eb328ff 100644
> >> --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
> >> +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
> >> @@ -305,6 +305,8 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
> >>
> >>      wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR;
> >>
> >> +    wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
> >
> > So have you tested this and with what devices? For example, does it work
> > with recently added USB devices?
>
> I was looking into this as it looks suspicious to me. From reading the
> description of this ext_feature flag it seems this is an extention of CQM:
>
> """
>   * @NL80211_EXT_FEATURE_CQM_RSSI_LIST: With this driver the
>   *     %NL80211_ATTR_CQM_RSSI_THOLD attribute accepts a list of zero or more
>   *     RSSI threshold values to monitor rather than exactly one threshold.
> """
>
> Also looking at mt76x2_bss_info_changed() it does not handle
> BSS_CHANGED_CQM so I doubt it has support for it (yet). The driver does
> not use IEEE80211_VIF_SUPPORTS_CQM_RSSI which is a requirement for it.
>
> Regards,
> Arend
>

According to my understanding (please correct me if I am wrong)
BSS_CHANGED_CQM is only needed if CQM_RSSI is handled
by the driver/fw, while if it is not set mac80211 will take care of that
in ieee80211_handle_beacon_sig routine.
I am AFK at the moment, I will test that patch when I am back from vacations.

Regards,
Lorenzo

>
> https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/mediatek/mt76/mt76x2_main.c#L223
>
Arend van Spriel Aug. 13, 2018, 12:39 p.m. UTC | #5
On 8/13/2018 12:55 PM, Lorenzo Bianconi wrote:
>> On 8/12/2018 8:14 PM, Kalle Valo wrote:
>>> Kristian Evensen <kristian.evensen@gmail.com> writes:
>>>
>>>> Enable the use of CQM with mt76-devices.
>>>>
>>>> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
>>>> ---
>>>>    drivers/net/wireless/mediatek/mt76/mac80211.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> index 029d54bc..3eb328ff 100644
>>>> --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> @@ -305,6 +305,8 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
>>>>
>>>>       wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR;
>>>>
>>>> +    wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>>>
>>> So have you tested this and with what devices? For example, does it work
>>> with recently added USB devices?
>>
>> I was looking into this as it looks suspicious to me. From reading the
>> description of this ext_feature flag it seems this is an extention of CQM:
>>
>> """
>>    * @NL80211_EXT_FEATURE_CQM_RSSI_LIST: With this driver the
>>    *     %NL80211_ATTR_CQM_RSSI_THOLD attribute accepts a list of zero or more
>>    *     RSSI threshold values to monitor rather than exactly one threshold.
>> """
>>
>> Also looking at mt76x2_bss_info_changed() it does not handle
>> BSS_CHANGED_CQM so I doubt it has support for it (yet). The driver does
>> not use IEEE80211_VIF_SUPPORTS_CQM_RSSI which is a requirement for it.
>>
>> Regards,
>> Arend
>>
>
> According to my understanding (please correct me if I am wrong)
> BSS_CHANGED_CQM is only needed if CQM_RSSI is handled
> by the driver/fw, while if it is not set mac80211 will take care of that
> in ieee80211_handle_beacon_sig routine.

Yeah. That explains it. Seems like mac80211 could actually set the 
NL80211_EXT_FEATURE_CQM_RSSI_LIST flag is the driver does not set 
IEEE80211_VIF_SUPPORTS_CQM_RSSI. That way all mac80211 drivers support 
the list. Just a problem as the ext_feature is not a per-vif flag.

Regards,
Arend
Kristian Evensen Aug. 13, 2018, 2:25 p.m. UTC | #6
Hi Lorenzo,

On Mon, Aug 13, 2018 at 12:55 PM Lorenzo Bianconi
<lorenzo.bianconi@redhat.com> wrote:
> According to my understanding (please correct me if I am wrong)
> BSS_CHANGED_CQM is only needed if CQM_RSSI is handled
> by the driver/fw, while if it is not set mac80211 will take care of that
> in ieee80211_handle_beacon_sig routine.
> I am AFK at the moment, I will test that patch when I am back from vacations.

That matches my understanding as well (base on for example the message
for commit ae44b502669d0cd1f167cdb48994292aa20fd3dd).

Great that you are willing to test the patch with one of the mt76
USB-dongles. Unless anyone objects, I will postpone sending a v2 until
you report back with your findings :)

BR,
Kristian
Andrew Zaborowski Aug. 18, 2018, 2:28 a.m. UTC | #7
Hi,

On 13 August 2018 at 16:25, Kristian Evensen <kristian.evensen@gmail.com> wrote:
> On Mon, Aug 13, 2018 at 12:55 PM Lorenzo Bianconi
> <lorenzo.bianconi@redhat.com> wrote:
>> According to my understanding (please correct me if I am wrong)
>> BSS_CHANGED_CQM is only needed if CQM_RSSI is handled
>> by the driver/fw, while if it is not set mac80211 will take care of that
>> in ieee80211_handle_beacon_sig routine.
>> I am AFK at the moment, I will test that patch when I am back from vacations.
>
> That matches my understanding as well (base on for example the message
> for commit ae44b502669d0cd1f167cdb48994292aa20fd3dd).

This is right, mt7601u had this flag added in that commit but the mt76
drivers were added later.  It seems mt76_rx_convert always sets the
ieee80211_rx_status.signal field so mac80211 can check the rssi values
and call any CQM callbacks if needed.

Best regards
Kristian Evensen Aug. 31, 2018, 12:25 p.m. UTC | #8
Hi Lorenzo,

On Mon, Aug 13, 2018 at 4:25 PM Kristian Evensen
<kristian.evensen@gmail.com> wrote:
> On Mon, Aug 13, 2018 at 12:55 PM Lorenzo Bianconi
> <lorenzo.bianconi@redhat.com> wrote:
> > I am AFK at the moment, I will test that patch when I am back from vacations.

I hope you enjoyed you vacations. Have you been able to test if any of
the mt76 USB-dongles support CQM?

BR,
Kristian
Lorenzo Bianconi Aug. 31, 2018, 1:15 p.m. UTC | #9
> Hi Lorenzo,
> 
> On Mon, Aug 13, 2018 at 4:25 PM Kristian Evensen
> <kristian.evensen@gmail.com> wrote:
> > On Mon, Aug 13, 2018 at 12:55 PM Lorenzo Bianconi
> > <lorenzo.bianconi@redhat.com> wrote:
> > > I am AFK at the moment, I will test that patch when I am back from vacations.
> 
> I hope you enjoyed you vacations. Have you been able to test if any of
> the mt76 USB-dongles support CQM?

I have not tested your patch yet sorry but there are no differences
between pcie and usb devices regarding rx RSSI management (both of them use
shared code for it) so I guess the feature will work on usb devices if
it works on pcie ones.

Regards,
Lorenzo

> 
> BR,
> Kristian
Kristian Evensen Aug. 31, 2018, 1:20 p.m. UTC | #10
Hi,
On Fri, Aug 31, 2018 at 3:15 PM Lorenzo Bianconi
<lorenzo.bianconi@redhat.com> wrote:
> I have not tested your patch yet sorry but there are no differences
> between pcie and usb devices regarding rx RSSI management (both of them use
> shared code for it) so I guess the feature will work on usb devices if
> it works on pcie ones.

Thanks for letting me know. I will submit a v2 with an updated and
more detailed commit message.

BR,
Kristian
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
index 029d54bc..3eb328ff 100644
--- a/drivers/net/wireless/mediatek/mt76/mac80211.c
+++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
@@ -305,6 +305,8 @@  int mt76_register_device(struct mt76_dev *dev, bool vht,
 
 	wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR;
 
+	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
+
 	wiphy->available_antennas_tx = dev->antenna_mask;
 	wiphy->available_antennas_rx = dev->antenna_mask;