Thread (8 messages) 8 messages, 3 authors, 2016-10-07

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help