Skip to content
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

net: wifi: Fix CONFIG_EAP_TTLS not enabling all dependencies #82309

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TjazVracko
Copy link
Contributor

CONFIG_EAP_TTLS now correctly adds all .c files it requires from hostap.

Related: #82098

MaochenWang1
MaochenWang1 previously approved these changes Nov 29, 2024
@@ -398,6 +398,7 @@ zephyr_library_compile_definitions_ifdef(CONFIG_EAP_TLS

zephyr_library_sources_ifdef(CONFIG_EAP_TTLS
${HOSTAP_SRC_BASE}/eap_peer/eap_ttls.c
${HOSTAP_SRC_BASE}/eap_common/chap.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

to avoid adding twice, maybe we can do something like:

if(CONFIG_EAP_TTLS OR CONFIG_EAP_MSCHAPV2)
  zephyr_library_sources( ${HOSTAP_SRC_BASE}/eap_common/chap.c)
endif()

Copy link
Contributor

@Rex-Chen-NXP Rex-Chen-NXP Nov 29, 2024

Choose a reason for hiding this comment

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

Hi @krish2718
TTLS and MSCHAPV2 are separated EPA security types. Should we keep the original config, but not add this change?
If want to support EAP-TTLS-MSCHAPv2, we should enable both EAP-TTLS macro and EAP-MSCHAPV2 macros, is this will be more reasonable?

What my concern is TTLS for pharse1 and MSCHAPV2 for pharse2, there may be so many sets for different pharse1 and pharse2, so can we still make it configured independent?

This comment also apply for EAP-PEAP-MSCHAPv2.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, but my suggested fix is a or b, so, chap.c would be includes in that case, no?

Copy link
Contributor

@Rex-Chen-NXP Rex-Chen-NXP Nov 29, 2024

Choose a reason for hiding this comment

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

EAP-TTLS is independent with MSCHAPV2.
EAP-TTLS can also combine with other pharse2 EAP type, in this case, eap_common/chap.c will be build in application, but not used.

MaochenWang1
MaochenWang1 previously approved these changes Nov 29, 2024
)

zephyr_library_compile_definitions_ifdef(CONFIG_EAP_MSCHAPV2
EAP_MSCHAPv2
)

if(CONFIG_EAP_TTLS OR CONFIG_EAP_MSCHAPV2)
zephyr_library_sources( ${HOSTAP_SRC_BASE}/eap_common/chap.c)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
zephyr_library_sources( ${HOSTAP_SRC_BASE}/eap_common/chap.c)
zephyr_library_sources(${HOSTAP_SRC_BASE}/eap_common/chap.c)

CONFIG_EAP_TTLS now correctly adds all .c files it requires from
hostap.

Signed-off-by: Tjaž Vračko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Wi-Fi Wi-Fi size: XS A PR changing only a single line of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants