On 5/7/2024 3:17 PM, Johannes Berg wrote:
On Sat, 2024-04-27 at 08:45 +0530, Karthikeyan Periyasamy wrote:
Currently, the interface combination input parameter num_different_channels
and iftype_num are directly filled in by the caller under the assumption
that all channels and interfaces belong to a single hardware device. This
assumption is incorrect for multi-device interface combinations because
each device supports a different set of channels and interfaces. As
discussed in [1], need to refactor the input parameters to encode enough
data to handle both single and multiple device interface combinations.
This can be achieved by encoding the frequency and interface type under
the interface entity itself. With this new input parameter structure, the
cfg80211 can classify and construct the device parameters, then verify them
against the device specific interface combinations.
^^ This should probably mention _something_ about links too :)
sure
[1]: https://lore.kernel.org/linux-wireless/ca70eeb3cdee1e8c3caee69db595bc8160eb4115.camel@xxxxxxxxxxxxxxxx/
Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@xxxxxxxxxxx>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 44 +++++--
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 60 +++++++--
.../net/wireless/quantenna/qtnfmac/cfg80211.c | 32 +++--
include/net/cfg80211.h | 37 +++++-
net/mac80211/util.c | 124 +++++++++++++++---
net/wireless/util.c | 56 ++++++--
6 files changed, 276 insertions(+), 77 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index 8993028709ec..3f9f5f56bd19 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -625,17 +625,25 @@ static int wil_cfg80211_validate_add_iface(struct wil6210_priv *wil,
{
int i;
struct wireless_dev *wdev;
- struct iface_combination_params params = {
- .num_different_channels = 1,
- };
+ struct iface_combination_params params = { 0 };
nit: just use "= {}".
sure, will address in the next version.
+ ifaces = kcalloc(total_iface, sizeof(*ifaces), GFP_KERNEL);
+ if (!ifaces)
+ return -ENOMEM;
+
+ list_for_each_entry(pos, &cfg->vif_list, list) {
+ if (params.num_iface >= total_iface)
+ continue;
??
Seems like that should be a WARN_ON or something?
WARN_ON() is good.
+ struct iface_combination_interface *ifaces = NULL;
+ u16 total_iface = 0;
+ int ret;
list_for_each_entry(pos, &cfg->vif_list, list)
- params.iftype_num[pos->wdev.iftype]++;
+ total_iface++;
- params.iftype_num[new_type]++;
- return cfg80211_check_combinations(cfg->wiphy, ¶ms);
+ ifaces = kcalloc(total_iface, sizeof(*ifaces), GFP_KERNEL);
No point in "= NULL" if you overwrite it immediately.
sure, will address in the next version.
--
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி
