-
Notifications
You must be signed in to change notification settings - Fork 2.1k
sys/net/gnrc/netif: hold packets after tx_sync split #21855
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
base: master
Are you sure you want to change the base?
Conversation
mguetschow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the PR, especially for the provided test! The reasoning for the changes is clear, and I can confirm the test fails on master, but succeeds with the changes.
Just some minor nitpicks about the test code :)
Co-authored-by: mguetschow <[email protected]>
Co-authored-by: mguetschow <[email protected]>
mguetschow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you! Please squash :)
Contribution description
The
gnrc_tx_sync()function currently blocks infinitely after a packet with tx_sync is sent viagnrc_netapi_send()through a legacy netdev when thegnrc_netif_pktqmodule is being used. This happens, because the tc_sync snippet users get increased and the sync snippet can not be released properly.Testing procedure
The test provided only uses nrfmin as legacy netdev. Therefore the test should only work on nrf5x devices. By adjusting the Makefile any other legacy netdev can be used.
The test starts a separate
result_threadand usesztimer_secto schedule a timeout error message after one second. As soon asgnrc_tx_sync()releases, the timeout error message gets canceled and a success message is being sent to theresult_thread.Issues/PRs references
PR #15694; PR #21709