2023-06-29 04:01:30

by Abhishek Kumar

[permalink] [raw]
Subject: [PATCH 1/2] wifi: cfg80211: call reg_call_notifier on beacon hints

Currently the channel property updates are not propagated to
driver. This causes issues in the discovery of hidden SSIDs and
fails to connect to them.
This change defines a new wiphy flag which when enabled by vendor
driver, the reg_call_notifier callback will be trigger on beacon
hints. This ensures that the channel property changes are visible
to the vendor driver. The vendor changes the channels for active
scans. This fixes the discovery issue of hidden SSID.

Signed-off-by: Abhishek Kumar <[email protected]>
---

include/net/cfg80211.h | 3 +++
net/wireless/reg.c | 20 ++++++++++++--------
2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9e04f69712b1..48e6ebcdacb3 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4783,6 +4783,8 @@ struct cfg80211_ops {
* @WIPHY_FLAG_SUPPORTS_EXT_KCK_32: The device supports 32-byte KCK keys.
* @WIPHY_FLAG_NOTIFY_REGDOM_BY_DRIVER: The device could handle reg notify for
* NL80211_REGDOM_SET_BY_DRIVER.
+ * @WIPHY_FLAG_CHANNEL_CHANGE_ON_BEACON: reg_call_notifier() is called if driver
+ * set this flag to update channels on beacon hints.
*/
enum wiphy_flags {
WIPHY_FLAG_SUPPORTS_EXT_KEK_KCK = BIT(0),
@@ -4809,6 +4811,7 @@ enum wiphy_flags {
WIPHY_FLAG_SUPPORTS_5_10_MHZ = BIT(22),
WIPHY_FLAG_HAS_CHANNEL_SWITCH = BIT(23),
WIPHY_FLAG_NOTIFY_REGDOM_BY_DRIVER = BIT(24),
+ WIPHY_FLAG_CHANNEL_CHANGE_ON_BEACON = BIT(25),
};

/**
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 26f11e4746c0..c76bfaad650b 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2149,6 +2149,13 @@ static bool reg_is_world_roaming(struct wiphy *wiphy)
return false;
}

+static void reg_call_notifier(struct wiphy *wiphy,
+ struct regulatory_request *request)
+{
+ if (wiphy->reg_notifier)
+ wiphy->reg_notifier(wiphy, request);
+}
+
static void handle_reg_beacon(struct wiphy *wiphy, unsigned int chan_idx,
struct reg_beacon *reg_beacon)
{
@@ -2156,6 +2163,7 @@ static void handle_reg_beacon(struct wiphy *wiphy, unsigned int chan_idx,
struct ieee80211_channel *chan;
bool channel_changed = false;
struct ieee80211_channel chan_before;
+ struct regulatory_request *lr = get_last_request();

sband = wiphy->bands[reg_beacon->chan.band];
chan = &sband->channels[chan_idx];
@@ -2181,8 +2189,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->flags & WIPHY_FLAG_CHANNEL_CHANGE_ON_BEACON)
+ reg_call_notifier(wiphy, lr);
+ }
}

/*
@@ -2325,13 +2336,6 @@ static void reg_process_ht_flags(struct wiphy *wiphy)
reg_process_ht_flags_band(wiphy, wiphy->bands[band]);
}

-static void reg_call_notifier(struct wiphy *wiphy,
- struct regulatory_request *request)
-{
- if (wiphy->reg_notifier)
- wiphy->reg_notifier(wiphy, request);
-}
-
static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev)
{
struct cfg80211_chan_def chandef = {};
--
2.41.0.162.gfafddb0af9-goog



2023-06-29 04:01:31

by Abhishek Kumar

[permalink] [raw]
Subject: [PATCH 2/2] ath10k: mac: enable WIPHY_FLAG_CHANNEL_CHANGE_ON_BEACON on ath10k

Enabling this flag, ensures that reg_call_notifier is called
on beacon hints from handle_reg_beacon in cfg80211. This call
propagates the channel property changes to ath10k driver, thus
changing the channel property from passive scan to active scan
based on beacon hints.
Once the channels are rightly changed from passive to active,the
connection to hidden SSID does not fail.

Signed-off-by: Abhishek Kumar <[email protected]>
---

drivers/net/wireless/ath/ath10k/mac.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7675858f069b..12df3228b120 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -10033,6 +10033,7 @@ int ath10k_mac_register(struct ath10k *ar)

ar->hw->wiphy->features |= NL80211_FEATURE_STATIC_SMPS;
ar->hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
+ ar->hw->wiphy->flags |= WIPHY_FLAG_CHANNEL_CHANGE_ON_BEACON;

if (ar->ht_cap_info & WMI_HT_CAP_DYNAMIC_SMPS)
ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;
--
2.41.0.162.gfafddb0af9-goog


2023-10-03 14:44:40

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] ath10k: mac: enable WIPHY_FLAG_CHANNEL_CHANGE_ON_BEACON on ath10k

Abhishek Kumar <[email protected]> wrote:

> Enabling this flag, ensures that reg_call_notifier is called
> on beacon hints from handle_reg_beacon in cfg80211. This call
> propagates the channel property changes to ath10k driver, thus
> changing the channel property from passive scan to active scan
> based on beacon hints.
> Once the channels are rightly changed from passive to active,the
> connection to hidden SSID does not fail.
>
> Signed-off-by: Abhishek Kumar <[email protected]>

There's no Tested-on tag, on which hardware/firmware did you test this?

This flag is now enabled on ALL ath10k supported hardware: SNOC, PCI, SDIO and
maybe soon USB. I'm just wondering can we trust that this doesn't break
anything.

--
https://patchwork.kernel.org/project/linux-wireless/patch/20230629035254.2.I23c5e51afcc6173299bb2806c8c38364ad15dd63@changeid/

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

2023-10-14 05:22:15

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] ath10k: mac: enable WIPHY_FLAG_CHANNEL_CHANGE_ON_BEACON on ath10k

Kalle Valo <[email protected]> writes:

> Abhishek Kumar <[email protected]> wrote:
>
>> Enabling this flag, ensures that reg_call_notifier is called
>> on beacon hints from handle_reg_beacon in cfg80211. This call
>> propagates the channel property changes to ath10k driver, thus
>> changing the channel property from passive scan to active scan
>> based on beacon hints.
>> Once the channels are rightly changed from passive to active,the
>> connection to hidden SSID does not fail.
>>
>> Signed-off-by: Abhishek Kumar <[email protected]>
>
> There's no Tested-on tag, on which hardware/firmware did you test this?
>
> This flag is now enabled on ALL ath10k supported hardware: SNOC, PCI, SDIO and
> maybe soon USB. I'm just wondering can we trust that this doesn't break
> anything.

Jeff, what are your thoughts on this? I'm worried how different ath10k
firmwares can be and if this breaks something.

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

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

2023-10-16 16:49:40

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 2/2] ath10k: mac: enable WIPHY_FLAG_CHANNEL_CHANGE_ON_BEACON on ath10k

On 10/13/2023 10:20 PM, Kalle Valo wrote:
> Kalle Valo <[email protected]> writes:
>
>> Abhishek Kumar <[email protected]> wrote:
>>
>>> Enabling this flag, ensures that reg_call_notifier is called
>>> on beacon hints from handle_reg_beacon in cfg80211. This call
>>> propagates the channel property changes to ath10k driver, thus
>>> changing the channel property from passive scan to active scan
>>> based on beacon hints.
>>> Once the channels are rightly changed from passive to active,the
>>> connection to hidden SSID does not fail.
>>>
>>> Signed-off-by: Abhishek Kumar <[email protected]>
>>
>> There's no Tested-on tag, on which hardware/firmware did you test this?
>>
>> This flag is now enabled on ALL ath10k supported hardware: SNOC, PCI, SDIO and
>> maybe soon USB. I'm just wondering can we trust that this doesn't break
>> anything.
>
> Jeff, what are your thoughts on this? I'm worried how different ath10k
> firmwares can be and if this breaks something.
>

Since the 1/2 patch is already in pull-request: wireless-next-2023-10-06
I went through the logic of that again. It would have been nice if that
actually described how it fixes the problem. What actually causes a
channel to change from passive to active?

Note the existing logic prior to the 1/2 patch already updates the wiphy
and userspace with the updated channel flags, so it seems reasonable to
also update the driver

However, this led me down the rabbit hole of trying to figure out what
happens if a beacon hint causes us to change a channel from passive to
active, but then that AP goes away. What, if anything, causes the
channel to revert back to passive? I'm not immediately seeing that logic
anywhere.

My concern is that we have an AP with a hidden SSID on a DFS channel,
and as a result of a beacon hint we switch that channel to active scan.
But then later that AP detects radar and vacates the channel. Then we
potentially have stations doing active scan on a DFS channel with an
active radar.

Hopefully this is all handled, and it just isn't obvious in my
admittedly very quick 10 minute scan of the code.

And as far as the 2/2 patch, note this logic is all dependent upon
reg_is_world_roaming(wiphy) returning true, so ath10k impact would
really depend upon the board regulatory settings, whether configured for
a fixed regulatory domain/country code or configured for world roaming.

/jeff