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.
> + * 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?
> + 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 :)
johannes
