Hi Johannes,
>> 2. Since "diff_beacon_int_gcd_min" is only specified / advertised in
>> the interface combinations , our logic was to get the minimal
>> "diff_beacon_int_gcd_min" of all the matching combinations and later
>> compare with the new beacon interval. (API
>> "cfg80211_iter_combinations" has to be invoked to get the
>> corresponding "diff_beacon_int_gcd_min")
>Yeah, but you get the diff_beacon_int_gcd_min for all combinations that allow the *current* combination of interfaces, not the combinations that allow the new *new* combination of interfaces, no?
>I mean, consider the case that you have a single AP interface, with beacon interval 300, and you're adding another AP, with beacon interval 150, and the following allowed combinations:
>* ap = 1
> mesh = 1
> diff_beacon_int_gcd_min = 100
> * ap = 2
> diff_beacon_int_gcd_min = 50
> Wouldn't you prevent that, or something?
> Or say you have
> * ap = 1
> mesh = 1
> diff_beacon_int_gcd_min = 100
> * ap = 2
> diff_beacon_int_gcd_min = 200
> Probably doesn't really make sense, but now if you have an existing AP interface, you would think the min is 100, when really while adding another AP it should be 200.
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.
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 ?
>> If I understand this correctly , are you saying that this new argument
>> to cfg80211_iter_combinations shall be the GCD of all the existing
>> beacon intervals and would be used to match with the corresponding
>> "diff_beacon_int_gcd_min" of the interface combinations in
>> "cfg80211_iter_combinations".
>Yes, that's what I was thinking.
>> If yes , this GCD argument does not represent , if the beacon
>> intervals of all the existing interfaces differ or not , isn’t ? If
>> the "diff_beacon_int_gcd_min" of the all the matching interface
>> combinations is 0 , such differed beacon intervals would pass the
>> check , contradicting the existing logic ( not allowing the differed
>> beacon intervals), isn't ?
>Oh, well, ok - if all existing and new beacon intervals are the same we'd have to do the lookup without the existing beacon interval GCD and still compare to the min separately.
Let me reiterate our problem here with this approach.
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.
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.
Please correct us.
Regards,
Sunil
