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

drivers: can: stm32: can_add_rx_filter() does not respect CONFIG_CAN_MAX_FILTER #44725

Closed
henrikbrixandersen opened this issue Apr 10, 2022 · 7 comments · Fixed by #48213
Closed
Assignees
Labels
area: CAN bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug

Comments

@henrikbrixandersen
Copy link
Member

Describe the bug
Calling can_add_rx_filter() on an STM32 bxCAN driver instance does not respect CONFIG_CAN_MAX_FILTER setting.

When setting CONFIG_CAN_MAX_FILTER=5 the driver returns -ENOSPC when attempting to add standard, unmasked CAN-ID filter number 5. On the other hand, it seems to allow for infinite number of extended, unmasked CAN-ID filters (which clearly overflows the internal arrays of the driver).

Expected behavior
The driver should accept up to the number of configured filters and should not overflow internal arrays.

Environment (please complete the following information):

  • OS: Ubuntu Linux 20.04 LTS
  • Toolchain Zephyr SDK v0.14.0
  • Commit SHA: 70c4841

Additional context
This was found during implementation of #44687.

@henrikbrixandersen henrikbrixandersen added the bug The issue is a bug, or the PR is fixing a bug label Apr 10, 2022
@henrikbrixandersen henrikbrixandersen self-assigned this Apr 10, 2022
@henrikbrixandersen henrikbrixandersen added priority: medium Medium impact/importance bug area: CAN labels Apr 10, 2022
@FRASTM
Copy link
Collaborator

FRASTM commented Jun 7, 2022

Should this issue be closed by the #44688 ?

@henrikbrixandersen
Copy link
Member Author

henrikbrixandersen commented Jun 7, 2022

Should this issue be closed by the #44688 ?

No. It was found during implementation of #44687. It is not fixed yet.

@FRASTM
Copy link
Collaborator

FRASTM commented Jun 7, 2022

OK.
Maybe, this issue has a link with the error we get when running the testcase : tests/subsys/canbus/isotp/implementation/ on the nucleo_g474re board :

Running TESTSUITE isotp
===================================================================
START - test_bind_unbind
 PASS - test_bind_unbind in 2.509 seconds
===================================================================
START - test_send_receive_net_sf
 PASS - test_send_receive_net_sf in 0.6 seconds
===================================================================
START - test_send_receive_net_blocks
 PASS - test_send_receive_net_blocks in 1.20 seconds
===================================================================
START - test_send_receive_net_single_blocks
 PASS - test_send_receive_net_single_blocks in 0.166 seconds
===================================================================
START - test_send_receive_sf
 PASS - test_send_receive_sf in 0.6 seconds
===================================================================
START - test_send_receive_blocks
 PASS - test_send_receive_blocks in 0.454 seconds
===================================================================
START - test_send_receive_single_block

E: Reception of next FC has timed out
    Assertion failed at WEST_TOPDIR/zephyr/tests/subsys/canbus/isotp/implementa)
data should be received at once (ret: -14)

    Assertion failed at WEST_TOPDIR/zephyr/tests/subsys/canbus/isotp/implementa)
Sending failed (-2)
 FAIL - test_send_receive_single_block in 1.167 seconds
===================================================================
START - test_buffer_allocation

    Assertion failed at WEST_TOPDIR/zephyr/tests/subsys/canbus/isotp/implementa)
recv error: -14
 FAIL - test_buffer_allocation in 1.116 seconds
===================================================================
START - test_buffer_allocation_wait
E: No buffer for FF left

    Assertion failed at WEST_TOPDIR/zephyr/tests/subsys/canbus/isotp/implementa)
Binding failed (-11)
 FAIL - test_buffer_allocation_wait in 0.18 seconds
===================================================================
TESTSUITE isotp failed.
===================================================================
PROJECT EXECUTION FAILED

@martinjaeger
Copy link
Member

@FRASTM I think the issue you stated and the original issue described by @henrikbrixandersen cannot be related as the STM32G474 has the M_CAN IP, whereas this error is in the bxCAN driver for "older" STM32 MCUs.

I've just been working on the bxCAN driver for #46646, so I can have a closer look at this issue after that PR was merged.

@martinjaeger martinjaeger self-assigned this Jun 17, 2022
@FRASTM FRASTM added the platform: STM32 ST Micro STM32 label Jun 27, 2022
@henrikbrixandersen
Copy link
Member Author

@martinjaeger Do you still plan to take a closer look at this?

@martinjaeger
Copy link
Member

@martinjaeger Do you still plan to take a closer look at this?

Thanks for the reminder. I started working on it, but it turned out to be a bit more complicated than expected, so I couldn't find a fix immediately. Having another try now :)

@martinjaeger
Copy link
Member

In order to reproduce the bug, I started with implementing the can_get_max_filters API call (see my working branch).

Now tests/drivers/can/api fails on all filter-related tests.

Click to expand log output for CONFIG_CAN_LOG_LEVEL_DBG=y
[previous tests pass]

===================================================================
START - test_max_std_filters
D: Setting filter ID: 0x1, mask: 0x7ff
D: Filter type: standard ID without mask (1)
D: Filter set: id 0, index 0, bank 14
D: Setting filter ID: 0x2, mask: 0x7ff
D: Filter type: standard ID without mask (1)
D: Filter set: id 1, index 1, bank 14
D: Setting filter ID: 0x3, mask: 0x7ff
D: Filter type: standard ID without mask (1)
D: Filter set: id 2, index 2, bank 14
D: Setting filter ID: 0x4, mask: 0x7ff
D: Filter type: standard ID without mask (1)
D: Filter set: id 3, index 3, bank 14
D: Setting filter ID: 0x5, mask: 0x7ff
D: Filter type: standard ID without mask (1)
I: No space for a new filter!
D: Filter set: id -28, index 4, bank 15

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/can/api/src/main.c:476: add_rx_msgq: (filter_id equal to -ENOSPC)
no filters available
 FAIL - test_max_std_filters in 0.71 seconds
===================================================================
START - test_max_ext_filters
D: Setting filter ID: 0x1, mask: 0x1fffffff
D: Filter type: extended ID without mask (3)
D: Filter set: id 8, index 6, bank 16
D: Setting filter ID: 0x2, mask: 0x1fffffff
D: Filter type: extended ID without mask (3)
D: Filter set: id 10, index 7, bank 16
D: Setting filter ID: 0x3, mask: 0x1fffffff
D: Filter type: extended ID without mask (3)
D: Filter set: id 12, index 8, bank 17
D: Setting filter ID: 0x4, mask: 0x1fffffff
D: Filter type: extended ID without mask (3)
D: Filter set: id 14, index 9, bank 17
D: Setting filter ID: 0x5, mask: 0x1fffffff
D: Filter type: extended ID without mask (3)
D: Filter set: id 16, index 10, bank 18
D: Setting filter ID: 0x6, mask: 0x1fffffff
D: Filter type: extended ID without mask (3)
D: Filter set: id 18, index 11, bank 18

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/can/api/src/main.c:760: add_remove_max_filters: (filter_id not equal to -ENOSPC)
added more than max filters
 FAIL - test_max_ext_filters in 0.84 seconds
===================================================================
START - test_receive_timeout
D: Setting filter ID: 0x555, mask: 0x7ff
D: Filter type: standard ID without mask (1)
I: No space for a new filter!
D: Filter set: id -28, index 12, bank 19

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/can/api/src/main.c:476: add_rx_msgq: (filter_id equal to -ENOSPC)
no filters available
 FAIL - test_receive_timeout in 0.27 seconds
===================================================================
START - test_send_callback
D: Sending 8 bytes on CAN_2. Id: 0x555, ID type: standard, Remote Frame: no
D: Using mailbox 0
 PASS - test_send_callback in 0.10 seconds
===================================================================
START - test_send_receive_std_id
D: Setting filter ID: 0x555, mask: 0x7ff
D: Filter type: standard ID without mask (1)
I: No space for a new filter!
D: Filter set: id -28, index 14, bank 20

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/can/api/src/main.c:476: add_rx_msgq: (filter_id equal to -ENOSPC)
no filters available
 FAIL - test_send_receive_std_id in 0.27 seconds
===================================================================
START - test_send_receive_ext_id
D: Setting filter ID: 0x15555555, mask: 0x1fffffff
D: Filter type: extended ID without mask (3)
D: Filter set: id 28, index 16, bank 21
D: Sending 8 bytes on CAN_2. Id: 0x15555555, ID type: extended, Remote Frame: no
D: Using mailbox 0

[device gets stuck]

Digging deeper into the driver implementation I realized that there is no quick fix for this bug, as it requires a rework of significant parts of the driver. I opened a separate issue #47986 to discuss possible solutions.

I would appreciate your feedback.

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

Successfully merging a pull request may close this issue.

3 participants