Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f0383c934075f90d50af6e91aa3307057f6188f8.camel@sipsolutions.net>
Subject: Re: CMD_REMAIN_ON_CHANNEL vs CMD_FRAME (offchannel)
From:   Johannes Berg <johannes@sipsolutions.net>
To:     James Prestwood <prestwoj@gmail.com>,
        linux-wireless@vger.kernel.org
Date:   Tue, 25 Apr 2023 20:10:36 +0200
In-Reply-To: <98f53583-400d-665f-d264-92e9f3e67280@gmail.com>
References: <6d3869e9-6d8f-f7cf-2fe9-b8188a02d086@gmail.com>
         <9b4b6d3e-8840-f37a-52ec-6ea391fa67db@gmail.com>
         <f5d14ce0b5053e4a99529677614d6d6aa797ac81.camel@sipsolutions.net>
         <98f53583-400d-665f-d264-92e9f3e67280@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

Hi,

> > I can't tell with the information you gave - depends on the waiting
> > period for a response frame you ask for, I guess? With HW ROC (if you'r=
e
> > using iwlwifi) we cannot extend the previous period, see
> > ieee80211_coalesce_hw_started_roc().
>=20
> This is purely virtual at the moment, but in my case its looking like it=
=20
> cannot be extended either since the CMD_FRAME just queues a separate=20
> request.

Hm. Not sure then? I guess then you'd be falling into the max duration
or so?

                if (!local->ops->remain_on_channel) {
                        /* If there's no hardware remain-on-channel, and
                         * doing so won't push us over the maximum r-o-c
                         * we allow, then we can just add the new one to
                         * the list and mark it as having started now.
                         * If it would push over the limit, don't try to
                         * combine with other started ones (that haven't
                         * been running as long) but potentially sort it
                         * with others that had the same fate.
                         */
                        unsigned long now =3D jiffies;
                        u32 elapsed =3D jiffies_to_msecs(now - tmp->start_t=
ime);
                        struct wiphy *wiphy =3D local->hw.wiphy;
                        u32 max_roc =3D wiphy->max_remain_on_channel_durati=
on;

                        if (elapsed + roc->duration > max_roc) {
                                combine_started =3D false;
                                continue;
                        }

Or maybe that logic here is broken somewhat ...

> > Not sure I understand this part. ROC is fine mostly for the "wait for
> > some frame and send a response", but not so much suited for "send a
> > frame and wait for a response" part. So 3-way-handshakes are iffy with
> > it...
>=20
> Yeah, and it actually has worked great for the entire DPP procedure=20
> using the same channel (presence -> auth request -> auth response ->=20
> auth confirm) assuming both sides respond in a timely fashion.
>=20
> The comes when changing the channel after the auth request. The auth=20
> request gets queued separately, which then delays the ROC and we=20
> can't/shouldn't send anything until ROC starts. The only strange thing=
=20
> is we actually receive the auth response on the new channel before the=
=20
> ROC for that new channel even starts. Its like the hardware and driver=
=20
> aren't quite in sync.

Did you say hwsim? That'd be weird. Though in hwsim I think you have an
additional quirk - it never really *leaves* the original channel it's
connected on, it kind of sticks around on *both* which isn't real but
some kind of simplification there. We might want to fix that eventually.
But not sure it's connected already in this case?

> But anyways I think its best to use ROC for presence (waiting for=20
> announcements) but then use CMD_FRAME for the rest of the protocol.

Right, that's pretty much the intent for this kind of thing. Similar in
P2P where we designed all this, really.

johannes