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

Assert on enabling Socket CAN #37132

Closed
rbabub opened this issue Jul 22, 2021 · 5 comments · Fixed by #37311
Closed

Assert on enabling Socket CAN #37132

rbabub opened this issue Jul 22, 2021 · 5 comments · Fixed by #37311
Assignees
Labels
area: CAN area: Networking bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@rbabub
Copy link
Contributor

rbabub commented Jul 22, 2021

Assert while running socket can application seen during interface initialization.

ASSERTION FAIL [net_if_get_link_addr(iface)->addr != ((void *)0)] @ WEST_TOPDIR/zephyr/subsys/net/ip/net_if.c:3993

Flags added in prj.conf of socket can application, CONFIG_ASSERT=y

Expected behavior
Socket CAN does not require interface link address to be assigned, above check is not applicable for socket can interfaces.
As this address is NULL for socket CAN interface it results in assertion.

Logs and console output
ASSERTION FAIL [net_if_get_link_addr(iface)->addr != ((void *)0)] @ WEST_TOPDIR/zephyr/subsys/net/ip/net_if.c:3993
[00:00:00.138,092] os: Faulting instruction address (r15/pc): 0x6000f8bc
[00:00:00.146,820] os: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
[00:00:00.155,334] os: Current thread: 0x60081160 (unknown)
[00:00:00.162,841] os: Halting system

Note:
This check was introduced part of commit id "1e4f268ea94066cfc60c80019cf515b2baafc167".

@rbabub rbabub added the bug The issue is a bug, or the PR is fixing a bug label Jul 22, 2021
@alexanderwachter
Copy link
Member

@carlescufi we rather need a network stack expert here. Ping @rlubos

@alexanderwachter
Copy link
Member

For me it looks like we need something like | (IS_ENABLED(SOCKET_CAN) && is_socket_can(iface)) in the assert

@cfriedt cfriedt added the priority: low Low impact/importance bug label Jul 27, 2021
@rlubos
Copy link
Contributor

rlubos commented Jul 28, 2021

For me it looks like we need something like | (IS_ENABLED(SOCKET_CAN) && is_socket_can(iface)) in the assert

Well if CAN interfaces indeed do not specify the LL address on the interface then indeed the above assertion check does not make sense (I'm assuming that CAN packet processing doesn't actually make use of this field across the stack). I wouldn't hovewer modify the assert condition itself as it's pretty clear right now, I'd rather encapsulate the entire assert with an if, i. e.

if (IS_ENABLED(SOCKET_CAN) && is_socket_can(iface) {
    /* Nothing to check. */
} else {
    /* assert here */
}

IMO it'd be easier to follow then.

@alexanderwachter
Copy link
Member

@rbabub can you make a PR with a fix?

@rbabub
Copy link
Contributor Author

rbabub commented Jul 28, 2021

ya sure, will check & submit the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: CAN area: Networking bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants