[PATCH 1/5] wpa_supplicant: Collect and use extended data on used frequencies

[Peer, Ilan]

Hi Jouni,

> That sounds fine in general, but this patch is quite painful to review since it
> includes number of changes that seem unrelated (e.g., small changes to
> debug messages, function/variable renaming, and whitespace changes). I
> undid some of those to make this easier to read and understand the real
> functional changes.

Sorry for this.

> 
> Unfortunately, there is not yet very good coverage in the automated hwsim
> test cases for channel selection in multi channel concurrency cases. As such,
> I'm worried about regressions with any non-trivial changes in this area and
> this patch is very much non-trivial.
> 

Sure. Already started working on adding channel selection tests. Hope we have something ready by next week.

> I found couple of cases that would not seem to work properly in some cases
> (P2P group on wlan0 was not handled correctly, i.e., need to check
> current_ssid->p2p_group instead of wpa_s->p2p_group_interface; and
> assumption in wpas_p2p_pick_best_used_freq() about all frequencies being
> valid for P2P which is certainly not the case in many cases since this includes
> operating channels from non-P2P station interfaces). I tried to fix these while
> reviewing the changes, but I'm not sure how complete that was.

The assumption was intended for whoever called this API, so the caller has to guarantee that the assumption is valid (that why before calling this API, I always used wpas_p2p_valid_oper_freqs()).

> 
> My current work version from the review is here:
> http://w1.fi/p/p2p-freq/
> 

Started to review them and few changes might be required. How would you like me to send the changes? I can take you patches and edit them and send again.

> I'll continue this at some point, but this is taking significant enough effort
> from my part, that this will be likely delayed (as will obviously the following
> series that depend on this). If you want to speed up this, a contribution of

Yep ... the following series are the more interesting ones. 

> hwsim test case or two to exercise and demonstrate this functionality would
> be a nice approach. Otherwise, I'll probably need to do that myself before
> being able to get convinced that these do not break some existing use cases.
> I'd also recommend taking a look at the patch 2/6 in the series (that is this 1/5
> from your set -- I moved part of 2/5 to be before 1/5 to avoid changes that
> get deleted immediately within the same series) and check if I changed
> something that does not look correct for your use case.
>