diff mbox series

[1/6] wifi: mt76: mt7915: Support vht mu-mimo sniffer feature.

Message ID 20230428205000.2647945-1-greearb@candelatech.com
State New
Headers show
Series [1/6] wifi: mt76: mt7915: Support vht mu-mimo sniffer feature. | expand

Commit Message

Ben Greear April 28, 2023, 8:49 p.m. UTC
From: Ben Greear <greearb@candelatech.com>

This feature allows mac80211 to update the driver with mu-mimo
group to allow the monitor port to capture MU-MIMO (VHT) frames.

The mt7915 driver implementation will only enable this feature
when there is only a monitor vdev.  I was afraid that it would mess
up a sta + monitor vdev combination, for instance.

Original code from Ryder

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/mediatek/mt76/mt76.h     |  5 +++
 .../net/wireless/mediatek/mt76/mt7915/init.c  |  1 +
 .../net/wireless/mediatek/mt76/mt7915/main.c  | 41 +++++++++++++++++++
 .../net/wireless/mediatek/mt76/mt7915/regs.h  | 10 +++++
 4 files changed, 57 insertions(+)

Comments

Ryder Lee April 28, 2023, 9:17 p.m. UTC | #1
On Fri, 2023-04-28 at 13:49 -0700, greearb@candelatech.com wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> From: Ben Greear <greearb@candelatech.com>
> 
> This feature allows mac80211 to update the driver with mu-mimo
> group to allow the monitor port to capture MU-MIMO (VHT) frames.
> 
> The mt7915 driver implementation will only enable this feature
> when there is only a monitor vdev.  I was afraid that it would mess
> up a sta + monitor vdev combination, for instance.
> 
> Original code from Ryder
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76.h     |  5 +++
>  .../net/wireless/mediatek/mt76/mt7915/init.c  |  1 +
>  .../net/wireless/mediatek/mt76/mt7915/main.c  | 41
> +++++++++++++++++++
>  .../net/wireless/mediatek/mt76/mt7915/regs.h  | 10 +++++
>  4 files changed, 57 insertions(+)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h
> b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 6b07b8fafec2..d4ae53d80d07 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -945,6 +945,11 @@ static inline u16 mt76_rev(struct mt76_dev *dev)
>         return dev->rev & 0xffff;
>  }
> 
> +static inline int mt76_vif_count(struct mt76_dev *dev)
> +{
> +       return hweight_long(dev->vif_mask);
> +}
> +
>  #define mt76xx_chip(dev) mt76_chip(&((dev)->mt76))
>  #define mt76xx_rev(dev) mt76_rev(&((dev)->mt76))
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/init.c
> b/drivers/net/wireless/mediatek/mt76/mt7915/init.c
> index ac2049f49bb3..bea75615872f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/init.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/init.c
> @@ -370,6 +370,7 @@ mt7915_init_wiphy(struct mt7915_phy *phy)
>         wiphy_ext_feature_set(wiphy,
> NL80211_EXT_FEATURE_FILS_DISCOVERY);
>         wiphy_ext_feature_set(wiphy,
> NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT);
>         wiphy_ext_feature_set(wiphy,
> NL80211_EXT_FEATURE_CAN_REPLACE_PTK0);
> +       wiphy_ext_feature_set(wiphy,
> NL80211_EXT_FEATURE_MU_MIMO_AIR_SNIFFER);
> 
>         if (!is_mt7915(&dev->mt76))
>                 wiphy_ext_feature_set(wiphy,
> NL80211_EXT_FEATURE_STA_TX_PWR);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> index 1b361199c061..7566db0cf523 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> @@ -592,6 +592,34 @@ mt7915_update_bss_color(struct ieee80211_hw *hw,
>         }
>  }
> 
> +static void
> +mt7915_update_mu_group(struct ieee80211_hw *hw, struct ieee80211_vif
> *vif,
> +                      struct ieee80211_bss_conf *info)
> +{
> +       struct mt7915_phy *phy = mt7915_hw_phy(hw);
> +       struct mt7915_dev *dev = mt7915_hw_dev(hw);
> +       u8 i, band = phy->mt76->band_idx;
> +       u32 *mu;
> +
> +       mu = (u32 *)info->mu_group.membership;
> +       for (i = 0; i < WLAN_MEMBERSHIP_LEN / sizeof(*mu); i++) {
> +               if (is_mt7916(&dev->mt76))
> +                       mt76_wr(dev,
> MT_WF_PHY_RX_GID_TAB_VLD_MT7916(band, i),
> +                               mu[i]);
> +               else
> +                       mt76_wr(dev, MT_WF_PHY_RX_GID_TAB_VLD(band,
> i), mu[i]);
> +       }
> +
> +       mu = (u32 *)info->mu_group.position;
> +       for (i = 0; i < WLAN_USER_POSITION_LEN / sizeof(*mu); i++) {
> +               if (is_mt7916(&dev->mt76))
> +                       mt76_wr(dev,
> MT_WF_PHY_RX_GID_TAB_POS_MT7916(band, i),
> +                               mu[i]);
> +               else
> +                       mt76_wr(dev, MT_WF_PHY_RX_GID_TAB_POS(band,
> i), mu[i]);
> +       }
> +}
> +
>  static void mt7915_bss_info_changed(struct ieee80211_hw *hw,
>                                     struct ieee80211_vif *vif,
>                                     struct ieee80211_bss_conf *info,
> @@ -650,6 +678,19 @@ static void mt7915_bss_info_changed(struct
> ieee80211_hw *hw,
>                        BSS_CHANGED_FILS_DISCOVERY))
>                 mt7915_mcu_add_beacon(hw, vif, info->enable_beacon,
> changed);
> 
> +       if (changed & BSS_CHANGED_MU_GROUPS) {
> +               /* Assumption is that in case of non-monitor VDEV
> existing, then
> +                * that device should control the mu-group directly.
> +                */
> +               int vif_count = mt76_vif_count(&dev->mt76);
> +               int max_ok = 0;

I don't think we need checks here as user can fully handle this with
toggling iw command - iw dev mon0 set monitor mumimo-groupid <Group ID>

Also, the Group ID mgmt frame is transmitted by the AP to assign or
change the user position of a STA, which will notify underlying driver
of changes.

> +               if (phy->monitor_vif)
> +                       max_ok = 1;
> +               if (vif_count <= max_ok)
> +                       mt7915_update_mu_group(hw, vif, info);
> +       }
> +
>         mutex_unlock(&dev->mt76.mutex);
>  }
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
> b/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
> index c8e478a55081..5e057cce5c9f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
> @@ -1183,6 +1183,16 @@ enum offs_rev {
>  #define MT_WF_PHY_BASE                 0x83080000
>  #define MT_WF_PHY(ofs)                 (MT_WF_PHY_BASE + (ofs))
> 
> +#define MT_WF_PHY_RX_GID_TAB_VLD(_phy,
> i)              MT_WF_PHY(0x1054 + \
> +                                                                 (i)
> * 4 + ((_phy) << 16))
> +#define MT_WF_PHY_RX_GID_TAB_VLD_MT7916(_phy,
> i)       MT_WF_PHY(0x1054 + \
> +                                                                 (i)
> * 4 + ((_phy) << 20))
> +
> +#define MT_WF_PHY_RX_GID_TAB_POS(_phy,
> i)              MT_WF_PHY(0x105c + \
> +                                                                 (i)
> * 4 + ((_phy) << 16))
> +#define MT_WF_PHY_RX_GID_TAB_POS_MT7916(_phy,
> i)       MT_WF_PHY(0x105c + \
> +                                                                 (i)
> * 4 + ((_phy) << 20))
> +
>  #define MT_WF_PHY_RX_CTRL1(_phy)       MT_WF_PHY(0x2004 + ((_phy) <<
> 16))
>  #define MT_WF_PHY_RX_CTRL1_MT7916(_phy)        MT_WF_PHY(0x2004 +
> ((_phy) << 20))
>  #define MT_WF_PHY_RX_CTRL1_IPI_EN      GENMASK(2, 0)
> --
> 2.40.0
>
Ben Greear April 28, 2023, 10:06 p.m. UTC | #2
On 4/28/23 14:48, Ryder Lee wrote:
> On Fri, 2023-04-28 at 13:49 -0700, greearb@candelatech.com wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> From: Ben Greear <greearb@candelatech.com>
>>
>> This enables capturing more frames, and now when the rx5 group
>> option is also enabled for rx-status, wireshark shows HE-TRIG
>> as well as HE-MU frames.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>   .../net/wireless/mediatek/mt76/mt7915/main.c  | 26
>> +++++++++++++++++--
>>   .../net/wireless/mediatek/mt76/mt7915/regs.h  | 16 ++++++++++++
>>   2 files changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
>> b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
>> index 64c14fc303a2..55aed3c6d3be 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
>> @@ -562,6 +562,12 @@ static void __mt7915_configure_filter(struct
>> ieee80211_hw *hw,
>>                          MT_WF_RFCR1_DROP_BF_POLL |
>>                          MT_WF_RFCR1_DROP_BA |
>>                          MT_WF_RFCR1_DROP_CFEND |
>> +                       MT_WF_RFCR1_DROP_PS_BFRPOL |
>> +                       MT_WF_RFCR1_DROP_PS_NDPA |
>> +                       MT_WF_RFCR1_DROP_NO2ME_TF |
>> +                       MT_WF_RFCR1_DROP_NON_MUBAR_TF |
>> +                       MT_WF_RFCR1_DROP_RXS_BRP |
>> +                       MT_WF_RFCR1_DROP_TF_BFRP |
>>                          MT_WF_RFCR1_DROP_CFACK;
>>          u32 flags = 0;
>>          bool is_promisc = *total_flags & FIF_CONTROL || phy-
>>> monitor_vif ||
>> @@ -587,7 +593,9 @@ static void __mt7915_configure_filter(struct
>> ieee80211_hw *hw,
>>                             MT_WF_RFCR_DROP_BCAST |
>>                             MT_WF_RFCR_DROP_DUPLICATE |
>>                             MT_WF_RFCR_DROP_A2_BSSID |
>> -                          MT_WF_RFCR_DROP_UNWANTED_CTL |
>> +                          MT_WF_RFCR_DROP_UNWANTED_CTL | /* 0 means
>> drop */
>> +                          MT_WF_RFCR_IND_FILTER_EN_OF_31_23_BIT |
>> +                          MT_WF_RFCR_DROP_DIFFBSSIDMGT_CTRL |
>>                             MT_WF_RFCR_DROP_STBC_MULTI);
>>
>>          phy->rxfilter |= MT_WF_RFCR_DROP_OTHER_UC;
>> @@ -602,8 +610,22 @@ static void __mt7915_configure_filter(struct
>> ieee80211_hw *hw,
>>                               MT_WF_RFCR_DROP_RTS |
>>                               MT_WF_RFCR_DROP_CTL_RSV |
>>                               MT_WF_RFCR_DROP_NDPA);
>> -       if (is_promisc)
>> +       if (is_promisc) {
>>                  phy->rxfilter &= ~MT_WF_RFCR_DROP_OTHER_UC;
>> +               phy->rxfilter |=
>> MT_WF_RFCR_IND_FILTER_EN_OF_31_23_BIT;
>> +               if (flags & FIF_CONTROL) {
>> +                       phy->rxfilter |=
>> MT_WF_RFCR_DROP_UNWANTED_CTL; /* 1 means receive */
>> +                       phy->rxfilter |= MT_WF_RFCR_SECOND_BCN_EN;
>> +                       phy->rxfilter |=
>> MT_WF_RFCR_RX_MGMT_FRAME_CTRL;
>> +                       phy->rxfilter |=
>> MT_WF_RFCR_RX_SAMEBSSIDPRORESP_CTRL;
>> +                       phy->rxfilter |=
>> MT_WF_RFCR_RX_DIFFBSSIDPRORESP_CTRL;
>> +                       phy->rxfilter |=
>> MT_WF_RFCR_RX_SAMEBSSIDBCN_CTRL;
>> +                       phy->rxfilter |=
>> MT_WF_RFCR_RX_SAMEBSSIDNULL_CTRL;
>> +                       phy->rxfilter |=
>> MT_WF_RFCR_RX_DIFFBSSIDNULL_CTRL;
>> +                       phy->rxfilter &=
>> ~(MT_WF_RFCR_DROP_DIFFBSSIDMGT_CTRL);
> 
> FIF_CONTROL: pass control frame. However, many of these are not control
> frames. I think we should move monitor dedicated misc parts to
> IEEE80211_CONF_CHANGE_MONITOR mt7915_set_monitor and leave this
> function as-is ... as my reply in [2/6].

My understanding is that the rxfilter and related phy fields should take all settings and vdevs into account.
So if monitor mode is enabled, and another sta/ap vdev exists, and someone takes any action that causes
the stack to call the set_filter() logic on the sta/vdev, then set_filter must know about the monitor port to
do the right thing.  This is why mt7915_configure_filter should handle everything and be aware of
monitor port existing or not.

Thanks,
Ben
Ryder Lee April 29, 2023, 2:33 a.m. UTC | #3
On Fri, 2023-04-28 at 15:06 -0700, Ben Greear wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 4/28/23 14:48, Ryder Lee wrote:
> > On Fri, 2023-04-28 at 13:49 -0700, greearb@candelatech.com wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > From: Ben Greear <greearb@candelatech.com>
> > > 
> > > This enables capturing more frames, and now when the rx5 group
> > > option is also enabled for rx-status, wireshark shows HE-TRIG
> > > as well as HE-MU frames.
> > > 
> > > Signed-off-by: Ben Greear <greearb@candelatech.com>
> > > ---
> > >   .../net/wireless/mediatek/mt76/mt7915/main.c  | 26
> > > +++++++++++++++++--
> > >   .../net/wireless/mediatek/mt76/mt7915/regs.h  | 16 ++++++++++++
> > >   2 files changed, 40 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > > b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > > index 64c14fc303a2..55aed3c6d3be 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > > @@ -562,6 +562,12 @@ static void __mt7915_configure_filter(struct
> > > ieee80211_hw *hw,
> > >                          MT_WF_RFCR1_DROP_BF_POLL |
> > >                          MT_WF_RFCR1_DROP_BA |
> > >                          MT_WF_RFCR1_DROP_CFEND |
> > > +                       MT_WF_RFCR1_DROP_PS_BFRPOL |
> > > +                       MT_WF_RFCR1_DROP_PS_NDPA |
> > > +                       MT_WF_RFCR1_DROP_NO2ME_TF |
> > > +                       MT_WF_RFCR1_DROP_NON_MUBAR_TF |
> > > +                       MT_WF_RFCR1_DROP_RXS_BRP |
> > > +                       MT_WF_RFCR1_DROP_TF_BFRP |
> > >                          MT_WF_RFCR1_DROP_CFACK;
> > >          u32 flags = 0;
> > >          bool is_promisc = *total_flags & FIF_CONTROL || phy-
> > > > monitor_vif ||
> > > 
> > > @@ -587,7 +593,9 @@ static void __mt7915_configure_filter(struct
> > > ieee80211_hw *hw,
> > >                             MT_WF_RFCR_DROP_BCAST |
> > >                             MT_WF_RFCR_DROP_DUPLICATE |
> > >                             MT_WF_RFCR_DROP_A2_BSSID |
> > > -                          MT_WF_RFCR_DROP_UNWANTED_CTL |
> > > +                          MT_WF_RFCR_DROP_UNWANTED_CTL | /* 0
> > > means
> > > drop */
> > > +                          MT_WF_RFCR_IND_FILTER_EN_OF_31_23_BIT
> > > |
> > > +                          MT_WF_RFCR_DROP_DIFFBSSIDMGT_CTRL |
> > >                             MT_WF_RFCR_DROP_STBC_MULTI);
> > > 
> > >          phy->rxfilter |= MT_WF_RFCR_DROP_OTHER_UC;
> > > @@ -602,8 +610,22 @@ static void __mt7915_configure_filter(struct
> > > ieee80211_hw *hw,
> > >                               MT_WF_RFCR_DROP_RTS |
> > >                               MT_WF_RFCR_DROP_CTL_RSV |
> > >                               MT_WF_RFCR_DROP_NDPA);
> > > -       if (is_promisc)
> > > +       if (is_promisc) {
> > >                  phy->rxfilter &= ~MT_WF_RFCR_DROP_OTHER_UC;
> > > +               phy->rxfilter |=
> > > MT_WF_RFCR_IND_FILTER_EN_OF_31_23_BIT;
> > > +               if (flags & FIF_CONTROL) {
> > > +                       phy->rxfilter |=
> > > MT_WF_RFCR_DROP_UNWANTED_CTL; /* 1 means receive */
> > > +                       phy->rxfilter |=
> > > MT_WF_RFCR_SECOND_BCN_EN;
> > > +                       phy->rxfilter |=
> > > MT_WF_RFCR_RX_MGMT_FRAME_CTRL;
> > > +                       phy->rxfilter |=
> > > MT_WF_RFCR_RX_SAMEBSSIDPRORESP_CTRL;
> > > +                       phy->rxfilter |=
> > > MT_WF_RFCR_RX_DIFFBSSIDPRORESP_CTRL;
> > > +                       phy->rxfilter |=
> > > MT_WF_RFCR_RX_SAMEBSSIDBCN_CTRL;
> > > +                       phy->rxfilter |=
> > > MT_WF_RFCR_RX_SAMEBSSIDNULL_CTRL;
> > > +                       phy->rxfilter |=
> > > MT_WF_RFCR_RX_DIFFBSSIDNULL_CTRL;
> > > +                       phy->rxfilter &=
> > > ~(MT_WF_RFCR_DROP_DIFFBSSIDMGT_CTRL);
> > 
> > FIF_CONTROL: pass control frame. However, many of these are not
> > control
> > frames. I think we should move monitor dedicated misc parts to
> > IEEE80211_CONF_CHANGE_MONITOR mt7915_set_monitor and leave this
> > function as-is ... as my reply in [2/6].
> 
> My understanding is that the rxfilter and related phy fields should
> take all settings and vdevs into account.
> So if monitor mode is enabled, and another sta/ap vdev exists, and
> someone takes any action that causes
> the stack to call the set_filter() logic on the sta/vdev, then
> set_filter must know about the monitor port to
> do the right thing.  This is why mt7915_configure_filter should
> handle everything and be aware of
> monitor port existing or not.
> 

This depends on what we end up doing with mixed modes. IMO, monitor
mode should be in the driver's seat. Hence we set/clear phy->rxfilter
or specific registers addtionally via IEEE80211_CONF_CHANGE_MONITOR as
we alreay did for MT_WF_RFCR_DROP_OTHER_US, right?

Ryder
Ben Greear April 29, 2023, 2:42 p.m. UTC | #4
On 4/28/23 7:33 PM, Ryder Lee wrote:
> On Fri, 2023-04-28 at 15:06 -0700, Ben Greear wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> 
>> 
>> On 4/28/23 14:48, Ryder Lee wrote:
>> > On Fri, 2023-04-28 at 13:49 -0700, greearb@candelatech.com wrote:
>> > > External email : Please do not click links or open attachments
>> > > until
>> > > you have verified the sender or the content.
>> > > 
>> > > 
>> > > From: Ben Greear <greearb@candelatech.com>
>> > > 
>> > > This enables capturing more frames, and now when the rx5 group
>> > > option is also enabled for rx-status, wireshark shows HE-TRIG
>> > > as well as HE-MU frames.
>> > > 
>> > > Signed-off-by: Ben Greear <greearb@candelatech.com>
>> > > ---
>> > >   .../net/wireless/mediatek/mt76/mt7915/main.c  | 26
>> > > +++++++++++++++++--
>> > >   .../net/wireless/mediatek/mt76/mt7915/regs.h  | 16 ++++++++++++
>> > >   2 files changed, 40 insertions(+), 2 deletions(-)
>> > > 
>> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
>> > > b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
>> > > index 64c14fc303a2..55aed3c6d3be 100644
>> > > --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
>> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
>> > > @@ -562,6 +562,12 @@ static void __mt7915_configure_filter(struct
>> > > ieee80211_hw *hw,
>> > >                          MT_WF_RFCR1_DROP_BF_POLL |
>> > >                          MT_WF_RFCR1_DROP_BA |
>> > >                          MT_WF_RFCR1_DROP_CFEND |
>> > > +                       MT_WF_RFCR1_DROP_PS_BFRPOL |
>> > > +                       MT_WF_RFCR1_DROP_PS_NDPA |
>> > > +                       MT_WF_RFCR1_DROP_NO2ME_TF |
>> > > +                       MT_WF_RFCR1_DROP_NON_MUBAR_TF |
>> > > +                       MT_WF_RFCR1_DROP_RXS_BRP |
>> > > +                       MT_WF_RFCR1_DROP_TF_BFRP |
>> > >                          MT_WF_RFCR1_DROP_CFACK;
>> > >          u32 flags = 0;
>> > >          bool is_promisc = *total_flags & FIF_CONTROL || phy-
>> > > > monitor_vif ||
>> > > 
>> > > @@ -587,7 +593,9 @@ static void __mt7915_configure_filter(struct
>> > > ieee80211_hw *hw,
>> > >                             MT_WF_RFCR_DROP_BCAST |
>> > >                             MT_WF_RFCR_DROP_DUPLICATE |
>> > >                             MT_WF_RFCR_DROP_A2_BSSID |
>> > > -                          MT_WF_RFCR_DROP_UNWANTED_CTL |
>> > > +                          MT_WF_RFCR_DROP_UNWANTED_CTL | /* 0
>> > > means
>> > > drop */
>> > > +                          MT_WF_RFCR_IND_FILTER_EN_OF_31_23_BIT
>> > > |
>> > > +                          MT_WF_RFCR_DROP_DIFFBSSIDMGT_CTRL |
>> > >                             MT_WF_RFCR_DROP_STBC_MULTI);
>> > > 
>> > >          phy->rxfilter |= MT_WF_RFCR_DROP_OTHER_UC;
>> > > @@ -602,8 +610,22 @@ static void __mt7915_configure_filter(struct
>> > > ieee80211_hw *hw,
>> > >                               MT_WF_RFCR_DROP_RTS |
>> > >                               MT_WF_RFCR_DROP_CTL_RSV |
>> > >                               MT_WF_RFCR_DROP_NDPA);
>> > > -       if (is_promisc)
>> > > +       if (is_promisc) {
>> > >                  phy->rxfilter &= ~MT_WF_RFCR_DROP_OTHER_UC;
>> > > +               phy->rxfilter |=
>> > > MT_WF_RFCR_IND_FILTER_EN_OF_31_23_BIT;
>> > > +               if (flags & FIF_CONTROL) {
>> > > +                       phy->rxfilter |=
>> > > MT_WF_RFCR_DROP_UNWANTED_CTL; /* 1 means receive */
>> > > +                       phy->rxfilter |=
>> > > MT_WF_RFCR_SECOND_BCN_EN;
>> > > +                       phy->rxfilter |=
>> > > MT_WF_RFCR_RX_MGMT_FRAME_CTRL;
>> > > +                       phy->rxfilter |=
>> > > MT_WF_RFCR_RX_SAMEBSSIDPRORESP_CTRL;
>> > > +                       phy->rxfilter |=
>> > > MT_WF_RFCR_RX_DIFFBSSIDPRORESP_CTRL;
>> > > +                       phy->rxfilter |=
>> > > MT_WF_RFCR_RX_SAMEBSSIDBCN_CTRL;
>> > > +                       phy->rxfilter |=
>> > > MT_WF_RFCR_RX_SAMEBSSIDNULL_CTRL;
>> > > +                       phy->rxfilter |=
>> > > MT_WF_RFCR_RX_DIFFBSSIDNULL_CTRL;
>> > > +                       phy->rxfilter &=
>> > > ~(MT_WF_RFCR_DROP_DIFFBSSIDMGT_CTRL);
>> > 
>> > FIF_CONTROL: pass control frame. However, many of these are not
>> > control
>> > frames. I think we should move monitor dedicated misc parts to
>> > IEEE80211_CONF_CHANGE_MONITOR mt7915_set_monitor and leave this
>> > function as-is ... as my reply in [2/6].
>> 
>> My understanding is that the rxfilter and related phy fields should
>> take all settings and vdevs into account.
>> So if monitor mode is enabled, and another sta/ap vdev exists, and
>> someone takes any action that causes
>> the stack to call the set_filter() logic on the sta/vdev, then
>> set_filter must know about the monitor port to
>> do the right thing.  This is why mt7915_configure_filter should
>> handle everything and be aware of
>> monitor port existing or not.
>> 
> 
> This depends on what we end up doing with mixed modes. IMO, monitor
> mode should be in the driver's seat. Hence we set/clear phy->rxfilter
> or specific registers addtionally via IEEE80211_CONF_CHANGE_MONITOR as
> we alreay did for MT_WF_RFCR_DROP_OTHER_US, right?

My patch should make monitor mode work whether or not other vdevs are active
(and causing calls to set_filter logic after monitor mode was enabled).
And it puts the somewhat tricky filter and related register logic into a single location.

If we do not pay attention to monitor-mode in the set_filter, then it
will probably break in mixed vdev mode.

Thanks,
Ben
Ryder Lee April 30, 2023, 6:02 p.m. UTC | #5
On Sat, 2023-04-29 at 07:42 -0700, Ben Greear wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 4/28/23 7:33 PM, Ryder Lee wrote:
> > On Fri, 2023-04-28 at 15:06 -0700, Ben Greear wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On 4/28/23 14:48, Ryder Lee wrote:
> > > > On Fri, 2023-04-28 at 13:49 -0700, greearb@candelatech.com
> > > > wrote:
> > > > > External email : Please do not click links or open
> > > > > attachments
> > > > > until
> > > > > you have verified the sender or the content.
> > > > > 
> > > > > 
> > > > > From: Ben Greear <greearb@candelatech.com>
> > > > > 
> > > > > This enables capturing more frames, and now when the rx5
> > > > > group
> > > > > option is also enabled for rx-status, wireshark shows HE-TRIG
> > > > > as well as HE-MU frames.
> > > > > 
> > > > > Signed-off-by: Ben Greear <greearb@candelatech.com>
> > > > > ---
> > > > >   .../net/wireless/mediatek/mt76/mt7915/main.c  | 26
> > > > > +++++++++++++++++--
> > > > >   .../net/wireless/mediatek/mt76/mt7915/regs.h  | 16
> > > > > ++++++++++++
> > > > >   2 files changed, 40 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > > > > b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > > > > index 64c14fc303a2..55aed3c6d3be 100644
> > > > > --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > > > > @@ -562,6 +562,12 @@ static void
> > > > > __mt7915_configure_filter(struct
> > > > > ieee80211_hw *hw,
> > > > >                          MT_WF_RFCR1_DROP_BF_POLL |
> > > > >                          MT_WF_RFCR1_DROP_BA |
> > > > >                          MT_WF_RFCR1_DROP_CFEND |
> > > > > +                       MT_WF_RFCR1_DROP_PS_BFRPOL |
> > > > > +                       MT_WF_RFCR1_DROP_PS_NDPA |
> > > > > +                       MT_WF_RFCR1_DROP_NO2ME_TF |
> > > > > +                       MT_WF_RFCR1_DROP_NON_MUBAR_TF |
> > > > > +                       MT_WF_RFCR1_DROP_RXS_BRP |
> > > > > +                       MT_WF_RFCR1_DROP_TF_BFRP |
> > > > >                          MT_WF_RFCR1_DROP_CFACK;
> > > > >          u32 flags = 0;
> > > > >          bool is_promisc = *total_flags & FIF_CONTROL || phy-
> > > > > > monitor_vif ||
> > > > > 
> > > > > @@ -587,7 +593,9 @@ static void
> > > > > __mt7915_configure_filter(struct
> > > > > ieee80211_hw *hw,
> > > > >                             MT_WF_RFCR_DROP_BCAST |
> > > > >                             MT_WF_RFCR_DROP_DUPLICATE |
> > > > >                             MT_WF_RFCR_DROP_A2_BSSID |
> > > > > -                          MT_WF_RFCR_DROP_UNWANTED_CTL |
> > > > > +                          MT_WF_RFCR_DROP_UNWANTED_CTL | /*
> > > > > 0
> > > > > means
> > > > > drop */
> > > > > +                          MT_WF_RFCR_IND_FILTER_EN_OF_31_23_
> > > > > BIT
> > > > > > 
> > > > > 
> > > > > +                          MT_WF_RFCR_DROP_DIFFBSSIDMGT_CTRL
> > > > > |
> > > > >                             MT_WF_RFCR_DROP_STBC_MULTI);
> > > > > 
> > > > >          phy->rxfilter |= MT_WF_RFCR_DROP_OTHER_UC;
> > > > > @@ -602,8 +610,22 @@ static void
> > > > > __mt7915_configure_filter(struct
> > > > > ieee80211_hw *hw,
> > > > >                               MT_WF_RFCR_DROP_RTS |
> > > > >                               MT_WF_RFCR_DROP_CTL_RSV |
> > > > >                               MT_WF_RFCR_DROP_NDPA);
> > > > > -       if (is_promisc)
> > > > > +       if (is_promisc) {
> > > > >                  phy->rxfilter &= ~MT_WF_RFCR_DROP_OTHER_UC;
> > > > > +               phy->rxfilter |=
> > > > > MT_WF_RFCR_IND_FILTER_EN_OF_31_23_BIT;
> > > > > +               if (flags & FIF_CONTROL) {
> > > > > +                       phy->rxfilter |=
> > > > > MT_WF_RFCR_DROP_UNWANTED_CTL; /* 1 means receive */
> > > > > +                       phy->rxfilter |=
> > > > > MT_WF_RFCR_SECOND_BCN_EN;
> > > > > +                       phy->rxfilter |=
> > > > > MT_WF_RFCR_RX_MGMT_FRAME_CTRL;
> > > > > +                       phy->rxfilter |=
> > > > > MT_WF_RFCR_RX_SAMEBSSIDPRORESP_CTRL;
> > > > > +                       phy->rxfilter |=
> > > > > MT_WF_RFCR_RX_DIFFBSSIDPRORESP_CTRL;
> > > > > +                       phy->rxfilter |=
> > > > > MT_WF_RFCR_RX_SAMEBSSIDBCN_CTRL;
> > > > > +                       phy->rxfilter |=
> > > > > MT_WF_RFCR_RX_SAMEBSSIDNULL_CTRL;
> > > > > +                       phy->rxfilter |=
> > > > > MT_WF_RFCR_RX_DIFFBSSIDNULL_CTRL;
> > > > > +                       phy->rxfilter &=
> > > > > ~(MT_WF_RFCR_DROP_DIFFBSSIDMGT_CTRL);
> > > > 
> > > > FIF_CONTROL: pass control frame. However, many of these are not
> > > > control
> > > > frames. I think we should move monitor dedicated misc parts to
> > > > IEEE80211_CONF_CHANGE_MONITOR mt7915_set_monitor and leave this
> > > > function as-is ... as my reply in [2/6].
> > > 
> > > My understanding is that the rxfilter and related phy fields
> > > should
> > > take all settings and vdevs into account.
> > > So if monitor mode is enabled, and another sta/ap vdev exists,
> > > and
> > > someone takes any action that causes
> > > the stack to call the set_filter() logic on the sta/vdev, then
> > > set_filter must know about the monitor port to
> > > do the right thing.  This is why mt7915_configure_filter should
> > > handle everything and be aware of
> > > monitor port existing or not.
> > > 
> > 
> > This depends on what we end up doing with mixed modes. IMO, monitor
> > mode should be in the driver's seat. Hence we set/clear phy-
> > >rxfilter
> > or specific registers addtionally via IEEE80211_CONF_CHANGE_MONITOR
> > as
> > we alreay did for MT_WF_RFCR_DROP_OTHER_US, right?
> 
> My patch should make monitor mode work whether or not other vdevs are
> active
> (and causing calls to set_filter logic after monitor mode was
> enabled).
> And it puts the somewhat tricky filter and related register logic
> into a single location.
> 
> If we do not pay attention to monitor-mode in the set_filter, then it
> will probably break in mixed vdev mode.
> 
> 
Similar to [1/6] VHT mu-mimo sniffer thing.

That's the reason why "iw monitor mode flags" exists, right? I think
users can handle this situation themselves via iw command, which is
more flexible than adding a fixed vdev check in driver. That said, user
can decide what fif_flags they want by doing so. 

And we can just put extra monitor specific controls into
IEEE80211_CONF_CHANGE_MONITOR.

Ryder
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 6b07b8fafec2..d4ae53d80d07 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -945,6 +945,11 @@  static inline u16 mt76_rev(struct mt76_dev *dev)
 	return dev->rev & 0xffff;
 }
 
+static inline int mt76_vif_count(struct mt76_dev *dev)
+{
+	return hweight_long(dev->vif_mask);
+}
+
 #define mt76xx_chip(dev) mt76_chip(&((dev)->mt76))
 #define mt76xx_rev(dev) mt76_rev(&((dev)->mt76))
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/init.c b/drivers/net/wireless/mediatek/mt76/mt7915/init.c
index ac2049f49bb3..bea75615872f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/init.c
@@ -370,6 +370,7 @@  mt7915_init_wiphy(struct mt7915_phy *phy)
 	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_FILS_DISCOVERY);
 	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT);
 	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CAN_REPLACE_PTK0);
+	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_MU_MIMO_AIR_SNIFFER);
 
 	if (!is_mt7915(&dev->mt76))
 		wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_STA_TX_PWR);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
index 1b361199c061..7566db0cf523 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
@@ -592,6 +592,34 @@  mt7915_update_bss_color(struct ieee80211_hw *hw,
 	}
 }
 
+static void
+mt7915_update_mu_group(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+		       struct ieee80211_bss_conf *info)
+{
+	struct mt7915_phy *phy = mt7915_hw_phy(hw);
+	struct mt7915_dev *dev = mt7915_hw_dev(hw);
+	u8 i, band = phy->mt76->band_idx;
+	u32 *mu;
+
+	mu = (u32 *)info->mu_group.membership;
+	for (i = 0; i < WLAN_MEMBERSHIP_LEN / sizeof(*mu); i++) {
+		if (is_mt7916(&dev->mt76))
+			mt76_wr(dev, MT_WF_PHY_RX_GID_TAB_VLD_MT7916(band, i),
+				mu[i]);
+		else
+			mt76_wr(dev, MT_WF_PHY_RX_GID_TAB_VLD(band, i), mu[i]);
+	}
+
+	mu = (u32 *)info->mu_group.position;
+	for (i = 0; i < WLAN_USER_POSITION_LEN / sizeof(*mu); i++) {
+		if (is_mt7916(&dev->mt76))
+			mt76_wr(dev, MT_WF_PHY_RX_GID_TAB_POS_MT7916(band, i),
+				mu[i]);
+		else
+			mt76_wr(dev, MT_WF_PHY_RX_GID_TAB_POS(band, i), mu[i]);
+	}
+}
+
 static void mt7915_bss_info_changed(struct ieee80211_hw *hw,
 				    struct ieee80211_vif *vif,
 				    struct ieee80211_bss_conf *info,
@@ -650,6 +678,19 @@  static void mt7915_bss_info_changed(struct ieee80211_hw *hw,
 		       BSS_CHANGED_FILS_DISCOVERY))
 		mt7915_mcu_add_beacon(hw, vif, info->enable_beacon, changed);
 
+	if (changed & BSS_CHANGED_MU_GROUPS) {
+		/* Assumption is that in case of non-monitor VDEV existing, then
+		 * that device should control the mu-group directly.
+		 */
+		int vif_count = mt76_vif_count(&dev->mt76);
+		int max_ok = 0;
+
+		if (phy->monitor_vif)
+			max_ok = 1;
+		if (vif_count <= max_ok)
+			mt7915_update_mu_group(hw, vif, info);
+	}
+
 	mutex_unlock(&dev->mt76.mutex);
 }
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/regs.h b/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
index c8e478a55081..5e057cce5c9f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/regs.h
@@ -1183,6 +1183,16 @@  enum offs_rev {
 #define MT_WF_PHY_BASE			0x83080000
 #define MT_WF_PHY(ofs)			(MT_WF_PHY_BASE + (ofs))
 
+#define MT_WF_PHY_RX_GID_TAB_VLD(_phy, i)		MT_WF_PHY(0x1054 + \
+								  (i) * 4 + ((_phy) << 16))
+#define MT_WF_PHY_RX_GID_TAB_VLD_MT7916(_phy, i)	MT_WF_PHY(0x1054 + \
+								  (i) * 4 + ((_phy) << 20))
+
+#define MT_WF_PHY_RX_GID_TAB_POS(_phy, i)		MT_WF_PHY(0x105c + \
+								  (i) * 4 + ((_phy) << 16))
+#define MT_WF_PHY_RX_GID_TAB_POS_MT7916(_phy, i)	MT_WF_PHY(0x105c + \
+								  (i) * 4 + ((_phy) << 20))
+
 #define MT_WF_PHY_RX_CTRL1(_phy)	MT_WF_PHY(0x2004 + ((_phy) << 16))
 #define MT_WF_PHY_RX_CTRL1_MT7916(_phy)	MT_WF_PHY(0x2004 + ((_phy) << 20))
 #define MT_WF_PHY_RX_CTRL1_IPI_EN	GENMASK(2, 0)