2021-08-20 20:36:06

by Ben Greear

[permalink] [raw]
Subject: [PATCH v3 1/2] mt76: mt7915: fix STA mode connection on DFS channels

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

diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
index 8747e452e114..a6e142d27b60 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
@@ -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


2021-08-20 20:36:21

by Ben Greear

[permalink] [raw]
Subject: [PATCH v3 2/2] mt76: mt7915: fix radar detector logic

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.

Signed-off-by: Ben Greear <[email protected]>
---
.../net/wireless/mediatek/mt76/mt7915/init.c | 7 +-
.../net/wireless/mediatek/mt76/mt7915/mac.c | 102 ++++++++++++------
.../net/wireless/mediatek/mt76/mt7915/main.c | 23 +++-
.../wireless/mediatek/mt76/mt7915/mt7915.h | 6 +-
4 files changed, 100 insertions(+), 38 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/init.c b/drivers/net/wireless/mediatek/mt76/mt7915/init.c
index f46ea989a348..543bb1e947e3 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/init.c
@@ -205,6 +205,7 @@ mt7915_regd_notifier(struct wiphy *wiphy,
struct mt76_phy *mphy = hw->priv;
struct mt7915_phy *phy = mphy->priv;
struct cfg80211_chan_def *chandef = &mphy->chandef;
+ int ret;

memcpy(dev->mt76.alpha2, request->alpha2, sizeof(dev->mt76.alpha2));
dev->mt76.region = request->dfs_region;
@@ -215,7 +216,10 @@ mt7915_regd_notifier(struct wiphy *wiphy,
if (!(chandef->chan->flags & IEEE80211_CHAN_RADAR))
return;

- mt7915_dfs_init_radar_detector(phy);
+ ret = mt7915_dfs_init_radar_detector(phy);
+ if (ret < 0)
+ dev_err(dev->mt76.dev, "init-wifi: dfs-init-radar-detector failed: %d",
+ ret);
}

static void
@@ -832,7 +836,6 @@ int mt7915_register_device(struct mt7915_dev *dev)

dev->mphy.hw->wiphy->available_antennas_rx = dev->mphy.chainmask;
dev->mphy.hw->wiphy->available_antennas_tx = dev->mphy.chainmask;
- dev->phy.dfs_state = -1;

#ifdef CONFIG_NL80211_TESTMODE
dev->mt76.test_ops = &mt7915_testmode_ops;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
index a6e142d27b60..f61c98905458 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
@@ -2404,14 +2404,29 @@ void mt7915_mac_work(struct work_struct *work)
MT7915_WATCHDOG_TIME);
}

-static void mt7915_dfs_stop_radar_detector(struct mt7915_phy *phy)
+int mt7915_dfs_stop_radar_detector(struct mt7915_phy *phy, bool ext_phy)
{
struct mt7915_dev *dev = phy->dev;
+ int err;
+
+ dev_dbg(dev->mt76.dev, "dfs-stop-radar-detector, rdd-state: 0x%x",
+ phy->rdd_state);
+
+ err = mt7915_mcu_rdd_cmd(dev, RDD_NORMAL_START, ext_phy,
+ MT_RX_SEL0, 0);
+ if (err < 0) {
+ dev_err(dev->mt76.dev, "mcu-rdd-cmd RDD_NORMAL_START failed: %d",
+ err);
+ /* I think best to carry on, even if we have an error here. */
+ }

if (phy->rdd_state & BIT(0))
mt7915_mcu_rdd_cmd(dev, RDD_STOP, 0, MT_RX_SEL0, 0);
if (phy->rdd_state & BIT(1))
mt7915_mcu_rdd_cmd(dev, RDD_STOP, 1, MT_RX_SEL0, 0);
+ phy->rdd_state = 0;
+
+ return err;
}

static int mt7915_dfs_start_rdd(struct mt7915_dev *dev, int chain)
@@ -2419,10 +2434,14 @@ static int mt7915_dfs_start_rdd(struct mt7915_dev *dev, int chain)
int err;

err = mt7915_mcu_rdd_cmd(dev, RDD_START, chain, MT_RX_SEL0, 0);
+
+ dev_dbg(dev->mt76.dev, "dfs-start-rdd, RDD_START rv: %d", err);
if (err < 0)
return err;

- return mt7915_mcu_rdd_cmd(dev, RDD_DET_MODE, chain, MT_RX_SEL0, 1);
+ err = mt7915_mcu_rdd_cmd(dev, RDD_DET_MODE, chain, MT_RX_SEL0, 1);
+ dev_dbg(dev->mt76.dev, "dfs-start-rdd, RDD_DET_MODE rv: %d", err);
+ return err;
}

static int mt7915_dfs_start_radar_detector(struct mt7915_phy *phy)
@@ -2523,47 +2542,68 @@ int mt7915_dfs_init_radar_detector(struct mt7915_phy *phy)
int err;
struct mt7915_vif_counts counts = {0};

- if (dev->mt76.region == NL80211_DFS_UNSET) {
- phy->dfs_state = -1;
- if (phy->rdd_state)
- goto stop;
+ dev_dbg(dev->mt76.dev,
+ "dfs-init-radar-detector, region: %d freq: %d chandef dfs-state: %d",
+ dev->mt76.region, chandef->chan->center_freq,
+ chandef->chan->dfs_state);

+ if (test_bit(MT76_SCANNING, &phy->mt76->state)) {
+ dev_dbg(dev->mt76.dev, "init-radar, was scanning, no change.\n");
return 0;
}

- if (test_bit(MT76_SCANNING, &phy->mt76->state))
- return 0;
+ if (dev->mt76.region == NL80211_DFS_UNSET) {
+ dev_dbg(dev->mt76.dev,
+ "dfs-init-radar, region is UNSET, disable radar.");
+ goto stop;
+ }
+
+ if (!(chandef->chan->flags & IEEE80211_CHAN_RADAR)) {
+ dev_dbg(dev->mt76.dev,
+ "dfs-init-radar, chandef does not want radar.");
+ goto stop;
+ }

- if (phy->dfs_state == chandef->chan->dfs_state)
+ ieee80211_iterate_active_interfaces(phy->mt76->hw,
+ IEEE80211_IFACE_ITER_RESUME_ALL,
+ mt7915_vif_counts, &counts);
+
+ if (!(counts.ap + counts.adhoc + counts.mesh)) {
+ /* No beaconning interfaces, do not start CAC */
+ dev_dbg(dev->mt76.dev,
+ "dfs-init-radar, no AP/Mesh/Adhoc vifs active, stop radar.");
+ goto stop;
+ }
+
+ /* At this point, we need radar detection, see if we have started
+ * it already.
+ */
+ if (phy->rdd_state) {
+ if (chandef->chan->dfs_state == NL80211_DFS_AVAILABLE) {
+ /* CAC is already complete. */
+ dev_dbg(dev->mt76.dev,
+ "init-radar, RADAR started and DFS state is AVAILABLE, call RDD_CAC_END");
+ return mt7915_mcu_rdd_cmd(dev, RDD_CAC_END, ext_phy,
+ MT_RX_SEL0, 0);
+ }
+ dev_dbg(dev->mt76.dev,
+ "init-radar, rdd_state indicates RADAR already started,"
+ " DFS state: %d not YET available, rdd_state: 0x%x",
+ chandef->chan->dfs_state, phy->rdd_state);
return 0;
+ }

err = mt7915_dfs_init_radar_specs(phy);
if (err < 0) {
- phy->dfs_state = -1;
+ dev_err(dev->mt76.dev, "dfs-init-radar-specs failed: %d",
+ err);
goto stop;
}

- phy->dfs_state = chandef->chan->dfs_state;
-
- if (chandef->chan->flags & IEEE80211_CHAN_RADAR) {
- 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);
- }
+ err = mt7915_dfs_start_radar_detector(phy);
+ dev_dbg(dev->mt76.dev, "dfs-start-radar-detector rv: %d", err);
+ return err;

stop:
- err = mt7915_mcu_rdd_cmd(dev, RDD_NORMAL_START, ext_phy,
- MT_RX_SEL0, 0);
- if (err < 0)
- return err;
-
- mt7915_dfs_stop_radar_detector(phy);
- return 0;
+ return mt7915_dfs_stop_radar_detector(phy, ext_phy);
}
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
index f3df5ea066a2..6bffddb63461 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
@@ -298,6 +298,8 @@ static void mt7915_init_dfs_state(struct mt7915_phy *phy)
struct mt76_phy *mphy = phy->mt76;
struct ieee80211_hw *hw = mphy->hw;
struct cfg80211_chan_def *chandef = &hw->conf.chandef;
+ struct mt7915_dev *dev = phy->dev;
+ bool ext_phy = phy != &dev->phy;

if (hw->conf.flags & IEEE80211_CONF_OFFCHANNEL)
return;
@@ -305,11 +307,23 @@ static void mt7915_init_dfs_state(struct mt7915_phy *phy)
if (!(chandef->chan->flags & IEEE80211_CHAN_RADAR))
return;

- if (mphy->chandef.chan->center_freq == chandef->chan->center_freq &&
- mphy->chandef.width == chandef->width)
+ if (phy->dfs_center_freq == chandef->chan->center_freq &&
+ phy->dfs_ch_width == chandef->width)
return;

- phy->dfs_state = -1;
+ /* We are being moved to a new frequency/bw, still on DFS. Stop
+ * any existing DFS, then will start it again in the
+ * init-radar-detector logic.
+ */
+ if (phy->rdd_state) {
+ dev_dbg(dev->mt76.dev,
+ "init-dfs-state, channel changed, old: %d:%d new: %d:%d, stopping radar.",
+ phy->dfs_center_freq, phy->dfs_ch_width,
+ chandef->chan->center_freq, chandef->width);
+ mt7915_dfs_stop_radar_detector(phy, ext_phy);
+ }
+ phy->dfs_center_freq = chandef->chan->center_freq;
+ phy->dfs_ch_width = chandef->width;
}

int mt7915_set_channel(struct mt7915_phy *phy)
@@ -337,6 +351,9 @@ int mt7915_set_channel(struct mt7915_phy *phy)

mt7915_mac_set_timing(phy);
ret = mt7915_dfs_init_radar_detector(phy);
+ if (ret < 0)
+ dev_err(dev->mt76.dev, "set-channel: dfs-init-radar-detector failed: %d",
+ ret);
mt7915_mac_cca_stats_reset(phy);

mt7915_mac_reset_counters(phy);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h b/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
index f46816cefa3f..a9f55748cf1e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
@@ -284,8 +284,9 @@ struct mt7915_phy {
s16 coverage_class;
u8 slottime;

- u8 rdd_state;
- int dfs_state;
+ u8 rdd_state; /* radar detection started bitfield */
+ enum nl80211_chan_width dfs_ch_width;
+ u32 dfs_center_freq;

u32 rx_ampdu_ts;
u32 ampdu_ref;
@@ -574,6 +575,7 @@ int mt7915_dfs_init_radar_detector(struct mt7915_phy *phy);
void mt7915_set_stream_he_caps(struct mt7915_phy *phy);
void mt7915_set_stream_vht_txbf_caps(struct mt7915_phy *phy);
void mt7915_update_channel(struct mt76_phy *mphy);
+int mt7915_dfs_stop_radar_detector(struct mt7915_phy *phy, bool ext_phy);
int mt7915_init_debugfs(struct mt7915_dev *dev);
#ifdef CONFIG_MAC80211_DEBUGFS
void mt7915_sta_add_debugfs(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
--
2.20.1

2021-08-29 18:18:29

by Janusz Dziedzic

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] mt76: mt7915: fix STA mode connection on DFS channels

pt., 20 sie 2021 o 22:37 <[email protected]> napisał(a):
>
> 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(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
> index 8747e452e114..a6e142d27b60 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
> @@ -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
>

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.

--- a/mt7615/mac.c
+++ b/mt7615/mac.c
@@ -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;

if (chandef->chan->flags & IEEE80211_CHAN_RADAR) {
- if (chandef->chan->dfs_state != NL80211_DFS_AVAILABLE)
- return mt7615_dfs_start_radar_detector(phy);
-
- return mt7615_mcu_rdd_cmd(dev, RDD_CAC_END, ext_phy,
- MT_RX_SEL0, 0);
+ return mt7615_dfs_start_radar_detector(phy);
}

stop:

BR
Janusz

2021-08-30 22:42:19

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] mt76: mt7915: fix STA mode connection on DFS channels

On 8/29/21 11:17 AM, Janusz Dziedzic wrote:
> pt., 20 sie 2021 o 22:37 <[email protected]> napisał(a):
>>
>> 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(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
>> index 8747e452e114..a6e142d27b60 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
>> @@ -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
>>
>
> 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.

So this config option would mean 'I think CAC spec means I don't have to do
CAC if station is already on this channel', with the understanding that this
may actually be an incorrect interpretation? That seems like something that
would not be acceptable upstream for good reasons.

Do you have any pointers to the specification(s) section(s) that apply to this?


> 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.
>
> --- a/mt7615/mac.c
> +++ b/mt7615/mac.c
> @@ -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;
>
> if (chandef->chan->flags & IEEE80211_CHAN_RADAR) {
> - if (chandef->chan->dfs_state != NL80211_DFS_AVAILABLE)
> - return mt7615_dfs_start_radar_detector(phy);
> -
> - return mt7615_mcu_rdd_cmd(dev, RDD_CAC_END, ext_phy,
> - MT_RX_SEL0, 0);
> + return mt7615_dfs_start_radar_detector(phy);
> }
>
> stop:

It would certainly be nice to simplify the code, but I don't know
enough about the firmware/driver details to comment intelligently on this
part.

Thanks,
Ben

>
> BR
> Janusz
>


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2021-11-10 20:32:54

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] mt76: mt7915: fix radar detector logic

On 8/20/21 1:35 PM, [email protected] wrote:
> 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.

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,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com


2021-11-10 22:36:43

by Janusz Dziedzic

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] mt76: mt7915: fix radar detector logic

śr., 10 lis 2021 o 21:36 Ben Greear <[email protected]> napisał(a):
>
> On 8/20/21 1:35 PM, [email protected] wrote:
> > 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.
>
> 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,

--- a/mt7615/mac.c
+++ b/mt7615/mac.c
@@ -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;

if (chandef->chan->flags & IEEE80211_CHAN_RADAR) {
- if (chandef->chan->dfs_state != NL80211_DFS_AVAILABLE)
- return mt7615_dfs_start_radar_detector(phy);
-
- return mt7615_mcu_rdd_cmd(dev, RDD_CAC_END, ext_phy,
- MT_RX_SEL0, 0);
+ return mt7615_dfs_start_radar_detector(phy);
}

--
Janusz Dziedzic