Received-SPF: pass (google.com: domain of linux-wireless+bounces-8074-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <72b97a7b-7d8a-46a6-a613-8461b5480e3f@quicinc.com>
Date: Mon, 27 May 2024 12:28:17 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 00/13] wifi: Add multi physical hardware iface combination
 support
To: Johannes Berg <johannes@sipsolutions.net>, Felix Fietkau <nbd@nbd.name>,
        Karthikeyan Periyasamy <quic_periyasa@quicinc.com>,
        <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20240328072916.1164195-1-quic_periyasa@quicinc.com>
 <0dcc9afc-98ed-4f58-a368-79a5242a5bec@nbd.name>
 <d86fc714375c8adf772ddc83ab4de514273034bd.camel@sipsolutions.net>
Content-Language: en-US
From: Aditya Kumar Singh <quic_adisi@quicinc.com>
In-Reply-To: <d86fc714375c8adf772ddc83ab4de514273034bd.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

On 5/23/24 22:11, Johannes Berg wrote:
> On Wed, 2024-05-22 at 16:55 +0200, Felix Fietkau wrote:
>>
>> The key differences are:
>> - Only band bitmask and optionally frequency ranges are provided, so no
>> per-radio channel list
>> This is easier to track and vastly reduces the amount of data sent to
>> user space in the wiphy dump
> 
> That makes sense, though in your RFC I'd probably remove the band bitmap
> thing, and make the frequency range not be optional. Perhaps in the
> kernel it could be filled in by cfg80211 via a band enum (taking
> lowest/highest frequency in the band's channels that are there), but I
> don't know if I'd want to have to check with this all optional
> throughout the kernel and the userspace advertising API.
> 

Agree on that band bitmap thing.

>> - No integration with ifcomb. I don't really see the need for that one
>> at this point. It can easily be added later if it's actually needed.
> 
> I mean, sure? But I think that's being lazy, I think everyone else
> thinks it's actually needed. I just got a question about interface
> combinations being broken on iwlwifi because we advertise AP interface
> type in a combination with two channels, which can't be right. I'm
> fixing that, but actually it _would_ be good to know for hardware that
> actually does physically have the capability to operate on two channels,
> and then have the bands etc.
> 
> So I do think (some) integration with interface combinations is needed.

Yes! At least for radar detection, some changes are required.
Grouping 5 GHz radio with any other radio, we will need it or else 
current interface combination check will fail to register the single 
wiphy hardware.

We have a check like this in wiphy_verify_combinations() -

/* DFS only works on one channel. */
if (WARN_ON(c->radar_detect_widths &&
         (c->num_different_channels > 1)))
     return -EINVAL;

And if the proposal is to keep c->num_different_channels advertised as 1 
only from the driver then in [RFC 2/2], this change -

+    if (ieee80211_num_chanctx(local) >= ieee80211_max_num_channels(local))
+        return false;

will never allow to create a channel ctx in any of the other radios 
except the one which is brought up first right?

ieee80211_max_num_channels() uses the interface combination advertised 
value from driver, so that will be 1 and hence if you bring up on 5 GHz 
after 2 GHz, that will lead to 2 >= 1 and hence if condition will be 
true and it will return false. Then 5 GHz bring up will fail. So not so 
clear on this approach.

Considering above points, feels like under any situation without making 
interface combination changes, simply it can not be done. Some or other 
issue might pop up later once we try to enable all features in MLO as 
well which currently exist in non-MLO case. :)