On Wed, Jan 11, 2017 at 02:31:31PM +0100, Johannes Berg wrote: > > + * @timeout_reason: reason for connection timeout. This is used when > > the > > + * connection fails due to a timeout instead of an explicit > > rejection from > > + * the AP. 0 (NL80211_CONNECT_TIMEOUT_UNSPECIFIED) is used > > for other cases. > > I think this description is misleading - one could easily understand > "for other cases" to indicate for the cases that the AP did explicitly > reject it, but that's obviously not true. Well, the expectation here really was that the reason for the timeout would be known if there was a timeout and the unspecified value would be used in all other cases, i.e., in cases where the AP did indeed explicitly reject the connection. I guess there might be a driver where the connect request goes into firmware implementation and the driver would not know whether the operation failed due to authentication frame or association frame timeout out.. Implementation itself would still be fine, but I agree that it might be a bit confusing the try to interpret the description here on what the driver should do. > Perhaps that could be reworded, to say it's used when it's not known, > or such? I'd not indicate the value (0) either, just specify the name, > and put a % in front to get better formatting for it please. Sure, I can say that NL80211_TIMEOUT_UNSPECIFIED is used when the reason for the timeout is not known or there was an explicit rejection instead of a timeout. That said, cfg80211_connect_bss() is really currently documented to be used only for the success case just like cfg80211_connect_result(). In other words, if a driver were to call cfg80211_connect_bss(), it should really always specify NL80211_TIMEOUT_UNSPECIFIED here based on the current documented use. All failure would then need to be reported with cfg80211_connect_timeout() for the timeout case or by not following the documentation and calling cfg80211_connect_result() or cfg80211_connect_bss() for rejection cases. That said, the documentation for the connect() callback function does describe the failure case behavior correctly. I think I cleaned up that at some point, but did not update the function documentation at the same time. So it looks like some additional cleanup would be needed to make the documentation actually match what we expect the driver to do for rejection cases.. I'd like to leave it out from this specific patch and address the cleanup of existing failure case description in a separate patch. > > + NL80211_TIMEOUT_UNSPECIFIED); > > NL80211_CONNECT_TIMEOUT_UNSPECIFIED in the comment is wrong then. Yeah, these got renamed at some point and looks like that one was missed. -- Jouni Malinen PGP id EFC895FA
