Re: [PATCH v8] cfg80211: Provision to allow the support for different beacon intervals
From: Johannes Berg <johannes@sipsolutions.net>
Date: 2016-09-28 11:21:06
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