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@xxxxxxxxxxxxxxx
> > > > wrote:
> > > > > External email : Please do not click links or open
> > > > > attachments
> > > > > until
> > > > > you have verified the sender or the content.
> > > > >
> > > > >
> > > > > From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
> > > > >
> > > > > 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@xxxxxxxxxxxxxxx>
> > > > > ---
> > > > > .../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
