mt76: mt7915: fix STA mode connection on DFS channels

From: Ben Greear <[email protected]>

Before this patch, if AP went from ch 100 to ch 36, the radar detector logic in the firmware was not being disabled. This made the AP appear to be up, but no beacons were seen on air until module reload or reboot.

To reproduce this, I change hostapd.conf and restart hostapd. Others on openwrt used their UI to make changes and problem was seen, but stil others changed channels in some other way and/or had some other difference and could *not* reproduce it. So, something perhaps a bit subtle.

To fix the problem, stop depending on comparing dfs_state, store last freq/bandwidth to detect changes in that, and streamline code that checks to enable/disable radar detection. And add in error checking and dev_dbg logic so one can see what is actually happening if need to debug this again.

Quoted 13 lines

From: Ben Greear <[email protected]>

Before this patch, if AP went from ch 100 to ch 36, the radar detector logic in the firmware was not being disabled. This made the AP appear to be up, but no beacons were seen on air until module reload or reboot.

To reproduce this, I change hostapd.conf and restart hostapd. Others on openwrt used their UI to make changes and problem was seen, but stil others changed channels in some other way and/or had some other difference and could *not* reproduce it. So, something perhaps a bit subtle.

Quoted 5 lines

To fix the problem, stop depending on comparing dfs_state, store last freq/bandwidth to detect changes in that, and streamline code that checks to enable/disable radar detection. And add in error checking and dev_dbg logic so one can see what is actually happening if need to debug this again.

Back when I was working on this, someone sent me an email or chat or something about some other way to do this that was simpler and just relied on mac80211 timers. But, I cannot find that email for the life of me.

In case you are reading, please resend! Evidently this 2/2 patch had some regression for someone so it is not acceptable upstream in current state...maybe the mac80211 option is best.

Thanks,

From: Ben Greear <[email protected]>

Only AP, adhoc and mesh mode needs to check CAC.
Stations, in particular, do not need this check.

Signed-off-by Rubio Lu <[email protected]>

Signed-off-by Ben Greear <[email protected]>

---
v3: Fix typo in SOB in 1/2, fix rebase typo in 2/2,
split long line in 2/2

 .../net/wireless/mediatek/mt76/mt7915/mac.c   | 38 +++++++++++++++++--
 1 file changed, 35 insertions(+), 3 deletions(-)

drivers/net/wireless/mediatek/mt76/mt7915/mac.c +35 -4

index 8747e452e114..a6e142d27b60 100644
@@ -2455,6 +2455,32 @@ static int mt7915_dfs_start_radar_detector(struct mt7915_phy *phy)
        return 0;
 }
 
+struct mt7915_vif_counts {
+       u32 mesh;
+       u32 adhoc;
+       u32 ap;
+};
+
+static void
+mt7915_vif_counts(void *priv, u8 *mac, struct ieee80211_vif *vif)
+{
+       struct mt7915_vif_counts *counts = priv;
+
+       switch (vif->type) {
+       case NL80211_IFTYPE_ADHOC:
+               counts->adhoc++;
+               break;
+       case NL80211_IFTYPE_MESH_POINT:
+               counts->mesh++;
+               break;
+       case NL80211_IFTYPE_AP:
+               counts->ap++;
+               break;
+       default:
+               break;
+       }
+}
+
 static int
 mt7915_dfs_init_radar_specs(struct mt7915_phy *phy)
 {

@@ -2495,6 +2521,7 @@ int mt7915_dfs_init_radar_detector(struct mt7915_phy *phy)
        struct mt7915_dev *dev = phy->dev;
        bool ext_phy = phy != &dev->phy;
        int err;
+       struct mt7915_vif_counts counts = {0};
 
        if (dev->mt76.region == NL80211_DFS_UNSET) {
                phy->dfs_state = -1;

@@ -2519,9 +2546,14 @@ int mt7915_dfs_init_radar_detector(struct mt7915_phy *phy)
        phy->dfs_state = chandef->chan->dfs_state;
 
        if (chandef->chan->flags & IEEE80211_CHAN_RADAR) {
-               if (chandef->chan->dfs_state != NL80211_DFS_AVAILABLE)
-                       return mt7915_dfs_start_radar_detector(phy);
-
+               if (chandef->chan->dfs_state != NL80211_DFS_AVAILABLE) {
+                       ieee80211_iterate_active_interfaces(phy->mt76->hw,
+                               IEEE80211_IFACE_ITER_RESUME_ALL,
+                               mt7915_vif_counts, &counts);
+                       if (counts.ap + counts.adhoc + counts.mesh)
+                               mt7915_dfs_start_radar_detector(phy);
+                       return 0;
+               }
                return mt7915_mcu_rdd_cmd(dev, RDD_CAC_END, ext_phy,
                                          MT_RX_SEL0, 0);
        }
--

2.20.1

pt., 20 sie 2021 o 22:37 <[email protected]> napisał(a):

Quoted 78 lines

From: Ben Greear <[email protected]>

Only AP, adhoc and mesh mode needs to check CAC.
Stations, in particular, do not need this check.

Signed-off-by Rubio Lu <[email protected]>

Signed-off-by Ben Greear <[email protected]>

---
v3: Fix typo in SOB in 1/2, fix rebase typo in 2/2,
split long line in 2/2

  .../net/wireless/mediatek/mt76/mt7915/mac.c   | 38 +++++++++++++++++--
  1 file changed, 35 insertions(+), 3 deletions(-)

drivers/net/wireless/mediatek/mt76/mt7915/mac.c +35 -4

index 8747e452e114..a6e142d27b60 100644
@@ -2455,6 +2455,32 @@ static int mt7915_dfs_start_radar_detector(struct mt7915_phy *phy)
         return 0;
  }
 
+struct mt7915_vif_counts {
+       u32 mesh;
+       u32 adhoc;
+       u32 ap;
+};
+
+static void
+mt7915_vif_counts(void *priv, u8 *mac, struct ieee80211_vif *vif)
+{
+       struct mt7915_vif_counts *counts = priv;
+
+       switch (vif->type) {
+       case NL80211_IFTYPE_ADHOC:
+               counts->adhoc++;
+               break;
+       case NL80211_IFTYPE_MESH_POINT:
+               counts->mesh++;
+               break;
+       case NL80211_IFTYPE_AP:
+               counts->ap++;
+               break;
+       default:
+               break;
+       }
+}
+
  static int
  mt7915_dfs_init_radar_specs(struct mt7915_phy *phy)
  {

@@ -2495,6 +2521,7 @@ int mt7915_dfs_init_radar_detector(struct mt7915_phy *phy)
         struct mt7915_dev *dev = phy->dev;
         bool ext_phy = phy != &dev->phy;
         int err;
+       struct mt7915_vif_counts counts = {0};
 
         if (dev->mt76.region == NL80211_DFS_UNSET) {
                 phy->dfs_state = -1;

@@ -2519,9 +2546,14 @@ int mt7915_dfs_init_radar_detector(struct mt7915_phy *phy)
         phy->dfs_state = chandef->chan->dfs_state;
 
         if (chandef->chan->flags & IEEE80211_CHAN_RADAR) {
-               if (chandef->chan->dfs_state != NL80211_DFS_AVAILABLE)
-                       return mt7915_dfs_start_radar_detector(phy);
-
+               if (chandef->chan->dfs_state != NL80211_DFS_AVAILABLE) {
+                       ieee80211_iterate_active_interfaces(phy->mt76->hw,
+                               IEEE80211_IFACE_ITER_RESUME_ALL,
+                               mt7915_vif_counts, &counts);
+                       if (counts.ap + counts.adhoc + counts.mesh)
+                               mt7915_dfs_start_radar_detector(phy);
+                       return 0;
+               }
                 return mt7915_mcu_rdd_cmd(dev, RDD_CAC_END, ext_phy,
                                           MT_RX_SEL0, 0);
         }
--

Quoted 2 lines

2.20.1

This depends on spec interpretation - when we have multiple ifaces on the same radio/channel (STA + APs).
Maybe this is good time to start discussion about it - how we handle DFS and if we should improve.

Some vendors "derive" CAC from STA.
So, while STA don't need to run CAC and first VIF sta will be connected on DFS channel, assume don't need to run CAC for second, third ... AP VIF. Still required ISM (In service monitoring, radar detection if AP ifaces) but simple skip CAC. This simplify implementation a lot for multi-vif (STA+APs) case.

So, maybe we should/could add kconfig for that - CONFIG_DFS_DERIVE_STA_CAC. When set, we could simple set NL80211_DFS_AVAILABLE when STA will connect on DFS channel - then any other APs we will add on the same channel will not require CAC, radar detection still required.

Regarding STA connection on DFS channel, I agree - today MT76x have a bug for that (eg. single VIF station).
I have much older code and fix it simplest way I could.

mt7615/mac.c +5 -0

@@ -2034,6 +2034,11 @@ static int mt7615_dfs_start_radar_detect
   phy->rdd_state |= BIT(1);
   }
 
+   /* end CAC - upper layer will care about it, lock tx, beacon setup */
+   err = mt7615_mcu_rdd_cmd(dev, RDD_CAC_END, ext_phy, MT_RX_SEL0, 0);
+   if (err < 0)
+       return err;
+
   return 0;
  }
 

@@ -2104,11 +2109,7 @@ int mt7615_dfs_init_radar_detector(struc
   phy->dfs_state = chandef->chan->dfs_state;
 

BR
Janusz