On 10/21/2022 5:34 PM, Johannes Berg wrote:
On Tue, 2022-09-20 at 15:35 +0530, Vasanthakumar Thiagarajan wrote:
The prerequisite for MLO support in cfg80211/mac80211 is that all
the links participating in MLO must be from the same wiphy/ieee80211_hw.
To meet this expectation, some drivers may need to group multiple discrete
hardware each acting as a link in MLO under one wiphy. Though most of the
hardware abstractions must be handled within the driver itself, it may be
required to have some sort of mapping while describing interface
combination capabilities for each of the underlying hardware. This commit
adds an advertisement provision for drivers supporting such MLO mode of
operation.
Capability advertisement such as the number of supported channels for each
physical hardware has been identified to support some of the common use
cases.
Signed-off-by: Vasanthakumar Thiagarajan <quic_vthiagar@xxxxxxxxxxx>
---
include/net/cfg80211.h | 24 +++++++++
net/wireless/core.c | 109 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 133 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e09ff87146c1..4662231ad068 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5088,6 +5088,18 @@ struct wiphy_iftype_akm_suites {
int n_akm_suites;
};
+/**
+ * struct ieee80211_supported_chans_per_hw - supported channels as per the
+ * underlying physical hardware configuration
+ *
+ * @n_chans: number of channels in @chans
+ * @chans: list of channels supported by the constituent hardware
+ */
+struct ieee80211_chans_per_hw {
+ int n_chans;
nit: unsigned?
+ * @hw_chans: list of the channels supported by every constituent underlying
"every" here refers to the fact that you have all the channels, right? I
mean, hw_chans is per hardware, basically.
Correct, it refers all the channels supported per hardware registered
something like below
hw_chans[0] = {
// 2 GHz radio
<num_2ghz_chans>,
{
<ieee80211_channel_2ghz_1>,
<ieee80211_channel_2ghz_2>, ...
}
}
hw_chans[1] = {
{
// 5 GHz radio
<num_5ghz_chans>,
{
<ieee80211_channel_5ghz_1>,
<ieee80211_channel_5ghz_2>, ...
}
}
...
+ * hardware. Drivers abstracting multiple discrete hardware (radio) under
+ * one wiphy can advertise the list of channels supported by each physical
+ * hardware in this list. Underlying hardware specific channel list can be
+ * used while describing interface combination for each of them.
+ * @num_hw: number of underlying hardware for which the channels list are
+ * advertised in @hw_chans.
+ *
*/
struct wiphy {
struct mutex mtx;
@@ -5445,6 +5466,9 @@ struct wiphy {
u8 ema_max_profile_periodicity;
u16 max_num_akm_suites;
+ struct ieee80211_chans_per_hw **hw_chans;
+ int num_hw;
also here, maybe unsigned.
+static bool
+cfg80211_hw_chans_in_supported_list(struct wiphy *wiphy,
+ const struct ieee80211_chans_per_hw *chans)
+{
+ enum nl80211_band band;
+ struct ieee80211_supported_band *sband;
+ bool found;
+ int i, j;
+
+ for (i = 0; i < chans->n_chans; i++) {
+ found = false;
nit: you can move the variable declaration here
bool found = false;
\n
since you don't need it outside the loop.
+ for (i = 0; i < wiphy->num_hw; i++) {
+ for (j = 0; j < wiphy->num_hw; j++) {
+ const struct ieee80211_chans_per_hw *hw_chans1;
+ const struct ieee80211_chans_per_hw *hw_chans2;
+
+ if (i == j)
+ continue;
It's symmetric, if I read the code above right, so you can do
for (j = i; j < ...; j++)
in the second loop and avoid this?
Sure
+ hw_chans = wiphy->hw_chans[i];
+ if (!cfg80211_hw_chans_in_supported_list(wiphy, hw_chans)) {
+ WARN_ON(1);
I can kind of see the point in not using WARN_ON(!cfg80211_hw_chans...)
since it gets really long, but maybe just remove the warning?
+ if (cfg80211_validate_per_hw_chans(&rdev->wiphy)) {
+ WARN_ON(1);
+ return -EINVAL;
Anyway you'll have one here, and it's not directly visible which
condition failed anyway.
And you could use WARN_ON(validate(...)) here :)
Sure, thanks!
Vasanth
