| Message ID | 20240130140918.1172387-5-quic_adisi@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: cfg80211/mac80211: add link_id handling in AP channel switch during Multi-Link Operation | expand |
On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote: > Add changes to start a channel switch as well as finalize it on link basis > in order to support CSA with MLO as well. > FYI, this had a number of conflicts with my other work - please check the tree now. johannes
On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote: > On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote: > > Add changes to start a channel switch as well as finalize it on link basis > > in order to support CSA with MLO as well. > > > > FYI, this had a number of conflicts with my other work - please check > the tree now. > Also here btw, some hostap test with hwsim would be nice - though again don't know how well hostapd supports all this yet. johannes
On 2/8/24 19:35, Johannes Berg wrote: > On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote: >> On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote: >>> Add changes to start a channel switch as well as finalize it on link basis >>> in order to support CSA with MLO as well. >>> >> >> FYI, this had a number of conflicts with my other work - please check >> the tree now. >> > > Also here btw, some hostap test with hwsim would be nice - though again > don't know how well hostapd supports all this yet. > > johannes Sure, I will rebase on latest ToT and will add some hwsim test cases (if supported well in hostapd) as well.
On 2/12/24 12:10, Aditya Kumar Singh wrote: > On 2/8/24 19:35, Johannes Berg wrote: >> On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote: >>> On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote: >>>> Add changes to start a channel switch as well as finalize it on link >>>> basis >>>> in order to support CSA with MLO as well. >>>> >>> >>> FYI, this had a number of conflicts with my other work - please check >>> the tree now. >>> >> >> Also here btw, some hostap test with hwsim would be nice - though again >> don't know how well hostapd supports all this yet. >> >> johannes > > Sure, I will rebase on latest ToT and will add some hwsim test cases (if > supported well in hostapd) as well. > > My bad! I see you have applied the changes. Thanks :) Looks good to me. Let me see on the hwsim test cases and if possible send it soon for review to Jouni.
On Mon, 2024-02-12 at 12:48 +0530, Aditya Kumar Singh wrote: > My bad! I see you have applied the changes. Thanks :) Right, sorry that wasn't clear. > Looks good to me. OK, thanks for checking. > Let me see on the hwsim test cases and if possible > send it soon for review to Jouni. So ... I'm looking at the client side, and thinking about that. According to the spec, multi-link element should be present in beacons of APs affiliated with the same MLD if one of the (other) links is doing CSA, and then have also the CSA counters of course, relative to the target link's TBTT (of course.) Theoretically, both mac80211 and hostapd today support different beacon interval on different links, I believe. This makes the whole thing of including CSA for link A on beacons/probe responses transmitted on link B very tricky, because you have to know the timing, etc. For the CSA counter of a link _itself_ we have counter_offsets_beacon and counter_offsets_presp (for probe response offload) in struct cfg80211_csa_settings, and also counter offsets in struct cfg80211_mgmt_tx_params for sending probe responses. But ... for the cross-link information in the MLE this gets way more tricky? Especially if the beacon interval is different - we couldn't just count down, we'd have to fill in the information when we know where the frame is transmitted. For probe responses maybe we can afford to not be perfect, but for beacons it better be right - so we have to calculate the right counter value(s) for (all of) the switching link(s) according to the current TSF, TSF offset and TBTT - not all of which we even have in the driver? I can see a few ways of implementing this: a) Punt to firmware and it parses the multi-link element etc. to find the places to update. But that's awful, it requires parsing possibly fragmented MLE with fragmented subelements containing the CSA elements inside ... b) Punt to firmware and give it a (possibly long) list of offsets K_N where to put the N'th counter for link K when transmitting the frame. c) Require the get_tsf operation and add an operation to retrieve TSF offset (**), and then calculate the TBTT for each link when beacon_get is called with the per-link beacon intervals and update the values correctly if CSA is in progress on any link ... requires against parsing like in (a) or offset information like in (b), but at least now it's in software? For probe responses this could be a bit off I guess, but maybe that doesn't matter as much - probe responses are not authenticated so a client probably shouldn't use them for real CSA anyway. (**) which I guess we need anyway for hostapd to put it into beacons etc.? d) Require beacon intervals to be the same, and then just count down? Still requires the offset information etc., but that's also not great because if the configuration happens during a TBTT on any of the links (some before, some after), it'll be very wrong. So not very inclined to do this one ... Do you have any plans for any of this? I'm mostly asking right now because I want a reliable way to test the work I'm trying to do on the client side though, so I could also live with some hacks (inject through debugfs?), but having it for real would be nice. johannes
On 2/12/24 20:16, Johannes Berg wrote: > So ... I'm looking at the client side, and thinking about that. > According to the spec, multi-link element should be present in beacons > of APs affiliated with the same MLD if one of the (other) links is doing > CSA, and then have also the CSA counters of course, relative to the > target link's TBTT (of course.) > Yes correct, you are referring to critical update(s) right? > Theoretically, both mac80211 and hostapd today support different beacon > interval on different links, I believe. > > This makes the whole thing of including CSA for link A on beacons/probe > responses transmitted on link B very tricky, because you have to know > the timing, etc. At least handling for probe response seems to be a bit easier since there we need not maintain any timer as such (as you have said too - we need not be perfect there). But beacons, as you said, it is indeed bit tricky to handle full support considering both links could beacon at different intervals. > > For the CSA counter of a link _itself_ we have counter_offsets_beacon > and counter_offsets_presp (for probe response offload) in struct > cfg80211_csa_settings, and also counter offsets in struct > cfg80211_mgmt_tx_params for sending probe responses. > > But ... for the cross-link information in the MLE this gets way more > tricky? Especially if the beacon interval is different - we couldn't > just count down, we'd have to fill in the information when we know where > the frame is transmitted. For probe responses maybe we can afford to not > be perfect, but for beacons it better be right - so we have to calculate > the right counter value(s) for (all of) the switching link(s) according > to the current TSF, TSF offset and TBTT - not all of which we even have > in the driver? > Yes correct :) > I can see a few ways of implementing this: > > a) Punt to firmware and it parses the multi-link element etc. to > find the places to update. But that's awful, it requires parsing > possibly fragmented MLE with fragmented subelements containing the > CSA elements inside ... > > b) Punt to firmware and give it a (possibly long) list of offsets K_N > where to put the N'th counter for link K when transmitting the > frame. > At least for beacons if firmware takes care of it then it will be good. Firmware could maintain the counter (for the affected link) and (I assume it will be aware of the partner links) so whenever partner link transmits a beacon it could add the CSA (or such IEs) in per STA profile of the reporting link. It could get the contents from the affected link but actual value of the counter could be filled from the global counter being maintained. But not sure whether we can force every driver's firmware to do so? Let aside actual drivers, adding the test case for mac80211_hw_sim would also be tricky since there beacon Tx is handled in software only. > c) Require the get_tsf operation and add an operation to retrieve TSF > offset (**), and then calculate the TBTT for each link when > beacon_get is called with the per-link beacon intervals and update > the values correctly if CSA is in progress on any link ... requires > against parsing like in (a) or offset information like in (b), but > at least now it's in software? > For probe responses this could be a bit off I guess, but maybe that > doesn't matter as much - probe responses are not authenticated so a > client probably shouldn't use them for real CSA anyway. > > (**) which I guess we need anyway for hostapd to put it into beacons > etc.? Yes I'm too anticipating hostapd changes to support the critical update. > > d) Require beacon intervals to be the same, and then just count down? > Still requires the offset information etc., but that's also not > great because if the configuration happens during a TBTT on any of > the links (some before, some after), it'll be very wrong. So not > very inclined to do this one ... > Agreed. Can't enforce the intervals to be same. > > Do you have any plans for any of this? I do have some idea (possibly half baked code) for handling critical updates in probe responses. Beacons, I assume firmware takes care hence I have changes to mark it in the link_conf so that drivers could read that and give it to its firmware. Handling in beacon at kernel level, I need to think of it since anyways hw_sim test case is good to have :) Do we have any data right now, how many actual drivers have offloaded beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)? If we could see majority are using firmware then we could first add support for such drivers then we can think about handling it in kernel? > > I'm mostly asking right now because I want a reliable way to test the > work I'm trying to do on the client side though, so I could also live > with some hacks (inject through debugfs?), but having it for real would > be nice. Okay. I'm right now working to add basic test case in hw_sim for MLO CSA (on first link) as we discussed. There is need of a few mac80211 changes in order to support for other links. I'm checking those now. Once done with these, then I'll take up critical updates.
On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote: > On 2/12/24 20:16, Johannes Berg wrote: > > So ... I'm looking at the client side, and thinking about that. > > According to the spec, multi-link element should be present in beacons > > of APs affiliated with the same MLD if one of the (other) links is doing > > CSA, and then have also the CSA counters of course, relative to the > > target link's TBTT (of course.) > > Yes correct, you are referring to critical update(s) right? I was actually thinking the critical update flag is rather not all that interesting, but the inclusion of the CSA-related elements in a (partial) per-STA profile in the Multi-Link Element is. Those contain the countdown fields, so need to be updated - according to the TBTT of the link doing the CSA, not the link sending it. > > Theoretically, both mac80211 and hostapd today support different beacon > > interval on different links, I believe. > > > > This makes the whole thing of including CSA for link A on beacons/probe > > responses transmitted on link B very tricky, because you have to know > > the timing, etc. > > At least handling for probe response seems to be a bit easier since > there we need not maintain any timer as such (as you have said too - we > need not be perfect there). OTOH, if we already need some infrastructure for beacons, we can also use that infrastructure to fill in probe responses? Might be easier overall, because otherwise you'd have to start timers in hostapd etc. if you actually wanted to fill that in hostapd, and that's messy ... And as a complete aside: link removal _also_ has a countdown, but I'm not sure that affects cross-link, but if it does we might want a more general cross-link counter infrastructure? Though that might not mesh well with how (your) firmware handles it? > But beacons, as you said, it is indeed bit > tricky to handle full support considering both links could beacon at > different intervals. Indeed. > > For the CSA counter of a link _itself_ we have counter_offsets_beacon > > and counter_offsets_presp (for probe response offload) in struct > > cfg80211_csa_settings, and also counter offsets in struct > > cfg80211_mgmt_tx_params for sending probe responses. > > > > But ... for the cross-link information in the MLE this gets way more > > tricky? Especially if the beacon interval is different - we couldn't > > just count down, we'd have to fill in the information when we know where > > the frame is transmitted. For probe responses maybe we can afford to not > > be perfect, but for beacons it better be right - so we have to calculate > > the right counter value(s) for (all of) the switching link(s) according > > to the current TSF, TSF offset and TBTT - not all of which we even have > > in the driver? > > > > Yes correct :) :) > > I can see a few ways of implementing this: > > > > a) Punt to firmware and it parses the multi-link element etc. to > > find the places to update. But that's awful, it requires parsing > > possibly fragmented MLE with fragmented subelements containing the > > CSA elements inside ... > > > > b) Punt to firmware and give it a (possibly long) list of offsets K_N > > where to put the N'th counter for link K when transmitting the > > frame. > > > > At least for beacons if firmware takes care of it then it will be good. Note that I'm basically asking you, Mediatek and Realtek how this should work. We're almost certainly not going to support multi-link AP in our device any time soon, and we could adjust the firmware to it anyway if needed. So while I agree that punting it to firmware would be somewhat nice, we still have to agree on (a) or (b), because (b) requires more information from hostapd etc., but (a) requires _substantially_ more (and very complex) parsing in firmware... If you wanted to support multi-BSSID, then it's even more complex, which would argue for (b)? I was trying to keep opinions out of this, but personally, I really don't like (a), it puts too much complexity in the hands of firmware engineers ;-) > Firmware could maintain the counter (for the affected link) and (I > assume it will be aware of the partner links) so whenever partner link > transmits a beacon it could add the CSA (or such IEs) in per STA profile > of the reporting link. It could get the contents from the affected link > but actual value of the counter could be filled from the global counter > being maintained. But not sure whether we can force every driver's > firmware to do so? I'm not sure I understand that description - how would firmware even know which elements to take from what partner link etc.? I think hostapd still has to add the elements, and firmware would just populate the counter field(s)? But yeah that requires knowing from which partner link to populate. If you really wanted the firmware to handle the partner link information completely it'd not only have to parse the Multi-Link Element on other links but also _modify_, and know what to do, etc. - I don't think that'll really work so well. Think about it some more: On the link doing the CSA you'd have to parse the beacon template and understand where the CSA-related elements are (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them aside. Then on the partner link(s) parse the Multi-Link Element, see if there's a partial STA profile already for some reason, merge the CSA- related elements into that, update the counters; all while first removing the 2 levels of fragmentation, and then re-doing them ... I'm not even sure how I'd implement that in hwsim, and there I already have all the helper functions and pretty much unlimited memory/code size, so doing it in firmware seems awful? Not to mention that it would never extend for future stuff, since new elements wouldn't be copied over, firmware wouldn't necessarily know the right order to merge them with existing elements in the per-STA profile, etc. > Let aside actual drivers, adding the test case for mac80211_hw_sim would > also be tricky since there beacon Tx is handled in software only. Yeah but that's easy, especially if we go with (b) approach, even mac80211 basically has all the necessary information to handle hwsim's model of beacon transmission, we could add a little bit of additional help there too (e.g. pass the TBTT to beacon_get). > > c) Require the get_tsf operation and add an operation to retrieve TSF > > offset (**), and then calculate the TBTT for each link when > > beacon_get is called with the per-link beacon intervals and update > > the values correctly if CSA is in progress on any link ... requires > > against parsing like in (a) or offset information like in (b), but > > at least now it's in software? > > For probe responses this could be a bit off I guess, but maybe that > > doesn't matter as much - probe responses are not authenticated so a > > client probably shouldn't use them for real CSA anyway. > > > > (**) which I guess we need anyway for hostapd to put it into beacons > > etc.? > > Yes I'm too anticipating hostapd changes to support the critical update. Oh hostapd will have to make changes here for everything - the (**) footnote was about how hostapd knows the TSF offset between links? Unless we're planning to always keep the TSF offset zero? > > Do you have any plans for any of this? > > I do have some idea (possibly half baked code) for handling critical > updates in probe responses. Beacons, I assume firmware takes care hence > I have changes to mark it in the link_conf so that drivers could read > that and give it to its firmware. > Handling in beacon at kernel level, I need to think of it since anyways > hw_sim test case is good to have :) > > Do we have any data right now, how many actual drivers have offloaded > beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)? > If we could see majority are using firmware then we could first add > support for such drivers then we can think about handling it in kernel? I agree that we can go with one approach for now, but I think we should agree on the first model of how firmware does this. I'm not convinced that your description above where firmware even magically handles propagating the CSA elements to partner links will really work? But see above. > > I'm mostly asking right now because I want a reliable way to test the > > work I'm trying to do on the client side though, so I could also live > > with some hacks (inject through debugfs?), but having it for real would > > be nice. > > Okay. I'm right now working to add basic test case in hw_sim for MLO CSA > (on first link) as we discussed. There is need of a few mac80211 changes > in order to support for other links. I'm checking those now. Once done > with these, then I'll take up critical updates. > > >
Uh, sorry, accidentally sent too soon. > > > I'm mostly asking right now because I want a reliable way to test the > > > work I'm trying to do on the client side though, so I could also live > > > with some hacks (inject through debugfs?), but having it for real would > > > be nice. > > > > Okay. I'm right now working to add basic test case in hw_sim for MLO CSA > > (on first link) as we discussed. There is need of a few mac80211 changes > > in order to support for other links. I'm checking those now. Once done > > with these, then I'll take up critical updates. OK. I also have a few patches that I was working on. Mostly client related, but maybe these would be relevant for you as well: https://p.sipsolutions.net/f9e976055a616eda.txt https://p.sipsolutions.net/054dcb0502247890.txt Sorry about the format - once I've fully tested them etc. I'll send them out. johannes
On 2/13/24 14:12, Johannes Berg wrote: > On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote: >> On 2/12/24 20:16, Johannes Berg wrote: >>> So ... I'm looking at the client side, and thinking about that. >>> According to the spec, multi-link element should be present in beacons >>> of APs affiliated with the same MLD if one of the (other) links is doing >>> CSA, and then have also the CSA counters of course, relative to the >>> target link's TBTT (of course.) >> >> Yes correct, you are referring to critical update(s) right? > > I was actually thinking the critical update flag is rather not all that > interesting, but the inclusion of the CSA-related elements in a > (partial) per-STA profile in the Multi-Link Element is. Those contain > the countdown fields, so need to be updated - according to the TBTT of > the link doing the CSA, not the link sending it. > Yes correct, this CSA thing is interesting but there is one more thing - handling of the BSS param change count (BPCC). Let's say the affected link undergoes CSA, then in its Basic Multi Link IE, BPCC should be incremented. But at the same time, in RNR of its partner links, where this affected link is there, the BPCC should also be incremented. All though here it is not incremented with every beacon Tx, but still change in one affects beacon of other partner link(s) at least once. >>> Theoretically, both mac80211 and hostapd today support different beacon >>> interval on different links, I believe. >>> >>> This makes the whole thing of including CSA for link A on beacons/probe >>> responses transmitted on link B very tricky, because you have to know >>> the timing, etc. >> >> At least handling for probe response seems to be a bit easier since >> there we need not maintain any timer as such (as you have said too - we >> need not be perfect there). > > OTOH, if we already need some infrastructure for beacons, we can also > use that infrastructure to fill in probe responses? Might be easier > overall, because otherwise you'd have to start timers in hostapd etc. if > you actually wanted to fill that in hostapd, and that's messy ... > Correct! > And as a complete aside: link removal _also_ has a countdown, but I'm > not sure that affects cross-link, but if it does we might want a more > general cross-link counter infrastructure? Though that might not mesh > well with how (your) firmware handles it? > Yes. >> But beacons, as you said, it is indeed bit >> tricky to handle full support considering both links could beacon at >> different intervals. > > Indeed. > >>> For the CSA counter of a link _itself_ we have counter_offsets_beacon >>> and counter_offsets_presp (for probe response offload) in struct >>> cfg80211_csa_settings, and also counter offsets in struct >>> cfg80211_mgmt_tx_params for sending probe responses. >>> >>> But ... for the cross-link information in the MLE this gets way more >>> tricky? Especially if the beacon interval is different - we couldn't >>> just count down, we'd have to fill in the information when we know where >>> the frame is transmitted. For probe responses maybe we can afford to not >>> be perfect, but for beacons it better be right - so we have to calculate >>> the right counter value(s) for (all of) the switching link(s) according >>> to the current TSF, TSF offset and TBTT - not all of which we even have >>> in the driver? >>> >> >> Yes correct :) > > :) > >>> I can see a few ways of implementing this: >>> >>> a) Punt to firmware and it parses the multi-link element etc. to >>> find the places to update. But that's awful, it requires parsing >>> possibly fragmented MLE with fragmented subelements containing the >>> CSA elements inside ... >>> >>> b) Punt to firmware and give it a (possibly long) list of offsets K_N >>> where to put the N'th counter for link K when transmitting the >>> frame. >>> >> >> At least for beacons if firmware takes care of it then it will be good. > > Note that I'm basically asking you, Mediatek and Realtek how this should > work. We're almost certainly not going to support multi-link AP in our > device any time soon, and we could adjust the firmware to it anyway if > needed. > > So while I agree that punting it to firmware would be somewhat nice, we > still have to agree on (a) or (b), because (b) requires more information > from hostapd etc., but (a) requires _substantially_ more (and very > complex) parsing in firmware... If you wanted to support multi-BSSID, > then it's even more complex, which would argue for (b)? > Correct. Both has got its own advantages and disadvantages. Need to agree on any one so that all can be benefited. Let's hear from other folks. > I was trying to keep opinions out of this, but personally, I really > don't like (a), it puts too much complexity in the hands of firmware > engineers ;-) > >> Firmware could maintain the counter (for the affected link) and (I >> assume it will be aware of the partner links) so whenever partner link >> transmits a beacon it could add the CSA (or such IEs) in per STA profile >> of the reporting link. It could get the contents from the affected link >> but actual value of the counter could be filled from the global counter >> being maintained. But not sure whether we can force every driver's >> firmware to do so? > > I'm not sure I understand that description - how would firmware even > know which elements to take from what partner link etc.? I think hostapd > still has to add the elements, and firmware would just populate the > counter field(s)? But yeah that requires knowing from which partner link > to populate. > Yeah so underlying assumption/thought was - 1. Firmware is aware of partner links 2. Host driver just needs to update the beacon template of the affected link and some how indicate to firmware that this is critical update. 3. As per spec, there are around 21-22 changes classified as critical update and among those only handful of those which has adding the IE in per STA profile. So firmware could possibly maintain a table to see if the affected link's beacon template has such IEs which if present needs to be added in partner links, it could add it as and when partner link transmits a beacon. 4. Once CSA is finished, the host would update the beacon template which will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to per STA profile as well of the reporting links. But yeah adding to per STA profile when already elements are there and then first de-fragment and then add and then later fragment would be a tedious job :) > If you really wanted the firmware to handle the partner link information > completely it'd not only have to parse the Multi-Link Element on other > links but also _modify_, and know what to do, etc. - I don't think > that'll really work so well. > > Think about it some more: On the link doing the CSA you'd have to parse > the beacon template and understand where the CSA-related elements are > (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them > aside. Then on the partner link(s) parse the Multi-Link Element, see if > there's a partial STA profile already for some reason, merge the CSA- > related elements into that, update the counters; all while first > removing the 2 levels of fragmentation, and then re-doing them ... > Yeah complex on firmware level. But even if we solve this on kernel level, existing drivers who have offloaded beacon to firmware from 11ax or before itself, still need to handle it? It would looks ugly that till 11ax it is offloaded and from 11be it is again put back to kernel? > I'm not even sure how I'd implement that in hwsim, and there I already > have all the helper functions and pretty much unlimited memory/code > size, so doing it in firmware seems awful? > Yes. > Not to mention that it would never extend for future stuff, since new > elements wouldn't be copied over, firmware wouldn't necessarily know the > right order to merge them with existing elements in the per-STA profile, > etc. > Yeah so if we go with handle on firmware level, as and when spec changes, firmware changes are required too. > >> Let aside actual drivers, adding the test case for mac80211_hw_sim would >> also be tricky since there beacon Tx is handled in software only. > > Yeah but that's easy, especially if we go with (b) approach, even > mac80211 basically has all the necessary information to handle hwsim's > model of beacon transmission, we could add a little bit of additional > help there too (e.g. pass the TBTT to beacon_get). > >>> c) Require the get_tsf operation and add an operation to retrieve TSF >>> offset (**), and then calculate the TBTT for each link when >>> beacon_get is called with the per-link beacon intervals and update >>> the values correctly if CSA is in progress on any link ... requires >>> against parsing like in (a) or offset information like in (b), but >>> at least now it's in software? >>> For probe responses this could be a bit off I guess, but maybe that >>> doesn't matter as much - probe responses are not authenticated so a >>> client probably shouldn't use them for real CSA anyway. >>> >>> (**) which I guess we need anyway for hostapd to put it into beacons >>> etc.? >> >> Yes I'm too anticipating hostapd changes to support the critical update. > > Oh hostapd will have to make changes here for everything - the (**) > footnote was about how hostapd knows the TSF offset between links? > Unless we're planning to always keep the TSF offset zero? > >>> Do you have any plans for any of this? >> >> I do have some idea (possibly half baked code) for handling critical >> updates in probe responses. Beacons, I assume firmware takes care hence >> I have changes to mark it in the link_conf so that drivers could read >> that and give it to its firmware. >> Handling in beacon at kernel level, I need to think of it since anyways >> hw_sim test case is good to have :) >> >> Do we have any data right now, how many actual drivers have offloaded >> beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)? >> If we could see majority are using firmware then we could first add >> support for such drivers then we can think about handling it in kernel? > > I agree that we can go with one approach for now, but I think we should > agree on the first model of how firmware does this. I'm not convinced > that your description above where firmware even magically handles > propagating the CSA elements to partner links will really work? But see > above. > >>> I'm mostly asking right now because I want a reliable way to test the >>> work I'm trying to do on the client side though, so I could also live >>> with some hacks (inject through debugfs?), but having it for real would >>> be nice. >> >> Okay. I'm right now working to add basic test case in hw_sim for MLO CSA >> (on first link) as we discussed. There is need of a few mac80211 changes >> in order to support for other links. I'm checking those now. Once done >> with these, then I'll take up critical updates. >> >> >>
On Tue, 2024-02-13 at 15:48 +0530, Aditya Kumar Singh wrote: > On 2/13/24 14:12, Johannes Berg wrote: > > On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote: > > > On 2/12/24 20:16, Johannes Berg wrote: > > > > So ... I'm looking at the client side, and thinking about that. > > > > According to the spec, multi-link element should be present in beacons > > > > of APs affiliated with the same MLD if one of the (other) links is doing > > > > CSA, and then have also the CSA counters of course, relative to the > > > > target link's TBTT (of course.) > > > > > > Yes correct, you are referring to critical update(s) right? > > > > I was actually thinking the critical update flag is rather not all that > > interesting, but the inclusion of the CSA-related elements in a > > (partial) per-STA profile in the Multi-Link Element is. Those contain > > the countdown fields, so need to be updated - according to the TBTT of > > the link doing the CSA, not the link sending it. > > > > Yes correct, this CSA thing is interesting but there is one more thing - > handling of the BSS param change count (BPCC). Let's say the affected > link undergoes CSA, then in its Basic Multi Link IE, BPCC should be > incremented. But at the same time, in RNR of its partner links, where > this affected link is there, the BPCC should also be incremented. > All though here it is not incremented with every beacon Tx, but still > change in one affects beacon of other partner link(s) at least once. Yes, I know, but IMHO that's not all that interesting, I'd think hostapd could handle that. > > Note that I'm basically asking you, Mediatek and Realtek how this should > > work. We're almost certainly not going to support multi-link AP in our > > device any time soon, and we could adjust the firmware to it anyway if > > needed. > > > > So while I agree that punting it to firmware would be somewhat nice, we > > still have to agree on (a) or (b), because (b) requires more information > > from hostapd etc., but (a) requires _substantially_ more (and very > > complex) parsing in firmware... If you wanted to support multi-BSSID, > > then it's even more complex, which would argue for (b)? > > > > Correct. Both has got its own advantages and disadvantages. Need to > agree on any one so that all can be benefited. Let's hear from other folks. Do you _have_ firmware for this already, perhaps? I mean, I'm happy to be designing this "greenfield", but ... it seems unlikely at this point? Also what about ath12k with the multi-device changes, would the driver have to handle it? Or I guess you have to sync the TSF somehow anyway, so you could handle it based on TSF/TBTT? > > I was trying to keep opinions out of this, but personally, I really > > don't like (a), it puts too much complexity in the hands of firmware > > engineers ;-) > > > > > Firmware could maintain the counter (for the affected link) and (I > > > assume it will be aware of the partner links) so whenever partner link > > > transmits a beacon it could add the CSA (or such IEs) in per STA profile > > > of the reporting link. It could get the contents from the affected link > > > but actual value of the counter could be filled from the global counter > > > being maintained. But not sure whether we can force every driver's > > > firmware to do so? > > > > I'm not sure I understand that description - how would firmware even > > know which elements to take from what partner link etc.? I think hostapd > > still has to add the elements, and firmware would just populate the > > counter field(s)? But yeah that requires knowing from which partner link > > to populate. > > > > Yeah so underlying assumption/thought was - > 1. Firmware is aware of partner links Sure. > 2. Host driver just needs to update the beacon template of the affected > link and some how indicate to firmware that this is critical update. Sure. > 3. As per spec, there are around 21-22 changes classified as critical > update and among those only handful of those which has adding the IE in > per STA profile. So firmware could possibly maintain a table to see if > the affected link's beacon template has such IEs which if present needs > to be added in partner links, it could add it as and when partner link > transmits a beacon. That seems messy for all the reasons I mentioned, since it involves parsing/defragmenting/merging/fragmenting per-STA profiles etc. Do you really want to go there? > 4. Once CSA is finished, the host would update the beacon template which > will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to > per STA profile as well of the reporting links. Yeah I guess that's just a corollary of 3 :) > But yeah adding to per STA profile when already elements are there and > then first de-fragment and then add and then later fragment would be a > tedious job :) Yeah... > > If you really wanted the firmware to handle the partner link information > > completely it'd not only have to parse the Multi-Link Element on other > > links but also _modify_, and know what to do, etc. - I don't think > > that'll really work so well. > > > > Think about it some more: On the link doing the CSA you'd have to parse > > the beacon template and understand where the CSA-related elements are > > (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them > > aside. Then on the partner link(s) parse the Multi-Link Element, see if > > there's a partial STA profile already for some reason, merge the CSA- > > related elements into that, update the counters; all while first > > removing the 2 levels of fragmentation, and then re-doing them ... > > > > Yeah complex on firmware level. But even if we solve this on kernel > level, existing drivers who have offloaded beacon to firmware from 11ax > or before itself, still need to handle it? It would looks ugly that till > 11ax it is offloaded and from 11be it is again put back to kernel? Not sure what you mean here - we can still offload beaconing with a template, just have to fill in the template a bit more (with partner link CSA counters rather than just own CSA counters)? I'm basically thinking this: 1. Firmware is aware of partner link CSA counters/TSF offset/TBTT 2. Host driver (hostapd) sets beacon template of affected link to include CSA elements, and already today we include in the setting the offset to the countdown fields, so it doesn't matter where those are. 3. Host driver (hostapd) *also* updates beacon templates of partner links, but instead of saying "here's the CSA counter to update" it says "here's the CSA counter for your partner link M to set" (OK, so there potentially are multiple of those, but that's OK) 4. Firmware doesn't need to parse anything, it just needs to go through the beacon when transmitting it (for any link) and set the CSA counter fields for the link itself and/or for the partner links according to what the correct counter is per TBTT rules, TSF offset, beacon interval etc. Note that this could easily be implemented even in software for hwsim or drivers that don't offload beacon template to the device (like ath9k though obviously that has no multi-link), *iff* you know the TSF offset and TBTT of the beacon being created. Currently the CSA counters are just decremented every time you request a beacon, but the partner link counters would have to be calculated - but if you know - the intended TBTT of the beacon to transmit - the TSF offset to the link undergoing CSA - the beacon interval of the link undergoing CSA you can calculate - the TSF of the link undergoing CSA at the TBTT of the beacon we're creating, - the previous TBTT of the link undergoing CSA by the TSF (some calculation involving TSF modulo the beacon interval) - therefore the CSA counter of the link undergoing CSA to fill in In hwsim it's even easier because we only request the beacon at the TBTT, and the TSF offset is 0 ... johannes
On 2/13/24 16:25, Johannes Berg wrote: > On Tue, 2024-02-13 at 15:48 +0530, Aditya Kumar Singh wrote: >> On 2/13/24 14:12, Johannes Berg wrote: >>> On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote: >>>> On 2/12/24 20:16, Johannes Berg wrote: >>>>> So ... I'm looking at the client side, and thinking about that. >>>>> According to the spec, multi-link element should be present in beacons >>>>> of APs affiliated with the same MLD if one of the (other) links is doing >>>>> CSA, and then have also the CSA counters of course, relative to the >>>>> target link's TBTT (of course.) >>>> >>>> Yes correct, you are referring to critical update(s) right? >>> >>> I was actually thinking the critical update flag is rather not all that >>> interesting, but the inclusion of the CSA-related elements in a >>> (partial) per-STA profile in the Multi-Link Element is. Those contain >>> the countdown fields, so need to be updated - according to the TBTT of >>> the link doing the CSA, not the link sending it. >>> >> >> Yes correct, this CSA thing is interesting but there is one more thing - >> handling of the BSS param change count (BPCC). Let's say the affected >> link undergoes CSA, then in its Basic Multi Link IE, BPCC should be >> incremented. But at the same time, in RNR of its partner links, where >> this affected link is there, the BPCC should also be incremented. >> All though here it is not incremented with every beacon Tx, but still >> change in one affects beacon of other partner link(s) at least once. > > Yes, I know, but IMHO that's not all that interesting, I'd think hostapd > could handle that. > >>> Note that I'm basically asking you, Mediatek and Realtek how this should >>> work. We're almost certainly not going to support multi-link AP in our >>> device any time soon, and we could adjust the firmware to it anyway if >>> needed. >>> >>> So while I agree that punting it to firmware would be somewhat nice, we >>> still have to agree on (a) or (b), because (b) requires more information >>> from hostapd etc., but (a) requires _substantially_ more (and very >>> complex) parsing in firmware... If you wanted to support multi-BSSID, >>> then it's even more complex, which would argue for (b)? >>> >> >> Correct. Both has got its own advantages and disadvantages. Need to >> agree on any one so that all can be benefited. Let's hear from other folks. > > Do you _have_ firmware for this already, perhaps? > Yes, private firmware is there. Not yet upstreamed since the infrastructure in mac80211 and driver is not there. AFAIK it is in pipeline. Internal testing have so far been good and did not see any issues. So at least from Qualcomm perspective, we are fine with offloading (the above we discussed) to firmware. > I mean, I'm happy to be designing this "greenfield", but ... it seems > unlikely at this point? > > Also what about ath12k with the multi-device changes, would the driver > have to handle it? Or I guess you have to sync the TSF somehow anyway, > so you could handle it based on TSF/TBTT? > Ath12k firmware also supports beacon offload and hence firmware is taking care to add the respective IEs in per-STA profile of the reporting links while host just sends the template for the affected link via beacon template update and indicating that critical update has happened. > >>> I was trying to keep opinions out of this, but personally, I really >>> don't like (a), it puts too much complexity in the hands of firmware >>> engineers ;-) >>> >>>> Firmware could maintain the counter (for the affected link) and (I >>>> assume it will be aware of the partner links) so whenever partner link >>>> transmits a beacon it could add the CSA (or such IEs) in per STA profile >>>> of the reporting link. It could get the contents from the affected link >>>> but actual value of the counter could be filled from the global counter >>>> being maintained. But not sure whether we can force every driver's >>>> firmware to do so? >>> >>> I'm not sure I understand that description - how would firmware even >>> know which elements to take from what partner link etc.? I think hostapd >>> still has to add the elements, and firmware would just populate the >>> counter field(s)? But yeah that requires knowing from which partner link >>> to populate. >>> >> >> Yeah so underlying assumption/thought was - >> 1. Firmware is aware of partner links > > Sure. > >> 2. Host driver just needs to update the beacon template of the affected >> link and some how indicate to firmware that this is critical update. > > Sure. > >> 3. As per spec, there are around 21-22 changes classified as critical >> update and among those only handful of those which has adding the IE in >> per STA profile. So firmware could possibly maintain a table to see if >> the affected link's beacon template has such IEs which if present needs >> to be added in partner links, it could add it as and when partner link >> transmits a beacon. > > That seems messy for all the reasons I mentioned, since it involves > parsing/defragmenting/merging/fragmenting per-STA profiles etc. > > Do you really want to go there? > I understand things looks to be messy. But at least internally we have done in that way and things look simpler (on kernel level). Not sure whether all firmwares would agree to that. >> 4. Once CSA is finished, the host would update the beacon template which >> will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to >> per STA profile as well of the reporting links. > > Yeah I guess that's just a corollary of 3 :) > >> But yeah adding to per STA profile when already elements are there and >> then first de-fragment and then add and then later fragment would be a >> tedious job :) > > Yeah... > >>> If you really wanted the firmware to handle the partner link information >>> completely it'd not only have to parse the Multi-Link Element on other >>> links but also _modify_, and know what to do, etc. - I don't think >>> that'll really work so well. >>> >>> Think about it some more: On the link doing the CSA you'd have to parse >>> the beacon template and understand where the CSA-related elements are >>> (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them >>> aside. Then on the partner link(s) parse the Multi-Link Element, see if >>> there's a partial STA profile already for some reason, merge the CSA- >>> related elements into that, update the counters; all while first >>> removing the 2 levels of fragmentation, and then re-doing them ... >>> >> >> Yeah complex on firmware level. But even if we solve this on kernel >> level, existing drivers who have offloaded beacon to firmware from 11ax >> or before itself, still need to handle it? It would looks ugly that till >> 11ax it is offloaded and from 11be it is again put back to kernel? > > Not sure what you mean here - we can still offload beaconing with a > template, just have to fill in the template a bit more (with partner > link CSA counters rather than just own CSA counters)? > > I'm basically thinking this: > > 1. Firmware is aware of partner link CSA counters/TSF offset/TBTT > > 2. Host driver (hostapd) sets beacon template of affected link to > include CSA elements, and already today we include in the setting the > offset to the countdown fields, so it doesn't matter where those are. > > 3. Host driver (hostapd) *also* updates beacon templates of partner > links, but instead of saying "here's the CSA counter to update" it says > "here's the CSA counter for your partner link M to set" (OK, so there > potentially are multiple of those, but that's OK) > > 4. Firmware doesn't need to parse anything, it just needs to go through > the beacon when transmitting it (for any link) and set the CSA counter > fields for the link itself and/or for the partner links according to > what the correct counter is per TBTT rules, TSF offset, beacon interval > etc. > Hmm.. I see your point. So basically there is tradeoff - complexity of handling the parsing and all in firmware or complexity of handling those many offsets in kernel. I believe when hostapd updates the templates for the partner links, it would have to send offsets so we may need to extend our NL infrastructure for MLO in order to accommodate all those list of offsets during beacon template update. > > Note that this could easily be implemented even in software for hwsim or > drivers that don't offload beacon template to the device (like ath9k > though obviously that has no multi-link), *iff* you know the TSF offset > and TBTT of the beacon being created. > Currently the CSA counters are just decremented every time you request a > beacon, but the partner link counters would have to be calculated - but > if you know > - the intended TBTT of the beacon to transmit > - the TSF offset to the link undergoing CSA > - the beacon interval of the link undergoing CSA > you can calculate > - the TSF of the link undergoing CSA at the TBTT of the beacon we're > creating, > - the previous TBTT of the link undergoing CSA by the TSF (some > calculation involving TSF modulo the beacon interval) > - therefore the CSA counter of the link undergoing CSA to fill in > > In hwsim it's even easier because we only request the beacon at the > TBTT, and the TSF offset is 0 ... > Yep, agree.
On Tue, 2024-02-13 at 18:11 +0530, Aditya Kumar Singh wrote: > > > Correct. Both has got its own advantages and disadvantages. Need to > > > agree on any one so that all can be benefited. Let's hear from other folks. > > > > Do you _have_ firmware for this already, perhaps? > > > > Yes, private firmware is there. Not yet upstreamed since the > infrastructure in mac80211 and driver is not there. AFAIK it is in > pipeline. Internal testing have so far been good and did not see any > issues. So at least from Qualcomm perspective, we are fine with > offloading (the above we discussed) to firmware. Ah, OK. Note that if we have all the offsets I guess you could still do all the parsing? Though adding the per-STA profile in cross-link scenarios would be a difference if hostapd has to do it in the template vs. firmware copying the elements ... For the record, I really don't like copying the elements and not sure I'd want to implement that in mac80211/hwsim... > > I mean, I'm happy to be designing this "greenfield", but ... it seems > > unlikely at this point? > > > > Also what about ath12k with the multi-device changes, would the driver > > have to handle it? Or I guess you have to sync the TSF somehow anyway, > > so you could handle it based on TSF/TBTT? > > > > Ath12k firmware also supports beacon offload and hence firmware is > taking care to add the respective IEs in per-STA profile of the > reporting links while host just sends the template for the affected link > via beacon template update and indicating that critical update has happened. OK. So I guess we have to support that? Or are there any chances we could agree on something here overall and then the firmware can be changed to support another mode? > > That seems messy for all the reasons I mentioned, since it involves > > parsing/defragmenting/merging/fragmenting per-STA profiles etc. > > > > Do you really want to go there? > > > > I understand things looks to be messy. But at least internally we have > done in that way and things look simpler (on kernel level). Not sure > whether all firmwares would agree to that. And hwsim, but yeah :) > > Not sure what you mean here - we can still offload beaconing with a > > template, just have to fill in the template a bit more (with partner > > link CSA counters rather than just own CSA counters)? > > > I'm basically thinking this: > > > > 1. Firmware is aware of partner link CSA counters/TSF offset/TBTT > > > > 2. Host driver (hostapd) sets beacon template of affected link to > > include CSA elements, and already today we include in the setting the > > offset to the countdown fields, so it doesn't matter where those are. > > > > 3. Host driver (hostapd) *also* updates beacon templates of partner > > links, but instead of saying "here's the CSA counter to update" it says > > "here's the CSA counter for your partner link M to set" (OK, so there > > potentially are multiple of those, but that's OK) > > > > 4. Firmware doesn't need to parse anything, it just needs to go through > > the beacon when transmitting it (for any link) and set the CSA counter > > fields for the link itself and/or for the partner links according to > > what the correct counter is per TBTT rules, TSF offset, beacon interval > > etc. > > > > Hmm.. I see your point. So basically there is tradeoff - complexity of > handling the parsing and all in firmware or complexity of handling those > many offsets in kernel. I believe when hostapd updates the templates for > the partner links, it would have to send offsets so we may need to > extend our NL infrastructure for MLO in order to accommodate all those > list of offsets during beacon template update. Yes, of course. But that doesn't seem _too_ complicated? Even supporting max # of links (14) and ~3 counters each isn't a huge list... netlink doesn't care, and drivers have lower link limits anyway. Maybe I'll do a prototype later? But for now a) I need to test the client-side code I'm writing in some other way (element injection or something) b) let's wait for Realtek to comment also after the Lunar New Year :) Thanks! johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Tuesday, February 13, 2024 8:49 PM > To: Aditya Kumar Singh <quic_adisi@quicinc.com> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ping-Ke Shih <pkshih@realtek.com>; Ryder Lee > <ryder.lee@mediatek.com> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > b) let's wait for Realtek to comment also after the Lunar New Year :) > Sorry for the late. Realtek firmware can update partner links' CSA/ECSA by given offset of beacon template, which matches the conclusion of this thread, and Realtek vendor (out of tree) driver has verified the firmware interface. So that will work to Realtek WiFi 7 chips. Ping-Ke
> > Sorry for the late. > > Realtek firmware can update partner links' CSA/ECSA by given offset of > beacon template, which matches the conclusion of this thread, and Realtek vendor > (out of tree) driver has verified the firmware interface. So that will > work to Realtek WiFi 7 chips. Thanks! I guess that'd also apply to probe responses? Or does it not send those at all? But we discussed before that maybe we don't have to be perfect there, so I guess we can find some solution to that. So we have, so far Realtek: - uses offset in beacon template to set partner counter, - requires host to set CSA/ECSA elements Qualcomm: - copies and updates CSA/ECSA elements all by itself - btw, not sure here about probe responses, does it do that too? hwsim: - personally, I'd prefer it works like Realtek, for less complexity Intel: - not implemented, probably not going to happen any time soon, but I think we might prefer the Realtek way too if we ever do this Mediatek? Broadcom? Anyone else? I guess then for Qualcomm we'll need to just add an extended feature flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not update the parter link templates, or so? Thanks, johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Wednesday, February 21, 2024 4:10 PM > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > I guess that'd also apply to probe responses? Or does it not send those > at all? But we discussed before that maybe we don't have to be perfect > there, so I guess we can find some solution to that. Unfortunately, Realtek firmware doesn't send probe responses at all. Still need hostapd to reply those. Ping-Ke
On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote: > > -----Original Message----- > > From: Johannes Berg <johannes@sipsolutions.net> > > Sent: Wednesday, February 21, 2024 4:10 PM > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org> > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > > > I guess that'd also apply to probe responses? Or does it not send those > > at all? But we discussed before that maybe we don't have to be perfect > > there, so I guess we can find some solution to that. > > Unfortunately, Realtek firmware doesn't send probe responses at all. Still > need hostapd to reply those. > Right, but filling in the CSA counters? johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Wednesday, February 21, 2024 4:20 PM > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote: > > > -----Original Message----- > > > From: Johannes Berg <johannes@sipsolutions.net> > > > Sent: Wednesday, February 21, 2024 4:10 PM > > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org> > > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > > > > > I guess that'd also apply to probe responses? Or does it not send those > > > at all? But we discussed before that maybe we don't have to be perfect > > > there, so I guess we can find some solution to that. > > > > Unfortunately, Realtek firmware doesn't send probe responses at all. Still > > need hostapd to reply those. > > > > Right, but filling in the CSA counters? > No, firmware doesn't modify content of probe response frame. Ping-Ke
On Wed, 2024-02-21 at 08:28 +0000, Ping-Ke Shih wrote: > > > -----Original Message----- > > From: Johannes Berg <johannes@sipsolutions.net> > > Sent: Wednesday, February 21, 2024 4:20 PM > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org> > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > > > On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote: > > > > -----Original Message----- > > > > From: Johannes Berg <johannes@sipsolutions.net> > > > > Sent: Wednesday, February 21, 2024 4:10 PM > > > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > > > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > > > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > > > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org> > > > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > > > > > > > I guess that'd also apply to probe responses? Or does it not send those > > > > at all? But we discussed before that maybe we don't have to be perfect > > > > there, so I guess we can find some solution to that. > > > > > > Unfortunately, Realtek firmware doesn't send probe responses at all. Still > > > need hostapd to reply those. > > > > > > > Right, but filling in the CSA counters? > > > > No, firmware doesn't modify content of probe response frame. > Can you get that fixed? ;-) With differing beacon intervals etc., I don't know there's a good way to keep the counters even with a semblance of correctness, especially if we don't know when the beacon was transmitted? Or maybe just fill it in the driver since you probably have some beacon timing data more easily accessible? johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Wednesday, February 21, 2024 4:35 PM > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@nbd.name> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > On Wed, 2024-02-21 at 08:28 +0000, Ping-Ke Shih wrote: > > > > > -----Original Message----- > > > From: Johannes Berg <johannes@sipsolutions.net> > > > Sent: Wednesday, February 21, 2024 4:20 PM > > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org> > > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > > > > > On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote: > > > > > -----Original Message----- > > > > > From: Johannes Berg <johannes@sipsolutions.net> > > > > > Sent: Wednesday, February 21, 2024 4:10 PM > > > > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > > > > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > > > > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > > > > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org> > > > > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > > > > > > > > > I guess that'd also apply to probe responses? Or does it not send those > > > > > at all? But we discussed before that maybe we don't have to be perfect > > > > > there, so I guess we can find some solution to that. > > > > > > > > Unfortunately, Realtek firmware doesn't send probe responses at all. Still > > > > need hostapd to reply those. > > > > > > > > > > Right, but filling in the CSA counters? > > > > > > > No, firmware doesn't modify content of probe response frame. > > > > Can you get that fixed? ;-) > > With differing beacon intervals etc., I don't know there's a good way to > keep the counters even with a semblance of correctness, especially if we > don't know when the beacon was transmitted? > > Or maybe just fill it in the driver since you probably have some beacon > timing data more easily accessible? If driver can get CSA or ECSA offset simply, I probably can fill a reasonable CSA counter (not sure if I can get 100% accurate counter for now), but seemingly neither ieee80211_tx_control nor ieee80211_tx_info (SKB_CB) doesn't have these offsets. Any suggestions? I wonder our out-of-tree driver generates probe response itself. Let's me check how it does. Ping-Ke
On Wed, 2024-02-21 at 08:57 +0000, Ping-Ke Shih wrote: > > > > > > No, firmware doesn't modify content of probe response frame. > > > > > > > Can you get that fixed? ;-) > > > > With differing beacon intervals etc., I don't know there's a good way to > > keep the counters even with a semblance of correctness, especially if we > > don't know when the beacon was transmitted? > > > > Or maybe just fill it in the driver since you probably have some beacon > > timing data more easily accessible? > > If driver can get CSA or ECSA offset simply, I probably can fill a reasonable > CSA counter (not sure if I can get 100% accurate counter for now), but > seemingly neither ieee80211_tx_control nor ieee80211_tx_info (SKB_CB) doesn't > have these offsets. Any suggestions? We're not there yet! This thread is debating how we want to handle it. Although .. you have a point, we have that issue now already, and we don't pass the CSA offsets in probe responses if hostapd is filling them in. I guess we also have work to do on this. > I wonder our out-of-tree driver generates probe response itself. Let's me > check how it does. You _could_ also actually just implement probe response "offload" in the driver, then you get the template from hostapd/mac80211, and that should come with the offsets to fill in. Might be easier, overall. johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Wednesday, February 21, 2024 5:00 PM > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@nbd.name> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > You _could_ also actually just implement probe response "offload" in the > driver, then you get the template from hostapd/mac80211, and that should > come with the offsets to fill in. Might be easier, overall. Agree. That will be easier. What I only concern is hostapd can still work well without probe request, which is dropped by driver implementing offload. Ping-Ke
On Wed, 2024-02-21 at 09:17 +0000, Ping-Ke Shih wrote: > > > -----Original Message----- > > From: Johannes Berg <johannes@sipsolutions.net> > > Sent: Wednesday, February 21, 2024 5:00 PM > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@nbd.name> > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > > > You _could_ also actually just implement probe response "offload" in the > > driver, then you get the template from hostapd/mac80211, and that should > > come with the offsets to fill in. Might be easier, overall. > > Agree. That will be easier. What I only concern is hostapd can still work well > without probe request, which is dropped by driver implementing offload. That's what happens today, so yeah, that should work? You need to not answer&drop certain P2P probe requests though. But since we're talking about this in WiFi7 AP context, you could just make it different depending on that, I think we get the templates all the time in the kernel and just ignore them if not handled. johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Wednesday, February 21, 2024 5:20 PM > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@nbd.name> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis > > You need to not answer&drop certain P2P probe requests though. But since > we're talking about this in WiFi7 AP context, you could just make it > different depending on that, I think we get the templates all the time > in the kernel and just ignore them if not handled. > Got it!
> I guess then for Qualcomm we'll need to just add an extended feature > flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not > update the parter link templates, or so? > Or then again ... Maybe the other way around, since it works for Qualcomm now, assuming it all works at all? Doesn't really matter, we can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted logic. johannes
On 2/21/24 15:12, Johannes Berg wrote: > >> I guess then for Qualcomm we'll need to just add an extended feature >> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not >> update the parter link templates, or so? >> > Or then again ... Maybe the other way around, since it works for > Qualcomm now, assuming it all works at all? Doesn't really matter, we > can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted > logic. > Yeah anything is fine. Basically this flag should be visible till hostapd (so probably some wiphy level flag), so that it may skip forming beacon templates of partner links in CSA case. Based on the flag, those drivers which want it, it should form and send.
On 2/21/24 15:12, Johannes Berg wrote: > >> I guess then for Qualcomm we'll need to just add an extended feature >> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not >> update the parter link templates, or so? >> > Or then again ... Maybe the other way around, since it works for > Qualcomm now, assuming it all works at all? Doesn't really matter, we > can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted > logic. > > johannes Yeah anything is fine. Basically this flag should be visible till hostapd (so probably some wiphy level flag), so that it may skip forming beacon templates of partner links in CSA case. Based on the flag, those drivers which want it, it should form and send.
On 2/21/24 13:39, Johannes Berg wrote: > Qualcomm: > - copies and updates CSA/ECSA elements all by itself > - btw, not sure here about probe responses, does it do that too? We had a thought about keeping this CSA/ECSA handling at host/kernel level only. But the major point of concern is _synchronization_ among firmware of each of the links participating in the MLD. * Even if we ignore TSF/TBTT synchronization for a moment, how firmware will know when to transmit the beacon with a particular counter or when CSA has finished on other link? If rely on host's update then there is room for further delay and hence errors. - This is because, counter value on the reported link depends on the last beacon transmitted by the affected link. * Host can send the template on all links but how to ensure that first template is reached on the affected link and then only on the partner links? Host will queue the command properly but reaping of the command on n (no of links) independent firmware can not be guaranteed in the same order in which host has filled. It depends how busy each of host to firmware path is. * And then obviously, considering TSF/TBTT will be again complicating the synchronization part and making it more difficult to manage just via host. Hence there is a strong urge to let firmware handle all this for beacons. As far as how firmware will _magically_ communicate among themselves is concerned, we have *IPC* in place to achieve that. One link firmware can talk to other link firmware when required. Kernel Level
Hi, On Wed, 2024-02-21 at 18:22 +0530, Aditya Kumar Singh wrote: > On 2/21/24 13:39, Johannes Berg wrote: > > Qualcomm: > > - copies and updates CSA/ECSA elements all by itself > > - btw, not sure here about probe responses, does it do that too? > > We had a thought about keeping this CSA/ECSA handling at host/kernel > level only. But the major point of concern is _synchronization_ among > firmware of each of the links participating in the MLD. Sure. > * Even if we ignore TSF/TBTT synchronization for a moment, how firmware > will know when to transmit the beacon with a particular counter or when > CSA has finished on other link? If rely on host's update then there is > room for further delay and hence errors. > - This is because, counter value on the reported link depends on > the last beacon transmitted by the affected link. Sure. I don't think anyone suggested that the host will put the exact counter value there, just to have the template. > * Host can send the template on all links Right. > but how to ensure that first > template is reached on the affected link and then only on the partner > links? Host will queue the command properly but reaping of the command > on n (no of links) independent firmware can not be guaranteed in the > same order in which host has filled. It depends how busy each of host to > firmware path is. True, and there's a potential for race conditions there I suppose, but I suppose in the Intel, Realtek and hwsim case at least we wouldn't have *different* firmwares running multiple links, but a single one. In any case, you could solve this even with multiple, by applying the new template only after you have the CSA'ing link's new beacon template, if it requires filling in CSA counters, or such. > * And then obviously, considering TSF/TBTT will be again complicating > the synchronization part and making it more difficult to manage just via > host. Again, not suggesting that it is managed completely by the host, just the templates. > Hence there is a strong urge to let firmware handle all this for beacons. Sure, that's fine, your call :) > As far as how firmware will _magically_ communicate among themselves is > concerned, we have *IPC* in place to achieve that. One link firmware can > talk to other link firmware when required. :-) It probably doesn't actually help you make it race-free though, so does it really matter? But again, it's your call how you want to do it, and we'll just have to handle it in software appropriately. While I'd prefer to have _one_ way of doing things, at least so far we've basically seen one way of having the host involved and ath12k not having the host involved, so it's all still really simple. > Kernel Level > ____________________________________________________________________________ > -------------- -------------- -------------- > > Firmware 1 | <- IPC -> | Firmware 2 | <- IPC -> | Firmware 3 | > > on HW 1 | | on HW 2 | | on HW 3 | > -------------- -------------- -------------- > > > Hence, host just needs to update template of the affected link and > indicate to firmware that it is a critical update. This firmware then > can indicate other link firmware(s) to append CSA/ECSA IE with a given > counter value to its beacon via this IPC. Sure. You still have a race, because if you send a message over IPC saying that the CSA needs to be included and it's just before the TBTT, chances are the TBTT event will still happen without that. So in a sense it's similar to the host updating the partner link's beacon template "too late". You can have a situation where the CSA link's template is updated just before _its_ TBTT, and the partner link's TBTT is just a little bit later, but the update is delayed ... You could probably solve that by making your IPC synchronous but then you risk your TBTT timings in cases like this. Arguably, I'm not sure it matters. I'm thinking we'll enforce that the CSA must be in progress when updating the partner links beacon/probe response templates referring to it (*), but ... ultimately, I think we can accept that the partner link updates the CSA one beacon later if their TBTTs are very close. (*) and now that I think about it, that might have to immediately come with a separate template to use _after_ the switch, like CSA does for the CSA link > Parsing the IE and > de-fragmenting and fragmenting it again can be done by firmware itself. > (Agree that it is bit complex but when comparing with complexity of > maintaining synchronicity across links, this looks more doable) That sync maintenance is because of your hardware design though, others don't necessarily have that because multiple links are handled by the same NIC, not separate ones. > Hence we have taken "offloading beacons fully to firmware" approach. Sure, fair enough. > For probe responses, it is handled in host/kernel only. Firmware sends > back the last transmitted count in beacon to host. So we have the last > transmitted count info. Per STA profile generation logic is also there. > So we manage via that. So I think like in Realtek's case I'd probably advocate doing that in the driver with the offsets given by hostapd/software stack, although that requires having *two* feature flags, one for beacons and one for probe responses ... since you lack the "magic" for probe responses. johannes
On 2/21/24 18:38, Johannes Berg wrote: > Hi, > > On Wed, 2024-02-21 at 18:22 +0530, Aditya Kumar Singh wrote: >> On 2/21/24 13:39, Johannes Berg wrote: >>> Qualcomm: >>> - copies and updates CSA/ECSA elements all by itself >>> - btw, not sure here about probe responses, does it do that too? >> >> We had a thought about keeping this CSA/ECSA handling at host/kernel >> level only. But the major point of concern is _synchronization_ among >> firmware of each of the links participating in the MLD. > > Sure. > >> * Even if we ignore TSF/TBTT synchronization for a moment, how firmware >> will know when to transmit the beacon with a particular counter or when >> CSA has finished on other link? If rely on host's update then there is >> room for further delay and hence errors. >> - This is because, counter value on the reported link depends on >> the last beacon transmitted by the affected link. > > Sure. > > I don't think anyone suggested that the host will put the exact counter > value there, just to have the template. > >> * Host can send the template on all links > > Right. > >> but how to ensure that first >> template is reached on the affected link and then only on the partner >> links? Host will queue the command properly but reaping of the command >> on n (no of links) independent firmware can not be guaranteed in the >> same order in which host has filled. It depends how busy each of host to >> firmware path is. > > True, and there's a potential for race conditions there I suppose, but I > suppose in the Intel, Realtek and hwsim case at least we wouldn't have > *different* firmwares running multiple links, but a single one. oh okay, makes sense. > > In any case, you could solve this even with multiple, by applying the > new template only after you have the CSA'ing link's new beacon template, > if it requires filling in CSA counters, or such. > >> * And then obviously, considering TSF/TBTT will be again complicating >> the synchronization part and making it more difficult to manage just via >> host. > > Again, not suggesting that it is managed completely by the host, just > the templates. > >> Hence there is a strong urge to let firmware handle all this for beacons. > > Sure, that's fine, your call :) > >> As far as how firmware will _magically_ communicate among themselves is >> concerned, we have *IPC* in place to achieve that. One link firmware can >> talk to other link firmware when required. > > :-) > > It probably doesn't actually help you make it race-free though, so does > it really matter? But again, it's your call how you want to do it, and > we'll just have to handle it in software appropriately. While I'd prefer > to have _one_ way of doing things, at least so far we've basically seen > one way of having the host involved and ath12k not having the host > involved, so it's all still really simple. Yes! > >> Kernel Level >> ____________________________________________________________________________ >> -------------- -------------- -------------- >>> Firmware 1 | <- IPC -> | Firmware 2 | <- IPC -> | Firmware 3 | >>> on HW 1 | | on HW 2 | | on HW 3 | >> -------------- -------------- -------------- >> >> >> Hence, host just needs to update template of the affected link and >> indicate to firmware that it is a critical update. This firmware then >> can indicate other link firmware(s) to append CSA/ECSA IE with a given >> counter value to its beacon via this IPC. > > Sure. You still have a race, because if you send a message over IPC > saying that the CSA needs to be included and it's just before the TBTT, > chances are the TBTT event will still happen without that. So in a sense > it's similar to the host updating the partner link's beacon template > "too late". You can have a situation where the CSA link's template is > updated just before _its_ TBTT, and the partner link's TBTT is just a > little bit later, but the update is delayed ... > > You could probably solve that by making your IPC synchronous but then > you risk your TBTT timings in cases like this. > > Arguably, I'm not sure it matters. I'm thinking we'll enforce that the > CSA must be in progress when updating the partner links beacon/probe > response templates referring to it (*), but ... ultimately, I think we > can accept that the partner link updates the CSA one beacon later if > their TBTTs are very close. > > > (*) and now that I think about it, that might have to immediately come > with a separate template to use _after_ the switch, like CSA does for > the CSA link True. So each link will have to send two templates. beacon_csa and beacon_after > >> Parsing the IE and >> de-fragmenting and fragmenting it again can be done by firmware itself. >> (Agree that it is bit complex but when comparing with complexity of >> maintaining synchronicity across links, this looks more doable) > > That sync maintenance is because of your hardware design though, others > don't necessarily have that because multiple links are handled by the > same NIC, not separate ones. > >> Hence we have taken "offloading beacons fully to firmware" approach. > > Sure, fair enough. > >> For probe responses, it is handled in host/kernel only. Firmware sends >> back the last transmitted count in beacon to host. So we have the last >> transmitted count info. Per STA profile generation logic is also there. >> So we manage via that. > > So I think like in Realtek's case I'd probably advocate doing that in > the driver with the offsets given by hostapd/software stack, although > that requires having *two* feature flags, one for beacons and one for > probe responses ... since you lack the "magic" for probe responses. > :) I guess yeah. So this approach we discussed is fine right? Have some _feature flag_ to indicate whether host would like templates (with offsets) for reporting links or not. If it does then hostapd can send those, otherwise it won't. Hosts can further handle as per their need.
On Wed, 2024-02-21 at 09:09 +0100, Johannes Berg wrote: > > I guess that'd also apply to probe responses? Or does it not send > those > at all? But we discussed before that maybe we don't have to be > perfect > there, so I guess we can find some solution to that. > > So we have, so far > > Realtek: > - uses offset in beacon template to set partner counter, > - requires host to set CSA/ECSA elements > > Qualcomm: > - copies and updates CSA/ECSA elements all by itself > - btw, not sure here about probe responses, does it do that too? > > hwsim: > - personally, I'd prefer it works like Realtek, for less complexity > > Intel: > - not implemented, probably not going to happen any time soon, but I > think we might prefer the Realtek way too if we ever do this > > > Mediatek? Broadcom? Anyone else? > > > I guess then for Qualcomm we'll need to just add an extended feature > flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not > update the parter link templates, or so? > > Thanks, Hi, Sorry for the late reply. In summary, MediaTek firmware behaves similarly to Realtek's firmware, so we agree on the conclusion of this thread. By the way, this behavior does not apply to probe response in MediaTek firmware. In other words, MediaTek firmware does not modify the probe response and it requires hostapd to fill all CSA-related Elements. I think this could be fixed if hostapd provides CSA offsets when sending the probe response. Best, Michael > johannes
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index a5d510932cbe..4427259154e2 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -3629,6 +3629,7 @@ static int __ieee80211_csa_finalize(struct ieee80211_link_data *link_data) { struct ieee80211_sub_if_data *sdata = link_data->sdata; struct ieee80211_local *local = sdata->local; + struct ieee80211_bss_conf *link_conf = link_data->conf; u64 changed = 0; int err; @@ -3650,22 +3651,21 @@ static int __ieee80211_csa_finalize(struct ieee80211_link_data *link_data) if (link_data->reserved_ready) return 0; - return ieee80211_link_use_reserved_context(&sdata->deflink); + return ieee80211_link_use_reserved_context(link_data); } - if (!cfg80211_chandef_identical(&link_data->conf->chandef, + if (!cfg80211_chandef_identical(&link_conf->chandef, &link_data->csa_chandef)) return -EINVAL; - sdata->vif.bss_conf.csa_active = false; + link_conf->csa_active = false; - err = ieee80211_set_after_csa_beacon(&sdata->deflink, &changed); + err = ieee80211_set_after_csa_beacon(link_data, &changed); if (err) return err; - if (sdata->vif.bss_conf.eht_puncturing != sdata->vif.bss_conf.csa_punct_bitmap) { - sdata->vif.bss_conf.eht_puncturing = - sdata->vif.bss_conf.csa_punct_bitmap; + if (link_conf->eht_puncturing != link_conf->csa_punct_bitmap) { + link_conf->eht_puncturing = link_conf->csa_punct_bitmap; changed |= BSS_CHANGED_EHT_PUNCTURING; } @@ -3693,7 +3693,8 @@ static void ieee80211_csa_finalize(struct ieee80211_link_data *link_data) struct ieee80211_sub_if_data *sdata = link_data->sdata; if (__ieee80211_csa_finalize(link_data)) { - sdata_info(sdata, "failed to finalize CSA, disconnecting\n"); + sdata_info(sdata, "failed to finalize CSA on link %d, disconnecting\n", + link_data->link_id); cfg80211_stop_iface(sdata->local->hw.wiphy, &sdata->wdev, GFP_KERNEL); } @@ -3869,7 +3870,10 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, struct ieee80211_channel_switch ch_switch; struct ieee80211_chanctx_conf *conf; struct ieee80211_chanctx *chanctx; + struct ieee80211_bss_conf *link_conf; + struct ieee80211_link_data *link_data; u64 changed = 0; + u8 link_id = params->link_id; int err; lockdep_assert_wiphy(local->hw.wiphy); @@ -3880,16 +3884,23 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, if (sdata->wdev.cac_started) return -EBUSY; - if (cfg80211_chandef_identical(¶ms->chandef, - &sdata->vif.bss_conf.chandef)) + if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS)) + return -EINVAL; + + link_data = wiphy_dereference(wiphy, sdata->link[link_id]); + if (!link_data) + return -ENOLINK; + + link_conf = link_data->conf; + + if (cfg80211_chandef_identical(¶ms->chandef, &link_conf->chandef)) return -EINVAL; /* don't allow another channel switch if one is already active. */ - if (sdata->vif.bss_conf.csa_active) + if (link_conf->csa_active) return -EBUSY; - conf = rcu_dereference_protected(sdata->vif.bss_conf.chanctx_conf, - lockdep_is_held(&local->hw.wiphy->mtx)); + conf = wiphy_dereference(wiphy, link_conf->chanctx_conf); if (!conf) { err = -EBUSY; goto out; @@ -3913,7 +3924,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, if (err) goto out; - err = ieee80211_link_reserve_chanctx(&sdata->deflink, ¶ms->chandef, + err = ieee80211_link_reserve_chanctx(link_data, ¶ms->chandef, chanctx->mode, params->radar_required); if (err) @@ -3922,44 +3933,44 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, /* if reservation is invalid then this will fail */ err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0); if (err) { - ieee80211_link_unreserve_chanctx(&sdata->deflink); + ieee80211_link_unreserve_chanctx(link_data); goto out; } /* if there is a color change in progress, abort it */ - if (sdata->vif.bss_conf.color_change_active) + if (link_conf->color_change_active) ieee80211_color_change_abort(sdata); - err = ieee80211_set_csa_beacon(&sdata->deflink, params, &changed); + err = ieee80211_set_csa_beacon(link_data, params, &changed); if (err) { - ieee80211_link_unreserve_chanctx(&sdata->deflink); + ieee80211_link_unreserve_chanctx(link_data); goto out; } - if (params->punct_bitmap && !sdata->vif.bss_conf.eht_support) + if (params->punct_bitmap && link_conf->eht_support) goto out; - sdata->deflink.csa_chandef = params->chandef; - sdata->deflink.csa_block_tx = params->block_tx; - sdata->vif.bss_conf.csa_active = true; - sdata->vif.bss_conf.csa_punct_bitmap = params->punct_bitmap; + link_data->csa_chandef = params->chandef; + link_data->csa_block_tx = params->block_tx; + link_conf->csa_active = true; + link_conf->csa_punct_bitmap = params->punct_bitmap; - if (sdata->deflink.csa_block_tx) + if (link_data->csa_block_tx) ieee80211_stop_vif_queues(local, sdata, IEEE80211_QUEUE_STOP_REASON_CSA); cfg80211_ch_switch_started_notify(sdata->dev, - &sdata->deflink.csa_chandef, 0, + &link_data->csa_chandef, link_id, params->count, params->block_tx, - sdata->vif.bss_conf.csa_punct_bitmap); + link_conf->csa_punct_bitmap); if (changed) { - ieee80211_link_info_change_notify(sdata, &sdata->deflink, - changed); + ieee80211_link_info_change_notify(sdata, link_data, changed); + /* link_id to be passed here? */ drv_channel_switch_beacon(sdata, ¶ms->chandef); } else { /* if the beacon didn't change, we can finalize immediately */ - ieee80211_csa_finalize(&sdata->deflink); + ieee80211_csa_finalize(link_data); } out: diff --git a/net/mac80211/link.c b/net/mac80211/link.c index d4f86955afa6..0a1d0ef09d9f 100644 --- a/net/mac80211/link.c +++ b/net/mac80211/link.c @@ -73,6 +73,8 @@ void ieee80211_link_stop(struct ieee80211_link_data *link) ieee80211_mgd_stop_link(link); cancel_delayed_work_sync(&link->color_collision_detect_work); + wiphy_work_cancel(link->sdata->local->hw.wiphy, + &link->csa_finalize_work); ieee80211_link_release_channel(link); }
Add changes to start a channel switch as well as finalize it on link basis in order to support CSA with MLO as well. Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com> --- net/mac80211/cfg.c | 69 ++++++++++++++++++++++++++------------------- net/mac80211/link.c | 2 ++ 2 files changed, 42 insertions(+), 29 deletions(-)