This repository has been archived by the owner on Nov 14, 2024. It is now read-only.
Use unbuffered logging for debugs #92
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Modifications for building supplicant on Zephyr RTOS. Signed-off-by: Sridhar Nuvusetty <[email protected]> Signed-off-by: Sachin Kulkarni <[email protected]> Signed-off-by: Ravi Dondaputi <[email protected]> Signed-off-by: Krishna T <[email protected]> Co-authored-by: krishna T <[email protected]>
The file was removed but was still included.
sdk-nrf has a different version where these variables are not private, so, remove the private macro for build.
* Adding CODEOWNERS file .. Adding @krish2718 @sr1dh48r @rlubos @sachinthegreen Co-authored-by: krish2718 <[email protected]>
Remove uninitialized variable that is no longer needed, this causes crash in case supplicant thread exits.
* Include HOSTAP_BASE to fix header file paths * Select WEP automatically through Kconfig
nRF CI treats warnings as errors, so, this is must.
Add proper pre-processor conditions to the code that uses IPv4 API. Signed-off-by: Damian Krolik <[email protected]>
PR [1] is now up-merged to NCS, so, remove the workaround. [1] - zephyrproject-rtos/zephyr#45592 Signed-off-by: Krishna T <[email protected]>
Previous to [1] we used to get L2 header and that was used to filter unregistered frames (another bug), but now that L2 header is removed, we cannot use L2 header, so, directly parse payload and filter EAPoL frames only. [1] - zephyrproject-rtos/zephyr#45592 Signed-off-by: Krishna T <[email protected]>
Add const qualifier to the declaration. Signed-off-by: Krishna T <[email protected]>
1. Add WPA_SUPP_LOG_LEVEL_* Kconfig options to control the WPA supplicant log level the same way as other SDK components. 2. Add WPA_SUPP_DEBUG_LEVEL Kconfig option to be used for compile-time filtering of WPA supplicant debug messages. By default, it is aligned with WPA_SUPP_LOG_LEVEL. 3. Implement Zephyr variants of wpa_debug.h and wpa_debug.c files that use Zephyr logging subystem as the default output and apply compile-time filtering for the messages. Signed-off-by: Damian Krolik <[email protected]>
As per Apache-2.0 license, we need to indicate that the files have been modified, the recommended way is to add an extra copyright header. Signed-off-by: Krishna T <[email protected]>
Eloop framework in wpa_supplicant uses select with minimal timeout of all registered users, but by default it is 10secs (periodic cleanup task), so, in case of no other events all registered users will be delayed by 10secs. In Linux, select exits without waiting for full 10secs as for there is a handler registered for NL80211 sockets and whenever there is a event from Kernel select exits and processes expired events immediately. In Zephyr, we don't have such mechanism as we use direction function calls between kernel and wpa_supplicant, so, add an event socket and register it with Eloop and use this to post the event, the socket handler pass the event to wpa_supplicant. For user interface we just post a dummy message only to unblock select. This solves both problems: * Unblocking select for all interesting events immediately * Terminate driver context for events as we use sockets, so, remove mbox + thread. This significantly improves the association time from 30s to 5s. Signed-off-by: Krishna T <[email protected]>
This fixes unnecessary wait for scan results in case of failure scenarios. Signed-off-by: krishna T <[email protected]>
Scan results are allocated by driver using k_malloc Zephyr allocator but are freed by wpa_supplicant using libc free, due to changes in metadata differences between them, we free the pointer which is 8 bytes below the actual one and cause a bus and mem fault. Copy the scan results from driver before passing to the wpa_supplicant and then let driver free them using the same k_free allocator API. We can now enable the scan results free code. Signed-off-by: Krishna T <[email protected]>
This can cause allocation failures and an unnecessary prints. Signed-off-by: Krishna T <[email protected]>
Using first interface handle everywhere is not ideal, esp. when we add support for multi-VIF, so, query wpa_supplicant with the interface name to get the handle. The interface name itself is hard coded to "wlan0" everywhere for now.
Sometimes we are getting a unsolicited or spurious scan result from UMAC but we have already freed the scan results buffer, so, add a null check before processing scan result.
This print is only for debugging, so, should use DEBUG level. Signed-off-by: Krishna T <[email protected]>
These APIs will be used by nRF Wi-Fi management to interact with wpa_supplicant. Signed-off-by: Krishna T <[email protected]> Signed-off-by: Ravi Dondaputi <[email protected]>
This is now purely an internal utility, so, moved from sdk-nrf to here as it works with wpa_supplicant. Signed-off-by: Krishna T <[email protected]>
Now that display scan is natively supported using wifi_mgmt, remove it from WPA supplicant API and wpa_cli. Signed-off-by: Krishna T <[email protected]>
Using UDP sockets need an interface with properly configured IP address and then either IPv4/IPv6 enabled, UNIX socket don't need any of those and work perfectly well for IPC. This solves two bugs: * Matter doesn't enable IPv4, so, the events stop working, as the code doesn't support IPv6 sockets and also doesn't protect with IPv4 define. * Wi-Fi sample assigns IP address in the `prj.conf` but if an application doesn't do that, then socket send fails. Signed-off-by: Krishna T <[email protected]>
After every successful association, restart DHCP to get a fresh lease. For the initial association this avoid delay due to DHCP exponential retries, and also handles the case where interface has changed IP subnet. Signed-off-by: Krishna T <[email protected]>
With recent changes, WPA CLI build is broken. Signed-off-by: Krishna T <[email protected]>
All options depend on WPA_SUPP, so, add a check. Signed-off-by: Krishna T <[email protected]>
Don't use the same acronym in help. Signed-off-by: Krishna T <[email protected]>
The scan result is always freed in the driver for both success and failure cases, so, no need for wpa_supplicant to free in failure case. Signed-off-by: Krishna T <[email protected]>
If a second configuration is defined but failed to read, then the first configuration is leaked. Signed-off-by: Krishna T <[email protected]>
This reverts commit 70eb764. This commit is part of a PR that introduced "section `rodata' will not fit in region `FLASH'" regression to sdk-nrf. Signed-off-by: Johann Fischer <[email protected]>
This reverts commit 2e02dcd. This commit is part of a PR that introduced "section `rodata' will not fit in region `FLASH'" regression to sdk-nrf. Signed-off-by: Johann Fischer <[email protected]>
Before the control interface changes we had added a new command to set the H2E in the WPA supplicant configuration, but this was removed, so, add back the command that can be set through WPA cli. Signed-off-by: Krishna T <[email protected]>
This is a common modules used by P2P, WPA3 and Enterprise, so, it needs to be enabled with a custom config to avoid re-definitions. Signed-off-by: Ravi Dondaputi <[email protected]>
WPA3 also needs to enable PMKSA caching, though the file is included but the define (IEEE8021X_EAPOL) is not passed. Due to code size (text) increase when WPA_SUPP_EAPOL is enabled, this feature is not enabled by default. If required this can be enabled explicitly by application. Signed-off-by: Ravi Dondaputi <[email protected]>
Read beacon interval and DTIM period as part of Wi-Fi status. Signed-off-by: Ravi Dondaputi <[email protected]>
Commit 044ed05 ("Make WPA CLI as passthrough") made this SYS_INIT() a no-op. It's breaking the build now that SYS_INIT function signatures are 'int callback(void)'. Remove it entirely since it does nothing. Signed-off-by: Martí Bolívar <[email protected]>
gcc 12.1 complains about using pointer after realloc as it could potentially be moved/freed, causing any uses after UB. Fix this by doing checks before alloc and use those statuses and update with new BSS. Signed-off-by: Krishna T <[email protected]>
While fetching connection info as part of processing the interface status command, if error is returned by driver, set the params to 0 and reset the return value, so that other params are not ignored by the caller. Signed-off-by: Ravi Dondaputi <[email protected]>
When copying event from driver and postnig to WPA supplicant, the IE's are not copied, so, if WPA supplicant takes longer to process the event the driver frees the IEs causing invalid data to be processed by WPA supplicant. This is a common issue to all events. For now Authentication events are being affected due to WPA3-SAE taking longer to process, so, fix only Authentication events for now as a generic fix needs more work and thought. Signed-off-by: Krishna T <[email protected]>
As the command is a global configuration, no need to add a separate command, it can be set using "wpa_cli set" itself. This reverts commit a5b03c4.
krish2718
requested review from
Damian-Nordic,
markaj-nordic,
ajayparida and
krga2022
added 2 commits
May 9, 2023 01:38
WPA supplicant debugs are excessive and the incoming rate is high esp. during connection, so, even though we increase the log buffer size, we still miss quite a few events. So, disable buffered logging for debugs, one caveat is that we lose the timestamp that comes with LOG_ macros and also the prints might be jumbled but this is still preferred over losing events. Signed-off-by: Krishna T <[email protected]>
This is handy in debugging crypto related issues. Also, configure the WPA supplicant after initialization. Signed-off-by: Krishna T <[email protected]>
krish2718
force-pushed
the
improve_debug_logging
branch
from
9e7dca6 to
5645649
Compare
| @@ -115,4 +115,11 @@ config WPA_SUPP_DEBUG_LEVEL | |||
| available levels and functions for emitting the messages. Note that | |||
| runtime filtering can also be configured in addition to the compile-time | |||
| filtering. | |||
|
|
|||
| config WPA_SUPP_DEBUG_SHOW_KEYS | |||
There was a problem hiding this comment.
Is it possible to make WPA_SUPP_LOG_LEVEL_DBG as a pre-condition for this?
sachinthegreen
approved these changes
May 9, 2023
rado17
approved these changes
May 9, 2023
| @@ -115,4 +115,11 @@ config WPA_SUPP_DEBUG_LEVEL | |||
| available levels and functions for emitting the messages. Note that | |||
| runtime filtering can also be configured in addition to the compile-time | |||
| filtering. | |||
|
|
|||
| config WPA_SUPP_DEBUG_SHOW_KEYS | |||
| #elif defined (CONFIG_WPA_SUPP_LOG_LEVEL_DBG) | ||
| wpa_supplicant_set_debug_params(global, MSG_DEBUG, 1, 0); | ||
| #else | ||
| wpa_supplicant_set_debug_params(global, MSG_INFO, 0, 0); |
There was a problem hiding this comment.
So for the default we will end up with INFO. In Matter we set WPA_SUPP_LOG_LEVEL_ERR=y - is it going to be overridden by the code above?
There was a problem hiding this comment.
Ah good point, I haven't really taken care of the levels, will do.
markaj-nordic
requested review from
kkasperczyk-no
and removed request for
Damian-Nordic
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WPA supplicant debugs are excessive and the incoming rate is high esp. during connection, so, even though we increase the log buffer size, we still miss quite a few events.
So, disable buffered logging for debugs, one caveat is that we lose the timestamp that comes with LOG_ macros and also the prints might be jumbled but this is still preferred over losing events.