[PATCH 0/3] wifi: ath11k: fix CAC running state

[PATCH 0/3] wifi: ath11k: fix CAC running state

[Aditya Kumar Singh]

Currently CAC running flag in ath11k is not set if DFS radar is required in
secondary channel. This is due to the fact that only primary channel's DFS
state and CAC time is being checked before setting the flag.

Fix this issue by checking the DFS state of all the sub-channels inside a
channel definition and not just the primary channel.

Also, make minor change in debug prints to show the CAC time

Aditya Kumar Singh (3):
  wifi: cfg80211: export DFS CAC time and usable state helper functions
  wifi: ath11k: fix CAC running state during virtual interface start
  wifi: ath11k: fix Tx power value during active CAC

 drivers/net/wireless/ath/ath11k/mac.c | 27 +++++++++++++++++++--------
 include/net/cfg80211.h                | 24 ++++++++++++++++++++++++
 net/wireless/chan.c                   |  2 ++
 net/wireless/core.h                   | 17 -----------------
 4 files changed, 45 insertions(+), 25 deletions(-)


base-commit: a4756ac34a7002861c9bdf8cf45aec53a77fb78d
-- 
2.17.1

[PATCH 1/3] wifi: cfg80211: export DFS CAC time and usable state helper functions

[Aditya Kumar Singh]

cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
at least one channel in the chandef is in usable state or not. Also,
cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
time required for the given chandef.

Make these two functions visible to drivers by exporting their symbol
to global list of kernel symbols.

Lower level drivers can make use of these two functions to be aware
if CAC is required on the given chandef and for how long. For example
drivers which maintains the CAC state internally can make use of these.

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 include/net/cfg80211.h | 24 ++++++++++++++++++++++++
 net/wireless/chan.c    |  2 ++
 net/wireless/core.h    | 17 -----------------
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9e04f69712b1..e99e83a7308e 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -950,6 +950,30 @@ int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
 				  const struct cfg80211_chan_def *chandef,
 				  enum nl80211_iftype iftype);
 
+/**
+ * cfg80211_chandef_dfs_usable - checks if chandef is DFS usable and we
+ *				 can/need start CAC on such channel
+ * @wiphy: the wiphy to validate against
+ * @chandef: the channel definition to check
+ *
+ * Return: true if all channels available and at least
+ *	   one channel requires CAC (NL80211_DFS_USABLE)
+ */
+bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
+				 const struct cfg80211_chan_def *chandef);
+
+/**
+ * cfg80211_chandef_dfs_cac_time - get the DFS CAC time (in ms) for given
+ *				   channel definition
+ * @wiphy: the wiphy to validate against
+ * @chandef: the channel definition to check
+ *
+ * Returns: DFS CAC time (in ms) which applies for this channel definition
+ */
+unsigned int
+cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
+			      const struct cfg80211_chan_def *chandef);
+
 /**
  * nl80211_send_chandef - sends the channel definition.
  * @msg: the msg to send channel definition
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 0b7e81db383d..a78a6183d11e 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -666,6 +666,7 @@ bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
 
 	return (r1 + r2 > 0);
 }
+EXPORT_SYMBOL(cfg80211_chandef_dfs_usable);
 
 /*
  * Checks if center frequency of chan falls with in the bandwidth
@@ -965,6 +966,7 @@ cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
 
 	return max(t1, t2);
 }
+EXPORT_SYMBOL(cfg80211_chandef_dfs_cac_time);
 
 static bool cfg80211_secondary_chans_ok(struct wiphy *wiphy,
 					u32 center_freq, u32 bandwidth,
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 7c61752f6d83..c0eaa4d4f42e 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -462,29 +462,12 @@ int cfg80211_scan(struct cfg80211_registered_device *rdev);
 
 extern struct work_struct cfg80211_disconnect_work;
 
-/**
- * cfg80211_chandef_dfs_usable - checks if chandef is DFS usable
- * @wiphy: the wiphy to validate against
- * @chandef: the channel definition to check
- *
- * Checks if chandef is usable and we can/need start CAC on such channel.
- *
- * Return: true if all channels available and at least
- *	   one channel requires CAC (NL80211_DFS_USABLE)
- */
-bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
-				 const struct cfg80211_chan_def *chandef);
-
 void cfg80211_set_dfs_state(struct wiphy *wiphy,
 			    const struct cfg80211_chan_def *chandef,
 			    enum nl80211_dfs_state dfs_state);
 
 void cfg80211_dfs_channels_update_work(struct work_struct *work);
 
-unsigned int
-cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
-			      const struct cfg80211_chan_def *chandef);
-
 void cfg80211_sched_dfs_chan_update(struct cfg80211_registered_device *rdev);
 
 int
-- 
2.17.1

Re: [PATCH 1/3] wifi: cfg80211: export DFS CAC time and usable state helper functions

[Aditya Kumar Singh]

On 6/7/2023 18:16, Aditya Kumar Singh wrote:
> cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
> at least one channel in the chandef is in usable state or not. Also,
> cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
> time required for the given chandef.
> 
> Make these two functions visible to drivers by exporting their symbol
> to global list of kernel symbols.
> 
> Lower level drivers can make use of these two functions to be aware
> if CAC is required on the given chandef and for how long. For example
> drivers which maintains the CAC state internally can make use of these.
> 
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
> ---
Hi Johannes,

Do you have any comments for this patch?

- Aditya

Re: [PATCH 1/3] wifi: cfg80211: export DFS CAC time and usable state helper functions

[Kalle Valo]

Aditya Kumar Singh <quic_adisi@quicinc.com> writes:

> On 6/7/2023 18:16, Aditya Kumar Singh wrote:
>> cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
>> at least one channel in the chandef is in usable state or not. Also,
>> cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
>> time required for the given chandef.
>> Make these two functions visible to drivers by exporting their
>> symbol
>> to global list of kernel symbols.
>> Lower level drivers can make use of these two functions to be aware
>> if CAC is required on the given chandef and for how long. For example
>> drivers which maintains the CAC state internally can make use of these.
>> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
>> ---
> Hi Johannes,
>
> Do you have any comments for this patch?

The wireless trees are closed for July:

https://lore.kernel.org/all/87y1kncuh4.fsf@kernel.org/

Though I changed my plans and came back earlier.

And honestly these kind of emails don't really help. If a maintainer is
busy (and they usually are) sending more mails will do the opposite. If
you want to speed up your review help the maintainer somehow, for
example by reviewing or testing other patches etc which can reduce the
load of the maintainer.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 1/3] wifi: cfg80211: export DFS CAC time and usable state helper functions

[Jeff Johnson]

On 6/7/2023 5:46 AM, Aditya Kumar Singh wrote:
> cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
> at least one channel in the chandef is in usable state or not. Also,
> cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
> time required for the given chandef.
> 
> Make these two functions visible to drivers by exporting their symbol
> to global list of kernel symbols.
> 
> Lower level drivers can make use of these two functions to be aware
> if CAC is required on the given chandef and for how long. For example
> drivers which maintains the CAC state internally can make use of these.
> 
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>

Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com>

[PATCH 2/3] wifi: ath11k: fix CAC running state during virtual interface start

[Aditya Kumar Singh]

Currently channel definition's primary channel's DFS CAC time
as well as primary channel's state i.e usable are used to set
the CAC_RUNNING flag for the ath11k radio structure. However,
this is wrong since certain channel definition are possbile
where primary channel may not be a DFS channel but, secondary
channel is a DFS channel. For example - channel 36 with 160 MHz
bandwidth.
In such cases, the flag will not be set which is wrong.

Fix this issue by using cfg80211_chandef_dfs_usable() function
from cfg80211 which return trues if at least one channel is in
usable state.

While at it, modify the CAC running debug log message to print
the CAC time as well in milli-seconds.

Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/mac.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index a31b8e89684b..31982545266b 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -5,6 +5,7 @@
  */
 
 #include <net/mac80211.h>
+#include <net/cfg80211.h>
 #include <linux/etherdevice.h>
 #include <linux/bitfield.h>
 #include <linux/inetdevice.h>
@@ -7193,6 +7194,7 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif,
 	struct wmi_vdev_start_req_arg arg = {};
 	const struct cfg80211_chan_def *chandef = &ctx->def;
 	int ret = 0;
+	unsigned int dfs_cac_time;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
@@ -7272,20 +7274,21 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif,
 	ath11k_dbg(ab, ATH11K_DBG_MAC,  "vdev %pM started, vdev_id %d\n",
 		   arvif->vif->addr, arvif->vdev_id);
 
-	/* Enable CAC Flag in the driver by checking the channel DFS cac time,
-	 * i.e dfs_cac_ms value which will be valid only for radar channels
-	 * and state as NL80211_DFS_USABLE which indicates CAC needs to be
+	/* Enable CAC Flag in the driver by checking the all sub-channel's DFS
+	 * state as NL80211_DFS_USABLE which indicates CAC needs to be
 	 * done before channel usage. This flags is used to drop rx packets.
 	 * during CAC.
 	 */
 	/* TODO Set the flag for other interface types as required */
-	if (arvif->vdev_type == WMI_VDEV_TYPE_AP &&
-	    chandef->chan->dfs_cac_ms &&
-	    chandef->chan->dfs_state == NL80211_DFS_USABLE) {
+	if (arvif->vdev_type == WMI_VDEV_TYPE_AP && ctx->radar_enabled &&
+	    cfg80211_chandef_dfs_usable(ar->hw->wiphy, chandef)) {
 		set_bit(ATH11K_CAC_RUNNING, &ar->dev_flags);
+		dfs_cac_time = cfg80211_chandef_dfs_cac_time(ar->hw->wiphy,
+							     chandef);
 		ath11k_dbg(ab, ATH11K_DBG_MAC,
-			   "CAC Started in chan_freq %d for vdev %d\n",
-			   arg.channel.freq, arg.vdev_id);
+			   "CAC (for %u ms) Started in center_freq %d center_freq1 %d for vdev %d\n",
+			   dfs_cac_time, arg.channel.freq, chandef->center_freq1,
+			   arg.vdev_id);
 	}
 
 	ret = ath11k_mac_set_txbf_conf(arvif);
-- 
2.17.1

Re: [PATCH 2/3] wifi: ath11k: fix CAC running state during virtual interface start

[Jeff Johnson]

On 6/7/2023 5:46 AM, Aditya Kumar Singh wrote:
> Currently channel definition's primary channel's DFS CAC time
> as well as primary channel's state i.e usable are used to set
> the CAC_RUNNING flag for the ath11k radio structure. However,
> this is wrong since certain channel definition are possbile
> where primary channel may not be a DFS channel but, secondary
> channel is a DFS channel. For example - channel 36 with 160 MHz
> bandwidth.
> In such cases, the flag will not be set which is wrong.
> 
> Fix this issue by using cfg80211_chandef_dfs_usable() function
> from cfg80211 which return trues if at least one channel is in
> usable state.
> 
> While at it, modify the CAC running debug log message to print
> the CAC time as well in milli-seconds.
> 
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>

Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

> ---
>   drivers/net/wireless/ath/ath11k/mac.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index a31b8e89684b..31982545266b 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -5,6 +5,7 @@
>    */
>   
>   #include <net/mac80211.h>
> +#include <net/cfg80211.h>
>   #include <linux/etherdevice.h>
>   #include <linux/bitfield.h>
>   #include <linux/inetdevice.h>
> @@ -7193,6 +7194,7 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif,
>   	struct wmi_vdev_start_req_arg arg = {};
>   	const struct cfg80211_chan_def *chandef = &ctx->def;
>   	int ret = 0;
> +	unsigned int dfs_cac_time;
>   
>   	lockdep_assert_held(&ar->conf_mutex);
>   
> @@ -7272,20 +7274,21 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif,
>   	ath11k_dbg(ab, ATH11K_DBG_MAC,  "vdev %pM started, vdev_id %d\n",
>   		   arvif->vif->addr, arvif->vdev_id);
>   
> -	/* Enable CAC Flag in the driver by checking the channel DFS cac time,
> -	 * i.e dfs_cac_ms value which will be valid only for radar channels
> -	 * and state as NL80211_DFS_USABLE which indicates CAC needs to be
> +	/* Enable CAC Flag in the driver by checking the all sub-channel's DFS
> +	 * state as NL80211_DFS_USABLE which indicates CAC needs to be
>   	 * done before channel usage. This flags is used to drop rx packets.
>   	 * during CAC.
>   	 */
>   	/* TODO Set the flag for other interface types as required */
> -	if (arvif->vdev_type == WMI_VDEV_TYPE_AP &&
> -	    chandef->chan->dfs_cac_ms &&
> -	    chandef->chan->dfs_state == NL80211_DFS_USABLE) {
> +	if (arvif->vdev_type == WMI_VDEV_TYPE_AP && ctx->radar_enabled &&
> +	    cfg80211_chandef_dfs_usable(ar->hw->wiphy, chandef)) {
>   		set_bit(ATH11K_CAC_RUNNING, &ar->dev_flags);
> +		dfs_cac_time = cfg80211_chandef_dfs_cac_time(ar->hw->wiphy,
> +							     chandef);
>   		ath11k_dbg(ab, ATH11K_DBG_MAC,
> -			   "CAC Started in chan_freq %d for vdev %d\n",
> -			   arg.channel.freq, arg.vdev_id);
> +			   "CAC (for %u ms) Started in center_freq %d center_freq1 %d for vdev %d\n",
> +			   dfs_cac_time, arg.channel.freq, chandef->center_freq1,
> +			   arg.vdev_id);
>   	}
>   
>   	ret = ath11k_mac_set_txbf_conf(arvif);

Re: [PATCH 2/3] wifi: ath11k: fix CAC running state during virtual interface start

[Aditya Kumar Singh]

On 6/7/23 18:16, Aditya Kumar Singh wrote:
> Currently channel definition's primary channel's DFS CAC time
> as well as primary channel's state i.e usable are used to set
> the CAC_RUNNING flag for the ath11k radio structure. However,
> this is wrong since certain channel definition are possbile
> where primary channel may not be a DFS channel but, secondary
> channel is a DFS channel. For example - channel 36 with 160 MHz
> bandwidth.
> In such cases, the flag will not be set which is wrong.
> 
> Fix this issue by using cfg80211_chandef_dfs_usable() function
> from cfg80211 which return trues if at least one channel is in
> usable state.
> 
> While at it, modify the CAC running debug log message to print
> the CAC time as well in milli-seconds.
> 
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
Hi Kalle,

The upstream dependency got merged today. I have sent [v2] of this 
series which was just rebased on latest ToT and the [PATCH v2 1/3] got 
merged. With this, [PATCH v2 2/3] and [PATCH v2 3/3] are unblocked now.

- Aditya

Re: [PATCH 2/3] wifi: ath11k: fix CAC running state during virtual interface start

[Kalle Valo]

Aditya Kumar Singh <quic_adisi@quicinc.com> writes:

> On 6/7/23 18:16, Aditya Kumar Singh wrote:
>> Currently channel definition's primary channel's DFS CAC time
>> as well as primary channel's state i.e usable are used to set
>> the CAC_RUNNING flag for the ath11k radio structure. However,
>> this is wrong since certain channel definition are possbile
>> where primary channel may not be a DFS channel but, secondary
>> channel is a DFS channel. For example - channel 36 with 160 MHz
>> bandwidth.
>> In such cases, the flag will not be set which is wrong.
>> Fix this issue by using cfg80211_chandef_dfs_usable() function
>> from cfg80211 which return trues if at least one channel is in
>> usable state.
>> While at it, modify the CAC running debug log message to print
>> the CAC time as well in milli-seconds.
>> Tested-on: QCN9074 hw1.0 PCI
>> WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
> Hi Kalle,
>
> The upstream dependency got merged today. I have sent [v2] of this
> series which was just rebased on latest ToT and the [PATCH v2 1/3] got
> merged. With this, [PATCH v2 2/3] and [PATCH v2 3/3] are unblocked
> now.

Thanks, this is helpful. I still need to merge wireless-next into
ath-next but hopefully that happens next week.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 3/3] wifi: ath11k: fix Tx power value during active CAC

[Aditya Kumar Singh]

Tx power is fetched from firmware's pdev stats. However, during active
CAC, firmware does not fill the current Tx power and sends the max
initialised value filled during firmware init. If host sends this power
to user space, this is wrong since in certain situations, the Tx power
could be greater than the max allowed by the regulatory. Hence, host
should not be fetching the Tx power during an active CAC.

Fix this issue by returning Tx power as 0 during active CAC since it
is known that during CAC, there will be no transmission happening.

Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1

Fixes: 9a2aa68afe3d ("wifi: ath11k: add get_txpower mac ops")
Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/mac.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 31982545266b..9707910928b4 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -9045,6 +9045,14 @@ static int ath11k_mac_op_get_txpower(struct ieee80211_hw *hw,
 	if (ar->state != ATH11K_STATE_ON)
 		goto err_fallback;
 
+	/* Firmware doesn't provide Tx power during CAC hence no need to fetch
+	 * the stats.
+	 */
+	if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) {
+		mutex_unlock(&ar->conf_mutex);
+		return -EAGAIN;
+	}
+
 	req_param.pdev_id = ar->pdev->pdev_id;
 	req_param.stats_id = WMI_REQUEST_PDEV_STAT;
 
-- 
2.17.1

Re: [PATCH 3/3] wifi: ath11k: fix Tx power value during active CAC

[Jeff Johnson]

On 6/7/2023 5:46 AM, Aditya Kumar Singh wrote:
> Tx power is fetched from firmware's pdev stats. However, during active
> CAC, firmware does not fill the current Tx power and sends the max
> initialised value filled during firmware init. If host sends this power
> to user space, this is wrong since in certain situations, the Tx power
> could be greater than the max allowed by the regulatory. Hence, host
> should not be fetching the Tx power during an active CAC.
> 
> Fix this issue by returning Tx power as 0 during active CAC since it
> is known that during CAC, there will be no transmission happening.
> 
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
> 
> Fixes: 9a2aa68afe3d ("wifi: ath11k: add get_txpower mac ops")
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>

Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

> ---
>   drivers/net/wireless/ath/ath11k/mac.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index 31982545266b..9707910928b4 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -9045,6 +9045,14 @@ static int ath11k_mac_op_get_txpower(struct ieee80211_hw *hw,
>   	if (ar->state != ATH11K_STATE_ON)
>   		goto err_fallback;
>   
> +	/* Firmware doesn't provide Tx power during CAC hence no need to fetch
> +	 * the stats.
> +	 */
> +	if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) {
> +		mutex_unlock(&ar->conf_mutex);
> +		return -EAGAIN;
> +	}
> +
>   	req_param.pdev_id = ar->pdev->pdev_id;
>   	req_param.stats_id = WMI_REQUEST_PDEV_STAT;
>