diff mbox series

[v3,1/2] mt76: mt7915: fix STA mode connection on DFS channels

Message ID 20210820203531.20706-1-greearb@candelatech.com
State Superseded
Headers show
Series [v3,1/2] mt76: mt7915: fix STA mode connection on DFS channels | expand

Commit Message

Ben Greear Aug. 20, 2021, 8:35 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

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

Signed-off-by: Rubio Lu <Rubio-DW.Lu@mediatek.com>
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
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(-)

Comments

Janusz Dziedzic Aug. 29, 2021, 6:17 p.m. UTC | #1 
pt., 20 sie 2021 o 22:37 <greearb@candelatech.com> napisał(a):
>

> From: Ben Greear <greearb@candelatech.com>

>

> Only AP, adhoc and mesh mode needs to check CAC.

> Stations, in particular, do not need this check.

>

> Signed-off-by: Rubio Lu <Rubio-DW.Lu@mediatek.com>

> Signed-off-by: Ben Greear <greearb@candelatech.com>

> ---

> 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
Ben Greear Aug. 30, 2021, 10:38 p.m. UTC | #2 
On 8/29/21 11:17 AM, Janusz Dziedzic wrote:
> pt., 20 sie 2021 o 22:37 <greearb@candelatech.com> napisał(a):

>>

>> From: Ben Greear <greearb@candelatech.com>

>>

>> Only AP, adhoc and mesh mode needs to check CAC.

>> Stations, in particular, do not need this check.

>>

>> Signed-off-by: Rubio Lu <Rubio-DW.Lu@mediatek.com>

>> Signed-off-by: Ben Greear <greearb@candelatech.com>

>> ---

>> 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 <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com
diff mbox series

Patch

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