Received-SPF: pass (google.com: domain of linux-wireless+bounces-8029-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <d86fc714375c8adf772ddc83ab4de514273034bd.camel@sipsolutions.net>
Subject: Re: [PATCH 00/13] wifi: Add multi physical hardware iface
 combination support
From: Johannes Berg <johannes@sipsolutions.net>
To: Felix Fietkau <nbd@nbd.name>, Karthikeyan Periyasamy
	 <quic_periyasa@quicinc.com>, ath12k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Date: Thu, 23 May 2024 18:41:43 +0200
In-Reply-To: <0dcc9afc-98ed-4f58-a368-79a5242a5bec@nbd.name>
References: <20240328072916.1164195-1-quic_periyasa@quicinc.com>
	 <0dcc9afc-98ed-4f58-a368-79a5242a5bec@nbd.name>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Wed, 2024-05-22 at 16:55 +0200, Felix Fietkau wrote:
>=20
> The key differences are:
> - Only band bitmask and optionally frequency ranges are provided, so no=
=20
> per-radio channel list
> This is easier to track and vastly reduces the amount of data sent to=20
> 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.

> - No integration with ifcomb. I don't really see the need for that one=
=20
> 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.

> - Validation happens in mac80211 instead of cfg80211, because that=20
> removes a lot of complexity

Sure, that's an internal thing. I don't really _like_ that too much, but
I also don't like the approach of building a huge list here. Perhaps a
reasonable compromise would be for mac80211 to pass some 'iterate' and
'getinfo' callbacks or something to a validation function, instead of
having to pre-build. Then the iteration can be in mac80211, but the
validation can be in mac80211, and IMHO that makes the separation and
how validation happens also easier to understand.

> The radio id is tracked per chanctx and only one chanctx per radio is=20
> allowed.

I may be misunderstanding this, but as phrased this seems completely
wrong? We absolutely support two channel contexts on a single radio
today, with e.g. a regular BSS connection and a P2P-client interface. So
not sure what you mean here, but I think it needs to be captured by the
driver what it actually supports here, and that's basically interface
combinations today for a single radio.

johannes