Fix socket can race condition #223
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ssue" This reverts commit ac2c321.
If we just clear the IOB when failed to queue, we'll leak it. Signed-off-by: Zhe Weng <[email protected]>
In case CONFIG_NET_TIMESTAMP is enabled, subtract the timestamp size from the buflen Signed-off-by: Jukka Laitinen <[email protected]>
If pktlen is 0, there is no need to update Signed-off-by: Jukka Laitinen <[email protected]>
… always fit in one IOB Signed-off-by: Jukka Laitinen <[email protected]>
…if_poll Signed-off-by: Jukka Laitinen <[email protected]>
It is better to just atomically remove the iob from the readahead queue first, and only after that work on the iob. Signed-off-by: Jukka Laitinen <[email protected]>
pussuw
approved these changes
Mar 12, 2024
|
lgtm. The PR has reverted most of my previous workaround which is not needed if the race condition is fixed in arch/risc-v/src/mpfs/mpfs_fpga_canfd.c |
haitomatic
approved these changes
Mar 12, 2024
jlaitine
pushed a commit
that referenced
this pull request
Sep 27, 2024
"/home/guoshichao/work_profile/vela_os/vela_car_5/nuttx/include/crypto/curve25519.h", line 44: error #223-D: function arc4random_buf declared implicitly arc4random_buf(secret, CURVE25519_KEY_SIZE); ^ Signed-off-by: guoshichao <[email protected]>
jlaitine
pushed a commit
that referenced
this pull request
Sep 29, 2024
"/mnt/yang/qixinwei_commit/nuttx/include/nuttx/timers/timer.h", line 257: error #223-D: function DEBUGASSERT declared implicitly DEBUGASSERT(lower->ops->tick_getstatus); Signed-off-by: yanghuatao <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This fixes the https://ssrc.atlassian.net/browse/DP-8022
The cause for the crash seems to be IOB chain corruption, which is caused by insufficient locking in the can / network stack.
The same locking issue is actually the root cause for the earlier "semcount runaway issue". Therefor, I have reverted the earlier fix for the semcount runaway issue.
The locking is added in patch "arch/risc-v/src/mpfs/mpfs_fpga_canfd.c: Fix a race condition with devif_poll"
Since the reverted patch contained also other things, I created separate patches for those. @haitomatic could you please check that those are correct; I preserved you as the "author" for those.
This PR also contains the following changes, which are not directly causing the issue, but are related:
"net/can: Release IOB when failed to queue in datahandler" <- this is a cherry pick from upstream
"net/can/can_recvmsg.c: Cleanup can_readahead" <- this just removes useless code and improves atomicity of can_readahead
Impact
Fix random crashes when using uavcan on Saluki
Testing
On-desk trials with a drone using can escs