-
Notifications
You must be signed in to change notification settings - Fork 21
zephyr: Use k_fifo instead of socketpair for IPC #86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
zephyr: Use k_fifo instead of socketpair for IPC #86
Conversation
In my test setup it saved 6528 bytes RAM and 3060 bytes ROM in STA mode compared to current upstream version that uses socketpair. |
Added initial support for hostapd which does not use socketpair. |
ab5167a
to
12306a0
Compare
12306a0
to
9d5eaf8
Compare
intptr_t fifo; | ||
|
||
/** | ||
* Dynamic data to send. The receiver is responsible for freeing the data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why const data was removed?Without const, dynamic data doesn't make much send, we can just call i t data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It complicated my debugging about the heap corruption and was a bit early premature optimization. We can bring it back when everything is working smoothly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can do it in a subsequent PR.
goto fail; | ||
} | ||
|
||
priv->sock_pair[0] = ret; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we rename this? As we are removing select NET_SOCKETPAIR
, this might confuse the users, as it's basically two separate sockets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can certainly rename it, I did not want to do too many changes so it looks still a bit like socketpair even if it is not.
9d5eaf8
to
fa3f7da
Compare
fa3f7da
to
141aa5c
Compare
@MaochenWang1 can you please review? |
Hi @jukkar is this feature targeted for v4.2 release? |
It would be good to get this in for 4.2, as it says RAM. |
but I saw issues on RW612, and don't have time to deep research it |
Same build command and hal_nxp as zephyrproject-rtos/zephyr#90317 (comment) ? |
use the latest codebase, the latest hal_nxp |
@MaochenWang1 I just tried frdm_rw612 board and see no issues when connecting to either OPEN mode AP or to WPA3-SAE-HNP. I used the same command line option as you. |
Sorry, I have some problem to build the latest upstream, as it requires python3.12, still fixing it. |
I tried the frdm_rw612 board, and have the same issue. |
Hi @jukkar can we hold this feature after v4.2? |
I have version
|
If you cannot verify the functionality, then we have to postpone this. I am just wondering what could cause the issues in your setup as everything works ok for my NXP board. Things work ok also for Nordic wifi test setup. |
Hi jukkar, your FW version is kind of old. please remove |
@MaochenWang1 here is a log to connecting to open AP
|
Compilation for my above test was done like this:
|
As socketpair uses almost 10kb of memory in AP mode, and almost 7kb in STA mode, try to lower RAM consumption by getting rid of socketpair as a IPC between wpa_supplicant and hostapd related threads. The commit uses k_fifo instead and just sends a pointer between threads. This also avoids some memory copy of the data. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Replace socketpair by k_fifo for internal IPC between different hostap threads. This will save both RAM and ROM as we can avoid large buffers that are needed by socketpair. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
141aa5c
to
e97f328
Compare
|
As socketpair uses almost 10kb of memory in AP mode, and almost 7kb in STA mode, try to lower RAM consumption by getting rid of socketpair as a IPC between wpa_supplicant and hostapd related threads. The commit uses k_fifo instead and just sends a pointer between threads. This also avoids some memory copy of the data.