On Thu, Oct 6, 2011 at 9:49 AM, Christian Lamparter <chunkeey@xxxxxxxxxxxxxx> wrote: > On Thursday, October 06, 2011 12:27:03 AM Luis R. Rodriguez wrote: >> You do have a good point, but I disagree that you do not need to test >> / regress test hardware / driver code for DFS. > > Actually, you are sort of contradicting yourself here. > > Take a look at your "wireless: add DFS master support" patch > series. I don't see any IFDEFs to select between the old > and the new "way" even though you know full well that there's > some black magic going on. Well its true, but the regdb and CRDA stuff is can have the DFS master region support, its just the mapping, not a technical implementation. IMHO DFS support should be a kconfig on both the 802.11 stack and driver part, and the driver part depend on the 802.11 stack option. > http://permalink.gmane.org/gmane.linux.kernel.wireless.general/78455 > Quote: > "Here's a puzzle though... If we change this series to use the other > pad byte that was available, the first pad byte, instead of the last > one, we loose backward compatibility support and I cannot figure out > why." That's an implementation weirdness with python pack on generating the binary output, not a DFS issue per se. > [Note: This is just an recent enough example. I do think I could come > up with a better one, e.g.: why didn't athXk have a compile-time > switch to disable ANI when it was introduced because it can(and has?) > caused some regressions as well?] No ANI is *required*, without it the cards are useless. ANI is also properly tested and validated by our systems team. Have you tried disabling ANI? When we introduced a revamp of the new ANI though we did enable users to use the new ANI for older generation cards, the module parameter is still there. DFS is a different beast. Testing DFS cannot be compared to testing ANI, DFS has a slew of different tests you need to run against, and then *if* you do want to sell cards in certain geographies with support for DFS channels you need to get proper regulatory certification for an intended radiator that supports DFS properly. If you get this certification technically you cannot even expose a knob to users to disable DFS when operating on a DFS channel. > so, why do you want a useless compile-time for *this option* *now*? What I want to do is enable an option which lets distributors disable DFS if they don't want to even deal with the question of whether or not a card had DFS support enabled through driver support. > Is this something about politics/laws I don't know about [I'm just > curious, because I don't really buy "testing" here, since Zefir > obviously has a working prototype No, we haven't passed certified testing with this code yet. > and Atheros has a working and certificated codebase as well Exactly, and that is the code we are enabling Neratec with to be able to upstream into the kernel for, but the code being referenced uses a different 802.11 stack, different base driver, etc. > which he can access and base his work on. So I don't think its that unstable and needs added > ugliness.] Its the same with 802.11s, its new code and people may want to disable this crap to not deal with it in code path or even consider the support for it. >> This is what I'm talking about. But yes, userspace also submits >> itself to the same criteria. >> >> You may also want to simply disable DFS if you do not want to >> >> deal with the regulatory test implications of having it enabled. >> > AFAIK you can't "simply" disable the DFS requirement: hostapd >> > (hw_features.c), [cfg80211] (checks if tx on secondary channel >> > is possible) and mac80211 (tx.c) all have checks. Indeed, the >> > easiest way is to modify crda's database. So there's no need >> > for an extra compile-time option. >> >> No, DFS is set for certain channels on wireless-regdb/CRDA, I just >> posted DFS master region support for wireless-regdb and CRDA. Apart >> from this we then need driver support. To get DFS you need all of >> these + hostapd part. Each one has its own set of components and does >> deserve its own set of tests and review. > > This "deserve its own set of tests and review". Does it translate in: > "ath9k [every driver], mac80211, cfg80211 and hostapd need extra > DFS IFDEFS?". I still have yet to see patches for cfg80211 / mac80211 for DFS. What I'm saying is we have an kconfig option on the 802.11 stack to allow us to disable DFS support, and the driver respective component depend on it. > In fact, ifdefs make it harder to do reviews, because > sometimes you just forget the IFDEF/ELSIF/ELSE context of the code. I hate ifdefs, and if you read my e-mail carefully what I was suggesting was to do this properly by building all the code if the kconfig option is enabled therefore eliminating all ifdef junk from the code and only leaving it for header files. > And regression testing can be done by "git bisect". In fact, isn't > this what git bisect is for? There is a difference between regression testing and finding the culprit of an issue. git bisect is used to find the culprit of an issue. However if you want to ensure code does not regress you need to ensure to run a suite of tests on code after a delta is applied. Only if you find an issue do you then use git bisect. I want proper test infrastructure set up before I even consider enabling any DFS code upstream for ath9k. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
