Received-SPF: softfail (google.com: domain of transitioning linux-wireless-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <3fa8911111ff380615ca5b3ffcd61644fae5e8ea.camel@sipsolutions.net>
Subject: Re: [RFC v2] cfg80211: Indicate MLO connection info in connect and
 roam callbacks
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Veerendranath Jakkam <quic_vjakkam@quicinc.com>
Cc:     linux-wireless@vger.kernel.org
Date:   Wed, 08 Jun 2022 09:59:21 +0200
In-Reply-To: <e75ddf50-348b-eaf2-4033-6a32ef65a41c@quicinc.com>
References: <1654536690-12588-1-git-send-email-quic_vjakkam@quicinc.com>
         <a2cc93f959394a3430755d4664d9f2abeabb3ab7.camel@sipsolutions.net>
         <e75ddf50-348b-eaf2-4033-6a32ef65a41c@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.2 (3.44.2-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2022-06-08 at 13:15 +0530, Veerendranath Jakkam wrote:
> On 6/8/2022 1:01 PM, Johannes Berg wrote:
> > On Mon, 2022-06-06 at 23:01 +0530, Veerendranath Jakkam wrote:
> >=20
> > > +	if (cr->status =3D=3D WLAN_STATUS_SUCCESS) {
> > > +		for_each_valid_link(cr, link) {
> > > +			if (!cr->links[link].bss)
> > > +				break;
> > > +		}
> > > +
> > > +		WARN_ON_ONCE((!cr->valid_links && link !=3D 1) ||
> > > +			     (cr->valid_links &&
> > > +			      link !=3D ARRAY_SIZE(wdev->links)));
> > > +
> > I will say I'm not super happy with using the link variable after the
> > loop, that always feels a bit magic to me, especially if the loop is
> > hidden like that...
> >=20
> > But I guess I don't see a lot of alternatives here, other than open-
> > coding it, or keeping track of "how many BSSes do I have".
>=20
>=20
> Since we need to WARN even if single BSS is not present I think we can=
=20
> use "bss_not_found" flag?
>=20
> >=20
> > Actually, for the MLO case, is this even valid? link[14] could be set,
> > so you wouldn't break, ending up with link=3D=3D15? Or am I confused?
> In MLO case the link value will be always 15 after loop completes if=20
> bsses are available for all valid links since the check is only for the=
=20
> valid links
>=20
> so, In above case also when "link=3D=3D15" the condition fails and WARN w=
ill=20
> be skipped right.
>=20

Ah, indeed. I was thinking of the 'break', but you only get there for
valid links and the valid links should indeed have a .bss pointer.

OK, so I guess let's leave it as is, that way we check both cases
accurately.

johannes