Skip to content

Conversation

@fengming-ye
Copy link
Contributor

Add wifi l2 mgmt dpp handlers.
Add wifi subcommand dpp to call l2 mgmt dpp handlers.

DPP shell commands body is the same as wpa_cli commands but with different prefix "wifi dpp". eg.
"wifi dpp dpp_auth_init peer=1 role=enrollee"

DPP shell command handler will parse user args to params in enum and send l2 mgmt dpp requests.
DPP l2 handlers will parse params to hostap wpa_cli format args and send wpa_cli commands to hostap.

@fengming-ye
Copy link
Contributor Author

Complicance check fail because of unknown kconfig CONFIG_WIFI_NM_WPA_SUPPLICANT_DPP
Because it relies on following PR
#73249

Pls help review regardless of this CI error so that this PR can go in parallel.

@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch 4 times, most recently from f05c55b to ffd20ce Compare June 5, 2024 06:54
@jukkar
Copy link
Member

jukkar commented Jun 6, 2024

This PR seems to be related to this hostap PR zephyrproject-rtos/hostap#10
It is important that we link these PRs together so that things get at least compile tested before we merge the PRs.
So in this PR, you should add a commit that adds the hostap PR to west.yml manifest file.
The hostap revision should be set to revision: pull/10/head and then you should re-submit this PR. After the hostap PR is merged, you need to send one more push of this PR and change the revision to the actual head commit ID of the hostap repo.
This will mean one extra round of tests but that is unavoidable here.

@zephyrbot
Copy link

zephyrbot commented Jun 6, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-hostap DNM This PR should not be merged (Do Not Merge) labels Jun 6, 2024
krish2718
krish2718 previously approved these changes Jun 6, 2024
@jukkar
Copy link
Member

jukkar commented Jun 6, 2024

The hostap PRs are now merged, next steps:

  • update west.yml hostap revision to commit 20f4cacf5a69eed9bc684f11c62b8f75349f223a which is latest commit in hostap repo
  • it might make sense to wait until the zephyr upstream has fixed the sensing subsystem issue which is causing CI issues before basing and re-submitting the pr, the hotfix should come still today The CI should be fixed now.

Comment on lines 799 to 823
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Member

@jukkar jukkar Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use strncpy() variant when copying. This way we can be sure that we will not overwrite dpp_cmd_buf. There are external tools that will complain the use of these unsafe functions so we could avoid future issues for these.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated as comments

@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch from c3729df to 83ce631 Compare June 7, 2024 01:45
@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Jun 7, 2024
@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch from 83ce631 to ccf2457 Compare June 7, 2024 03:07
@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch 2 times, most recently from 5ee7834 to 0228257 Compare June 7, 2024 08:57
@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Jun 7, 2024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DPP in comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@aescolar
Copy link
Member

@fengming-ye Rebase needed

@jukkar
Copy link
Member

jukkar commented Jul 31, 2024

#76454
rebased on this PR

This is very confusing if you create a new PR, please just use this one and just force push new commit versions on top of this PR.

@jukkar
Copy link
Member

jukkar commented Jul 31, 2024

For this PR, just rebase your branch to latest main, and force push. Github complains now about some conflict which hopefully is resolved by a rebase.

@fengming-ye fengming-ye dismissed stale reviews from krish2718, jukkar, and rlubos via e8bdebb July 31, 2024 09:20
@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch from d3cc6bd to e8bdebb Compare July 31, 2024 09:20
@fengming-ye
Copy link
Contributor Author

fengming-ye commented Jul 31, 2024

For this PR, just rebase your branch to latest main, and force push. Github complains now about some conflict which hopefully is resolved by a rebase.

rebased, thanks.
and hostap revision is merged already. so no need to change manifest

@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch from e8bdebb to bee39fb Compare August 1, 2024 02:41
@fengming-ye
Copy link
Contributor Author

fengming-ye commented Aug 1, 2024

CI fail because of unrelated error

INFO    - 659/659 qemu_x86                  tests/net/lib/http_server/tls/net.http.server.tls   FAILED unexpected eof (qemu 0.551s)
INFO    - /__w/zephyr/zephyr/twister-out/qemu_x86/tests/net/lib/http_server/tls/net.http.server.tls/handler.log
ERROR   - SeaBIOS (version zephyr-v1.0.0-0-g31d4e0e-dirty-20200714_234759-fv-az50-zephyr)
Booting from ROM..
*** Booting Zephyr OS build v3.7.0-310-gc326e014cab1 ***
W: No entropy device on the system, TLS communication is insecure!
Running TESTSUITE framework_tests_tls
===================================================================
START - test_tls
E: Page fault at address 0x18a00c (error code 0x0)
E: Linear address not present in page tables
E: Access violation: supervisor thread not allowed to read
E: PTE: not present
E: EAX: 0x00139180, EBX: 0x0018a008, ECX: 0x00000059, EDX: 0x00000000
E: ESI: 0x00000000, EDI: 0x0018a010, EBP: 0x007e5e8c, ESP: 0x007e5e78
E: EFLAGS: 0x00000283 CS: 0x0008 CR3: 0x0014a000
E: call trace:
E: EIP: 0x0011e171
E:      0x0011e974 (0x14f1f4)
E:      0x0011df4a (0x14eed0)
E:      0x0010134c (0x0)
E: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
E: Current thread: 0x14ee00 (http_server_tid)
E: Halting system

@jukkar
Copy link
Member

jukkar commented Aug 2, 2024

The CI issue is already fixed by #76580. Please rebase against latest main and force push a new version.

@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch from bee39fb to 9b6ebca Compare August 2, 2024 08:02
@fengming-ye
Copy link
Contributor Author

The CI issue is already fixed by #76580. Please rebase against latest main and force push a new version.

rebased and add NXP copyright since we are adding new features to wifi mgmt.

Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just minor comment about the doxygen comments

Comment on lines 870 to 873
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these comments referring to? I mean the individual fields are already commented below so these four lines seems unnecessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use proper English here as this data is used to generate the documentation, so start with capital letter here "Operation enum"

Comment on lines 943 to 947
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit weird, could you rewrite it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you place the shell changes to a separate commit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated as above requests

@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch from 9b6ebca to b12a310 Compare August 5, 2024 02:29
Add wifi l2 mgmt dpp handlers.
Add wifi subcommand dpp to call l2 mgmt dpp handlers.

DPP l2 handlers will parse params to hostap wpa_cli format args
and send wpa_cli commands to hostap.

Signed-off-by: Fengming Ye <[email protected]>
DPP shell command handler will parse user args to params in enum
and send l2 mgmt DPP requests.

Signed-off-by: Fengming Ye <[email protected]>
@fengming-ye fengming-ye force-pushed the feature/hostap_dpp_wifi_l2_support branch from b12a310 to 2ac4125 Compare August 5, 2024 02:36
@nashif nashif merged commit 86b5b59 into zephyrproject-rtos:main Aug 5, 2024
yvanderv pushed a commit to nxp-zephyr/nxp-zephyr that referenced this pull request Dec 12, 2024
Add wifi l2 mgmt dpp handlers.
Add wifi subcommand dpp to call l2 mgmt dpp handlers.

DPP shell commands body is the same as wpa_cli commands but with
different prefix "wifi dpp". eg.  "wifi dpp dpp_auth_init peer=1
role=enrollee"

DPP shell command handler will parse user args to params in enum and
send l2 mgmt dpp requests.  DPP l2 handlers will parse params to hostap
wpa_cli format args and send wpa_cli commands to hostap.

From list
zephyrproject-rtos/zephyr#73707

Signed-off-by: Fengming Ye <[email protected]>
yvanderv pushed a commit to nxp-zephyr/nxp-zephyr that referenced this pull request Dec 12, 2024
Add wifi l2 mgmt dpp handlers.
Add wifi subcommand dpp to call l2 mgmt dpp handlers.

DPP shell commands body is the same as wpa_cli commands but with
different prefix "wifi dpp". eg.  "wifi dpp dpp_auth_init peer=1
role=enrollee"

DPP shell command handler will parse user args to params in enum and
send l2 mgmt dpp requests.  DPP l2 handlers will parse params to hostap
wpa_cli format args and send wpa_cli commands to hostap.

From list
zephyrproject-rtos/zephyr#73707

Signed-off-by: Fengming Ye <[email protected]>
yvanderv pushed a commit to nxp-zephyr/nxp-zephyr that referenced this pull request Dec 12, 2024
Add wifi l2 mgmt dpp handlers.
Add wifi subcommand dpp to call l2 mgmt dpp handlers.

DPP shell commands body is the same as wpa_cli commands but with
different prefix "wifi dpp". eg.  "wifi dpp dpp_auth_init peer=1
role=enrollee"

DPP shell command handler will parse user args to params in enum and
send l2 mgmt dpp requests.  DPP l2 handlers will parse params to hostap
wpa_cli format args and send wpa_cli commands to hostap.

From list
zephyrproject-rtos/zephyr#73707

Signed-off-by: Fengming Ye <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

9 participants