[PATCH 0/2] Update the channel list if the change in channel flags

[PATCH 0/2] Update the channel list if the change in channel flags

[Youghandhar Chintala]

There are connection failures in hidden SSID case when the device is with
default reg domain WW.
For WW reg domain most of the 5 GHz channels are passive. When device
listens to the beacon on that channel, the driver is updating its channel
flag but firmware is not aware of it and firmware is not sending probes
on that channels.
Due to this, we are seeing connection failures when the device is tries to
connect with hidden SSID AP.
Whenever the driver detects a channel flag change, send the updated channel
list command to the firmware.

Youghandhae Chintala (2):
  wifi: cfg80211: Add beacon hint notifier support
  wifi: ath10k: update the channel list if change in channel flags.

 drivers/net/wireless/ath/ath10k/mac.c | 11 +++++++++++
 include/net/cfg80211.h                |  2 ++
 net/wireless/reg.c                    |  5 ++++-
 3 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.38.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 1/2] wifi: cfg80211: Add beacon hint notifier support

[Youghandhar Chintala]

There are connection failures in hidden SSID case when the device is
with default reg domain WW.
For WW reg domain most of the 5 GHz channels are passive. When device
listens to the beacon on that channel, the driver is updating its
channel flag but firmware is not aware of it and firmware is not
sending probes on that channels.
Due to this, we are seeing connection failures when the device is trying
to connect with hidden SSID AP.

In the case of devices using the ath10k driver, it is required to update
the change in channel flags to firmware as well. Therefore, we need a
mechanism to notify the driver from the regulatory core regarding the
channel flag changes.
Adding a beacon hint notifier logic, so that drivers can register
callbacks to get notified whenever there is a change in channel flags.

Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
---
 include/net/cfg80211.h | 2 ++
 net/wireless/reg.c     | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 11a370e64143..07b2f2da77d6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5386,6 +5386,8 @@ struct wiphy {
 	void (*reg_notifier)(struct wiphy *wiphy,
 			     struct regulatory_request *request);
 
+	void (*beacon_hint_notifier)(struct wiphy *wiphy);
+
 	/* fields below are read-only, assigned by cfg80211 */
 
 	const struct ieee80211_regdomain __rcu *regd;
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index c3d950d29432..6ea2455d4964 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2219,8 +2219,11 @@ static void handle_reg_beacon(struct wiphy *wiphy, unsigned int chan_idx,
 		channel_changed = true;
 	}
 
-	if (channel_changed)
+	if (channel_changed) {
 		nl80211_send_beacon_hint_event(wiphy, &chan_before, chan);
+		if (wiphy->beacon_hint_notifier)
+			wiphy->beacon_hint_notifier(wiphy);
+	}
 }
 
 /*
-- 
2.38.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 1/2] wifi: cfg80211: Add beacon hint notifier support

[Johannes Berg]

> +++ b/include/net/cfg80211.h
> @@ -5386,6 +5386,8 @@ struct wiphy {
>  	void (*reg_notifier)(struct wiphy *wiphy,
>  			     struct regulatory_request *request);
>  
> +	void (*beacon_hint_notifier)(struct wiphy *wiphy);


missing documentation, for sure

Also this should probably be in the ops, rather than here? Not sure why
reg_notifier is here.

> -	if (channel_changed)
> +	if (channel_changed) {
>  		nl80211_send_beacon_hint_event(wiphy, &chan_before, chan);
> +		if (wiphy->beacon_hint_notifier)
> +			wiphy->beacon_hint_notifier(wiphy);
> +	}

This also seems excessive if you're not even passing the channel - you
call it for every (affected) channel, but you don't tell it anything
about the channel? Seems strange.


However ...

Why is this even needed? You should always get reg_notifier after this
anyway?

johannes

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 1/2] wifi: cfg80211: Add beacon hint notifier support

[Youghandhar Chintala (Temp)]

On 1/18/2023 10:00 PM, Johannes Berg wrote:
>> +++ b/include/net/cfg80211.h
>> @@ -5386,6 +5386,8 @@ struct wiphy {
>>   	void (*reg_notifier)(struct wiphy *wiphy,
>>   			     struct regulatory_request *request);
>>   
>> +	void (*beacon_hint_notifier)(struct wiphy *wiphy);
>
> missing documentation, for sure
I will add the missed documentation in next version of patch.
> Also this should probably be in the ops, rather than here? Not sure why
> reg_notifier is here.
I followed reg_notifier implementation.
>> -	if (channel_changed)
>> +	if (channel_changed) {
>>   		nl80211_send_beacon_hint_event(wiphy, &chan_before, chan);
>> +		if (wiphy->beacon_hint_notifier)
>> +			wiphy->beacon_hint_notifier(wiphy);
>> +	}
> This also seems excessive if you're not even passing the channel - you
> call it for every (affected) channel, but you don't tell it anything
> about the channel? Seems strange.
I will address this in next version of patch.
>
>
> However ...
>
> Why is this even needed? You should always get reg_notifier after this
> anyway?
>
> johannes

Currently when channel flag changed through the beacon hints are not 
informed to driver.

reg_notifier will be triggered for regdomain changes but not for channel flag changes due to beacon hints.

Regards,
Youghandhar


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 1/2] wifi: cfg80211: Add beacon hint notifier support

[Johannes Berg]

> > 
> > Why is this even needed? You should always get reg_notifier after this
> > anyway?
> 
> Currently when channel flag changed through the beacon hints are not 
> informed to driver.
> 
> reg_notifier will be triggered for regdomain changes but not for channel flag changes due to beacon hints.
> 

So maybe triggering reg notifier once would be sufficient, a la Wen's
patch that I recently merged?

johannes

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 1/2] wifi: cfg80211: Add beacon hint notifier support

[Wen Gong]

On 2/15/2023 11:10 PM, Johannes Berg wrote:
>>> Why is this even needed? You should always get reg_notifier after this
>>> anyway?
>> Currently when channel flag changed through the beacon hints are not
>> informed to driver.
>>
>> reg_notifier will be triggered for regdomain changes but not for channel flag changes due to beacon hints.
>>
> So maybe triggering reg notifier once would be sufficient, a la Wen's
> patch that I recently merged?

My patch only take effect for this flag, not for all wiphy.

if (wiphy->flags & WIPHY_FLAG_NOTIFY_REGDOM_BY_DRIVER)

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit?id=d99975c4953eb79e389d4630e848435c700e2dfc

>
> johannes
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 1/2] wifi: cfg80211: Add beacon hint notifier support

[Johannes Berg]

On Wed, 2023-02-22 at 08:22 +0800, Wen Gong wrote:
> On 2/15/2023 11:10 PM, Johannes Berg wrote:
> > > > Why is this even needed? You should always get reg_notifier after this
> > > > anyway?
> > > Currently when channel flag changed through the beacon hints are not
> > > informed to driver.
> > > 
> > > reg_notifier will be triggered for regdomain changes but not for channel flag changes due to beacon hints.
> > > 
> > So maybe triggering reg notifier once would be sufficient, a la Wen's
> > patch that I recently merged?
> 
> My patch only take effect for this flag, not for all wiphy.
> 
> if (wiphy->flags & WIPHY_FLAG_NOTIFY_REGDOM_BY_DRIVER)
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit?id=d99975c4953eb79e389d4630e848435c700e2dfc
> 

Oh sure, I meant this patch could do something similar, instead of
introducing a new notifier callback.

johannes

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 2/2] wifi: ath10k: update the channel list if change in channel flags.

[Youghandhar Chintala]

There are connection failures in hidden SSID case when the device is
with default reg domain WW.
For WW reg domain most of the 5 GHz channels are passive. When device
listens to the beacon on that channel, the driver is updating its
channel flag but firmware is not aware of it and firmware is not
sending probes on that channels.
Due to this, we are seeing connection failures when a device is trying
to connect with hidden SSID AP.
Register beacon hint notifier to the regulatory core so that driver get
notified when there is a change in channel flags. Driver's notifier
callback will send the updated flags to the firmware.

Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2.c10-00754-QCAHLSWMTPL-1
Tested-on: QCA6174 hw3.2 SDIO WLAN.RMH.4.4.1-00174

Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index ec8d5b29bc72..91a957295456 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3534,6 +3534,15 @@ static void ath10k_mac_update_channel_list(struct ath10k *ar,
 	}
 }
 
+static void ath10k_mac_beacon_notifier(struct wiphy *wiphy)
+{
+	struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+	struct ath10k *ar = hw->priv;
+
+	if (ath10k_update_channel_list(ar))
+		ath10k_warn(ar, "failed to update channel list\n");
+}
+
 static void ath10k_reg_notifier(struct wiphy *wiphy,
 				struct regulatory_request *request)
 {
@@ -10286,6 +10295,8 @@ int ath10k_mac_register(struct ath10k *ar)
 			goto err_unregister;
 	}
 
+	ar->hw->wiphy->beacon_hint_notifier = ath10k_mac_beacon_notifier;
+
 	return 0;
 
 err_unregister:
-- 
2.38.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 2/2] wifi: ath10k: update the channel list if change in channel flags.

[Wen Gong]

On 12/22/2022 8:42 PM, Youghandhar Chintala wrote:
...
>   
> +static void ath10k_mac_beacon_notifier(struct wiphy *wiphy)
> +{
> +	struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
> +	struct ath10k *ar = hw->priv;
> +
> +	if (ath10k_update_channel_list(ar))
> +		ath10k_warn(ar, "failed to update channel list\n");
> +}
> +

Will this called while scan is running?
On ath11k, if send channel list to firmware, then the running scan will 
be cancel and removed.
I guess this is same for ath10k.

> ...

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 2/2] wifi: ath10k: update the channel list if change in channel flags.

[Youghandhar Chintala (Temp)]

On 1/19/2023 7:56 AM, Wen Gong wrote:
> On 12/22/2022 8:42 PM, Youghandhar Chintala wrote:
> ...
>>   +static void ath10k_mac_beacon_notifier(struct wiphy *wiphy)
>> +{
>> +    struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
>> +    struct ath10k *ar = hw->priv;
>> +
>> +    if (ath10k_update_channel_list(ar))
>> +        ath10k_warn(ar, "failed to update channel list\n");
>> +}
>> +
>
> Will this called while scan is running?
> On ath11k, if send channel list to firmware, then the running scan 
> will be cancel and removed.
> I guess this is same for ath10k.
>
>> ...

Yes Wen. You are right.

Regards,

Youghandhar


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 0/2] Update the channel list if the change in channel flags

[Kalle Valo]

Youghandhar Chintala <quic_youghand@quicinc.com> writes:

> There are connection failures in hidden SSID case when the device is with
> default reg domain WW.
> For WW reg domain most of the 5 GHz channels are passive. When device
> listens to the beacon on that channel, the driver is updating its channel
> flag but firmware is not aware of it and firmware is not sending probes
> on that channels.
> Due to this, we are seeing connection failures when the device is tries to
> connect with hidden SSID AP.
> Whenever the driver detects a channel flag change, send the updated channel
> list command to the firmware.
>
> Youghandhae Chintala (2):
>   wifi: cfg80211: Add beacon hint notifier support
>   wifi: ath10k: update the channel list if change in channel flags.

If you submit patches again, please _always_ increase the version number
(ie. this one should be v2). Now there are two v1 patchsets and
difficult for maintainers to pick which one to take.

But don't resend the patchset just because of this. Hopefully this mail
makes it clear that this is v2.

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

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k