Hi, > In PATCH v8 , cfg80211_validate_beacon_int -> > cfg80211_iter_combinations carries the argument iftype_num , which > also considers the "new interface" that is getting added. Ah, right, I remember now, sorry. > Thus , in the example you have quoted above , the iftype_num shall > represent 2 ( AP + AP ) , and thus the min_gcd obtained out of > cfg80211_iter_combinations (cfg80211_get_beacon_int_min_gcd) shall be > 50 for the example 1 and 200 for the example 2 . > Thus , considering the two examples mentioned above , the second AP > should succeed for "example 1" vs failure for "example 2" with patch > V8 , isn't ? Yeah, I tried to simplify and did so too much. I believe you are, for this purpose, ignoring for example radar detection. Since you're passing 0 for num_different_channels and radar_detect, you might find a combination isn't actually currently usable, but that allows the new beacon interval configuration. So I think overall this will only work right if it's done with all necessary information, no? Trying to construct another example ... let's say permitted combinations are * go = 3, channels = 1, min_bcn_gcd = 50 * go = 3, channels = 2, min_bcn_gcd = 100 (which isn't actually all that far-fetched, since channel hopping takes time) For simplification, say you already have two GOs active on different channels (with BI 100), and want to add another one - with beacon interval 50 - this isn't possible, but I don't think your code would detect it? Or, perhaps I'm reading this wrong, if your code *does* detect it then changing the scenario a bit to have just a single channel, your code would prevent it even though it's allowed? > The current behavior of the kernel is to not allow the configuration > of different beacon intervals and our expectation is that this new > patch should be backward compatible with the existing behavior. Correct, and I agree, we shouldn't break that. > Now , if we go with this approach of "introducing a new argument to > cfg80211_iter_combinations which shall be the GCD of all the existing > combinations to check against the respective > "diff_beacon_int_gcd_min"" , consider the case ( same one which is > mentioned above ) that we have a single AP interface ( beacon > interval = 300 ) , and a new AP is getting added ( beacon interval = > 150 ), with the following allowed combinations: > > 1) * ap = 2 > diff_beacon_int_gcd_min = 0 ( rather not specified ) > 2) * ap = 2 > diff_beacon_int_gcd_min = 100 > > The GCD calculated is 150 . cfg80211_iter_combinations shall return > success for both the scenarios ( 1 and 2 ) (The intention here is to > compare with only the nonzero " diff_beacon_int_gcd_min" ) > > This success from "cfg80211_iter_combinations" does not represent if > the GCD passed is compared against a 0 / non zero > "diff_beacon_int_gcd_min" , isn't ? > > Thus , we are trying to solve this problem , by getting the > "diff_beacon_int_gcd_min" for the respective interface combination , > before comparing it with the calculated GCD. Oh. I think I finally understand your concern - good point! Let me see if I understand correctly - you're saying that if I first calculate g = GCD(all BIs, including the new one) and then do cfg80211_iter_combinations(... existing variables ..., g) then I cannot accurately determine whether or not I can use a combination that doesn't specify a min_beacon_interval_gcd, you can't know if the "all BIs" were the same, or not. That's actually a very good point. However, it seems pretty easy to solve by passing another bool that indicates "all the same"? johannes
