Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <587c4a25-4977-b2d7-a587-f2a742105a43@quicinc.com>
Date:   Wed, 18 Oct 2023 17:09:58 -0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/3] cfg80211: validate RU puncturing bitmap
To:     Johannes Berg <johannes@sipsolutions.net>,
        <linux-wireless@vger.kernel.org>
References: <20220214223051.3610-1-quic_alokad@quicinc.com>
 <20220214223051.3610-3-quic_alokad@quicinc.com>
 <a9813545a25cd63f71cc31476230514a80350802.camel@sipsolutions.net>
 <6cf56be5-16d6-2bcd-150f-bf29f98b7f1b@quicinc.com>
 <58fdd62041c0388740cabea5a421c5417f959124.camel@sipsolutions.net>
 <9fd4a3097e078c1fe2acd5fbd0c559b0390daa49.camel@sipsolutions.net>
 <460cb443-868c-ec05-7aec-5b1eee381ae2@quicinc.com>
 <28b099e7a37824f0b59ab824e67b3437485e45d5.camel@sipsolutions.net>
Content-Language: en-US
From:   Aloka Dixit <quic_alokad@quicinc.com>
In-Reply-To: <28b099e7a37824f0b59ab824e67b3437485e45d5.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/18/2023 5:58 AM, Johannes Berg wrote:
> 
> Are you thinking about (separately?) configuring the OFDMA puncturing?
> Which spec-wise you do per PPDU, controlled by the AP (trigger frame), I
> think?
> 

Need to study the spec again so not any time soon.
Will send a new series if it is needed.

> 
>>>      1. The DSP/radio can receive punctured PPDUs if listening on the non
>>>         punctured channel.
>>>         
>>>         At least for our device that's not true, not sure about ath12k? It
>>>         seems you have a per-peer puncturing configuration even, but that
>>>         seems odd, and it's always just set to the vif puncturing
>>>         configuration.
>>>         
>>
>> Yes, same vif puncturing pattern is assigned for all the peers
>> associated on that vif, but firmware requires it to be sent separately
>> for each peer.
> 
> OK, thanks.
> 
> What if it differs for different vifs?
> 

So far that use-case hasn't come up but I'm confirming if we really need 
that support or not. Will get back you.

> 
>> The main reason to put in chandef was that I thought of the bitmap as a
>> radio characteristic (not vif).
> 
> Right.
> 
>> But after you brought up that AP+STA
>> mode can have different bitmaps, even though all other channel
>> characteristics (width, cf etc) are same, I realized my original
>> assumption wrong incorrect.
> 
> So I convinced you, I guess, but what I'm saying is that - at least as
> far as our hardware is concerned - I was wrong!
> 
> Thing is: you're not just transmitting with this bitmap, you're also
> listening - for both CCA and RX - in a specific way. And at least the
> way our hardware works, we apparently can't do puncturing just based on
> the preamble, and can't do CCA depending on the next frame.
> 
> So that means the (non-OFDMA) puncturing bitmap *does* in fact become a
> radio characteristic.
> 

Got it.

> 
>> Now if you move it back into chandef, how exactly will this work if you
>> need different bitmaps?
> 
> You'd get two chanctx since it's not compatible, unless we define some
> extra callback or hw flags to determine what's treated as compatible and
> what isn't. But see above - I actually want that, now that I know how
> the HW works :)
> 
>>>      B. How does ath12k cope #1/#2 above? Would we need to have a callback
>>>         to the driver to compare if two channel contexts are compatible or
>>>         not (e.g. if they have different puncturing), or does ath12k also
>>>         have limitations on RX/TX that mean it would actually prefer two
>>>         channel contexts for the cases I had outlined in the quoted text
>>>         above (STA+STA/AP+STA)?
>>>
>>
>> If we do end up moving the bitmap back to chandef, we may need some
>> changes, because as I said above, when I originally added it I hadn't
>> thought of different bitmaps for each vif.
>> But can you give an example of what you would consider as compatible
>> channel contexts and what would be incompatible? I'm not clear on that part.
> 
> Easy example:
> 
>   * control channel 36, 80 MHz, puncturing bitmap 0x2
>   * control channel 36, 80 MHz, puncturing bitmap 0
> 
> Contrary to what I thought and said before, I want to treat these as
> *not* compatible now, and allocate two channel contexts if I end up
> having to do this.
> 
> johannes

I'm okay if you want to move it back to chandef, in fact I myself can 
send a series for it.
As far as two contexts are concerned, sounds like you don't need that 
for your use-case. And I will confirm if we need it or not.

Thanks.