| 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 |
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 >
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
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
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
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 --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)