| Message ID | 20220214223051.3610-1-quic_alokad@quicinc.com |
|---|---|
| Headers | show |
| Series | RU puncturing support | expand |
On Mon, 2022-02-14 at 14:30 -0800, Aloka Dixit wrote: > > + if (rdev->wiphy.ru_punct_supp_bw > NL80211_RU_PUNCT_SUPP_BW_320) > + return -EINVAL; > This check really belongs somewhere else, e.g. wiphy_register() or so? johannes
On 2/15/2022 4:37 PM, Aloka Dixit wrote: > On 2/15/2022 12:19 AM, Johannes Berg wrote: >> On Mon, 2022-02-14 at 14:30 -0800, Aloka Dixit wrote: >>> >>> + >>> +bool cfg80211_ru_punct_bitmap_valid(const struct cfg80211_chan_def >>> *chandef) >> >> Conceptually, I'm wondering if it really belongs into the chandef? Can >> you explain why it's part of the channel configuration? If you've got >> two chandefs with the same control channel, CCFS and bandwidth, but >> different puncturing, does it really make sense to treat them as two >> separate channel contexts, e.g. in mac80211? It seems strange to do >> that. >> >> johannes > > Added it here so that any command working with chandef will be updated > without any other change. > Example: During channel switch, user can provide a puncturing bitmap > with a new option I added in userspace, and because it is part of > chandef, the same code path validates it for that command automatically. > > Regarding if different puncturing pattern should be considered as a new > context - yes, depending on if it is HE or non-HE mode, the new bitmap > may be invalid and the operation should fail. > > Thanks, > Aloka Hi Johannes, Any comments on the above. Thanks.
On 10/12/2023 4:53 AM, Johannes Berg wrote: > Hi Aloka, all, > > Reviving an ancient thread, but this is where we discussed these things > last I think ... > > So I said, back at the beginning of 2022: > >>>> Conceptually, I'm wondering if it really belongs into the chandef? Can >>>> you explain why it's part of the channel configuration? If you've got >>>> two chandefs with the same control channel, CCFS and bandwidth, but >>>> different puncturing, does it really make sense to treat them as two >>>> separate channel contexts, e.g. in mac80211? It seems strange to do >>>> that. > > > And you replied two things: > >>> Added it here so that any command working with chandef will be updated >>> without any other change. >>> Example: During channel switch, user can provide a puncturing bitmap >>> with a new option I added in userspace, and because it is part of >>> chandef, the same code path validates it for that command automatically. >> >> Yeah but is it really a CSA/chanswitch if the puncturing changes? I >> don't think so? > > Which, well, I think I was confused about - it could we CSA/chanswitch > depending on what you actually want to do, i.e. it's up to the AP to do > this as just a puncturing bitmap update in the beacon, or with CSA. It > might do one way or the other depending on what it wants... > > > But also, I read this as being a bit more about the software POV, which > I didn't really think was the most important part. > > > And also, you said: > >>> Regarding if different puncturing pattern should be considered as a new >>> context - yes, depending on if it is HE or non-HE mode, the new bitmap >>> may be invalid and the operation should fail. > > Which I'm not sure I understood then, and certainly not sure I > understand now, but I said: > 802.11be allows only few patterns when AP is operating in non-OFDMA mode but if OFDMA is used then each 80 MHz sub-block can have a different puncturing pattern when BW > 80MHz. I know *_HE was not the best terminology, originally it was *_OFDMA but later changed because we decided to base the puncturing bitmap validation based on HE vs older modes. Function "cfg80211_ru_punct_bitmap_valid" added in this version first checks for non-OFDMA patterns, and only if "ru_punct_bitmap_supp_he" attribute is set by the userspace then it goes further to also check against patterns allowed for OFDMA. I could not find any other way to decide OFDMA vs non-OFDMA than letting userspace explicitly indicate latter. It would be great if you can provide your inputs on this. >> That wasn't really the question though. Consider this: >> >> Say you have STA + STA, if the first STA is connected to an AP with no >> puncturing, and the second STA is connected to an AP where the channel >> and bandwidth are the same, but some puncturing is done, should that >> really be two channel contexts as far as mac80211 is concerned, and thus >> require channels=2 in the interface combinations etc.? It doesn't seem >> right to me. >> >> Or consider AP + STA, where the AP is set up for some channel but the >> STA is connecting to an AP on the exact same channel, but with >> puncturing... Again, same thing, I don't think it should consume two >> channel resources. > > Which, actually, I've learned since that I was completely wrong about! > It should, and likely must, in fact be two separate channel contexts, > with all the limitations that implies. > > The thing is - back then I was making not just one, but in fact *two* > wrong assumptions: > > 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. > 2. You can simply transmit punctured PPDUs when on a non-punctured > channel, i.e. it's just a rate control decision. > > This is perhaps less important, but it's also not really true. > While you can clearly _transmit_ this way, that's not the only > thing - you also need to do the CCA before transmitting, and if > there's noise/interference on the punctured channel, you'd much > more rarely find the channel to be clear and be able to transmit > if this doesn't consider the puncturing, but that's something to > do sort of generally in the background for the transmit. > > It might be possible to work around #2, but I'm not sure it's possible > to work around #1? > > > So I think I have two questions: > A. Would you object if I moved the puncturing into the chandef after > all? > This is where I'm getting confused. The main reason to put in chandef was that I thought of the bitmap as a radio characteristic (not vif). 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. Moving the bitmap to cfg80211_ap_settings() meant that each AP vif can have different bitmap, and I'm guessing you similarly added for each STA vif context. Now if you move it back into chandef, how exactly will this work if you need different bitmaps? > 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. Thanks, Aloka.
Hi, > > > > Regarding if different puncturing pattern should be considered as a new > > > > context - yes, depending on if it is HE or non-HE mode, the new bitmap > > > > may be invalid and the operation should fail. > > > > Which I'm not sure I understood then, and certainly not sure I > > understand now, but I said: > > > > 802.11be allows only few patterns when AP is operating in non-OFDMA mode > but if OFDMA is used then each 80 MHz sub-block can have a different > puncturing pattern when BW > 80MHz. Right, but that's not the same, it's per PPDU more or less, no? I mean, you say in the trigger frame for example that some RU is not allocated. So is that relevant here? > I know *_HE was not the best terminology, originally it was *_OFDMA but > later changed because we decided to base the puncturing bitmap > validation based on HE vs older modes. > Function "cfg80211_ru_punct_bitmap_valid" added in this version first > checks for non-OFDMA patterns, and only if "ru_punct_bitmap_supp_he" > attribute is set by the userspace then it goes further to also check > against patterns allowed for OFDMA. > I could not find any other way to decide OFDMA vs non-OFDMA than letting > userspace explicitly indicate latter. > It would be great if you can provide your inputs on this. This ... doesn't exist upstream? OK actually it did exist in this patch though. Not sure I understand. Let's go back to what we have in the tree now. The only thing we check there is cfg80211_valid_disable_subchannel_bitmap(), which on the client at least is just for the "disabled subchannel bitmap" in the EHT operation. Are you thinking about (separately?) configuring the OFDMA puncturing? Which spec-wise you do per PPDU, controlled by the AP (trigger frame), I think? > > Which, actually, I've learned since that I was completely wrong about! > > It should, and likely must, in fact be two separate channel contexts, > > with all the limitations that implies. > > > > The thing is - back then I was making not just one, but in fact *two* > > wrong assumptions: > > > > 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? > > 2. You can simply transmit punctured PPDUs when on a non-punctured > > channel, i.e. it's just a rate control decision. > > > > This is perhaps less important, but it's also not really true. > > While you can clearly _transmit_ this way, that's not the only > > thing - you also need to do the CCA before transmitting, and if > > there's noise/interference on the punctured channel, you'd much > > more rarely find the channel to be clear and be able to transmit > > if this doesn't consider the puncturing, but that's something to > > do sort of generally in the background for the transmit. > > > > It might be possible to work around #2, but I'm not sure it's possible > > to work around #1? > > > > > > So I think I have two questions: > > A. Would you object if I moved the puncturing into the chandef after > > all? > > > > This is where I'm getting confused. :) > 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. I don't know though is if that's really true for all hardware in general, or just a side effect of our design. I could see that it might be possible to receive punctured/non-punctured without changing hardware configuration, and certainly that it might be doable to do CCA depending on which frame you're going to transmit. But in any case, as far as I'm told the hardware design we have doesn't allow that, so I think I'd like to move this to the chandef/chanctx, and then perhaps define a driver callback to determine compatibility, if needed? > Moving the bitmap to cfg80211_ap_settings() meant that each AP vif can > have different bitmap, and I'm guessing you similarly added for each STA > vif context. Yes. > 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
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.
On Wed, 2023-10-18 at 17:09 -0700, Aloka Dixit wrote: > 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. OK. > > > > 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. Thanks. (Also reminder, but yeah, I've also been busy otherwise.) > > > 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. > I'm okay if you want to move it back to chandef, in fact I myself can > send a series for it. I'm planning to start working on it now/soon. > 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. Not sure what you mean - I do in fact want two channel contexts for this? But please check if you need that or not, as discussed above - this is the "different puncturing pattern for different vifs" case. johannes
On Wed, 2023-11-08 at 13:58 +0100, Johannes Berg wrote: > > > I'm okay if you want to move it back to chandef, in fact I myself can > > send a series for it. > > I'm planning to start working on it now/soon. > Actually, I need to work on the wider bandwidth OFDMA thing first ... which has similar implications. See 802.11be D4.0 - 36.3.2.7/.8 "80/160 MHz operating non-AP EHT STAs participating in wider bandwidth OFDMA". Do you have any thoughts on that? I'm thinking I should add it to the chanctx in mac80211, which will add some implementation complexity there though but I think it makes more sense, i.e. adding an "ap_def" to struct ieee80211_chanctx_conf. johannes